Re: [secdir] SecDir review of draft-ietf-jose-cookbook-06

[⌘ Matt Miller <[e-mail]>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hello Yaron,

Thank you very much for the review.  My other comments are inline.

On 11/22/14, 12:39 PM, Yaron Sheffer wrote:
> 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.
> 
> This document contains a large set of examples of JOSE objects:
> JWK, JWS and JWE. The document is purely informational, though in
> "real life", I would expect developers to use it as an
> authoritative source.
> 
> Summary
> 
> The document is ready to be published, with some issues.
> 
> By the way, I really appreciate the large effort that surely went
> into creating this document.
> 

Thank you!  It's always great to hear one's efforts are appreciated!

> Details
> 
> • Unless I missed it, the document does not mention a machine
> readable repository of these examples, which I am sure the author
> has created while writing the draft. Making such a repository
> publicly available would result in a much more useful resource than
> the current document, which essentially requires testers to scrape
> the document when creating their test cases.
> 

You did not miss it; I don't have a such a repository right now, but I
can put one together.  Would something on github.com be acceptable, or
is there a better suggestion?

> • (Not a comment to the current document:) I wonder why there is
> nothing explicit to distinguish a public key from a private key,
> and they are only distinguished by the presence or absence of
> several parameters, something that will not be natural to most
> developers. PEM is doing it very well: "-----BEGIN RSA PRIVATE
> KEY-----".
> 
> • 3.4: the text is contradictory re: zero-padding of the value "d".
> Is it using the minimum number of octets, or exactly 256 octets
> (for a 2048-bit key)?
> 

The intent is that "d" is not zero-padded, and I overqualified in my
text.  Would the following be acceptable?

   For a 2048-bit key, the field "n" is 256 octets in length when
   decoded and the field "d" is not longer than 256 octets in
   length when decoded.


> • Why invent a new term "octet key", for a simple "symmetric key"?

Very good point.  Will fix in the next revision.

> 
> • 4.2: the first sentence discusses PS256, the actual example is
> PS384.
> 

Silly error that I thought I fixed long ago /-:

Will fix in the next revision.

> • 4.7: "only the JSON serialization" - please clarify which of the
> three serializations is meant. Ditto top of 4.8.
> 

Will fix in the next revision.

> • 5.1.1: since this is a "cookbook", we should be using the public
> key, not the private key. A private key would only be used in rare
> cases. Similarly 5.2.1.
> 

The private keys are included for both reproduction (which only needs
the public key) and verification (which necessitates the private key).

If I can put an online repository together, I can change the examples
to just include the public keys; otherwise would the following in 5.1
(and 5.2) be sufficient?

   Note that only the RSA public key is necessary to perform the
   encryption.  However, the example includes the RSA private key to
   allow readers to validate the example's output.


> • 5.3.1: the "plaintext content" is a list of keys, which is
> either confusing to the reader, or an actual error in the
> document.
> 

It is not in error.  The most common usecase for password-based
encryption was the import and export of key sets, and the Working
Group desired a thorough example.

Would it help if the following is added to 5.3?

   A common use of password-based encryption is the import/export of
   keys.  Therefore this example uses a JWK Set for the plaintext
   content instead of the plaintext from figure 72.


> • 5.3.5: In the General Serialization version, I don't understand
> why only the encrypted key is per-recipient. I would expect the
> PBES2 parameters too (e.g., the salt)  to be per-recipient.
> Presumably each of them is using a different password, and there's
> no reason to use a common salt. Similarly in 5.4.5.
> 

For compatibility across serializations (compact, general JSON,
flattened JSON), all of the parameters need to be in the JWE Protected
Header.  In the general serialization, that means only the
"encrypted_key" field is present for the (presumably) sole recipient.

Would it be acceptable if the following were added to 5.3?

   Note that if password-based encryption is used for multiple
   recipients, it is expected that each recipient use different
   values for the PBES2 parameters "p2s" and "p2c".

> • 5.7: same as above, it makes sense for the per-recipient key to
> have an ID, rather than the content encryption key (typically an
> ephemeral key). And then that ID should be in the per-recipient
> data in 5.7.5. And similarly for 5.8. The later Sec. 5.13 shows a
> syntax for multiple recipients that's inconsistent with the
> single-recipient case, which would make sense if we got rid of the
> array.
> 

For compatibility across serializations (compact, general JSON,
flattened JSON), all of the parameters need to be in the JWE Protected
Header.  Also, the mixing of "recipients" and "encrypted_key"/"header"
in the top-level object is not permitted for the general serialization.

> • 5.11: this example seems strange to me - why would anybody NOT
> want to integrity-protect the key ID and algorithm? I would prefer
> a more realistic example, or failing that, a recommendation to
> developers to avoid this practice. Similarly 5.12, which is an even
> worse idea.
> 

Integrity protection was thoroughly discussed in the JOSE WG.  While
there are some limited attacks possible when some parameters are
unprotected, the WG felt there were enough use cases where these
attacks are mitigated through other means that integrity protection of
the part of all of the header is not always required.


> Thanks, Yaron
> 
> _______________________________________________ secdir mailing
> list [e-mail] https://www.ietf.org/mailman/listinfo/secdir 
> wiki: http://tools.ietf.org/area/sec/trac/wiki/SecDirReview

And thanks again for the thorough review.

- -- 
- - m&m

Matt Miller < [e-mail] >
Cisco Systems, Inc.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
Comment: GPGTools - https://gpgtools.org

iQEcBAEBCgAGBQJUfjbtAAoJEDWi+S0W7cO1rSUH/1j3BZKQz5zwo/N9/nlfFwmS
JFGOCww9xOjc2PcdWh4XWh0325abNxc6iMLdiphbQXsad1eB/g5Zg/zXy/fRwv25
M7YMiaiAa2YAEDyogkMvX1ErQyoN3I9KKcAsxqQmftjpsosyeyfIi0KBJ+yNF8+X
RNvR/wmzEeBjnJpoVCOI00gw5Zb9cjdTZAePrVJkm3wqUx0u98Rtj/Ws5P4LppWF
jV7XJAkSo9yOZkfXYXUIArpLAtzgob/wfvEy32ESIzlDbQVI/fB7TeaowrM68xJj
7MEC1iATNW1KGWLnnt8VHY6x+qhH3tz+lyt+Qj5Hmr/BWCu0Rj12i2rbCh23SY4=
=4rNI
-----END PGP SIGNATURE-----