On Thu, Nov 14, 2013 at 05:38:14PM -0700, Theo de Raadt wrote:
> Beautiful.
I seems there was enough discussion. The Security argument is more
important than the others. The new diff has no performance impact
when pf is turned on.
So I need OKs.
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 {
