m/n/c/n/dhcp4c: support route configuration (including RFC 3442)
This implements support for configuring routes other than the default
route via our DHCP client. It extends the DHCP lease interface with a
method to return canonicalized routes from a lease containing Router,
StaticRoute or ClasslessStaticRouting options.
It also extends and renames the former ManageDefaultRoutes callback into
ManageRoutes and makes it use the new canonicalized routing data instead
of just the default router.
Change-Id: Ie6ec20d67c0e9cdfa6be088324b42e0d811e81e9
Reviewed-on: https://review.monogon.dev/c/monogon/+/482
Reviewed-by: Sergiusz Bazanski <serge@monogon.tech>
diff --git a/metropolis/node/core/network/dhcp4c/callback/callback.go b/metropolis/node/core/network/dhcp4c/callback/callback.go
index 52276ab..0ac0289 100644
--- a/metropolis/node/core/network/dhcp4c/callback/callback.go
+++ b/metropolis/node/core/network/dhcp4c/callback/callback.go
@@ -111,13 +111,13 @@
}
}
-// ManageDefaultRoute manages a default route through the first router offered
-// by DHCP. It does nothing if DHCP doesn't provide any routers. It takes
-// ownership of all RTPROTO_DHCP routes on the given interface, so it's not
-// possible to run multiple DHCP clients on the given interface.
-func ManageDefaultRoute(iface netlink.Link) dhcp4c.LeaseCallback {
+// ManageRoutes installs and removes routes according to the current DHCP lease,
+// including the default route (if any).
+// It takes ownership of all RTPROTO_DHCP routes on the given interface, so it's
+// not possible to run multiple DHCP clients on the given interface.
+func ManageRoutes(iface netlink.Link) dhcp4c.LeaseCallback {
return func(old, new *dhcp4c.Lease) error {
- newRouter := new.Router()
+ newRoutes := new.Routes()
dhcpRoutes, err := netlink.RouteListFiltered(netlink.FAMILY_V4, &netlink.Route{
Protocol: unix.RTPROT_DHCP,
@@ -126,30 +126,45 @@
if err != nil {
return fmt.Errorf("netlink failed to list routes: %w", err)
}
- ipv4DefaultRoute := net.IPNet{IP: net.IPv4zero, Mask: net.CIDRMask(0, 32)}
for _, route := range dhcpRoutes {
// Don't remove routes which can be atomically replaced by
// RouteReplace to prevent potential traffic disruptions.
- if !isIPNetEqual(&ipv4DefaultRoute, route.Dst) && newRouter != nil {
- continue
+ //
+ // This is O(n^2) but the number of routes is bounded by the size
+ // of a DHCP packet (around 100 routes). Sorting both would be
+ // be marginally faster for large amounts of routes only and in 99%
+ // of cases it's going to be <5 routes.
+ var found bool
+ for _, newRoute := range newRoutes {
+ if isIPNetEqual(newRoute.Dest, route.Dst) {
+ found = true
+ break
+ }
}
- err := netlink.RouteDel(&route)
- if !os.IsNotExist(err) && err != nil {
- return fmt.Errorf("failed to delete DHCP route: %w", err)
+ if !found {
+ err := netlink.RouteDel(&route)
+ if !os.IsNotExist(err) && err != nil {
+ return fmt.Errorf("failed to delete DHCP route: %w", err)
+ }
}
}
- if newRouter != nil {
- err := netlink.RouteReplace(&netlink.Route{
+ for _, route := range newRoutes {
+ newRoute := netlink.Route{
Protocol: unix.RTPROT_DHCP,
- Dst: &ipv4DefaultRoute,
- Gw: newRouter,
+ Dst: route.Dest,
+ Gw: route.Router,
Src: new.AssignedIP,
LinkIndex: iface.Attrs().Index,
Scope: netlink.SCOPE_UNIVERSE,
- })
+ }
+ // Routes with a non-L3 gateway are link-scoped
+ if route.Router.IsUnspecified() {
+ newRoute.Scope = netlink.SCOPE_LINK
+ }
+ err := netlink.RouteReplace(&newRoute)
if err != nil {
- return fmt.Errorf("failed to add default route via %s: %w", newRouter, err)
+ return fmt.Errorf("failed to add %s: %w", route, err)
}
}
return nil
diff --git a/metropolis/node/core/network/dhcp4c/callback/callback_test.go b/metropolis/node/core/network/dhcp4c/callback/callback_test.go
index d2982db..7f5b5d3 100644
--- a/metropolis/node/core/network/dhcp4c/callback/callback_test.go
+++ b/metropolis/node/core/network/dhcp4c/callback/callback_test.go
@@ -147,6 +147,23 @@
return lease
}
+func leaseAddClasslessRoutes(lease *dhcp4c.Lease, routes ...*dhcpv4.Route) *dhcp4c.Lease {
+ lease.Options.Update(dhcpv4.OptClasslessStaticRoute(routes...))
+ return lease
+}
+
+func mustParseCIDR(cidr string) *net.IPNet {
+ _, n, err := net.ParseCIDR(cidr)
+ if err != nil {
+ panic(err)
+ }
+ // Equality checks don't know about net.IP's canonicalization rules.
+ if n.IP.To4() != nil {
+ n.IP = n.IP.To4()
+ }
+ return n
+}
+
func TestDefaultRouteCallback(t *testing.T) {
if os.Getenv("IN_KTEST") != "true" {
t.Skip("Not in ktest")
@@ -255,6 +272,37 @@
Type: unix.RTN_UNICAST,
}},
},
+ {
+ name: "AddsClasslessStaticRoutes",
+ initialRoutes: []netlink.Route{},
+ oldLease: nil,
+ newLease: leaseAddClasslessRoutes(
+ // Router should be ignored
+ leaseAddRouter(trivialLeaseFromNet(testNet1), testNet1Router),
+ // P2P/foreign gateway route
+ &dhcpv4.Route{Dest: mustParseCIDR("192.168.42.1/32"), Router: net.IPv4zero},
+ // Standard route over foreign gateway set up by previous route
+ &dhcpv4.Route{Dest: mustParseCIDR("0.0.0.0/0"), Router: net.IPv4(192, 168, 42, 1)},
+ ),
+ expectedRoutes: []netlink.Route{{
+ Protocol: unix.RTPROT_DHCP,
+ Dst: nil,
+ Gw: net.IPv4(192, 168, 42, 1).To4(), // Equal() doesn't know about canonicalization
+ Src: testNet1.IP,
+ Table: mainRoutingTable,
+ LinkIndex: -1, // Filled in dynamically with test interface
+ Type: unix.RTN_UNICAST,
+ }, {
+ Protocol: unix.RTPROT_DHCP,
+ Dst: mustParseCIDR("192.168.42.1/32"),
+ Gw: nil,
+ Src: testNet1.IP,
+ Table: mainRoutingTable,
+ LinkIndex: -1, // Filled in dynamically with test interface
+ Type: unix.RTN_UNICAST,
+ Scope: unix.RT_SCOPE_LINK,
+ }},
+ },
}
for i, test := range tests {
t.Run(test.name, func(t *testing.T) {
@@ -300,7 +348,7 @@
}
}
- cb := ManageDefaultRoute(testLink)
+ cb := ManageRoutes(testLink)
if err := cb(test.oldLease, test.newLease); err != nil {
t.Fatalf("callback returned an error: %v", err)
}
diff --git a/metropolis/node/core/network/dhcp4c/lease.go b/metropolis/node/core/network/dhcp4c/lease.go
index 63c51be..86daa0c 100644
--- a/metropolis/node/core/network/dhcp4c/lease.go
+++ b/metropolis/node/core/network/dhcp4c/lease.go
@@ -62,21 +62,133 @@
}
}
-// Router returns the first valid router from the DHCP router option or nil if
-// none such exists.
-// It returns nil if the lease is nil.
-func (l *Lease) Router() net.IP {
+// Routes returns all routes assigned by a DHCP answer. It combines and
+// normalizes data from the Router, StaticRoutingTable and ClasslessStaticRoute
+// options.
+func (l *Lease) Routes() []*dhcpv4.Route {
if l == nil {
return nil
}
- routers := dhcpv4.GetIPs(dhcpv4.OptionRouter, l.Options)
- for _, r := range routers {
+
+ // Note that this is different from l.IPNet() because we care about the
+ // network base address of the network instead of the assigned IP.
+ assignedNet := &net.IPNet{IP: l.AssignedIP.Mask(l.SubnetMask()), Mask: l.SubnetMask()}
+
+ // RFC 3442 Section DHCP Client Behavior:
+ // If the DHCP server returns both a Classless Static Routes option and
+ // a Router option, the DHCP client MUST ignore the Router option.
+ // Similarly, if the DHCP server returns both a Classless Static Routes
+ // option and a Static Routes option, the DHCP client MUST ignore the
+ // Static Routes option.
+ var routes dhcpv4.Routes
+ rawCIDRRoutes := l.Options.Get(dhcpv4.OptionClasslessStaticRoute)
+ if rawCIDRRoutes != nil {
+ // TODO(#96): This needs a logging story
+ // Ignore errors intentionally and just return what has been parsed
+ _ = routes.FromBytes(rawCIDRRoutes)
+ return sanitizeRoutes(routes, assignedNet)
+ }
+ // The Static Routes option contains legacy classful routes (i.e. routes
+ // whose mask is determined by the IP of the network).
+ // Each static route is expressed as a pair of IPs, the first one being
+ // the destination network and the second one being the router IP.
+ // See RFC 2132 Section 5.8 for further details.
+ legacyRouteIPs := dhcpv4.GetIPs(dhcpv4.OptionStaticRoutingTable, l.Options)
+ // Routes are only valid in pairs, cut the last one off if necessary
+ if len(legacyRouteIPs)%2 != 0 {
+ legacyRouteIPs = legacyRouteIPs[:len(legacyRouteIPs)-1]
+ }
+ for i := 0; i < len(legacyRouteIPs)/2; i++ {
+ dest := legacyRouteIPs[i*2]
+ if dest.IsUnspecified() {
+ // RFC 2132 Section 5.8:
+ // The default route (0.0.0.0) is an illegal destination for a
+ // static route.
+ continue
+ }
+ via := legacyRouteIPs[i*2+1]
+ destNet := net.IPNet{
+ // Apply the default mask just to make sure this is a valid route
+ IP: dest.Mask(dest.DefaultMask()),
+ Mask: dest.DefaultMask(),
+ }
+ routes = append(routes, &dhcpv4.Route{Dest: &destNet, Router: via})
+ }
+ for _, r := range dhcpv4.GetIPs(dhcpv4.OptionRouter, l.Options) {
if r.IsGlobalUnicast() || r.IsLinkLocalUnicast() {
- return r
+ routes = append(routes, &dhcpv4.Route{
+ Dest: &net.IPNet{IP: net.IPv4zero, Mask: net.IPv4Mask(0, 0, 0, 0)},
+ Router: r,
+ })
+ // Only one default router can exist, exit after the first one
+ break
}
}
- // No (valid) router found
- return nil
+ return sanitizeRoutes(routes, assignedNet)
+}
+
+// sanitizeRoutes filters the list of routes by removing routes that are
+// obviously invalid. It filters out routes according to the following criteria:
+// 1. The route is not an interface route and its router is not a unicast or
+// link-local address.
+// 2. Each route's router must be reachable according to the routes listed
+// before it and the assigned network.
+// 3. The network mask must consist of all-ones followed by all-zeros. Non-
+// contiguous routes are not allowed.
+// 4. If multiple routes match the same destination, only the first one is kept.
+// 5. Routes covering the loopback IP space (127.0.0.0/8) will be ignored if
+// they are smaller than a /9 to prevent them from interfering with loopback
+// IPs.
+func sanitizeRoutes(routes []*dhcpv4.Route, assignedNet *net.IPNet) []*dhcpv4.Route {
+ var saneRoutes []*dhcpv4.Route
+ for _, route := range routes {
+ if route.Router != nil && !route.Router.IsUnspecified() {
+ if !(route.Router.IsGlobalUnicast() || route.Router.IsLinkLocalUnicast()) {
+ // Ignore non-unicast routers
+ continue
+ }
+ reachable := false
+ for _, r := range saneRoutes {
+ if r.Dest.Contains(route.Router) {
+ reachable = true
+ break
+ }
+ }
+ if assignedNet.Contains(route.Router) {
+ reachable = true
+ }
+ if !reachable {
+ continue
+ }
+ }
+ ones, bits := route.Dest.Mask.Size()
+ if bits == 0 && len(route.Dest.Mask) > 0 {
+ // Bitmask is not ones followed by zeros, i.e. invalid
+ continue
+ }
+ // Ignore routes that would be able to redirect loopback IPs
+ if route.Dest.IP.IsLoopback() && ones >= 8 {
+ continue
+ }
+ // Ignore routes that would shadow the implicit interface route
+ assignedOnes, _ := assignedNet.Mask.Size()
+ if assignedNet.IP.Equal(route.Dest.IP) && assignedOnes == ones {
+ continue
+ }
+ validDest := true
+ for _, r := range saneRoutes {
+ rOnes, _ := r.Dest.Mask.Size()
+ if r.Dest.IP.Equal(route.Dest.IP) && ones == rOnes {
+ // Exact duplicate, ignore
+ validDest = false
+ break
+ }
+ }
+ if validDest {
+ saneRoutes = append(saneRoutes, route)
+ }
+ }
+ return saneRoutes
}
// DNSServers represents an ordered collection of DNS servers
diff --git a/metropolis/node/core/network/dhcp4c/lease_test.go b/metropolis/node/core/network/dhcp4c/lease_test.go
index 823656f..070d1b6 100644
--- a/metropolis/node/core/network/dhcp4c/lease_test.go
+++ b/metropolis/node/core/network/dhcp4c/lease_test.go
@@ -17,6 +17,7 @@
package dhcp4c
import (
+ "bytes"
"net"
"testing"
@@ -53,3 +54,116 @@
})
}
}
+
+func makeIPNet(cidr string) *net.IPNet {
+ _, n, err := net.ParseCIDR(cidr)
+ if err != nil {
+ panic(err)
+ }
+ return n
+}
+
+type testRoute struct {
+ dest string
+ via string
+ expected bool
+}
+
+func (t testRoute) toRealRoute() *dhcpv4.Route {
+ ip, n, err := net.ParseCIDR(t.dest)
+ if err != nil {
+ panic(err)
+ }
+ if !ip.Equal(n.IP) {
+ panic("testRoute is not aligned to route boundary")
+ }
+ routerIP := net.ParseIP(t.via)
+ if routerIP == nil && t.via != "" {
+ panic("routerIP is not valid")
+ }
+ return &dhcpv4.Route{
+ Dest: n,
+ Router: routerIP,
+ }
+}
+
+func TestSanitizeRoutes(t *testing.T) {
+ var tests = []struct {
+ name string
+ assignedNet *net.IPNet
+ routes []testRoute
+ }{{
+ name: "SimpleAdditionalRoute",
+ assignedNet: makeIPNet("10.0.5.0/24"),
+ routes: []testRoute{
+ {"10.0.7.0/24", "10.0.5.1", true},
+ },
+ }, {
+ name: "OutOfNetworkGateway",
+ assignedNet: makeIPNet("10.5.0.2/32"),
+ routes: []testRoute{
+ {"10.0.7.1/32", "", true},
+ {"0.0.0.0/0", "10.0.7.1", true},
+ },
+ }, {
+ name: "InvalidRouter",
+ assignedNet: makeIPNet("10.0.5.0/24"),
+ routes: []testRoute{
+ // Router is localhost
+ {"10.0.7.0/24", "127.0.0.1", false},
+ // Router is unreachable
+ {"10.0.8.0/24", "10.254.0.1", false},
+ },
+ }, {
+ name: "SameDestinationRoutes",
+ assignedNet: makeIPNet("10.0.5.0/24"),
+ routes: []testRoute{
+ {"0.0.0.0/0", "10.0.5.1", true},
+ {"10.0.7.0/24", "10.0.5.1", true},
+ {"0.0.0.0/0", "10.0.7.1", false},
+ },
+ }, {
+ name: "RoutesShadowingLoopback",
+ assignedNet: makeIPNet("10.0.5.0/24"),
+ routes: []testRoute{
+ // Default route, technically covers 127.0.0.0/8, but less-specific
+ {"0.0.0.0/0", "10.0.5.1", true},
+ // 127.0.0.0/8 is still more-specific
+ {"126.0.0.0/7", "10.0.5.1", true},
+ // Duplicate of 127.0.0.0/8, behavior undefined, disallowed
+ {"127.0.0.0/8", "10.0.5.1", false},
+ // Shadows localhost, disallowed
+ {"127.0.0.1/32", "10.0.5.1", false},
+ },
+ }}
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ var routes []*dhcpv4.Route
+ var expectedRoutes []*dhcpv4.Route
+ for _, r := range test.routes {
+ routes = append(routes, r.toRealRoute())
+ if r.expected {
+ expectedRoutes = append(expectedRoutes, r.toRealRoute())
+ }
+ }
+ out := sanitizeRoutes(routes, test.assignedNet)
+ if len(out) != len(expectedRoutes) {
+ t.Errorf("expected %d routes, got %d", len(expectedRoutes), len(out))
+ t.Error("Expected:")
+ for _, r := range expectedRoutes {
+ t.Errorf("\t%s via %s", r.Dest, r.Router)
+ }
+ t.Error("Actual:")
+ for _, r := range out {
+ t.Errorf("\t%s via %s", r.Dest, r.Router)
+ }
+ return
+ }
+ for i, r := range expectedRoutes {
+ if !out[i].Router.Equal(r.Router) || !out[i].Dest.IP.Equal(r.Dest.IP) || !bytes.Equal(out[i].Dest.Mask, r.Dest.Mask) {
+ t.Errorf("expected %s via %s, got %s via %s", r.Dest, r.Router, out[i].Dest, out[i].Router)
+ }
+ }
+ })
+ }
+}
diff --git a/metropolis/node/core/network/main.go b/metropolis/node/core/network/main.go
index 6940da6..aa7bd84 100644
--- a/metropolis/node/core/network/main.go
+++ b/metropolis/node/core/network/main.go
@@ -152,7 +152,7 @@
}
s.dhcp.VendorClassIdentifier = "dev.monogon.metropolis.node.v1"
s.dhcp.RequestedOptions = []dhcpv4.OptionCode{dhcpv4.OptionRouter, dhcpv4.OptionNameServer}
- s.dhcp.LeaseCallback = dhcpcb.Compose(dhcpcb.ManageIP(iface), dhcpcb.ManageDefaultRoute(iface), s.statusCallback)
+ s.dhcp.LeaseCallback = dhcpcb.Compose(dhcpcb.ManageIP(iface), dhcpcb.ManageRoutes(iface), s.statusCallback)
err = supervisor.Run(ctx, "dhcp", s.dhcp.Run)
if err != nil {
return err
diff --git a/metropolis/test/nanoswitch/nanoswitch.go b/metropolis/test/nanoswitch/nanoswitch.go
index e44fc80..6d0b6bb 100644
--- a/metropolis/test/nanoswitch/nanoswitch.go
+++ b/metropolis/test/nanoswitch/nanoswitch.go
@@ -299,7 +299,7 @@
logger.Fatalf("Failed to create DHCP client: %v", err)
}
dhcpClient.RequestedOptions = []dhcpv4.OptionCode{dhcpv4.OptionRouter}
- dhcpClient.LeaseCallback = dhcpcb.Compose(dhcpcb.ManageIP(externalLink), dhcpcb.ManageDefaultRoute(externalLink))
+ dhcpClient.LeaseCallback = dhcpcb.Compose(dhcpcb.ManageIP(externalLink), dhcpcb.ManageRoutes(externalLink))
supervisor.Run(ctx, "dhcp-client", dhcpClient.Run)
if err := os.WriteFile("/proc/sys/net/ipv4/ip_forward", []byte("1\n"), 0644); err != nil {
logger.Fatalf("Failed to write ip forwards: %v", err)