On Thu, Nov 14, 2013 at 4:27 AM, Alexander Bluhm
<[email protected]> wrote:
> On Fri, Oct 18, 2013 at 08:45:02PM +0200, Alexander Bluhm wrote:
>> Our IPv6 stack scans all extension headers for routing header type
>> 0 and drops the packet if it finds one. RFC 5095 demands to handle
>> a routing header type 0 like an unrecognised routing type. This
>> is enough to protect the own machine.
>>
>> To protect a network as a firewall, we have pf which does the same
>> full scan in pf_walk_header6(). As pf is enabled by default, nothing
>> changes for most users. If you turn off pf on your router, you
>> should not expect extra protection.
>>
>> I would like to get rid of the double scanning and the old disabled
>> code in the IPv6 stack.
>
> Theo and others don't like that change as it decreases security.
> There are hosts out there that still process RH0 and there are
> OpenBSD routers with pf disabled.
>
> This diff brings back the header chain scanning. As an improvement
> it only scans if pf has not done that before.
>
> Note that ip6_check_rh0hdr() can be easily tricked by hiding the
> routing header type 0 behind a fragment header. Only pf can protect
> you correctly as it reassembles on the forwarding path. So I am
> not sure wether it is worth adding it again.
>
> Comments?
I guess it would help people who decide to disable pf for slight
performance benefit ?
>
> bluhm
>
> Index: net/pf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.857
> diff -u -p -u -p -r1.857 pf.c
> --- net/pf.c 30 Oct 2013 11:35:10 -0000 1.857
> +++ net/pf.c 13 Nov 2013 23:14:32 -0000
> @@ -6490,6 +6490,7 @@ pf_test(sa_family_t af, int fwdir, struc
> }
> }
> pd.eh = eh;
> + pd.m->m_pkthdr.pf.flags |= PF_TAG_PROCESSED;
>
> switch (pd.virtual_proto) {
>
> Index: netinet6/ip6_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.120
> diff -u -p -u -p -r1.120 ip6_input.c
> --- netinet6/ip6_input.c 11 Nov 2013 09:15:35 -0000 1.120
> +++ netinet6/ip6_input.c 13 Nov 2013 23:38:22 -0000
> @@ -122,6 +122,7 @@ struct ifqueue ip6intrq;
> struct ip6stat ip6stat;
>
> void ip6_init2(void *);
> +int ip6_check_rh0hdr(struct mbuf *, int *);
>
> int ip6_hopopts_input(u_int32_t *, u_int32_t *, struct mbuf **, int *);
> struct mbuf *ip6_pullexthdr(struct mbuf *, size_t, int);
> @@ -331,6 +332,20 @@ ip6_input(struct mbuf *m)
> srcrt = !IN6_ARE_ADDR_EQUAL(&odst, &ip6->ip6_dst);
> #endif
>
> + /*
> + * Be more secure than RFC5095 and scan for type 0 routing headers.
> + * If pf has already scanned the header chain, do not do it twice.
> + */
> + if (!(m->m_pkthdr.pf.flags & PF_TAG_PROCESSED) &&
> + ip6_check_rh0hdr(m, &off)) {
> + ip6stat.ip6s_badoptions++;
> + in6_ifstat_inc(m->m_pkthdr.rcvif, ifs6_in_discard);
> + in6_ifstat_inc(m->m_pkthdr.rcvif, ifs6_in_hdrerr);
> + icmp6_error(m, ICMP6_PARAM_PROB, ICMP6_PARAMPROB_HEADER, off);
> + /* m is already freed */
> + return;
> + }
> +
> if (IN6_IS_ADDR_LOOPBACK(&ip6->ip6_src) ||
> IN6_IS_ADDR_LOOPBACK(&ip6->ip6_dst)) {
> ours = 1;
> @@ -698,6 +713,74 @@ ip6_input(struct mbuf *m)
> return;
> bad:
> m_freem(m);
> +}
> +
> +/* scan packet for RH0 routing header. Mostly stolen from pf.c:pf_test() */
> +int
> +ip6_check_rh0hdr(struct mbuf *m, int *offp)
> +{
> + struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
> + struct ip6_rthdr rthdr;
> + struct ip6_ext opt6;
> + u_int8_t proto = ip6->ip6_nxt;
> + int done = 0, lim, off, rh_cnt = 0;
> +
> + off = ((caddr_t)ip6 - m->m_data) + sizeof(struct ip6_hdr);
> + lim = min(m->m_pkthdr.len, ntohs(ip6->ip6_plen) + sizeof(*ip6));
> + do {
> + switch (proto) {
> + case IPPROTO_ROUTING:
> + *offp = off;
> + if (rh_cnt++) {
> + /* more then one rh header present */
> + return (1);
> + }
> +
> + if (off + sizeof(rthdr) > lim) {
> + /* packet to short to make sense */
> + return (1);
> + }
> +
> + m_copydata(m, off, sizeof(rthdr), (caddr_t)&rthdr);
> +
> + if (rthdr.ip6r_type == IPV6_RTHDR_TYPE_0) {
> + *offp += offsetof(struct ip6_rthdr,
> ip6r_type);
> + return (1);
> + }
> +
> + off += (rthdr.ip6r_len + 1) * 8;
> + proto = rthdr.ip6r_nxt;
> + break;
> + case IPPROTO_AH:
> + case IPPROTO_HOPOPTS:
> + case IPPROTO_DSTOPTS:
> + /* get next header and header length */
> + if (off + sizeof(opt6) > lim) {
> + /*
> + * Packet to short to make sense, we could
> + * reject the packet but as a router we
> + * should not do that so forward it.
> + */
> + return (0);
> + }
> +
> + m_copydata(m, off, sizeof(opt6), (caddr_t)&opt6);
> +
> + if (proto == IPPROTO_AH)
> + off += (opt6.ip6e_len + 2) * 4;
> + else
> + off += (opt6.ip6e_len + 1) * 8;
> + proto = opt6.ip6e_nxt;
> + break;
> + case IPPROTO_FRAGMENT:
> + default:
> + /* end of header stack */
> + done = 1;
> + break;
> + }
> + } while (!done);
> +
> + return (0);
> }
>
> /*
> Index: sys/mbuf.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v
> retrieving revision 1.168
> diff -u -p -u -p -r1.168 mbuf.h
> --- sys/mbuf.h 13 Oct 2013 10:10:04 -0000 1.168
> +++ sys/mbuf.h 13 Nov 2013 23:14:32 -0000
> @@ -100,6 +100,7 @@ struct pkthdr_pf {
> #define PF_TAG_DIVERTED_PACKET 0x10
> #define PF_TAG_REROUTE 0x20
> #define PF_TAG_REFRAGMENTED 0x40 /* refragmented ipv6
> packet */
> +#define PF_TAG_PROCESSED 0x80 /* packet was checked
> by pf */
>
> /* record/packet header in first mbuf of chain; valid if M_PKTHDR set */
> struct pkthdr {
>
--
This message is strictly personal and the opinions expressed do not
represent those of my employers, either past or present.
