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?
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 {
