Subject: Re: [IPsec] Reviews of section 1 and 2 of the draft-ietf-ipsecme-ikev2

From: Paul Hoffman <[email-masked]>
Date: Sun, 17 Jan 2010 08:02:10 -0800

Tero:

Thank you for your careful (if not a bit late...) reviews. The author team will incorporate the non-controversial editorial changes into the version going to IETF Last Call. I have also already opened issues in the issue tracker for the ones that look like technical changes or clarifications that might be controversial; I'll start threads for those soon.

There are a few remaining changes you asked for that I don't think should be made; I have listed them here. If anyone (including you, of course) really object to these rejections, please do open individual issues in the tracker instead of just replying to this message. Really: trying to follow ideas in a single thread on this message will be too hard.

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

Section 1.3.1 now talks about the USE_TRANSPORT_MODE,
ESP_TFC_PADDING_NOT_SUPPORTED and NON_FIRST_FRAMENTS_ALSO notifications,
and says they can be included, but the packet figure does not include
them. Adding them to the figure would be easy, but the problem is that we
currently say that you should follow the order of payloads from the
figures (altough we do refer to section 2 not section 1, but this is one
of the open issues).

So depending what we decided on the payload order and figures, it might
be better to add one extra ", [N]" to the figure before SA payload.

[[ Response: It is not an open issue. The spec says "implementations MUST NOT reject as invalid a message with those payloads in any other order". Thus, putting the "[N]" in the figure is likely to be more confusing than helpful because [N] can appear in almost *any* of the figures. ]]

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

Also the section 1.3.1 has text which perhaps should be removed:

   Failure of an attempt to create a Child SA SHOULD NOT tear down the
   IKE SA: there is no reason to lose the work done to set up the IKE
   SA.  When an IKE SA is not created, the error message return SHOULD
   NOT be encrypted because the other party will not be able to
   authenticate that message.  See Section 2.21 for a list of error
   messages that might occur if creating a Child SA fails.

This section 1.3.1 is talking about the CREATE_CHILD_SA not initial
IKE_AUTH, thus this section is not needed here. The section 1.2 already
has text about the Child SA failing during the initial exchange, so this
how paragaraph needs to be removed.

