Subject: Re: [jose] Pete Resnick's Abstain on draft-ietf-jose-json-web-signature-37: (with COMMENT)

From: Mike Jones <[email-masked]>
Date: Wed, 10 Dec 2014 02:57:26 +0000

Thanks for taking the time to provide these comments, Pete.  Descriptions of the actions that were taken in the -38 drafts in response to them are included inline below.

> -----Original Message-----
> From: Pete Resnick [mailto:[email-masked]]
> Sent: Tuesday, December 02, 2014 8:37 PM
> To: The IESG
> Cc: [email-masked]; draft-ietf-jose-json-web-
> [email-masked]
> Subject: Pete Resnick's Abstain on draft-ietf-jose-json-web-signature-37:
> (with COMMENT)
> 
> Pete Resnick has entered the following ballot position for
> draft-ietf-jose-json-web-signature-37: Abstain
> 
> When responding, please keep the subject line intact and reply to all email
> addresses included in the To and CC lines. (Feel free to cut this introductory
> paragraph, however.)
> 
> 
> Please refer to http://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> http://datatracker.ietf.org/doc/draft-ietf-jose-json-web-signature/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> I've got some suggestions for improvements below, but overall I cannot
> support this document, so I'm entering an ABSTAIN. I don't think this WG has
> actually fulfilled its charter with regard to this document set. The WG was
> supposed to produce a "JSON syntax that can be used by applications to
> describe secure data objects". I don't believe it did that. Instead, it produced
> a compact serialization for a particular purpose and then backed into a JSON
> syntax. The compact serialization drove the effort and produced IMO a
> substandard JSON serialization. I don't believe that (reliable) interoperable
> implementations of the JSON serialization will be possible using this
> document set. However, there is at least rough consensus that these
> documents are usable as-is, and I don't think there's anything to be gained by
> holding them up any further.
> 
> I hope the WG will adopt some of the remaining comments, but my intention
> is not to discuss this any further. If you wish to address any of the comments
> and need clarification, I'm glad to help with that.
> 
> ------
> 
> I really wish you would have factored out what's common between JWS and
> JWE so you didn't have so much repeated text.

There used to be a lot of duplicated text, but a concerted effort was made some time ago to do exactly as you asked.  I'll cite examples of the improvements made if you want.  I know that people would be open to more changes that reduce duplication (subject to maintaining readability), should specific changes be proposed.  But I also know that you've stated that you don't want to discuss this anymore, so this may be water under the bridge...

> Throughout: I thought you were changing all "recipient"s to "consumer"s.
> I haven't changed them below, but you might want to do a proper search for
> them.

