From f681bcc095fb168ae26f9af44004e4e67993a393 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Tue, 17 Sep 2024 14:30:56 -0700 Subject: [PATCH] Implement RTM_DELROUTE in netstack. PiperOrigin-RevId: 675711612 --- pkg/sentry/inet/inet.go | 3 + pkg/sentry/inet/test_stack.go | 5 + pkg/sentry/socket/hostinet/stack.go | 5 + pkg/sentry/socket/netlink/route/protocol.go | 14 +++ pkg/sentry/socket/netstack/stack.go | 86 +++++++++++----- pkg/tcpip/stack/stack.go | 12 ++- pkg/tcpip/stack/stack_test.go | 13 ++- test/rtnetlink/linux/route_test.sh | 12 +++ test/syscalls/linux/socket_netlink_route.cc | 105 ++++++++++++++++++++ 9 files changed, 220 insertions(+), 35 deletions(-) diff --git a/pkg/sentry/inet/inet.go b/pkg/sentry/inet/inet.go index ba7cd422a..d8b093402 100644 --- a/pkg/sentry/inet/inet.go +++ b/pkg/sentry/inet/inet.go @@ -85,6 +85,9 @@ type Stack interface { // RouteTable returns the network stack's route table. RouteTable() []Route + // RemoveRoute deletes the specified route. + RemoveRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error + // NewRoute adds the given route to the network stack's route table. NewRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error diff --git a/pkg/sentry/inet/test_stack.go b/pkg/sentry/inet/test_stack.go index 4e3b4ddd1..34e8d1d56 100644 --- a/pkg/sentry/inet/test_stack.go +++ b/pkg/sentry/inet/test_stack.go @@ -159,6 +159,11 @@ func (s *TestStack) RouteTable() []Route { return s.RouteList } +// RemoveRoute implements Stack. +func (s *TestStack) RemoveRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error { + return nil +} + // NewRoute implements Stack. func (s *TestStack) NewRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error { return syserr.ErrNotPermitted diff --git a/pkg/sentry/socket/hostinet/stack.go b/pkg/sentry/socket/hostinet/stack.go index f91d4a013..275894e5c 100644 --- a/pkg/sentry/socket/hostinet/stack.go +++ b/pkg/sentry/socket/hostinet/stack.go @@ -388,6 +388,11 @@ func (*Stack) NewRoute(context.Context, *nlmsg.Message) *syserr.Error { return syserr.ErrNotSupported } +// RemoveRoute implements inet.Stack.RemoveRoute. +func (*Stack) RemoveRoute(context.Context, *nlmsg.Message) *syserr.Error { + return syserr.ErrNotSupported +} + // Pause implements inet.Stack.Pause. func (*Stack) Pause() {} diff --git a/pkg/sentry/socket/netlink/route/protocol.go b/pkg/sentry/socket/netlink/route/protocol.go index 1fe9602ee..073c95060 100644 --- a/pkg/sentry/socket/netlink/route/protocol.go +++ b/pkg/sentry/socket/netlink/route/protocol.go @@ -411,6 +411,18 @@ func (p *Protocol) newRoute(ctx context.Context, s *netlink.Socket, msg *nlmsg.M return stack.NewRoute(ctx, msg) } +// deleteRoute handles RTM_DELROUTE requests. +func (p *Protocol) deleteRoute(ctx context.Context, s *netlink.Socket, msg *nlmsg.Message, ms *nlmsg.MessageSet) *syserr.Error { + stack := s.Stack() + if stack == nil { + return syserr.ErrNoNet + } + if msg.Header().Flags&linux.NLM_F_REQUEST != linux.NLM_F_REQUEST { + return syserr.ErrProtocolNotSupported + } + return stack.RemoveRoute(ctx, msg) +} + // dumpRoutes handles RTM_GETROUTE requests. func (p *Protocol) dumpRoutes(ctx context.Context, s *netlink.Socket, msg *nlmsg.Message, ms *nlmsg.MessageSet) *syserr.Error { // RTM_GETROUTE dump requests need not contain anything more than the @@ -635,6 +647,8 @@ func (p *Protocol) ProcessMessage(ctx context.Context, s *netlink.Socket, msg *n return p.newRoute(ctx, s, msg, ms) case linux.RTM_GETROUTE: return p.dumpRoutes(ctx, s, msg, ms) + case linux.RTM_DELROUTE: + return p.deleteRoute(ctx, s, msg, ms) case linux.RTM_NEWADDR: return p.newAddr(ctx, s, msg, ms) case linux.RTM_DELADDR: diff --git a/pkg/sentry/socket/netstack/stack.go b/pkg/sentry/socket/netstack/stack.go index 47277c63b..44ed1feb8 100644 --- a/pkg/sentry/socket/netstack/stack.go +++ b/pkg/sentry/socket/netstack/stack.go @@ -749,87 +749,86 @@ func (s *Stack) RouteTable() []inet.Route { return routeTable } -// NewRoute implements inet.Stack.NewRoute. -func (s *Stack) NewRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error { - var routeMsg linux.RouteMessage - attrs, ok := msg.GetData(&routeMsg) +// localRoute constructs a local route from the netlink message. +func (s *Stack) localRoute(msg *nlmsg.Message) (tcpip.Route, *syserr.Error) { + var rtMsg linux.RouteMessage + attrs, ok := msg.GetData(&rtMsg) if !ok { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } route := inet.Route{ - Family: routeMsg.Family, - DstLen: routeMsg.DstLen, - SrcLen: routeMsg.SrcLen, - TOS: routeMsg.TOS, - Table: routeMsg.Table, - Protocol: routeMsg.Protocol, - Scope: routeMsg.Scope, - Type: routeMsg.Type, - Flags: routeMsg.Flags, + Family: rtMsg.Family, + DstLen: rtMsg.DstLen, + SrcLen: rtMsg.SrcLen, + TOS: rtMsg.TOS, + Table: rtMsg.Table, + Protocol: rtMsg.Protocol, + Scope: rtMsg.Scope, + Type: rtMsg.Type, + Flags: rtMsg.Flags, } for !attrs.Empty() { ahdr, value, rest, ok := attrs.ParseFirst() if !ok { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } attrs = rest switch ahdr.Type { case linux.RTA_DST: if len(value) < 1 { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } route.DstAddr = value case linux.RTA_SRC: if len(value) < 1 { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } route.SrcAddr = value case linux.RTA_OIF: oif := nlmsg.BytesView(value) outputInterface, ok := oif.Int32() if !ok { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } if _, exist := s.Interfaces()[outputInterface]; !exist { - return syserr.ErrNoDevice + return tcpip.Route{}, syserr.ErrNoDevice } route.OutputInterface = outputInterface case linux.RTA_GATEWAY: if len(value) < 1 { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } route.GatewayAddr = value case linux.RTA_PRIORITY: default: - ctx.Warningf("Unknown attribute: %v", ahdr.Type) - return syserr.ErrNotSupported + log.Warningf("Unknown attribute: %v", ahdr.Type) + return tcpip.Route{}, syserr.ErrNotSupported } } - var dest tcpip.Subnet // When no destination address is provided, the new route might be the default route. if route.DstAddr == nil { if route.GatewayAddr == nil { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } switch len(route.GatewayAddr) { case header.IPv4AddressSize: subnet, err := tcpip.NewSubnet(tcpip.AddrFromSlice(tcpip.IPv4Zero), tcpip.MaskFromBytes(tcpip.IPv4Zero)) if err != nil { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } dest = subnet case header.IPv6AddressSize: subnet, err := tcpip.NewSubnet(tcpip.AddrFromSlice(tcpip.IPv6Zero), tcpip.MaskFromBytes(tcpip.IPv6Zero)) if err != nil { - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } dest = subnet default: - return syserr.ErrInvalidArgument + return tcpip.Route{}, syserr.ErrInvalidArgument } } else { dest = tcpip.AddressWithPrefix{ @@ -842,9 +841,42 @@ func (s *Stack) NewRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error Gateway: tcpip.AddrFromSlice(route.GatewayAddr), NIC: tcpip.NICID(route.OutputInterface), } + if len(route.SrcAddr) != 0 { localRoute.SourceHint = tcpip.AddrFromSlice(route.SrcAddr) } + + return localRoute, nil +} + +// RemoveRoute implements inte.Stack.RemoveRoute. +func (s *Stack) RemoveRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error { + localRoute, err := s.localRoute(msg) + if err != nil { + return err + } + if removed := s.Stack.RemoveRoutes(func(rt tcpip.Route) bool { + // Both gateway and NIC are compared with existing routes + // only when they are present in the netlink message. + if localRoute.Gateway.Len() > 0 && !localRoute.Gateway.Equal(rt.Gateway) { + return false + } + if localRoute.NIC > 0 && localRoute.NIC != rt.NIC { + return false + } + return rt.Destination.Equal(localRoute.Destination) + }); removed == 0 { + return syserr.ErrNoProcess + } + return nil +} + +// NewRoute implements inet.Stack.NewRoute. +func (s *Stack) NewRoute(ctx context.Context, msg *nlmsg.Message) *syserr.Error { + localRoute, err := s.localRoute(msg) + if err != nil { + return err + } found := false for _, rt := range s.Stack.GetRouteTable() { if localRoute.Equal(rt) { diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 7dc7cd357..093cb39a6 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -779,23 +779,27 @@ func (s *Stack) addRouteLocked(route *tcpip.Route) { s.routeTable.PushBack(route) } -// RemoveRoutes removes matching routes from the route table. -func (s *Stack) RemoveRoutes(match func(tcpip.Route) bool) { +// RemoveRoutes removes matching routes from the route table, it +// returns the number of routes that are removed. +func (s *Stack) RemoveRoutes(match func(tcpip.Route) bool) int { s.routeMu.Lock() defer s.routeMu.Unlock() - s.removeRoutesLocked(match) + return s.removeRoutesLocked(match) } // +checklocks:s.routeMu -func (s *Stack) removeRoutesLocked(match func(tcpip.Route) bool) { +func (s *Stack) removeRoutesLocked(match func(tcpip.Route) bool) int { + count := 0 for route := s.routeTable.Front(); route != nil; { next := route.Next() if match(*route) { s.routeTable.Remove(route) + count++ } route = next } + return count } // ReplaceRoute replaces the route in the routing table which matchse diff --git a/pkg/tcpip/stack/stack_test.go b/pkg/tcpip/stack/stack_test.go index a85d1d282..de900bbe4 100644 --- a/pkg/tcpip/stack/stack_test.go +++ b/pkg/tcpip/stack/stack_test.go @@ -4449,15 +4449,16 @@ func TestRemoveRoutes(t *testing.T) { t.Fatal(err) } - // Initialize the route table with three routes. - s.SetRouteTable([]tcpip.Route{ + routeList := []tcpip.Route{ {Destination: subnet1, Gateway: tcpip.AddrFromSlice([]byte("\x00\x00\x00\x00")), NIC: 1}, {Destination: subnet2, Gateway: tcpip.AddrFromSlice([]byte("\x00\x00\x00\x00")), NIC: 1}, {Destination: subnet3, Gateway: tcpip.AddrFromSlice([]byte("\x00\x00\x00\x00")), NIC: 1}, - }) + } + // Initialize the route table with three routes. + s.SetRouteTable(routeList) // Remove routes with the specific address. - s.RemoveRoutes(func(r tcpip.Route) bool { + removed := s.RemoveRoutes(func(r tcpip.Route) bool { return r.Destination.ID() == addressToRemove }) @@ -4471,6 +4472,10 @@ func TestRemoveRoutes(t *testing.T) { t.Fatalf("Unexpected route got = %#v, want = %#v", got, want) } } + + if got, want := removed, len(routeList)-len(expected); want != removed { + t.Fatalf("stack.RemoveRoutes(_) removes %v routes, want = %v", got, want) + } } func TestFindRouteWithForwarding(t *testing.T) { diff --git a/test/rtnetlink/linux/route_test.sh b/test/rtnetlink/linux/route_test.sh index 9cd22391a..c83aae112 100755 --- a/test/rtnetlink/linux/route_test.sh +++ b/test/rtnetlink/linux/route_test.sh @@ -23,11 +23,23 @@ ip link add name veth1 type veth peer name eth0 netns test ip netns exec test ip link set up dev lo ip netns exec test ip link set up dev eth0 ip netns exec test ip addr add 192.168.11.2/24 dev eth0 +ORIGINAL_ROUTES=$(ip netns exec test ip r) ip netns exec test ip r add default via 192.168.11.1 dev eth0 ip netns exec test ip r list | grep "default via 192.168.11.1 dev eth0" ip netns exec test ip r add 192.168.146.48/28 dev eth0 ip netns exec test ip r list | grep "192.168.146.48/28 dev eth0" +ip netns exec test ip route # Replace the routes. ip netns exec test ip r replace default via 192.168.11.2 dev eth0 ip netns exec test ip r list | grep "default via 192.168.11.2 dev eth0" + +# Remove all routes that are add/modified above. +ip netns exec test ip r del default via 192.168.11.2 dev eth0 +ip netns exec test ip r del 192.168.146.48/28 +CURRENT_ROUTES=$(ip netns exec test ip r) + +if [[ "$ORIGINAL_ROUTES" != "$CURRENT_ROUTES" ]]; then + fail "unexpected routes are present" + exit 1 +fi diff --git a/test/syscalls/linux/socket_netlink_route.cc b/test/syscalls/linux/socket_netlink_route.cc index e4a009f85..138c27df8 100644 --- a/test/syscalls/linux/socket_netlink_route.cc +++ b/test/syscalls/linux/socket_netlink_route.cc @@ -1195,6 +1195,111 @@ TEST_P(NetlinkRouteIpInvariantTest, NewRoute) { EXPECT_TRUE(routeDstFound); } +TEST_P(NetlinkRouteIpInvariantTest, DeleteRoute) { + // CAP_NET_ADMIN is required to modify the routing table. + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_NET_ADMIN))); + SKIP_IF(!IsRunningOnGvisor()); + SKIP_IF(IsRunningWithHostinet()); + // Routes are not savable. + DisableSave ds; + + const std::string dst_v4_address = "192.0.3.0"; + const std::string dst_v6_address = "2011:db8::"; + + // Based on the test parameter, build an IPv4 or IPv6 destination subnet. + int family = GetParam(); + void* dst = nullptr; + int dst_len; + int prefixlen; + struct in_addr dst_v4; + struct in6_addr dst_v6; + switch (family) { + case AF_INET: + ASSERT_EQ(inet_pton(family, dst_v4_address.c_str(), &dst_v4), 1); + prefixlen = 24; + dst = &dst_v4; + dst_len = sizeof(dst_v4); + break; + case AF_INET6: + ASSERT_EQ(inet_pton(family, dst_v6_address.c_str(), &dst_v6), 1); + prefixlen = 64; + dst = &dst_v6; + dst_len = sizeof(dst_v6); + break; + default: + FAIL() << "address family must be AF_INET or AF_INET6"; + } + + Link loopback_link = ASSERT_NO_ERRNO_AND_VALUE(LoopbackLink()); + + ASSERT_NO_ERRNO( + AddUnicastRoute(loopback_link.index, family, prefixlen, dst, dst_len)); + ASSERT_NO_ERRNO( + DelUnicastRoute(loopback_link.index, family, prefixlen, dst, dst_len)); + + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(NetlinkBoundSocket(NETLINK_ROUTE)); + + struct request { + struct nlmsghdr hdr; + struct rtmsg rtm; + }; + + struct request req = {}; + req.hdr.nlmsg_len = sizeof(req); + req.hdr.nlmsg_type = RTM_GETROUTE; + req.hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP; + req.hdr.nlmsg_seq = kSeq; + req.rtm.rtm_family = AF_UNSPEC; + + bool routeDstFound = false; + ASSERT_NO_ERRNO(NetlinkRequestResponse( + fd, &req, sizeof(req), + [&](const struct nlmsghdr* hdr) { + // Validate the reponse to RTM_GETROUTE. + EXPECT_THAT(hdr->nlmsg_type, AnyOf(Eq(RTM_NEWROUTE), Eq(NLMSG_DONE))); + // The test should not proceed if it's not a RTM_NEWROUTE message. + if (hdr->nlmsg_type != RTM_NEWROUTE) { + return; + } + const struct rtmsg* msg = + reinterpret_cast(NLMSG_DATA(hdr)); + int len = RTM_PAYLOAD(hdr); + for (struct rtattr* attr = RTM_RTA(msg); RTA_OK(attr, len); + attr = RTA_NEXT(attr, len)) { + if (attr->rta_type == RTA_DST) { + char v4_address[INET_ADDRSTRLEN] = {}; + char v6_address[INET6_ADDRSTRLEN] = {}; + switch (family) { + case AF_INET: + inet_ntop(AF_INET, RTA_DATA(attr), v4_address, + sizeof(v4_address)); + if (strcmp(v4_address, dst_v4_address.c_str()) == 0) { + routeDstFound = true; + return; + } + break; + case AF_INET6: + inet_ntop(AF_INET6, RTA_DATA(attr), v6_address, + sizeof(v6_address)); + if (strcmp(v6_address, dst_v6_address.c_str()) == 0) { + routeDstFound = true; + return; + } + break; + } + } + } + }, + false)); + // No route that matches the given destination address can be found. + EXPECT_FALSE(routeDstFound); + // Removing a route that doens't exist returns an error. + EXPECT_THAT( + DelUnicastRoute(loopback_link.index, family, prefixlen, dst, dst_len), + PosixErrorIs(ESRCH, _)); +} + TEST_P(NetlinkRouteIpInvariantTest, AddAndRemoveRoute) { // Gvisor does not support `RTM_NEWROUTE` or `RTM_DELROUTE`. SKIP_IF(IsRunningOnGvisor() && GvisorPlatform() != Platform::kStarnix);