From owner-freebsd-current Tue Dec 17 10:23:19 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 6533F37B401; Tue, 17 Dec 2002 10:23:18 -0800 (PST) Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0747D43EC5; Tue, 17 Dec 2002 10:23:18 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: from apollo.backplane.com (localhost [127.0.0.1]) by apollo.backplane.com (8.12.5/8.12.5) with ESMTP id gBHINFOM086171; Tue, 17 Dec 2002 10:23:15 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.12.5/8.12.5/Submit) id gBHINFku086170; Tue, 17 Dec 2002 10:23:15 -0800 (PST) (envelope-from dillon) Date: Tue, 17 Dec 2002 10:23:15 -0800 (PST) From: Matthew Dillon Message-Id: <200212171823.gBHINFku086170@apollo.backplane.com> To: "M. Warner Losh" Cc: sam@errno.com, mux@FreeBSD.ORG, obrien@FreeBSD.ORG, current@FreeBSD.ORG Subject: Re: ipfw userland breaks again. References: <200212151908.gBFJ811I081774@apollo.backplane.com> <20021215.121446.79106618.imp@bsdimp.com> <200212151940.gBFJeA1l086827@apollo.backplane.com> <20021216.213525.04740792.imp@bsdimp.com> 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 Huh. Interesting. The IP_FW_ADD test threw me but now that I look at the code more closely it is only there because IP_FW_ADD is a valid SOPT_GET op as well as a SOPT_SET op. But FLUSH and friends are SOPT_SET only. Now I see how it works :-) -Matt :.. :: 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