Subject: [Id-event] AD review of draft-ietf-secevent-token-08

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 12 Apr 2018 17:22:24 -0500

Hi all,

[procedural note]
(AD Review)

This document is currently scheduled for the May 10 telechat (which
is only the second telechat from now, thanks to the IESG retreat),
but I worry that some of the issues I raise here may require
substantial rearrangement of text or other changes.  In order to
stay on that telechat, we'd need to get any required changes
published within the next 6-ish days in order to run a 2-week IETF
LC and give the IESG sufficient review time following that; if we
can't make that timeline we'll probably have to push back to a
later telechat.  Sorry that I didn't get this done sooner; I guess I
wasn't expecting to have so many comments.

IMPORTANT ISSUES

I'm worried about the content and layout of the sections covering
"Distinguishing SETs from <foo>".  We're kind of in a bad place
given that unrecognized claims are ignored and explicit typing is
not universal, but the text in Section 4.7 seems to understate the
severity of burden on profiles and deployments, in particular:
   [...] If it would otherwise be
   possible for an attacker to substitute a SET for one of these (or
   other) kinds of JWTs, then the SET profile must be defined in such a
   way that any substituted SET will result in its rejection when
   validated as the intended kind of JWT.
But a naive reading of this text would require the author of a SET
profile to know about all existing and future JWT usage in order to
satisfy the (lowercase-"must") requirement!  In practice, I think
that the difficulties can be scoped down per-issuer (and potentially
per-audience as well), so that a deployment must only know about the
uses of JWT with the same issuer in order to avoid type confusion.
With respect to the layout, in addition to not leading with the
large-scale nature of the potential issues, I also find it
surprising that similar requirements (not using "exp", in
particular) are spread across subsections.  So, my recommendation
would be to group Sections 4.5, 4.6, and 4.7 into a dedicated
subsection "Distinguishing SETs from other types", with some general
discussion in the introdcutory part.  This could note that the
problem in general is really hard (and why), and note the common
techniques of using "typ" and "events" to indicate JWT type,
separate audiences, and avoiding "exp" when ID or authentication
tokens are in play; this could also include discussion of the
strengths and weaknesses of the listed mechanisms, such as which
components (existing and/or new) need to be modified in order for
them to be effective mitigations.  Then the ID Token section could
say "because OpenID Connect uses ID tokens, when SET and OIDC are
used together, "exp" MUST NOT be used", the text about "defense in
depth" could be adjusted to the new context where avoiding "exp" is
not OIDC-specific, etc.

Section 2.1.2 claims that the "sid" claim is registered, but I did
not see it on https://www.iana.org/assignments/jwt/jwt.xhtml .  I
know I've seen it elsewhere, but who/what is actually going to cause
it to be registered?

The "Illustrative Examples" are actually the first place that
several important concepts get introduced, such as having a separate
issuer for the event and the event subject, and using non-standard
claims to identify the subject.  (I'm also not very enthusiastic
about allowing the use of non-standard claims to identify the
subject at all, but I suppose that ship has probably already
sailed.)  I think we should probably try to introduce these concepts
earlier, though I'm a little reluctant to suggest moving the
examples after the actual definitions.  Also, did we consider a
RECOMMENDED-level statement about either using top-level "sub" or
"sub" in the event payload (as opposed to other, less-standard,
mechanism)?

"jti" uniqueness:  In Section 2.2, where we define "jti", we say
that it "contains a unique identifier", and yet only that it "SHOULD
be unique within a particular event feed".  Please justify the
"SHOULD" (as opposed to "MUST") at least in this email thread, and
possibly in the document as well depending on the outcome here.
It's also traditional for us to ask whether things that must be
unique must also be unguessable/unforgeable; for signed SETs this is
probably not a concern, but since unsigned SETs are allowed, perhaps
this requires some additional thought.

"toe" precision:  Right now, we just have a parenthetical "Note that
in some cases, the represented time might be approximate", which
seems quite inadequate, given how important this will be to at least
some consumers.  IIRC the WG did consider and reject adding an
explicit precision (but that would have been a while ago, so I don't
have great faith in my recollection), but I think we should at least
give profile authors a recommendation to give additional guidance on
the precisions involved, if not give guidance ourselves here.

