Pf was handling IPv4 and IPv6 differently regarding AH extension

headers.  pf_walk_header6() steps over it and detects the real
protocol.  So to implement a minimal header walking function
pf_walk_header() for IPv4.  It does the header checks and jumps
over AH.  Then pf does not understand AH as a protocol, it is just
an extension that authenticates the packet.  Move some header and
option checks to pf_walk_header() for consistency with IPv6.  This
also improves the header check for IPv4 packets in ICMP payload.
OK henning@

1 files changed, 52 insertions, 18 deletions

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 2ad769c4d4a..243c0be10db 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.1027 2017/05/23 09:09:03 bluhm Exp $ */
+/*	$OpenBSD: pf.c,v 1.1028 2017/05/28 14:54:00 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -222,6 +222,8 @@ u_int16_t		 pf_calc_mss(struct pf_addr *, sa_family_t, int,
 static __inline int	 pf_set_rt_ifp(struct pf_state *, struct pf_addr *,
 			    sa_family_t);
 struct pf_divert	*pf_get_divert(struct mbuf *);
+int			 pf_walk_header(struct pf_pdesc *, struct ip *,
+			    u_short *);
 int			 pf_walk_option6(struct pf_pdesc *, struct ip6_hdr *,
 			    int, int, u_short *);
 int			 pf_walk_header6(struct pf_pdesc *, struct ip6_hdr *,
@@ -4986,9 +4988,10 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state,
 			}
 
 			/* offset of protocol header that follows h2 */
-			pd2.off = ipoff2 + (h2.ip_hl << 2);
+			pd2.off = ipoff2;
+			if (pf_walk_header(&pd2, &h2, reason) != PF_PASS)
+				return (PF_DROP);
 
-			pd2.proto = h2.ip_p;
 			pd2.tot_len = ntohs(h2.ip_len);
 			pd2.src = (struct pf_addr *)&h2.ip_src;
 			pd2.dst = (struct pf_addr *)&h2.ip_dst;
@@ -6109,6 +6112,44 @@ pf_get_divert(struct mbuf *m)
 	return ((struct pf_divert *)(mtag + 1));
 }
 
+int
+pf_walk_header(struct pf_pdesc *pd, struct ip *h, u_short *reason)
+{
+	struct ip6_ext		 ext;
+	u_int32_t		 hlen, end;
+
+	hlen = h->ip_hl << 2;
+	if (hlen < sizeof(struct ip) || hlen > ntohs(h->ip_len)) {
+		REASON_SET(reason, PFRES_SHORT);
+		return (PF_DROP);
+	}
+	end = pd->off + ntohs(h->ip_len);
+	pd->off += hlen;
+	pd->proto = h->ip_p;
+	/* stop walking over non initial fragments */
+	if ((h->ip_off & htons(IP_OFFMASK)) != 0)
+		return (PF_PASS);
+	for (;;) {
+		switch (pd->proto) {
+		case IPPROTO_AH:
+			/* fragments may be short */
+			if ((h->ip_off & htons(IP_MF | IP_OFFMASK)) != 0 &&
+			    end < pd->off + sizeof(ext))
+				return (PF_PASS);
+			if (!pf_pull_hdr(pd->m, pd->off, &ext, sizeof(ext),
+			    NULL, reason, AF_INET)) {
+				DPFPRINTF(LOG_NOTICE, "IP short exthdr");
+				return (PF_DROP);
+			}
+			pd->off += (ext.ip6e_len + 2) * 4;
+			pd->proto = ext.ip6e_nxt;
+			break;
+		default:
+			return (PF_PASS);
+		}
+	}
+}
+
 #ifdef INET6
 int
 pf_walk_option6(struct pf_pdesc *pd, struct ip6_hdr *h, int off, int end,
@@ -6326,26 +6367,23 @@ pf_setup_pdesc(struct pf_pdesc *pd, sa_family_t af, int dir,
 		}
 
 		h = mtod(pd->m, struct ip *);
-		pd->off = h->ip_hl << 2;
-
-		if (pd->off < sizeof(struct ip) ||
-		    pd->off > ntohs(h->ip_len) ||
-		    pd->m->m_pkthdr.len < ntohs(h->ip_len)) {
+		if (pd->m->m_pkthdr.len < ntohs(h->ip_len)) {
 			REASON_SET(reason, PFRES_SHORT);
 			return (PF_DROP);
 		}
 
+		if (pf_walk_header(pd, h, reason) != PF_PASS)
+			return (PF_DROP);
+
 		pd->src = (struct pf_addr *)&h->ip_src;
 		pd->dst = (struct pf_addr *)&h->ip_dst;
-		pd->virtual_proto = pd->proto = h->ip_p;
 		pd->tot_len = ntohs(h->ip_len);
 		pd->tos = h->ip_tos & ~IPTOS_ECN_MASK;
 		pd->ttl = h->ip_ttl;
 		if (h->ip_hl > 5)	/* has options */
 			pd->badopts++;
-
-		if (h->ip_off & htons(IP_MF | IP_OFFMASK))
-			pd->virtual_proto = PF_VPROTO_FRAGMENT;
+		pd->virtual_proto = (h->ip_off & htons(IP_MF | IP_OFFMASK)) ?
+		     PF_VPROTO_FRAGMENT : pd->proto;
 
 		break;
 	}
@@ -6360,8 +6398,6 @@ pf_setup_pdesc(struct pf_pdesc *pd, sa_family_t af, int dir,
 		}
 
 		h = mtod(pd->m, struct ip6_hdr *);
-		pd->off = 0;
-
 		if (pd->m->m_pkthdr.len <
 		    sizeof(struct ip6_hdr) + ntohs(h->ip6_plen)) {
 			REASON_SET(reason, PFRES_SHORT);
@@ -6384,13 +6420,11 @@ pf_setup_pdesc(struct pf_pdesc *pd, sa_family_t af, int dir,
 
 		pd->src = (struct pf_addr *)&h->ip6_src;
 		pd->dst = (struct pf_addr *)&h->ip6_dst;
-		pd->virtual_proto = pd->proto;
 		pd->tot_len = ntohs(h->ip6_plen) + sizeof(struct ip6_hdr);
 		pd->tos = (ntohl(h->ip6_flow) & 0x0fc00000) >> 20;
 		pd->ttl = h->ip6_hlim;
-
-		if (pd->fragoff != 0)
-			pd->virtual_proto = PF_VPROTO_FRAGMENT;
+		pd->virtual_proto = (pd->fragoff != 0) ?
+			PF_VPROTO_FRAGMENT : pd->proto;
 
 		break;
 	}