[Xorp-users] [Fwd: [PATCH] PF: Allow IP Router Alert option by default]

Pavlin Radoslavov pavlin at ICSI.Berkeley.EDU
Thu Oct 9 12:39:55 PDT 2008


Stegen Smith <stegen at owns.com> wrote:

> Hi Pavlin and Bruce,
> 
> I just wanted to let the list know that I updated xorp from cvs and  all is
> working out great.  Thanks again for all the help and quick  turnaroun.  Great
> stuff!

Stegen, thanks for the update on the subject.

Bruce, it was a good call on your side when you pushed that change
to FreeBSD. However, it appears that in OpenBSD the IP Router Alert
options are explicitly disabled by default. Given that you have
done the FreeBSD implementation nits and details, can you raise the
issue with the OpenBSD folks so they also don't disable the RAs in
their stack. You have my full support on that.
Disabled RAs breaks IGMP on both sides (router and host), and
debugging multicast-related issues is hard enough even without this
obstacle.

Pavlin

> stegen
> On Oct 9, 2008, at 5:51 AM, Bruce M. Simpson wrote:
> 
> > I actually submitted a patch to dhartmei@ to allow the RA option  filter to
> > be disabled by default, as this is almost always what  bites us with PF on
> > BSD systems.
> >
> > Forwarding here to pick up in archives.
> >
> > thanks
> > BMS
> >
> > From: Bruce M Simpson <bms at incunabulum.net>
> > Date: May 4, 2008 8:38:14 PM PDT
> > To: dhartmei at freebsd.org, Max Laier <max at love2party.net>, Pavlin  Radoslavov
> > <pavlin at icir.org>
> > Subject: [PATCH] PF: Allow IP Router Alert option by default
> >
> >
> > I've tested this in a virtual machine, against FreeBSD 6.3-RELEASE,  and it
> > looks fine.
> >
> > I'd like to commit it as soon as possible if there are no objections.
> >
> >   Performance impact should be negligible, though I haven't  measured. The
> > pf_pull_hdr() call is however required as you know,  the mbuf chain might
> > not be pulled up.
> >
> >   The match is a loose match, although the RA option must come right  after
> > the header. This is the norm for the implementations I've  seen. The value
> > of the option isn't checked, I believe hardware  router boxes don't either.
> >
> >   I deliberately rolled this to respect the existing values of
> > "allow_opts", so that existing pfsync installations will continue to  work;
> > nodes which don't understand the PF_ALLOWOPTS_RA flag will  treat it as
> > "allow all options" however. Caveat user: should really  upgrade redundant
> > firewalls in tandem.
> >
> >   The syntax originally suggested ("allow-opts(RA)") doesn't work,  because
> > the parser treats '(' and ')' as special tokens which can't  be part of
> > other tokens, so I added a keyword "allow-ra-opt".
> >
> > cheers
> > BMS
> > --- contrib/pf/pfctl/parse.y.orig	2005-05-03 17:55:20.000000000 +0100
> > +++ contrib/pf/pfctl/parse.y	2008-05-05 03:15:05.000000000 +0100
> > @@ -405,7 +405,8 @@
> > %token	PASS BLOCK SCRUB RETURN IN OS OUT LOG LOGALL QUICK ON FROM TO
> > FLAGS
> > %token	RETURNRST RETURNICMP RETURNICMP6 PROTO INET INET6 ALL ANY
> > ICMPTYPE
> > %token	ICMP6TYPE CODE KEEP MODULATE STATE PORT RDR NAT BINAT ARROW
> > NODF
> > -%token	MINTTL ERROR ALLOWOPTS FASTROUTE FILENAME ROUTETO DUPTO
> > REPLYTO NO LABEL
> > +%token	MINTTL ERROR ALLOWOPTS ALLOWOPTSRA
> > +%token	FASTROUTE FILENAME ROUTETO DUPTO REPLYTO NO LABEL
> > %token	NOROUTE FRAGMENT USER GROUP MAXMSS MAXIMUM TTL TOS DROP TABLE
> > %token	REASSEMBLE FRAGDROP FRAGCROP ANCHOR NATANCHOR RDRANCHOR
> > BINATANCHOR
> > %token	SET OPTIMIZATION TIMEOUT LIMIT LOGINTERFACE BLOCKPOLICY
> > RANDOMID
> > @@ -1902,7 +1903,10 @@
> > 			filter_opts.fragment = 1;
> > 		}
> > 		| ALLOWOPTS {
> > -			filter_opts.allowopts = 1;
> > +			filter_opts.allowopts = PF_ALLOWOPTS_ALL;
> > +		}
> > +		| ALLOWOPTSRA {
> > +			filter_opts.allowopts |= PF_ALLOWOPTS_RA;
> > 		}
> > 		| label	{
> > 			if (filter_opts.label) {
> > @@ -4577,6 +4581,7 @@
> > 	static const struct keywords keywords[] = {
> > 		{ "all",		ALL},
> > 		{ "allow-opts",		ALLOWOPTS},
> > +		{ "allow-ra-opt",		ALLOWOPTSRA},
> > 		{ "altq",		ALTQ},
> > 		{ "anchor",		ANCHOR},
> > 		{ "antispoof",		ANTISPOOF},
> > --- contrib/pf/pfctl/pfctl_parser.c.orig	2005-05-03  17:55:20.000000000
> > +0100
> > +++ contrib/pf/pfctl/pfctl_parser.c	2008-05-05 03:16:29.000000000  +0100
> > @@ -928,8 +928,10 @@
> > 		printf(" min-ttl %d", r->min_ttl);
> > 	if (r->max_mss)
> > 		printf(" max-mss %d", r->max_mss);
> > -	if (r->allow_opts)
> > +	if (r->allow_opts & PF_ALLOWOPTS_ALL)
> > 		printf(" allow-opts");
> > +	else if (r->allow_opts & PF_ALLOWOPTS_RA)
> > +		printf(" allow-ra-opt");
> > 	if (r->action == PF_SCRUB) {
> > 		if (r->rule_flag & PFRULE_REASSEMBLE_TCP)
> > 			printf(" reassemble tcp");
> > --- sys/contrib/pf/net/pf.c.orig	2008-05-05 04:12:00.000000000 +0100
> > +++ sys/contrib/pf/net/pf.c	2008-05-05 04:24:48.000000000 +0100
> > @@ -6681,13 +6681,35 @@
> > 	}
> >
> > done:
> > -	if (action == PF_PASS && h->ip_hl > 5 &&
> > -	    !((s && s->allow_opts) || r->allow_opts)) {
> > -		action = PF_DROP;
> > -		REASON_SET(&reason, PFRES_IPOPTIONS);
> > -		log = 1;
> > -		DPFPRINTF(PF_DEBUG_MISC,
> > -		    ("pf: dropping packet with ip options\n"));
> > +	if (action == PF_PASS && h->ip_hl > 5) {
> > +		do {
> > +			/* If allow-opts is set, allow any IP option. */
> > +			if ((s && (s->allow_opts & PF_ALLOWOPTS_ALL)) ||
> > +			    (r->allow_opts & PF_ALLOWOPTS_ALL))
> > +				break;
> > +			/*
> > +			 * If allow-ra-opt is set, allow only the
> > +			 * IP Router Alert option; it must be the first
> > +			 * IP option, and its value is ignored.
> > +			 */
> > +			if ((s && (s->allow_opts & PF_ALLOWOPTS_RA)) ||
> > +			    (r->allow_opts & PF_ALLOWOPTS_RA)) {
> > +				uint8_t opt[4];
> > +				if (pf_pull_hdr(m, sizeof(struct ip),
> > +						&opt[0], sizeof(opt),
> > +						&action, &reason,
> > +						AF_INET) != NULL) {
> > +					if (opt[0] == IPOPT_RA &&
> > +					    opt[1] == 4)
> > +						break;
> > +				}
> > +			}
> > +			action = PF_DROP;
> > +			REASON_SET(&reason, PFRES_IPOPTIONS);
> > +			log = 1;
> > +			DPFPRINTF(PF_DEBUG_MISC,
> > +			    ("pf: dropping packet with ip options\n"));
> > +		} while (0);
> > 	}
> >
> > 	if (s && s->tag)
> > --- sys/contrib/pf/net/pf_ioctl.c.orig	2007-01-04 18:31:43.000000000
> > +0000
> > +++ sys/contrib/pf/net/pf_ioctl.c	2008-05-05 04:23:02.000000000 +0100
> > @@ -320,6 +320,7 @@
> > 	/* default rule should never be garbage collected */
> > 	pf_default_rule.entries.tqe_prev = &pf_default_rule.entries.tqe_next;
> > 	pf_default_rule.action = PF_PASS;
> > +	pf_default_rule.allow_opts = PF_ALLOWOPTS_RA;
> > 	pf_default_rule.nr = -1;
> >
> > 	/* initialize default timeouts */
> > @@ -391,6 +392,7 @@
> > 	/* default rule should never be garbage collected */
> > 	pf_default_rule.entries.tqe_prev = &pf_default_rule.entries.tqe_next;
> > 	pf_default_rule.action = PF_PASS;
> > +	pf_default_rule.allow_opts = PF_ALLOWOPTS_RA;
> > 	pf_default_rule.nr = -1;
> >
> > 	/* initialize default timeouts */
> > --- sys/contrib/pf/net/pfvar.h.orig	2005-12-30 00:50:18.000000000  +0000
> > +++ sys/contrib/pf/net/pfvar.h	2008-05-05 03:05:42.000000000 +0100
> > @@ -651,6 +651,9 @@
> > 	u_int8_t		 flagset;
> > 	u_int8_t		 min_ttl;
> > 	u_int8_t		 allow_opts;
> > +#define PF_ALLOWOPTS_ALL	0x1
> > +#define PF_ALLOWOPTS_RA		0x2			/* IP Router Alert */
> > +#define PF_ALLOWOPTS_MASK	0x3
> > 	u_int8_t		 rt;
> > 	u_int8_t		 return_ttl;
> > 	u_int8_t		 tos;
> >
> >
> 
> _______________________________________________
> Xorp-users mailing list
> Xorp-users at xorp.org
> http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-users



More information about the Xorp-users mailing list