In the "events" definition, we only say that the it "SHOULD NOT be
used to express multiple independent logical events".  It seems
pretty likely that at least one of my fellow ADs is going to ask why
this is not a "MUST NOT", in the absence of some example case or
reasoning why it might be desirable.

"sub" uniqueness:  In Section 3 we say only that it's "NOT
RECOMMENDED for profiling specifications to use the "sub" claim in
cases in which the subject is not globally unique and has a
different issuer from the SET itself."  Similar to the previous
note, why is this not a "MUST NOT"?  (Do we also need to specify
that this is only for the "toplevel" claim of the SET, as opposed to
in an event payload?)

In Section 5, we mention in passing the use of a hash value instead
of a literal subject.  But, a plain hash is somewhat porously
reversable when the structure of the subject identifier is
well-known, like an email address -- the attacker can hash plausible
guess values and search for a match.  So we'd probably do the reader
a service by referring to a salted or keyed hash instead.

STYLE NOTES

I think we would benefit from being consistent in how we refer to an
event.  Right now we have:

(Abstract) "an event that occurred to the security subject"
("events" claim definition) "event that has occurred about a
    security subject"
[I thought there was at least one more but apparently failed to note it.]

So far the best proposal I have come up with is "an even that
occurred that involved the security subject", which is a bit awkward
with the two "that"s.

In the definition of the "events" claim, I think it would flow
better if the prohibition against duplicate identifier values was
moved after the first sentence of the next paragraph, which actually
defines that the value of the "events" claim is a JSON object with
name/value pairs, (whose names we constrain to be unique).  It's
also a little weird to follow that up with "identifiers SHOULD be
stable values", since we're talking about the "name" part and not
the "value" part; maybe "Events SHOULD be identified using stable
names"?

I was a little surprised that the SCIM example in Figure 5 did not
have a caption talking about how the subject of the SET is
identified, since "sub" is not in use.

Section 4.3 seems rather optimistic about distributed systems being
able to do an ordered counter (for "txn") the pargraph after saying
that they will have clock skew.  The problems involved in fixing
clock skew and getting a distributed unique+ordered value are pretty
similar, so I would have expected more similar language to be used
in these two pargaraphs.  Similarly, some more couched language
regarding time could be used at the bottom of the page (16), when
talking about logout time.

In Section 4.4, we talk of it being "more appropriate to cancel a
session", which sounds an awful lot like a command and not a
statement of fact.  Maybe "to scope a SET to the logout of a session
rather than a user"?

