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

From: Mike Jones <[email-masked]>
Date: Wed, 15 Oct 2014 07:12:27 +0000

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 <[email-masked]>
> To: [email-masked]
> Cc: [email-masked]
> 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

---

From: Barry Leiba <[email-masked]>
Date: Wed, 15 Oct 2014 08:54:31 -0400

> 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.

Ray is looking at the HTML generated by tools.ietf.org, which creates
the HTML by analyzing the text, not by looking at the markup.  When
the section name is separated from the reference, it can't figure it
out, and assumes that the section reference is to the current
document.  So, in JWE, for example, this:

   the initial contents of this registry are the values defined
   in Section 4.1 of the JSON Web Algorithms (JWA) [JWA] specification.

...doesn't work, and makes "Section 4.1" link to Section 4.1 of the
current document, JWE, and then gives a link to the top of JWA at
"[JWA]".

You can make it work as desired by doing this:

OLD
   the initial contents of this registry are the values defined
   in Section 4.1 of the JSON Web Algorithms (JWA) [JWA] specification.
NEW
   the initial contents of this registry are the values defined
   in the JSON Web Algorithms specification (JWA) [JWA], Section 4.1.
END

That will cause "[JWA], Section 4.1" to link as you'd expect.

(I also suggest that you don't need "(JWA) [JWA]" and can just use
"[JWA]" there, but perhaps that is an artifact of your markup tool.)

There are many places where this is an issue -- it's a tools issue,
not a document issue.  If you're willing to "fix" this in the
documents, please go through and try to find all the places where that
sentence structure is used.

Barry