Re: Request for policy decision: kernel nat vs/and/or natd

[Hiroki Sato]

Ian Smith  wrote
  in <20110108220300.q15...@sola.nimnet.asn.au>:

sm> On Sat, 8 Jan 2011 15:02:29 +1100, Ian Smith wrote:
sm>  > On Fri, 7 Jan 2011, Brandon Gooch wrote:
sm>  >  > On Thu, Dec 23, 2010 at 8:58 AM, Ian Smith  
wrote:
sm> [..]
sm>  >  > > We could:
sm>  >  > >
sm>  >  > > 1) Preference kernel nat over natd when both are enabled.
sm>  >  >
sm>  >  > I vote for #1.
sm>  >
sm>  > Thanks.  So far, that makes an overwhelming majority of 2 / NIL :)
sm>  >
sm>  > I see that h...@freebsd.org has just grabbed two related PRs:
sm>  >
sm>  > kern/148928: [ipfw] Problem with loading of ipfw NAT rules during system 
startup
sm>  > conf/153155: [PATCH] [8.2-BETA1] ipfw rules fail to load cleanly on 
start if nat enabled
sm>  >
sm>  > so this seems a good time to work up patches to that effect for review
sm>  > (/etc/rc.d/ipfw, maybe natd, /etc/rc.firewall) later tonight my time.
sm>
sm> Ok, the attached patches are against HEAD, which is currently identical
sm> to 8-STABLE for these files.  rc.d_ipfw.patch also applies to 7-STABLE
sm> with an offset but rc.firewall.patch needs more work for 7.  I've no box
sm> on which to actually run-test tonight, and will be away for a few days.
sm>
sm> /etc/rc.d/ipfw:
sm>  . prefer kernel nat (loading ipfw_nat) to natd when both are enabled
sm>  . add ipdivert to required_modules - when only natd is enabled - as
sm>proposed by Thomas Sandford in conf/153155 and also re kern/148928
sm>also fixing the related issue in conf/148137 (and possibly others)
sm>  . prefix /etc/rc.d/natd to firewall_coscripts when only natd is enabled
sm>
sm> /etc/rc.d/natd:
sm>  . seems nothing is needed; has KEYWORD nostart and so should only be
sm>started now by ipfw when natd - but not firewall_nat - is enabled
sm>
sm> /etc/rc.firewall:
sm>  . move firewall_nat and natd code into a function, setup_nat()
sm>preferring kernel firewall_nat to natd if both are enabled
sm>  . couldn't resist tidying up that code to within 80 columns
sm>  . call setup_nat also in 'simple' ruleset, with same intent as
sm>proposed in conf/148144 by David Naylor
sm>  . couldn't resist fixing unnecessarily long line in 'workstation'

 The patches look good to me, but one thing I am wondering is
 rc.d/natd invocation in rc.d/ipfw.  When natd_enable="YES", rc.d/natd
 invokes the daemon after the rc.d/ipfw script eventually even if
 firewall_nat_enable="YES".  What do you think about adding natd to
 REQUIRE: line of rc.d/ipfw?  Although I did not test it extensively,
 rc.d/natd can run safely before rc.d/ipfw and using REQUIRE is
 reasonable instead of using $firewall_coscripts from a viewpoint of
 the rc.d framework.

-- Hiroki


pgpRCFD4W4925.pgp
Description: PGP signature

CFR: ipfw0 pseudo-interface clonable

[Hiroki Sato]

Hi,

 I created the attached patch to make the current ipfw0
 pseudo-interface clonable.  The functionality of ipfw0 logging
 interface is not changed by this patch, but the ipfw0
 pseudo-interface is not created by default and can be created with
 the following command:

  # ifconfig ipfw0 create

 Any objection to commit this patch?  The primary motivation for this
 change is that presence of the interface by default increases size of
 the interface list, which is returned by NET_RT_IFLIST sysctl even
 when the sysadmin does not need it.  Also this pseudo-interface can
 confuse the sysadmin and/or network-related userland utilities like
 SNMP agent.  With this patch, one can use ifconfig(8) to
 create/destroy the pseudo-interface as necessary.

