Re: [secdir] Secdir last call review of draft-ietf-cose-aes-ctr-and-cbc-04

[Russ Housley <[e-mail]>]

> On May 16, 2023, at 2:22 PM, Daniel Migault via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Daniel Migault
> Review result: Ready
> 
> Reviewer: Daniel Migault
> Review result: Ready
> 
> 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
> 
> section 4 AES counter mode
> 
> In "AES encryption of (IV +1) mod 2^128" I am wondering if "mod 2^128" is
> needed as I see the encryption returning a 128 bit block. That said we
> understand why it is there, it is more that I am curious if there is any reason.

If the IV is zero, then the "mod 2^128" has no impact.  If you start with another value, it needs to wrap.

> I am also wondering if we should mention the IV + i is called the counter block
> as this is mentioned in section 8.

Okay.  I think the following does the job:

   AES-CTR has many properties that make it an attractive COSE Content
   Encryption algorithm.  AES-CTR uses the AES block cipher to create a
   stream cipher.  Data is encrypted and decrypted by XORing with the
   key stream produced by AES encrypting sequential IV block values,
   called counter blocks.  The first block of the key stream is the AES
   encryption of the IV, the second block of the key stream is the AES
   encryption of (IV + 1) mod 2^128, the third block of the key stream
   is the AES encryption of (IV + 2) mod 2^128, and so on.  AES-CTR is
   easy to implement, and AES-CTR can be pipelined and parallelized.
   AES-CTR also supports key stream precomputation.  Sending of the IV
   is the only source of expansion because the plaintext and ciphertext
   are the same size.

> The following text sounded cryptic to me until I reached section 6. I suspect
> that adding a reference to section 6 might be useful. The same comment applies
> for CBC.
> 
> """
> Since AES-CTR cannot provide integrity protection for external
> additional authenticated data, the decryptor MUST ensure that no
> external additional authenticated data was supplied.
> """

I suggest:

   Since AES-CTR cannot provide integrity protection for external
   additional authenticated data, the decryptor MUST ensure that no
   external additional authenticated data was supplied.  See Section 6.

and:

   Since AES-CBC cannot provide integrity protection for external
   additional authenticated data, the decryptor MUST ensure that no
   external additional authenticated data was supplied.  See Section 6.

> section 4.2.  AES-CTR COSE Algorithm Identifiers
> 
> In the title “Algoritm” needs to be changed.

Fixed.

> It is surprising to define a "Deprecated", but the note provides the rationale.
> I am wondering if that rationale could be also mentioned in the IANA page -
> this is just a suggestion.

This has been discussed on the thread with Rob Wilton.  Not sure where that will land yet.

> section 5.  AES Cipher Block Chaining Mode
> 
> I believe that another reason for using integrity protection is the
> vulnerability to padding oracle.

When CBC decryption returns an "invalid padding" error instead of a generic "decryption failed" error, then attacker can gain a lot of information.  It is better to not distinguish between these two error cases.

Russ