The word "recipients" is part of the normative JWE JSON Serialization syntax (see https://tools.ietf.org/html/draft-ietf-jose-json-web-encryption-37#section-7.2.1) and so changing this terminology everywhere would result in a breaking change, which seems unnecessarily heavy-handed at this point.  Also, as background, the term “recipient” came from CMS [RFC 3852], in which it occurs over 100 times, and in which the term “consumer” doesn’t appear.  Therefore, I've lightly edited the docs to use "producer" in contexts that we were otherwise talking about "consumers", and have replaced uses of the term "sender", but have not made the wholesale terminology switch.

> 1: Lose the last paragraph. It's not a core goal. Certainly not in the charter.

Compactness is a core goal, which is reflected in the charter using the language "The second will be a smaller serialization that can be
used in URLs".

> 2. A "JSON Web Signature Signature" is an unfortunate terminology choice.
> If
> there is a whole *thing* that is the combination of the header, the signature,
> and the payload, perhaps that should have been a "JOSE Web Object", or
> better yet a "JOSE Object". Then you could simply have "JOSE Signature",
> "JOSE Payload", and "JOSE Header".

A JWS is syntactically and semantically different from a JWE, which is why they're called different things.  In the cases were things are actually the same, such as the JOSE Header, we *did* unify the terminology as of the -29 drafts, which used use distinct terms.

> I'm also unclear why the word "Web"
> appears in each of these things -- except "JOSE Header", which I find equally
> strange.
> They should not just be for use on the Web. This whole "JW*" terminology
> thing is goofy.

The etymology of the term "JSON Web Token" is that the "JSON Web Token" was conceived of as a JSON-based replacement for the "Simple Web Token".  The relationship between JWTs and SWTs is described in Appendix C of the JWT spec.  When the signature encoding was split out of the JWT spec, the natural name was "JSON Web Signature".  The JWE, JWK, and JWA names soon followed.

> 3. "JWS object" is undefined.

Thanks - I replaced uses of this phrase with simply "JWS".  Likewise for "JWE object" and "JWK object".

> Please reverse the order of section 3.1 and 3.2.

The order is purposeful.  The simple serialization is intentionally presented before the more general, more complicated serialization.

> 3.1: Failure to have an unprotected header in the compact serialization
> means that these things are not round-trippable. That's very unfortunate
> and I think a failure of the protocol.

As Jim Schaad wrote earlier, this was a conscious design decision of the working group.

> 3.2:
> 
> OLD
>    In the JWS JSON Serialization, a JWS object is represented as the
>    combination of these four values,
>       BASE64URL(UTF8(JWS Protected Header)),
>       JWS Unprotected Header,
>       BASE64URL(JWS Payload), and
>       BASE64URL(JWS Signature)
> 
> NEW
>    In the JSON Serialization, the JWS object is represented as a JSON
>    object with 3 members:
> 
>       header, whose value is JWS Shared Unprotected Header
>       protected, whose value is BASE64URL(UTF8(JWS Protected Header))
>       payload, whose value is BASE64URL(JWS Payload)
>       signature, whose value is BASE64URL(JWS Signature)
> 
> (This doesn't account for multiple signatures, and I'd wish you'd clarify what
> the structure actually looks like, but this is at least better.)

Done.  Thanks for the suggestion.

> Also, there is no need for the Payload to be (a) part of the serialization and
> (b) base64ed.

See http://tools.ietf.org/html/draft-ietf-jose-json-web-signature-37#appendix-F for the detached content syntax.  See http://trac.tools.ietf.org/wg/jose/trac/ticket/26 for the issue resolution on the payload being base64url encoded.

> 3.3:
> 
> OLD
>    The following example JWS Protected Header declares that the encoded
>    object is a JSON Web Token (JWT) [JWT] and the JWS Protected Header
>    and the JWS Payload are secured using the HMAC SHA-256 [RFC2104, SHS]
>    algorithm:
> 
> There's a couple of typos in there. I think you meant:
> 
> NEW (1)
>    The following example JWS Protected Header declares that the encoded
>    object is a JSON Web Token (JWT) [JWT] and the JWS Payload is secured
>    using the HMAC SHA-256 [RFC2104, SHS] algorithm:
> 
> However, I'm not clear why that's an example of a *Protected* Header.
> Looks
> like it could be any old header. So I think better is:
> 
> NEW (1)
>    The following example JOSE Header declares that the encoded object is
>    a JSON Web Token (JWT) [JWT] and the JWS Payload is secured using the
>    HMAC SHA-256 [RFC2104, SHS] algorithm:

Both the protected header and the payload are included in the signature input, hence the present wording.

> Then you would fix:
> 
> OLD
>    Encoding this JWS Protected Header as BASE64URL(UTF8(JWS Protected
>    Header)) gives this value:
> NEW
>    If this were a JWS Protected Header, the encoding of
>    BASE64URL(UTF8(JWS Protected Header)) gives this value:

The conditional language isn't appropriate because this section presents a specific, concrete example value - one in which the JWS Protected Header is used.

> Also:
> 
>    Concatenating these values...
> 
> This section is not about the compact serialization.

No, it's not about the compact serialization, but the particular example value presented uses the compact serialization.

> If you want to give both
> example serializations in this section, fine, but if you are giving a general
> "Example JWS" as the title of the section states, don't just give the compact.

The desire for ready access to JSON Serialization examples was addressed during IESG review by adding this text at the end of the example section:

"See Appendix A for additional examples, including examples using the JWS JSON Serialization in Sections A.6 and A.7."

It was handled this way, rather than by adding more examples up front, to keep the up-front example section reasonably brief.  It's there to give the reader a quick initial example of how JWSs work - not to be a tutorial on all the possible combinations of features and representations.

> 4.1.1/4.2: I see no need to mention that the alg could be a "Collision-
> Resistant Name" (what a term!). The alg should be registered.
> If it's not, you're in private agreement space anyway, so it needn't be
> specified in the spec. Same thing for Public Header Parameter Names: If
> you're going to do this interoperably, you're going to have to know what the
> thing means; otherwise, it's out of band anyway. I say get rid of the whole
> concept of using non-registered names.

The current collision-resistant namespace text is the result of discussions of http://trac.tools.ietf.org/wg/jose/trac/ticket/102 and http://trac.tools.ietf.org/wg/jose/trac/ticket/66.    This construct was also discussed in the context of Stephen Kent's SECDIR review of the JWK spec.

> 4.1.6:
> 
> This is a section where you did not clean up the "reject" language. Here are
> my suggested changes:
> 
> OLD
>    The recipient MUST validate
>    the certificate chain according to RFC 5280 [RFC5280] and reject the
>    signature if any validation failure occurs.
> NEW
>    The recipient MUST validate
>    the certificate chain according to RFC 5280 [RFC5280] and consider
>    the signature invalid if any validation failure occurs.

Thanks - done

> 4.1.11:
> 
> This is a section where you did not clean up the "reject" language. Here are
> my suggested changes:
> 
> OLD
>    If any of the listed extension Header
>    Parameters are not understood and supported by the recipient, it MUST
>    reject the JWS.
> NEW
>    If any of the listed extension Header
>    Parameters are not understood and supported by the recipient, it MUST
>    consider the JWS signature invalid.

This is a case where the JWS must be rejected due to the implementation not providing sufficient functionality whether the signature is valid or not.

> 5.1: s/MUST perform/performs

Done

>   2. BASE64URLing the payload shouldn't be required.

See http://trac.tools.ietf.org/wg/jose/trac/ticket/26 for the issue resolution on the payload being base64url encoded.

>   3. Replace everything after the colon with "the JWS Protected Header
>   and/or the JWS Unprotected Header, as appropriate."

That wouldn't be parallel to the treatment of the preceding list.

>   4. Strike the parenthetical "(which can only happen...)". Sounds like you are
>   discouraging use of the JSON Serialization and the Unprotected Header.

As previously discussed - it's not pejorative - it's intended to be informative to implementers.

>   5. I don't see a need for the second sentence. Delete.

Jim Schaad had previously explicitly asked that it be added.  And saying that the ordering is only partial clarifies the intent of the text.

>   8. Replace with "Create the desired serialized output, as described in
>   Section 7." Stop privileging Compact.

As previously discussed, the Compact Serialization is (intentionally!) simple enough it can be fully described in the single sentence.  The same cannot be said of JSON Serialization, hence the need for the section reference in that case.

> 5.2: s/MUST be taken/are performed.

Done

> I made an attempt to untangle the serialization from the actual algorithm. I
> failed. I think this is likely to be hard to implement strictly from the spec.

There are over 20 known interoperable implementations, and likely many more at this point that haven't been widely publicized.

> 10.12:
> 
> This is a section where you did not clean up the "reject" language. Here are
> my suggested changes:
> 
> OLD
>    In particular, any JSON inputs not
>    conforming to the JSON-text syntax defined in RFC 7159 input MUST be
>    rejected in their entirety.
> NEW
>    In particular, any JSON inputs not
>    conforming to the JSON-text syntax defined in RFC 7159 input MUST be
>    treated as invalid in their entirety by JWS consumers.
> OR
>    In particular, any JSON inputs not
>    conforming to the JSON-text syntax defined in RFC 7159 input MUST be
>    rejected in their entirety by JSON parsers.

Done

> OLD
>    Such input MUST be rejected in its
>    entirety.
> NEW
>    JWS consumers MUST treat the signatures of such input as invalid.

It's not the signature that's invalid - it's the input syntax.

> Appendix E:
> 
> s/reject/considered invalid/g

Anything with invalid or not understood syntax really does need to be rejected in its entirety.  There's nothing safely usable in the input if you can't parse it.

				Thanks again, Pete!
				-- Mike