-- Hiroki
Index: sys/netinet/ipfw/ip_fw_log.c
===
--- sys/netinet/ipfw/ip_fw_log.c	(revision 234428)
+++ sys/netinet/ipfw/ip_fw_log.c	(working copy)
@@ -46,6 +46,7 @@ __FBSDID("$FreeBSD$");
 #include 
 #include  /* for ETHERTYPE_IP */
 #include 
+#include 
 #include 
 #include 	/* for IFT_ETHER */
 #include 		/* for BPF */
@@ -89,8 +90,11 @@ ipfw_log_bpf(int onoff)
 {
 }
 #else /* !WITHOUT_BPF */
+static struct mtx log_if_mtx;
 static struct ifnet *log_if;	/* hook to attach to bpf */

+#define	IPFWNAME	"ipfw"
+
 /* we use this dummy function for all ifnet callbacks */
 static int
 log_dummy(struct ifnet *ifp, u_long cmd, caddr_t addr)
@@ -116,39 +120,106 @@ ipfw_log_start(struct ifnet* ifp)
 static const u_char ipfwbroadcastaddr[6] =
 	{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };

-void
-ipfw_log_bpf(int onoff)
+static int
+ipfw_log_clone_match(struct if_clone *ifc, const char *name)
 {
+
+	return (strncmp(name, IPFWNAME, sizeof(IPFWNAME) - 1) == 0);
+}
+
+static int
+ipfw_log_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
+{
+	int error;
+	int unit;
 	struct ifnet *ifp;

-	if (onoff) {
-		if (log_if)
-			return;
-		ifp = if_alloc(IFT_ETHER);
-		if (ifp == NULL)
-			return;
-		if_initname(ifp, "ipfw", 0);
-		ifp->if_mtu = 65536;
-		ifp->if_flags = IFF_UP | IFF_SIMPLEX | IFF_MULTICAST;
-		ifp->if_init = (void *)log_dummy;
-		ifp->if_ioctl = log_dummy;
-		ifp->if_start = ipfw_log_start;
-		ifp->if_output = ipfw_log_output;
-		ifp->if_addrlen = 6;
-		ifp->if_hdrlen = 14;
-		if_attach(ifp);
-		ifp->if_broadcastaddr = ipfwbroadcastaddr;
-		ifp->if_baudrate = IF_Mbps(10);
-		bpfattach(ifp, DLT_EN10MB, 14);
+	error = ifc_name2unit(name, &unit);
+	if (error)
+		return (error);
+
+	error = ifc_alloc_unit(ifc, &unit);
+	if (error)
+		return (error);
+
+	ifp = if_alloc(IFT_ETHER);
+	if (ifp == NULL) {
+		ifc_free_unit(ifc, unit);
+		return (ENOSPC);
+	}
+	ifp->if_dname = IPFWNAME;
+	ifp->if_dunit = unit;
+	snprintf(ifp->if_xname, IFNAMSIZ, "%s%d", IPFWNAME, unit);
+	strlcpy(name, ifp->if_xname, len);
+	ifp->if_mtu = 65536;
+	ifp->if_flags = IFF_UP | IFF_SIMPLEX | IFF_MULTICAST;
+	ifp->if_init = (void *)log_dummy;
+	ifp->if_ioctl = log_dummy;
+	ifp->if_start = ipfw_log_start;
+	ifp->if_output = ipfw_log_output;
+	ifp->if_addrlen = 6;
+	ifp->if_hdrlen = 14;
+	ifp->if_broadcastaddr = ipfwbroadcastaddr;
+	ifp->if_baudrate = IF_Mbps(10);
+
+	mtx_lock(&log_if_mtx);
+	if (log_if == NULL) {
 		log_if = ifp;
+		mtx_unlock(&log_if_mtx);
 	} else {
-		if (log_if) {
-			ether_ifdetach(log_if);
-			if_free(log_if);
-		}
+		mtx_unlock(&log_if_mtx);
+		if_free(ifp);
+		ifc_free_unit(ifc, unit);
+		return (EEXIST);
+	}
+	if_attach(ifp);
+	bpfattach(ifp, DLT_EN10MB, 14);
+
+	return (0);
+}
+
+static int
+ipfw_log_clone_destroy(struct if_clone *ifc, struct ifnet *ifp)
+{
+	int unit;
+
+	if (ifp == NULL)
+		return (0);
+
+	mtx_lock(&log_if_mtx);
+	if (log_if != NULL && ifp == log_if)
 		log_if = NULL;
+	else {
+		mtx_unlock(&log_if_mtx);
+		return (EINVAL);
 	}
+	mtx_unlock(&log_if_mtx);
+
+	unit = ifp->if_dunit;
+	bpfdetach(ifp);
+	if_detach(ifp);
+	if_free(ifp);
+	ifc_free_unit(ifc, unit);
+
+	return (0);
 }
+
+static struct if_clone ipfw_log_cloner = IFC_CLONE_INITIALIZER(
+IPFWNAME, NULL, IF_MAXUNIT,
+NULL, ipfw_log_clone_match, ipfw_log_clone_create, ipfw_log_clone_destroy);
+
+void
+ipfw_log_bpf(int onoff)
+{
+
+	if (onoff) {
+		mtx_init(&log_if_mtx, "ipfw log_if mtx", NULL, 0);
+		if_clone_attach(&ipfw_log_cloner);
+	} else {
+		if_clone_detach(&ipfw_log_cloner);
+		mtx_destroy(&log_if_mtx);
+	}
+}
 #endif /* !WITHOUT_BPF */

 /*


pgpSsBQkAqBtv.pgp
Description: PGP signature

Re: CFR: ipfw0 pseudo-interface clonable

[Hiroki Sato]

"Alexander V. Chernikov"  wrote
  in <4f96d11b.2060...@freebsd.org>:

me> On 24.04.2012 19:26, Hiroki Sato wrote:
me> > Hi,
me> >
me> >   I created the attached patch to make the current ipfw0
me> >   pseudo-interface clonable.  The functionality of ipfw0 logging
me> >   interface is not changed by this patch, but the ipfw0
me> >   pseudo-interface is not created by default and can be created with
me> >   the following command:
me> >
me> ># ifconfig ipfw0 create
me> >
me> >   Any objection to commit this patch?  The primary motivation for this
me> >   change is that presence of the interface by default increases size of
me> >   the interface list, which is returned by NET_RT_IFLIST sysctl even
me> >   when the sysadmin does not need it.  Also this pseudo-interface can
me> >   confuse the sysadmin and/or network-related userland utilities like
me> >   SNMP agent.  With this patch, one can use ifconfig(8) to
me> >   create/destroy the pseudo-interface as necessary.
me>
me> ipfw_log() log_if usage is not protected, so it is possible to trigger
me> use-after-free.

 Ah, right.  I will revise lock handling and resubmit the patch.

me> Maybe it is better to have some interface flag which makes
me> NET_RT_IFLIST skip given interface ?

 I do not think so.  NET_RT_IFLIST should be able to list all of the
 interfaces because it is the purpose.

-- Hiroki


pgp3pMv2jzIWt.pgp
Description: PGP signature

Re: CFR: ipfw0 pseudo-interface clonable

[Hiroki Sato]

"Alexander V. Chernikov"  wrote
  in <4f96e71b.9020...@freebsd.org>:

me> On 24.04.2012 21:05, Hiroki Sato wrote:
me> > "Alexander V. Chernikov"  wrote
me> >in<4f96d11b.2060...@freebsd.org>:
me> >
me> > me>  On 24.04.2012 19:26, Hiroki Sato wrote:
me> > me>  >  Hi,
me> > me>  >
me> > me>  >I created the attached patch to make the current ipfw0
me> > me> > pseudo-interface clonable.  The functionality of ipfw0 logging
me> > me>  >interface is not changed by this patch, but the ipfw0
me> > me> > pseudo-interface is not created by default and can be created
me> > with
me> > me>  >the following command:
me> > me>  >
me> > me>  > # ifconfig ipfw0 create
me> > me>  >
me> > me> > Any objection to commit this patch?  The primary motivation for
me> > this
me> > me> > change is that presence of the interface by default increases
me> > size of
me> > me> > the interface list, which is returned by NET_RT_IFLIST sysctl
me> > even
me> > me> > when the sysadmin does not need it.  Also this pseudo-interface
me> > can
me> > me> > confuse the sysadmin and/or network-related userland utilities
me> > like
me> > me>  >SNMP agent.  With this patch, one can use ifconfig(8) to
me> > me>  >create/destroy the pseudo-interface as necessary.
me> > me>
me> > me> ipfw_log() log_if usage is not protected, so it is possible to
me> > trigger
me> > me>  use-after-free.
me> >
me> >   Ah, right.  I will revise lock handling and resubmit the patch.
me> >
me> > me>  Maybe it is better to have some interface flag which makes
me> > me>  NET_RT_IFLIST skip given interface ?
me> >
me> >   I do not think so.  NET_RT_IFLIST should be able to list all of the
me> >   interfaces because it is the purpose.
me> Okay, another try (afair already discussed somewhere):
me> Do we really need all BPF providers to have ifnets?
me> It seems that removing all bp_bif depends from BPF code is not so hard
me> task.

 Hmm, I cannot imagine how to decouple ifnet from the bpf code because
 bpf heavily depends on it in its API (you probably know better than
 me).  Do you have any specific idea?

-- Hiroki


pgpwbo6UdUx9L.pgp
Description: PGP signature

Re: CFR: ipfw0 pseudo-interface clonable

[Hiroki Sato]

Hiroki Sato  wrote
  in <20120425.020518.406495893112283552@allbsd.org>:

hr> "Alexander V. Chernikov"  wrote
hr>   in <4f96d11b.2060...@freebsd.org>:
hr>
hr> me> On 24.04.2012 19:26, Hiroki Sato wrote:
hr> me> > Hi,
hr> me> >
hr> me> >   I created the attached patch to make the current ipfw0
hr> me> >   pseudo-interface clonable.  The functionality of ipfw0 logging
hr> me> >   interface is not changed by this patch, but the ipfw0
hr> me> >   pseudo-interface is not created by default and can be created with
hr> me> >   the following command:

hr> me> ipfw_log() log_if usage is not protected, so it is possible to trigger
hr> me> use-after-free.
hr>
hr>  Ah, right.  I will revise lock handling and resubmit the patch.

Michael Sierchio  wrote
  in :

ku> A man page for the ipfw pseudo-interface would be welcome.

 A revised patch is attached.  The lock around log_if should be fixed
 and ipfw(8) manual page is updated.  Also, an rc.conf(5) variable
 $firewall_logif is added to create ipfw0 interface at boot time (NO
 by default).

 Any comments are welcome.  Thank you.

-- Hiroki
Index: sys/netinet/ipfw/ip_fw_log.c
===
--- sys/netinet/ipfw/ip_fw_log.c	(revision 234428)
+++ sys/netinet/ipfw/ip_fw_log.c	(working copy)
@@ -44,8 +44,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include  /* for ETHERTYPE_IP */
 #include 
+#include 
 #include 
 #include 	/* for IFT_ETHER */
 #include 		/* for BPF */
@@ -90,7 +93,16 @@
 }
 #else /* !WITHOUT_BPF */
 static struct ifnet *log_if;	/* hook to attach to bpf */
