From 8c0a0450d911ff9b0f2ffa5ffd8e73d6abf0366c Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 17 Mar 2021 17:04:32 -0700 Subject: [PATCH] ipn/ipnlocal: allow client access to exit node's public IPs. "public IP" is defined as an IP address configured on the exit node itself that isn't in the list of forbidden ranges (RFC1918, CGNAT, Tailscale). Fixes #1522. Signed-off-by: David Anderson --- ipn/ipnlocal/local.go | 21 +++++++++++++++++++++ ipn/ipnlocal/local_test.go | 32 +++++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index c488a6e95..0d8ef1582 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -720,10 +720,16 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs) netaddr.MustParseIPPrefix("192.168.0.0/16"), netaddr.MustParseIPPrefix("172.16.0.0/12"), netaddr.MustParseIPPrefix("10.0.0.0/8"), + // IPv4 link-local + netaddr.MustParseIPPrefix("169.254.0.0/16"), + // IPv4 multicast + netaddr.MustParseIPPrefix("224.0.0.0/4"), // Tailscale IPv4 range tsaddr.CGNATRange(), // IPv6 Link-local addresses netaddr.MustParseIPPrefix("fe80::/10"), + // IPv6 multicast + netaddr.MustParseIPPrefix("ff00::/8"), // Tailscale IPv6 range tsaddr.TailscaleULARange(), } @@ -733,6 +739,7 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs) func shrinkDefaultRoute(route netaddr.IPPrefix) (*netaddr.IPSet, error) { var b netaddr.IPSetBuilder b.AddPrefix(route) + var hostIPs []netaddr.IP err := interfaces.ForeachInterfaceAddress(func(_ interfaces.Interface, pfx netaddr.IPPrefix) { if tsaddr.IsTailscaleIP(pfx.IP) { return @@ -740,11 +747,25 @@ func shrinkDefaultRoute(route netaddr.IPPrefix) (*netaddr.IPSet, error) { if pfx.IsSingleIP() { return } + hostIPs = append(hostIPs, pfx.IP) b.RemovePrefix(pfx) }) if err != nil { return nil, err } + + // Having removed all the LAN subnets, re-add the hosts's own + // IPs. It's fine for clients to connect to an exit node's public + // IP address, just not the attached subnet. + // + // Truly forbidden subnets (in removeFromDefaultRoute) will still + // be stripped back out by the next step. + for _, ip := range hostIPs { + if route.Contains(ip) { + b.Add(ip) + } + } + for _, pfx := range removeFromDefaultRoute { b.RemovePrefix(pfx) } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 667cc2287..2a6fc9315 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -9,6 +9,7 @@ "testing" "inet.af/netaddr" + "tailscale.com/net/interfaces" "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "tailscale.com/types/netmap" @@ -122,11 +123,21 @@ func TestNetworkMapCompare(t *testing.T) { } } +func inRemove(ip netaddr.IP) bool { + for _, pfx := range removeFromDefaultRoute { + if pfx.Contains(ip) { + return true + } + } + return false +} + func TestShrinkDefaultRoute(t *testing.T) { tests := []struct { - route string - in []string - out []string + route string + in []string + out []string + localIPFn func(netaddr.IP) bool // true if this machine's local IP address should be "in" after shrinking. }{ { route: "0.0.0.0/0", @@ -139,19 +150,24 @@ func TestShrinkDefaultRoute(t *testing.T) { "172.16.0.1", "172.31.255.255", "100.101.102.103", + "224.0.0.1", + "169.254.169.254", // Some random IPv6 stuff that shouldn't be in a v4 // default route. "fe80::", "2601::1", }, + localIPFn: func(ip netaddr.IP) bool { return !inRemove(ip) && ip.Is4() }, }, { route: "::/0", in: []string{"::1", "2601::1"}, out: []string{ "fe80::1", + "ff00::1", tsaddr.TailscaleULARange().IP.String(), }, + localIPFn: func(ip netaddr.IP) bool { return !inRemove(ip) && ip.Is6() }, }, } @@ -171,6 +187,16 @@ func TestShrinkDefaultRoute(t *testing.T) { t.Errorf("shrink(%q).Contains(%v) = true, want false", test.route, ip) } } + ips, _, err := interfaces.LocalAddresses() + if err != nil { + t.Fatal(err) + } + for _, ip := range ips { + want := test.localIPFn(ip) + if gotContains := got.Contains(ip); gotContains != want { + t.Errorf("shrink(%q).Contains(%v) = %v, want %v", test.route, ip, gotContains, want) + } + } } }