Re: [Curdle] AD review of draft-ietf-curdle-cms-chacha20-poly1305-03

[Stephen Farrell <[e-mail]>]

Hi Russ,

On 28/11/16 22:56, Russ Housley wrote:
> Stephen:
> 
>> My AD review of this draft is below. I think a revised I-D is
>> needed before IETF LC for at least section 4, but please argue with
>> me if I'm wrong about that.
> 
> I do not think the changes are very big.  

Agreed. The only change that I think is really needed is
to fix section 4 so that people have something to review
at IETF LC.

> However, I’m pleased to
> make them before IETF Last Call, assuming you agree with the
> direction laid out below.
> 
> 
>> (1) section 2: Is it correct to say that BCP107 is a MUST the way
>> you do? I think you need to say that implementations MUST support
>> some form(s) of AKM, but I don't think you can say "MUST use" as I
>> can't see how that could be enforced at this level of the code.  If
>> it can't be enforced, then it's not worth the MUST you have here.
> 
> I do not see this concern.  The specification does not allow for the
> AEAD_CHACHA20_POLY1305 key (that is, the
> content-authenticated-encryption key) to be used for more that one
> CMS object.  A fresh one must be established, and there are four ways
> that can be used to do so:
> 
> Key Transport:  the content-authenticated-encryption key is encrypted
> in the recipient's public key;
> 
> Key Agreement:  the recipient's public key and the sender's private
> key are used to generate a pairwise symmetric key, then the
> content-authenticated-encryption key is encrypted in the pairwise
> symmetric key;
> 
> Symmetric Key-Encryption Keys:  the content-authenticated- encryption
> key is encrypted in a previously distributed symmetric key-encryption
> key; and
> 
> Passwords:  the content-authenticated-encryption key is encrypted in
> a key-encryption key that is derived from a password or other shared
> secret value.

Sure, that's all fine. My point though is that the person
implementing this will likely not be in a position to try
to enforce BCP107 in that same code base. And maybe they
actually ought not try enforce that BCP while adding this.
I think the current MUST is indicative of a layering
violation and will be ignored, if it's not already satisfied
by the overall system to which this construct is being
added.

To be clear, I'm fine that this not block LC.

> 
> 
>> (2) section 3: What does "content authenticated encryption" mean
>> exactly? You use that phrase a lot, and I don't know what it means
>> and it does not occur in [CMS]. Do we need a better term?
>> (Apologies if I'm forgetting the WG discussing that.)
> 
> RFC 5083 uses the term "content-authenticated-encryption keys”.  I
> can add the hyphens id that improves clarity.

Personally, I think that term lacks clarity and adds confusion.
I don't see why it's better than just saying "key" when you
mean that, and "key+nonce" when that's meant.

> 
> 
>> (3) section 4: this is clearly not ready for publication - why not
>> allocate the OID? But you at least need to write the text or get
>> rid of the section.
> 
> Other S/MIME Capabilities just include the hex values of the
> attribute, which is simply the DER encoding of the OID.  I can put
> the words there, but the hex string cannot be provided until the OID
> is assigned.

Please. A section containing one TBD string is fine. An
entirely blank section I think asks too much from folks
reading this. (Separately, why not allocate the OID now?
We can do early allocations.)

> 
> 
>> (4) I hope I remember to add the downref to the IETF LC:-)
> 
> For which document?

RFC 7539. That's an action on me for the LC text though,
nothing there for you to do.

> 
> 
>> nits...

One comment below, otherwise what you say about all of
these is fine.

>> 
>> - Is 1.1 really useful to have here? That text is basically part of
>> [1] which is what an implementer would need to read anyway. So it 
>> may be better to omit 1.1 and just point at [1].
>> 
>> [1] https://tools.ietf.org/html/rfc7539#section-2.8
> 
> Sure.  It can be reduced to a simple paragraph that contains a
> pointer, much like the pointer to ASN.1 In section 1.3.
> 
>> - section 2: "...destroys the security guarantees." That would be
>> better with a reference to some paper that describes the issues.
> 
> This is explained in RFC 7539.  It says:
> 
> Consequences of repeating a nonce: If a nonce is repeated, then both 
> the one-time Poly1305 key and the keystream are identical between
> the messages.  This reveals the XOR of the plaintexts, because the
> XOR of the plaintexts is equal to the XOR of the ciphertexts.
> 
> I’m pleased to point to that.
> 
> 
>> - typo: "I can be extremely..."
>> 
>> - typo: "There is one algorithm identifiers...”
> 
> I’m pleased to correct those.
> 
> 
>> - section 3: I'm not clear what is done with the "AuthEnvelopedData
>> mac field" here.  Given we've an AEAD, there's no real need for a 
>> separate MAC, so I find the text confusing. Can you explain?
> 
> RFC 7539 says:
> 
> chacha20_aead_encrypt(aad, key, iv, constant, plaintext): nonce =
> constant | iv otk = poly1305_key_gen(key, nonce) ciphertext =
> chacha20_encrypt(key, 1, nonce, plaintext) mac_data = aad |
> pad16(aad) mac_data |= ciphertext | pad16(ciphertext) mac_data |=
> num_to_4_le_bytes(aad.length) mac_data |=
> num_to_4_le_bytes(ciphertext.length) tag = poly1305_mac(mac_data,
> otk) return (ciphertext, tag)
> 
> This is where the authentication tag is carried.  Authentication tag
> is really another name for MAC.
> 
> 
>> - section 6: This says "When using AEAD_CHACHA20_POLY1305, the
>> resulting ciphertext is always the same size as the original
>> plaintext. " What about the extra 128 bit authentication tag?
> 
> As above, the authentication tag is carried in the AuthEnvelopedData
> mac field.

I'd say adding a note to that effect somewhere would help
the reader. (Though an implementer will get it right I
guess.)

Cheers,
S.

> 
> Russ
>