Thanks for the useful review, Warren. I’m cc’ing the working group so they’re aware of the contents of your review. Replies inline below…
-----Original Message----- From: Warren Kumari [mailto:war...@kumari.net] Sent: Monday, September 01, 2014 3:40 PM To: sec...@ietf.org; draft-ietf-oauth-json-web-token....@tools.ietf.org Subject: Review of: draft-ietf-oauth-json-web-token Be ye not afraid -- I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other last call comments. Disclaimer: I know next to nothing about JOSE. In reading this document I also went off and read some other JOSE work / WG documents. The main thing that I learnt was that them thar JOSE folk sure do like their acronyms.. :-) My unfamiliarity with JOSE means that, unlike what the above boilerplate says, you should treat these less seriously than any other last call comments! Summary: Needs some work, nothing major. Notes: In a number of places the document says things like: "If any of the listed steps fails then the JWT MUST be rejected for processing." - does it actually *mean* to reject a JWT? What should an application do when it rejects a JTW (yes, I realize that this is somewhat application specific, but a general "Explode, killing everybody inside" vs "Simply pretend you didn't notice this" would be helpful). As you point out, what it means to reject the JWS is actually application specific, so it’s not clear what else to say in this regard in the specification. I suppose that we could say that it must be rejected by the application and leave it at that. Would that work for you? I'm a little confused by something in the Terminology section (Section 2): Plaintext JWT A JWT whose Claims are not integrity protected or encrypted. The term plaintext to me means something like "is readable without decrypting / much decoding" (something like, if you cat the file to a terminal, you will see the information). Integrity protecting a string doesn't make it not easily readable. If this document / JOSE uses "plaintext" differently (and a quick skim didn't find anything about this) it might be good to clarify. Section 6 *does* discuss plaintext JWTs, but doesn't really clarify the (IMO) unusual meaning of the term "plaintext" here. I’ve discussed this with the other document editors and we agree with you that “plaintext” is not the most intuitive wording choice in this context. Possible alternative terms are “Unsecured JWT” or “Unsigned JWT”. I think that “Unsecured JWT” is probably the preferred term, since JWTs that are JWEs are also unsigned, but they are secured. Working group – are you OK with this possible terminology change? (Note that the parallel change “Plaintext JWS” -> “Unsecured JWS” would also be made in the JWS spec.) MACed does not seem to be a well known term - surprisingly enough even MAC doesn't have an asterisk at https://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt Do you have another suggestion to replace “MACed” and “MACing”, other than verbose formulations like “that have a MAC applied to them”? Given that in English usage it’s common to “verb a noun” (e.g., usage of the verb “Google”), I don’t think there’s actually any ambiguity as to the intended meaning. Section 4: "... recipients MUST either reject JWTs with duplicate Claim Names or use a JSON parser that returns only the lexically last duplicate member name..." This somewhat made me itch - some implementations will reject a given JWT, some will accept it -- I know very little about parsing JSON, but could you suggest which an implementation should prefer? Can I instruct standard parsers to do X in this case? I understand the itchy feeling. ☺ Unfortunately, the intentional laxness in the spec in this regard is a reflection of the semantics of the actual JSON specifications and implementations. For instance, http://tools.ietf.org/html/rfc7159#section-4 says: An object whose names are all unique is interoperable in the sense that all software implementations receiving that object will agree on the name-value mappings. When the names within an object are not unique, the behavior of software that receives such an object is unpredictable. Many implementations report the last name/value pair only. Other implementations report an error or fail to parse the object, and some implementations report all of the name/value pairs, including duplicates. This topic has been heavily discussed by the working group, and while the specs used to just say that objects with duplicate member names MUST be rejected, working group members, including Tim Bray (the editor of the JSON spec), prevailed on us to weaken this so that parsers that implement the ECMAscript behavior of returning only the last member name may be legally used. (The argument was made that there was more security downside in effectively requiring people to write and debug their own strict parsers than in using laxer, but well-supported and debugged parsers.) However, we also intentionally require that producers use only one instance of each member name, so that legally produced objects will never exercise the ambiguities that are present in real JSON parsers. That seemed to be the most practical solution to the working group. Section 4.1.4. "exp" (Expiration Time) Claim (and other time based Claims: What should my behavior be if I simply don't know what the time is? (I'm just a dumb device, and my RTC is claiming it is Jan1st, 1970) - I'm assuming I must not process this JWT? Does this create bootstrapping issues? The use of all claims is optional. It’s up to applications which ones make sense for them to use. In use cases in which participants don’t know the time, either this claim would not be used by the application or the application would need to define application-specific behaviors for what to do in those cases. 5.3. Replicating Claims as Header Parameters This section scares me, and I hope I'm simply not understanding what is being proposed. If you send the unencrypted version of some encrypted Claims some implementations will make important security decisions based upon those unencrypted claims, even if you tell them in a serious voice not to. http://xkcd.com/1181/ For what it’s worth, the context in which this arose was when application used intermediate software that inspected the audience of an encrypted JWT in order to route it to the correct recipient. Only the recipient has the key to decrypt the JWT and look at the encrypted audience value. But by placing an unencrypted copy of the audience in the header, the routing software could inspect it and route it correctly. This seemed to the working group like a legitimate use case, and so we decided to support it. This feature was proposed and discussed in this thread: http://www.ietf.org/mail-archive/web/oauth/current/msg11315.html. Also, the SHOULD in "If such replicated Claims are present, the application receiving them SHOULD verify that their values are identical, ..." - why is this not a MUST? And if an application *does* compare them and they are not identical, what should it do? Perhaps a much stronger justification for carrying 2 copies of the data is in order. The text right after this in the spec already answers this question: “unless the application defines other specific processing rules for these Claims”. It’s a “SHOULD” because applications might need to do this. Editorial: The intro is almost identical to the abstract. Making the abstract more abstract, or the intro more introductory (I have no idea what many of the acronyms were!) would be nice. Something short explaining what a JWT is, why I'd like one,what they get used for, why I should keep reading this document would be very helpful - basically a background type section... Specific wording suggestions would be welcomed. As for not knowing what the acronyms are, I’m told that the style guides don’t allow references to be put in the abstract. Otherwise, for instance, there would be a reference there to RFC 7159 so people who don’t know what JSON is know where to look to go find out. Nits: Abstract O: is a compact URL-safe means P: is a compact, URL-safe means Thanks 3. JSON Web Token (JWT) Overview O: The contents of the JOSE Header describe P: Spell out JOSE; first use in document as far as I could see Actually, the first use of the term “JOSE Header” is in Section 2 (Terminology), where it is incorporated into this specification by reference. 5.2 "cty" (Content Type) Header Parameter O: normal case where nested signing P: normal case in which nested signing Thanks 8. Implementation Requirements O: For instance, an application might require support for encrypted JWTs and Nested JWTs; another might require support P: For instance, one application might require support [...], while another might require support [...] Thanks 11. Security Considerations O: The entire list of security considerations is beyond the scope of this document, but some significant considerations are listed here. P: The entire list of security considerations is beyond the scope of this document. R: A few of the considerations are already listed above; we don't need to restate that they are listed here -- and if we do, the assumption is that said list would follow, not be above. Several reviewers have objected to this sentence. Its removal is planned. 11.2 Signing and Encryption Order O: While syntactically, the signing and encryption operations for Nested JWTs may be applied in any order, P: While syntactically the signing and encryption operations for Nested JWTs may be applied in any order, Thanks -- I don't think the execution is relevant when it was obviously a bad idea in the first place. This is like putting rabid weasels in your pants, and later expressing regret at having chosen those particular rabid weasels and that pair of pants. ---maf Thanks again, Warren, -- Mike
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
