From b6fc02d501bfa7c023ca30e4501a06d7ab6a4f1e Mon Sep 17 00:00:00 2001 From: Mazdak Nasab Date: Fri, 30 Oct 2026 08:55:05 -0700 Subject: [PATCH] feature/conn25: block private destinations (#22073) Similar to 7304dd4 for host-scoped destinations, conn25 needs to block private destinations: a connector forwards to app destinations, and the packet filter only ever sees the transit IP, never the destination, so a private destination would let a client reach into the connector's own network. Updates https://github.com/tailscale/corp/issues/50118 Updates https://github.com/tailscale/corp/issues/46169 Change-Id: I700b0b5a270723f5a77907e35c52d729f88c44b6 Signed-off-by: Mazdak Nasab --- feature/conn25/conn25.go | 4 +-- feature/conn25/conn25_test.go | 61 +++++++++++++++++++++++++++++--- feature/conn25/connector.go | 5 +++ feature/conn25/connector_test.go | 10 +++--- 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index 44c5035a8..0efce8c42 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -710,8 +710,8 @@ const ( TransitIPNotInPool = 7 // DisallowedDestination indicates that the destination IP address is not - // allowed (including but not limited to loopback, Tailscale, and link-local - // addresses). + // allowed (including but not limited to loopback, Tailscale, link-local, + // and private addresses). DisallowedDestination = 8 ) diff --git a/feature/conn25/conn25_test.go b/feature/conn25/conn25_test.go index 928446fd9..3402ca2eb 100644 --- a/feature/conn25/conn25_test.go +++ b/feature/conn25/conn25_test.go @@ -114,10 +114,11 @@ func TestHandleConnectorTransitIPRequest(t *testing.T) { // tip(v4, 1) written in its IPv4-in-IPv6 form; the same address as tip(v4, 1). tipV4_1In6 := netip.MustParseAddr("::ffff:169.254.0.1") - // Destination IPs - // v4 10.0.0.0 - 10.0.0.255 - // v6 fc00:: - fc00::ff - dip := ipGenerator("dip", netip.MustParseAddr("10.0.0.0"), netip.MustParseAddr("fc00::")) + // Destination IPs. These have to be globally routable: conn25 rejects + // private destinations. + // v4 203.0.113.0 - 203.0.113.255 + // v6 2001:db8:dd:: - 2001:db8:dd::ff + dip := ipGenerator("dip", netip.MustParseAddr("203.0.113.0"), netip.MustParseAddr("2001:db8:dd::")) // Peer nodes peerV4V6 := (&tailcfg.Node{ @@ -392,6 +393,58 @@ func TestHandleConnectorTransitIPRequest(t *testing.T) { }, }, }, + // Single peer, private destinations + { + name: "one-peer-private-dip", + ctipReqPeers: []tailcfg.NodeView{peerV4V6}, + ctipReqs: []ConnectorTransitIPRequest{ + {TransitIPs: []TransitIPRequest{ + {TransitIP: tip(v4, 1), DestinationIP: netip.MustParseAddr("10.1.2.3"), App: appName}, + {TransitIP: tip(v4, 2), DestinationIP: netip.MustParseAddr("172.16.0.9"), App: appName}, + {TransitIP: tip(v4, 3), DestinationIP: netip.MustParseAddr("172.31.255.254"), App: appName}, + {TransitIP: tip(v4, 4), DestinationIP: netip.MustParseAddr("192.168.1.1"), App: appName}, + {TransitIP: tip(v6, 1), DestinationIP: netip.MustParseAddr("fc00::1"), App: appName}, + {TransitIP: tip(v6, 2), DestinationIP: netip.MustParseAddr("fd12:3456:789a::1"), App: appName}, + }}, + }, + wants: []ConnectorTransitIPResponse{ + {TransitIPs: slices.Repeat([]TransitIPResponse{{Code: DisallowedDestination, Message: disallowedDestinationMessage}}, 6)}, + }, + wantLookups: [][][]netip.Addr{ + { + {pip(v4, 1), tip(v4, 1), netip.Addr{}}, + {pip(v4, 1), tip(v4, 2), netip.Addr{}}, + {pip(v4, 1), tip(v4, 3), netip.Addr{}}, + {pip(v4, 1), tip(v4, 4), netip.Addr{}}, + {pip(v6, 1), tip(v6, 1), netip.Addr{}}, + {pip(v6, 1), tip(v6, 2), netip.Addr{}}, + }, + }, + }, + // Single peer, destinations just outside the private ranges are allowed + { + name: "one-peer-near-private-dip", + ctipReqPeers: []tailcfg.NodeView{peerV4V6}, + ctipReqs: []ConnectorTransitIPRequest{ + {TransitIPs: []TransitIPRequest{ + {TransitIP: tip(v4, 1), DestinationIP: netip.MustParseAddr("172.15.255.254"), App: appName}, + {TransitIP: tip(v4, 2), DestinationIP: netip.MustParseAddr("172.32.0.1"), App: appName}, + {TransitIP: tip(v6, 1), DestinationIP: netip.MustParseAddr("fb00::1"), App: appName}, + {TransitIP: tip(v6, 2), DestinationIP: netip.MustParseAddr("fe00::1"), App: appName}, + }}, + }, + wants: []ConnectorTransitIPResponse{ + {TransitIPs: slices.Repeat([]TransitIPResponse{{Code: OK, Message: ""}}, 4)}, + }, + wantLookups: [][][]netip.Addr{ + { + {pip(v4, 1), tip(v4, 1), netip.MustParseAddr("172.15.255.254")}, + {pip(v4, 1), tip(v4, 2), netip.MustParseAddr("172.32.0.1")}, + {pip(v6, 1), tip(v6, 1), netip.MustParseAddr("fb00::1")}, + {pip(v6, 1), tip(v6, 2), netip.MustParseAddr("fe00::1")}, + }, + }, + }, // Single peer, invalid app name { name: "one-peer-invalid-app", diff --git a/feature/conn25/connector.go b/feature/conn25/connector.go index 410eaedcd..cd3ef2b74 100644 --- a/feature/conn25/connector.go +++ b/feature/conn25/connector.go @@ -34,6 +34,11 @@ func (c *connector) handleTransitIPRequest(n tailcfg.NodeView, peerV4 netip.Addr return TransitIPResponse{Code: DisallowedDestination, Message: disallowedDestinationMessage} } + // TODO(tailscale/corp#50542): remove this condition when we have a configurable check + if tipr.DestinationIP.IsPrivate() { + return TransitIPResponse{Code: DisallowedDestination, Message: disallowedDestinationMessage} + } + // The transit address has to come from a transit IP pool we're configured with. if !c.transitIPInPool(tipr.TransitIP) { c.logf("[Unexpected] peer attempt to map a transit IP outside of the configured pools: node: %s, IP: %v", diff --git a/feature/conn25/connector_test.go b/feature/conn25/connector_test.go index 97968b6cf..efea1d618 100644 --- a/feature/conn25/connector_test.go +++ b/feature/conn25/connector_test.go @@ -53,14 +53,14 @@ func TestConnectorExpireTransitIPs(t *testing.T) { }) } - register(peerANode, tipOne, netip.MustParseAddr("10.0.0.1")) - register(peerANode, tipFour, netip.MustParseAddr("10.0.0.4")) - register(peerBNode, tipThree, netip.MustParseAddr("10.0.0.3")) + register(peerANode, tipOne, netip.MustParseAddr("203.0.113.1")) + register(peerANode, tipFour, netip.MustParseAddr("203.0.113.4")) + register(peerBNode, tipThree, netip.MustParseAddr("203.0.113.3")) clock.Advance(30 * time.Minute) - register(peerANode, tipTwo, netip.MustParseAddr("10.0.0.2")) + register(peerANode, tipTwo, netip.MustParseAddr("203.0.113.2")) // write over the tipFour mapping, with a different dst - register(peerANode, tipFour, netip.MustParseAddr("10.0.0.5")) + register(peerANode, tipFour, netip.MustParseAddr("203.0.113.5")) // advance past the hour expiry time of peerA+tipOne and peerB+tipThree // the peerA+tipTwo and the updated peerA+tipFour registrations are still within expiry