It's not clear that "MUST" is appropriate in Section 5 when talking
about "appropriate legal agreements" and user consent and such; this
text is essentially a notification to the reader that there are
legal rules in play, but we have no standing to bind the reader to
those rules.  (That's what the law is for, after all :) )

NITS

The definition of "SET" includes "that is distributed to one or more
SET recipients".  Do we really need the qualification?

In the definition of "Event Payload", missing "a" in "MUST be a
JSON object".

The SCIM example is really unclear about what a "reset attempt"
means, and uses different wording in the three locations that refer
to it (end of first paragraph, end of first bullet in "The JWT
Claims Set consists of" list, and first sentence of last pargaraph,
all on page 7).  Maybe "password reset attempts since last
successful login"?

In Section 2.1.2, the last paragph says the "SET has an empty JSON
object", but probably "empty JSON object as an event payload" is
more clear.

In the definition of the "sub" claim, the "For example, ..."
statement should probably be reformatted as a comma-separated list
in a complete sentence.

In the text after Figure 6, I'd suggest going with "integrity
protection" instead of just "integrity".

In Section 4.1, can we leave SSL in its coffin and just say "a TLS
server certificate check"?

In section 4.2, I think "without (mutual) TLS" is probably better,
since we don't a priori know whether the transmitter is TLS client
or server.

Thanks,

Benjamin

---

From: Phil Hunt <[email-masked]>
Date: Thu, 12 Apr 2018 16:39:38 -0700

Thanks Ben,

I am conferring with the co-authors and will try to respond shortly.

Phil

Oracle Corporation, Identity Cloud Services Architect
@independentid
www.independentid.com <http://www.independentid.com/>[email-masked] <mailto:[email-masked]>

> On Apr 12, 2018, at 3:22 PM, Benjamin Kaduk <[email-masked]> wrote:
> 
> Hi all,
> 
> [procedural note]
> (AD Review)
>

---

From: Phil Hunt <[email-masked]>
Date: Fri, 13 Apr 2018 15:52:44 -0700

Ben,

Thanks the thorough review. Very helpful.

Please see responses from Mike and myself in line below.  Will work on a new draft over the weekend and Monday.

Phil & Mike

> uses of JWT with the same issuer in order to avoid type confusion.

<Mike>Will think about the issuer comments some more, but for many use cases it is not the issuer distinguishes behaviors; it is the profiles of JWT that are being used that determine the behavior.  

It is not clear to us that finding ways to distinguish JWTs when necessary is a difficult burden. Each JWT profile defines claims usage and knowledge of those profiles is both necessary to implement them and can be used to implement effective means of preventing confusion. The current security considerations text is there to help profile developers think rigorously about potential problems in their context.

> depth" could be adjusted to the new context where avoiding "exp" is

<Mike> Will move sections 4.5-4.7 into a new section with some introductory text giving more context. The security considerations should probably reference the new section.

> know I've seen it elsewhere, but who/what is actually going to cause

<Phil> http://openid.net/specs/openid-connect-frontchannel-1_0.html <http://openid.net/specs/openid-connect-frontchannel-1_0.html> will register the "sid" claim.  Mike will request registration (the IANA text is there it just hasn’t been filed).
> claims to identify the subject.

<Phil/Mike> People often learn well from examples. Yes there are other ways we could structure this, but re-ordering and having it still make sense would be major surgery. The editors don’t think we should not do this given the short time before the IETF last call.

>  (I'm also not very enthusiastic

<Mike> The group did consider making “sub” recommended, but multiple use cases emerged for which “sub” would be insufficient and/or have the wrong syntax for the use case.
> this requires some additional thought.

<Mike/Phil> Agreed, will change to MUST

> the precisions involved, if not give guidance ourselves here.

<Phil/Mike>Profiles can certainly require as much precision as are needed for their use case. 

We had a number of people who had cases where they do AI type of analysis - this is why the parenthetical remark is there. An event may be suspected to have occurred based on a time estimate based on evidence rather than a specific state change.

> reasoning why it might be desirable.

<Phil/Mike> Will change to MUST NOT.

> that this is only for the "toplevel" claim of the SET, as opposed to

<Mike> Like JWT itself, SET intentionally leaves most decisions about claims usage up to the profile.  (Remember that JWTs have no no required claims at all.)  The SET spec does provide guidance to help profile writers think about how to best use existing claims, when applicable.
> a service by referring to a salted or keyed hash instead.

<Phil> Will add the word “salted” to the for example text.
> [I thought there was at least one more but apparently failed to note it.]

<Phil/Mike> This looks like an area that could use some more improved consistency (as we have been doing in the last few revisions). Will have a look at it.
> with the two "that”s.

<Phil> Will think about this. Would like a different word than involved.
> names”?

<Mike/Phil> Will take a stab at making this editorial improvement.
> identified, since "sub" is not in use.

<Phil> Will revise the example explanation text.
> talking about logout time.

<Phil> This was intended to be cautionary, not optimistic. Is there a specific change you would like to suggest?
> rather than a user”?

<Phil/Mike> Agreed, cancel sounds like a command. Will re-word to reflect that the SET represents a state change.
> those rules.  (That's what the law is for, after all :) )

<Mike/Phil> Agreed this is not normative will switch MUST for “should”.
> SET recipients".  Do we really need the qualification?

<Phil> Agreed, will delete “that is distributed…"
> JSON object”.

<Phil>Thanks
> successful login”?

<Phil> Will take a stab at cleaning this up. 
> object", but probably "empty JSON object as an event payload" is

<M/P> Agreed.
> statement should probably be reformatted as a comma-separated list

<m/P> Agreed
> protection" instead of just "integrity”.

<M/P> Agreed
> server certificate check”?

<M/P> Agreed  :-)
> since we don't a priori know whether the transmitter is TLS client

<M/P> Agreed

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 17 Apr 2018 15:09:05 -0500

> Please see responses from Mike and myself in line below.  Will work on a new draft over the weekend and Monday.

Thanks, I'll look for that.

> It is not clear to us that finding ways to distinguish JWTs when necessary is a difficult burden. Each JWT profile defines claims usage and knowledge of those profiles is both necessary to implemen…

Agreed about knowing the profile in use, but I don't think that "the
endpoints involved have agreed on a profile to be used for a given
data flow" is the whole story.  That is, I'm coming at this from the
point of view of "I have a signed SET/JWT from issuer A, that I'm
not supposed to have, and I'm an attacker.  Who can I send this
signed blob to and have them do something that's good for me (but
bad for them)?"  In this case it seems like the potential attack
surface includes all profiles for which that issuer/key might have
issued JWTs (potentially even including profiles that that issuer
does not actually use in normal operation, depending on receiver
configuration).

> <Mike> Will move sections 4.5-4.7 into a new section with some introductory text giving more context. The security considerations should probably reference the new section.

Yes, that would be a good internal reference to add, thanks.

> <Phil> http://openid.net/specs/openid-connect-frontchannel-1_0.html <http://openid.net/specs/openid-connect-frontchannel-1_0.html> will register the "sid" claim.  Mike will request registration (th…

Cool.  (Do we want an informative reference to frontchannel in our
doc as a placeholder for now?)

> <Phil/Mike> People often learn well from examples. Yes there are other ways we could structure this, but re-ordering and having it still make sense would be major surgery. The editors don’t think w…

I agree that a major restucturing is quite risky.  Would you want to
add forward-references to the later coverage of these topics, from
the descriptive text that accompanies the examples?

> <Mike> The group did consider making “sub” recommended, but multiple use cases emerged for which “sub” would be insufficient and/or have the wrong syntax for the use case.

Okay, glad to hear it was considered.

> <Phil/Mike>Profiles can certainly require as much precision as are needed for their use case. 

I don't disagree with that statement.  I am asking whether the
document itself should include a statement akin to "profiling
specifications might need to place restrictions on the precision of
this field".

> We had a number of people who had cases where they do AI type of analysis - this is why the parenthetical remark is there. An event may be suspected to have occurred based on a time estimate based…

Understood.  (I don't think we need an explicit way to indicate that
such inference was used, in this core spec.)

> <Phil/Mike> Will change to MUST NOT.

Oh, okay.  I somehow remembered this being an incredibly contentious
point within the WG and expected pushback on such a suggestion :)

