Subject: [IPsec] Review of section 2 to 2.23 (not 2.23.1 forward yet).

From: Tero Kivinen <[email-masked]>
Date: Fri, 15 Jan 2010 16:29:32 +0200

Here is second set of issues now from the section 2. I didn't get
completely through it yet, but I decided to send this now as next time
I will be able to get more comments is on Monday. 

----------------------------------------------------------------------

In section 2.2 there is duplicate text:

	   The Message ID is reset to zero in the new
   IKE SA after the IKE SA is rekeyed.  Rekeying an IKE SA resets the
   sequence numbers.  Thus, the first pair of IKE_AUTH messages will
   have ID of 1, the second (when EAP is used) will be 2, and so on.

I suggest removing the "Rekeying an IKE SA resets the sequence numbers.",
as the first sentence says it better (i.e. explains that the new IKE SA
has message id 0, not the old one).

Also the last sentense starting with "Thus, the first ..." should be
moved before the text I quoted, so that the whole paragraph will be:

   The Message ID is a 32-bit quantity, which is zero for the
   IKE_SA_INIT messages (including retries of the message due to
   responses such as COOKIE and INVALID_KE_PAYLOAD), and incremented for
   each subsequent exchange.  Thus, the first pair of IKE_AUTH messages
   will have ID of 1, the second (when EAP is used) will be 2, and so on.
   The Message ID is reset to zero in the new IKE SA after the IKE SA is
   rekeyed. 

Also later there is two definations for the "original initiator":

   Throughout this document, "initiator" refers to the party who
   initiated the exchange being described, and "original initiator"
   refers to the party who initiated the whole IKE SA.  The "original
   initiator" always refers to the party who initiated the exchange
   which resulted in the current IKE SA.  In other words, if the
   "original responder" starts rekeying the IKE SA, that party becomes
   the "original initiator" of the new IKE SA.

I suggest changing that to:

   Throughout this document, "initiator" refers to the party who
   initiated the exchange being described.  The "original
   initiator" always refers to the party who initiated the exchange
   which resulted in the current IKE SA.  In other words, if the
   "original responder" starts rekeying the IKE SA, that party becomes
   the "original initiator" of the new IKE SA.

----------------------------------------------------------------------

Section 2.5 changes MUST to SHOULD and SHOULD to MUST NOT, and I think
this should be noted in the section 1.7, i.e this text was in the
original RFC4306:

   Although new payload types may be added in the future and may appear
   interleaved with the fields defined in this specification,
   implementations MUST send the payloads defined in this specification
   in the order shown in the figures in section 2 and implementations
   SHOULD reject as invalid a message with those payloads in any other
   order.

and this is the text from the current IKEv2bis draft:

   Although new payload types may be added in the future and may appear
   interleaved with the fields defined in this specification,
   implementations SHOULD send the payloads defined in this
   specification in the order shown in the figures in Section 2;
   implementations MUST NOT reject as invalid a message with those
   payloads in any other order.

The change from "MUST send payloads in order" to "SHOULD send", and
"SHOULD reject" to "MUST NOT reject" is important enough to be noted in
1.7.

----------------------------------------------------------------------

In section 2.6 there is text explaining why Ni is added to the cookie,
but that text is not really about the Ni, but SPIi:

     Also, incorporating Ni in the hash prevents an
   attacker from fetching one cookie from the other end, and then
   initiating many IKE_SA_INIT exchanges all with different initiator
   SPIs (and perhaps port numbers) so that the responder thinks that
   there are lots of machines behind one NAT box who are all trying to
   connect.

I.e. adding Ni does not protect against that attack, but adding SPIi will
prevent attacker from using one cookie for different initiator SPIs.
Adding Ni will prevent attacker from sending multiple packets each having
same SPIi and IPi (which would most likely being rejected as
retransmissions). Better text would be:

     Also, incorporating SPIi in the hash prevents an
   attacker from fetching one cookie from the other end, and then
   initiating many IKE_SA_INIT exchanges all with different initiator
   SPIs (and perhaps port numbers) so that the responder thinks that
   there are lots of machines behind one NAT box who are all trying to
   connect.

