Re: [jose] AppsDir reviews of draft-ietf-jose-json-web-signature-32

[Mike Jones <[e-mail]>]

Thanks for your review Ray.  My apologies for not responding to it until now.  It had gotten sorted into a mail folder and I hadn't seen it until Kathleen brought it to my attention.  Responses are inline below...

> Date: Wed, 1 Oct 2014 20:21:42 -0700 (PDT)
> From: Ray Polk <[e-mail]>
> To: [e-mail]
> Cc: [e-mail]
> Subject: Re: URGENT AppsDir reviews of the JOSE document set - asigned drafts
> 
> Hi Claudio (and Mike),
> 
> I've finished reviewing draft-ietf-jose-json-web-signature-32 for AppsDir.  I could
> not find another AppsDir review on the jose mailing list to use as a model.  So, I
> don't know to whom I should send my review, the format it should take, or the
> severity of the issues to include (include Nits?  include minor, non-blocking
> issues?).
> 
> For now, I'll include all of my notes.  If you can advise me of proper
> format/protocol/procedure, I'll craft an email to the jose list.
> 
> Major:  None
> 
> Minor:
> 
> 4.1.1. & 4.1.2. The links to Section 4.1 and Section 5.1 of JWA are incorrect.
> They link to JWE instead of JWA.
> 
> In 4.1.1. the link is:
> https://tools.ietf.org/html/draft-ietf-jose-json-web-encryption-32#section-4.1
> ...but it should be:
> https://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-33#section-4.1
> 
> In 4.1.2. the link is:
> https://tools.ietf.org/html/draft-ietf-jose-json-web-encryption-32#section-5.1
> ...but it should be:
> https://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-33#section-5
> (JWA doesn't seem to have an anchor for 5.1)

These link URLs are actually created by the IETF tools - not in the draft itself.  (You'll see "Html markup produced by rfcmarkup 1.109, available from https://tools.ietf.org/tools/rfcmarkup/" at the bottom of the drafts.)  I'm not sure who to file a bug on this with.

> 9. saying "separated by X period ('.') characters" is ambiguous:
> 
> JWSs have three segments separated by two period ('.') characters.
> This means:  segment..segment..segment
> 
> JWEs have five segments separated by four period ('.') characters.
> This means:  segment....segment....segment....segment....segment
> 
> Say instead:  ___s have X segments.  Each segment is separated from the next by
> a single period ('.') for a total of X-1 delimiting periods ('.').

Thanks - I'll plan to make this correction.

> Nit:
> 
> 3.2 change "of these eight values," to "...values:", remove commas and the
> 'and', change "...with the six" to a complete sentence.

The "values" correction is already present in the -34 draft.  I agree that not trying to include the list in a sentence structure would make it easier to read.

> 3.3 remove the and from "...to produce the JWE Encrypted Key and" and the
> period from the next bullet

OK

> 4.1.3. fix comma splicing in:  "This Header Parameter MUST be integrity
> protected, and therefore MUST occur only within the JWE Protected Header,
> when used."  For example, "When used, this Header Parameter MUST be
> integrity protected; therefore, it MUST occur only within the JWE Protected
> Header."

OK

> Sections 4.1.4. through 4.1.10. are almost entirely redundant.  Combine them
> like so:
> 
> The following parameters have the same meaning, syntax, and processing rules
> as those defined in JWS, except that the certificate referenced by the thumbprint
> contains the public key to which the JWE was encrypted; this can be used to
> determine the private key needed to decrypt the JWE.
> 
> jku defined in Section 4.1.2. of [JWS]
> jwk defined in Section 4.1.3. of [JWS]
> etc.

I understand this suggestion but disagree because there's value to implementers and other readers to having each of the header parameters listed as a section header in the table of contents.  It makes it easy to see all of them in one place.  Combining them would lose this benefit.

				Thanks again,
				-- Mike