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>