Hi Tero,
CJ has already made changes to the github copy of the draft (many thanks to
him!).
Few comments below.
> -----Original Message-----
> From: IPsec <ipsec-boun...@ietf.org> On Behalf Of Tero Kivinen
> Sent: Saturday, June 4, 2022 8:36 PM
> To: draft-ietf-ipsecme-ikev2-multiple...@ietf.org
> Cc: ipsec@ietf.org
> Subject: [IPsec] My shepherd review to
draft-ietf-ipsecme-ikev2-multiple-ke
>
> In section 1.2 the text is describing the protocol in bit too much
historic point
> of view, it could benefit from more direct approach.
>
> For example changing:
>
> Some post-quantum key exchange payloads may have sizes larger than
> the standard maximum transmission unit (MTU) size, and therefore
> there could be issues with fragmentation at the IP layer. IKE does
> allow transmission over TCP where fragmentation is not an issue
> [RFC8229]; however, we believe that a UDP-based solution will be
> required too. IKE does have a mechanism to handle fragmentation
> within UDP [RFC7383], however that is only applicable to messages
> exchanged after the IKE_SA_INIT exchange. To use this mechanism,
> this specification relies on the IKE_INTERMEDIATE exchange as
> outlined in [I-D.ietf-ipsecme-ikev2-intermediate].
>
> to something like
>
> Some post-quantum key exchange payloads may have sizes larger than
> the standard maximum transmission unit (MTU) size, and therefore
> there could be issues with fragmentation at the IP layer. To allow
> using those larger payload sizes this mechanism relies on the
> IKE_INTERMEDIATE exchange as specified in
> [I-D.ietf-ipsecme-ikev2-intermediate].
Done.
> I.e., as the current protocol as specified here always uses the
> IKE_INTERMEDIATE, there is no point of explaining the issue with
IKE_SA_INIT
> not being able to be fragmented, or whether we use TCP or not. Those do
not
> really matter. Also I think the I-D.ietf-ipsecme-ikev2-intermediate
actually do
> specify the protocol, not just outline it :-)
>
> The text after that do explain that we use RFC7383 fragmentation etc to
allow
> those larger messages to go through.
Changed.
> --
>
> Also in section 1.2 change "discussed" in:
>
> A short term solution
> to make IKEv2 key exchange quantum secure is to use post-quantum pre-
> shared keys as discussed in [RFC8784].
> ^^^^^^^^^
>
> to "specified", or "described" etc. I think RFC8784 does bit more than
just
> discuss about post-quantum pre-shared keys....
Fixed.
> --
>
> In section 1.2 change "draft" to "specification" in:
>
> This draft does not attempt to address key exchanges with KE payloads
> longer than 64k;
Fixed.
> --
>
> In section 2 remove "proposed" in:
>
> The design of the proposed extension is driven by the following
> criteria:
OK.
> --
>
> In section 2 change "ECDH" to "(EC)DH":
>
>
> Hybrid. Currently, there does not exist a post-quantum key
> exchange that is trusted at the level that ECDH is trusted
> against conventional (non-quantum) adversaries.
>
> The current IKEv2 have both normal DH and ECDH, and I think we still
consider
> both of them to be trusted, and we do use term "(EC)DH"
> elsewhere in the draft. Actually we also use "DH or ECDH" and "DH and
ECDH"
> quite a lot, so we could harmonize all of them to "(EC)DH"
> through out the draft.
Done.
> --
>
> In section 3.1 change "the proposed framework" to "this specification"
> in:
>
>
> In order to be able to use IKE fragmentation [RFC7383] for those key
> exchanges that may have long public keys, the proposed framework
> utilizes the IKE_INTERMEDIATE exchange defined in
> [I-D.ietf-ipsecme-ikev2-intermediate].
Done.
> --
>
> In section 3.2.1:
>
> If the responder selected NONE for some Additional Key Exchange types
> (provided they were proposed by the initiator), then the
> corresponding IKE_INTERMEDIATE exchanges should not take place. The
> IKE_INTERMEDIATE exchanges MUST only be performed for Additional Key
> Exchange types containing non-NONE responders choices.
>
> The "should not take place" in first sentence and "MUST only be performed"
in
> second sentence are contradictionary. I think changing the first sentence
so
> that instead of saying "should not take place"
> to "MUST NOT take place" is correct, but then we say same thing twice...
The second sentence is removed.
> Also perhaps we should also said that if initiator jumps over some
additional
> key exchange (like in example we have AKE2, AKE3, and AKE5, but no AKE4),
> then the corresponding IKE_INTERMEDIATE exchange is also omitted (the
> second "MUST only" sentence do cover this as if we do not have AKE4, we
> can't have non-NONE value there, but do we want to mention this case
> explictly?).
Some text added.
> The end of that paragraph seems to have something missing:
>
> It means that
> if the initiator includes NONE in all Additional Key Exchange
> transforms and the responder selects this value for all of them, then
> no IKE_INTERMEDIATE exchanges will take place between the peers.
> perform additional key exchanges will take place (note that they
> still may take place for other purposes).
>
> (i.e. text starting "perform additional key exchanges" seems to be
misplaced,
> or miss something).
This is the leftover from some edits. Removed.
> --
>
> In section 3.2.5 I think the text:
>
> One such example is in G-IKEv2 protocol
> [I-D.ietf-ipsecme-g-ikev2] where cryptographic materials are
> exchanged in IKE_SA_INIT messages between group member and the group
> controller.
>
> is incorrect, as I do not think g-ikev2 has any cryptographic material in
> IKE_SA_INIT, but it do have cryptogarphic material in the GSA_AUTH, so
> replace IKE_SA_INIT with GSA_AUTH.
Sure, I noticed this typo before while reviewing the draft. Already fixed.
> In appendix A, indicate the "Diffie-Hellman Group Num" in KE[ir]* payloads
of
> the examples. For example in A.2 change the line:
>
> HDR(CREATE_CHILD_SA), SK{ SAi(.. AKE*...), Ni, KEi } ---> ...
> Nr, KEr,
>
> to
>
> HDR(CREATE_CHILD_SA), SK{ SAi(.. AKE*...), Ni, KEi(Curve25519) } --->
...
> Nr, KEr(Curve25519),
>
>
> and
>
> HDR(IKE_FOLLOWUP_KE), SK{ KEi(1), --->
> N(ADDITIONAL_KEY_EXCHANGE)(link1) }
> <--- HDR(IKE_FOLLOWUP_KE), SK{ KEr(1),
> N(ADDITIONAL_KEY_EXCHANGE)(link2) } to
>
> HDR(IKE_FOLLOWUP_KE), SK{ KEi1(PQ_KEM_2), --->
> N(ADDITIONAL_KEY_EXCHANGE)(link1) }
> <--- HDR(IKE_FOLLOWUP_KE), SK{ KEr1(PQ_KEM_2),
> N(ADDITIONAL_KEY_EXCHANGE)(link2) }
>
> and
>
> HDR(IKE_FOLLOWUP_KE), SK{ KEi(2), --->
> N(ADDITIONAL_KEY_EXCHANGE)(link2) }
> <--- HDR(IKE_FOLLOWUP_KE), SK{ KEr(2) }
>
> to
>
> HDR(IKE_FOLLOWUP_KE), SK{ KEi2(PQ_KEM_5), --->
> N(ADDITIONAL_KEY_EXCHANGE)(link2) }
> <--- HDR(IKE_FOLLOWUP_KE), SK{ KEr2(PQ_KEM_5) }
>
Done.
> --
>
> In A.3 example the text:
>
> The
> initiator indicates, using the key exchange method NONE, that either
> PQ_KEM_1 or PQ_KEM_2 must be used to establish a security
> association.
>
> is misleading. The initiator indicates using key exchange method NONE,
that
> selecting PQ_KEM_3 or PQ_KEM_4 is optional, but PQ_KEM_1 or
> PQ_KEM_2 is mandatory.
>
> Change that to:
>
> The initiator
> indicates, that either PQ_KEM_1 or PQ_KEM_2 must be used to
> establish a security association, but AKE2 is optional so responder
> can either selecte PQ_KEM_3, or PQ_KEM_4, or omit the both of the
> key exchanges by selecting NONE.
>
> --
>
> It would be good idea to also have one exchange where everything goes
> correctly and we use some of the additional key exchanges also in the
> IKE_INTERMEDIATE exchange, i.e., same example as A.2 but with using
> IKE_INTERMEDIATE instead of IKE_FOLLOWUP_KE, perhaps even explain the
> SKEYSEED, SK_d and SK_{a,e}{i,r} key updates.
Done.
> --
>
> In appendix B I think the text:
>
> payloads instead. In fact, as described above, we use the KE
> payload in the first IKE_SA_INIT request round and the notify
> payload to carry the post-quantum proposals and policies. We use
> one or more of the existing KE payloads to carry the hybrid public
> values.
>
> is misleading, as I do not think we use "and the notify payloads to carry
the
> post-quantum proposals and policies". I would remove that part completely.
Done.
> --
>
> As I think the solution we are using is not solved only by
IKE_INTERMEDIATE,
> but also generic IKEv2 fragmentation, so the text:
>
> service (DoS) attacks. By using IKE_INTERMEDIATE to transport the
> large post-quantum key exchange payloads, there is no longer any
> issue with fragmentation.
>
> is bit misleading and perhaps changing it to:
>
> service (DoS) attacks. By using IKE_INTERMEDIATE to transport the
> large post-quantum key exchange payloads, and using generic
> IKEv2 fragmentation protocol [RFC7383] solves the issue.
Done.
Regards,
Valery.
> --
> kivi...@iki.fi
>
> _______________________________________________
> IPsec mailing list
> IPsec@ietf.org
> https://www.ietf.org/mailman/listinfo/ipsec
_______________________________________________
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec
