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

[CJ Tjhai <[e-mail]>]

On Mon, 16 Aug 2021 at 21:24, Paul Wouters <paul.wouters=
[e-mail]> wrote:

> 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. 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.
>
> I think it would be worth thinking about this problem a bit more.
>

I think the problem above could be addressed by running a matching
algorithm where both peers use the initiator's ordering as the reference.
So when the responder receives the Additional Key Exchange proposals, it
could run the following matching algorithm:

  ACCEPTED = []
  for (i : all initiator AKE proposals) {
    found = no
    for (r : all responder AKE proposals) {
      if (r-th responder AKE proposals not been accepted) {
        selected = intersection(i-th initiator AKE proposals, r-th
responder AKE proposals)[0] or-else NO_MATCH
        if (selected != NO_MATCH) {
          mark r-th responder AKE proposals as accepted
          ACCEPTED = [ACCEPTED selected]
          found = yes
          break
        }
      }
    }
    if (found == no &&  NONE not in i-th initiator AKE proposal) {
      Failure, NO_PROPOSAL_CHOSEN?
    }
  }

So it will either obtain a list of accepted Additional Key Exchange
algorithms or a negotiation failure which will result in NO_PROPOSAL_CHOSEN
notification. With the above matching algorithm, the difference in ordering
of Additional Key Exchange proposals between initiator and responder does
not matter as the responder will try to best match the proposals of the
initiator. Of course, further check needs to be run on the initiator site
to make sure that the accepted Additional Key Exchange proposals are the
supported ones and that there is an accepted proposal for each Additional
Key Exchange transform type where NONE is not advertised.

If we need to collapse Additional Key Exchange 1, ..., Additional Key
Exchange n transform types into one, I think we will still need to have the
ability to offer different grouping of proposals. So I am not sure how
simpler this could be, but yes, it would be good to have more discussion on
this.


> > 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.
>
> >>      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.
>
>
> 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?
>
> > 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.
>
> >> 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 ? 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 ?
>
> 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.
>
>
>
> >>      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.
>
>
Yes, I agree. We will update the draft to include some ASCII art showing
the exchanges.



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

The subsequent paragraphs in the Appendix also state that it is possible to
protect against this attack with some configuration/policy.

Best regards
CJ


>
> >>      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?
>
> 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.
>
> Paul
>
> _______________________________________________
> IPsec mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ipsec
>

-- 

PQ Solutions Limited (trading as ‘Post-Quantum’) is a private limited 
company incorporated in England and Wales with registered number 06808505.
 

This email is meant only for the intended recipient. If you have received 
this email in error, any review, use, dissemination, distribution, or 
copying of this email is strictly prohibited. Please notify us immediately 
of the error by return email and please delete this message from your 
system. Thank you in advance for your cooperation.


For more information 
about Post-Quantum, please visit www.post-quantum.com 
<http://www.post-quantum.com>.

In the course of our business relationship, 
we may collect, store and transfer information about you. Please see our 
privacy notice at www.post-quantum.com/privacy-notice 
<http://www.post-quantum.com/privacy-notice> to learn about how we use this 
information.