Subject: [IPsec] IKEv2bis, comments about sections 3-

From: <[email-masked]>
Date: Tue, 19 Jan 2010 11:14:06 +0100

Somewhat substantial:

- Section 3.13.1: the paragraph about Mobility Header is very
confusing; suggested rephrasing:

   Traffic selectors can use IP Protocol ID 135 to match the IPv6
   mobility header [MIPV6].  As specified in [IPSECARCH], the IPv6
   mobility header (MH) message type is placed in the most significant
   eight bits of the 16-bit local port selector.  The direction
   semantics of TSi/TSr port fields are the same as for ICMP.

- Section 3.*: should we omit the UNSPECIFIED values? They don't
really help the reader here...

- Overall: The document uses 192.0.1.0/24 in examples (in addition to
the official 192.0.2.0/24 documentation block). Back in RFC 4306/4718
times, we didn't have much of a choice, but now
draft-iana-ipv4-examples (already approved) provides two additional
blocks. Suggest replacing "192.0.1" -> "198.51.100" .

- Appendix C.5: brackets should be removed from [KEi]/[KEr]

- Section 1.7, suggest mentioning the new notifications explicitly;
  i.e. rephrase the last paragraph

  "Section 2.25 was added to explain how to act when there are timing
  collisions when deleting and/or rekeying SAs, and two new error
  notifications (TEMPORARY_FAILURE and CHILD_SA_NOT_FOUND) were
  defined."

- Section 1.7: suggest adding
   
  "This document requires doing a Diffie-Hellman exchange when
  rekeying the IKE_SA.  In theory, RFC 4306 allowed a policy where
  the Diffie-Hellman exchange was optional, this was not useful (or
  appropriate) when rekeying the IKE_SA.

Editorial suggestions:

- Section 3.1: "The bits are defined LSB first, so bit 0 would be the
least significant bit of the Flags octet." This seems to be exactly
the opposite of the bit numbering used in the figure above, which
sounds confusing. Instead of using numbers, we should just show a
diagram?

    0 1 2 3 4 5 6 7 
   +-+-+-+-+-+-+-+-+
   |X X|R|V|I|X X X|
   +-+-+-+-+-+-+-+-+

- Section 3.1, the text about critical bit should have a pointer to
Section 2.5.

- Section 3.3.6: suggest removing the last paragraph (the
INVALID_KE_PAYLOAD case is already well described in 1.2 and 2.7), 
and there's no need to repeat it here.

- Section 3.4, 2nd-to-last paragraph: suggest adding pointer to
Sections 1.2 and 2.7.

- Section 3.10.1: there's one "{{ Demoted the SHOULD }}" that
should be removed.

- Section 3.10.1: the list should contain CHILD_SA_NOT_FOUND
and TEMPORARY_FAILURE.

- Section 3.15.1: the table column header should be "Multi-Valued"
(like it is in RFC 4306) instead of "Valued"

- Section 3.5: "MUST not" -> "MUST NOT" (twice)

- Section 1.6: the spelling in the 2nd paragraph isn't exactly
what RFC 2119 has (and this causes idnits to complain). 

- From idnits: Duplicate reference: RFC4291, mentioned in 'IPV6ADDR', 
was also mentioned in 'ADDRIPV6'.

- Appendix D: can be removed, since the changes from RFC 4306 are
now in Section 1.7

Best regards,
Pasi

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 19 Jan 2010 14:31:55 +0200

> sounds confusing. Instead of using numbers, we should just show a

So you are suggesting that we change the bit order to use MSB instead
of LSB as it is now? It might be better to get rid of bit numbers
completely then and put the separate flags to the main figure:

                        1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                       IKE SA Initiator's SPI                  |
   |                                                               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                       IKE SA Responder's SPI                  |
   |                                                               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |  Next Payload | MjVer | MnVer | Exchange Type |X|X|R|V|I|X|X|X|
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                          Message ID                           |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                            Length                             |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                    Figure 4:  IKE Header Format

and remove the whole bit numbering mess.

Note, that this bit order thing has been inherited from the RFC2408
which also used MSB bit order when showing figures having 32 bits
words, but used LSB bit order when actually counting bits inside the
octect.

BTW, your format confused me as I am so used to talking about bit 0
when I am talking about the lowest bit of the octect (i.e the one you
get by doing (octect & 1)).

I would actually prefer to keep the text as it is now, even though it
is not consistent with the bit order in 32-bit figures and the bit
order in the text, but at least it clearly defines the bit order.
-- 
[email-masked]

---

From: <[email-masked]>
Date: Tue, 19 Jan 2010 14:03:08 +0100

Removing the whole bit numbering mess, and showing the flags
in the main figure as you suggest, sounds good to me.

(But since nobody has noticed this before -- AFAIK, at least --
perhaps leaving this as-is would also be acceptable...)

Best regards,
Pasi

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of ext Tero Kivinen
> Sent: 19 January, 2010 14:32
> To: Eronen Pasi (Nokia-NRC/Helsinki)
> Cc: [email-masked]
> Subject: [IPsec] Flags in header (was: IKEv2bis, comments about
> sections 3-)
> 
> _______________________________________________
> IPsec mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ipsec

---

From: Paul Hoffman <[email-masked]>
Date: Tue, 19 Jan 2010 15:52:34 -0800

Thanks for the careful review (although co-authors of the document are kinda expected to do this...). Only two things that stuck out as not done:

Based on an earlier comment, I have already reworded that as:

The traffic selector types 7 and 8 can also refer to ICMP or
ICMPv6 type and code fields, as well as MH Type fields for the IPv6
mobility header <xref target='MIPV6'/>.  Note, however, that neither
ICMP nor MIPv6 packets have separate source and destination fields.
The method for specifying the traffic selectors for ICMP and MIPv6 is
shown by example in Section 4.4.1.3 of <xref target='IPSECARCH'/>.

Does that meet your needs?

They do, in fact, help the reader. They tell them that 4306 specified these values but didn't tell you how to use them. That's important.

--Paul Hoffman, Director
--VPN Consortium

---

From: <[email-masked]>
Date: Wed, 20 Jan 2010 14:16:52 +0100

> Does that meet your needs?

Hmm -- which paragraph(s) would be replaced by that text? 
(The important detail "MH type goes in most significant 8 bits"
is *not* in Section 4.4.1.3 of RFC 4301...)

> They do, in fact, help the reader. They tell them that 4306 specified
> these values but didn't tell you how to use them. That's important.

Ok, I can buy that argument. So let's leave them as they are now.

Best regards,
Pasi