Last Call Review of draft-ietf-jose-json-web-key-31
review-ietf-jose-json-web-key-31-secdir-lc-kent-2014-09-04-00

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




 




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




 




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.




 




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?




 




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.




 




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




 




 




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.




 




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.




 




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.




 




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.




 




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?




 




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.




 




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.




 




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.




 




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.




 




 




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.




 




 




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? 




 




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.




 




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.




 




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




 




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




 




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.




 




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.




 




 




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.




 




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.




 




 




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. 




 




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




 




 




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:




 




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




 




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