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

[Valery Smyslov <[e-mail]>]

Hi Paul,

> On Fri, 30 Jul 2021, Valery Smyslov wrote:
> 
> [ replying as I got prompted by Tero on this regarding WGLC ]
> 
> >> 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.
> >
> > A one para above new Transform Types are introduced:
> > Additional Key Exchange 1, Additional Key Exchange 2,
> > Additional Key Exchange 3, etc. Once added to IANA registry,
> > they will get their numbers (hopefully in an order of their names).
> > The text is trying to say, that when additional KE are being negotiated,
> > the order in which they will take place is defined by the order
> > of assigned Transform Types (which will correspond to the order of their names).
> >
> > In other words, if at the end of negotiation peers decided that
> > they will perform base KE (negotiated via Transform Type 4, which this draft
> > renames to "Key Exchange Method (KE)") 2048-bit MODP Group,
> > and two additional Key Exchanges - SIKE_P434, negotiated in
> > "Additional Key Exchange 3" transform, and FRODOKEM_640_AES,
> > negotiated in "Additional Key Exchange 5" transform, then
> > they agree to use 2048-bit MODP Group in the IKE_SA_INIT,
> > then SIKE_P434 in the first IKE_INTERMEDIATE
> > followed IKE_SA_INIT and FRODOKEM_640_AES in the next
> > IKE_INTERMEDIATE.
> 
> I guess this all still feels like a bit of a kludge. I wonder if there
> is a way to do this with one new transform type, and an ordered list of
> proposals inside. 

Currently in IKEv2 a single transform specifies a single algorithm. If initiator wants to propose
several algorithms of one type as a choice, it includes in a proposal several transforms.
Do you suggest to change this logic, so that a single transform contains a list of 
of possible algorithms? It would be quit a major change to IKEv2 negotiation
mechanism and in my opinion would lead to more complexity (e.g. how you
associate attributes with algorithms in this case?).

Or am I missing your point?

I understand the desire to specify order, but I feel
> the current method is fragile and error prone. Eg if one end has
> Additional Key Exchange 1 = SIKE_P434, Additional Key Exchange 2 = FRODOKEM_640_AES
> and the other end has Additional Key Exchange 1 = FRODOKEM_640_AES, Additional Key Exchange 2 =
> SIKE_P434,
> where both might not really care which goes first or second.

In your example both initiator and responder do care about the order. That's why they cannot 
negotiate an order appropriate for both. If they don't care then the policy on both ends would be:

Additional Key Exchange 1 = SIKE_P434 or FRODOKEM_640_AES, Additional Key Exchange 2 = SIKE_P434 or FRODOKEM_640_AES

In this case they either end up with SIKE_P434 + FRODOKEM_640_AES or FRODOKEM_640_AES + SIKE_P434

Note, that the draft prohibits negotiation the same algorithm in different additional key exchanges,
so they cannot end up with SIKE_P434 + SIKE_P434 or FRODOKEM_640_AES + FRODOKEM_640_AES.

> I think it would be worth thinking about this problem a bit more.

If you have better proposals in mind we'll be happy to consider them.

> > It is implied here (and defined in Section 3.1). But the real message here is that the order of key exchanges
> > performed in a sequence of IKE_INTERMEDIATE is defined by the order of "Additional Key Exchange *"
> > transforms via which they are negotiated.
> 
> Then please say it clearly, instead of implying it.

OK.

> >>  	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?
> >
> > We didn't consider such exotic key exchange methods and, I think,
> > if they appear, they will deserve a separate document.
> 
> Sure, but it would be best if such a new document would not need to
> Updates: this document just for this one little change. So why not:
> 
> - Each IKE_INTERMEDIATE exchange MUST bear exactly one key exchange method.
> + All Additional Key Exchanges MUST be in their own Exchange - that is
> + these KE's cannot be exchanged via a single IKE message exchange.

To my (non-native) eye this looks more difficult to understand.
We'll think if some more simple text with similar meaning can be crafted.

> I also just realised that Addtional Key Exchange is not the best term
> for this, as it is very close to Internet Key Exchange and people might
> think this is a chain of IKEv2 and something else. Maybe "Additional KE
> Exchange" would be better?

I got your point, but I don't think this possible confusion is real enough to be worry about.
In addition, if in your proposal the acronym KE is expanded,
then we get awkward "Additional Key Exchange Exchange"...

That said, I agree that the term "Key Exchange" (with or without "Additional")
is a bit inaccurate. It's OK for DH or ECDH, but for example in PQ
world most current methods are in fact KEMs.
Probably Key Determination is better? But then we need to rename KE payload
to KD payload and so on...

