From 84b2c3433f4f9b1f6e25c30c222a7e3ce369fcfb Mon Sep 17 00:00:00 2001 From: bluhm Date: Fri, 7 Jun 2024 18:24:16 +0000 Subject: [PATCH] Read IP forwarding variables only once. Do not assume that ip_forwarding and ip_directedbcast cannot change while processing one packet. Read it once and pass down its value with a flag. This is necessary for unlocking the sysctl path. There are a few places where a consistent value does not really matter, they are unchanged. Use a proper ip_ prefix for the global variable. OK claudio@ --- sys/net/pf.c | 11 ++++++-- sys/netinet/ip_icmp.c | 4 +-- sys/netinet/ip_input.c | 56 +++++++++++++++++++++----------------- sys/netinet/ip_output.c | 4 +-- sys/netinet/ip_var.h | 14 ++++++---- sys/netinet6/ip6_forward.c | 4 +-- 6 files changed, 53 insertions(+), 40 deletions(-) diff --git a/sys/net/pf.c b/sys/net/pf.c index 8591b044e43..ecc6bfef43c 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf.c,v 1.1196 2024/05/14 08:26:13 jsg Exp $ */ +/* $OpenBSD: pf.c,v 1.1197 2024/06/07 18:24:16 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -7958,12 +7958,17 @@ done: switch (pd.naf) { case AF_INET: if (pd.dir == PF_IN) { - if (ipforwarding == 0) { + int flags; + + if (ip_forwarding == 0) { ipstat_inc(ips_cantforward); action = PF_DROP; break; } - ip_forward(pd.m, ifp, NULL, 1); + flags = IP_FORWARDING | IP_REDIRECT; + if (ip_directedbcast) + SET(flags, IP_ALLOWBROADCAST); + ip_forward(pd.m, ifp, NULL, flags); } else ip_output(pd.m, NULL, NULL, 0, NULL, NULL, 0); break; diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c index 30bec63a512..2d42796dc47 100644 --- a/sys/netinet/ip_icmp.c +++ b/sys/netinet/ip_icmp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_icmp.c,v 1.192 2023/09/16 09:33:27 mpi Exp $ */ +/* $OpenBSD: ip_icmp.c,v 1.193 2024/06/07 18:24:16 bluhm Exp $ */ /* $NetBSD: ip_icmp.c,v 1.19 1996/02/13 23:42:22 christos Exp $ */ /* @@ -589,7 +589,7 @@ reflect: struct sockaddr_in ssrc; struct rtentry *newrt = NULL; - if (icmp_rediraccept == 0 || ipforwarding == 1) + if (icmp_rediraccept == 0 || ip_forwarding == 1) goto freeit; if (code > 3) goto badcode; diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index ad0455b50f1..38057fceb7c 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_input.c,v 1.394 2024/05/08 13:01:30 bluhm Exp $ */ +/* $OpenBSD: ip_input.c,v 1.395 2024/06/07 18:24:16 bluhm Exp $ */ /* $NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $ */ /* @@ -84,10 +84,10 @@ #endif /* values controllable via sysctl */ -int ipforwarding = 0; +int ip_forwarding = 0; int ipmforwarding = 0; int ipmultipath = 0; -int ipsendredirects = 1; +int ip_sendredirects = 1; int ip_dosourceroute = 0; int ip_defttl = IPDEFTTL; int ip_mtudisc = 1; @@ -108,8 +108,8 @@ const struct sysctl_bounded_args ipctl_vars[] = { #ifdef MROUTING { IPCTL_MRTPROTO, &ip_mrtproto, SYSCTL_INT_READONLY }, #endif - { IPCTL_FORWARDING, &ipforwarding, 0, 2 }, - { IPCTL_SENDREDIRECTS, &ipsendredirects, 0, 1 }, + { IPCTL_FORWARDING, &ip_forwarding, 0, 2 }, + { IPCTL_SENDREDIRECTS, &ip_sendredirects, 0, 1 }, { IPCTL_DEFTTL, &ip_defttl, 0, 255 }, { IPCTL_DIRECTEDBCAST, &ip_directedbcast, 0, 1 }, { IPCTL_IPPORT_FIRSTAUTO, &ipport_firstauto, 0, 65535 }, @@ -137,8 +137,8 @@ static struct mbuf_queue ipsendraw_mq; extern struct niqueue arpinq; int ip_ours(struct mbuf **, int *, int, int); -int ip_dooptions(struct mbuf *, struct ifnet *); -int in_ouraddr(struct mbuf *, struct ifnet *, struct route *); +int ip_dooptions(struct mbuf *, struct ifnet *, int); +int in_ouraddr(struct mbuf *, struct ifnet *, struct route *, int); int ip_fragcheck(struct mbuf **, int *); struct mbuf * ip_reass(struct ipqent *, struct ipq *); @@ -431,7 +431,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp) #if NPF > 0 struct in_addr odst; #endif - int pfrdr = 0; + int flags = 0; KASSERT(*offp == 0); @@ -461,9 +461,15 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp) goto bad; ip = mtod(m, struct ip *); - pfrdr = odst.s_addr != ip->ip_dst.s_addr; + if (odst.s_addr != ip->ip_dst.s_addr) + SET(flags, IP_REDIRECT); #endif + if (ip_forwarding != 0) + SET(flags, IP_FORWARDING); + if (ip_directedbcast) + SET(flags, IP_ALLOWBROADCAST); + hlen = ip->ip_hl << 2; /* @@ -472,7 +478,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp) * error was detected (causing an icmp message * to be sent and the original packet to be freed). */ - if (hlen > sizeof (struct ip) && ip_dooptions(m, ifp)) { + if (hlen > sizeof (struct ip) && ip_dooptions(m, ifp, flags)) { m = *mp = NULL; goto bad; } @@ -483,7 +489,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp) goto out; } - switch(in_ouraddr(m, ifp, &ro)) { + switch(in_ouraddr(m, ifp, &ro, flags)) { case 2: goto bad; case 1: @@ -565,7 +571,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp) /* * Not for us; forward if possible and desirable. */ - if (ipforwarding == 0) { + if (!ISSET(flags, IP_FORWARDING)) { ipstat_inc(ips_cantforward); goto bad; } @@ -585,7 +591,7 @@ ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp) } #endif /* IPSEC */ - ip_forward(m, ifp, &ro, pfrdr); + ip_forward(m, ifp, &ro, flags); *mp = NULL; rtfree(ro.ro_rt); return IPPROTO_DONE; @@ -807,7 +813,7 @@ ip_deliver(struct mbuf **mp, int *offp, int nxt, int af, int shared) #undef IPSTAT_INC int -in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro) +in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro, int flags) { struct rtentry *rt; struct ip *ip; @@ -837,7 +843,8 @@ in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro) * if it is received on the interface with that address. */ if (ISSET(rt->rt_flags, RTF_BROADCAST) && - (!ip_directedbcast || rt->rt_ifidx == ifp->if_index)) { + (!ISSET(flags, IP_ALLOWBROADCAST) || + rt->rt_ifidx == ifp->if_index)) { match = 1; /* Make sure M_BCAST is set */ @@ -876,7 +883,8 @@ in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro) break; } } - } else if (ipforwarding == 0 && rt->rt_ifidx != ifp->if_index && + } else if (!ISSET(flags, IP_FORWARDING) && + rt->rt_ifidx != ifp->if_index && !((ifp->if_flags & IFF_LOOPBACK) || (ifp->if_type == IFT_ENC) || (m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST))) { /* received on wrong interface. */ @@ -1150,7 +1158,7 @@ ip_flush(void) * 0 if the packet should be processed further. */ int -ip_dooptions(struct mbuf *m, struct ifnet *ifp) +ip_dooptions(struct mbuf *m, struct ifnet *ifp, int flags) { struct ip *ip = mtod(m, struct ip *); unsigned int rtableid = m->m_pkthdr.ph_rtableid; @@ -1371,8 +1379,8 @@ ip_dooptions(struct mbuf *m, struct ifnet *ifp) } } KERNEL_UNLOCK(); - if (forward && ipforwarding > 0) { - ip_forward(m, ifp, NULL, 1); + if (forward && ISSET(flags, IP_FORWARDING)) { + ip_forward(m, ifp, NULL, flags | IP_REDIRECT); return (1); } return (0); @@ -1514,7 +1522,7 @@ const u_char inetctlerrmap[PRC_NCMDS] = { * of codes and types. * * If not forwarding, just drop the packet. This could be confusing - * if ipforwarding was zero but some routing protocol was advancing + * if ip_forwarding was zero but some routing protocol was advancing * us as a gateway to somewhere. However, we must let the routing * protocol deal with that. * @@ -1522,7 +1530,7 @@ const u_char inetctlerrmap[PRC_NCMDS] = { * via a source route. */ void -ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int srcrt) +ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int flags) { struct mbuf mfake, *mcopy; struct ip *ip = mtod(m, struct ip *); @@ -1588,7 +1596,7 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int srcrt) if ((rt->rt_ifidx == ifp->if_index) && (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0 && satosin(rt_key(rt))->sin_addr.s_addr != 0 && - ipsendredirects && !srcrt && + ip_sendredirects && !ISSET(flags, IP_REDIRECT) && !arpproxy(satosin(rt_key(rt))->sin_addr, m->m_pkthdr.ph_rtableid)) { if ((ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_netmask) == ifatoia(rt->rt_ifa)->ia_net) { @@ -1602,9 +1610,7 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int srcrt) } } - error = ip_output(m, NULL, ro, - (IP_FORWARDING | (ip_directedbcast ? IP_ALLOWBROADCAST : 0)), - NULL, NULL, 0); + error = ip_output(m, NULL, ro, flags | IP_FORWARDING, NULL, NULL, 0); rt = ro->ro_rt; if (error) ipstat_inc(ips_cantforward); diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c index c0aeb76930d..b21683a11f3 100644 --- a/sys/netinet/ip_output.c +++ b/sys/netinet/ip_output.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_output.c,v 1.399 2024/05/16 13:01:04 bluhm Exp $ */ +/* $OpenBSD: ip_output.c,v 1.400 2024/06/07 18:24:16 bluhm Exp $ */ /* $NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $ */ /* @@ -428,7 +428,7 @@ sendit: #endif #ifdef IPSEC - if ((flags & IP_FORWARDING) && ipforwarding == 2 && + if ((flags & IP_FORWARDING) && ip_forwarding == 2 && (!ipsec_in_use || m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL) == NULL)) { error = EHOSTUNREACH; diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h index a963798db34..d4e2163446c 100644 --- a/sys/netinet/ip_var.h +++ b/sys/netinet/ip_var.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_var.h,v 1.117 2024/04/17 20:48:51 bluhm Exp $ */ +/* $OpenBSD: ip_var.h,v 1.118 2024/06/07 18:24:16 bluhm Exp $ */ /* $NetBSD: ip_var.h,v 1.16 1996/02/13 23:43:20 christos Exp $ */ /* @@ -204,10 +204,11 @@ struct ipoffnxt { }; /* flags passed to ip_output */ -#define IP_FORWARDING 0x1 /* most of ip header exists */ -#define IP_RAWOUTPUT 0x2 /* raw ip header exists */ -#define IP_ALLOWBROADCAST SO_BROADCAST /* can send broadcast packets */ -#define IP_MTUDISC 0x0800 /* pmtu discovery, set DF */ +#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_ALLOWBROADCAST SO_BROADCAST /* can send broadcast packets */ +#define IP_MTUDISC 0x0800 /* pmtu discovery, set DF */ extern struct ipstat ipstat; extern int ip_defttl; /* default IP ttl */ @@ -221,11 +222,12 @@ extern int ipport_firstauto; /* min port for port allocation */ extern int ipport_lastauto; /* max port for port allocation */ extern int ipport_hifirstauto; /* min dynamic/private port number */ extern int ipport_hilastauto; /* max dynamic/private port number */ -extern int ipforwarding; /* enable IP forwarding */ +extern int ip_forwarding; /* enable IP forwarding */ #ifdef MROUTING extern int ipmforwarding; /* enable multicast forwarding */ #endif extern int ipmultipath; /* enable multipath routing */ +extern int ip_directedbcast; /* accept all broadcast packets */ extern unsigned int la_hold_total; extern const struct pr_usrreqs rip_usrreqs; diff --git a/sys/netinet6/ip6_forward.c b/sys/netinet6/ip6_forward.c index 913fb64304c..13e60375492 100644 --- a/sys/netinet6/ip6_forward.c +++ b/sys/netinet6/ip6_forward.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip6_forward.c,v 1.117 2024/04/16 12:56:39 bluhm Exp $ */ +/* $OpenBSD: ip6_forward.c,v 1.118 2024/06/07 18:24:16 bluhm Exp $ */ /* $KAME: ip6_forward.c,v 1.75 2001/06/29 12:42:13 jinmei Exp $ */ /* @@ -75,7 +75,7 @@ * of codes and types. * * If not forwarding, just drop the packet. This could be confusing - * if ipforwarding was zero but some routing protocol was advancing + * if ip6_forwarding was zero but some routing protocol was advancing * us as a gateway to somewhere. However, we must let the routing * protocol deal with that. * -- 2.20.1