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

From: Stephen Farrell <[email-masked]>
Date: Thu, 24 Nov 2016 12:54:04 +0000

Hiya,

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.

Cheers,
S.

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

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

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

(4) I hope I remember to add the downref to
the IETF LC:-)

nits...

- 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

- section 2: "...destroys the security
guarantees." That would be better with a
reference to some paper that describes the
issues.

- typo: "I can be extremely..."

- typo: "There is one algorithm identifiers..."

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

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

---

From: Russ Housley <[email-masked]>
Date: Mon, 28 Nov 2016 17:56:29 -0500

> least section 4, but please argue with me if

I do not think the changes are very big.  However, I’m pleased to make them before IETF Last Call, assuming you agree with the direction laid out below.

> can't be enforced, then it's not worth the

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.

> term?  (Apologies if I'm forgetting the WG

RFC 5083 uses the term "content-authenticated-encryption keys”.  I can add the hyphens id that improves clarity.

> you at least need to write the text or get rid

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.

> (4) I hope I remember to add the downref to

For which document?

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

> reference to some paper that describes the

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: "There is one algorithm identifiers...”

I’m pleased to correct those.

> separate MAC, so I find the text confusing.

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.

> original plaintext. " What about the extra 128

As above, the authentication tag is carried in the AuthEnvelopedData mac field.

Russ

---

From: Stephen Farrell <[email-masked]>
Date: Tue, 29 Nov 2016 12:03:22 +0000

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

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.

---

From: Russ Housley <[email-masked]>
Date: Wed, 30 Nov 2016 17:41:02 -0500

Responding only to the comment where we still have a disconnect.

> mean that, and "key+nonce" when that's meant.

No, that is not what is meant.

This key is used to authenticate and encrypt the content.  That is why RFC 5083 calls it the "content-authenticated-encryption key”.

I have just re-read Section 3, and I am at a loss to see where this confusion is coming from.

Russ