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

Stegen Smith stegen at owns.com
Thu Oct 9 10:02:59 PDT 2008


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
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;
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.ICSI.Berkeley.EDU/pipermail/xorp-users/attachments/20081009/4dc97231/attachment-0001.html 


More information about the Xorp-users mailing list