Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Jan 2000 21:51:44 -0800
From:      gdonl@tsc.tdk.com (Don Lewis)
To:        Matthew Dillon <dillon@apollo.backplane.com>, Giorgos Keramidas <charon@hades.hell.gr>
Cc:        Brett Glass <brett@lariat.org>, Warner Losh <imp@village.org>, Darren Reed <avalon@coombs.anu.edu.au>, security@FreeBSD.ORG
Subject:   Re: stream.c worst-case kernel paths
Message-ID:  <200001220551.VAA15775@salsa.gv.tsc.tdk.com>
In-Reply-To: Matthew Dillon <dillon@apollo.backplane.com> "Re: stream.c worst-case kernel paths" (Jan 21,  7:59pm)

next in thread | previous in thread | raw e-mail | index | archive | help
On Jan 21,  7:59pm, Matthew Dillon wrote:
} Subject: Re: stream.c worst-case kernel paths

} :(a) drop all multicast packets that reach the tcp stack.
} :(b) extend ICMP_BANDLIM to RST packets, and
} :(c) avoid sending anything tcp to a multicast address

}     That's pretty much it.  I've already sent a patch set to Warner for (b).
}     I don't think we should do (a) or (c) until after the release, multicast
}     isn't going to explode on us in the next 4 months.

Here's a patch that takes care of (a) and (c), but does it outside the
main code path.

This patch also skips the second PCB hash lookup for non-SYN packets.  In
addition to saving some CPU cycles, this part of the patch also forces ACK
packets sent to listening sockets to take the shortest path to
"dropwithreset" (and this particular path happens to already be protected
by ICMP_BANDLIM).  This change also has the advantage of closing the old
hole (fixed in tcp_subr.c 1.48) that allowed port scanners to use ACK
packets to detect listening sockets.

(b) still needs to be generalized to cover other paths that generate
RST packets.

I think ACK packets also might need to be rate limited, because an attacker
can spoof a SYN packet to a listening socket to create a socket in the
SYN_SENT state, then send a large number of packets with the same source
and destination addresses, to which the victim will respond with ACKs.
This will be trickier to implement without causing performance problems
under heavy traffic conditions where a large number of ACKs should actually
be sent.

I'm less enthusiastic about deferring the checksum.  It complicates the
code and the CPU really should be sized to have enough cycles so that
it can checksum legitimate data at full bandwidth.  I don't see much
advantage of optimizing the code so that the machine has more idle cycles
when it is under attack than when it is occupied doing useful work.


This patch has only been lightly tested under benign conditions.


--- tcp_input.c.orig	Fri Jan 21 09:04:37 2000
+++ tcp_input.c	Fri Jan 21 21:06:44 2000
@@ -381,6 +381,7 @@
 	struct tcpopt to;		/* options in this segment */
 	struct rmxp_tao *taop;		/* pointer to our TAO cache entry */
 	struct rmxp_tao	tao_noncached;	/* in case there's no cached entry */
+	int wildcard = 0;
 #ifdef TCPDEBUG
 	short ostate = 0;
 #endif
@@ -513,6 +514,8 @@
 	/*
 	 * Locate pcb for segment.
 	 */
+	if ((thflags & (TH_ACK|TH_SYN)) == TH_SYN)
+		wildcard = 1;
 findpcb:
 #ifdef IPFIREWALL_FORWARD
 	if (ip_fw_fwd_addr != NULL
@@ -533,12 +536,12 @@
 			if (!ip_fw_fwd_addr->sin_port) {
 				inp = in_pcblookup_hash(&tcbinfo, ip->ip_src,
 				    th->th_sport, ip_fw_fwd_addr->sin_addr,
-				    th->th_dport, 1, m->m_pkthdr.rcvif);
+				    th->th_dport, wildcard, m->m_pkthdr.rcvif);
 			} else {
 				inp = in_pcblookup_hash(&tcbinfo,
 				    ip->ip_src, th->th_sport,
 	    			    ip_fw_fwd_addr->sin_addr,
-				    ntohs(ip_fw_fwd_addr->sin_port), 1,
+				    ntohs(ip_fw_fwd_addr->sin_port), wildcard,
 				    m->m_pkthdr.rcvif);
 			}
 		}
@@ -549,12 +552,12 @@
 #ifdef INET6
 	if (isipv6)
 		inp = in6_pcblookup_hash(&tcbinfo, &ip6->ip6_src, th->th_sport,
-					 &ip6->ip6_dst, th->th_dport, 1,
+					 &ip6->ip6_dst, th->th_dport, wildcard,
 					 m->m_pkthdr.rcvif);
 	else
 #endif /* INET6 */
 	inp = in_pcblookup_hash(&tcbinfo, ip->ip_src, th->th_sport,
-	    ip->ip_dst, th->th_dport, 1, m->m_pkthdr.rcvif);
+	    ip->ip_dst, th->th_dport, wildcard, m->m_pkthdr.rcvif);
       }
 
 #ifdef IPSEC
@@ -998,6 +1001,11 @@
 
 		if (thflags & TH_RST)
 			goto drop;
+		/*
+		 * XXX - the following two tests should no longer be
+		 * necessary because of the "wildcard" test added
+		 * above.
+		 */
 		if (thflags & TH_ACK)
 			goto dropwithreset;
 		if ((thflags & TH_SYN) == 0)
@@ -1017,16 +1025,22 @@
 		 * RFC1122 4.2.3.10, p. 104: discard bcast/mcast SYN
 		 * in_broadcast() should never return true on a received
 		 * packet with M_BCAST not set.
+		 *
+		 * Packets with a multicast source address should also
+		 * be discarded.
 		 */
 		if (m->m_flags & (M_BCAST|M_MCAST))
 			goto drop;
 #ifdef INET6
 		if (isipv6) {
-			if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst))
+			if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst) ||
+			    IN6_IS_ADDR_MULTICAST(&ip6->ip6_src))
 				goto drop;
 		} else
 #endif
-		if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)))
+		if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) ||
+		    IN_MULTICAST(ntohl(ip->ip_src.s_addr)) ||
+		    IN_EXPERIMENTAL(ntohl(ip->ip_src.s_addr)))
 			goto drop;
 #ifdef INET6
 		if (isipv6) {
@@ -2217,11 +2231,14 @@
 		goto drop;
 #ifdef INET6
 	if (isipv6) {
-		if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst))
+		if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst) ||
+		    IN6_IS_ADDR_MULTICAST(&ip6->ip6_src))
 			goto drop;
 	} else
 #endif /* INET6 */
-	if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)))
+	if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) ||
+	    IN_MULTICAST(ntohl(ip->ip_src.s_addr)) ||
+	    IN_EXPERIMENTAL(ntohl(ip->ip_src.s_addr)))
 		goto drop;
 	/* IPv6 anycast check is done at tcp6_input() */
 #ifdef TCPDEBUG


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




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