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

From: ⌘ Matt Miller <[email-masked]>
Date: Tue, 2 Dec 2014 15:02:21 -0700

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 [email-masked] 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 < [email-masked] >
Cisco Systems, Inc.

---

From: Richard Barnes <[email-masked]>
Date: Tue, 2 Dec 2014 14:11:19 -0800

On Tue, Dec 2, 2014 at 2:02 PM, ⌘ Matt Miller <[email-masked]> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> Hello Yaron,
>

It's also worth noting that of all of the CMS / S/MIME deployments that
have existed for many years, none of them apply integrity protection to the
CMS parameters that are equivalent to these JOSE parameters.

--Richard

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

---

From: "Jim Schaad" <[email-masked]>
Date: Tue, 2 Dec 2014 14:43:29 -0800

> -----Original Message-----
> From: ? Matt Miller [mailto:[email-masked]]
> Sent: Tuesday, December 02, 2014 2:02 PM
> To: Yaron Sheffer; IETF Security Directorate; The IESG; draft-ietf-jose-
> [email-masked]
> Subject: Re: [secdir] SecDir review of draft-ietf-jose-cookbook-06
> 
> > 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.
> >
> > "real life", I would expect developers to use it as an authoritative
> > source.
> >
> > Summary
> >
> > By the way, I really appreciate the large effort that surely went into
> > creating this document.
> >
> 
> >   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?
> 
> >   Why invent a new term "octet key", for a simple "symmetric key"?
> 
> >   4.2: the first sentence discusses PS256, the actual example is
> > PS384.
> >
> 
> >   4.7: "only the JSON serialization" - please clarify which of the
> >   5.1.1: since this is a "cookbook", we should be using the public
> 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?
> 

I believe that it makes far more sense to have the private keys as well as the public keys.  Historically in S/MIME people stated by trying to decrypt known good messages and then went on to try and encrypt and test against a known good decryption implementation.

Additionally, one would of course need to have private keys on the signing side as well.

Jim

> 
> >   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.
> >
> 
> >   5.3.5: In the General Serialization version, I don't understand
> >   5.7: same as above, it makes sense for the per-recipient key to
> >   5.11: this example seems strange to me - why would anybody NOT

---

From: Yaron Sheffer <[email-masked]>
Date: Thu, 04 Dec 2014 23:07:44 +0200

Hi Matt,

Please see my comments below. I have removed much of the original text, 
and only left points that need further discussion.

Thanks,
	Yaron

On 12/03/2014 12:02 AM, ⌘ Matt Miller wrote:
[...]
> is there a better suggestion?

GitHub would be a great place.

>     decoded and the field "d" is not longer than 256 octets in

Yes.

[snip]
>     allow readers to validate the example's output.

I'm fine with this new text.

>     content instead of the plaintext from figure 72.

Yes, this would help.

>     values for the PBES2 parameters "p2s" and "p2c".

So I still don't understand: don't we need an example that demonstrates 
how the JSON structure (or multiple structures) is generated so that 
"each recipient use(s) different values"?

In general I don't understand this "compatibility" thing. Obviously 
there are some cases that cannot be expressed in all serialization 
types. Otherwise why would we need three of them?

> in the top-level object is not permitted for the general serialization.

Still confused. Sorry.

> the part of all of the header is not always required.

So (personal opinion here) I think the WG took a security risk that 
should not have been taken in 2014, for a minor performance gain. We 
have seen too many protocols fail because of integrity-protection 
shortcuts. Who would have thought you need to integrity-protect the 
padding field! The fact that CMS took this route back in 1999 is sort of 
irrelevant, as we've all learned a lot since then.

---

From: "Jim Schaad" <[email-masked]>
Date: Mon, 8 Dec 2014 16:55:46 -0800

> From: Yaron Sheffer [mailto:[email-masked]]
> Sent: Thursday, December 04, 2014 1:08 PM
> To: ? Matt Miller; IETF Security Directorate; The IESG; draft-ietf-jose-
> On 12/03/2014 12:02 AM, ? Matt Miller wrote:
> [...]
> 
> >

It is not clear to me what you are asking for at this point.    Section 5.13 has a demonstration of how to deal with multiple recipients for a set of key management algorithms, but for every different type of key management that is possible.   It is not clear to me that a password based encryption is a good place to show multiple recipients.  

The JWE specification allows for three different locations for parameters to be placed.  Any parameter can go in any of these different locations and they are all treated differently.  There are rules that a parameter of any given name cannot occur in two different locations, this means that if one did have multiple recipient situations, that parameters which have the same name but are different would be forced into the unprotected bag of the per recipient location.  This would be true for any of the different parameters - the most common would the 'alg' parameter which is generally different for each recipient.

> types. Otherwise why would we need three of them?

I will assume that you have read (or at least scanned) section 7 of the JWE document.   However a summary of it is:

* Compact serialization does not support either the global or per recipient unprotected parameter bag
* Compact serialization does not support application AAD information.
* Compact serialization and Flattened JSON serialization only support one recipient

