Download raw body.
ip forwarding ipsec read once
On Tue, Jul 02, 2024 at 04:42:43PM +0200, Alexander Bluhm wrote:
> Anyone?
>
ok mvs.
> I would like to implement the IPsec forwarding also for IPv6. Then
> the feature set is equivalent. But first IPv4 should be MP safe.
>
> bluhm
>
> On Wed, Jun 26, 2024 at 02:10:17PM +0200, Alexander Bluhm wrote:
> > 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