I already complained about this earlier (issue #19), but when it was put
to the draft I think the text got changed from "incorporating SPIi" to
"Incorporating Ni". 

Also the paragraph:

   In addition to cookies, there are several cases where the IKE_SA_INIT
   exchange does not result in the creation of an IKE SA (such as
   INVALID_KE_PAYLOAD or NO_PROPOSAL_CHOSEN).  In such a case, sending a
   zero value for the Responder's SPI is correct.  If the responder
   sends a non-zero responder SPI, the initiator should not reject the
   response for only that reason.

could perhaps clarify that it is talking about the FIRST IKE_SA_INIT
exchange that does not result in the creation of an IKE SA, as most of
those cases will result creation of IKE SA during next IKE_SA_INIT
exchanges. On the other hand section 2.7 has about same text covering
cases, so perhaps we should remove this text from section 2.6, and only
keep the text at the 2.7:

   When the IKE_SA_INIT exchange does not result in the creation of an
   IKE SA due to INVALID_KE_PAYLOAD, NO_PROPOSAL_CHOSEN, or COOKIE (see
   Section 2.6), the responder's SPI will be zero.  However, if the
   responder sends a non-zero responder SPI, the initiator should not
   reject the response for only that reason.

(There are several cases where we have added similar text to multiple
locations (which some are quite close to each other), because we have
made those changes incrementally. Now when I am reading the whole text
through those repeated texts just cause confusion and makes document
longer). 

----------------------------------------------------------------------

In section 2.6.1 it has text saying:

       For instance, if
   the responder includes the SAi1 and KEi payloads in cookie
   calculation, it will reject the request by sending a new cookie.

which is misleading, as even if SAi1 is included in the cookie, that
should not cause cookie to be rejected, as the retry behavior for
INVALID_KE_PAYLOAD says that SAi1 is going to be same (section 2.7 says:
"The initiator MUST again propose its full set of acceptable
cryptographic suites ..."). IT would be better to remove "SAi1 and" from
the sentence and only talk about KEi:

       For instance, if
   the responder includes the KEi payload in cookie
   calculation, it will reject the request by sending a new cookie.

----------------------------------------------------------------------

In section 2.8 the sentence:

	   Note that, when
   rekeying, the new Child SA SHOULD NOT have different traffic
   selectors and algorithms than the old one.

is in wrong place, it is after the paragraph talking about IKE SA rekey,
it should be moved to the previous paragraph talking about Child SA
rekeying.

There is also orphan paragraph:

   The node that initiated the surviving rekeyed SA should delete the
   replaced SA after the new one is established.

which should be moved to somewhere, I think the text before it got moved
to the subsections of 2.8.... so this text should probably be moved to
section 2.8.1 after the paragraph about checking cookies i.e. after:

   This form of rekeying may temporarily result in multiple similar SAs
   between the same pairs of nodes.  When there are two SAs eligible to
   receive packets, a node MUST accept incoming packets through either
   SA.  If redundant SAs are created though such a collision, the SA
   created with the lowest of the four nonces used in the two exchanges
   SHOULD be closed by the endpoint that created it.  "Lowest" means an
   octet-by-octet, lexicographical comparison (instead of, for instance,
   comparing the nonces as large integers).  In other words, start by
   comparing the first octet; if they're equal, move to the next octet,
   and so on.  If you reach the end of one nonce, that nonce is the
   lower one.

this paragraph.

----------------------------------------------------------------------

Also the text in the 2.8.1 is not consistent with the new error
notifications, so the end of the section should be changed to use
CHILD_SA_NOT_FOUND instead of NO_PROPOSAL_CHOSEN:

   To B, it looks like A is trying to rekey an SA that no longer exists;
   thus, B responds to the request with something non-fatal such as
   CHILD_SA_NOT_FOUND.

                                <--  send resp1: N(CHILD_SA_NOT_FOUND)
   recv resp1 <--

   When A receives this error, it already knows there was simultaneous
   rekeying, so it can ignore the error message.

----------------------------------------------------------------------

Section 2.8.2 seems to have quite fatal error:

	The new IKE SA containing the lowest nonce inherits the Child
   SAs.  

This is wrong. The one containing the lowest nonce, is the one that is
going to be deleted, not the one that survives. This needs to be changed
to:

  The new IKE SA containing the lowest nonce SHOULD be deleted by the node
  that created it and the other suriving new IKE SA MUST inherit all the
  Child SAs.

Note, that I used words MUST here as this is one of the few cases where
the correct behavior is really needed for interoperability reasons. It is
not needed for simultaneous Child SA cases, as traffic continues to flow,
even if they do not delete the loosing Child SA (we just have one extra
Child SA). In this case it is important for the interoprability that both
ends AGREE on which new IKE SA inherited the Child SAs from the old IKE
SA. If they disagree then all IKE SAs are messed up and future rekeys,
deletes etc will fail. Deleting the loosing IKE SA is not necessarely
needed for interoperability so thats why that is SHOULD (just like it is
in the child SA case), but moving Child SAs to correct IKE SA is MUST.

----------------------------------------------------------------------

In section 2.9 the paragraph

   It is possible for the responder's policy to contain multiple smaller
   ranges, all encompassed by the initiator's traffic selector, and with
   the responder's policy being that each of those ranges should be sent
   over a different SA.  Continuing the example above, the responder
   might have a policy of being willing to tunnel those addresses to and
   from the initiator, but might require that each address pair be on a
   separately negotiated Child SA.  If the initiator generated its
   request in response to an incoming packet from 192.0.1.43 to
   192.0.2.123, there would be no way for the responder to determine
   which pair of addresses should be included in this tunnel, and it
   would have to make a guess or reject the request with a status of
   SINGLE_PAIR_REQUIRED.

is bit confusing, as if the initiator generated its request in response
to an incoming packet then it should include first traffic selectors
based on the data from packet, and then responder should be able to pick
up the correct pair of addresses.

So I assume this paragraph talks about the case where a) initiator is not
following the SHOULD saying it SHOULD include very specific traffic
selector, b) the initiator didn't start creating the SA based on packet
at all. So the paragraph should be changed to:

   It is possible for the responder's policy to contain multiple smaller
   ranges, all encompassed by the initiator's traffic selector, and with
   the responder's policy being that each of those ranges should be sent
   over a different SA.  Continuing the example above, the responder
   might have a policy of being willing to tunnel those addresses to and
   from the initiator, but might require that each address pair be on a
   separately negotiated Child SA.  If the initiator didn't generated its
   request based on the packet, but for example upon startup, there would
   not be the very specific first traffic selectors helping responder to
   select correct range and there would be no way for the responder to
   determine which pair of addresses should be included in this tunnel,
   and it would have to make a guess or reject the request with a status
   of SINGLE_PAIR_REQUIRED.

