Thank you for the thorough feedback Tero. Sorry for the delay.
I just incorporated all your feedback except the test vectors in the -09
version
https://github.com/csosto-pk/pq-mlkem-ikev2/blob/main/draft-kampanakis-ml-kem-ikev2-09.txt
. I will submit it after the cutoff. I am tracking the test vectors in a git
issue and I will address it later.
-----Original Message-----
From: Tero Kivinen <kivi...@iki.fi>
Sent: Friday, September 27, 2024 8:35 PM
To: draft-kampanakis-ml-kem-ik...@ietf.org
Cc: ipsec@ietf.org
Subject: [EXTERNAL] Cmments for the draft-kampanakis-ml-kem-ikev2-08
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
While doing my IANA review I found out some comments that might need fixing in
the draft-kampanakis-ml-kem-ikev2-08.
First of all add actual RFC title when refering to them.
It is much more reader friendly to say
To address this concern, Mixing Preshared Keys in IKEv2 [RFC8784]
introduced ...
instead of
To address this concern, [RFC8784] introduced ...
Do this throughout of the draft, and also include titles for FIPS documents too.
--
In section 1.1 it would be more useful to explain what is transferred between
Initiator and Responder and where each of the operation is done, i.e., Figure 1
for FIPS203 explains things properly, and I think the list here should include
bit similar overview text.
I.e., change to:
* Initiator: 'KeyGen() -> (pk, sk)': A probabilistic key generation
algorithm, which generates a public / encapsulation key 'pk' and
a private / decapsulation key 'sk'. The resulting pk is sent to
the responder in the KEi payload.
* Responder: 'Encaps(pk) -> (ct, ss)': A probabilistic
encapsulation algorithm, which takes as input a public key 'pk'
(from the KEi) and outputs a ciphertext 'ct' and shared secret
'ss'. The ct is sent back to intiator in the KEr payload.
* Initiator: 'Decaps(sk, ct) -> ss': A decapsulation algorithm,
which takes as input a secret key 'sk' and ciphertext 'ct' (from
the KEr) and outputs a shared secret 'ss', or in some cases a
distinguished error value.
This will make it much clearer fo the reader what operation is run where, and
what is sent inside the KE payloads. The section 2.1 tries to explain this but
is quite confusing...
I am not sure whether the last paragraph of the section 1.1 really belongs
there. It explains the properties of the KEM, but this is not something that
IKEv2 implementors need to know for KEM overview section, and belongs more to
the security considerations section than here. And some of it is already
explained in the security considerations section in situations where it affect
implementations.
Perhaps remove the last paragraph and move some of to security considerations
section.
--
Section 2.1 summary of RFC9370 is quite confusing. Especially the partabout
what is transferred in KEi(1) and KEr(1). Perhaps rewrite
KEi(1) and KEr(1) are the subsequent key
exchange messages which carry the ML-KEM public key of a keypair (sk,
pk) generated by the initiator with ML-KEM KeyGen() and the 256-bit
ML-KEM shared secret SK(1) encapsulated by the responder to a
ciphertext ct by using Encaps(pk) respectively. The initiator then
decapsulates the 256-bit ML-KEM shared secret SK(1) from the
ciphertext ct by using its private key sk in Decaps(sk, ct). Both
peers have now reached a common SK(1) at the end of this KE(1) key
exchange.
to
The initiator will generate ML-KEM keypair (sk, pk) using KeyGen(),
and sends the public key (pk) to the responder inside the KEi(1)
payload. The responder will encapsulate shared secret using
Encaps(pk) and resulting ciphertext (ct) is sent to initiator using
the KEr(1). When initiator receives KEr(1) it will decapsulate it
using Decaps(sk, ct). Both Encaps and Decaps return the shared
secret (ss) and now both peers have common shared secret SK(1).
--
Section 2.2 first paragraph is misleadin and/or wrong. There is no point of
repeating things from the base IKEv2 specification etc. For example the Next
Payload value of the IKE header will not be 34 (Key Exchange), it will be 46
Encrypted and authenticated etc...
So just remove the first paragraph competely:
HDR, the IKE header, of the IKE_INTERMEDIATE messages carrying the
ML-KEM key exchange has a Next Payload value of 34 (Key Exchange),
Exchange Type of 43 (IKE_INTERMEDIATE) and Message ID of 1 assuming
this is the first additional key exchange (ADDKE1). For
IKE_FOLLOWUP_KE messages carrying the ML-KEM key exchange, the
Exchange Type would be 44 (IKE_FOLLOWUP_KE).
Then change
The IKE_INTERMEDIATE or IKE_FOLLOWUP_KE payload is shown below as
defined in Section 3.4 of [RFC7296]:
to:
The KE payload is shown below and the fields inside it has meaning
as defined in Section 3.4 of [RFC7296]:
But then you are actually missing what is stored in the KE payload, the next
paragraph does not really specify what is in it, so change:
Table 1 shows the Payload Length, Key Exchange Method Num identifier
and the Key Exchange Data Size in Octets for Key Exchange Payloads
from the initiator and the responder for the ML-KEM variants specific
in this document. The public key and the ciphertext in the Key
Exchange Data are encoded as raw byte arrays as defined in [FIPS203].
to
The Key Exchange Data from initiator to responder contains the
public key (pk) from the KeyGen() operation encoded as raw byte
array (i.e., output of ByteEncode) as defined in section 7.1 of
Module-Lattice-Based KEM standard [FIPS203).
The Key Exchange Data from responder to initiator contains the
ciphertext (ct) from the Encaps operation encoded as raw byte
array.
Table 1 shows the Payload Length, Key Exchange Method Num identifier
and the Key Exchange Data Size in Octets for Key Exchange Payloads
from the initiator and the responder for the ML-KEM variants specific
in this document.
--
In section 2.3 the text explaining error cases should be separated to initiator
and responder cases. The responder can send INVALID_SYNTAX notification back if
encapsulation key check descripbed in the section
7.2 of Module-Lattice-Based KEM standard [FIPS203), but initiator can't do that
as error happens when we process response. What needs to be done is similar
text than we have in 2.21.2 about AUTHENTICATION_FAILED.
So section 2.3 should change from:
Receiving and handling of malformed ML-KEM public key or ciphertext
SHOULD follow the input validation described in [FIPS203]. In
particular, entities receiving the ML-KEM public key to encapsulate
to SHOULD perform the checks in Section 7.1 of [FIPS203] and reject
the ML-KEM public key, if malformed. Entities receiving an ML-KEM
ciphertext for decapsulation SHOULD perform the ciphertext checks in
Section 7.2 of [FIPS203] and reject the ciphertext, if malformed.
These checks could be performed separately before performing the
encapsulation or decapsulation steps or be part of them. The
response message to an IKE_SA_INIT, or IKE_INTERMEDIATE, or
IKE_FOLLOWUP_KE with malformed ML-KEM public keys or ciphertext MUST
be a Notify payload of type INVALID_SYNTAX.
to:
Receiving and handling of malformed ML-KEM public key or ciphertext
SHOULD follow the input validation described in
Module-Lattice-Based KEM standard [FIPS203].
Responder SHOULD perform the checks specified in section 7.2 of
Module-Lattice-Based KEM standard [FIPS203] before doing Encaps(pk)
operation. If the checks fail it SHOULD send Notify payload of type
INVALID_SYNTAX as a response to the request from initiator.
Initiator SHOULD perform the checks specified in section 7.3 of
Module-Lattice-Based KEM standard [FIPS203] before doing Decaps(sk,
ct) operation. If the checks fail the initiator MUST reject the
ciphertext and MUST fail the exchaneg. In this case the initiator
MAY send Notify payload of type INVALID_SYNTAX to responder as a
separate INFORMATIONAL exchange, usually with no other payloads.
This is an exception for the general rule of not starting new
exchanges based on errors in responses.
--
In the security considerations section there is text saying:
Alternatively, the PRF combiner that derives the PQ/T
Hybrid SKEYSEED and KEYMAT could be updated to include the
ciphertexts which could be proven IND-CCA2 secure following similar
methodology to [PQ-PROOF2].
Is this text talking about possible future change to the IKEv2 to make it
IND-CCA2 or what? If this is talking about future change, I think we should
remove that text, as it does not belong here, but in the document that will
make that change. If this is something than actually affects current
implementations then it can stay here.
--
This document is missing test vectors it would be really good if test vectors
for each of the KEMs could be added. I.e. something that has hex printouts of
the pk, sk, ct, and ss, and then the KEi and KEr payloads, and resulting SK ==
ss.
I think some of the IKEv2 implementations already have test code for ML-KEM, so
perhaps someone having such implementation could generate those outputs. This
would at least allow people to test their pre encapsulation and decapsulation
tests.
--
None of the above is somehing that would block getting IANA numbers to be
allocated for this, thus I will send OK to IANA for allocations now (Valery
OKed it already, but wanted to have my input too).
--
kivi...@iki.fi
_______________________________________________
IPsec mailing list -- ipsec@ietf.org
To unsubscribe send an email to ipsec-le...@ietf.org
