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

[Fernando Gont <[e-mail]>]

Hi, Lars,

This is the rest of my response to your review.

Comments in-line.... (I have removed those changes that I will apply as
suggested, and that didn't require further discussion).



> The goal of this draft is to document and discuss attack mitigation 
> techniques that are implemented by various stacks. In some places, 
> the ID still talks about "proposed mechanisms" or "proposed 
> techniques", etc., which isn't quite aligned with the purpose of the 
> draft. Could you check those phrasings and change them to be about 
> what the surveyed implementations do?

Yes, I will. Regarding the document calling it (or other stuff)
"proposed" solutions, that is something that I should fix (I thought I
had done it, already... but apparently there are remaining bits of it).



> INTRODUCTION, paragraph 4:
>> This document discusses the use of the Internet Control Message 
>> Protocol (ICMP) to perform a variety of attacks against the 
>> Transmission Control Protocol (TCP) and other similar protocols.
> 
> I find nothing in this document that is not about TCP. Suggest to 
> remove the "and other similar protocols" bit.

Will do.



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



> Section 2.1., paragraph 3:
>> In the same way, there are a number of scenarios in which an 
>> end-system may generate an ICMP error message if it finds a problem
>>  while processing a datagram.  The received ICMP errors are handed 
>> to the corresponding transport-protocol instance, which will 
>> usually perform a fault recovery function.
> 
> The first sentence is about generating ICMP, the second about 
> receiving it. Move the second sentence to the previous paragraph, 
> which was all about receiving ICMP?

Sorry, I do not follow you. In the second paragraph of Section 2.1, the
first two sentences are about packet generation (first one about ICMP
generation by routers, and the second about ICMP generation by hosts).
Then the third sentence is about ICMP processing. So I'm not sure hwo
you'd like the text to be changed...



> Section 2.1.1., paragraph 0:
>> 2.1.1.  ICMP for IP version 4 (ICMP)
> 
> For completeness sake, you might want to add a sentence about & a 
> reference to RFC4884.

Will do.



> Section 3., paragraph 0:
>> 3.  Constraints in the possible solutions
> 
> This section dances around the main issue without ever explicitly 
> stating it: If a host wants to more carefully inspect ICMP messages 
> before acting on them, it is limited by whatever chunk of the packet
>  that triggered the ICMP the sender chose to include. And the 
> situation is difficult, because <rest of section>.

Will do.


> Section 4.2., paragraph 2:
>> [I-D.ietf-tsvwg-port-randomization] discusses a number of 
>> algorithms to randomize the ephemeral ports used by clients.
> 
> And in it, the IETF does recommend that clients use some form of port
>  randomizartion, right?

Yes. I will rephrase this.




> Section 5.2., paragraph 13:
>> Based on this analysis, most popular TCP implementations treat all
> 
> Odd wording. I doubt that the analysis in this ID has caused existing
>  TCP implementations to change years ago. Do you mean "An analysis of
>  most popular TCP implementations has shown..."?

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



> Section 5.2., paragraph 19:
>> This counter-measure has been implemented in BSD-derived TCP/IP 
>> implementations (e.g., [FreeBSD], [NetBSD], and [OpenBSD]) for more
>>  than ten years [Wright][McKusick].  The Linux kernel has also 
>> implemented this policy for more than ten years [Linux].
> 
> Which counter-measure is "this counter-measure"? The one that is 
> implemented in most stacks, which is described four paragraphs 
> before?

Yes. The whole Section 5.2 is about the same thing (hard_errors ->
soft_errors).



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



>> increasing processor utilization, and degrading the overall system
>>  performance.
> 
> 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"?



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



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



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



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

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