Re: [tcpm] AD Review: draft-ietf-tcpm-icmp-attacks-09

[Fernando Gont <[e-mail]>]

Hello, Lars,

>>> INTRODUCTION, paragraph 13:
>>>> Copyright Notice
>>> You're using the IETF Trust Provisions' Section 6.b License
>>> Notice from 12 Sep 2009 rather than the newer Notice from 28 Dec
>>> 2009.  (See http://trustee.ietf.org/license-info/)
>> There was no way (other manual) to fix this (at least when the last
>> rev of the document was published). That said, the "grace period"
>> announced by the IETF Trust still lasts till February 1st, so
>> there's no problem with the old boilerplate text.
> 
> Yes, but it's unlikely that the final revision will be submitted
> before Feb 1? IETF Last Call and IESG Review are still coming.

Hopefully by that time xml2rfc will already support the new boilerplate?
Or, if anybody can provide the correct boilerplate, I could manually
edit the I-D.



>> No, I meant what I wrote: Those implementations that already
>> included the hard_errors -> soft_errors processing did so for the
>> same reason stated in the draft (e.g., I did talk with Kirk
>> McKusick back in 2005 and these guys (and others) got to the same
>> conclusion (e.g., hard-errors -> soft-errors) through the same
>> analysis). That said, other implementations implemented this in
>> response to this I-D (e.g., Cisco, Microsoft, and others). Please
>> see the "ICMP attacks" advisories linked in:
>> http://www.gont.com.ar/advisories/index.html (and there are
>> others).
> 
> But this is a TCPM WG document, and it was your personal involvement
> that caused the changes, and not the TCPM WG document. So it wasn't
> "this analysis" in the sense of this document, but "an analysis done
> by someone who now happens to be editor of the WG document."

I don't get your point. The BSD people implemented the hard_errors->
soft_errors thing as a result of the same reasoning that is included in
the icmp-attacks I-D. Other vendors applied this change in response to
this I-D. (For the most part, they don't care about the "individual
submission" vs. "wg item" thing, as you may have seen that they link the
document as http://www.gont.com.ar/drafts/icmp-attacks-against-tcp.html
which is supposed to always contain the latest revision).

