Subject: [IPsec] AD review of draft-ietf-ipsecme-esp-null-heuristics-03

From: <[email-masked]>
Date: Wed, 27 Jan 2010 12:37:38 +0100

I've now done my AD review for the heuristics draft. Mostly the draft
looks good, and all my comments are relatively minor. Least-minor
first:

- Appendix A.1: The pseudocode has couple of places where it says
"Drop invalid packet"; it seems these are wrong when the packet is UDP
encapsulated (this could still be perfectly valid UDP traffic, just
something else than ESP).

- Section 8.1: AUTH_HMAC_MD5_128 and AUTH_HMAC_SHA1_160 are not
defined for IPsec ESP; these algorithms apply only to the FiberChannel
security protocols. So they should be removed from this list (and
since this was the only algorithm with 160-bit ICV, handling that 
case can be removed).

- Section 8.1: AUTH_AES_128/192/256_GMAC cannot be used in ESP, only
in AH; for ESP, the relevant algorithm is ENCR_NULL_AUTH_AES_GMAC.

- Appendix A.1: shouldn't we also have tests for WESP here?
"If IP protocol is WESP, process as described in [traffic-visibility]"
"If first 4 bytes of UDP packet are 0x00000002, process as.. "
(the details of WESP don't belong there, though, and a pointer would
be quite sufficient IMHO)

- Appendix A.2, "Verify TCP": the bits that are currently reserved
might get allocated in the future (and half of the bits that were
reserved in RFC 793 have been since allocated -- so it's not very
clear exactly what "TCP.reserved_bits" means).

- The document doesn't cover RSA authentication in ESP (RFC 4359). 
I guess this isn't really very relevant for environments where the
heuristics might be used, so perhaps a sentence saying this is beyond
the scope of this document would be sufficient.

- Section 2.1, suggesting that AH might have more bugs doesn't sound
like an argument that belongs in this document.

- Section 2.3: the discussion about IPv6 and NATs does not belong in
this document.

- Section 3, 2nd para: "state of the flows" -> "... IPsec flows"

Best regards,
Pasi

---

From: Tero Kivinen <[email-masked]>
Date: Wed, 27 Jan 2010 16:29:17 +0200

> something else than ESP).

True. Changed those now to continue packet processing with some
comments explaining the situation. 

> since this was the only algorithm with 160-bit ICV, handling that 

Removed.

> in AH; for ESP, the relevant algorithm is ENCR_NULL_AUTH_AES_GMAC.

True. Removed AUTH_AES*_GMAC and added paragraph explaining about the
ENCR_NULL_AUTH_AES_GMAC:

    In addition to the ESP authentication algorithms listed above,
    there is also encryption algorithm ENCR_NULL_AUTH_AES_GMAC which
    does not provide confidentiality but provides authentication, just
    like ESP-NULL does. This algorithm has ICV Length of 128 bits, and
    it also requires eight bytes of IV.

(this is just after the Algorithm / ICV Length table before the
paragarph about the IV). 

> be quite sufficient IMHO)

Added reference and basic selection rules:
----------------------------------------------------------------------
////////////////////////////////////////////////////////////
// This is the main processing code for the packet
// This will check if the packet requires ESP processing,
//
Process packet:
  * If IP protocol is ESP
       * Set SPI_offset to 0 bytes
       * Goto Process ESP
  * If IP protocol is UDP
       * Goto Process UDP
  * If IP protocol is WESP
       // For information about WESP processing see WESP
       // specification.
       * Continue WESP processing
  * Continue Non-ESP processing

////////////////////////////////////////////////////////////
// This code is run for UDP packets, and it checks if the
// packet is UDP encapsulated UDP packet, or UDP
// encapsulated IKE packet, or keepalive packet.
//
Process UDP:
  // Reassembly is not mandatory here, we could
  // do reassembly also only after detecting the
  // packet being UDP encapsulated ESP packet, but
  // that would complicated the pseudocode here
  // a lot, as then we would need to add code
  // for checking if the UDP header is in this
  // packet or not.
  // Reassembly is to simplify things
  * If packet is fragment
       * Do full reassembly before processing
  * If UDP_src_port != 4500 and UDP_dst_port != 4500
       * Continue Non-ESP processing
  * Set SPI_offset to 8 bytes
  * If UDP_len > 4 and first 4 bytes of UDP packet are 0x000000
       * Continue Non-ESP processing (pass IKE-packet)
  * If UDP_len > 4 and first 4 bytes of UDP packet are 0x000002
       * Continue WESP processing
  * If UDP_len == 1 and first byte is 0xff
       * Continue Non-ESP processing (pass NAT-Keepalive Packet)
  * Goto Process ESP

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

Cannot add reference to WESP directly there in pseudocode, as xmltorfc
seems to barf if I try to put <xref> inside the artwork. We do have
reference to WESP already in the main body of text.

> clear exactly what "TCP.reserved_bits" means).

All of those tests are something that depends on other standards, i.e.
TCP. Normally deep inspection engines etc will check all kind of TCP
fields, and depending on the them does something (drop, pass, clear,
set, modify etc), and usually those people writing the deep inspection
engines etc have quite good idea which bits they can check and which
not.

The reason we do not explictly mention what reserved_bits is just
because it can change in future. 

> the scope of this document would be sufficient.

