Re: [IPsec] WGLC for draft-ietf-ipsecme-ikev2-multiple-ke

[Paul Wouters <[e-mail]>]

On Tue, 27 Jul 2021, Tero Kivinen wrote:

> Subject: [IPsec] WGLC for draft-ietf-ipsecme-ikev2-multiple-ke
> 
> This is the start of 2 week WGLC on the
> draft-ietf-ipsecme-ikev2-multiple-ke document, ending 2021-08-10.

Note that this document has a prerequisite on the intermediate exchange,
so even if it passed WGLC/IETF LC before intermediate exchange, it will
have to wait in an RFC Editor cluster for the other draft.

> Please submit your comments to the list, also send a note if you have
> reviewed the document, so we can see how many people are interested in
> getting this out.

I have reviewed the document. In general I support this document. I
really like the idea of renaming the DH Registry to KE. I do think it
is not ready yet though. My comments and questions follow below.


 	Key exchange methods negotiated via Transform Type 4 MUST always take
 	place in the IKE_SA_INIT exchange.  Additional key exchanges
 	negotiated via newly defined transforms MUST take place in a series
 	of IKE_INTERMEDIATE exchanges, in an order of the values of their
 	transform types, so that key exchange negotiated using Transform Type
 	n always precedes that of Transform Type n + 1.

I don't understand this section, specifically the use of "Transform Type 4"
and "Transport Type n+1", as we only have transform type 5 and nothing
higher and that is Extended Sequence Number.

https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-8

I think it might be trying to say if there are more than one Key Exchange,
that the subsequent key exchange should follow in the next IKE message
exchange (eg in a round of IKE_INTERMEDIATE) ?

 	Each IKE_INTERMEDIATE exchange MUST bear exactly one key exchange method.

I don't understand why there is this limitation. What if some Key
Exchange mechanism will require 2 RTTs. Why preventively forbid that?

 	Additional key exchange methods are proposed
 	using Additional Key Exchanges transform types.  All these transform
 	types are optional, the initiator is free to select any of them for
 	proposing additional key exchange methods.  Consequently, if none of
 	Additional Key Exchange transforms are included in the proposal, then
 	this proposal indicates performing standard IKEv2, as defined in
 	[RFC7296].

So how does an intiiator convey that it deems an additional Key Exchange
to be mandatory?

 	If the initiator includes any transform of type n (where
 	n is among Additional Key Exchanges) in the proposal, the responder
 	MUST select one of the algorithms proposed using this type.  A
 	transform ID NONE may be added to those transform types which contain
 	key exchange methods that the initiator believes are optional.

And so I again do not understand this. What is "n" here? a new transform
type ? ( eg n=6 ??)  or a new entry in the Transform Type 4 Key Exchange
registry?


At his point, the Additional Key Exchange is introduced, and I am
beginning to understand things. This should really be explained before
the text I pointed at above to make any sense to the reader. And see
below on placing "Additional Key Exchanges" into the "Key Exchanges"
Registry.


The next part explains the CREATE_CHILD_SA and IKE_FOLLOWUP_KE exchanges. I
personally would prefer that a different exchange than CREATE_CHILD_SA
is used if the completion of such an exchange does not lead to a fully
rekeyed state. This use of completing a CREATE_CHILD_SA and being in a
state that is not "rekeyed" or "failed" complicates the state machine.

 	Each IKE_FOLLOWUP_KE exchange MUST bear exactly one key exchange method.

See above. Why this preventative limitation?

 	The data associated with this notification is a blob meaningful only to the responder

Why a blob? Why not the imminent new SPI it generated for this new IKE SA?
If you really want a blob, there should be an example of how to generate
the blobs. I don't see any such guidance in the document.

Below is an example of three additional key exchanges.

    Initiator                             Responder
    ---------------------------------------------------------------------
    HDR(CREATE_CHILD_SA), SK {SA, Ni, KEi} -->
                              <--  HDR(CREATE_CHILD_SA), SK {SA, Nr, KEr,
                                       N(ADDITIONAL_KEY_EXCHANGE)(link1)}