Since the document is attempting to show all three serializations where possible, this does impose a restriction on where parameters can be placed.

> > in the top-level object is not permitted for the general serialization.

This has the same basic response.

The protected parameter bag is not associated with the content encryption key.  It applies equally to the content encryption key and the recipient key.  A parameter such as kid is defined as only applying to a CEK in the direct encryption case (which can be argued is the recipient key).  In all other cases the kid parameter always applies to the recipient key and not to the CEK.  You cannot make an association of keys and location of attributes.

> irrelevant, as we've all learned a lot since then.

Allowing for integrity protection of kid and algorithm would have ended up with a very different looking structure than what we had.  This was the approach that was going to be used by the authors of the draft and was universally and roundly rejected by the WG.   If that is really what you want to do, then one might as well create n messages each for one recipient and protected the parameters in that way rather than create a multi-recipient message.  The final effect is going to be the same as either one needs to have a very complex structure of the protected structure where every recipient is included there (and thus force a full re-encryption by anybody changing the set of recipients) or each recipient has to be done independently in order to create a MAC value just for that recipient.  Neither of these approaches is very appetizing. 

How would you go about doing such a design to make it so that only a single encryption process is needed, and so that one can add new recipients without having to do a full decrypt and re-encrypt if one wants to either add or strip recipients from a message?

If full integrity protection is needed, one can always do a single message per recipient.   However, changing something like the kid is not currently very useful.  The fact that one finds the wrong key is much less of an attack (other than a DOS attack) with authenticated encryption that it was before with normal symmetric encryption.  Failing to successfully decrypt a message is not a huge attack vector that the WG cared about given that it can be accomplished by just changing one byte in the cipher text as well.

Jim

---

From: ⌘ Matt Miller <[email-masked]>
Date: Tue, 9 Dec 2014 11:35:29 -0700

Hello Yaron,

I believe < https://tools.ietf.org/html/draft-ietf-jose-cookbook-07 >
addresses most of your comments; the exceptions being those points
still under discussion (see Jim Schaad's latest reply).  Please let me
know if I missed anything I thought I fixed.

Thank you,

- -- 
- - m&m

On 12/4/14, 2:07 PM, Yaron Sheffer wrote:
> Hi Matt,
> 
> Please see my comments below. I have removed much of the original
> text, and only left points that need further discussion.
> 
> Thanks, Yaron
> 
> On 12/03/2014 12:02 AM, ⌘ Matt Miller wrote: [...]
>>> 
>>> 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?
>> 
>> decoded and the field "d" is not longer than 256 octets in length
>> when decoded.
>> 
> 
> Yes.
> 
>> 
> [snip]
>> 
>>> • 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?
>> 
>> encryption.  However, the example includes the RSA private key
>> to allow readers to validate the example's output.
>> 
> 
>> encryption was the import and export of key sets, and the
>> Working Group desired a thorough example.
>> 
>> 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.
>> 
> 
> Yes, this would help.
> 
>> 
>>> • 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.
>>> 
>> 
>> 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.
>> 
> So I still don't understand: don't we need an example that
> demonstrates how the JSON structure (or multiple structures) is
> generated so that "each recipient use(s) different values"?
> 
> In general I don't understand this "compatibility" thing.
> Obviously there are some cases that cannot be expressed in all
> serialization types. Otherwise why would we need three of them?
> 
>>> • 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.
>>> 
>> 
>> 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.
>> 
> 
> Still confused. Sorry.
> 
>>> • 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.
> 
> So (personal opinion here) I think the WG took a security risk
> that should not have been taken in 2014, for a minor performance
> gain. We have seen too many protocols fail because of
> integrity-protection shortcuts. Who would have thought you need to
> integrity-protect the padding field! The fact that CMS took this
> route back in 1999 is sort of irrelevant, as we've all learned a
> lot since then.

---

From: Yaron Sheffer <[email-masked]>
Date: Tue, 09 Dec 2014 22:40:03 +0200

Hi Matt,

Yes, I believe it does. I will respond to Jim's mail separately.

Thanks,
	Yaron

On 12/09/2014 08:35 PM, ⌘ Matt Miller wrote:
> iQEcBAEBCgAGBQJUh0DxAAoJEDWi+S0W7cO1yn8H/3qu51pBi99f8FmuI/ZyW+9+
> LJfToPCcaTlSWqLR14mzApvm2VTgX60R89+ykQzVcvRIqZZvr4gqtJFnNtSUsGgs
> PWEU837QEi0B/xfLADk6YF8om3B5XR6SX1xS4BSY4+oxh1XjIeRFoNqE2XLcFCd/
> P5NLs5NH3ZJS4khKFTgw7dzYBiSWUf+DEOZaPb8vcRVRUy7giJlKbax5u1jQgk9X
> 1e7+W72rpjSVfd9+cP7Jme4Atz/K7MKawebz+6UsTVRH3dImwg4qy2IggMrGxrba
> TZXdXE4wxVVlbc768cTjZJPtHwZ/HH/e/sgNrUK66WSrBveoBDf7v8NGu7+pS8w=
> =3XEs