Download raw body.
ip forwarding ipsec read once
Hi,
Claudio mentioned that there is a MP race between reading ip_forwarding
in ip_input() and checking ip_forwarding == 2 in ip_output().
In theory ip_forwarding could be 2 during ip_input() and later 0
in ip_output(). Then a packet would be forwarded that was never
allowed.
Currently exclusive netlock in sysctl(2) prevents all the races.
We could introduce IP_FORWARDING_IPSEC and pass it with the flags
parameter we have introduced for IP_FORWARDING.
Instead of calling m_tag_find(), traversing the list, and comparing
with NULL, we could just check PACKET_TAG_IPSEC_IN_DONE bit. Reading
ipsec_in_use in ip_output() is a performance hack that is not
necessary. New code only checks tree bits.
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
diff -u -p -r1.1199 pf.c
--- net/pf.c 21 Jun 2024 12:51:29 -0000 1.1199
+++ net/pf.c 25 Jun 2024 10:58:17 -0000
@@ -7958,20 +7958,26 @@ done:
case PF_AFRT:
if (pf_translate_af(&pd)) {
action = PF_DROP;
- break;
+ goto out;
}
pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
switch (pd.naf) {
case AF_INET:
if (pd.dir == PF_IN) {
- int flags;
+ int flags = IP_REDIRECT;
- if (ip_forwarding == 0) {
+ switch (ip_forwarding) {
+ case 2:
+ SET(flags, IP_FORWARDING_IPSEC);
+ /* FALLTHROUGH */
+ case 1:
+ SET(flags, IP_FORWARDING);
+ break;
+ default:
ipstat_inc(ips_cantforward);
action = PF_DROP;
- break;
+ goto out;
}
- flags = IP_FORWARDING | IP_REDIRECT;
if (ip_directedbcast)
SET(flags, IP_ALLOWBROADCAST);
ip_forward(pd.m, ifp, NULL, flags);
@@ -7985,7 +7991,7 @@ done:
if (ip6_forwarding == 0) {
ip6stat_inc(ip6s_cantforward);
action = PF_DROP;
- break;
+ goto out;
}
flags = IPV6_FORWARDING | IPV6_REDIRECT;
ip6_forward(pd.m, NULL, flags);
@@ -7993,10 +7999,8 @@ done:
ip6_output(pd.m, NULL, NULL, 0, NULL, NULL);
break;
}
- if (action != PF_DROP) {
- pd.m = NULL;
- action = PF_PASS;
- }
+ pd.m = NULL;
+ action = PF_PASS;
break;
#endif /* INET6 */
case PF_DROP:
@@ -8036,6 +8040,7 @@ done:
st->if_index_out = ifp->if_index;
}
+ out:
*m0 = pd.m;
pf_state_unref(st);
Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
diff -u -p -r1.396 ip_input.c
--- netinet/ip_input.c 24 Jun 2024 12:19:19 -0000 1.396
+++ netinet/ip_input.c 25 Jun 2024 11:00:54 -0000
@@ -465,8 +465,14 @@ ip_input_if(struct mbuf **mp, int *offp,
SET(flags, IP_REDIRECT);
#endif
- if (ip_forwarding != 0)
+ switch (ip_forwarding) {
+ case 2:
+ SET(flags, IP_FORWARDING_IPSEC);
+ /* FALLTHROUGH */
+ case 1:
SET(flags, IP_FORWARDING);
+ break;
+ }
if (ip_directedbcast)
SET(flags, IP_ALLOWBROADCAST);
@@ -529,7 +535,7 @@ ip_input_if(struct mbuf **mp, int *offp,
* ip_output().)
*/
KERNEL_LOCK();
- error = ip_mforward(m, ifp);
+ error = ip_mforward(m, ifp, flags);
KERNEL_UNLOCK();
if (error) {
ipstat_inc(ips_cantforward);
Index: netinet/ip_mroute.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_mroute.c,v
diff -u -p -r1.142 ip_mroute.c
--- netinet/ip_mroute.c 6 Apr 2024 14:23:27 -0000 1.142
+++ netinet/ip_mroute.c 25 Jun 2024 10:58:17 -0000
@@ -122,7 +122,7 @@ int get_api_support(struct mbuf *);
int get_api_config(struct mbuf *);
int socket_send(struct socket *, struct mbuf *,
struct sockaddr_in *);
-int ip_mdq(struct mbuf *, struct ifnet *, struct rtentry *);
+int ip_mdq(struct mbuf *, struct ifnet *, struct rtentry *, int);
struct ifnet *if_lookupbyvif(vifi_t, unsigned int);
struct rtentry *rt_mcast_add(struct ifnet *, struct sockaddr *,
struct sockaddr *);
@@ -1080,7 +1080,7 @@ socket_send(struct socket *so, struct mb
#define TUNNEL_LEN 12 /* # bytes of IP option for tunnel encapsulation */
int
-ip_mforward(struct mbuf *m, struct ifnet *ifp)
+ip_mforward(struct mbuf *m, struct ifnet *ifp, int flags)
{
struct ip *ip = mtod(m, struct ip *);
struct vif *v;
@@ -1121,7 +1121,7 @@ ip_mforward(struct mbuf *m, struct ifnet
/* Entry exists, so forward if necessary */
if (rt != NULL) {
- return (ip_mdq(m, ifp, rt));
+ return (ip_mdq(m, ifp, rt, flags));
} else {
/*
* If we don't have a route for packet's origin,
@@ -1183,7 +1183,7 @@ ip_mforward(struct mbuf *m, struct ifnet
* Packet forwarding routine once entry in the cache is made
*/
int
-ip_mdq(struct mbuf *m, struct ifnet *ifp0, struct rtentry *rt)
+ip_mdq(struct mbuf *m, struct ifnet *ifp0, struct rtentry *rt, int flags)
{
struct ip *ip = mtod(m, struct ip *);
struct mfc *mfc = (struct mfc *)rt->rt_llinfo;
@@ -1281,7 +1281,7 @@ ip_mdq(struct mbuf *m, struct ifnet *ifp
imo.imo_ttl = ip->ip_ttl - IPTTLDEC;
imo.imo_loop = 1;
- ip_output(mc, NULL, NULL, IP_FORWARDING, &imo, NULL, 0);
+ ip_output(mc, NULL, NULL, flags | IP_FORWARDING, &imo, NULL, 0);
if_put(ifp);
} while ((rt = rtable_iterate(rt)) != NULL);
Index: netinet/ip_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
diff -u -p -r1.400 ip_output.c
--- netinet/ip_output.c 7 Jun 2024 18:24:16 -0000 1.400
+++ netinet/ip_output.c 25 Jun 2024 10:58:17 -0000
@@ -331,7 +331,7 @@ reroute:
int rv;
KERNEL_LOCK();
- rv = ip_mforward(m, ifp);
+ rv = ip_mforward(m, ifp, flags);
KERNEL_UNLOCK();
if (rv != 0)
goto bad;
@@ -428,9 +428,8 @@ sendit:
#endif
#ifdef IPSEC
- if ((flags & IP_FORWARDING) && ip_forwarding == 2 &&
- (!ipsec_in_use ||
- m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL) == NULL)) {
+ if (ISSET(flags, IP_FORWARDING) && ISSET(flags, IP_FORWARDING_IPSEC) &&
+ !ISSET(m->m_pkthdr.ph_tagsset, PACKET_TAG_IPSEC_IN_DONE)) {
error = EHOSTUNREACH;
goto bad;
}
Index: netinet/ip_var.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
diff -u -p -r1.118 ip_var.h
--- netinet/ip_var.h 7 Jun 2024 18:24:16 -0000 1.118
+++ netinet/ip_var.h 25 Jun 2024 10:58:17 -0000
@@ -207,6 +207,7 @@ struct ipoffnxt {
#define IP_FORWARDING 0x0001 /* most of ip header exists */
#define IP_RAWOUTPUT 0x0002 /* raw ip header exists */
#define IP_REDIRECT 0x0004 /* redirected by pf or source route */
+#define IP_FORWARDING_IPSEC 0x0008 /* only packets processed by IPsec */
#define IP_ALLOWBROADCAST SO_BROADCAST /* can send broadcast packets */
#define IP_MTUDISC 0x0800 /* pmtu discovery, set DF */
@@ -246,7 +247,7 @@ int ip_getmoptions(int, struct ip_mopti
void ip_init(void);
struct mbuf*
ip_insertoptions(struct mbuf *, struct mbuf *, int *);
-int ip_mforward(struct mbuf *, struct ifnet *);
+int ip_mforward(struct mbuf *, struct ifnet *, int);
int ip_optcopy(struct ip *, struct ip *);
int ip_output(struct mbuf *, struct mbuf *, struct route *, int,
struct ip_moptions *, const struct ipsec_level *, u_int32_t);
ip forwarding ipsec read once