I'm not sure if this serves as a data point, but: many vendors that have
a clue don't even care about the "Informational" vs. "bcp" vs. "std
track" vs. "individual I-D" vs. "wg item i-d" -- particularly when it
comes to security-related issues. It took us something like a year to
adopt this I-D as a wg item, and more than five years to get it
published as an RFC. Unless we do better time-wise, I'm afraid this
situation is unlikely to change (not that I'm happy about it).



>>> Section 7.1., paragraph 1:
>>>> systems determine the Path MTU of an arbitrary internet path.
>>> Nit: s/internet/Internet/ (also in many other places in the ID)
>> I did mean "internet". The mechanism works if you're using it in
>> e.g. a corporate network that is not connected to the Internet.
> 
> So that by "internet" you mean "IP-based isolated network" was
> completely lost on me. Is this really an important point? 

No. I mean any network that uses IETF technology, no matter whether its
connected to the public Internet or not.


> Why
> wouldn't a technique for this that works on the Internet work for
> isolated deployments? And if it does, why don't you simply say
> "Internet"?

Because Internet implies "the Internet" (i.e., the public Internet).



>>> I thought most modern operating systems didn't use hard IRQs
>>> anymore in their stacks? Isn't it all soft-interrupts or similar
>>> now? (I do agree that packet processing overhead per payload byte
>>> goes up with smaller packets, but I don't think it's because of
>>> IRQs.)
>> Would you be happier with "increased interrupt and
>> context-switching rate"?
> 
> I'd just talk about increased processing overheads, but this isn't a
> terribly important issue.

Ok.



>>> Section 7.2., paragraph 1:
>>>> The IETF has standardized a Path-MTU Discovery mechanism called
>>>>  "Packetization Layer Path MTU Discovery" that does not depend
>>>> on ICMP error messages.  Implementation of the aforementioned 
>>>> mechanism in replacement of the traditional PMTUD (specified in
>>>>  [RFC1191] and [RFC1981]) would eliminate this vulnerability. 
>>>> However, it might also lead to an increase of the PMTUD
>>>> convergence time.
>>> Add a citation to RFC4821 here. Also, why the conjunctive?
>>> PLPMTUD *does* eliminate this vulnerability.
>> PLPMTUD can be implemented with or without ICMP support (i.e., as a
>>  replacement for classical PMTUD, or for black-hole detection). 
>> Therefore, it does not necessarily eliminate this vulnerability.
> 
> OK, but then say this in the document. It wasn't clear to me at all
> that you tried to allude to that by saying "would eliminate."

Ok. Will do.




>>> Section 7.2., paragraph 11:
>>>> On the other hand, maxsizeacked holds the size (in octets) of
>>>> the largest packet that has so far been acknowledged for this 
>>>> connection. It is initialized to 68 (the minimum IPv4 MTU) when
>>>> the underlying internet protocol is IPv4, and is initialized to
>>>> 1280 (the minimum IPv6 MTU) when the underlying internet
>>>> protocol is IPv6.  Whenever an acknowledgement for a packet
>>>> larger than maxsizeacked octets is received, maxsizeacked is
>>>> set to the size of that acknowledged packet.
>>> Maybe I'm dense, but how does this work with delayed ACKs? If you
>>> see the left edge of the window shift by n bytes, you don't know
>>> that maxsizeacked is n bytes, right?
>> Correct. The worst-case scenario is that you do the "path mtu
>> update phase" rather than "initial path mtu discovery". i.e., the
>> mechanism fails on the safe side.
> 
> Let's discuss this for a second to make sure I understand this
> correctly.
> 
> The definition for acked_packet_size is: "Variable holding the packet
> size (data, plus headers) the ACK that has just been received is
> acknowledging." But that's actually not accurate, because an incoming
> ACK doesn't ACK a specific transmitted segment - all you know when
> you receive an ACK is that the left edge of the window moved by a
> certain amount. Is acked_packet_size simply set to that amount?

Yes.



> If yes, then with delayed ACKs (or otherwise in corner cases)
> acked_packet_size is actually larger than maxsizesent always. 

Well, that depends on the amount of data you send. But yes, you could
say that "acked_packet_size is actually larger than maxsizesent *usually*".


> On
> second reading, I don't think that's a problem for the technique, but
> the name and description of the variable confused me earlier.

Should I add a clarification?



>>> Section 7.2., paragraph 20:
>>>> MAXSEGRTO can be set, in principle, to any value greater than
>>>> or equal to 0.  Setting MAXSEGRTO to 0 would make TCP perform
>>>> the traditional PMTUD mechanism defined in [RFC1191] and
>>>> [RFC1981].  A MAXSEGRTO of 1 provides enough protection for
>>>> most cases.  In any case, implementations are free to choose
>>>> higher values for this constant.  MAXSEGRTO could be a function
>>>> of the Next-Hop MTU claimed in the received ICMP "Packet Too
>>>> Big" message.  That is, higher values for MAXSEGRTO could be
>>>> imposed when the received ICMP "Packet Too Big" message claims
>>>> a Next-Hop MTU that is smaller than some specified value.
>>> Which value do the different implementations you surveyed use for
>>>  MAXSEGRTO?
>> They use a MAXSEGRTO of 1. I wrote and commited the OpenBSD 
>> implementation myself. NetBSD ported the one in OpenBSD to their
>> OS, so they use the same MAXSEGRTO.
> 
> I forgot to say in my original comment that that may be interesting
> information to put in the draft, for readers who are wondering.

Agreed. Will do.



>>> Section 7.2., paragraph 22:
>>>> Section 7.3 shows the proposed counter-measure in action.
>>>> Section 7.4 shows the proposed counter-measure in pseudo-code.
>>> Since the publication target is Informational, calling this the 
>>> "proposed" something is slightly misleading. Can you rephrase
>>> this to be about what current implementations do?
>> I can, and I will. However, I think this has been overstressed for
>> this I-D. It's an Informational I-D. So it should be clear from the
>> target itself and the Page-1 boilerplate to be used for the RFC
>> that it does not contain any "formal IETF recommendation".
> 
> Not all readers know the differences between different types of RFCs,
> and I'd prefer if the wording in the text was consistent with the
> message that the WG wants to send about these mitigation techniques.

As I said, I have no problem with this. I will remove any remaining
instances of "proposed".

Thanks,
-- 
Fernando Gont
e-mail: [e-mail] || [e-mail]
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1