Re: [secdir] SECDIR review of draft-ietf-jose-json-web-key-31

[Mike Jones <[e-mail]>]

Hi Stephen.  Thanks for your detailed and useful review.  I've cc'ed the working group in my reply so they're aware of the contents of your review.  Replies are inline below...

From: Stephen Kent [mailto:[e-mail]]
Sent: Tuesday, September 02, 2014 1:09 PM
To: [e-mail]<mailto:[e-mail]>; Mike Jones; [e-mail]<mailto:[e-mail]>; Moriarty, Kathleen
Subject: SECDIR review of draft-ietf-jose-json-web-key-31

Bottom Line: This needs work before it's ready for publication.
I encountered many examples of confusing wording, some of which I fixed; others can be fixed based on my comments. I also found some worrisome requirements that need more careful evaluation: A MUST that probably is a SHOULD, alternatives to using an IANA registry (without a rationale for such an option, etc.).

This document is part of a series from the JOSE WG, defining formats and procedures for using JSON to convey keys, encrypted, authenticated, and/or signed data. This document focuses on keys. A significant portion (about 50%) of the document is devoted to appendices, several of which provide detailed examples of JOSE headers conveying key material.

I am not knowledgeable about JSON. I anticipate that the intended audience for the doc is. So, I started by jumping to the Security Considerations section. This section addresses several good topics, but the wording is very awkward in places.

For example, the section begins by saying:


   All of the security issues that are pertinent to any cryptographic

   application must be addressed by JWS/JWE/JWK agents.  Among these

   issues are protecting the user's asymmetric private and symmetric

   secret keys, preventing various attacks, and helping avoid

   mistakes such as inadvertently encrypting a message to the wrong

   recipient.

Many attacks cannot be prevented; one can employ countermeasures so that the attacks are not successful, but that's not what the text says.  Also, helping avoid a mistake such as sending an encrypted message to the wring recipient is a laudable goal, but it seems like a poor example for this document (and it is likely to be impossible in many cases).

I agree that "employing countermeasures to" is more accurate than "preventing".  I also agree that the "avoiding mistakes" language is not actionable - I propose to just remove it.

Another questionable example appears at the beginning of 9.1:



   One should place no more trust in the data associated with a key than

   in than the method by which it was obtained and in the trustworthiness

   of the entity asserting an association with the key.


Even removing the apparently redundant "than in" this sounds like advice spoken by Yoda.

Actually, it was spoken by then-Security AD Sean Turner. ;-)

It's not clear whether the author is referring to the data about a key, vs. data decrypted or authenticated using a key. This point needs to be made more clearly.

How about changing "data associated with a key" to "data cryptographically secured by a key"?  (And of course, deleting the extraneous "than".)

Section 9.2 discusses the importance of protecting private and symmetric keys. It says:


   Private and symmetric keys MUST be protected from disclosure to

   unintended parties.  One recommended means of doing so is to encrypt

   JWKs or JWK Sets containing them by using the JWK or JWK Set value as
   the plaintext of a JWE.

The wording above is needlessly awkward. Nonetheless, this says that key sets containing symmetric or private keys should be encrypted by embedding them in another JSON crypto format (JWE). It would be nice to add that this implies a that there is secure way to deliver the needed decryption key for the JWE, else this recommendation just adds a layer of indirection, and does not solve the problem.

Fair enough.  I propose that we add something along those lines.

Section 9.3 discusses a countermeasure against a specific attack on RSA key use. This seems unduly narrow, since this spec is intended for use with RSA, DH, DSS, and ECDH keys. Why devote a long paragraph to this one issue, while saying nothing about equally serious concerns that arise for other algorithms?

This particular attack is described both because the countermeasure requires specific key representation actions and because a working group member asked it to be included.  For what it's worth, I expect that additional security considerations will be added when resolving Russ Housley's gen-art review of the JWS specification.

Returning to the body of the document, I noticed an awkward sentence in the introduction:


   Goals for this specification do not include representing new kinds of

   certificate chains, representing new kinds of certified keys, or

   replacing X.509 certificates.

This seems like an arbitrary set of non-goals. Perhaps the JOSE WG discussions prompted this declaration. If so, more text to establish that context would be helpful.

