Re: [Id-event] Review of draft-ietf-secevent-subject-identifiers-05 by Michael Jones

["Richard Backman, Annabelle" <[e-mail]>]

Thanks for the review, Mike. I had a few follow up questions [richanna] inline [/richanna].

–
Annabelle Backman (she/her)
AWS Identity
https://aws.amazon.com/identity/


From: Id-event <[e-mail]> on behalf of Mike Jones <Michael.Jones=[e-mail]>
Date: Thursday, July 9, 2020 at 8:55 PM
To: "[e-mail]" <[e-mail]>
Subject: [EXTERNAL] [Id-event] Review of draft-ietf-secevent-subject-identifiers-05 by Michael Jones


CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.


I did a cover-to-cover read of draft-ietf-secevent-subject-identifiers-05.  My resulting review comments follow, grouped into substantive and editorial sections.

SUBSTANTIVE COMMENTS

Abstract – Remove [JSON], [JWT], and [SET] references.  Past IESG reviews of other specs have told me that citations are not allowed in abstracts, since they need to stand alone without the rest of the specification.

2.  Notational Conventions

Copy https://tools.ietf.org/html/draft-ietf-secevent-http-poll-12#section-1.2 and make it a Definitions or Terminology sub-section under the current Notational Conventions section.

3.  Subject Identifiers – The language “Every Subject Identifier Type MUST have a unique name registered…” is surprisingly different that the corresponding JWT spec language with also allows Private identifiers and Collision-Resistant identifiers.  Why not also allow them here?

[richanna]
Funnily enough, this exact thing came up on the OIDF SSE call this week. 😃
[/richanna]

3.  Subject Identifiers – The term “payload claims” is confusing.  Please change this term to “Subject Identification Claims” throughout the specification.

[richanna]
Agreed on not using “payload claims”. Replacement is TBD, but I think we can and should avoid the word “claim” here, since that is a very specific thing in the JWT world.
[/richanna]

3.1. Account Subject Identifier Type – Figure 1 – This figure is confusing because it makes it seem like “subject_type” and “uri” are top-level JWT claims.  Please correct this possible confusion by adding "sub_id": to the example.  The updated example would look like this:

   "sub_id": {
     "subject_type": "account",
     "uri": "acct:[e-mail]",
   }

Also add "sub_id": to all other examples, such as those in 3.2, 3.3, 3.4, etc.

[richanna]
Subject Identifiers can occur outside of JWTs, and not only as the value of the JWT “sub_id” claim. Adding context representing one use case for Subject Identifiers makes these examples unnecessarily complicated. I suggested on Phil’s review that we could add examples of Subject Identifiers being used in context (e.g., as a “sub_id” claim or embedded within a SET event payload) earlier in the doc, and move away from using “claim” to refer to the members within a Subject Identifier JSON object. Would that address your concern?
[/richanna]

3.2.1 Email Canonicalization – Please add a note that the domain portion of an RFC 5322 addr-spec already must be treated in a case-insensitive manner, per RFC 1034.

[richanna]
It strikes me as weird to pull this into the doc. If we changed the second sentence of 3.2.1 to specify the local part, e.g., “some providers treat the local part of email addresses as case-insensitive” would that address your concern?
[/richanna]

3.3. Phone Number Subject Identifier – Please change all instances of “phone-number” to “phone_number” to match the JWT claim of the same name.  This occurs multiple times throughout the specification.

3.4. Issuer and Subject Subject Type Identifier – Add a note that this is the way that subjects are identified in OpenID Connect ID Tokens.

5.1. Identifier Correlation – Add this phrase to the end of the last sentence “or when correlation is essential to the use case”.

6. Security Considerations – Talk about integrity-protecting SETs.  Borrow or reference security considerations from the other SecEvent specs.

7.1. Security Even Subject Identifier Types Registry – Please add a suggestion to IANA that this registry be located at https://www.iana.org/assignments/secevent/.

7.1.1. Registration Template – Add underscore “_” to the set of acceptable characters.

7.1.1. Registration Template – Change Controller – Remove OpenID statements and replace them by IETF specifications and “IETF”.

7.1.1. Registration Template – Defining Document(s) – Also add “prohibited” to the list of conditions to be described.

7.1.3. Guidance for Expert Reviewers – State that multiple Expert Reviewers should be appointed.  You can borrow language about that from RFC 7519..

7.2. JSON Web Token Claims Registration – Change “established by [SET]” to “established by “[JWT]”.

Acknowledgements – Also acknowledge the SecEvent working group and its members.

EDITORIAL COMMENTS

1. Introduction – Change “section 1.2” to “Section 1.2” (and similarly change to Title Case for any other occurrences of “section”, “figure”, or “table”).

1. Introduction – Add a comma after “but not limited to”.

1. Introduction – Add a comma after “for example”.

2.  Notational Conventions – Update to use RFC 8417 language.

3.  Subject Identifiers – Change “which” to “that” in “which are to be interpreted”

3.1. Account Subject Identifier Type – Figure 1 – It’s odd to have a period ending a sentence fragment.  Delete the period at the end of the figure caption.  Do this for all other figures when the caption is also a sentence fragment, such as Figure 2, Figure 3, Figure 4, etc.

4.1. "sub_id" Claim – Change "sub_id" to <spanx style="verb">sub_id</spanx> in the source so that the formatting is correctly applied across all output representations.  Similarly, use spanx style verb for all other instances of literals in the specification.

4.1. "sub_id" Claim – Likewise, change `sub` to <spanx style="verb">sub</spanx> and `sub_id` to <spanx style="verb">sub_id</spanx>.  Search for other similar instances of uses of " ' or ` in the spec and correct them to spanx style verb.

4.2. “sub_id” and “iss-sub” Subject Identifiers – This section seems to repeat information in Section 3.4 (Issuer and Subject Subject Identifier Type).  Please consolidate them.

8.1. Normative References – IANA.JWT.Claims – Delete the odd text “n.d.,”.

8.1. Normative References – Choose one of [JWT] or [RFC7519] and delete the other one.

8.1. Informative References – Change [OIDC] to the standard reference name [OpenID.Core].

                                                       -- Mike