Subject: Re: [OPSEC] review for draft-ietf-opsec-ip-security-01

From: Andrew Yourtchenko <[email-masked]>
Date: Tue, 17 Feb 2009 21:17:22 +0100 (CET)

Hello,

I've gone through the doc, below will go comments, anchored to the layout 
as per http://tools.ietf.org/html/draft-gont-opsec-ip-security-01

First, the references that did not get hyperlinked in the text of the 
document.

It's a bit tedious to enumerate all, something similar to the following 
should list them:

wget -o /dev/null -O - http://tools.ietf.org/html/draft-gont-opsec-ip-security-01 |
     egrep '\[<a name|Normative References' |
     fgrep -B 10000 'Normative References</span'

Now - more specific textual comments. There is a "TODO" in the place of 
IP ID comment - I need a bit more thinking on that topic, meanwhile you 
can consider my points from my first mail as an initial placeholder on 
that item.

[Page 6]:

"If the packet does not pass this check, it should be dropped."

I think this event (as other packet drops) should be reflected in
counter variable(s) somewhere that can be inspected - both from the 
security point of view and from the troubleshooting point of view. The 
same applies to other places where it is recommended to drop the packets.

[Page 7]:

"3.2. IHL (Internet Header Length)
..."

More of a nit - the explicit mention of what is to be done when the check 
fails is missing.

[Page 8]:

"3.3. TOS ..." -> RFC1349 is obsoleted by the RFC2474 (DSCP), so this 
chapter needs a rewrite.

[Page 9]:

"3.4. Total Length..."

The section talks about the corner cases with discrepancy of length value 
in the header and the actual packet length, but does not discuss any 
potential interaction with EMTU_R limitations in case there are any.

9kb limit - the document should have some specific references to the 
stacks that have it. Also, probably the reference to the RFC1122 (at least 
for the terminological definition of the EMTU_R?)

[Page 11]:

"3.5.2. Possible security improvements"

TODO - discussion of a quality of IP ID of being able to provide a 
"fingerprint" of the remote host.

[Page 17]:

"Fingerprinting the physical device from which the packets originate" -

orphan word sequence.

[Page 21]:

reference [Ed3f2002] is a dead link - 
also include http://www.phrack.org/issues.html?issue=60&id=12&mode=txt ?

[Page 24]:

"Enforce a limit on the maximum number of options" - to me this looks a 
bit of a dangerous recommendation in this form, as it would cause 
arbitrary hard limits set by middle devices, resulting in creation of "IP 
Option MTU". (Not that anyone is adding a lot of IP options anyway these 
days, so it is more a theoretical nitpick :-)

[Page 28]:

Discussion of the semantics of the "LSRR.Pointer" - might be also simpler 
(for some?) to say that this is a 1-based index of the starting byte 
which will be used as the first storage byte of the address, in an array 
of length LSRR.Length, starting with LSRR option type as the first element.

[Page 31]:

LSRR, SSRR and RR options - can their restrictions be combined as much as 
possible ? To me they look largely similar, and the repetition is a cause 
for potential mistakes, IMHO.

[Page 41]:

stray "</t>"

[Page 42]:

Man in the middle threat mention for DSCP: is this the only field for 
which the MITM attacks are a concern ?

[Page 51]:

The sequence# check: should here be a reference to the section 3.4.3 of 
rfc2406 as a pointer to what constitutes the "valid sequence#" ?

Step three: should there be more specifics about some randomization for 
the process of dropping ?

Also - this algorithm omits the frequently used NAT-T (essentially IPSEC 
over UDP/4500)

[Page 52]:

"virtually impossible" - I'd replace this with "harder" :) And, as the 
MITM is mentioned for DSCP, I'd not make the task easier for IPSEC 
specifically :) the algorighm which is supposed to protect IPSEC should 
be able to resist the on-path attacker.

"Unfortunately, some TCP/IP stacks, when performing fragmentation,
    send the corresponding fragments in reverse order.  In such cases, at
    the point of flushing the fragment buffer, legitimate fragments will
    receive the same treatment as the possible forged fragments."

Should have a reference to the stacks in question.

[Page 53]:

"Additionally, given that many middle-boxes such as firewalls create
    state according to the contents of the first fragment of a given
    packet, it is best that, in the event an end-system receives
    overlapping fragments, it honors the information contained in the
    fragment that was received first."

received first if there was a box behind the middlebox that has reordered 
the packets afterwards, or if there was no such box ? :-)

If the two received fragments contain conflicting information, we do not 
have enough info to discern which of the two is "correct", IMHO. So, we 
should mark the corresponding packet as "bad", treat the reassembly as 
usual, and then drop it with an auditable event. (not drop immediately in 
order to avoid the DoS where colliding fragments would be dropped and 
cause the accumulation of the remaining fragments till the max reassembly 
timeout occurs).

[Page 54]:

sending with high precedence values: ingress filtering ?

(Also, with the DSCP/ToS duality, worth doublechecking on how does it 
translate to real forwarding devices?)

[Page 55]:

Address resolution: the passage about storing the packets for a long time 
on the router - isn't it something that should be directly discouraged, 
precisely because of its big impact ?

[Page 56]:

"Dropping packets":

should be mentioned that all dropped packets should be *counted* 
somewhere ?

thanks,
andrew