[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [OPSEC] review for draft-ietf-opsec-ip-security-01
Hello Fernando,
On Fri, 21 Aug 2009, Fernando Gont wrote:
Hello, Andrew,
Thanks so much for the thorough review! (and sorry for the "delay" in my
response...)
I'll respond to those comments that I think that need some
clarification. (the rest of them have been applied ot the last rev of
the document).
[Page 8]:
"3.3. TOS ..." -> RFC1349 is obsoleted by the RFC2474 (DSCP), so this
chapter needs a rewrite.
Are you arguing that I should eliminate the TOS discussion? Or that DSCP
No, this byte still remains in IP header, so there should be a discussion
that touches the security implications of it.
should be put into the main header, and the discussion of TOS be
relegated to a somewhat minor section?
If I understand the relation of the RFCs correctly, we should be
discussing the semantics of this byte in terms of RFC2474, not RFC1349.
However, from the security point of view, since we do not know how a
particular end node interprets it, it may be even a good idea to
look at the relationship between both of the semantics.
The current text implies the ToS/Precedence is the current standards
definition - which is AFAIK not the case.
[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.
mmm... not sure what you mean. Could you clarify this one?
It was one long paragraph, so the comment below :)
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?)
Rather than stating that there are stacks that *have* this limitation,
the I-D states that virtually all stacks *can* reassemble datagrams of
at least 9KB.
Re-reading it, it looks ok - it sounds more like a back-up for
the reasoning for a given stack to be prepared to receive the datagrams
of 9K that were fragmented by the other side, so it's ok.
[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.
You mean you could fingerprint the host by "figuring out" the algorithm
used to set the IP ID, or what?
I used this technique several times to understand whether the
particular packet was sent by a host, or by someone sending a packet on
its behalf.
[Page 17]:
"Fingerprinting the physical device from which the packets originate" -
orphan word sequence.
Not sure what you mean....
Ok, that was just a very long title. Visually it looked as part
of the text.
[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 :-)
Well, just pick a very large limit that will never limit anything, and
that's it.
"never limit anything" for a particular environment that I see that I pick
for. And if you send me the options you can never be sure whether I am
going to process them or I am going to persistently drop them.
Rate-limiting alone should be fine - as it allows the options still to be
processed when not under load, whereas the upper bound puts a hard-stop.
[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.
I agree that repetition is a potential source for mistakes, and that
conceptually speaking, they *could* be combined. However, I think it's
useful to be able to read whatever you need to know about each option in
a single section. And that of having "repeated" stuff allows me to use
"descriptive "acronyms" for each option ("LSRR.length" vs.
"SSRR.length", etc.). Please let me know if you feel strongly about this
change.
Ok, let's see. Copypasting from the draft.
valid LSRR:
LSRR.Length >= 3
LSRR.Offset + LSRR.Length < IHL *4
LSRR.Pointer >= 4
LSRR.Pointer % 4 == 0
empty LSRR:
LSRR.Pointer > LSRR.Length
can write LSRR:
LSRR.Length - LSRR.Pointer >= 3
valid SSRR:
SSRR.Length >= 3
SSRR.Offset + SSRR.Length < IHL *4
SSRR.Pointer >= 4
SSRR.Pointer % 4 == 0
empty SSRR:
SSRR.Pointer > SSRR.Length
can write SSRR:
SSRR.Length - SSRR.Pointer >=3
valid RR:
RR.Length >= 3
RR.Offset + RR_Length < IHL *4
RR.Pointer >= 3 <------------- wrong ?
RR.Pointer % 4 == 0
full RR:
RR.Pointer > RR.Length
additional RR validity check:
RR.Pointer - RR.Length >= 3 <---- valid if Pointer is bigger than length + 3 ?
The RR ones that I marked with the arrow are interesting:
"The pointer field is relative to this option, with the minimum legal
value being 4. Therefore, the following check should be performed:
RR.Pointer >= 3
"
-----> So this looks like a bug, the value should be 4, not 3 ?
"
The option is full if:
RR.Pointer > RR.Length
If the option is full, the datagram should be forwarded without
further processing of this option. If not, the following check
should be performed before writing any route data into the option:
RR.Pointer - RR.Length >= 3
If the packet does not pass this check, the packet should be
considered in error, and therefore should be silently dropped.
"
this translates into:
if (RR.pointer > RR.length) {
// full, forward with no recording
} else if (RR.pointer >= 3 + RR.Length) {
// never reached. And the pointer beyond the length is wrong anyway.
}
So, these two should be the case in point :)
And, I think if I had to code this, I'd group the first four conditionals
for all the RR-like options into:
/* Return TRUE if RR-like options (LSRR, SSRR, RR) pass the sanity checks */
bool sane_rrX_option(int ihl, int length, int offset, int pointer) {
return (length >= 3) && (offset+length < ihl*4) &&
(pointer >= 4) && (pointer % 4 == 0);
}
and then call this from the option processing where needed.
Currently the reader has to first spot that the checks are pretty much
the same, then write them down close by to verify this = extra cognitive
load for the reader. And, appears, for the writer as well :-)
So, yes, I'm rather strongly in favour of joining whatever can be joined.
[Page 42]:
Man in the middle threat mention for DSCP: is this the only field for
which the MITM attacks are a concern ?
I reviewed the DSCP section, but found no discussion of MITM attacks.
Could you quote the text you're referring to?
http://tools.ietf.org/html/draft-ietf-opsec-ip-security-00:
" However, if this moderate congestion turned into heavy congestion,
routers should switch to drop packets, regardless of whether the
packets have the ECT codepoint set or not.
A number of other threats could arise if an attacker was a man in the
middle (i.e., was in the middle of the path the packets travel to get
to the destination host). For a detailed discussion of those cases,
we urge the reader to consult Section 16 of RFC 3168.
"
^------
ECN/DSCP and ToS (if the ToS is there at all) should definitely sit
somewhere together IMHO.
[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#" ?
Would just a pointer (reference) address your comment?
yes, reference is ideal.
Step three: should there be more specifics about some randomization for
the process of dropping ?
Hints? (Argue something like "it's recommended that fragments are
flushed on a random basis" or the like?)
Probably. One question about this algorithm in general - is this something
that works and tested in real life with code, or it is something we think
may work if implemented ? If the former, there should be references to the
implementation, if the latter - maybe this deserves a separate doc, and a
a good scrutiny of IPSEC people ?
Also - this algorithm omits the frequently used NAT-T (essentially IPSEC
over UDP/4500)
Could this really be incorporated in the algorithm?
That's what I wonder. :-) I really think now that this piece should be
looked at by the IPSEC people, even if to say "We don't need this because
we never fragment".
Realistically, that's pretty much the case, because if you need to
reassemble the fragments before decryption, you're playing in a much lower
league performance-wise. So, this text might be redundant altogether.
[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.
Well, I don't think you could protect IPsec traffic that relies on
fragmentation. If a have access to the IPsec fragments, I could simply
forge fragments that would lead to a failure in the reassembly process.
Ok. So I have a growing conclusion that then the more complex reassembly
algorithm would be useless then. Either use the system-standard one, or
don't fragment, period. Less bugs :)
[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 ? :-)
Not sure what you mean.
I meant that I do not really see the logic here. You have a middlebox that
forwards the "fragmentA", then "fragmentB". On the way to you they get
reordered. You keep the "fragment B". But according to your statement the
firewall keeps the state according to "fragmentA". So we're doing exactly
the opposite of what you intended to say ?
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",
Isn't this more aggressive than what the existing text recommends?
to an extent. But it's also more predictable - you get packet lost, not
corrupted. Else if you reassemble, either the upper layer detects the
error (so we still have the same problem, but at an upper layer), or it
does not - at which point we are unsure whether there is no error because
the upper layer check is weak, or because we actually reconstructed the
original packet..
OTOH with the "drop if suspicious" idea, there's an easy DoS vector...
You're right. Scratch this.
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).
I agree with this "remaining fragments thing". However, obvious
question: how long should we wait? This might close one door of attack,
but open another one....
same as for regular reassembly. The fact that you have two colliding
fragments, does not matter - you could as well receive two
spoofed fragments for different IP IDs, and there you can't be sure if
they are spoofed or not. So I'd treat these cases functionally the same.
[Page 54]:
sending with high precedence values: ingress filtering ?
ingress-filtering based on the IP addresses, you mean?
Yes, I mean that this is not the business of the host to deal with this -
especially that there's no real recommendation: "yeah. They say you should
not drop this. but bad stuff is easy to do. So, be bear aware!"
(Also, with the DSCP/ToS duality, worth doublechecking on how does it
translate to real forwarding devices?)
mmm... not sure what you mean.
i.e. how much of a problem it really is, and whether it is something that
was shown practically possible, or is more a theory.
[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 ?
What specific behavior would you recommend?
This is not about recommending, but rather about the practical
behaviour. AFAIK, the router would in practice throw such packets down
the drain. Or there exist a real-world routing device that actually
holds all the packets for an unresolved L2 adjacency ?
[Page 56]:
"Dropping packets":
should be mentioned that all dropped packets should be *counted*
somewhere ?
I think I forgot to include the "this should be logged" here. But will
do in the next rev. (As you may have seen, I added a "this should be
logged" statement in every passage of the I-D that recommends to drop a
packet).
Thoughts?
It would be great if we could converge on all the above items, so that I
can address these remaining issues in te next rev of the I-D.
Sure! Let me know if this reply helps.
cheers,
andrew