[IPsec] My shepherd review to draft-ietf-ipsecme-ikev2-multiple-ke

[Tero Kivinen <[e-mail]>]

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].

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. 

--

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....

--

In section 1.2 change "draft" to "specification" in:

   This draft does not attempt to address key exchanges with KE payloads
   longer than 64k;

--

In section 2 remove "proposed" in:

   The design of the proposed extension is driven by the following
   criteria:

--

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.

--

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].  

--

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...

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?).

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).

--

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.

--

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) }

--

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.

--

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.

--

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.
-- 
[e-mail]