Skip to main content

Multiple Key Exchanges in the Internet Key Exchange Protocol Version 2 (IKEv2)
draft-ietf-ipsecme-ikev2-multiple-ke-12

Yes

Roman Danyliw

No Objection

(Andrew Alston)
(Robert Wilton)

Note: This ballot was opened for revision 08 and is now closed.

Roman Danyliw
Yes
Erik Kline
No Objection
Comment (2022-11-29 for -10) Sent
# Internet AD comments for draft-ietf-ipsecme-ikev2-multiple-ke-10
CC @ekline

## Nits

### S2

* s/FIPS complaint/FIPS compliant/

### S3.2.1

* I take it that it's not relevant to the example flow that there is no
  transform called AKE4.  :-)

### S5

* "can dwarfed"?
Francesca Palombini
No Objection
Comment (2022-12-01 for -10) Not sent
Thank you for the work on this document.

Many thanks to Russ Housley for his ART ART review: https://mailarchive.ietf.org/arch/msg/art/vuHfHEaOT8HvzHwHpnCNw23T_PM/.
John Scudder
No Objection
Comment (2022-11-30 for -10) Sent
Thanks for this. I have just one comment, about what's probably just a typographical error but it interfered with my understanding of the point in question so it seemed worth mentioning.

### Section 2, (2) is missing a verb, but what verb?

```
Hybrid. Currently, there does not exist a post-quantum key exchange that is trusted at the level that (EC)DH is trusted against conventional (non-quantum) adversaries. A hybrid post-quantum algorithm to be introduced next to well-established primitives, since the overall security is at least as strong as each individual primitive.
```

The second sentence seems, at minimum, to be missing a verb. For instance you could interpolate "needs" between "algorithm" and "to be", but I'm not sure if that properly captures your intended meaning.
Murray Kucherawy
No Objection
Comment (2022-11-30 for -10) Sent
This document has seven authors while the RFC Editor guideline is five.  Have we considered moving a couple of them to Section 6?

While not a DISCUSS-level concern, I would really like to see more division among the actions requested of IANA in Section 4.  There are 12 actions across two sections; it wouldn't take much to put each action in its own section, for example.  I can see in the datatracker that IANA has already indicated they understand what's being asked of them, but still I think it's helpful to other readers to organize it.
Paul Wouters
(was Discuss) No Objection
Comment (2022-12-01) Sent for earlier
# Sec AD review of draft-ietf-ipsecme-ikev2-multiple-ke-10

CC @paulwouters

Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

This review uses the format specified in https://github.com/mnot/ietf-comments/ which allows
automated tools to process items (eg to produce github issers)

Let me first apologize to Valery for not getting to review this document earlier. He surely
reminded me enough times to do it before it landed at the IESG.


This is a very well written document. Thanks to everyone involved. While I have a few DISCUSS
comments, these should be easy to address or convince me why no changes are required.

Note to self (and Valery): draft-kampati-ipsecme-ikev2-sa-ts-payloads-opt needs to be updated
to support this document. It currently only supports sending one KE payload.

## OLD DISCUSSes resolved


### IANA entries mentions in the Introduction ?

Shouldn't the introduction mention this draft introduces the IKE_FOLLOWUP_KE Exchange
and the STATE_NOT_FOUND Notify Message Type, along with additional entries to the
(now renamed) Key Exchanges Methods registry?

### Additional key exchanges DoS ?

The following paragraph raised an issue for me:
```
   If the responder selects NONE for some Additional Key Exchange types
   (provided they are proposed by the initiator), then the corresponding
   IKE_INTERMEDIATE exchanges MUST NOT take place.
```    
If the initiator's local policy requires at least one Additional Key Exchange,
an attacker sending back a quick reply with only NONE replies would be a DoS.
I think similar to like the original IKE_SA_INIT, perhaps we need to give some
advise that if the local policy would lead to permanent failure, that it
should wait for other (more legitimate) responses to this IKE_SA_INIT ?

