Use of ML-KEM in the Cryptographic Message Syntax (CMS)
draft-ietf-lamps-cms-kyber-13

Thanks for the work done in this document.

I am trusting the responsible AD,the LAMPS WG chairs, and the shepherd about the IPR situation and the associated consensus.

Minor regret: there is no justification for the intended status in the shepherd write-up.

Thanks for the work is described in this document. I do not see any transport-related concerns for this I-D.

In Section 3, rather than saying "elsewhere", can you reference where the authoritative location(s) of these definitions is/are?

Hi Julien, Mike, and Daniel, 

Thank you for the effort put into this specification.

I trust that the WG/responsible AD checked the validity of the module and various examples.

Please find below some comments, fwiw. Major comments are marked with (*).

# RFC5911 and RFC9629 are normative (*)

These two are needed, for example, for this import

CURRENT:
   This appendix includes the ASN.1 module [X680] for ML-KEM.  This
   module imports objects from [RFC5911], [RFC9629], [RFC8619],
   [I-D.ietf-lamps-kyber-certificates].
 
# Deviation from the NIST spec  (*)

Maybe I’m not looking in the correct NIST reference, but the ciphertext sizes indicated below for ML-KEM-768/1024 does not match with the one indicated in Table 3 of NIST.FIPS.203.

CURRENT:
    +=============+=======+==========+==========+============+========+
    | Parameter   | Level | Encap.   | Decap.   | Ciphertext | Shared |
    | Set         |       | Key Size | Key Size | Size       | Secret |
    |             |       |          |          |            | Size   |
    +=============+=======+==========+==========+============+========+
    | ML-KEM-512  | 1     | 800      | 1632     | 768        | 32     |
    +-------------+-------+----------+----------+------------+--------+
    | ML-KEM-768  | 3     | 1184     | 2400     | 1952       | 32     |
    +-------------+-------+----------+----------+------------+--------+
    | ML-KEM-1024 | 5     | 1568     | 3168     | 2592       | 32     |
    +-------------+-------+----------+----------+------------+--------+

# Lack of reasoning  (*)

CURRENT:
      ukm is an optional random input to the key-derivation function.
      For ML-KEM, ukm doesn't provide any additional security benefits.
      Originators using ML-KEM MAY choose to send a ukm, though there is
      no reason to.  

Don’t get why the parameter is sent then. 

# How to enforce the requirement  (*)

CURRENT:
   When ML-KEM is employed in the CMS, the underlying components used
   within the KEMRecipientInfo structure SHOULD be consistent with a
   minimum desired security level.

I’m not familiar with the conventions here, but this seems to me week characterization of an expected behavior. How to enforced this?
 
# Why these are not MUST? (*)

CURRENT:
      ML-KEM-512 SHOULD be used with a KDF capable of outputting a key
      with at least 128 bits of preimage strength and with a key
      wrapping algorithm with a key length of at least 128 bits.

      ML-KEM-768 SHOULD be used with a KDF capable of outputting a key
      with at least 192 bits of preimage strength and with a key
      wrapping algorithm with a key length of at least 192 bits.

      ML-KEM-1024 SHOULD be used with a KDF capable of outputting a key
      with at least 256 bits of preimage strength and with a key
      wrapping algorithm with a key length of at least 256 bits.

Maybe obvious for the authors, but it would expect some explanation what this is not a MUST.

# Redundant Behaviors  (*)

The above SHOULDs are redundant with the ones in Section 7. Unless I missed subtle things here, please keep the use of normative language in one single place:

Section 7:
   To achieve 128-bit security, ML-KEM-512 SHOULD be used, the key-
   derivation function SHOULD provide at least 128 bits of preimage
   strength, and the symmetric key-encryption algorithm SHOULD have a
   security strength of at least 128 bits.  To achieve 192-bit security,
   ML-KEM-768 SHOULD be used, the key-derivation function SHOULD provide
   at least 192 bits of preimage strength, and the symmetric key-
   encryption algorithm SHOULD have a security strength of at least 192
   bits.  In the case of AES Key Wrap, a 256-bit key is typically used
   because AES-192 is not as commonly deployed.  

# Other comments are provided below

## Abstract

### Regional matters

Please s/NIST/US National Institute of Standards and Technology (NIST)

### Abstract should be self-contained

OLD: using the KEMRecipientInfo structure

NEW: using the KEMRecipientInfo structure defined in “Using Key Encapsulation Mechanism (KEM) Algorithms in the Cryptographic Message Syntax (CMS)” (RFC 9629)
 