In RFC4306 this paragraph was followed by the explanation of the very
specific first traffic selectors, but now that text has moved earlier,
thus causing this confusion.

----------------------------------------------------------------------

In section 2.18 we now DO require Diffie-Hellman to be done on the IKE SA
rekey. This MUST level requirement is new to IKEv2bis, and needs to be
added to the section 1.7.

----------------------------------------------------------------------

In section 2.19 there is FAILED_CP_REQUIRED text twice:

   The FAILED_CP_REQUIRED notification is sent by responder in the case
   where CP(CFG_REQUEST) was expected but not received, and so is a
   conflict with locally configured policy.  There is no associated
   data.

   The responder MUST NOT send a CFG_REPLY without having first received
   a CP(CFG_REQUEST) from the initiator, because we do not want the IRAS
   to perform an unnecessary configuration lookup if the IRAC cannot
   process the REPLY.  In the case where the IRAS's configuration
   requires that CP be used for a given identity IDi, but IRAC has
   failed to send a CP(CFG_REQUEST), IRAS MUST fail the request, and
   terminate the IKE exchange with a FAILED_CP_REQUIRED error.  The
   FAILED_CP_REQUIRED is not fatal to the IKE SA; it simply causes the
   Child SA creation fail.  The initiator can fix this by later starting
   a new configuration payload request.

I think the first paragraph should be removed and second one splitted and
changed to::

   The responder MUST NOT send a CFG_REPLY without having first received
   a CP(CFG_REQUEST) from the initiator, because we do not want the IRAS
   to perform an unnecessary configuration lookup if the IRAC cannot
   process the REPLY.

   In the case where the IRAS's configuration requires that CP be used
   for a given identity IDi, but IRAC has failed to send a
   CP(CFG_REQUEST), IRAS MUST fail the request, and terminate the Child
   SA creation with a FAILED_CP_REQUIRED error. The FAILED_CP_REQUIRED is
   not fatal to the IKE SA; it simply causes the Child SA creation fail.
   The initiator can fix this by later starting a new configuration
   payload request. There is no associated data in the FAILED_CP_REQUIRED
   error.

(Changed "terminate the IKE exchange" to "terminate the Child SA
creation" and added text about the associated data).

----------------------------------------------------------------------

In section 2.21.2 we still have NOTE FOR WG DISCUSSION:

   NOTE FOR WG DISCUSSION: Having other payloads in the message is
   allowed but there are none suggested.  One WG member mentioned the
   possibility of adding a DELETE payload when the error is sent in a
   separate INFORMATIONAL exchange.  Do we want to allow such additional
   payloads that have operational semantics?

----------------------------------------------------------------------

In section 2.23 the word "float" needs to be replaced:

   An initiator can use port 4500, regardless whether or not there
   is NAT, even at the beginning of IKE.  When either side is using port

In section 2.23 the paragraph:

   o  The original source and destination IP address required for the
      transport mode TCP and UDP packet checksum fixup (see [UDPENCAPS])
      are obtained from the Traffic Selectors associated with the
      exchange.  In the case of NAT traversal, the Traffic Selectors
      MUST contain exactly one IP address, which is then used as the
      original IP address.

Says that In case of NAT traversal there MUST only be exactly one IP
address, but that actually only covers the transport mode NAT-Traversal
(as said in the beginning of paragraph) so the text should be changed to:
      
   o  The original source and destination IP address required for the
      transport mode TCP and UDP packet checksum fixup (see [UDPENCAPS])
      are obtained from the Traffic Selectors associated with the
      exchange.  In the case of transport mode NAT traversal, the Traffic
      Selectors MUST contain exactly one IP address, which is then used
      as the original IP address.

-- 
[email-masked]