[Gen-art] Gen-ART review for draft-ietf-ipsecme-esp-null-heuristics-05.txt

[Tero Kivinen <[e-mail]>]

Spencer Dawkins writes:
> 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.

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

> 2.1.  AH
> 
>    The another problem is that in the new IPsec Architecture [RFC4301]
> 
> Spencer (clarity): "The second problem" or "Another Problem" here...

Fixed to "Another problem".
      
>    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".

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

> 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!

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

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

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

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

Fixed.

> 4.  IPsec flows
> 
>    ESP is a stateful protocol, meaning there is state stored in the both
> 
> Spencer (nit): s/the both/both/

Fixed.

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

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.

> 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/

Fixed.

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

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

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

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. 

> 
> 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?

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

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

Fixed.

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

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.

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.

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

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.


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

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.


> 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? 

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.
-- 
[e-mail]