Subject: Gen-ART review for draft-ietf-ipsecme-esp-null-heuristics-05.txt

From: "Spencer Dawkins" <[email-masked]>
Date: Mon, 15 Feb 2010 22:59:09 -0600

I have been selected as the General Area Review Team (Gen-ART) reviewer for
this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments you
may receive.

Document: draft-ietf-ipsecme-esp-null-heuristics-05.txt
Reviewer: Spencer Dawkins
Review Date: 2010-02-15 (sorry!)
IETF LC End Date: 2010-02-10
IESG Telechat date: (not known)
Summary:

1.2.  Terminology

   IPsec Flow

      An IPsec flow is a stream of packets sharing the same source IP,
      destination IP, protocol (ESP/AH) and SPI.  Strictly speaking, the
      source IP does not need to be as part of the flow identification,
      but as it can be there depending on the receiving implementation
      it is safer to assume it is always part of the flow
      identification.

Spencer (clarity): Last sentence is difficult to parse. My suggestion is to
use something like

An IPsec flow is a stream of packets sharing the same source IP, destination
IP, protocol (ESP/AH) and SPI.  Strictly speaking, the source IP does not
need to be as part of the flow identification, but it can be. For this
reason, it is safer to assume that the source IP is always part of the flow
indentification.

2.1.  AH

   The another problem is that in the new IPsec Architecture [RFC4301]

Spencer (clarity): "The second problem" or "Another Problem" here...

   the support for AH is now optional, meaning not all implementations
   support it.  ESP-NULL has been defined to be mandatory to implement
   by the Cryptographic Algorithm Implementation Requirements for
   Encapsulating Security Payload (ESP) document [RFC4835].

   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.

Spencer (clarity): I think there needs to be a semicolon or other break
before "also".

2.2.  Mandating by Policy

   Another easy way to solve this problem is to mandate the use of ESP-
   NULL with common parameters within an entire organization.  This
   either removes the need for heuristics (if no ESP encrypted traffic
   is allowed at all) or simplifies them considerably (only one set of
   parameters needs to be inspected, e.g. everybody in the organization
   who is using ESP-NULL must use HMAC-SHA-1-96 as their integrity
   algorithm).  This does not work unless one of a pair of communicating
   machines is not under the same administrative domain as the deep

Spencer (minor): I don't understand. I expected this to say "DOES work
unless". The text says that's the only situation where it fails!

   inspection engine.  (IPsec Security Associations must be satisfactory
   to all communicating parties, so only one communicating peer needs to
   have a sufficiently narrow policy.)  Also, such a solution might
   require some kind of centralized policy management to make sure
   everybody in an administrative domain uses the same policy.

Spencer (minor): Is it fair to point out that this type of heuristic will
make changing the common attribute value you're looking for more difficult?
If you decide to move away from HMAC-SHA-1-96, for instance...

3.  Description of Heuristics

   As described in section 7, UDP encapsulated ESP traffic may also have
   have NAPT applied to it, and so there is already a 5-tuple state in
   the stateful inspection gateway

Spencer (nit): missing period for this sentence.

4.  IPsec flows

   ESP is a stateful protocol, meaning there is state stored in the both

Spencer (nit): s/the both/both/

   end nodes of the ESP IPsec SA, and the state is identified by the
   pair of destination IP and SPI.  End nodes also often fix the source
   IP address in an SA unless the destination is a multicast group.
   Typically most (if not all) flows of interest to an intermediate
   device are unicast, so it is safer to assume the receiving node also
   uses a source address, and the intermediate device should therefore
   do the same.  In some cases this might cause extraneous cached ESP
   IPsec SA flows, but by using the source address two distinct flows
   will never be mixed.  For sites which heavily use multicast, such
   traffic is deterministically identifiable (224.0.0.0/4 for IPv4 and
   ff00::0/8 for IPv6), and an implementation can save the space of
   multiple cache entries for a multicast flow by checking the
   destination address first.

   There are several reasons why a single packet might not be enough to
   detect type of flow.  One of them is that the next header number was
   unknown, i.e. if heuristics do not know about the protocol for the
   packet, it cannot verify it has properly detected ESP-NULL
   parameters, even when the packet otherwise looks like ESP-NULL.  If
   the packet does not look like ESP-NULL at all, then encrypted ESP
   status can be returned quickly.  As ESP-NULL heuristics should know
   the same protocols as a deep inspection device, an unknown protocol
   should not be handled any differently than a cleartext instance of an
   unknown protocol if possible.

Spencer (minor): Are you saying that it might not be possible to handle the
two things the same way? I don't understand why. Prohibited by policy, sure,
and there may be other reasons to treat them differently, but I don't 
understand why this is "should" ...