### ADDITIONAL_KEY_EXCHANGE
```
   After IKE SA is created the window size may be greater than one and
   multiple concurrent exchanges may be in progress, it is essential to
   link the IKE_FOLLOWUP_KE exchanges together
```
I had some trouble figuring out why these are needed. For Child SA rekeys, these
would not be needed, because we would have an old SPI and MSGID that would make the
order obvious. But for adding addtional Child SA's, we have no old SPI. But we have
a new SPI on the initiator (and then a new SPI on the responder in the answer. Since
these are coupled by MSGID, I wonder if ADDITIONAL_KEY_EXCHANGE is really needed?
Looking at the useful appendix examples, I realise that the IKE_FOLLOWUP_KE exchange
does not have an SA payload so no SPI, so it makes sense to me now. Perhaps a sentence
in the document would be useful to explain this?

I still do not know why not to use the SPI as value for ADDITIONAL_KEY_EXCHANGE instead
of an opaque linking blob? The SPI is traditionally our linking blob.

Could the IKE_FOLLOWUP_KE set the SPI value in the IKE header instead of using
a new ADDITIONAL_KEY_EXCHANGE payload and use that with the MSGID as linking blob?


### State loss issue
```     
   After receiving this notification the initiator MAY start
   a new CREATE_CHILD_SA exchange which may eventually be followed by
   the IKE_FOLLOWUP_KE exchanges, to retry the failed attempt.  If the
   initiator continues to receive STATE_NOT_FOUND notifications [...]
```
How could this happen? If the state was lost, eg due to reboot, there would
need to come a new IKE SA, that can then send a new CREATE_CHILD_SA. I don't
see how that could lead to another STATE_NOT_FOUND. But the paragraph then
also continues with "and delete [the] IKE SA". But this IKE SA is brand new? 
I would just remove this entire paragraph as I think this cannot happen. Or
at least it is not a special case and existing abort code handles this already.

### IKE session resumption

Should there be a section updating RFC 5723 Section 5.1, or is the method there
specified quantum-safe if the initial IKE SA was protected using this document's
mechanism? See https://www.rfc-editor.org/rfc/rfc5723.html#section-5.1

I think the IKE resumption can work "as normal", as no KE payload is
involved in the resumption, but it would be nice if a sentence somewhere
in this document could confirm this. 

Also RFC 5723 states:
```
The keys and cryptographic protection algorithms should be at
      least 128 bits in strength.
```
IF we live in Grover universe, perhaps that should be 256 bits in strength? And since
we are making things quantum safe with this document, perhaps we should then at least
state session tickets should be 256 bits. Note if we do, then this document must
Update: RFC 5723. Perhaps this note on 5723 can be added in the Security Considerations
Section paragraph that talks about Grover and Shor.

### non-fatal NO_PROPOSAL_CHOSEN?
```
   In this case, the responder may respond with
   non-fatal error such as NO_PROPOSAL_CHOSEN notify message type.
```     
Technically, this error is non-fatal. But in this context, wouldn't it be fatal if the
responder insists on additional exchanges during the initial exchange and the initiator
doesn't suppor this? It is sort of a lame duck IKE SA ? :)

Also the "may" responder is unclear to me. What other response could there be and why?

### misplaced text?
```
   Note that if the initial IKE SA is used to transfer sensitive
   information, then this information will not be protected using the
   additional key exchanges [...]
```
This paragraph appears in the Section "Interaction with Childless IKE SA", but should probably
be moved to the Security Considerations section.

### IKE_FOLLOWUP_KE name

I find the name IKE_FOLLOWUP_KE a little confusing, as this exchange applies to
IKE and IPsec SA rekey negotiations. Why is it not called FOLLOWUP_ADDITIONAL_KE ?
Or CREATE_CHILD_SA_FOLLOWUP(_KE) (a sort of bad name too but that at least follows the
bad name from the original IKEv2 spec)


### authentication ?

``` 
        This document does not address authentication since it is less urgent
        at this stage.
```
While true, it does state that PPKs can be used. It might also want to say that
no IKE protocol level changes would be needed for authentication. A new RFC 7427
Digital Signature algorithm that is quantum-safe could be defined for X.509 and would
become available immediately without any IKEv2 level changes. So in a way, this
issue will be addressed but no IKEv2 document is needed for that. Perhaps this
can be clarified in the draft?

Related to this is text in the Security Considerations:

```
   In particular, the authenticity of the SAs established
   under IKEv2 is protected using a pre-shared key, RSA, DSA, or ECDSA
   algorithms.
``` 
This text is also incorrect as RFC 7427 allows us to use post-quantum authentication
algorithms that have a SubjectPublicKeyInfo (SPKI) definition. There might not be any
now, but there will presumbly be some in the future.

### AH

Section 4 lists AH. Is there much point in using this document when deploying AH?
The idea was the protect against _future_ quantum computers breaking encryption,
not MITM style packet modification. So using AH (or ESP_NULL) with this document
seems pointless :)

