Subject: Re: [Unbearable] Review by Michael Jones of draft-ietf-tokbind-https-08

From: =JeffH <[email-masked]>
Date: Mon, 27 Mar 2017 12:42:04 -0700

> IMPORTANT:
 >
 > Section 5.2:  Change “the Token Binding ID in the identity token” to
 > “the Token Binding ID (or a cryptographic hash of it) in the identity
 > token”.
 >
 >
 >
 > Section 5.2:  Change “include in the token the Token Binding ID” to
 > “include in the token the Token Binding ID (or a cryptographic hash
 > of it)”.

done

see: https://github.com/TokenBinding/Internet-Drafts/pull/96

 > NITS:
 >
 >
 >
 > Author list:  Change “Paypal” to “PayPal”.

thx :)  done

 >
 > Abstract:  When editing other specs to become RFCs, IESG reviewers
 > told me that references must not be present in the abstract, because
 > it will be used in contexts such as references about the document,
 > which the document itself is not present.  Therefore, delete
 > [RFC5246] and [I.D.ietf-tokbind-protocol] from the abstract, while
 > keeping the spec names that precede those references.

done.

 > Abstract:  Delete the underscores around “first-party”, “federated”,
 > and “different”.  It’s not clear why they are there and they are very
 > distracting when reading the text.  Likewise, delete underscores used
 > in this way other places in the draft, such as in the introduction.

done.

 > Introduction: Add a comma after “e.g.”.  This needs to be done in
 > other places as well.

done

 > Introduction:  Add an informative to reference to [RFC6749] upon
 > first use of the term “OAuth”.

done

 > Section 2:  Change “pad characters” to “padding characters”.

done

 > Section 2:  Change “Section 4.2.  "Server Processing Rules"” to
 > Section 4.2 (Server Processing Rules)”.  Make similar parallel
 > changes where applicable.

actually, the style the RFC-editor employed in RFC6797 is like so..

   Section 12.1 ("No User Recourse")

..ie, a combination of the two styles.

So i did that.

 > Section 2.1:  Delete the comma in “various application protocols, and
 > application contexts”.
 >
 >
 >
 > Section 2.1: “Add a hyphen to “general purpose”.
 >
 >
 >
 > Section 2.1, paragraph 2:  Change “(below)” to a section reference.
 > Also, this sentence is torturously complicated.  Break it into two or
 > three concise sentences.
 >
 >
 >
