From owner-freebsd-current Mon Dec 16 20:36:13 2002 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EFB2837B401; Mon, 16 Dec 2002 20:36:11 -0800 (PST) Received: from harmony.village.org (rover.bsdimp.com [204.144.255.66]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2ECBE43EDC; Mon, 16 Dec 2002 20:36:11 -0800 (PST) (envelope-from imp@bsdimp.com) Received: from localhost (warner@rover2.village.org [10.0.0.1]) by harmony.village.org (8.12.6/8.12.3) with ESMTP id gBH4a3uB008352; Mon, 16 Dec 2002 21:36:03 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Mon, 16 Dec 2002 21:35:25 -0700 (MST) Message-Id: <20021216.213525.04740792.imp@bsdimp.com> To: dillon@apollo.backplane.com Cc: sam@errno.com, mux@FreeBSD.ORG, obrien@FreeBSD.ORG, current@FreeBSD.ORG Subject: Re: ipfw userland breaks again. From: "M. Warner Losh" In-Reply-To: <200212151940.gBFJeA1l086827@apollo.backplane.com> References: <200212151908.gBFJ811I081774@apollo.backplane.com> <20021215.121446.79106618.imp@bsdimp.com> <200212151940.gBFJeA1l086827@apollo.backplane.com> X-Mailer: Mew version 2.1 on Emacs 21.2 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In message: <200212151940.gBFJeA1l086827@apollo.backplane.com> Matthew Dillon writes: : Here's a new patch. But there isn't much of a point if we do not : also disallow ipfw DELETE and FLUSH. And the pipe config commands : as well as anything else that changes the firewall state. Firewalls : are there to protect the systems behind them. I think deleting the : rule that, say, prevents spoofing is as bad as adding a rule that : allows everything through :-( This comment got me thinking. The thinking lead to a lot of looking at code between compiles today, and more this evening. It would appear that the test that was there was sufficient to deal with the cases that I was worried about. Revisiting the change: - if (sopt->sopt_name == IP_FW_ADD || + if (sopt->sopt_name == IP_FW_ADD || sopt->sopt_name == IP_FW_UNBREAK || (sopt->sopt_dir == SOPT_SET && sopt->sopt_name != IP_FW_RESETLOG)) { Earlier, we only allow IP_FW_{ADD,UNBREAK,RESETLOG,FLUSH,DELETE} for SOPT_SET requests and IP_FW_ADD (and a few others) for SOPT_GET requests. Since GET + ADD is only case that isn't a SET that changes things, the == SOPT_SET takes care of the case that you added. For a while I thought one could do nasty things based on GET + FLUSH, say, but in raw_ip.c, we do the proper checks before calling ip_fw_ctl_ptr(). So it looks like this code is subtle enough to have fooled both of us. This one change isn't needed for this patch. Warner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message