Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 25 Jan 2000 22:44:06 -0800
From:      Don Lewis <Don.Lewis@tsc.tdk.com>
To:        Brett Glass <brett@lariat.org>, Warner Losh <imp@village.org>, security@FreeBSD.ORG
Cc:        freebsd-net@FreeBSD.ORG
Subject:   Re: Merged patches
Message-ID:  <200001260644.WAA28985@salsa.gv.tsc.tdk.com>
In-Reply-To: <4.2.2.20000125095042.01a5aba0@localhost>
References:   <4.2.2.20000125095042.01a5aba0@localhost>

next in thread | previous in thread | raw e-mail | index | archive | help
On Jan 25, 10:09am, Brett Glass wrote:
} Subject: Re: Merged patches
} This is a very, VERY concervative patch. As such, it 
} doesn't include rate limiting of RSTs independently of 
} ICMP_BANDLIM (which is really a different beast -- on
} a router, you might NOT want to limit ICMP but want to
} limit bandwidth).

A router probably does want to limit ICMP.

} The patch also does not prevent a 
} non-SYN packet from matching a listening socket (this
} condition is caught later, but piecemeal with many
} individual tests; the coverage isn't comprehensive).

A better (more efficient and less convoluted) fix that the one I posted
in my patch would be to unroll the findpcb loop, but that change is
non-trivial and would not be appropriate to make at this time.   Even
without this particular fix the patch successfully limits the bandwidth
consumed by RST responses when the host is under attack.

} And it does not shield the entire TCP stack from 
} sending or receiving multicast packets -- just this 
} path. It's still possible to emit a TCP packet with 
} a multicast source or destination address after 
} this patch.

How?  The only responses that can be generated are RST packets
and ACK packets.  We check the problematical addresses before
generating RST packets, and ACK packets are sent to the other
endpoint of an "established" connection.  If we prevent the
creation of an established connection where the other endpoint
has a bogus address, then we don't have to worry about sending
an ACK to any bogus addresses.

I don't see how it would be possible to generate a response with
a multicast source address, since the source address has to be
one of the IP addresses on the local host.

} The source addresses of packets are still
} tested to see if they're muticast addresses in MANY
} places instead of in one place.... It seems to
} me that it pays to use a flag in the mbuf (as is
} done with B_CAST) to centralize the test. (A new
} flag called, say, SRC_B_CAST would do this. There's
} room in the flag word.)

That would require doing this test on each incoming packet.  I would
imagine that most of the packets received by ftp.cdrom.com are normal
TCP packets.  Doing extra sanity tests on these packets would consume
CPU cycles that it could be using for other things such as doing
filesystem I/O.  "Optimizing" the network stack so that it drops
multicast-source packets earlier to save CPU cycles when
ftp.cdrom.com is under attack and has its network interface saturated
with multicast-source packets is pointless because there will be
plenty of CPU cycles to spare because no file transfer requests will
be getting through so there won't be any filesystem I/O to do.

I believe that setting an mbuf flag for multicast-source packets would
be the wrong thing to do anyway, since packets with multicast-source
addresses are bogus.  The only thing you want to do with these packets
is to drop them.

} Also, in at least one place (maybe more), the code does
} multiple tests of the TCP option flags in succession.
} Several tests of this kind should generally be merged
} into a switch for speed (the many conditional jumps
} cause pipeline stalls on many processors, especially 
} older ones) and readability.

                if (thflags & TH_RST)
                        goto drop;
                if (thflags & TH_ACK) 
                        goto dropwithreset;
                if ((thflags & TH_SYN) == 0)
                        goto drop; 

vs
		switch (thflags & (TH_RST|TH_ACK|TH_SYN)) {
		case TH_RST:
		case TH_RST|TH_ACK:
		case TH_RST|TH_SYN:
		case TH_RST|TH_ACK|TH_SYN:
			goto drop;
		case TH_ACK:
		case TH_ACK|TH_SYN:
			goto dropwithreset;
		case TH_SYN:
			break;
		case 0:
			goto drop;
		}



To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200001260644.WAA28985>