And the Security Considerations kind of agree with me here:
```
   Until quantum computers
   become available there is no point in attacking the authenticity of a
   connection because there are no possibilities for exploitation.
```

## Comments



### Section 2

I would probably have moved the Design Criteria to a later Section or Appendix, after
the entire protocol specification and Security Considerations. It's nice to know the
background, but this is "optional information" and shouldn't be as much at the focus
at the start of the document. (this is comment, you are fine to disagree and leave it
where it is)

### should -> could
```
   However, if such a requirement is
   needed, [I-D.tjhai-ikev2-beyond-64k-limit] discusses approaches that
   should be taken to exchange huge payloads.
```
I think this should should be a could, because it is a draft and it isn't
even adopted yet. I don't think that is suitable for a "should" even in lowercase.

### Design Criterea

One design criteria I do not see mentioned is "limit the extra number of RTTs as
much as possible". I do believe that was an important design criterea ?

The "future proof" design criterea is probably better named as "not post-quantum specific" ?

### Section 3

```
   A hybrid solution, when multiple
   key exchanges are performed and the calculated shared key depends on
   all of them, allows us to deal with this uncertainty by combining a
   classical key exchange with a post-quantum one, as well as leaving
   open the possibility of multiple post-quantum key exchanges.
```

This is an excellent summary sentence. It would be great to actually have this one in the
introduction :)

### IKE_INTERMEDIATE "is encrypted"
```
   The additional key exchanges are performed using
   IKE_INTERMEDIATE messages; because these messages are encrypted, the
   standard IKE fragmentation mechanism is available.
```
I think this is confusing. It is not really "because" they are encrypted that the fragmentation
mechanism "is available". Additionally, "encrypted" probably should clarify the level of
encryption - eg it would not me post-quantum safe. And of course it does not need to be.
Mabye something like:

   The additional key exchanges are performed using IKE_INTERMEDIATE messages that follow the
   IKE_SA_INIT exchange. This is to allow for standard IKE fragmentation mechanisms to be
   available for the potentially large post-quantum Key Exchange algorithm payloads. The IKE_SA_INIT
   exchange does not [cannot?] support fragmentation.

###
```
      and that hybrid key exchange is not needed.
```
Maybe:

      and that a hybrid key exchange containing a classic (EC)DH is no longer needed.

### Section 3.1 Childless?
```
     Following that, the IKE_AUTH exchange authenticates peers
     and completes IKE SA establishment.
```
This made me wonder if it was required to do a Childless IKE SA. I think a clarification is in order
here. Perhaps:

Following that, the IKE_AUTH exchange comples at normal and authenticates the peers, completes
the IKE SA establishment and when not childless, a Child SA is also established.


### Section 3.1 ASCII art

Probably, it should be clarified here that {} means "encrypted", or point a sentence on syntax to the
explanation in RFC7296? While this is obvious to readers of IKEv2 documents, this document has not
actually stated that this is the meaning. Additionally, perhaps introduce a {{foo}} that means
encrypted safely for classic and quantum ?

### duplicated algorithms

```
   MUST NOT contain duplicated algorithms
```
But it goes on saying this _can_ be possible, if the algorithm properties are the same, so this
sentence needs to reflect that to avoid misimplementation. Maybe:

MUST NOT contain duplicated algorithms with identical attributes

### Section 3.2.1.  MUST stop SA
```
   the initiator should log the
   error and MUST stop creating the SA or delete the SA if it is a
   rekey.
```

There is ambiguity here on the "delete the SA if it is a rekey". I think you mean to say to
stop creating or delete the SA being negotiated, and not to delete the SA that was attempted
to be rekeyed. How about the simpler:

the initiator should log the error and MUST abort the exchange with a permanent error.

### Section 3.2.1 IKE_INTERMEDIATE
```
    then the corresponding IKE_INTERMEDIATE exchanges MUST NOT take place.
```
You are already then clarifying this statement, but perhaps to avoid misimplementing, rewrite this to:

then the corresponding Additional Key Exchange(s) in the IKE_INTERMEDIATE exchanges MUST NOT take place.
If there is no Additional Key Exchange left to negotiate, this could mean that there is no more need
to perform any IKE_INTERMEDIATE exchanges.
[and remove the following paragraph completely]

