Thanks for the useful review comments, Jim. Replies inline prefixed by "Mike>"...
-----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Jim Schaad Sent: Saturday, June 15, 2013 1:14 PM To: [email protected] Cc: [email protected] Subject: [jose] Comments on the web signature draft version 11 1. Going from the string to the UTF8 encoding followed by the base64 encoding in section 3.1 is difficult given that it is not really possible to figure out where the spaces, returns and new-lines are. It would be easier if you included the half-way step or better yet make the header be a single line. Mike> OK - I'll copy this intermediate step forward from the appendix where it's already present. 2. In section 3.1 rather than saying you are using ASCII, it might be better to omit the character set entirely. IMHO, having ASCII in there confuses the issue rather than clarifies it. Mike> Disagree. EBCDIC still exists, could represent this string, and uses a different encoding. The computation is dependent up the encoding, so it needs to be specified. 3. In section 3.1 s/Header.Payload.Signature/Header, Payload, Signature/ you are listing the items and the 'with' clause describes how to put in the separation character. Mike> OK 4. In section 4 para #1 - I am having an issue with the first sentence. a) Using "object(s)" to describe the possible hierarchy is messy as it might be possible to interpret this as interdependent rather than nested. Mike> "JSON object" is a term from RFC 4627 - not a statement about possible hierarchy. I could change the words "represented by" to "representing", as I think that would be clearer. Would that address the issue for you? b) 'optionally additional properties' is difficult for me to parse. I think that "and, optionally, additional properties" would be better Mike> OK 5. In section 4 para #2 - Two items, this does not talk about header parameter not defined in this document and I think it might be more understandable if this was done as a series of bullet points rather than just text. Mike> Actually, the paragraph does talk about header parameters not defined in this document: "Unless listed as a critical header parameter, per Section 4.1.10, all other header parameters MUST be ignored when not understood." Editorially, I don't think that bullets would be the best choice, because the points being made in the different sentences are not grammatically parallel. 6. In section 4.1.1 - I think the current text for rejection on alg is written on the assumption that there is only one signature/MAC on the object. If there are two and there is both an implementation and a key for one signature but the second either does not have an implementation or a key - is this still a MUST reject situation? Mike> About the multi-signature case, we could say something like: "If there are multiple signatures and/or MACs over the payload, then at least one "alg" value must be supported and have an associated key as described above, or the JWS must be rejected". Does that describe the semantics you had in mind? What does CMS do in this case? 7. In section 4.1.2 - Are you going to impose a minimum version TLS - specifically are SSL and TLS v1.0 allowed? Mike> I propose that we add this security considerations subsection, which is derived from that used in OAuth. (This language resulted from the resolution of a DISCUSS, if I recall correctly.) TLS Requirements Implementations MUST support TLS. Which version(s) ought to be implemented will vary over time, and depend on the widespread deployment and known security vulnerabilities at the time of implementation. At the time of this writing, TLS version 1.2 [RFC5246] is the most recent version, but has very limited actual deployment, and might not be readily available in implementation toolkits. TLS version 1.0 [RFC2246] is the most widely deployed version, and will give the broadest interoperability. To protect against information disclosure and tampering, confidentiality protection MUST be applied using TLS with a ciphersuite that provides confidentiality and integrity protection. Whenever TLS is used, a TLS server certificate check MUST be performed, per RFC 6125 [RFC6125]. 8. In section 4.1.2 - Is implementation of this parameter required? Mike> Only in the sense described in the second paragraph of section 4: "All other header parameters defined by this specification that are not so designated MUST be ignored when not understood." This was part of the "MUST understand" resolution proposed by Eric Rescorla, Nat Sakimura, and others and confirmed by the working group. Incomplete implementations are OK - just not fully functional. 9. In section 4.1.3 - see comment 8 Mike> Same answer as 8 10. In section 4.1.3 - Should we make a note that the private portions of a key MUST NOT be included? Mike> OK 11. In section 4.1.4 - Other than consistency - what is the requirement here for doing HTTPS - are you expecting self-signed certificates to be common and trying to get around trust anchor issues? Mike> Yes 12. In section 4.1.4 - see comment 8 -- ok - and lots of other sections Mike> Same answer as 8 13. In section 4.1.6 - There is a verification statement on X.509 certificates. There is not a similar statement for x5t or x5u. Should there be? Mike> Thanks for pointing this out. I think there should be. 14. In section 4.1.9 - This section gives why I thought that JWT was a cty value. It explicitly says so. Mike has said this is not true. Mike> I can see why the example is misleading. "cty": "JWT" is only used when the JWT representation signs or encrypts another JWT in a nested fashion - not when a set of claims are directly signed or encrypted. I'll remove it (and clarify the intent of "cty" and "typ" using language supplied by Richard). 15. In section 4.1.9 - The last paragraph states that ctyp and typ values come from the same values space. Mike has said this is not true. Mike> It is true. If it appeared that I contradicted this, I misspoke or was misunderstood. 16. In section 4.1.10 - I think there is an interesting discussion point on the use of 'crit' in the following case. I want to ensure that you implement a feature in the base specification that the base specification has said you do not need to implement. I am not sure based on the current document language if this is an empty set or not. Mike> There's a distinction that Eric Rescorla made when designing the "MUST understand" solution between fields that would result in security bugs if not fully supported versus fields that would result in the inability to process the content if not supported. For instance, the key selection fields are in the latter category. If someone doesn't support "x5t" you might not know what certificate to use and so you couldn't process the JWS, but that would just be functionality deficiency - not a security bug. "crit" is there to be able to dynamically designate new fields as being in the former category. 17. In section 5.1 step 3 - I don't know if there is a reason to make a statement about suggesting that whitespace be removed when you are planning on doing a compact representation. This is one of those things that would lead to a smaller encoding. Mike> The statement is that whitespace is permitted and no canonicalization need be performed. It doesn't say that you can't remove whitespace or perform canonicalization if you choose. 18. In section 5.1, step 4 - the phrase "If the JWS protected header is not present..." leaves me lost. I don't understand what this term is coming from as you have not yet introduced it in terms of how to build a signed message. Specifically, there has been nothing at this point which tells me how to decide what goes where. I strongly feel that section 8 needs to move before this section. Mike> The JWS Protected Header was already defined in the Terminology section. We could make this more explicit by saying that the base64url encoding of the JWS Protected Header is the value of the "protected" member in the JSON Serialization, if present. 19. In section 5.1, Step 11 - I think this should be moved forward to say - repeat the steps X to Y for each signature to be processed. I also don't think this needs to say anything about the serialization method being used. It is not relevant. Mike> I will add the language about "steps X to Y" as you suggest. Saying that this only applies for the JSON Serialization is important, since this step is inapplicable to the Compact Serialization. 20. In section 5.1, step 5 - the last sentence in this step would appear to be out of place. This is not a requirement of computing the signature, but one of building the JWS header which is step 2. This MUST is just a re-iteration of one someplace else so it does not add anything. Mike> Agreed - I'll delete this sentence. 21. In section 5.1, step 9 - I believe that the compact serialization should be moved to its own section. There should not be any apparent preference in the document for one of the serialization methods. Mike> I will create a Compact Serialization section paralleling the JSON Serialization section, but I disagree about removing the concatenation language here. One of the beautiful things about the Compact Serialization is that it *can* be described in a single sentence, as is step 9. It will be easier for most developers to continue having this single sentence in-line, rather than making them jump to another section when a single sentence in place already does the job nicely. 22. In Section 5 - I am going to make a suggestion that I would like you to consider, although I think that it might not be to your liking. My personal view of the world has been one of: There is a JSON serialization that is the object. There is a way to map from the JSON serialization to the compact serialization. The mapping requires a set of constraints on the original serialization. Thus in my world, I build the internal JSON object representation, run the computations on that internal representation and then either run the mapping to the compact serialization or serialize the JSON object representation to get the output. Using this as the method would mean that section 5 would be an object representation for JSON, section 6 would be how to do the mappings to and from the compact form, section 7 would be the how to do this. Mike> What you're doing is functionally equivalent to what's in the spec now, but pretty inefficient and unnecessary if you're only using the Compact Serialization, which will be a common model for many use cases. I think a better model is that the Compact Serialization and JSON Serialization are peers - sharing 95%+ of the processing rules and then applying a particular serialization for the data values manipulated by those processing rules. That's why the specs describe things in terms of abstract fields and bindings of those fields to elements of serializations - rather than describing things only in terms of the serialized representations. 23. In section 5.3 - Are steps #1 and #2 not the requirement of the JSON parser and not my application? I would suggest reading some of the updated text on member name string comparison from the JSON mailing list for a cleaner way to write this section. Mike> Specific text suggestions welcomed. 24. Section 6 appears to be extraneous and could be deleted. What is the purpose that this is supposed to serve? Why is that purpose not covered in the description of the alg member? Mike> I agree. FWIW, Richard privately made the same suggestion about the parallel section in JWE, which I also agree with. 25. Section 7 would appear to be better served by making it section 4.1.2 and making section on jku, jwk, x5u, x5t, x5c and kid as subjections of it. Mike> The problem with this suggestion is that it would make the list of reserved header parameters no longer syntactically parallel. And I do believe there's value in having a paragraph that talks about the semantics of key identification all-up, rather than trying to do it in a manner that's scattered in bits and pieces into the individual parameter definitions. 26. Section 8 - I found the presentation confusing. I would have expected that it would be made in the normal manner - that is you have an object and a list of members each with its own section - and those members may have sub members which are listed. There would then be references back to the members defined in section 4 Mike> I agree that an object-centered approach will be easier to follow. Thanks for suggesting it. Some of that content is now in prose. I'll restructure to use lists/tables instead as you suggest. 27. Section 9 - I would dispute the content of this section vigorously. Mike> Hannes and Derek and Stephen and Sean, at least to the OAuth WG, have made it VERY CLEAR that the IESG requires MTI features to be defined. Of the JSON and Compact serializations, the latter is the only one in widespread use, therefore it makes sense for it to be the MTI serialization. Particularly since JWTs mandate that only the Compact Serialization be used, it would make no sense from a working group coordination point of view to try to mandate that the JSON serialization also be MTI - especially since it's more complicated. I stopped reviewing after section 9. Mike> Thanks for all the useful comments. -- Mike Jim _______________________________________________ jose mailing list [email protected]<mailto:[email protected]> https://www.ietf.org/mailman/listinfo/jose
_______________________________________________ jose mailing list [email protected] https://www.ietf.org/mailman/listinfo/jose