6.  Special and Error Cases

   Each ESP-NULL flow should also keep statistics about how many packets
   have been detected as garbage by deep inspection, how many have
   passed checks, or how many have failed checks with policy violations
   (i.e. failed because actual inspection policy failures, not because

Spencer (clarity): s/because actual/because of actual/

   the packet looked like garbage).  If the number of garbage packets
   suddenly increases (e.g. most of the packets start to be look like
   garbage according to the deep inspection engine), it is possible the
   old ESP-NULL SA was replaced by an identical-SPI encrypting ESP SA.
   If both ends use random SPI generation, this is a very unlikely
   situation (1 in 2^32), but it is possible that some nodes reuse SPI
   numbers (e.g. a 32-bit memory address of the SA descriptor), thus
   this situation needs to be handled.

8.1.  ESP-NULL format

   The currently defined ESP authentication algorithms have 4 different
   lengths for the ICV field.  Most commonly used is 96 bits, and after
   that comes 128 bit ICV lengths.

Spencer (clarity): the second sentence in this paragraph is confusing, but I
think it's also unnecessary. I suggest dropping it... the next paragraph
replaces it nicely, anyway.

   Different ICV lengths for different algorithm:

       Algorithm                           ICV Length
       -------------------------------     ----------
       AUTH_HMAC_MD5_96                    96
       AUTH_HMAC_SHA1_96                   96
       AUTH_AES_XCBC_96                    96
       AUTH_AES_CMAC_96                    96
       AUTH_HMAC_SHA2_256_128              128
       AUTH_HMAC_SHA2_384_192              192
       AUTH_HMAC_SHA2_512_256              256

                                 Figure 2

   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.

Spencer (clarity): I'd add this algorithm to the table, and remove the first
part of the sentence so that you're just describing one of the table
entries.

8.2.  Self Describing Padding Check

   At this point a maximum of 1.6% of packets remain, so the next header

Spencer (minor): If I'm following you, this isn't "1.6% of packets", it's
"1.6% of possible byte values", or something like that, right?

   number is inspected.  If the next header number is known (and
   supported) then the packet can be inspected based on the next header
   number.  If the next header number is unknown (i.e. not any of those
   with protocol checking support) the packet is marked "unsure",
   because there is no way to detect the IV length without inspecting
   the inner protocol payload.

8.3.  Protocol Checks

   The worst case scenario is when an end node starts up communication,
   i.e. it does not have any previous flows through the device.
   Heuristics will run on the first few packets received from the end
   node.  The later subsections mainly cover these bring up cases, as

Spencer (clarity): suggest s/bring up/start-up/, or something like that.

   they are the most difficult.

8.3.1.  TCP checks

   The most obvious field, TCP checksum, might not be usable, as it is
   possible that the packet has already transited a NAT box, thus the IP
   numbers used in the checksum are wrong, thus the checksum is wrong.

Spencer (minor): this isn't something I'm smart about, but would you expect
to see NAT boxes changing IP addresses and not fixing-up transport
checksums? That's begging for the receiver of these packets to discard them
based on checksum mismatches, isn't it? I know a NAT could be doing
anything, but that that seems short-sighted.

   One good method of detection is if a packet is dropped then the next
   packet will most likely be a retransmission of the previous packet.

Spencer (minor): is this true when you have a transmit window size greater
than one packet, so that more than one packet is outstanding? I agree with
the heuristic, but not with the statement that it's a "good method of
detection" - I don't think it will be triggered very often for web browsers,
or or TCP-based streaming media, or anything that's not stop-and-wait.

   Thus if two packets are received with the same source, and
   destination port numbers, and where sequence numbers are either same
   or right after each other, then it's likely a TCP packet has been
   correctly detected.

8.3.4.  SCTP checks

   SCTP chunks can be inspected to see if their lengths are consistent
   across the total length of the IP datagram, so long as TFC padding is
   not present.

Spencer (nit): could you expand "TFC" (this is the first usage in the 
document)?

9.  Security Considerations

   Using ESP-NULL or especially forcing using of it everywhere inside
   the enterprise can have increased risk of sending confidential
   information where eavesdroppers can see it.

Spencer (minor): I'm not arguing with this statement, I'm just confused by 
it. "Increased risk" compared to what? Saying that forbidding encrypted ESP 
makes it easier to eavesdrop doesn't seem profound - was that what you 
meant?

---

From: "Spencer Dawkins" <[email-masked]>
Date: Wed, 17 Feb 2010 09:48:18 -0600

Hi, Tero,

Thanks for the quick response (so I can remember what I was thinking :-)...

Deleting everything that you and I are OK with, I'm looking at these 
questions.

Thanks,

Spencer

> That is why you need centralized policy management... If you have that
> then changing the policy in the whole organization should be quite
> easy (or at least possible)...

I don't feel strongly about this, but do suggest s/uses the same policy/uses 
the same policy, and that changes to that single policy can be coordinated 
throughout the administrative domain/, to capture what you said in your 
response, which I found helpful.

> That is not "SHOULD" in RFC2119 sense (this document does not specify
> protocol so there is no need for 2119 language).
>
> The text is just trying to say that in normal case for deep inspection
> engine it does not matter whether the unknown protocol was with
> ESP-NULL or without it. There is no real reason to change policy based
> on that fact, so both of them can/will/should receive same handling.

