diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 33b3a503e..e3a7f788b 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -1290,6 +1290,28 @@ func isNICForwarding(nic *nic, proto tcpip.NetworkProtocolNumber) bool { } } +// findRouteWithLocalAddrFromAnyInterfaceRLocked returns a route to the given +// destination address, leaving through the given NIC. +// +// Rather than preferring to find a route that uses a local address assigned to +// the outgoing interface, it finds any NIC that holds a matching local address +// endpoint. +// +// +checklocksread:s.mu +func (s *Stack) findRouteWithLocalAddrFromAnyInterfaceRLocked(outgoingNIC *nic, localAddr, remoteAddr, gateway tcpip.Address, netProto tcpip.NetworkProtocolNumber, multicastLoop bool) *Route { + for _, aNIC := range s.nics { + addressEndpoint := s.getAddressEP(aNIC, localAddr, remoteAddr, netProto) + if addressEndpoint == nil { + continue + } + + if r := constructAndValidateRoute(netProto, addressEndpoint, aNIC /* localAddressNIC */, outgoingNIC, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop); r != nil { + return r + } + } + return nil +} + // FindRoute creates a route to the given destination address, leaving through // the given NIC and local address (if provided). // @@ -1379,15 +1401,27 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n } } - // If the stack has forwarding enabled and we haven't found a valid route - // to the remote address yet, keep track of the first valid route. We - // keep iterating because we prefer routes that let us use a local - // address that is assigned to the outgoing interface. There is no - // requirement to do this from any RFC but simply a choice made to better - // follow a strong host model which the netstack follows at the time of - // writing. + // If the stack has forwarding enabled, we haven't found a valid route to + // the remote address yet, and we are routing locally generated traffic, + // keep track of the first valid route. We keep iterating because we + // prefer routes that let us use a local address that is assigned to the + // outgoing interface. There is no requirement to do this from any RFC + // but simply a choice made to better follow a strong host model which + // the netstack follows at the time of writing. + // + // Note that for incoming traffic that we are forwarding (for which the + // NIC and local address are unspecified), we do not keep iterating, as + // there is no reason to prefer routes that let us use a local address + // when routing forwarded (as opposed to locally-generated) traffic. + locallyGenerated := (id != 0 || localAddr != tcpip.Address{}) if onlyGlobalAddresses && chosenRoute.Equal(tcpip.Route{}) && isNICForwarding(nic, netProto) { - chosenRoute = route + if locallyGenerated { + chosenRoute = route + continue + } + if r := s.findRouteWithLocalAddrFromAnyInterfaceRLocked(nic, localAddr, remoteAddr, route.Gateway, netProto, multicastLoop); r != nil { + return r + } } } @@ -1427,15 +1461,8 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n if id == 0 { // If an interface is not specified, try to find a NIC that holds the local // address endpoint to construct a route. - for _, aNIC := range s.nics { - addressEndpoint := s.getAddressEP(aNIC, localAddr, remoteAddr, netProto) - if addressEndpoint == nil { - continue - } - - if r := constructAndValidateRoute(netProto, addressEndpoint, aNIC /* localAddressNIC */, nic /* outgoingNIC */, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop); r != nil { - return r, nil - } + if r := s.findRouteWithLocalAddrFromAnyInterfaceRLocked(nic, localAddr, remoteAddr, gateway, netProto, multicastLoop); r != nil { + return r, nil } } } diff --git a/pkg/tcpip/stack/stack_test.go b/pkg/tcpip/stack/stack_test.go index 0ed358e51..10def3921 100644 --- a/pkg/tcpip/stack/stack_test.go +++ b/pkg/tcpip/stack/stack_test.go @@ -4836,6 +4836,91 @@ func TestFindRouteWithForwarding(t *testing.T) { } } +func TestFindRoutePrefersLocalAddrOnlyForLocallyGeneratedTraffic(t *testing.T) { + const ( + nicID1 = 1 + nicID2 = 2 + ) + var ( + nic1Addr = tcpip.AddrFromSlice([]byte("\x01\x00\x00\x00")) + nic2Addr = tcpip.AddrFromSlice([]byte("\x02\x00\x00\x00")) + gatewayAddr = tcpip.AddrFromSlice([]byte("\x03\x00\x00\x00")) + ) + + tests := []struct { + name string + localAddr tcpip.Address + remoteAddr tcpip.Address + wantOutgoingNIC tcpip.NICID + }{ + { + name: "locally generated traffic routed through default gateway because we prefer local address on outgoing interface", + localAddr: nic1Addr, + remoteAddr: nic2Addr, + wantOutgoingNIC: nicID1, + }, + { + name: "forwarded traffic routed through NIC 2 because local address preference only applies to locally generated traffic", + localAddr: tcpip.Address{}, + remoteAddr: nic2Addr, + wantOutgoingNIC: nicID2, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s := stack.New(stack.Options{ + NetworkProtocols: []stack.NetworkProtocolFactory{fakeNetFactory}, + }) + + ep1 := channel.New(1, defaultMTU, "") + if err := s.CreateNIC(nicID1, ep1); err != nil { + t.Fatalf("CreateNIC(%d, _): %s:", nicID1, err) + } + + ep2 := channel.New(1, defaultMTU, "") + if err := s.CreateNIC(nicID2, ep2); err != nil { + t.Fatalf("CreateNIC(%d, _): %s:", nicID2, err) + } + + // NB: we do *not* assign nic2Addr on NIC 2. We are exercising the scenario when we are forwarding + // traffic to an address that we do not own. + protocolAddr1 := tcpip.ProtocolAddress{ + Protocol: fakeNetNumber, + AddressWithPrefix: tcpip.AddressWithPrefix{Address: nic1Addr, PrefixLen: fakeDefaultPrefixLen}, + } + if err := s.AddProtocolAddress(nicID1, protocolAddr1, stack.AddressProperties{}); err != nil { + t.Fatalf("AddProtocolAddress(%d, %+v, {}): %s", nicID1, protocolAddr1, err) + } + + if err := s.SetForwardingDefaultAndAllNICs(fakeNetNumber, true); err != nil { + t.Fatalf("SetForwardingDefaultAndAllNICs(%d, %t): %s", fakeNetNumber, true, err) + } + + unspecifiedSubnet := func() tcpip.Subnet { + unspecifiedSubnet, err := tcpip.NewSubnet(tcpip.AddrFrom4Slice([]byte("\x00\x00\x00\x00")), tcpip.MaskFrom("\x00\x00\x00\x00")) + if err != nil { + t.Fatal(err) + } + return unspecifiedSubnet + }() + s.SetRouteTable([]tcpip.Route{{Destination: nic2Addr.WithPrefix().Subnet(), NIC: nicID2}, {Destination: unspecifiedSubnet, Gateway: gatewayAddr, NIC: nicID1}}) + + r, err := s.FindRoute(0, test.localAddr, test.remoteAddr, fakeNetNumber, false /* multicastLoop */) + if err != nil { + t.Fatalf("FindRoute(0, %s, %s, %d, false): got %s, want nil", test.localAddr, test.remoteAddr, fakeNetNumber, err) + } + if r.NICID() != test.wantOutgoingNIC { + t.Errorf("got r.NICID() = %d, want = %d", r.NICID(), test.wantOutgoingNIC) + } + + if t.Failed() { + t.FailNow() + } + }) + } +} + func TestAddMulticastRoute(t *testing.T) { const ( incomingNICID = 1