[...]
 >
 >
 >
 > Section 4 title:  Change “First-party” to “First-Party”.
 >
 >
 >
 > Section 4:  Add a comma between “misuse” and “since”.
 >
 >
 >
 > Section 4:  Change “[I-D.ietf-tokbind-protocol] Section 5” to
 > “Section 5 of [I-D.ietf-tokbind-protocol]”.
 >
 >
 >
 > Section 5.1:  Change “OpenID Connect identity token” to “OpenID
 > Connect ID Token”.
 >
 >
 >
 > Section 5.1:  Add an informative reference to OpenID Connect upon
 > first use of the term.  The correct reference text is supplied at the
 > end of this review.
 >
 >
 >
 > Section 5.1:  Add a comma after “In this section”.
 >
 >
 >
 > Section 5.1:  Add a comma after “Below” in “Below we specify”.
 >
 >
 >
 > Section 5.2:  Change “(where the identity token is called "ID
 > Token")” to “(in which the identity token is called an "ID Token")”.
 > Also change the “(where” to “(in which” in the following SAML
 > clause.
 >
 >
 >
 > Section 5.2:  Change the colon to a semicolon in “said client can use
 > the identity token: The Relying Party will”.
 >
 >
 >
 > Section 5.2:  Change “reveals” to “uses”.
 >
 >
 >
 > Section 5.2:  Add a comma after “at the very least”.
 >
 >
 >
 > Section 5.2:  Change “redirect from Token Consumer to Token Provider”
 > to “redirect from the Token Consumer to the Token Provider”.
 >
 >
 >
 > Section 5.3:  Change “a” to “an” in “it SHOULD include a
 > Include-Referred-Token-Binding-ID”.
 >
 >
 >
 > Section 5.3:  Change “[RFC7231] Section 8.3” to “Section 8.3 of
 > [RFC7231]”.
 >
 >
 >
 > Section 5.3:  Change “(see above)” to “as described in <xref
 > target=”SECTION”/>”.
 >
 >
 >
 > Section 5.3:  Put commas around “for some deployment-specific
 > reason”.
 >
 >
 >
 > Section 5.5:  Remove the quotation remarks around "ID Token".
 >
 >
 >
 > Legend:  Change “the "ID Token" in OIDC,” to “the ID Token in OpenID
 > Connect;”.
 >
 >
 >
 > Legend:  Change “Open ID Connect” to “OpenID Connect”.
 >
 >
 >
 > Section 6:  Change “See Section 8 "Privacy Considerations"” to “See
 > Section 8 (Privacy Considerations)”.
 >
 >
 >
 > Section 7.3:  Change the colon after “moot” to a period or
 > semicolon.
 >
 >
 >
 > Section 7.3:  Change the colon after “field” to a period or
 > semicolon.
 >
 >
 >
 > Section 7.3:  Change “trick C to send” to “trick C into sending”.
 >
 >
 >
 > Section 7.3:  Add a comma after “as if C controls a certain key
 > pair”.

all above done.

 > Section 7.3:  Change “_javascript_” to “_javascript_”.  (This
 > correction needs to occur in other locations as well.)

do not understand.

 > Section 7.4:  Add a comma after “in a federated sign-in scenario”.
 >
 >
 >
 > Section 7.4:  Delete the semicolon in “Token Provider; or it can”.
 >
 >
 >
 > Section 7.4:  Change “as we do above” to “as is done above”.
 >
 >
 >
 > Section 7.4:  Delete the comma in “message sent by the client to the
 > Token Consumer, and any message”.
 >
 >
 >
 > Section 7.4:  Delete the comma in “controls the redirect URL, and can
 > tamper”.
 >
 >
 >
 > Section 7.4:  Change “trick the Token Provider to issue” to “trick
 > the Token Provider into issuing”.
 >
 >
 >
 > Section 7.4:  Change “can not” to “cannot”.
 >
 >
 >
 > Section 8.1:  Change “[I-D.ietf-tokbind-protocol] section 4.1” to
 > “Section 4.1 of [I-D.ietf-tokbind-protocol]”.
 >
 >
 >
 > Section 8.2 title:  Change “Life Time” to “Lifetime”.
 >
 >
 >
 > Section 8.2:  Change “across an extended period of time” to “for an
 > extended period of time”.
 >
 >
 >
 > Section 8.2:  Change “web” to “Web”.  Likewise, do this in other
 > places that the proper name “Web” is not capitalized.
 >
 >
 >
 > Section 8.2:  Change “life time” to “lifetime”.
 >
 >
 >
 > Section 8.3:  Delete the comma in “communicating endpoints, that
 > receive”.
 >
 >
 >
 > Section 8.3:  Delete the comma in “other than their own, obtain”.
 >
 >
 >
 > Section 8.3:  Change “, see Section 6 "Implementation
 > Considerations"” to “, per Section 6 (Implementation
 > Considerations)”.

above done.

 > Finally, I’ll say that there are a lot of tortured uses of long
 > parenthetical phrases in this spec.  At the very least, in cases in
 > which the parentheses enclose a complete sentence or sentences,
 > separate the parenthetical phrase from the containing sentence.  For
 > instance, 8.3 contains this text:
 >
 > An application's various communicating endpoints, that receive Token
 > Binding IDs for TLS connections other than their own, obtain
 > information about the application's other TLS connections (in this
 > context, "an application" is a combination of client-side and
 > server-
 > side components, communicating over HTTPS, where the client side may
 > be either or both web browser-based or native application-based).

 > At the very least, free the parenthetical sentences, as follows:

 > An application's various communicating endpoints, that receive Token
 > Binding IDs for TLS connections other than their own, obtain
 > information about the application's other TLS connections.  (In this
 > context, "an application" is a combination of client-side and
 > server-
 > side components, communicating over HTTPS, where the client side may
 > be either or both web browser-based or native application-based.)

done where it seemed to make sense.

 > The reference to use for OpenID Connect <xref target="OpenID.Core"/>
 > is:

done