[[ Response: This text was specifically added by the WG to deal with creating SAs, not with IKE_AUTH. This was Issue #9, which you initiated. ]]

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

Section 1.5 needs some reordering and the text in section 1.5 vand 2.21.4
needs to be combined, now we have about the same text in two places.

The first paragraph of the 1.5 talks about the INVALID_IKE_SPI even
though it does not say it explictly, the second paragraph talks about
INVALID_SPI and then in third paragraph it says that "there are two cases
when one-way message is sent: INVALID_IKE_SPI and INVALID_SPI", just like
this would be new thing, but the previous paragraphs have already talked
about this. This is confusing, so I think this section requires bit of
reordering and rewording. Also it might be useful to actually move all
this text to section 2.21.4 and remove this whole section (remember
to update references).

Especially the fact that exchange type is copied from the request in
the INVALID_IKE_SPI case is missing from the 2.21.4 and should be
added there and also the last paragraph about INVALID_SPI text in the
1.5 is something that is not that clearly explained in the 2.21.4 so
that also needs to be kept (and/or moved).

[[ Response: While I agree that there is a lot of duplication in the 1.5 and 2.21.4, I am quite worried about removing anything at this late date. If you have a specific reordering you want, please post it; otherwise, I think we should leave this duplication here. ]]

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

XX 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.

[[ Response: It looks like the sentence is, in fact, in the right place. The whole paragraph reads: "To rekey a Child SA within an existing IKE SA, create a new, equivalent SA (see Section 2.17 below), and when the new one is established, delete the old one.  Note that, when rekeying, the new Child SA SHOULD NOT have different traffic selectors and algorithms than the old one." That is, indeed, talking about rekeying a Child SA. ]]

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

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

[[ Response: The two uses of FAILED_CP_REQUIRED are different and therefore should both be left in. ]]

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

Again: thanks for the detailed review! It will hopefully cause others in the WG to do such a review in IETF Last Call.

--Paul Hoffman, Director
--VPN Consortium

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 19 Jan 2010 15:46:54 +0200

Paul Hoffman writes:
> [[ Response: It is not an open issue. The spec says "implementations
> MUST NOT reject as invalid a message with those payloads in any
> other order". Thus, putting the "[N]" in the figure is likely to be
> more confusing than helpful because [N] can appear in almost *any*
> of the figures. ]]

Yes [N] Can appear in any figure.

On the other hand we are now talking about Notification payloads in
the section 1.3.1 and the figures don't show them at all, which can
cause confusion.

BTW, so we are going to leave the reference to point to only section 2
in the end of section 2.5?

Section 2 used to have only very few figures, i.e. the one in 2.6 (IKE
SA SPIs and Cookies), 2.16 EAP, 2.19 requesting IP-address from remote
network, and 2.20 requesting peer's version.

Now we have some more figures in section 2 covering those exchange
collision etc cases, but all the important base figures are in section
1. I think we had ticket about this, but cannot seem to find it now.

We did have #16 and #45 which covered the order, but they didn't talk
about whether it should refer section 2 or section 1 (or both) for the
figures, so I guess we didn't have ticket about this.
-- 
[email-masked]

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 19 Jan 2010 15:52:13 +0200

Paul Hoffman writes:
> [[ Response: This text was specifically added by the WG to deal with
> creating SAs, not with IKE_AUTH. This was Issue #9, which you
> initiated. ]] 

Issue #9 covered only IKE_AUTH case and IKE SA creation in general ,
it didn't talk anything about the Child SA creation case.

Section 1.3.1 only talks about the creating Child SAs using
CREATE_CHILD_SA exchange. It does not cover creating IKE SA (i.e.
IKE_AUTH) at all.

We already have the text we need in the 1.2 we do not need to have
this text here, especially as it is very misleading.

The first sentence is only thing that is really correct about the
paragraph and even that should be obvious.

The rest starting talking about "When an IKE SA is not created" is
completely out of place, as this section does not talk about IKE SA
creation. 
-- 
[email-masked]

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 19 Jan 2010 15:57:43 +0200

Paul Hoffman writes:
> [[ Response: It looks like the sentence is, in fact, in the right
> place. The whole paragraph reads: "To rekey a Child SA within an
> existing IKE SA, create a new, equivalent SA (see Section 2.17
> below), and when the new one is established, delete the old one.
> Note that, when rekeying, the new Child SA SHOULD NOT have different
> traffic selectors and algorithms than the old one." That is, indeed,
> talking about rekeying a Child SA. ]]

That text you quoted is not from the
draft-ietf-ipsecme-ikev2bis-06.txt, so I assume you have already fixed
this bug.

The text from draft-ietf-ipsecme-ikev2bis-06 says:

   To rekey a Child SA within an existing IKE SA, create a new,
   equivalent SA (see Section 2.17 below), and when the new one is
   established, delete the old one.

   To rekey an IKE SA, establish a new equivalent IKE SA (see
   Section 2.18 below) with the peer to whom the old IKE SA is shared
   using a CREATE_CHILD_SA within the existing IKE SA.  An IKE SA so
   created inherits all of the original IKE SA's Child SAs, and the new
   IKE SA is used for all control messages needed to maintain those
   Child SAs.  After the new equivalent IKE SA is created, the initiator
   deletes the old IKE SA, and the Delete payload to delete itself MUST
   be the last request sent over the old IKE SA.  Note that, when
   rekeying, the new Child SA SHOULD NOT have different traffic
   selectors and algorithms than the old one.

and here you can see that the last sentence of the last paragraph is
in wrong place. I assume you have already moved the sentence to the end
of previous paragraph earlier. 
-- 
[email-masked]

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 19 Jan 2010 16:01:50 +0200

> therefore should both be left in. ]]

I do not think they are different cases. 

The first one uses word "responder" instead of "IRAS" but they are
talking about the exactly same case. I.e. when the client (initiator,
IRAC) do not send CP(CFG_REQUEST) in its packet, but the server
(responder, IRAS) requires it to be sent, and thats why the server
(responder, IRAS) returns FAILED_CP_REQUIRED error notification back
to the client (initiator, IRAC). 

If you think those two cases in the two paragraphs are really
different, can you explain what is the difference between them?
-- 
[email-masked]

---

From: Paul Hoffman <[email-masked]>
Date: Tue, 19 Jan 2010 14:21:56 -0800

>I do not think they are different cases.

On further staring at this, I agree, and I agree that your rewording is fine. Done.

--Paul Hoffman, Director
--VPN Consortium

---

From: Paul Hoffman <[email-masked]>
Date: Tue, 19 Jan 2010 14:51:02 -0800

>the section 1.3.1 and the figures don't show them at all, which can

My ability to predict confusion is better than yours. :-) Seriously, they both can be confusing, and I think that inconsistent placement of [N] is more confusing than consistent non-placement of [N].

>in the end of section 2.5?

No, it now says sections 1 and 2.

--Paul Hoffman, Director
--VPN Consortium

---

From: Tero Kivinen <[email-masked]>
Date: Wed, 20 Jan 2010 12:35:57 +0200

> No, it now says sections 1 and 2.

Not in the draft-ietf-ipsecme-ikev2bis-06.txt. Am I missing something,
or is there newer version availabe from somewhere?
-- 
[email-masked]

---

From: Paul Hoffman <[email-masked]>
Date: Wed, 20 Jan 2010 09:37:49 -0800

>or is there newer version availabe from somewhere?

Sorry. By "now" I mean "the version I am working on with the zillion WG LC comments". That is, the next one.

--Paul Hoffman, Director
--VPN Consortium