Why does the initiator not start out with a N(ADDITIONAL_KEY_EXCHANGE) ?
There has been an Additional Key Exchange in the initial exchanges, so
why not start out with one in the rekey from the initiator?

What would an initiator do if the responder omited N(ADDITIONAL_KEY_EXCHANGE) ?
It has given no indication it might be mandatory from the initiator's point of view.

 	It is possible that due to some unexpected events (e.g. reboot) the
 	initiator could forget that it is in the process of performing
 	additional key exchanges and never starts next IKE_FOLLOWUP_KE
 	exchanges.  The responder MUST handle this situation gracefully

And wouldn't that solve this weird state issue if the initiator already
signalled this clearly in CREATE_CHILD_SA, so the responder could in
that case already return an error in the CREATE_CHILD_SA exchange?
Note that I find the argument weird, why would an IKE peer forget some
of its state after a reboot? Both peers should always remember AKE's
were used and have to be used again upon (PFS) rekeys.

 	it MUST send back a new error type notification STATE_NOT_FOUND.
 	This is a non-fatal error notification
 	[...]
 	If the initiator receives this notification in
 	response to IKE_FOLLOWUP_KE exchange performing additional key
 	exchange, it MUST cancel this exchange and MUST treat the whole
 	series of exchanges started from the CREATE_CHILD_SA exchange as
 	failed.

So why use a non-fatal error notification that leads to a guaranteed failure ?

Note there seems to be a notion of AKE's having continuous state, as
opposed to (EC)DH where you start a new state with the rekey exchange.
Perhaps this should be clarified at the beginning of the document?


  This document adds the following Transform Types to the "Transform
    Type Values" registry:

    Type     Description                   Used In
    -----------------------------------------------------------------
    <TBA>    Additional Key Exchange 1     (optional in IKE, AH, ESP)
    <TBA>    Additional Key Exchange 2     (optional in IKE, AH, ESP)
    <TBA>    Additional Key Exchange 3     (optional in IKE, AH, ESP)
    <TBA>    Additional Key Exchange 4     (optional in IKE, AH, ESP)


Why are the descriptions referring to "additional" key exchange? I would
assume the registry is just a list of Key Exchange types, and whether
one of these is "additional" or not depends on its use in IKE ? That is,
one of the Key Exchanges that today is "additional" might one day be
used as the only Key Exchange without Additional Key Exchanges? If we
really are making some of these exchanges as "additional use only", then
we should really create a new transform type registry for AKE that is
separate from the Key Exchange Type (4).



 	the key lengths of these
 	transforms SHALL be at least 256 bits long in order to provide
 	sufficient resistance to quantum attacks.

I would use MUST instead of SHALL.

 	The main focus of this document is to prevent a passive attacker
 	performing a "harvest and decrypt" attack.  In other words, an
 	attacker that records messages exchanges today and proceeds to
 	decrypt them once he owns a quantum computer.  This attack is
 	prevented due to the hybrid nature of the key exchange.  Other
 	attacks involving an active attacker using a quantum-computer are not
 	completely solved by this document.  This is for two reasons.

I think this part and the text right underneath this belongs in the
Introduction more than it belongs in the Security Considerations
section.



 	Unfortunately, this design is susceptible to the following
 	downgrade attack.

I'm not convinced this is an issue actually. If you really do think a
proper configuration would not sufficiently protect against this, the
initiator could send a notify in the second IKE_SA_INIT of the rejected
KE value from the previous response, so that this attack would be
detected.

 	We discarded this approach because we believe that the working
 	group may not be happy using the RESERVED field to change the
 	format of a packet and that implementers may not like the
 	complexity added from checking the fragmentation flag in each
 	received payload.

I think that is exactly why we have RESERVED fields. As implementer, I
don't think that this would have been too complex. But I think using
INTERMEDIATE is fine, especially if multiple solutions can use this
method for their own usage, and having a generic method is better than
having specific methods per postquantum/hybrid solution.



Nits:

a typo:  CRETE_CHILD_SA


this side MUST not initiate -> this side MUST NOT initiate



Paul