> <Mike> Like JWT itself, SET intentionally leaves most decisions about claims usage up to the profile.  (Remember that JWTs have no no required claims at all.)  The SET spec does provide guidance to…

Well, if you want to try to run with it, I guess we can try.  But I
do expect an ART AD to ask again :)

> <Phil> Will think about this. Would like a different word than involved.

Thanks!

> <Phil> This was intended to be cautionary, not optimistic. Is there a specific change you would like to suggest?

Hmm, maybe

   For example, the
   "txn" claim could contain an ordered value (e.g., a counter) that the
   issuer includes, though just as for timestamps, ensuring such ordering
   can be difficult in distributed systems.

But I'm certainly open to other proposals.

Thanks again for working on this,

Benjamin

---

From: Mike Jones <[email-masked]>
Date: Wed, 18 Apr 2018 05:22:35 +0000

Thanks, Ben.  Draft -09 https://tools.ietf.org/html/draft-ietf-secevent-token-09 incorporates the changes discussed in this thread.  Replies are inline marked "Mike>".

-----Original Message-----
From: Id-event <[email-masked]> On Behalf Of Benjamin Kaduk
Sent: Tuesday, April 17, 2018 1:09 PM
To: Phil Hunt <[email-masked]>
Cc: Mike Jones <[email-masked]>; ID Events Mailing List <[email-masked]>
Subject: Re: [Id-event] AD review of draft-ietf-secevent-token-08