### Section 3.2.2
```
   The other keying materials SK_d, SK_ai, SK_ar, SK_ei,
   SK_er, SK_pi, SK_pr are updated as:

   [...]
```
Why not say: The other keying materials SK_d, SK_ai, SK_ar, SK_ei, SK_er, SK_pi, SK_pr are generated
from the SKEYSEED(n) as per RFC 7296.

### Section 3.2.3
```
   This exchange is the
   standard IKE exchange, except that the initiator and responder signed
   octets are modified as described in [RFC9242].
```
Instead of "modified", which might mislead the reader into thinking this documents "modifies" that
process, I would say:

This exchange is the standard IKE exchange as described in [RFC7296] and [RFC9242].

### Section 3.2.4 missed rename
```
    the peers may optionally perform a Diffie-Hellman key exchange
```
Should this not also be renamed into: perform an additional Key Exchange Method

### Section 3.2.4 Simultanious rekey
```
   the initiator of this exchange just stops the
   rekeying process and it MUST NOT initiate the IKE_FOLLOWUP_KE
   exchange.
```
should this clarify with: and MUST delete the state and MUST NOT send a Delete/notify  ?

### Section 3.2.5 Childless IKE SA

This section explains how to use establish Child SAs without using the
IKE_INTERMEDIATE exchange.

I guess I would prefer that there are not two ways to do something, as IKEv2 is
already complex enough. But I guess the infrastructure needed for rekeying causes
this additional method to creep in whether we want it or not.

### I did?
```
    Thanks to Paul Wouters for reviewing the document.
```
I have no memory of this. Or was this pro-actively added? More serious, I guess I did
review this a long long time ago when the document looked very different :)

### Example is a bit contrived :)
```
       Transform AKE1 (ID = PQ_KEM_1)
       Transform AKE1 (ID = PQ_KEM_2)
       Transform AKE1 (ID = NONE)
       Transform AKE2 (ID = PQ_KEM_3)
       Transform AKE2 (ID = PQ_KEM_4)
       Transform AKE2 (ID = NONE)
       Transform AKE3 (ID = PQ_KEM_5)
       Transform AKE3 (ID = PQ_KEM_6)
       Transform AKE3 (ID = NONE)
```
I understand this is just an example to show the processing, but it would be a little odd
that both the order of (1|2) before (3|4) before (5|6) would matter if these sets are all
themselves optional - they are "optional requirements" ? :)

### Sending post-quantum proposals and policies in KE payload only

This solution was rejected because of a downgrade attack. Note though, that a new notify
payload of 'I_TRIED_POST_QUANTUM_FIRST' could be sent and the attacker would have been caught
in IKE_AUTH by the responder seeing this notify but never having seen the PQ KE payloads.
(not saying we should abandon this doc and go back to this proposal :)


### NITS
```
    needs to be integrated into IPsec protocol
```

should be "into the IPsec protocol"

```
        Currently, there does not exist a post-quantum key
        exchange that is trusted at the level that (EC)DH is trusted
        against conventional (non-quantum) adversaries.  A hybrid post-
        quantum algorithm to be introduced next to well-established
        primitives, since the overall security is at least as strong as
        each individual primitive.
```

I found this hard to read. How about:

        There is currently no post-quantum key exchange that is trusted at
        the level that (EC)DH is trusted for against conventional (non-quantum)
        adversaries.  A hybrid post-quantum algorithm to be introduced along with
        the well-established primitives addresses this concern, since the overall
        security is at least as strong as each individual primitive.

```
        A passive attacker can
        eavesdrop on IPsec communication today and decrypt it once a
        quantum computer is available in the future.
```

I think "eavesdrop" can be misinterpreted here. How about:

        A passive attacker can store all monitored encrypted IPsec communication
        today and decrypt it once a quantum computer is available in the future.

```
        This is a very
        serious attack for which we do not have a solution.
```

We have a solution, this document. It reads a bit as if this is undefendable now.

How about:

        This attack can have serious consequences that won't be visible for
        years to come. This document presents a defense against this serious attack.


```
        Nonetheless, it is possible to
        combine this post-quantum algorithm with a FIPS complaint key
        establishment method so that the overall design is FIPS
        compliant
```

I would change "is FIPS compliant" to "remains FIPS compliant"

```
   The fact, that
```
Remove the comma

```
   but this behavior is already specified
```

change to "but that this behaviour ....."