+static struct rwlock log_if_lock;
+#define	LOGIF_LOCK_INIT(x)	rw_init(&log_if_lock, "ipfw log_if lock")
+#define	LOGIF_LOCK_DESTROY(x)	rw_destroy(&log_if_lock)
+#define	LOGIF_RLOCK(x)		rw_rlock(&log_if_lock)
+#define	LOGIF_RUNLOCK(x)	rw_runlock(&log_if_lock)
+#define	LOGIF_WLOCK(x)		rw_wlock(&log_if_lock)
+#define	LOGIF_WUNLOCK(x)	rw_wunlock(&log_if_lock)

+#define	IPFWNAME	"ipfw"
+
 /* we use this dummy function for all ifnet callbacks */
 static int
 log_dummy(struct ifnet *ifp, u_long cmd, caddr_t addr)
@@ -116,37 +128,104 @@
 static const u_char ipfwbroadcastaddr[6] =
 	{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };

+static int
+ipfw_log_clone_match(struct if_clone *ifc, const char *name)
+{
+
+	return (strncmp(name, IPFWNAME, sizeof(IPFWNAME) - 1) == 0);
+}
+
+static int
+ipfw_log_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
+{
+	int error;
+	int unit;
+	struct ifnet *ifp;
+
+	error = ifc_name2unit(name, &unit);
+	if (error)
+		return (error);
+
+	error = ifc_alloc_unit(ifc, &unit);
+	if (error)
+		return (error);
+
+	ifp = if_alloc(IFT_ETHER);
+	if (ifp == NULL) {
+		ifc_free_unit(ifc, unit);
+		return (ENOSPC);
+	}
+	ifp->if_dname = IPFWNAME;
+	ifp->if_dunit = unit;
+	snprintf(ifp->if_xname, IFNAMSIZ, "%s%d", IPFWNAME, unit);
+	strlcpy(name, ifp->if_xname, len);
+	ifp->if_mtu = 65536;
+	ifp->if_flags = IFF_UP | IFF_SIMPLEX | IFF_MULTICAST;
+	ifp->if_init = (void *)log_dummy;
+	ifp->if_ioctl = log_dummy;
+	ifp->if_start = ipfw_log_start;
+	ifp->if_output = ipfw_log_output;
+	ifp->if_addrlen = 6;
+	ifp->if_hdrlen = 14;
+	ifp->if_broadcastaddr = ipfwbroadcastaddr;
+	ifp->if_baudrate = IF_Mbps(10);
+
+	LOGIF_WLOCK();
+	if (log_if == NULL)
+		log_if = ifp;
+	else {
+		LOGIF_WUNLOCK();
+		if_free(ifp);
+		ifc_free_unit(ifc, unit);
+		return (EEXIST);
+	}
+	LOGIF_WUNLOCK();
+	if_attach(ifp);
+	bpfattach(ifp, DLT_EN10MB, 14);
+
+	return (0);
+}
+
+static int
+ipfw_log_clone_destroy(struct if_clone *ifc, struct ifnet *ifp)
+{
+	int unit;
+
+	if (ifp == NULL)
+		return (0);
+
+	LOGIF_WLOCK();
+	if (log_if != NULL && ifp == log_if)
+		log_if = NULL;
+	else {
+		LOGIF_WUNLOCK();
+		return (EINVAL);
+	}
+	LOGIF_WUNLOCK();
+
+	unit = ifp->if_dunit;
+	bpfdetach(ifp);
+	if_detach(ifp);
+	if_free(ifp);
+	ifc_free_unit(ifc, unit);
+
+	return (0);
+}
+
+static struct if_clone ipfw_log_cloner = IFC_CLONE_INITIALIZER(
+IPFWNAME, NULL, IF_MAXUNIT,
+NULL, ipfw_log_clone_match, ipfw_log_clone_create, ipfw_log_clone_destroy);
+
 void
 ipfw_log_bpf(int onoff)
 {
-	struct ifnet *ifp;

 	if (onoff) {
-		if (log_if)
-			return;
-		ifp = if_alloc(IFT_ETHER);
-		if (ifp == NULL)
-			return;
-		if_initname(ifp, "ipfw", 0);
-		ifp->if_mtu = 65536;
-		ifp->if_flags = IFF_UP | IFF_SIMPLEX | IFF_MULTICAST;
-		ifp->if_init = (void *)log_dummy;
-		ifp->if_ioctl = log_dummy;
-		ifp->if_start = ipfw_log_start;
-		ifp->if_output = ipfw_log_output;
-		ifp->if_addrlen = 6;
-		ifp->if_hdrlen = 14;
-		if_attach(ifp);
-		ifp->if_broadcastaddr = ipfwbroadcastaddr;
-		ifp->if_baudrate = IF_Mbps(10);
-		bpfattach(ifp, DLT_EN10MB, 14);
-		log_if = ifp;
+		LOGI

net.inet{,6}.fw.enable in /etc/rc

[Hiroki Sato]

Hi,

 I would like your comments about the attached patch to /etc/rc.

 The problem I want to fix by this patch is as follows.
 net.inet{,6}.fw.enable are set to 1 by default at boot time if IPFW
 kernel module is loaded or statically compiled into a kernel.  And by
 default IPFW has only a "deny ip from any to any" rule if it is
 compiled without IPFIREWALL_DEFAULT_TO_ACCEPT option.  In this case,
 the default-deny rule can prevent rc.d scripts before rc.d/ipfw from
 working as described in the patch.

 To fix this, the patch turns IPFW off before running rc.d scripts at
 boot time, and enables it again in rc.d/ipfw script.

 I think most of users use GENERIC kernel + ipfw kernel module.  In
 that case, IPFW is not activated before rc.d/ipfw script regardless
 of this patch, so there is no user-visible change.  This patch
 affects only a combination of a kernel with IPFW compiled and rc.d
 scripts running before rc.d/ipfw.  The behavior will be almost the
 same as GENERIC kernel + ipfw kernel module's.

 Please let me know if I am missing something.

-- Hiroki
Index: etc/rc
===
--- etc/rc	(revision 271853)
+++ etc/rc	(working copy)
@@ -87,6 +87,17 @@
 	fi
 fi

+# Clear *.fw.enable sysctls.  At boot time, some of network initialization
+# before rc.d/ipfw script requires network communications (e.g. DHCP and
+# IPv6 Duplicate Address Detection).  When *.fw.enable=1 and "default deny"
+# policy was applied---this can happen when IPFW is complied into the kernel
+# or ipfw kernel module is loaded by loader before rc.d/ipfw runs, those
+# comminucations are blocked.  To prevent this, set *.fw.enable=0 before
+# calling rc.d scripts.  The rc.d/ipfw script set this to 1 after
+# configuration.
+/sbin/sysctl -q net.inet.ip.fw.enable=0
+/sbin/sysctl -q net.inet6.ip6.fw.enable=0
+
 # If the firstboot sentinel doesn't exist, we want to skip firstboot scripts.
 if ! [ -e ${firstboot_sentinel} ]; then
 	skip_firstboot="-s firstboot"


pgpcTBGLkMrIS.pgp
Description: PGP signature

Re: net.inet{,6}.fw.enable in /etc/rc

[Hiroki Sato]

Julian Elischer  wrote
  in <542155fb.9020...@freebsd.org>:

ju> On 9/23/14, 2:01 AM, Andrey V. Elsukov wrote:
ju> > On 21.09.2014 09:58, Hiroki Sato wrote:
ju> >> Hi,
ju> >>
ju> >>   I would like your comments about the attached patch to /etc/rc.
ju> >>
ju> >>   The problem I want to fix by this patch is as follows.
ju> >>   net.inet{,6}.fw.enable are set to 1 by default at boot time if IPFW
ju> >>   kernel module is loaded or statically compiled into a kernel.  And by
ju> >>   default IPFW has only a "deny ip from any to any" rule if it is
ju> >>   compiled without IPFIREWALL_DEFAULT_TO_ACCEPT option.  In this case,
ju> >>   the default-deny rule can prevent rc.d scripts before rc.d/ipfw from
ju> >>   working as described in the patch.
ju> >>
ju> >>   To fix this, the patch turns IPFW off before running rc.d scripts at
ju> >>   boot time, and enables it again in rc.d/ipfw script.
ju> > Hi,
ju> >
ju> > I think this should be configurable, the change can be an unexpected
ju> > for
ju> > someone.
ju> it does open a window where there is networking but no firewalling.
ju> given that a reboot is remotely detectable. (ping stops responding
ju> etc.)
ju> there is a possibility that a targeted attack could include
ju> "use exploit ABC to cause a crash of the target and then strike with
ju> exploit XYZ after target system reboots while the firewall is
ju> disabled".
ju>
ju> I have not evaluated the danger of this window.

 Yes, certainly this opens a window between rc.d/netif to rc.d/ipfw.
 I admit it is a problem of disabling IPFW unconditionally.

 Does ipfw have rules which depend on interface initialization?  If
 not, moving rc.d/ipfw to just before rc.d/netif may be a better idea.

-- Hiroki


pgpSK5rPI6G7m.pgp
Description: PGP signature

Re: net.inet{,6}.fw.enable in /etc/rc

[Hiroki Sato]

Ian Smith  wrote
  in <20141003025830.d48...@sola.nimnet.asn.au>:

sm> which rules will be flushed when /etc/rc.d/ipfw runs, but should enable
sm> DHCP to work?  I'm not sure whether those rules are exactly correct or
sm> sufficient for DHCP, but principle is to anly allow what's necessary in
sm> the circumstances this addresses, vastly reducing vulnerable window.
sm>
sm> Using such a method, there should be no need to modify rc.d/ipfw?

 I created an experimental patch based on an idea installing a minimal
 ruleset.  Please review the attached patch.  rc.d/ipfw0 script to
 install such a ruleset is invoked before rc.d/netif.  The following
 two knobs are added:

 $firewall_minimal_rules_enable
Enable/disable installing a minimal ruleset.

 $firewall_minimal_ruleset
Ruleset number to be used for the ruleset.

sm>  >  Does ipfw have rules which depend on interface initialization?  If
sm>  >  not, moving rc.d/ipfw to just before rc.d/netif may be a better idea.
sm>
sm> It can.  If using (say) mpd with dialup or ADSL modems, as I do, the
sm> interface - here ng0 - needs to preexist, needing an IP address too.
sm>
sm> I think that by now, many will likely rely on netif preceding ipfw.

 AFAICC an IPFW rule for ng0 can be installed before the interface is
 created.  Do you have a specific rule which is problematic if IPFW
 rules are loaded before rc.d/netif runs?  I am also using mpd and a
 lot of cloned interfaces on my router box but it worked fine.

-- Hiroki
Index: etc/defaults/rc.conf
===
--- etc/defaults/rc.conf	(revision 272887)
+++ etc/defaults/rc.conf	(working copy)
@@ -116,6 +116,11 @@
 wpa_supplicant_conf_file="/etc/wpa_supplicant.conf"
 #
 firewall_enable="NO"		# Set to YES to enable firewall functionality
+firewall_minimal_rules_enable="YES"	# Set to YES to temporarily apply
+# minimal rules required for interface
+# initialization before applying the full rules.
+firewall_minimal_ruleset="30"	# Ruleset number for minimal rules
+firewall_link_enable="NO"	# Set to YES to enable L2 filtering
 firewall_script="/etc/rc.firewall" # Which script to run to set up the firewall
 firewall_type="UNKNOWN"		# Firewall type (see /etc/rc.firewall)
 firewall_quiet="NO"		# Set to YES to suppress rule display
Index: etc/rc.firewall
===
--- etc/rc.firewall	(revision 272887)
+++ etc/rc.firewall	(working copy)
@@ -42,6 +42,7 @@

 
 # Define the firewall type in /etc/rc.conf.  Valid values are:
+#   minimal - will allow only packets required for interface initialization
 #   open- will allow anyone in
 #   client  - will try to protect just this machine
 #   simple  - will try to protect a whole network
@@ -138,8 +139,14 @@
 #
 ${fwcmd} -f flush

-setup_loopback
-setup_ipv6_mandatory
+case ${firewall_type} in
+[Mm][Ii][Nn][Ii][Mm][Aa][Ll])
+;;
+*)
+	setup_loopback
+	setup_ipv6_mandatory
+;;
+esac

 
 # Network Address Translation.  All packets are passed to natd(8)
