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

Request Review of draft-ietf-jose-json-web-signature
Requested rev. no specific revision (document currently at 41)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2014-09-03
Requested 2014-08-21
Authors Michael Jones, John Bradley, Nat Sakimura
Draft last updated 2014-09-04
Completed reviews Genart Last Call review of -31 by Russ Housley (diff)
Secdir Last Call review of -31 by Tero Kivinen (diff)
Assignment Reviewer Tero Kivinen
State Completed
Review review-ietf-jose-json-web-signature-31-secdir-lc-kivinen-2014-09-04
Reviewed rev. 31 (document currently at 41)
Review result Has Issues
Review completed: 2014-09-04

Review
review-ietf-jose-json-web-signature-31-secdir-lc-kivinen-2014-09-04

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.

Summary: This document has issues.

This document is part of the jose-json document set, and describres
the JSON Web Signatures.

The security considerations section includes text which says:

   The entire list of security considerations is beyond the scope of
   this document, but some significant considerations are listed here.

but also lists quite a lot of security considerations. I think the
security considerations covering this document should be in scope with
the document. Of course there are generic security considerations
which might be outside the scope of this document, but I do not think
we need to explictly mention those.

I have following issues about the draft:

   1) "alg" and Protected Header

   2) Hash inside "alg" and inside the signature

   3) There is no explict warning about the "alg" "none".

   4) Thumbprint formats

There is also following nit:

   5) Terminology ordering.

----------------------------------------------------------------------

1) "alg" and Protected Header

Question: Shouldn't the "alg" header parameter be protected by the
signature, i.e. wouldn't it make sense to say MUST be in the
"Protected Header"?

If it is part of the "Unprotected Header" and is not protected by the
signature, that would allow all kind of attacks, i.e. changing the
"alg" to be "none" or changing the hash algorithm of the signature.

If it should be part of the "Protected Header" then that would mean
that "Proteced Header" cannot be empty, as "alg" is mandatory header
parameter, and MUST be present.

There are several cases where the text indicates that "Protected
Header" could be empty, which would mean that "alg" could be part of
the "Unprotected Header". (Section 5.1, 4. bullet; section 7.2,
"protected" element and other places in same section). In all examples
the "alg" is always in the "Proteced Header". 

I think the draft needs text saying something about the situation
where "alg" is not in "Protected Header" in the security sections
section. I.e. either say, that it has been analyzed that there is no
problem even when the "alg" is not protected, and reference to such
analysis, or otherwise add text/warning that it MUST/SHOULD be in the
"Protected Header". I do not know enough about the proposed signature
algorithms to know which one is true, especially as there might be new
algorithms in the future.

--

2) Hash inside "alg" and inside the signature

Also in some cases the signature itself has the hash function stored
internally, i.e. RSASSA-PKCS1-V1_5 contains the hash function oid
inside the signature, so what should the implementation do if the
"alg" parameter outside the signature does not match the oid inside
the signature? I.e the signature using "alg" of "RS256", but inside
the signature the oid is using the "SHA1". Most crypto libraries will
just take the oid from the signature, and use that to verify the
message. Adding some description what to do in such situation would be
needed.

--

3) There is no explict warning about the "alg" "none".

In the section 5.2 it says that "at least one signature ... MUST
successfully validate", but that does not limit alg "none" out from
it. I.e. if the application policy is to "one signature needs to
validate", and it gets JWS that has "none" as one of the algorithms,
then it will accept it.

I think there should be warning here or in the security considerations
section about the "none" algorithm, especially as the algorithm itself
is defined in the different draft (perhaps just reference to the
section 8.5 of the [JWA] draft).

--

4) Thumbprint formats

Section 4.1.7 and 4.1.8 defines a x5t and x5t#S256 thumbprints, but
those are over the whole certificate.

With the thumbprints, it has been noted lately, that quite often it is
more useful to use the hash of the SubjectPublicKeyInfo object of the
X.509 certificate, than the full X.509 certificate. This method has
been used in the raw public key methods (draft-ietf-tls-oob-pubkey,
draft-kivinen-ipsecme-oob-pubkey), and also in the DANE (it has two
options one for the full certificate and another for the
SubjectPublicKeyInfo object of the certificate).

Using hash of the SubjectPublicKeyInfo object allows changing the
certificate without invalidiating the certificates, i.e. when changing
CAs, or switching from SHA1 to SHA2 in certificates, or just renewing
the certificate. It also allows using raw public keys which do not
have defined X.509 certificate format, but which can be converted to
the SubjectPublicKeyInfo object when calculating the thumbprints. This
is very important in the Internet of Things type of things, which
might not be using the full X.509 certificates. 

--

5) Terminology ordering.

Terminology is not in any order. It would be useful to have it either
in logical order (i.e. define terms before they are used), or in
alphabetical order.

Now for example the "JWS Protected Header" is used before it is
defined in the "JWS Signature", and "Header Paramater" is between "JWS
Signature" and "JWS Protected Header", also "JWS Signature" uses both
"JWS Payload" and "JWS Protected Header", and one of those is defined
before and one after the "JWS Signature".
-- 
kivinen at iki.fi