CBOR Object Signing and Encryption (COSE): AES-CTR and AES-CBC
RFC 9459

# Internet AD comments for draft-ietf-cose-aes-ctr-and-cbc-04
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Nits

### S8

* "Accordingly, implementations MUST use of"

  -> "Accordingly, implementations MUST use", or
  -> "Accordingly, implementations MUST ensure use of", or something

* "replace it with their one of their"
  -> "replace it with one of their"

Thanks to Michael B. Jones for the shepherd write-up, without which I would have been a little lost as to why we need this document. The context was helpful.

I have just two small comments about the spec.

First, in

   If an attacker is able to strip the authentication and integrity
   mechanism, then the attacker can replace it with their one of their
   own creation

s/their one/one/

Second, there are two SHOULDs in Section 8. I am curious why they aren’t MUSTs. If SHOULD is the more appropriate choice, would it be possible to provide some commentary as to when it might be fine for an implementor to deviate?

# GEN AD review of draft-ietf-cose-aes-ctr-and-cbc-05

CC @larseggert

Thanks to Vijay Gurbani for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/QYDfbhRi0CKZyV4Wft6YqoOoAPg).

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Grammar/style

#### Section 7, paragraph 2
```
entations should change the key before before reaching this limit. To avoid c
                                ^^^^^^^^^^^^^
```
Possible typo: you repeated a word.

#### Section 8, paragraph 3
```
block values are ever used for more that one plaintext with the same key, th
                                    ^^^^
```
Did you mean "than"?

#### Section 8, paragraph 19
```
to prevent undetected stripping of the the authentication and integrity mecha
                                   ^^^^^^^
```
Possible typo: you repeated a word.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

I concur with John Scudder's remark about the use of SHOULD.

Hi,

Thanks for this document.  I had a few minor comments for you to consider please:

(1) p 2, sec 4.  AES Counter Mode

   When AES-CTR is used as a COSE Content Encryption algorithm, the
   encryptor generates a unique value that is communicated to the
   decryptor.  This value is called an initialization vector (IV) in
   this document.  The same IV and AES key combination MUST NOT be used
   more than once.  The encryptor can generate the IV in any manner that
   ensures the same IV value is not used more than once with the same
   AES key.

Is there any RFC that could be referenced here on how to generate suitable IVs?


(2) p 4, sec 5.  AES Cipher Block Chaining Mode

   AES-CBC mode requires an 16 octet Initialization Vector (IV).  Use of
   a randomly or pseudo-randomly generated IV ensures that the
   encryption of the same plaintext will yield different ciphertext.

The IV initialization text differs compared to section 4, I assume that this is intentional and these modes have fundamentally different IV initialization requirements?                                                                                     
                                                                                                                                                                                                                                                             
                                                                                                                                                                                                                                                             
(3) p 6, sec 5.2.  AES-CBC COSE Algoritm Identifiers                                                                                                                                                                                                         
                                                                                                                                                                                                                                                             
   The following table defines the COSE AES-CBC algorithm values.  Note                                                                                                                                                                                      
   that these algorithms are being registered as "Deprecated" to avoid                                                                                                                                                                                       
   accidental use without a companion integrity protection mechanism.                                                                                                                                                                                        
   +=========+=======+==========+========================+=============+                                                                                                                                                                                     
   | Name    | Value | Key Size |      Description       | Recommended |                                                                                                                                                                                     
   +=========+=======+==========+========================+=============+                                                                                                                                                                                     
   | A128CBC |  TBD4 |   128    |       AES-CBC w/       |  Deprecated |                                                                                                                                                                                     
   |         |       |          |      128-bit key       |             |                                                                                                                                                                                     
   +---------+-------+----------+------------------------+-------------+                                                                                                                                                                                     
   | A192CBC |  TBD5 |   192    |       AES-CBC w/       |  Deprecated |                                                                                                                                                                                     
   |         |       |          |      192-bit key       |             |                                                                                                                                                                                     
   +---------+-------+----------+------------------------+-------------+                                                                                                                                                                                     
   | A256CBC |  TBD6 |   256    |       AES-CBC w/       |  Deprecated |                                                                                                                                                                                     
   |         |       |          |      256-bit key       |             |                                                                                                                                                                                     
   +---------+-------+----------+------------------------+-------------+                                                                                                                                                                                     

I wanted to check that "Deprecated" is really the best choice for "Recommended" for both AES-CTR and AES-CBC.  I read 'deprecated' as meaning that other COSE algorithms should be used in preference to these, but it wasn't clear that is the intent here.  I note that this column contains some entries with a value such as "Filter Only", hence wondering it these should be labelled as "Confidentiality only", perhaps with the description indicating that integrity must be handled separately?

Thanks,
Rob

Thank you to Daniel Migault for the SECDIR review.

** Duplicate normative guidance.  Please say it once.
(a) Instance #1

-- Section 4
For this reason, it is inappropriate to use AES-CTR with static keys. Extraordinary measures would be needed to prevent reuse of an IV value with the static key across power cycles. To be safe, implementations MUST use fresh keys with AES-CTR.

-- Section 8
Therefore, it is inappropriate to use AES-CTR with static keys. Extraordinary measures would be needed to prevent reuse of a counter block value with the static key across power cycles. To be safe, implementations MUST use fresh keys with AES-CTR.

(b) Instance #2

-- Section 4
Implementations MUST use AES-CTR in conjunction with an authentication and integrity mechanism, such as a digital signature.

-- Section 8
Accordingly, implementations MUST use AES-CTR in conjunction with an authentication and integrity mechanism, such as a digital signature

(c) Instance #3

-- Section 4
Implementations MUST use AES-CBC in conjunction with an authentication and integrity mechanism, such as a digital signature.

-- Section 8
Accordingly, implementations MUST use of AES-CBC in conjunction with an authentication and integrity mechanism, such as a digital signature.

** Section 6.  Consistent guidance to implementers:

-- Section 5
Since AES-CBC cannot provide integrity protection for external additional authenticated data, the decryptor MUST ensure that no external additional authenticated data was supplied.
(aside, similar text in Section 4 for AES-CTR)

-- Section 6
COSE libraries that support either AES-CTR or AES-CBC and accept Additional Authenticated Data (AAD) as input should return an error if one of these non-AEAD content encryption algorithm is selected. 

Wouldn’t it be more appropriate to say “must return an error”?  Silent failure seems like a bad idea and inconsistent with the protocol guidance.

** Section 8
Since AES has a 128-bit block size, regardless of the mode employed, the ciphertext generated by AES encryption becomes distinguishable from random values after 2^64 blocks are encrypted with a single key. Implementations should change the key before before reaching this limit.

-- Typo. s/before before/before/

-- Why would it be safe to continue to use the key past 2^64 blocks?

** Section 8.  Editorial.
Since AES-GCM uses AES-CTR for encryption, an attacker can switch the algorithm identifier from AES-GCM to AES-CTR, and then strip the authentication tag to bypass the authentication and integrity, allowing the attacker to manipulate the ciphertext.

This is the first reference to AES-GCM.  All other modes got a definition.  Please explain or cite.

Thanks for working on this specification.

I don't have any transport related comments or issues. However, I was expecting a "MUST NOT" in section 5 regarding same IV usage more than once as it says in section 4.