@@ -187,6 +194,51 @@
 # Prototype setups.
 #
 case ${firewall_type} in
+[Mm][Ii][Nn][Ii][Mm][Aa][Ll])
+	#
+	# Temporary rule set for network interface initialization.
+	#
+	case $firewall_minimal_ruleset in
+	[0-9]|[12][0-9]|30)
+		# Valid if 0-30.
+	;;
+	*)
+		warn "Invalid ruleset number: $firewall_minimal_ruleset."
+		false
+	;;
+	esac
+	$fwcmd -q set disable $firewall_minimal_ruleset
+	$fwcmd -q delete set $firewall_minimal_ruleset
+
+	_set="set $firewall_minimal_ruleset"
+
+	# DHCPv4
+	# DHCPDISCOVER (from 0.0.0.0/32)
+	# DHCPREQUEST (broadcast)
+	$fwcmd -q add 65001 $_set allow udp \
+	from any to 255.255.255.255/32 \
+	mac ff:ff:ff:ff:ff:ff any \
+	src-port 68 dst-port 67 layer2 out
+
+	# DHCPREQUEST (unicast)
+	$fwcmd -q add 65001 $_set allow udp \
+	from any to any \
+	src-port 68 dst-port 67 layer2 out
+
+	# DHCPOFFER, DHCPACK
+	$fwcmd -q add 65001 $_set allow udp \
+	from any to any \
+	src-port 67 dst-port 68 layer2 in
+
+	# TODO: DHCPv6 65002
+
+	# ICMPv6 DAD
+	$fwcmd -q add 65003 $_set allow ipv6-icmp from :: to ff02::/16
+
+	# ICMPv6 link-local communication including ND/NS and RS/RA
+	$fwcmd -q add 65004 $_set allow ipv6-icmp from fe80::/10 to fe80::/10
+	$fwcmd -q add 65004 $_set allow ipv6-icmp from fe80::/10 to ff02::/16
+	;;
 [Oo][Pp][Ee][Nn])
 	${fwcmd} add 65000 pass all from any to any
 	;;
