From owner-freebsd-security Tue Jan 25 9:23:21 2000 Delivered-To: freebsd-security@freebsd.org Received: from rover.village.org (rover.village.org [204.144.255.49]) by hub.freebsd.org (Postfix) with ESMTP id 268E115153 for ; Tue, 25 Jan 2000 09:23:14 -0800 (PST) (envelope-from imp@harmony.village.org) Received: from harmony.village.org (harmony.village.org [10.0.0.6]) by rover.village.org (8.9.3/8.9.3) with ESMTP id KAA06480; Tue, 25 Jan 2000 10:23:11 -0700 (MST) (envelope-from imp@harmony.village.org) Received: from harmony.village.org (localhost.village.org [127.0.0.1]) by harmony.village.org (8.9.3/8.8.3) with ESMTP id KAA04527; Tue, 25 Jan 2000 10:22:58 -0700 (MST) Message-Id: <200001251722.KAA04527@harmony.village.org> To: Brett Glass Subject: Re: Merged patches Cc: security@FreeBSD.ORG In-reply-to: Your message of "Tue, 25 Jan 2000 10:09:32 MST." <4.2.2.20000125095042.01a5aba0@localhost> References: <4.2.2.20000125095042.01a5aba0@localhost> Date: Tue, 25 Jan 2000 10:22:58 -0700 From: Warner Losh Sender: owner-freebsd-security@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org In message <4.2.2.20000125095042.01a5aba0@localhost> Brett Glass writes: : 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). THat's the point. Yes, it is a conservative patch, but we're 4 days from code freeze. *NOTHING* is going to be radical at this point. : 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). : 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. 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.) By what code paths is this possible? Please be specific. : 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. It does? If so, it certainly doesn't ADD them. : In short, I'd only go with this patch as-is if my : purpose were to minimize the changes made before : release. If this were the goal, I'd go back to the : code immediately thereafter and try to tackle some : of the inefficiencies and holes in this key input : path more aggressively. Yes. That's exactly the goal. Like I said in my initial mail, I may remove the ICMP_BANDLIM option as an option, but bump the rate limiter to 1000. But that's about as far as I'd be willing to go at this time. Warner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-security" in the body of the message