[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