> > But the real message here is that you cannot perform
> > several key exchanges using a single IKE_INTERMEDIATE exchange.
> > In other words, you cannot put several public keys
> > of different Key Exchange methods in a single IKE_INTERMEDIATE
> > message.
> 
> I tried to phrase it above, but I think it can be improved upon further.

I hope so.

> >> So how does an intiiator convey that it deems an additional Key Exchange
> >> to be mandatory?
> >
> > 	If the initiator includes "Additional Key Exchange N" transform,
> > 	then the responder MUST choose a single method from
> > 	those included in this transform. If the list of included
> > 	methods doesn't contain NONE, then the responder
> > 	have to choose one of them, so performing this KE
> > 	method is mandatory. To make it optional the initiator
> > 	includes NONE among other methods. This allows
> > 	the responder to select NONE and thus to skip doing
> > 	this additional KE.
> 
> That makes it clearer, but. If each Additional Key Exchange carries a
> None, does that mean it these Exchanges can be fully omitted and we are
> only doing DH/KE from IKE_SA_INIT ? 

Yes.

> What if the Additional Key Exchange
> 1 contains None, and there is an Additional Key Exchange 2 ? Does it
> mean that everything can still be skipped ?

Then Additional Key Exchange 1 is omitted, but Additional Key Exchange 2 is not.
So, the sequence of exchanges is:

IKE_SA_INIT
IKE_INTERMEDIATE (corresponding to KE method from Additional Key Exchange 2 transform)
IKE_AUTH

> I'm still not a big fan of the specification this way. I wish we could
> find a way to do this in one new transform type and somehow enforcing
> an ordered list.

Please, see above.

I also want to say that the current draft makes no changes to IKEv2 code performing algorithms negotiation, 
and this this code is already complex enough, so I really don't want to make it more complex 
by changing the format of Transorm (as you seem to suggest).

> >>  	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?
> >
> > 	n here is new Transform Type. We define "Additional Key Exchange 1",
> > 	"Additional Key Exchange 2", "Additional Key Exchange 3" ...
> > 	up to "Additional Key Exchange 7".
> 
> Understood.
> 
> >> 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.
> >
> > Actually, the new transforms are introduced in 3.2.1, but I see your point
> > and probably we should add more clarifications before this section.
> 
> Please please please include some asci art of this exchange in
> IKE_SA_INIT. I ensure you that will make it much more easy for a new
> reader to understand this.

Will do.

> > (I also noted that Additional Key Exchange transforms are mentioned at the end of
> > 3.1 before their formal introduction, that must be fixed).
> >
> >> 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.
> >
> > That's true. However, there is a tradeoff here. If you defined a new two-message exchange,
> > that performed multiple key exchange methods at once, then
> > the initiator would include public keys for all KE methods it proposed
> > which is terrible for performance reasons, since the responder may
> > reject most of them. The message size is also would grow dramatically,
> > and in case the responder selected a tiny subset of what was proposed,
> > this would be extremely inefficient.
> 
> I'm not convinced this is a valid reason to break open the clear state
> of CREATE_CHILD_SA and a REKEYed IKE SA. This is really unpleasant for
> our state machine.

Well, your state machine must already handle similar situations, when 
a single CREATE_CHILD_SA exchange is insufficient to perform a rekey
or creating a new Child SA. As an example - if you receive TEMPORARY_FAILURE
notification, then you need to restart the exchange after some time.
So you have to create some state and remember to re-initiate 
the CREATE_CHILD_SA exchange. 

I agree, that it's a bit simpler, then IKE_FOLLOWUP_KE, but very similar.

And I still think that performance considerations and message size are very valid in PQ world,
so performing several key exchanges one by one is more attractive.

> >>  	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.
> >
> > The purpose of this data blob is to allow the responder to link
> > CREATE_CHILD_SA and subsequent IKE_FOLLOWUP_KE between
> > themselves in case the initiator creates several Child SAs
> > (or rekeys them, or rekeys IKE SA) simultaneously.
> 
> Again, then why not simply use the MSGID of the corresponding CREATE_CHILD_SA ?

This data is only meaningful to responder, so we don't mandate what to put there.
Message ID is a good choice, but again it's up to responder.
It's like cookie, which is only meaningful to responder. RFC 7296 doesn't
mandate how cookie is calculated, it only provides an example.

We can add a few words that e.g. using Message ID may be appropriate,
but is not mandated.

