diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index 9e59b0abd..9ec65a7cd 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -549,6 +549,7 @@ const noMatchingPeerIPFamilyMessage = "No peer IP found with matching IP family" const addrFamilyMismatchMessage = "Transit and Destination addresses must have matching IP family" const unknownAppNameMessage = "The App name in the request does not match a configured App" const missingAppPermissionMessage = "You do not have permission to use this App" +const transitIPNotInPoolMessage = "The transit address is not in a configured transit IP pool" // handleConnectorTransitIPRequest creates a ConnectorTransitIPResponse in response // to a ConnectorTransitIPRequest. It updates the connectors mapping of @@ -626,6 +627,13 @@ func (c *connector) handleTransitIPRequest(n tailcfg.NodeView, peerV4 netip.Addr return TransitIPResponse{Code: AddrFamilyMismatch, Message: addrFamilyMismatchMessage} } + // 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", + n.StableID(), tipr.TransitIP) + return TransitIPResponse{Code: TransitIPNotInPool, Message: transitIPNotInPoolMessage} + } + // Datapath lookups only have access to the peer IP, and that will match the family // of the transit IP, so we need to store v4 and v6 mappings separately. var peerAddr netip.Addr @@ -719,6 +727,12 @@ const ( // MissingAppPermission indicates that the client is not permitted to access // the App name that was specified in the request. MissingAppPermission = 6 + + // TransitIPNotInPool indicates that the transit address in the request is + // not within a transit IP pool the connector is configured with. A client + // which sees this has most likely allocated from a pool configuration that + // the connector has not received yet, or has already replaced. + TransitIPNotInPool = 7 ) // TransitIPResponse is the response to a TransitIPRequest @@ -1551,6 +1565,16 @@ func (c *connector) realIPForTransitIPConnection(srcIP netip.Addr, transitIP net return c.lookupAddrBySrcIPAndTransitIP(srcIP, transitIP) } +// transitIPInPool reports whether tip is within the transit IP pool of its +// address family that this connector is configured with. +func (c *connector) transitIPInPool(tip netip.Addr) bool { + ipSets := c.getIPSets() + if tip.Is4() { + return ipSets.v4Transit != nil && ipSets.v4Transit.Contains(tip) + } + return ipSets.v6Transit != nil && ipSets.v6Transit.Contains(tip) +} + const packetFilterAllowReason = "app connector transit IP" // packetFilterAllow returns true if the provided packet has a Src that is in diff --git a/feature/conn25/conn25_test.go b/feature/conn25/conn25_test.go index be1ca9fb4..c36874e7e 100644 --- a/feature/conn25/conn25_test.go +++ b/feature/conn25/conn25_test.go @@ -69,12 +69,16 @@ func TestHandleConnectorTransitIPRequest(t *testing.T) { pipV6_1 := netip.MustParseAddr("fd7a:115c:a1e0::101") pipV6_3 := netip.MustParseAddr("fd7a:115c:a1e0::103") - // Transit IPs - tipV4_1 := netip.MustParseAddr("0.0.0.1") - tipV4_2 := netip.MustParseAddr("0.0.0.2") + // Transit IPs, from the pools configured on the connector below. + tipV4_1 := netip.MustParseAddr("169.254.0.1") + tipV4_2 := netip.MustParseAddr("169.254.0.2") tipV6_1 := netip.MustParseAddr("FE80::1") + // Transit IPs from outside of the configured pools. + tipV4Outside := netip.MustParseAddr("192.0.2.1") + tipV6Outside := netip.MustParseAddr("2001:db8::1") + // Destination IPs dipV4_1 := netip.MustParseAddr("10.0.0.1") dipV4_2 := netip.MustParseAddr("10.0.0.2") @@ -375,6 +379,55 @@ func TestHandleConnectorTransitIPRequest(t *testing.T) { {}, }, }, + // Single peer, ipv4 transit IP from outside the configured pool + { + name: "one-peer-tip-not-in-pool-ipv4", + ctipReqPeers: []tailcfg.NodeView{peerV4Only}, + ctipReqs: []ConnectorTransitIPRequest{ + {TransitIPs: []TransitIPRequest{{TransitIP: tipV4Outside, DestinationIP: dipV4_1, App: appName}}}, + }, + wants: []ConnectorTransitIPResponse{ + {TransitIPs: []TransitIPResponse{{Code: TransitIPNotInPool, Message: transitIPNotInPoolMessage}}}, + }, + wantLookups: [][][]netip.Addr{ + {{pipV4_2, tipV4Outside, netip.Addr{}}}, + }, + }, + // Single peer, ipv6 transit IP from outside the configured pool + { + name: "one-peer-tip-not-in-pool-ipv6", + ctipReqPeers: []tailcfg.NodeView{peerV6Only}, + ctipReqs: []ConnectorTransitIPRequest{ + {TransitIPs: []TransitIPRequest{{TransitIP: tipV6Outside, DestinationIP: dipV6_1, App: appName}}}, + }, + wants: []ConnectorTransitIPResponse{ + {TransitIPs: []TransitIPResponse{{Code: TransitIPNotInPool, Message: transitIPNotInPoolMessage}}}, + }, + wantLookups: [][][]netip.Addr{ + {{pipV6_3, tipV6Outside, netip.Addr{}}}, + }, + }, + // Single peer, an out of pool transit IP does not stop the other + // mappings in the request from being processed. + { + name: "one-peer-multi-map-tip-not-in-pool", + ctipReqPeers: []tailcfg.NodeView{peerV4Only}, + ctipReqs: []ConnectorTransitIPRequest{ + {TransitIPs: []TransitIPRequest{ + {TransitIP: tipV4Outside, DestinationIP: dipV4_1, App: appName}, + {TransitIP: tipV4_2, DestinationIP: dipV4_2, App: appName}, + }}, + }, + wants: []ConnectorTransitIPResponse{ + {TransitIPs: []TransitIPResponse{ + {Code: TransitIPNotInPool, Message: transitIPNotInPoolMessage}, + {Code: OK, Message: ""}, + }}, + }, + wantLookups: [][][]netip.Addr{ + {{pipV4_2, tipV4Outside, netip.Addr{}}, {pipV4_2, tipV4_2, dipV4_2}}, + }, + }, } for _, tt := range tests { @@ -400,6 +453,10 @@ func TestHandleConnectorTransitIPRequest(t *testing.T) { Name: appName, }, }, + ipSets: ipSets{ + v4Transit: mustIPSetFromPrefix("169.254.0.0/24"), + v6Transit: mustIPSetFromPrefix("fe80::/64"), + }, }) for i, peer := range tt.ctipReqPeers { @@ -2723,6 +2780,7 @@ func TestConnectorExpireTransitIPs(t *testing.T) { c.reconfig(&config{ isConfigured: true, appsByName: map[string]appctype.Conn25Attr{appName: {}}, + ipSets: ipSets{v4Transit: mustIPSetFromPrefix("0.0.0.0/15")}, }) clock := tstest.NewClock(tstest.ClockOpts{Start: time.Now()}) // this would be a data race if we had started the sweeper, but we haven't.