## Introduction

* Acronym already introduced in the first sentence

s/Native support for Key Encapsulation Mechanisms (KEMs)/KEMs

## Business as usual

CURRENT:
      |  RFC EDITOR: Please replace the following references to
      |  [I-D.ietf-lamps-kyber-certificates] with a reference to the
      |  published RFC.

No need to have this note as this what the RFC will do anyway :-)

Consider deleting this note and similar ones.

## Section 3

CURRENT:
   All identifiers used to indicate ML-KEM within the CMS are defined
   elsewhere
   ^^^^^^^^ 

Can we please add references where these identifiers were defined? Thanks. 

## Can we please cite Appendix B and Appendix C in the main body?

Hope this helps. 

Cheers,
Med

# Orie Steele, ART AD, comments for draft-ietf-lamps-cms-kyber-11 
CC @OR13

* line numbers:
  - https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-lamps-cms-kyber-11.txt&submitcheck=True

* 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/

## Comments

### ukm

```
221	      no reason to.  For maximum interoperability, recipients using ML-
222	      KEM SHOULD accept and process the ukm.  Recipients that do not
223	      support the ukm field SHOULD gracefully discontinue processing
224	      when the ukm field is present.
```

This is framed weirdly imo.
Failure to process ukm consistently will lead to different key material.

I might consider framing this as:

Recipients who do not understand ukm MUST ignore it, and not raise an error.
Recipients who understand ukm MUST process it in order to achieve interoperability.

I find the treatment of ukm here amusing given its cousin in JOSE:

https://datatracker.ietf.org/doc/html/rfc7518#section-4.6.1.2

### Why not MUST?

```
416	   and ciphertext.  Implementations SHOULD NOT use intermediate values
417	   directly for any purpose.
```

```
419	   Implementations SHOULD NOT reveal information about intermediate
420	   values or calculations, whether by timing or other "side channels",
421	   otherwise an opponent may be able to determine information about the
422	   keying data and/or the recipient's private key.  Although not all
```

Under which circumstances is a side channel useful for interop?

### Weird may

```
433	   Parties MAY gain assurance that implementations are correct through
434	   formal implementation validation, such as the NIST Cryptographic
435	   Module Validation Program (CMVP) [CMVP].
```

[FIPS180], [I-D.kampanakis-ml-kem-ikev2] and [I-D.sfluhrer-cfrg-ml-kem-security-considerations] should be changed from Informative to Normative reference :)

Thank you to Joel Halpern for the GENART review.

** Section 4.
   To achieve 128-bit security, ML-KEM-512 SHOULD be used, the key-
   derivation function SHOULD provide at least 128 bits of preimage
   strength, and the symmetric key-encryption algorithm SHOULD have a
   security strength of at least 128 bits.  To achieve 192-bit security,
   ML-KEM-768 SHOULD be used, the key-derivation function SHOULD provide
   at least 192 bits of preimage strength, and the symmetric key-
   encryption algorithm SHOULD have a security strength of at least 192
   bits.  In the case of AES Key Wrap, a 256-bit key is typically used
   because AES-192 is not as commonly deployed.  To achieve 256-bit
   security, ML-KEM-1024 SHOULD be used, the key-derivation function
   SHOULD provide at least 256 bits of preimage strength, and the
   symmetric key-encryption algorithm SHOULD have a security strength of
   at least 256 bits.

I’m wondering if the editorial construct of “To achieve ###-bit of security, ML-KEM-### SHOULD be used …” is the clearest way to express this guidance.  It doesn’t motivate when you would NOT use this guidance (i.e., SHOULD suggests choice).  Additionally, the stated parameter set is really the minimum to achieve “##-bit security”.

** Appendix A
   This appendix includes the ASN.1 module [X680] for ML-KEM.  This
   module imports objects from [RFC5911], [RFC9629], [RFC8619],
   [I-D.ietf-lamps-kyber-certificates].
AND
     SMIME-CAPS
       FROM AlgorithmInformation-2009  -- [RFC5911]
         { iso(1) identified-organization(3) dod(6) internet(1)
           security(5) mechanisms(5) pkix(7) id-mod(0)
           id-mod-algorithmInformation-02(58) }

This import makes RFC5911 a normative reference.  It is currently informative.

** Idnit says:
  == The document seems to lack the recommended RFC 2119 boilerplate, even if
     it appears to use RFC 2119 keywords -- however, there's a paragraph with
     a matching beginning. Boilerplate error?

Please check that the right boiler plate is being used in Section 1.1