Added paragraph to the end of ESP-NULL format:

   This document does not cover RSA authentication in ESP ([RFC4359]),
   as it is considered being beyond the scope of this document.

> like an argument that belongs in this document.

It was one of the arguments which was given when people said why they
do not want to use AH. 

> - Section 2.3: the discussion about IPv6 and NATs does not belong in

I am trying explain there why I do not belive that modifying all end
peer is that easy thing to do, and modifying the intermediate devices
instead is much faster to deploy.

I think it provides reasons why people might want to implement this
even when WESP is the other alternative.

> - Section 3, 2nd para: "state of the flows" -> "... IPsec flows"

Added "IPsec".

I will submit new -04 version now. Argh, I mean I will submit it AFTER
I have updated my xml2rfc tools again!
-- 
[email-masked]

---

From: "[email-masked]" <[email-masked]>
Date: Thu, 28 Jan 2010 13:18:15 +0100

Tero Kivinen wrote:

> > defined for IPsec ESP; these algorithms apply only to the
> > FiberChannel security protocols. So they should be removed from
> > this list (and since this was the only algorithm with 160-bit ICV,
> > handling that case can be removed).
> 
> Removed.

Section 8.2 still has some old text that assumes five (instead of
four) different ICV lengths.

<snip>
> because it can change in future.

The reserved bits MUST be ignored if you don't understand them,
so testing whether they're zero (and returning failure if they're not)
is not correct behavior here (and would unnecessarily complicate deployment
of new TCP features in the future).

<snip>
> It was one of the arguments which was given when people said why they

Nevertheless, as it's currently written, it sounds more like FUD 
than a reasonable argument. Let's just delete it.

> even when WESP is the other alternative.

I understand the argument you're making, and I think for ESP-NULL
heuristics, it's a reasonable argument.

But the comparison to IPv6 is very misleading here -- I think many
folks would argue that for IPv6 it's been easier to update the end
nodes (many of which do support IPv6 today) than the intermediate
nodes (when e.g. travelling, how often do you find that the
routers/whatever in the access network actually provide v6?). And
probably some folks would say this argument is too simplistic and
wrong, too -- either way, it's not a discussion for this document.

Best regards,
Pasi

---

From: Tero Kivinen <[email-masked]>
Date: Thu, 28 Jan 2010 14:37:14 +0200

> four) different ICV lengths.

Ups. And it seems to affect the calculations, so need to recalculate
them too... 

> of new TCP features in the future).

Ok, I removed that part of pseudocode.

> than a reasonable argument. Let's just delete it.

Ok, removed the ", meaning it might have more bugs than ESP
implementations" part. Would this text be ok:

    AH has also quite complex processing rules compared to ESP when
    calculating the ICV, including things like zeroing out mutable
    fields, also as AH is not as widely used than ESP, the AH support
    is not as well tested in the interoperability events.

or do you think we should leave only the complexcity issue:

    AH has also quite complex processing rules compared to ESP when
    calculating the ICV, including things like zeroing out mutable
    fields.

> wrong, too -- either way, it's not a discussion for this document.

Hmm... true... Ok. I removed the IPv6 vs NAT text so the pragraph
looks like this:

    All of the aforementioned drafts require modification to ESP,
    which requires that all end nodes need to be modified before
    intermediate devices can assume that this new ESP format is in
    use. Updating end nodes will require lots of time. An example of
    slow end-node deployment is IKEv2. Considering an implementation
    that requires both IKEv2 and a new ESP format, it would take
    several years, possibly as long as a decade, before widespread
    deployment.

I will submit new draft tomorrow (before I leave for one week long
vacation). 
-- 
[email-masked]

---

From: <[email-masked]>
Date: Thu, 28 Jan 2010 13:54:46 +0100

Tero Kivinen wrote:

> > > It was one of the arguments which was given when people said why
> > > they do not want to use AH.
> >

I'm OK with either one (but AH is a very stable and mature protocol,
so while it's not as widely used, I would expect the level of testing
has been quite substantial...).

>     several years, possibly as long as a decade, before widespread

OK.

> I will submit new draft tomorrow (before I leave for one week long

I'm leaving for one-week-long trip tomorrow (Friday), so if there's
any chance you could get an update in today, I could ask the
secretariat to start IETF last call :-)

Best regards,
Pasi

---

From: Tero Kivinen <[email-masked]>
Date: Thu, 28 Jan 2010 15:21:16 +0200

> has been quite substantial...).

I think I last tested AH in interop events in San-Diego interop 2000
or so. We might have done some testing also in 2001 Espoo interop. And
some of that testing was for IPv6 conformance testing, and others were
those implementations which didn't support authentication in ESP, so
AH+ESP was only way to get authentication for ESP with them.

I do not think any of the IKEv2 interops tested anything with AH.

So stable and mature yes, not so sure about level of testing, as even
those ikev1 ah-tests was just basic ping tests, without any IP-options
or similar. 

> secretariat to start IETF last call :-)

Ok, I will send new one now (hopefully I do not need to update xml2rfc
again this time, I did it already yesterday :-)
-- 
[email-masked]

---

From: <[email-masked]>
Date: Thu, 28 Jan 2010 14:33:18 +0100

> again this time, I did it already yesterday :-)

Thanks -- I've now asked the secretariat to start the IETF Last Call.

Best regards,
Pasi