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

[Yaron Sheffer <[e-mail]>]

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:
[...]
>>
>> • 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?

GitHub would be a great place.

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

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

I'm fine with this new text.

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

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

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

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.