Hi Rebecca, Thanks for the detailed review. Please see inline
On Wed, 9 Apr 2025 at 20:50, Rebecca Guthrie <rmgu...@uwe.nsa.gov> wrote: > Thank you for writing and sharing the updated version of the draft. I > think it is in great shape. Some comments below (organized by section): > > > > About This Document > > The Datatracker link is broken: it's missing "-auth" at end of the draft > name. > > > > > > Section 1 > > "The Internet Key Exchange, or IKEv2 [RFC7296], is a key agreement and > security negotiation protocol; it is used for key establishment in IPsec. > In the initial set of exchanges, both parties independently select and use > their preferred authentication method, which may even differ between the > initiator and the responder. In IKEv2, it occurs in the exchange called > IKE_AUTH." > > -> > > "The Internet Key Exchange, or IKEv2 [RFC7296], is a key agreement and > security negotiation protocol; it is used for key establishment in IPsec. > In the IKE_AUTH exchange, the initiator and responder independently select > and use their preferred authentication method, which may differ between > peers." > > Edited for clarity and conciseness > Thanks, updated. > > > "One option for the authentication method" -> "The most common > authentication method" > Fixed. > > > "traditional digital signatures" > > Consider adding a reference here to > draft-ietf-pquip-pqt-hybrid-terminology. I see that it is referenced in > Section 2, but it would be good to cite it the first time the term > "traditional" is used as well. > I don't see a need for it as it is already referenced in Section 2. > > > "presence" -> "existence" > Okay. > > > First paragraph hyphenates "public-key" and second does not; should be > consistent throughout. > Fixed. > > > > > Section 2 > > Section 1 mostly uses the term "public key" while Section 2 favors > "asymmetric." I don't think it matters which you use as long as the > document is consistent with draft-ietf-pquip-pqt-hybrid-terminology and it > is stated somewhere that the two terms are synonymous (for those reading > who are not as familiar with cryptography). > Updated Section 1 to use ""asymmetric". > > > > > Section 3 > > "IKEv2 authentication relies on digital signatures" -> "IKEv2 > authentication commonly relies on digital signatures" > > Shared key message integrity code, generic secure password authentication > method, and NULL authentication are also options for authentication method, > though they may not be commonly deployed > Fixed. > > > > > Section 3.2 > > Consider stating that the terms 'deterministic' and 'hedged' in the first > sentence are being used in accordance with FIPS 204 and 205 (and not being > defined in this document). If other digital signature algorithms are > standardized in the future, it may not be the case that they can be > generated in one of these two modes- if these future algs are supported in > IKEv2 using this generic mechanism, it will be useful to have clarification > here that these terms are associated specifically with ML-DSA and SLH-DSA. > Good point, updated draft. > > > "The hedged mode mitigates this risk by incorporating both fresh > randomness generated at signing time and precomputed randomness included in > the signer’s private key." -> "The hedged mode mitigates this risk by > including precomputed randomness in the signer's private key and > incorporating fresh randomness generated at signing time." > Updated. > > > "Therefore, PQC signature algorithms" > > I don't follow why this sentence can be deduced from the previous one > (hence "therefore"). Consider either making this clearer, or removing > "therefore" (no preference for which) > Fixed. > > > "If the PQC signature algorithm uses a 'context' input parameter, it will > be set to an empty string." Is this "will" meant to be normative? (MUST?) > Yes, updated. > > > Consider writing "pure" in quotes the first time it is used, since it is > not explicitly defined in FIPS 204 and only used once in the document (in > quotes there as well). (did not consult FIPS 205 here, just 204- ignore if > FIPS 205 uses the term differently or defines it more formally) > Okay. > > > "In contrast," must be an artifact from a previous version of the draft. > Should this sentence be preceded with an explanation of pure mode? > Good catch, fixed. > > > "and initial exchange messages" -> "messages preceding IKE_AUTH" Not > imperative, but might add a little clarity and make it clearer that this > would also include IKE_INTERMEDIATE and not just IKE_SA_INIT. > Updated. > > > "which are typically within device memory constraints" > > I think this clause is included to justify the exclusion of pre-hash mode > from the draft, but it isn't explicitly stated. Removing the clause from > this sentence and rephrasing the sentence that follows could fix this: > > "As discussed in Section 7, while pre-hash mode was considered for > integration into IKEv2, various practical challenges led to the adoption of > pure mode." -> "While pre-hash modes can help mitigate complications that > may arise due to device memory constraints, the aforementioned > authentication data typically falls within these constraints; this, coupled > with various practical challenges (as discussed in Section 7), led to the > sole specification of "pure" modes." > Thanks, updated text for clarity. > > > > > Section 3.2.1 > > "The implementation MUST send a SIGNATURE_HASH_ALGORITHMS notify with an > 'Identity' (5) hash function." > > This sentence could use more detail: specifically, that both peers send > this notify payload, that the notify payload is specified in RFC7427, that > the peers send the payload in IKE_SA_INIT. Consider reiterating from > RFC7427 what the content of the notify payload looks like before saying > that it has to use the 'Identity' (5) hash function, as it might be > confusing what the purpose of 'Identity' is without that context. Also > consider including a reference to section 2 of RFC8420 (where identity is > defined). It should be clear from this document (without needing to visit > RFC8420) what the "(5)" represents. > > > > "PQC signature algorithms that inherently operate on the raw message > without preprocessing are only defined with the 'Identity' hash and MUST > NOT be used with a receiver that has not indicated support for the > 'Identity' hash." > > This sentence confused me- I think the MUST NOT is redundant to the > previous sentence's requirement for 'Identity' - is there another point > this sentence attempts to illustrate beyond the normative requirement (i.e. > that the previous sentence does not capture)? I do not think anything would > be lost from the section if this sentence were removed. > Thanks, re-worded text. > > > > > Section 3.3 > > "types of public/private key pairs" -> "digital signature algorithms and > parameters" > Fixed. > > > Consider including another sentence or two for motivation at the beginning > of this section. It would be useful to identify why peers might want to > signal the algorithms they support, and why it hasn't been an issue until > now that they are not able to identify signature algorithms in advance of > using them. > > Are there scenarios where it might not be necessary to signal these > algorithms? For example, when IPsec is used within an enterprise, it may be > the case that only one PQ algorithm/security level is supported, so the > inclusion of the SIGNATURE_HASH_ALGORITHMS notify payload may be enough to > implicitly indicate that the peer supports the PQ algorithm (and not just > whatever traditional algorithms they used prior to migrating, presuming the > traditional algorithms did not leverage SIGNATURE_HASH_ALGORITHMS). > Good point, updated the text to justify the need for it. > > > > > Section 4 > > "(part of the CRYSTALS suite)" suggest either removing this or adding a > reference > > > > "based on the hardness lattice problems over module lattices" should this > maybe say "hardness of"? Not sure. > > > > "Lyubashevsky, " -> "Lyubashevsky" (remove comma) > > > > "lattice based" -> "lattice-based" > > > > "ML-DSA uses uniform distribution" -> "ML-DSA uses a/the uniform > distribution" (missing an article- either could work) > Fixed all above nits. > > > "security categories 2, 3 and 5" -> "security categories 2, 3, and 5" > > Also consider ether explaining what is meant by each category or adding a > reference to the NIST document that defines security levels (I believe this > is the correct document- pages 16-18: > https://csrc.nist.gov/CSRC/media/Projects/Post-Quantum-Cryptography/documents/call-for-proposals-final-dec-2016.pdf > ) > Updated to point to Section 10 of "PQC for Engineers" draft. > > > > > Section 5 > > "to sign up to 2^64 messages" -> "to sign up to 2^64 messages per > instantiation" or something along those lines? > Please elaborate on the proposed change. > > > "The parameters for each of the security levels were chosen to provide 128 > bits of security, 192 bits of security, and 256 bits of security." -> "The > parameters for security levels 1, 3, and 5 were chosen to provide 128, 192, > and 256 bits of security respectively." > Updated. > The link is never made between security level and bits of security. It > could make the list of 12 combinations confusing to interpret (i.e., make > it hard for a reader to determine which options correspond to which > security level). > Updated to point to Section 10 of "PQC for Engineers" draft. > > > "at three security levels" -> "at each level" > Okay. > > > "It includes the small (S) or fast (F) versions of the algorithm. For > security level 1, SHA-256 ([FIPS180]) is used. For security levels 3 and > 5, SHA-512 ([FIPS180]) is used. SHAKE256 ([FIPS202]) is applicable for all > security levels." > > This excerpt confused me. I think it could be more clearly stated that for > each security level, a small version and a fast version are specified, > resulting in 6 different options. And then for each of these options, there > are two different variants (one instantiated with either SHA-256 or SHA-512 > depending on the security level, and the other instantiated with SHAKE256 > for all security levels), resulting in 12 options total. > Thanks, updated text. > > > "for a post-quantum world" -> "in the face of a CRQC" (a bit more specific) > Fixed. > > > "While attacks on lattice-based schemes like ML-DSA can compromise their > security" > > Perhaps rephrase to make clear that the attacks being referred to here are > hypothetical. > Updated text. > > > > > Section 6 > > Earlier, the document states that only "pure" signature modes are > specified. Consider making it clearer why the document does not consider > the ExternalMu variant to be a pre-hash variant or make sure to otherwise > align the rest of the document with the inclusion of ExternalMu. > Good point, updated text. > > > "crypto library" -> "cryptographic library" > > > > "an hash" -> "a hash" > > > > "which takes the hash" -> "which uses the hash" > Fixed all above. > > > > > Section 7 > > "into IKEv2, not just the method proposed above" -> "into IKEv2 other than > those proposed above" > > > > "first apply one of a number of hash functions" -> "first apply a hash > function" > Fixed. > > > In reason number 2, it might also be worth mentioning that the fact that > prehashed and "pure" variants use different OIDs could also create issues > within the IKEv2 protocol if a peer feels the need to support both variants. > Updated draft. > > > "while IKEv2 normally acts this way" what way? > Fixed text. > > > There is brief discussion of how RFC8420 provides an alternative method > for EdDSA- how is the solution there different from what is proposed here > (since both use Identity in SIGNATURE_HASH_ALGORITHMS)? > It is not different. > > > "as current" -> "as specified by the pre-hash modes in [FIPS204] and > [FIPS205]" > Updated text. > > > "but instead of running ML-DSA or SLH-DSA in prehash mode" -> "but instead > of proceeding with each pre-hash mode as specified" > The current text looks good to me. > > > "we have it sign it in pure mode as if it was the message" -> "the hash is > signed as if it were the unhashed message, as is done in "pure" mode" > Updated. > > > > > Section 8 > > "Both ML-DSA and SLH-DSA can utilize their hedged versions when used > within IKEv2." > > Does this document support only hedged signatures, or both hedged and > deterministic? Suggest rephrasing as follows: > > "IKEv2 peers can use either the hedged or deterministic variants of ML-DSA > and SLH-DSA for authentication in IKEv2." > > Hedged and deterministic variants are discussed in other parts of the > document as well. Consider including pointers between related/repeated > content that spans sections. > Yes, updated draft. > > > "In both cases, the 'context' input parameter for the signature generation > algorithm is set to an empty string." > This is already stated once in Section 3.2 and twice in Section 3.2.1 - > consider removing here. > Done. > > > > > 9 > > "The Security Considerations section" -> "The Security Considerations > sections" > > > > "applies" -> "apply" > Fixed. Please see the PR https://github.com/tireddy2/ikev2-pqc-auth/pull/19 to address all of the above comments. Cheers, -Tiru > > > Rebecca > > > > Rebecca Guthrie > > she/her > > Center for Cybersecurity Standards (CCSS) > > Cybersecurity Collaboration Center (CCC) > > National Security Agency (NSA) > > > > *From:* tirumal reddy <kond...@gmail.com> > *Sent:* Monday, April 7, 2025 1:31 AM > *To:* ipsec@ietf.org > *Subject:* [IPsec] Fwd: I-D Action: > draft-ietf-ipsecme-ikev2-pqc-auth-01.txt > > > > The revised draft > https://www.ietf.org/archive/id/draft-ietf-ipsecme-ikev2-pqc-auth-01.html > addresses the comments received during the WG adoption call. It proposes a > generic mechanism for integrating PQC digital signature algorithms into > IKEv2. > > Comments and suggestions are welcome. > > -Tiru > > > > ---------- Forwarded message --------- > From: <internet-dra...@ietf.org> > Date: Mon, 7 Apr 2025 at 10:55 > Subject: [IPsec] I-D Action: draft-ietf-ipsecme-ikev2-pqc-auth-01.txt > To: <i-d-annou...@ietf.org> > Cc: <ipsec@ietf.org> > > > > Internet-Draft draft-ietf-ipsecme-ikev2-pqc-auth-01.txt is now available. > It > is a work item of the IP Security Maintenance and Extensions (IPSECME) WG > of > the IETF. > > Title: Signature Authentication in the Internet Key Exchange Version > 2 (IKEv2) using PQC > Authors: Tirumaleswar Reddy > Valery Smyslov > Scott Fluhrer > Name: draft-ietf-ipsecme-ikev2-pqc-auth-01.txt > Pages: 13 > Dates: 2025-04-06 > > Abstract: > > Signature-based authentication methods are utilized in IKEv2 > [RFC7296]. The current version of the Internet Key Exchange Version > 2 (IKEv2) protocol supports traditional digital signatures. > > This document specifies a generic mechanism for integrating post- > quantum cryptographic (PQC) digital signature algorithms into the > IKEv2 protocol. The approach allows for seamless inclusion of any > PQC signature scheme within the existing authentication framework of > IKEv2. Additionally, it outlines how Module-Lattice-Based Digital > Signatures (ML-DSA) and Stateless Hash-Based Digital Signatures (SLH- > DSA), can be employed as authentication methods within the IKEv2 > protocol, as they have been standardized by NIST. > > The IETF datatracker status page for this Internet-Draft is: > https://datatracker.ietf.org/doc/draft-ietf-ipsecme-ikev2-pqc-auth/ > > There is also an HTML version available at: > https://www.ietf.org/archive/id/draft-ietf-ipsecme-ikev2-pqc-auth-01.html > > A diff from the previous version is available at: > > https://author-tools.ietf.org/iddiff?url2=draft-ietf-ipsecme-ikev2-pqc-auth-01 > > Internet-Drafts are also available by rsync at: > rsync.ietf.org::internet-drafts > > > _______________________________________________ > IPsec mailing list -- ipsec@ietf.org > To unsubscribe send an email to ipsec-le...@ietf.org >
_______________________________________________ IPsec mailing list -- ipsec@ietf.org To unsubscribe send an email to ipsec-le...@ietf.org