These non-goals were agreed to by the working group from the very beginning, while the working group was still being chartered.  The group wanted to build something simple and easily deployable to represent keys in JSON - not reinvent all the work that the PKIX working group did on certificates and certificate chains, etc.  Do any working group members want to suggest specific wording to try to capture this sentiment?

The example that comprises Section 3 should include an explanation of the parameters, else it's not a great example.

The parameters and values of them are explained in the paragraph preceding the example text.  It says:


   The following example JWK

   declares that the key is an Elliptic Curve [DSS<https://tools.ietf.org/html/draft-ietf-jose-json-web-key-31#ref-DSS>] key, it is used with

   the P-256 Elliptic Curve, and its x and y coordinates are the

   base64url encoded values shown.  A key identifier is also provided

   for the key.

Each statement above corresponds to a parameter in the example, and in the same order.

I suppose that one option is to be more verbose above and add parenthetical remarks after each statement above saying which parameter does this.  So for instance, the parenthetical phrase "("kty" parameter)" could be added before the first comma.  Do others in the working group think that would make the example easier to read, harder to read, or do any of you have an alternative suggestion?

Section 4 starts with awkward wording, to wit:


  In addition to the common parameters, each JWK will have members that

  are specific to the kind of key being represented.  These members

  represent the parameters of the key.

The reuse of the word "parameters" in the two sentences above creates an apparent conflict: How about:


  In addition to the common parameters, each JWK will have members that

  are algorithm-specific.



They're not algorithm-specific - they're key type-specific.  Another way of eliminating the repeated use of the word "parameters" is to replace the second sentence with "These members represent the key value".  Would that work for you (and the working group)?


This section imposes a rather wimpy constraint on parameter names:



   The member names within a JWK MUST be unique; recipients MUST either

   reject JWKs with duplicate member names or use a JSON parser that

   returns only the lexically last duplicate member name, as specified

   in Section 15.12 (The JSON Object) of ECMAScript 5.1 [ECMAScript].


This text says that member names MUST be unique, but if they are not, that's OK too; just use the last instance of a member with a duplicate name. This seems like a terrible design principle. It imposes what appears to be a requirement, then says how to accommodate data structures that fail to meet the requirement. This would seem to encourage sloppy implementations (for JWK generation). I'd like to see the rationale for this.





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.


The next paragraph defines a rather wimpy requirement:



   Member names used for representing key parameters for different keys

   [sic] types need not be distinct.  Any new member name should either be

   registered in the IANA JSON Web Key Parameters registry defined in

   Section 8.1 or be a value that contains a Collision-Resistant Name.


The text should include a pointer to the definition of "Collision-Resistant Name in the JWS doc (or add it to the terminology section here), to make this requirement less mysterious. The term is used extensively in this section. Since it is repeatedly offered as an alternative to IANA registration of a member name; I examined the definition in the JWS document, to discover what a C-R name is. The definition there emphasizes examples, one of which ensure uniqueness (OIDs) but the other, despite its name, does not. The text here should explain why using a C-R name is a good alternative to the use of am IANA registry and thus when use a C-R name is a good idea.



The term "Collision-Resistant Name" is already present in the Terminology section.  However, previous reviewers had requested that definitions not be repeated in multiple specs, so it's incorporated by reference, rather than repeating the definition here.  The notion is that of an implementation wants to use a collision-resistant name such as "http://names.example.com/the-name", it can do so without having to create a public specification and register the name with IANA.


In 4.2, the description of the "use" member, due to its name, creates a lot of awkward sentences. These can (and should) be fixed. For example:



   Use of the "use" member is OPTIONAL, unless the application requires

   its presence.


can become:



  The "use" member need not be present in a JWK, unless the

  application consuming the JWK requires it.



Thanks


This section says that a use type of enc SHOULD be employed when describing public keys used for key agreement. The use of SHOULD here raises the obvious question: When is it OK to not employ that use value for a key agreement key?



I agree that the "SHOULD" language is awkward.  Rather than saying "SHOULD be used", we could change it to just say "is used".


The mysterious SHOULD noted above is followed by an even more mysterious parenthetical statement:



   (The "alg" member can be used to specify the particular

   cryptographic operation to be performed, when desired.)


This comment conveys no clear meaning in this context, especially since the alg parameter not is defined until 4.4.


Would the language "The "alg" member can be used to specify the cryptographic operation that the key is intended to be used for" work better for you?  Or would people like to just see the parenthetical remark deleted?

Section 4.5 defines the key_ops parameter. It's not clear how this parameters and "use" relate. There is also an odd sentence at the end of the first paragraph:



   The "key_ops" parameter is intended for use cases in which public,

   private, or symmetric keys may be present.


This seems to encompass all of the types of keys that JWK carries, so the sentence seems to add no useful qualification for when this parameter is intended to be used.

This is in contrast to the related statement in the "use" definition:

   The "use" parameter is intended for use cases in which
   it is useful to distinguish between public signing keys and public
   encryption keys.

I am sorry to see this document defining "sign" as an operation that refers to both signatures and MACs. The IETF has done a disservice to the community by using the term "signature" to refer to message authentication codes in several RFCs. The text notes that the values for this parameter match those used in the Web Crypto API, a W3C document for key usage; this is true. However, I examined that document and found 35 instances of the term "signature." Only about 3 of these refer to a MAC vs. a digital signature. I also note that the cited document is not yet final, as per the W3C web site. Maybe it's not too late to fix this.

If you want to see this parameter name changed, you'll need to file a bug against the WebCrypto spec and get it changed there.  Then I'm sure that JOSE will gladly follow.

The list of key_ops values is followed by this text:


   Other values MAY be used.  Key operation values can be registered in

   the IANA JSON Web Key Operations registry defined in Section 8.3.

   The key operation values are case-sensitive strings.


The text says that it's OK to use values not in the list, and one "can" register new values, but, why bother? This seems to be a questionable approach to extensibility, one that could easily cause confusion when non-registered values are employed. The editor should explain why this approach is a good one.

This specification will be used both in open environments, in which multiple organizations will need to have a common understanding of any extensions used, and closed environments, which the producing and consuming organization will always be the same and private values could be safely used.  IANA registration is definitely the right thing to do for open environments.  It's probably unnecessary for deployments in closed environments.

Section 4.4 presents another register the parameter, or not, choice. Same comments as above. This parameter is cited as optional; this merits an explanation, since one can imagine a lot of bad outcomes if the algorithm is not identified.

Same answer as for Section 4.

The description of kid in 4.5 says:



     The "kid" (key ID) member can be used to match a specific key.


What else would a key ID be used for?

"Can" is being used as a non-2119 synonym for "MAY" here.  That being said, we could just change "can be" to "is", since it's explicitly said that its use is optional at the end of the paragraph.

In 4.6 there is a very confusing statement:


   While there is no requirement that members other than those

   representing the public key be populated when an "x5u" member is

   present, doing so may improve interoperability for applications that

   do not handle PKIX certificates.


This assertion, in addition to being an overly-long sentence, begs for an explanation. The parameter is a pointer to an X.509 cert, so how will its inclusion "improve interoperability" for an application that does not handle such certs? It also is confusing that this parameter can point to either a single cert or a cert chain, and the x5c parameter points to a chain, or a single cert. Why are there two different parameters? Is it just an encoding difference? If so, more descriptive names ought to be used for these two parameters.

It's the inclusion of other metadata about the key that might improve interoperability that's being referred to - not the inclusion of the cert reference.  For instance, including "use" or "alg" parameters might be useful to applications that can't process the certificate.

As for the cert vs. cert chain question, in the general case, a chain may be required to establish trust.  However, a chain of length one (a single certificate) will also be sufficient in some use cases.  We're not inventing anything new here.  The data format is specified in RFC 1421.

This section ends with an odd statement:



   Similarly, if the "alg" member is present, it should represent an

   algorithm that the certificate allows.


A cert always specifies the algorithm with which the public key in the cert is to be used. So the term "allows" above is odd, at best.

I had thought there were uses of RSA keys where the same key is used both for signing and encryption (even though this is a deprecated practice).

But we could change this to "Similarly, if the "alg" member is present, it SHOULD correspond to the algorithm specified in the certificate."  Or is that overly strong for some certificates and uses of them?

Section 4.7 includes inaccurate terminology. It says:



   This MAY be followed by additional certificates, with

   each subsequent certificate being the one used to certify the

   previous one.


Replace "certify" with "validate."

OK

Also, the name seems misleading since the chain MAY contain additional certs, and hence may not be a chain at all!

I'm not sure if I'm following you here.  Are you suggesting the possibility of having multiple certificates not chaining to one another in the representation?  This isn't allowed by the specification, as written.  Are you suggesting that it needs to be allowed?

Section 4.8 refers to a "thumbprint" of a certificate, and describes it informally as a "digest." The more common technical term is a one-way hash. Thumbprint seems to be a Microsoft term; "fingerprint" strikes me as more common. The text here, and in 4.9 should point to Appendix C of the JWS document, where the base64URL encoding is defined. That document should note that the appendix is normative.
Thumbprint is the term used in the Windows libraries, such as http://msdn.microsoft.com/en-us/library/system.security.cryptography.x509certificates.x509certificate2.thumbprint(v=vs.110).aspx.  Whereas OpenSSL uses fingerprint http://www.openssl.org/docs/apps/x509.html.  I know that there would be an uproar if we tried to make a breaking change to the "x5t" name at this point, because it's in widespread production use.  However, we could add language saying that certificate thumbprints are also known as certificate fingerprints, so people familiar with either term will know what this is.
The term "base64url" is incorporated by reference in the terminology section (Section 2).

Actually, Appendix C in JWS is not normative.  It's just example code.  The normative definition of the encoding is in Section 5 of RFC 4648.

Section 5 again repeats the semi-requirement description of name uniqueness that appeared at the beginning of Section 4. The same comments apply here, as there.

Same answer.

Section 7 begins with a rather wordy statement:


   Access to JWKs containing non-public key material by parties without

   legitimate access to the non-public information MUST be prevented.

   This can be accomplished by encrypting the JWK when potentially

   observable by such parties to prevent the disclosure of private or

   symmetric key values. The use of an Encrypted JWK, which is a JWE

   with the UTF-8 encoding of a JWK as its plaintext value, is

   recommended for this purpose.




How about a simpler way to say this:



    When private or symmetric keys are transported by JWK, the

     confidentiality of these keys MUST be ensured. It is RECOMMENDED

     that confidentiality be provided by encrypting the JWK when the data

     might be observable by unauthorized parties.

Thanks

This section then goes on to say:



   A "cty" (content type) Header Parameter value of "jwk+json" MUST be

   used to indicate that the content of the JWE is a JWK, unless the

   application knows that the encrypted content is a JWK by another means

   or convention, in  which case the "cty" value would typically be

   omitted.


Given the rather large loopholes here, this sounds more like a SHOULD, followed by a description of exception conditions, rather than a MUST.

It used to be a "SHOULD" but the working group felt that the "MUST ... unless" wording was a more accurate statement of the requirement.

Section 8 (IANA Considerations) establishes a two-week review period for creating new (IANA) registry items. This seems too short; some people take multi-week vacations. I note that the same text appears in the JWS and JWE documents.

This text was taken from RFC 6749.

Typo:



   Criteria that should be applied by the Designated Expert(s) includes


Should be:



   Criteria that should be applied by the Designated Expert(s) include

Thanks

I did not review all of the initial registry content defined in 8.1.2, 8.2.2, 8.3.2, or 8.4.2.


The Appendices appear to be informative; they should be labeled as such. I did not review the context of the Appendices. But, I did note the following:

Aren't appendices normally informative?

In C.2, the text says:



      o  the Plaintext is encrypted using the AES_128_CBC_HMAC_SHA_256

      algorithm to produce the Ciphertext


The algorithms described here (AES 128 in CBC mode, plus an HMAC computed using SHA-256) provide both encryption and integrity. Thus it may be confusing to refer to this as only "encryption."

We could be more explicit and talk about performing authenticated encryption.

I also note that the algorithms (algorithm suites) used in the examples in the appendices lack citations to documents that define these algorithms.

Thanks - they can be added.

                                                            Thanks again, Stephen,
                                                            -- Mike