I saw that this isn't a 2119 document, but it's hard for people who are 
familiar with 2119 language to ignore the words when they are in lower case 
:D

I really liked the explanation you gave in your response here. I suggest 
picking one of "can/will/should", probably "can", and including your 
response in the document.

The resulting text (with some additional edits) might look something like 
"As ESP-NULL heuristics need to know the same protocols as a deep inspection 
device, an ESP-NULL instance of an unknown protocol can be handled the same 
way as a cleartext instance of the same unknown protocol.

> No, I do expect NAT boxes to fix checksums IF they see them. In
> transport mode ESP-NULL case the checksum is INSIDE the ESP, thus NAT
> boxes will not see it, and cannot fix it.
>
> In the transport mode NAT-T in IPsec, there is special processing
> rules for that in the responder, where they will fix the (decrypted)
> checksums before giving the packet forward (See 3.1.2 of RFC3948).
>
> So as NAT boxes assume that when they see ESP it means the packet is
> encrypted, they not even try to fix the checksum and when the deep
> engine device gets the NATed packet in the checksum is incorrect
> because of that.

OK, that's the part that was missing for me. I would suggest "the packet has 
already transited a NAT box which changed the IP addresses but assumed any 
ESP payload was encryped and did not recalculate the transport checksums 
with the new IP addresses. Thus"

> The deep inspection engine could try to find out the NAT mapping and
> take that in to account when calculating the checksum, but it gets
> quite complicated thus I do not think it is worth while to do that
> here.
>
> As I point or here the retransmission usually have same source and
> destination ports (this is true for both retransmissions, and multiple
> packets in the same transmission window), and then the sequence
> numbers will either be same (retransmission), or right after each
> other (next packet in the same tcp session).
>
> Also the packet that is usually caught by this is the TCP SYN packet,
> and for that there will not be next packets yet, only after the TCP
> SYN ACK is received from the other end, thus in that case there will
> be mostly just retransmissions.
>
> As I pointed out before the most difficult case is the start-up case
> where we do not yet have any state we can match against, and for that
> start-up case this retransmission checks is good.

This explanation is helpful. I'd suggest adding "This hueristic is most 
helpful when only one packet is outstanding. For example, if a TCP SYN 
packet is lost, the next packet would always be a retransmission of the SYN 
packet".

> I meant that I do not belive that enterprices should be forbidding
> encrypted ESP, just to get accounting, logging, network monitoring,
> intrusion detection etc to work, as that makes eavesdroping so much
> easier, but still there are people who thinks that is the right
> solution.

Your explanation was very helpful. I'd suggest

"Forcing use of ESP-NULL everywhere inside the enterprise, so that 
accounting, logging, network monitoring, and intrusion detection all work, 
increases the risk of sending confidential information where eavesdroppers 
can see it"

---

From: Tero Kivinen <[email-masked]>
Date: Wed, 17 Feb 2010 12:12:12 +0200

> reason, it is safer to assume that the source IP is always part of the flow

Fixed. (also fixed typo indentification -> identification). 

> Spencer (clarity): "The second problem" or "Another Problem" here...

Fixed to "Another problem".
      
> Spencer (clarity): I think there needs to be a semicolon or other break

Changed to "...fields. Also as..."

> unless". The text says that's the only situation where it fails!

Yes, you are correct. Either "does work unless", or "does not work
if". Changed to "does work unless". 

> Spencer (nit): missing period for this sentence.

Fixed.

> Spencer (nit): s/the both/both/

Fixed.

> Spencer (clarity): s/because actual/because of actual/

Fixed.

> replaces it nicely, anyway.

Ok, removed the "Most commonly..." part.

>    Different ICV lengths for different algorithm:

The reason it is not added to the table, is that it is not ICV
algorithm for ESP-NULL. It is different encryption algorithm similar
to ESP-NULL. 

> "1.6% of possible byte values", or something like that, right?

True, as we might check these byte values in multiple places. Changed
it to "possible byte values". 

> Spencer (clarity): suggest s/bring up/start-up/, or something like that.

Fixed.

> Spencer (nit): could you expand "TFC" (this is the first usage in the 

It was already used in the section 8.3.2, where it was expanded:

                          The UDP length field might not match the
   overall packet length, as the sender is allowed to include TFC
   (traffic flow confidentiality, see section 2.7 of IP Encapsulating
   Security Payload document [RFC4303]) padding.

-- 
[email-masked]

---

From: Dan McDonald <[email-masked]>
Date: Fri, 19 Feb 2010 11:10:34 -0500

This last note sounds like Tero's addressed all of your concerns, modulo some
wording changes in here.

Am I correct?

Thanks,
Dan

---

From: Dan McDonald <[email-masked]>
Date: Fri, 19 Feb 2010 11:10:34 -0500

Am I correct?

Thanks,
Dan