> > can't make that timeline we'll probably have to push back to a later 
> > telechat.  Sorry that I didn't get this done sooner; I guess I 

Mike> I did incorporate issuer language in the introduction of the new Section 4 (Preventing Confusion between SETs and other JWTs).

> It is not clear to us that finding ways to distinguish JWTs when necessary is a difficult burden. Each JWT profile defines claims usage and knowledge of those profiles is both necessary to implemen…

Agreed about knowing the profile in use, but I don't think that "the endpoints involved have agreed on a profile to be used for a given data flow" is the whole story.  That is, I'm coming at this from the point of view of "I have a signed SET/JWT from issuer A, that I'm not supposed to have, and I'm an attacker.  Who can I send this signed blob to and have them do something that's good for me (but bad for them)?"  In this case it seems like the potential attack surface includes all profiles for which that issuer/key might have issued JWTs (potentially even including profiles that that issuer does not actually use in normal operation, depending on receiver configuration).

Mike> The introduction to the new Section 4 and the text in 4.3 do describe this kind of attack and viable defenses against it.

Mike> Done

> <Phil> http://openid.net/specs/openid-connect-frontchannel-1_0.html <http://openid.net/specs/openid-connect-frontchannel-1_0.html> will register the "sid" claim.  Mike will request registration (th…

Cool.  (Do we want an informative reference to frontchannel in our doc as a placeholder for now?)

Mike> I dropped the word "registered" for now and included an informative reference to the Back-Channel Logout spec, from which the example was drawn.

> <Phil/Mike> People often learn well from examples. Yes there are other ways we could structure this, but re-ordering and having it still make sense would be major surgery. The editors don’t think w…

I agree that a major restucturing is quite risky.  Would you want to add forward-references to the later coverage of these topics, from the descriptive text that accompanies the examples?

Mike> In skimming through the examples, it's not clear to me what specific forward references would be helpful.  I'll keep thinking about this but I also don't think it's worth delaying publishing -09 over this, especially given the Telechat deadline.

> <Phil/Mike>Profiles can certainly require as much precision as are needed for their use case. 

I don't disagree with that statement.  I am asking whether the document itself should include a statement akin to "profiling specifications might need to place restrictions on the precision of this field".

Mike> I added "statements about the accuracy of this field MAY be made by profiling specifications".

> We had a number of people who had cases where they do AI type of analysis - this is why the parenthetical remark is there. An event may be suspected to have occurred based on a time estimate based…

Understood.  (I don't think we need an explicit way to indicate that such inference was used, in this core spec.)

> <Phil/Mike> Will change to MUST NOT.

Oh, okay.  I somehow remembered this being an incredibly contentious point within the WG and expected pushback on such a suggestion :)

> <Mike> Like JWT itself, SET intentionally leaves most decisions about claims usage up to the profile.  (Remember that JWTs have no no required claims at all.)  The SET spec does provide guidance to…

Well, if you want to try to run with it, I guess we can try.  But I do expect an ART AD to ask again :)

Mike> This particular claim definition has already been wordsmithed to death by the working group.  I'm very reluctant to perturb the current rough consensus by changing the wording.

Thanks!

Mike> Phil did do some reworking of the text in the abstract and the introduction to make the nomenclature more consistent.

Hmm, maybe

   For example, the

Mike> I incorporated your proposed wording.

Benjamin

> > In the definition of "Event Payload", missing "a" in "MUST be a JSON 
> > object”.
> 
> <Phil>Thanks
> > 

_______________________________________________
Id-event mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/id-event

Mike> Thanks again for your through and useful review, as always!

				-- Mike

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 19 Apr 2018 11:33:44 -0500

> Thanks, Ben.  Draft -09 https://tools.ietf.org/html/draft-ietf-secevent-token-09 incorporates the changes discussed in this thread.  Replies are inline marked "Mike>".

Thanks for the updates!  I have requested that the secretariat start
the IETF LC.

-Benjamin