ICMP attacks against TCP and PMTUD

[Nikolay Denev]

On Jan 23, 2012, at 11:17 PM, Andre Oppermann wrote:

> On 23.01.2012 16:01, Nikolay Denev wrote:
>> 
>> On Jan 20, 2012, at 10:32 AM, Nikolay Denev wrote:
>> 
>>> On Jan 15, 2012, at 9:52 PM, Nikolay Denev wrote:
>>> 
>>>> On 15.01.2012, at 21:35, Andrey Zonov<andrey at zonov.org>  wrote:
>>>> 
>>>>> This helped me:
>>>>> /boot/loader.conf
>>>>> net.inet.tcp.hostcache.hashsizee536
>>>>> net.inet.tcp.hostcache.cachelimit66080
>>>>> 
>>>>> Actually, this is a workaround.  As I remember, real problem is in
>>>>> tcp_ctlinput(), it could not update MTU for destination IP if hostcache
>>>>> allocation fails.  tcp_hc_updatemtu() should returns NULL if
>>>>> tcp_hc_insert() returns NULL and tcp_ctlinput() should check this case
>>>>> and sets updated MTU for this particular connection if
>>>>> tcp_hc_updatemtu() fails.  Otherwise we've got infinite loop in MTU
>>>>> discovery.
>>>>> 
>>>>> 
>>>>> On 15.01.2012 22:59, Nikolay Denev wrote:
>>>>>> 
>>>>>> % uptime
>>>>>> 7:57PM  up 608 days,  4:06, 1 user, load averages: 0.30, 0.21, 0.17
>>>>>> 
>>>>>> % vmstat -z|grep hostcache
>>>>>> hostcache:                136,    15372,    15136,      236, 44946965, 10972760
>>>>>> 
>>>>>> 
>>>>>> Hmm… probably I should increase this….
>>>>>> 
>>>>> 
>>>>> --
>>>>> Andrey Zonov
>>>> 
>>>> Thanks, I will test this asap!
>>>> 
>>>> Regards,
>>>> Nikolay
>>> 
>>> I've upgraded from 7.3-STABLE to 8.2-STABLE and bumped significantly the hostcache tunables.
>>> So far so good, I'll report back if I see similar traffic spikes.
>>> 
>> 
>> Seems like I have been wrong about these traffic spikes being attacks, and
>> actually the problem seems to be the pmtu infinite loop Andrey described.
>> I'm now running 8.2-STABLE with hostcache significantly bumped and regularly
>> have more than 20K hostcache entries, which was more than the default limit of 15K I was running with before.
> 
> The bug is real.  Please try the attached patch to fix the issue for IPv4.
> It's against current but should apply to 8 or 9 as well.
> 
> -- 
> Andre
> 
> http://people.freebsd.org/~andre/tcp_subr.c-pmtud-20120123.diff
> 
> Index: netinet/tcp_subr.c
> ===================================================================
> --- netinet/tcp_subr.c	(revision 230489)
> +++ netinet/tcp_subr.c	(working copy)
> @@ -1410,9 +1410,11 @@
> 					     */
> 					    if (mtu <= tcp_maxmtu(&inc, NULL))
> 						tcp_hc_updatemtu(&inc, mtu);
> -					}
> -
> -					inp = (*notify)(inp, inetctlerrmap[cmd]);
> +					    /* XXXAO: Slighly hackish. */
> +					    inp = (*notify)(inp, mtu);
> +					} else
> +					    inp = (*notify)(inp,
> +							inetctlerrmap[cmd]);
> 				}
> 			}
> 			if (inp != NULL)
> @@ -1656,12 +1658,15 @@
>  * based on the new value in the route.  Also nudge TCP to send something,
>  * since we know the packet we just sent was dropped.
>  * This duplicates some code in the tcp_mss() function in tcp_input.c.
> + *
> + * XXXAO: Slight abuse of 'errno'.
>  */
> struct inpcb *
> tcp_mtudisc(struct inpcb *inp, int errno)
> {
> 	struct tcpcb *tp;
> 	struct socket *so;
> +	int mtu;
> 
> 	INP_WLOCK_ASSERT(inp);
> 	if ((inp->inp_flags & INP_TIMEWAIT) ||
> @@ -1671,7 +1676,12 @@
> 	tp = intotcpcb(inp);
> 	KASSERT(tp != NULL, ("tcp_mtudisc: tp == NULL"));
> 
> -	tcp_mss_update(tp, -1, NULL, NULL);
> +	/* Extract the MTU from errno for IPv4. */
> +	if (errno > PRC_NCMDS)
> +		mtu = errno;
> +	else
> +		mtu = -1;
> +	tcp_mss_update(tp, mtu, NULL, NULL);
> 
> 	so = inp->inp_socket;
> 	SOCKBUF_LOCK(&so->so_snd);

Hi Andre,

Thanks for the patch. I will apply it as soon as possible.
I'll probably first try to reproduce the problem locally since I've increased the hostcache
on my nginx balancers already, and changes require reboots which I'm not able to do at the moment.
Will let you know as soon as I have results.

Thanks!