Hi,
IPsec packets are passed through ip_input() a second time after
they have been decrypted. That means that all the IP header fields
are checked twice. Also fragment reassembly is tried twice.
In pf incoming packets in tunnel mode appear twice on the enc0
interface. Once as IP-in-IP and once as the inner packet. In the
outgoing path we only see the inner packet. Asymmetry is bad for
stateful filtering.
IPv6 shows that IPsec works without that. After decrypting we can
continue with local delivery. If we are in tunnel mode, the IP-in-IP
protocol functions do what we want. In transport mode only pf_test()
has to be called for the enc0 device.
Introducing ip_local() means less needless processing and cleaner
pf behavior.
ok?
bluhm
Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.299
diff -u -p -r1.299 ip_input.c
--- netinet/ip_input.c 11 May 2017 11:36:20 -0000 1.299
+++ netinet/ip_input.c 12 May 2017 12:34:42 -0000
@@ -483,9 +483,6 @@ ip_ours(struct mbuf *m)
hlen = ip->ip_hl << 2;
- /* pf might have modified stuff, might have to chksum */
- in_proto_cksum_out(m, NULL);
-
/*
* If offset or IP_MF are set, must reassemble.
* Otherwise, nothing need be done.
@@ -570,11 +567,26 @@ found:
ip_freef(fp);
}
+ ip_local(m, hlen, ip->ip_p);
+ return;
+bad:
+ m_freem(m);
+}
+
+void
+ip_local(struct mbuf *m, int off, int nxt)
+{
+ KERNEL_ASSERT_LOCKED();
+
+ /* pf might have modified stuff, might have to chksum */
+ in_proto_cksum_out(m, NULL);
+
#ifdef IPSEC
if (ipsec_in_use) {
- if (ip_input_ipsec_ours_check(m, hlen) != 0) {
+ if (ip_input_ipsec_ours_check(m, off) != 0) {
ipstat_inc(ips_cantforward);
- goto bad;
+ m_freem(m);
+ return;
}
}
/* Otherwise, just fall through and deliver the packet */
@@ -584,10 +596,7 @@ found:
* Switch out to protocol's input routine.
*/
ipstat_inc(ips_delivered);
- (*inetsw[ip_protox[ip->ip_p]].pr_input)(&m, &hlen, ip->ip_p, AF_INET);
- return;
-bad:
- m_freem(m);
+ (*inetsw[ip_protox[nxt]].pr_input)(&m, &off, nxt, AF_INET);
}
int
Index: netinet/ip_var.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.71
diff -u -p -r1.71 ip_var.h
--- netinet/ip_var.h 14 Apr 2017 20:46:31 -0000 1.71
+++ netinet/ip_var.h 12 May 2017 12:34:42 -0000
@@ -249,6 +249,7 @@ void ip_savecontrol(struct inpcb *, str
struct mbuf *);
void ipintr(void);
void ipv4_input(struct mbuf *);
+void ip_local(struct mbuf *, int, int);
void ip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
int rip_ctloutput(int, struct socket *, int, int, struct mbuf *);
void rip_init(void);
Index: netinet/ipsec_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.149
diff -u -p -r1.149 ipsec_input.c
--- netinet/ipsec_input.c 11 May 2017 12:14:43 -0000 1.149
+++ netinet/ipsec_input.c 12 May 2017 12:51:51 -0000
@@ -579,44 +579,38 @@ ipsec_common_input_cb(struct mbuf *m, st
return;
}
+#if NPF > 0
+ /*
+ * The ip_local() shortcut avoids running through ip_input() with the
+ * same IP header twice. Packets in transport mode have to be be
+ * passed to pf explicitly. In tunnel mode the inner IP header will
+ * run through ip_input() and pf anyway.
+ */
+ if ((tdbp->tdb_flags & TDBF_TUNNELING) == 0) {
+ struct ifnet *ifp;
+
+ /* This is the enc0 interface unless for ipcomp. */
+ if ((ifp = if_get(m->m_pkthdr.ph_ifidx)) == NULL) {
+ m_freem(m);
+ return;
+ }
+ if (pf_test(af, PF_IN, ifp, &m) != PF_PASS) {
+ if_put(ifp);
+ m_freem(m);
+ return;
+ }
+ if_put(ifp);
+ if (m == NULL)
+ return;
+ }
+#endif
/* Call the appropriate IPsec transform callback. */
switch (af) {
case AF_INET:
- if (niq_enqueue(&ipintrq, m) != 0) {
- DPRINTF(("ipsec_common_input_cb(): dropped packet "
- "because of full IP queue\n"));
- IPSEC_ISTAT(espstat.esps_qfull, ahstat.ahs_qfull,
- ipcompstat.ipcomps_qfull);
- }
+ ip_local(m, skip, prot);
return;
#ifdef INET6
case AF_INET6:
-#if NPF > 0
- /*
- * In the IPv4 case all packets run through ipv4_input()
- * twice. As the ip6_local() shortcut avoids this, IPv6
- * packets in transport mode have to be be passed to pf
- * explicitly. In tunnel mode the inner IP header will
- * run through ip_input() and pf already.
- */
- if ((tdbp->tdb_flags & TDBF_TUNNELING) == 0) {
- struct ifnet *ifp;
-
- /* This is the enc0 interface unless for ipcomp. */
- if ((ifp = if_get(m->m_pkthdr.ph_ifidx)) == NULL) {
- m_freem(m);
- return;
- }
- if (pf_test(AF_INET6, PF_IN, ifp, &m) != PF_PASS) {
- if_put(ifp);
- m_freem(m);
- return;
- }
- if_put(ifp);
- if (m == NULL)
- return;
- }
-#endif
ip6_local(m, skip, prot);
return;
#endif /* INET6 */