```
    The responder performs negotiation
```

The responder performs the negotiation   (added "the")

```
    rekeying them and rekeying IKE SA itself.
```
change to: rekeying these and rekeying the IKE SA itself.

```
   Its Exchange Type is 44.
```
change to: Its Exchange Type value is 44.

```
   After IKE SA is created the window size
```
After an IKE SA is [...]

```
    Its Notify Message Type is 16441
```
Its Notify Message Type value is 16441

```
   that would allow linking current exchange
```
that would allow linkinng the current exchange

```
   When rekeying IKE SA or Child SA
```
When rekeying the IKE SA or [the] Child SA

```
 multiple key exchanges using post-quantum algorithm can be composed
```
using a post-quantum algorithm

```
    Simply increasing the key length can dwarfed this attack.
```

```
IKE_INTERMEDIATE Exchanges Carrying Additional Key Exchange
      Payloads
```
Note this "Payloads" word is not rendered in bold like the rest of this text

```
the responder decides not to perform the additional key exchange.
```

"require" instead of "perform" ?

```
Both peers then computes
```
Both peers then compute   (no s)
Warren Kumari
No Objection
Comment (2022-11-29 for -10) Sent
Thank you for writing this document (and also making it easy for someone like me to understand :-))
Also thanks to Geoff Huston for his DNSDOR review (https://datatracker.ietf.org/doc/review-ietf-ipsecme-ikev2-multiple-ke-07-dnsdir-lc-huston-2022-10-10/)


I have a few non-blocking comments -- feel free to address them or not.

I think that moving Section 2, Bullet 2 towards to top of the document would help the reader better understand why this document exists... 


1: "While solving such a problem remains difficult with current
   computing power, it is believed that general purpose quantum
   computers will be able to solve this problem, implying that the
   security of IKEv2 is compromised."

'solving such a problem remains difficult with current computing power' implies that they *can* be solved with current computing power, while 'it is *believed* that general purpose quantum computers will be able to solve this problem' implies that quantum only *might* be able to solve them...this makes it sound like quantum machines are less of a concern than current ones :-). Perhaps 'general purpose quantum computers will *easily* be able to solve this problem'? Or 'solving such a problem is infeasible with current computing power'? (handwaving away trivial parameters) My suggestion isn't great, but hopefully I've managed to explain my concern?


2: Design Criteria - 3)   Focus on post-quantum confidentiality.
I understand what this is trying to say, but it feels very disjointed. I don't really have any suggested test to fix it, but just dropping the last sentence (or folding it into an earlier one) would make it much much easier to read.
Éric Vyncke
No Objection
Comment (2022-11-29 for -10) Sent
# Éric Vyncke, INT AD, comments for draft-ietf-ipsecme-ikev2-multiple-ke-10
CC @evyncke

Thank you for the work put into this document. Even if my IPsec knowledge is now very dated, I find it relatively easy to read.

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and some nits.

Special thanks to Tero Kivinen for the shepherd's write-up including the WG consensus *but* the justification of the intended status is missing. 

Other thanks to Geoff Huston for his Last Call DNS directorate review at: https://datatracker.ietf.org/doc/review-ietf-ipsecme-ikev2-multiple-ke-07-dnsdir-lc-huston-2022-10-10/ 

Please note that Charles Perkins is the Internet directorate reviewer (at my request) and you may want to consider this int-dir reviews as well when Charles will complete the review (no need to wait for it though):
https://datatracker.ietf.org/doc/draft-ietf-ipsecme-ikev2-multiple-ke/reviewrequest/16618/

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS

Out of all Paul Wouters's points, I support the last one about AH (I am not experience enough to appreciate the other ones). It is also related to bullet 3) of section 2.

### Missing reference RFC 8247

As indicated by idnits tool, RFC 8247 is used as a reference but is not defined ;-)

### Section 2

The bullet 2) is a nice explanation about *why* there must be multiple key exchanges with different methods. Until reading that part, I was really wondering why this I-D was about the link with PQC and multiple key exchanges. Should this be mentioned in the abstract already ?

Should "FIPS" be prefixed by "USA" as in "USA FIPS" ?


## NITS

### Section 1.2

`payloads longer than 64k` suggest to specify the units of measure.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
Andrew Alston Former IESG member
No Objection
No Objection (for -10) Not sent

                            
Robert Wilton Former IESG member
No Objection
No Objection (for -10) Not sent