> >> 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?
> >
> > Because N(ADDITIONAL_KEY_EXCHANGE)(data blob) is for the responder
> > to allow it to properly link IKE_FOLLOWUP_KE with this particular CREATE_CHILD_SA.
> 
> Understood (but see above)
> 
> >> It has given no indication it might be mandatory from the initiator's point of view.
> >
> > You are right, this must be clarified in the draft.
> 
> Okay.
> 
> >>  	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?
> >
> > Sorry, I didn't follow your idea. Can you elaborate?
> 
> I'm lost here too now. But I think however something workds if it saves
> state and reboots should not affect the protocol design?

I'm not sure. Do you constantly save all active IKE SAs to non-volatile
memory so that after reboot they all are restored? I don't think it's always
practical and possible. The common approach is that after reboot 
you completely forget all your SAs and start recreating them from scratch
(oh, that's what INITIAL_CONTACT notify for).

Consider another example. If initiator starts IKE_SA_INIT, completes it,
but then suddenly crashes, so that it never starts IKE_AUTH - should
the responder handle this situation? Note, that the half-baked SA 
is already created on the responder and the state for it is allocated.
Or the network outage happens right after the IKE_SA_INIT is completed.
Then the initiator will start IKE_AUTH, but eventually fails and clears its SA state,
but the responder is unaware of initiator's attempts and unless
some measures are taken will keep half-baked SA  waiting for IKE_AUTH.
So, the responder must clear this state if it sees no IKE_AUTH request
message within some time period. 

Here we have very similar situation, the only difference is
that instead IKE_SA_INIT and IKE_AUTH we have CREATE_CHILD_SA and IKE_FOLLOWUP_KE.

> >> 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's true, but in this case the initiator forgets that it started CREATE_CHILD_SA
> > (with AKEs) before reboot. In this case it never continues this sequence
> > after rebooting and restoring last stable IKE SA state.
> 
> Well yes. If the initiator is broken, the connection might fail. I don't
> think we need to guard against that.

The initiator isn't broken. And we need to guard. Please see above.

> >>  	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 ?
> >
> > Because the failure may be recoverable - just start new CREATE_CHILD_SA
> > with AKEs. I see no need to delete IKE SA in this case.
> > Of course, if the failure persisted after several attempts, then
> > the only thing peer can do - delete SA.
> 
> I think this just complicates things too much. We shouldn't have
> confused peers or forgetful peers. Just hard fail when something
> unexpected happens. assert() for the win :P

It's not only about forgetful peers, it's also about network connectivity.
if there is some temporary network outage when the initiator
starts IKE_FOLLOWUP_KE, so that the responder deletes
state associated with CREATE_CHILD_SA, then it makes sense
to retry the whole sequence of exchanges.

Again, RFC 7296 already has very similar notification - TEMPORARY_FAILURE
and you have to handle it anyway.

> >> 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?
> >
> > Can you please provide text candidate?
> 
> I can try.

OK.

> >>   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).
> >
> > Additional here means that KE methods negotiated via these transforms
> > are always performed in addition to KE method, negotiated via Transform Type 4.
> 
> so this was again me being confused about the entire mechanism of the
> draft. This is why I strongly urge you to add some ascii art about how
> these payloads flow (as part of the SA payload)

Will do.

> >>  	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.
> >
> > OK.
> >
> >>  	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.
> >
> > This probably makes sence. I'll discuss this with my co-authors.
> 
> Okay.
> 
> >>  	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.
> >
> > I won't comment on this - this is just an appendix with early
> > design alternatives. I'll let my co-authors to comment on this.
> 
> Okay.
> 
> >>  	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.
> >
> > OK, we are in agreement :-)
> 
> While we might be in agreement, we are in disagreement on why the other
> approach was discarded :) So the real question is, should it be
> considered?

It was discussed on the mailing list in length.
There were several alternate approaches.

What about this particular para - it discusses an alternative approach when
all KE payloads are sent in IKE_SA_INIT. Then we have a problem with
IP fragmentation of this message and this piece of text discusses a way to 
fragment IKE_SA_INIT message. It would introduce an alternative fragmentation
mechanism, because RFC 7383 can only be used after IKE_SA_INIT is completed.
Then it would open a lot of problems that were discussed when we 
worked on RFC 7383. Overall, the approach of re-using an existing
IKE Fragmentation mechanism was taken (via IKE_INTERMEDIATE). 

> I do think that this document needs some more work to clarify things,
> and I would really like to see more people reviewing this document as
> well.
> 
> I would also like to see a discusion of a "simpler" design that would
> not use 4 Transform Type's, but only 1, and see if that would be
> "simpler" (simpler in quotes twice because i can't evaluate yet
> without such discussion which would be the simpler solution.

If you have this design in mind, please share it.
I can only guess how you are going to perform the negotiation
with only one new transform type, and my guess can be wrong.

Regards,
Valery.

> Paul