Index: etc/rc.d/ipfw
===
--- etc/rc.d/ipfw	(revision 272887)
+++ etc/rc.d/ipfw	(working copy)
@@ -31,6 +31,15 @@
 	if checkyesno firewall_nat_enable; then
 		required_modules="$required_modules ipfw_nat"
 	fi
+	if checkyesno firewall_minimal_rules_enable; then
+		# Remove minimum ruleset.
+		/sbin/ipfw delete set $firewall_minimal_ruleset
+	fi
+	if checkyesno firewall_link_enable; then
+		${SYSCTL_W} net.link.ether.ipfw=1
+	else
+		${SYSCTL_W} net.link.ether.ipfw=0
+	fi
 }

 

Re: net.inet{,6}.fw.enable in /etc/rc

[Hiroki Sato]

Ian Smith  wrote
  in <20141013202423.j56...@sola.nimnet.asn.au>:

sm> Anyway, looking at rcorder /etc/rc.d/* there are quite a few possible
sm> interdependencies to explore before considering moving ipfw, including
sm> its relationship to pf - some people do use both - and perhaps routing,
sm> bridge, defaultroute, among others?  Hard to imagine all usage cases ..

 I do not think ordering of ipfw and pf is important.  Configuration
 of routes and others can depend on netif, but not on ipfw and other
 packet filters.  One possible obstacle is a packet filter rule
 requires ifname or ifindex before the rule is added.  IPFW does not
 have such restriction, though.

sm> I like the general idea of running ipfw0 (ono) earlier to enable needed
sm> rules in this specific context - perhaps also testing whether rc.conf
sm> indicates that DHCP (or ipv6 equivalents, about which I know nothing) is
sm> actually required on at least one of the to-be-configured interfaces?
sm>
sm> I don't think it's necessary to use a special set, set 0 would be fine
sm> as any firewall script will begin with a flush anyway.  I don't think
sm> this function need require any user configuration at all, if possible,
sm> as it really is coping with an (albeit important) edge case.
sm>
sm> Couldn't proposed additions to rc.firewall just go into rc.d/ipfw0?

 I think such a testing is more complex in practice from my experience
 with improving network.subr.

 Users who want the default-deny rule at boot time should be strict
 about the rule set.  So, I want to make clear what rules are applied
 and make it configurable, and then set it to a reasonable default
 which allow only DHCPv4 and IPv6 DAD.  This is the reason why I use
 rc.firewall.  And, rc.d scirpts must be as consistent as possible in
 terms of behaviors on start and stop.  "ipfw set" is easiest way to
 remove the minimal rule set upon stop.

 These implementation choices are not harmful or complex for normal
 users after all in my opinion.  Even if we need dynamically-added
 rules depending on configuration in the future, it can be supported
 easily in this framework.

sm> Can you explain why any layer2 rules are needed specifically? Apart from
sm> testing for dest mac ff:ff:ff:ff:ff:ff as well as 255.255.255.255/32 on
sm> the first rule, maybe overkill?, can't all these rules be run at layer3?

 L3 rule cannot catch the outgoing DHCPDISCOVER with the source
 address 0.0.0.0 and the destination address 255.255.255.255.  You can
 try this on your box.

sm> For one thing, if running L2 routing (and/or bridging), it's long been
sm> practice and advice to set net.link.ether.ipfw (&| net.link.bridge.ipfw)
sm> in /etc/sysctl.conf, and sysctl is run early .. on 9.3, first.
sm>
sm> This way you'll have to get people to update L2 configs to use this new
sm> firewall_link_enable variable as well or instead.  If L2 is really
sm> necessary, perhaps the previous setting of net.link.ether.ipfw could be
sm> remembered by ipfw0 and exported for rc.d/ipfw, seeing this is currently
sm> the only change to rc.d/ipfw, apart from clearing of the temporary
sm> rules - which I believe the subsequent flush makes unnecessary anyway.

 Ah, I see.  I agree with this.  I will update the patch.

-- Hiroki


pgpSZZEtXwpqD.pgp
Description: PGP signature