From mike@belopuhov.com Mon Jan 19 10:13:22 2015 Delivered-To: david@gwynne.id.au Received: by 10.216.49.9 with SMTP id w9csp54816web; Sun, 18 Jan 2015 16:13:25 -0800 (PST) X-Received: by 10.66.62.137 with SMTP id y9mr39548097par.87.1421626404291; Sun, 18 Jan 2015 16:13:24 -0800 (PST) Return-Path: Received: from mail.vantronix.net (mail.vantronix.net. [93.159.253.124]) by mx.google.com with ESMTPS id nw8si13626229pbb.161.2015.01.18.16.13.21 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 18 Jan 2015 16:13:22 -0800 (PST) Received-SPF: pass (google.com: domain of mike@belopuhov.com designates 93.159.253.124 as permitted sender) client-ip=93.159.253.124; Authentication-Results: mx.google.com; spf=pass (google.com: domain of mike@belopuhov.com designates 93.159.253.124 as permitted sender) smtp.mail=mike@belopuhov.com Received: by mail.vantronix.net (Postfix, from userid 1000) id 07B3C4C806E; Mon, 19 Jan 2015 01:08:01 +0100 (CET) Date: Mon, 19 Jan 2015 01:08:00 +0100 From: Mike Belopuhov To: David Gwynne Cc: hackers@openbsd.org Subject: Re: enable pfctl to print scoped v6 addresses Message-ID: <20150119000800.GB32054@dali.vantronix.net> References: <20141224084406.GJ265@animata.net> <20141224111644.GA3177@dali.vantronix.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20141224111644.GA3177@dali.vantronix.net> User-Agent: Mutt/1.5.22 (2013-10-16) Status: RO Content-Length: 9817 Lines: 270 On Wed, Dec 24, 2014 at 12:16 +0100, Mike Belopuhov wrote: > On Wed, Dec 24, 2014 at 18:44 +1000, David Gwynne wrote: > > i had a very frustrating afternoon yesterday when i discovered that we are now passing link local v6 addresses to pf with the interface index embedded in the address, but pf isnt unpacking them. > > > > this is frustrating because i have a default deny ruleset with very > > explicit allows for some ipv6 traffic. in particular: > > > > table persist { self } > > lladdr6="fe80::/10" > > > > pass in quick on vlan363 inet6 proto ospf to { $lladdr6 ff02::5 ff02::6 } keep state (no-sync) queue hi > > pass in quick on vlan364 inet6 proto ospf to { $lladdr6 ff02::5 ff02::6 } keep state (no-sync) queue hi > > > > pf doesnt match traffic to ff02::5 or ff01::6 because it see the following: > > > > 15:59:53.524761 rule 37/(match) [uid 0, pid 7507] block in on vlan363: fe80:3a::21d:71ff:fe9a:a840 > ff02:3a::5: OSPFv3-hello 36: rtrid 172.21.130.64 area 0.0.0.2 [|ospf] [class 0xe0] [hlim 1] (len 36) > > 16:00:02.647307 rule 37/(match) [uid 0, pid 7507] block in on vlan364: fe80:3b::7ada:6eff:fe25:cc00 > ff02:3b::5: OSPFv3-hello 36: rtrid 172.21.130.65 area 0.0.0.2 [|ospf] [class 0xe0] [hlim 1] (len 36) > > > > Unfortunately "to ff02::5" opens up too much in a generic case (when > "on iface" is not specified) and policy wise you want it to be as > concise as possible, e.g. this really should be ff02::5%vlan363 and > so on (please read below about a proposed syntax sugar to alleviate > the need to specify scope explicitly). > > > while i utterly loathe how the ifidx is encoded here, there are > > very good reasons to let pf see the ifidx and we should make it > > work better. part of that way down the line will be having pfctl > > properly handle interface indexes, which means we'll want to print > > them. > > > > most addresses in pfctl are displayed using inet_ntop, which just > > prints ips. getnameinfo is better in that it understands interface > > indexes and prints them correctly. however, pf addresses (pf_addr) > > are more like raw ips though, not the sockaddrs that getnameinfo > > expects, so ive made a wrapper around getnameinfo that translates > > the raw bits of an ip into the appropriate sockaddr. > > > > my previous change to src/sys/net/pfvar.h is necessary for this to > > work. > > > > ok? > > > > > I believe you should use IN6_IS_SCOPE_EMBED, not IN6_IS_ADDR_LINKLOCAL > since there are multicast addresses that are scoped as well. Also I > think you should not embed scope ids and instead use %iface notation > like other tools do. parse.y can parse the percentage specification > (e.g. ff02::1%vlan3) correctly due to the getaddrinfo call in the > host_v6. You should use internal ifa_indextoname not libc function > to do the conversion for performance reasons if you go this route. > > For loading you must make sure that ifindex (or scope) notion gets > properly conveyed to the kernel. > > Syntax sugar wise we can add an if to the expand_rule to add scope > of the 'interface' to the unsoped destination address if it satisfies > IN6_IS_SCOPE_EMBED test since we're not supposed to forward such > packets. This will enable this rule: > > pass in on vlan3 to ff02::5 > > to be silently converted to > > pass in on vlan3 to ff02::5%vlan3 > > without change to the policy. > > Beware of the "on rdomain" specification (interface->use_rdomain) > though. can you and anyone interested please give this diff a try. it allows some interesting things, for example i have two link local addresses fe81::1/64 and fe81::2/64 set up on carp0. now i can do things like: % echo "pass in on carp0 to { fe81::1 fe81::2 }" | pfctl -vnf - pass in on carp0 inet6 from any to fe81::1%carp0 flags S/SA pass in on carp0 inet6 from any to fe81::2%carp0 flags S/SA here, pfctl figures out the scope of fe81::1 and fe81::2 by looking at the interface. however scope can also be specified explicitely, for instance when "on interface" wasn't given: % echo "pass in to fe81::/64" | pfctl -vnf - stdin:1: address requires valid scope stdin:1: skipping rule due to errors stdin:1: rule expands to no valid combination pfctl: Syntax error in config file: pf rules not loaded % echo "pass in to fe81::%carp0/64" | pfctl -vnf - pass in on carp0 inet6 from any to fe81::%carp0/64 flags S/SA fe81::%carp0/64 might look funny but host() and friends parse it this way just fine. i'm not finished with this diff since i want to try to stuff gen_dyn* in the expand_address as well, but this is unrelated to the problem at hand. does this help your setup, david? Index: parse.y =================================================================== RCS file: /home/cvs/src/sbin/pfctl/parse.y,v retrieving revision 1.644 diff -u -p -r1.644 parse.y --- parse.y 16 Jan 2015 06:40:00 -0000 1.644 +++ parse.y 18 Jan 2015 23:29:00 -0000 @@ -329,6 +329,8 @@ int disallow_urpf_failed(struct node_h int disallow_alias(struct node_host *, const char *); int rule_consistent(struct pf_rule *, int); int process_tabledef(char *, struct table_opts *); +int expand_address(struct pf_rule_addr *, const char *, + sa_family_t, struct node_host *, struct node_port *); void expand_label_str(char *, size_t, const char *, const char *); void expand_label_if(const char *, char *, size_t, const char *); void expand_label_addr(const char *, char *, size_t, u_int8_t, @@ -4309,6 +4311,46 @@ expand_queue(char *qname, struct node_if } int +expand_address(struct pf_rule_addr *ra, const char *ifname, sa_family_t af, + struct node_host *h, struct node_port *p) +{ + struct pf_addr *a; + u_int32_t ifindex = 0; + + ra->addr = h->addr; + a = &ra->addr.v.a.addr; + + /* Send scope information to the kernel */ + if (af == AF_INET6 && h->addr.type == PF_ADDR_ADDRMASK && + (IN6_IS_ADDR_LINKLOCAL(&a->v6) || + IN6_IS_ADDR_MC_LINKLOCAL(&a->v6) || + IN6_IS_ADDR_MC_INTFACELOCAL(&a->v6))) { + if (h->ifindex) + ifindex = h->ifindex; + else if (strlen(ifname) > 0) { + if ((ifindex = ifa_nametoindex(ifname)) == 0 && + errno == ENXIO) { + yyerror("interface %s not found", + ifname); + return (1); + } + } else { + yyerror("address requires valid scope"); + return (1); + } + a->v6.s6_addr[2] = ifindex >> 8; + a->v6.s6_addr[3] = ifindex & 0xff; + } + + ra->neg = h->not; + ra->port[0] = p->port[0]; + ra->port[1] = p->port[1]; + ra->port_op = p->op; + + return (0); +} + +int collapse_redirspec(struct pf_pool *rpool, struct pf_rule *r, struct redirspec *rs, u_int8_t allow_if) { @@ -4628,6 +4669,11 @@ expand_rule(struct pf_rule *r, int keepr r->af, src_host, src_port, dst_host, dst_port, proto->proto); + error += expand_address(&r->src, r->ifname, r->af, src_host, + src_port); + error += expand_address(&r->dst, r->ifname, r->af, dst_host, + dst_port); + osrch = odsth = NULL; if (src_host->addr.type == PF_ADDR_DYNIFTL) { osrch = src_host; @@ -4645,16 +4691,6 @@ expand_rule(struct pf_rule *r, int keepr r->ifnot = interface->not; r->proto = proto->proto; - r->src.addr = src_host->addr; - r->src.neg = src_host->not; - r->src.port[0] = src_port->port[0]; - r->src.port[1] = src_port->port[1]; - r->src.port_op = src_port->op; - r->dst.addr = dst_host->addr; - r->dst.neg = dst_host->not; - r->dst.port[0] = dst_port->port[0]; - r->dst.port[1] = dst_port->port[1]; - r->dst.port_op = dst_port->op; r->uid.op = uid->op; r->uid.uid[0] = uid->uid[0]; r->uid.uid[1] = uid->uid[1]; Index: pf_print_state.c =================================================================== RCS file: /home/cvs/src/sbin/pfctl/pf_print_state.c,v retrieving revision 1.63 diff -u -p -r1.63 pf_print_state.c --- pf_print_state.c 17 Aug 2012 20:37:16 -0000 1.63 +++ pf_print_state.c 18 Jan 2015 20:54:18 -0000 @@ -97,13 +97,39 @@ print_addr(struct pf_addr_wrap *addr, sa PF_AZERO(&addr->v.a.mask, AF_INET6)) printf("any"); else { + char ifname[IFNAMSIZ]; char buf[48]; + struct pf_addr *a = &addr->v.a.addr; + u_int32_t ifindex = 0; + /* special handling for ipv6 scoped addresses */ + if (af == AF_INET6 && (IN6_IS_ADDR_LINKLOCAL(&a->v6) || + IN6_IS_ADDR_MC_LINKLOCAL(&a->v6) || + IN6_IS_ADDR_MC_INTFACELOCAL(&a->v6))) { + ifindex = a->v6.s6_addr[2] << 8 | + a->v6.s6_addr[3]; + /* + * let inet_ntop print embedded scope id if we + * fail to resolve the interface name + */ + if (ifa_indextoname(ifindex, ifname) == NULL) { + ifindex = 0; + goto ntop; + } + a->v6.s6_addr[2] = 0; + a->v6.s6_addr[3] = 0; + } + ntop: if (inet_ntop(af, &addr->v.a.addr, buf, sizeof(buf)) == NULL) printf("?"); else printf("%s", buf); + if (ifindex) { + printf("%%%s", ifname); + a->v6.s6_addr[2] = ifindex >> 8; + a->v6.s6_addr[3] = ifindex & 0xff; + } } break; case PF_ADDR_NOROUTE: Index: pfctl_parser.c =================================================================== RCS file: /home/cvs/src/sbin/pfctl/pfctl_parser.c,v retrieving revision 1.301 diff -u -p -r1.301 pfctl_parser.c --- pfctl_parser.c 16 Jan 2015 06:40:00 -0000 1.301 +++ pfctl_parser.c 18 Jan 2015 23:16:44 -0000 @@ -1381,6 +1381,9 @@ ifa_nametoindex(const char *ifa_name) { struct node_host *p; + if (iftab == NULL) + ifa_load(); + for (p = iftab; p; p = p->next) { if (p->af == AF_LINK && strcmp(p->ifname, ifa_name) == 0) return (p->ifindex); @@ -1394,6 +1397,9 @@ ifa_indextoname(unsigned int ifindex, ch { struct node_host *p; + if (iftab == NULL) + ifa_load(); + for (p = iftab; p; p = p->next) { if (p->af == AF_LINK && ifindex == p->ifindex) { strlcpy(ifa_name, p->ifname, IFNAMSIZ);