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 protected]> > To: [email protected] > Cc: [email protected] > 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 _______________________________________________ jose mailing list [email protected] https://www.ietf.org/mailman/listinfo/jose
