From 3ae7140690cee3d3454f977957f209a4b00b14a0 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Tue, 2 May 2023 12:12:44 -0700 Subject: [PATCH] net/tstun: handle exit nodes in NAT configs In the case where the exit node requires SNAT, we would SNAT all traffic not just the traffic meant to go through the exit node. This was a result of the default route being added to the routing table which would match basically everything. In this case, we need to account for all peers in the routing table not just the ones that require NAT. Fix and add a test. Updates tailscale/corp#8020 Signed-off-by: Maisem Ali --- net/tstun/wrap.go | 26 +++++++++++--- net/tstun/wrap_test.go | 81 +++++++++++++++++++++++++++--------------- 2 files changed, 74 insertions(+), 33 deletions(-) diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index 450885fbf..74bf54134 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -20,6 +20,7 @@ import ( "github.com/tailscale/wireguard-go/device" "github.com/tailscale/wireguard-go/tun" "go4.org/mem" + "golang.org/x/exp/slices" "gvisor.dev/gvisor/pkg/tcpip/stack" "tailscale.com/disco" "tailscale.com/net/connstats" @@ -590,16 +591,33 @@ func natConfigFromWGConfig(wcfg *wgcfg.Config) *natV4Config { dstMasqAddrs map[key.NodePublic]netip.Addr listenAddrs map[netip.Addr]struct{} ) + + // When using an exit node that requires masquerading, we need to + // fill out the routing table with all peers not just the ones that + // require masquerading. + exitNodeRequiresMasq := false // true if using an exit node and it requires masquerading + for _, p := range wcfg.Peers { + isExitNode := slices.Contains(p.AllowedIPs, tsaddr.AllIPv4()) || slices.Contains(p.AllowedIPs, tsaddr.AllIPv6()) + if isExitNode && p.V4MasqAddr != nil && p.V4MasqAddr.IsValid() { + exitNodeRequiresMasq = true + break + } + } for i := range wcfg.Peers { p := &wcfg.Peers[i] - if p.V4MasqAddr == nil || !p.V4MasqAddr.IsValid() { + var addrToUse netip.Addr + if p.V4MasqAddr != nil && p.V4MasqAddr.IsValid() { + addrToUse = *p.V4MasqAddr + mak.Set(&listenAddrs, addrToUse, struct{}{}) + } else if exitNodeRequiresMasq { + addrToUse = nativeAddr + } else { continue } rt.InsertOrReplace(p.PublicKey, p.AllowedIPs...) - mak.Set(&dstMasqAddrs, p.PublicKey, *p.V4MasqAddr) - mak.Set(&listenAddrs, *p.V4MasqAddr, struct{}{}) + mak.Set(&dstMasqAddrs, p.PublicKey, addrToUse) } - if len(listenAddrs) == 0 || len(dstMasqAddrs) == 0 { + if len(listenAddrs) == 0 && len(dstMasqAddrs) == 0 { return nil } return &natV4Config{ diff --git a/net/tstun/wrap_test.go b/net/tstun/wrap_test.go index f2ae0a614..f9e35beec 100644 --- a/net/tstun/wrap_test.go +++ b/net/tstun/wrap_test.go @@ -602,13 +602,13 @@ func TestFilterDiscoLoop(t *testing.T) { } func TestNATCfg(t *testing.T) { - node := func(ip, eip netip.Addr, otherAllowedIPs ...netip.Prefix) wgcfg.Peer { + node := func(ip, masqIP netip.Addr, otherAllowedIPs ...netip.Prefix) wgcfg.Peer { p := wgcfg.Peer{ PublicKey: key.NewNode().Public(), AllowedIPs: []netip.Prefix{ netip.PrefixFrom(ip, ip.BitLen()), }, - V4MasqAddr: ptr.To(eip), + V4MasqAddr: ptr.To(masqIP), } p.AllowedIPs = append(p.AllowedIPs, otherAllowedIPs...) return p @@ -619,13 +619,16 @@ func TestNATCfg(t *testing.T) { selfNativeIP = netip.MustParseAddr("100.64.0.1") selfEIP1 = netip.MustParseAddr("100.64.1.1") selfEIP2 = netip.MustParseAddr("100.64.1.2") + selfAddrs = []netip.Prefix{netip.PrefixFrom(selfNativeIP, selfNativeIP.BitLen())} peer1IP = netip.MustParseAddr("100.64.0.2") peer2IP = netip.MustParseAddr("100.64.0.3") - subnet = netip.MustParseAddr("192.168.0.1") + subnet = netip.MustParsePrefix("192.168.0.0/24") + subnetIP = netip.MustParseAddr("192.168.0.1") - selfAddrs = []netip.Prefix{netip.PrefixFrom(selfNativeIP, selfNativeIP.BitLen())} + exitRoute = netip.MustParsePrefix("0.0.0.0/0") + publicIP = netip.MustParseAddr("8.8.8.8") ) tests := []struct { @@ -638,9 +641,9 @@ func TestNATCfg(t *testing.T) { name: "no-cfg", wcfg: nil, snatMap: map[netip.Addr]netip.Addr{ - peer1IP: selfNativeIP, - peer2IP: selfNativeIP, - subnet: selfNativeIP, + peer1IP: selfNativeIP, + peer2IP: selfNativeIP, + subnetIP: selfNativeIP, }, dnatMap: map[netip.Addr]netip.Addr{ selfNativeIP: selfNativeIP, @@ -658,15 +661,15 @@ func TestNATCfg(t *testing.T) { }, }, snatMap: map[netip.Addr]netip.Addr{ - peer1IP: selfNativeIP, - peer2IP: selfEIP1, - subnet: selfNativeIP, + peer1IP: selfNativeIP, + peer2IP: selfEIP1, + subnetIP: selfNativeIP, }, dnatMap: map[netip.Addr]netip.Addr{ selfNativeIP: selfNativeIP, selfEIP1: selfNativeIP, selfEIP2: selfEIP2, - subnet: subnet, + subnetIP: subnetIP, }, }, { @@ -679,15 +682,15 @@ func TestNATCfg(t *testing.T) { }, }, snatMap: map[netip.Addr]netip.Addr{ - peer1IP: selfEIP1, - peer2IP: selfEIP2, - subnet: selfNativeIP, + peer1IP: selfEIP1, + peer2IP: selfEIP2, + subnetIP: selfNativeIP, }, dnatMap: map[netip.Addr]netip.Addr{ selfNativeIP: selfNativeIP, selfEIP1: selfNativeIP, selfEIP2: selfNativeIP, - subnet: subnet, + subnetIP: subnetIP, }, }, { @@ -696,19 +699,19 @@ func TestNATCfg(t *testing.T) { Addresses: selfAddrs, Peers: []wgcfg.Peer{ node(peer1IP, selfEIP1), - node(peer2IP, selfEIP2, netip.MustParsePrefix("192.168.0.0/24")), + node(peer2IP, selfEIP2, subnet), }, }, snatMap: map[netip.Addr]netip.Addr{ - peer1IP: selfEIP1, - peer2IP: selfEIP2, - subnet: selfEIP2, + peer1IP: selfEIP1, + peer2IP: selfEIP2, + subnetIP: selfEIP2, }, dnatMap: map[netip.Addr]netip.Addr{ selfNativeIP: selfNativeIP, selfEIP1: selfNativeIP, selfEIP2: selfNativeIP, - subnet: subnet, + subnetIP: subnetIP, }, }, { @@ -717,19 +720,19 @@ func TestNATCfg(t *testing.T) { Addresses: selfAddrs, Peers: []wgcfg.Peer{ node(peer1IP, selfEIP1), - node(peer2IP, selfEIP2, netip.MustParsePrefix("0.0.0.0/0")), + node(peer2IP, selfEIP2, exitRoute), }, }, snatMap: map[netip.Addr]netip.Addr{ - peer1IP: selfEIP1, - peer2IP: selfEIP2, - netip.MustParseAddr("8.8.8.8"): selfEIP2, + peer1IP: selfEIP1, + peer2IP: selfEIP2, + publicIP: selfEIP2, }, dnatMap: map[netip.Addr]netip.Addr{ selfNativeIP: selfNativeIP, selfEIP1: selfNativeIP, selfEIP2: selfNativeIP, - subnet: subnet, + subnetIP: subnetIP, }, }, { @@ -742,15 +745,35 @@ func TestNATCfg(t *testing.T) { }, }, snatMap: map[netip.Addr]netip.Addr{ - peer1IP: selfNativeIP, - peer2IP: selfNativeIP, - subnet: selfNativeIP, + peer1IP: selfNativeIP, + peer2IP: selfNativeIP, + subnetIP: selfNativeIP, }, dnatMap: map[netip.Addr]netip.Addr{ selfNativeIP: selfNativeIP, selfEIP1: selfEIP1, selfEIP2: selfEIP2, - subnet: subnet, + subnetIP: subnetIP, + }, + }, + { + name: "exit-node-require-nat-peer-doesnt", + wcfg: &wgcfg.Config{ + Addresses: selfAddrs, + Peers: []wgcfg.Peer{ + node(peer1IP, noIP), + node(peer2IP, selfEIP2, exitRoute), + }, + }, + snatMap: map[netip.Addr]netip.Addr{ + peer1IP: selfNativeIP, + peer2IP: selfEIP2, + publicIP: selfEIP2, + }, + dnatMap: map[netip.Addr]netip.Addr{ + selfNativeIP: selfNativeIP, + selfEIP2: selfNativeIP, + subnetIP: subnetIP, }, }, }