Replace temporary DHCP client with dhcp4c

This replaces our temporary DHCP client with the new one. The old GetIP()
interface is still preserved temporarily and will be ripped out in another revision
stacked on top of this one. nanoswitch also got some updates to support renewals which
it previously didn't have to do. This does leave the hacky channel system in place, supervisor observables are still in the design phase.

Test Plan: E2E tests still pass

X-Origin-Diff: phab/D656
GitOrigin-RevId: cc2f11e3989f4dbc6814fcfa22f6be81d7f88460
diff --git a/core/internal/network/BUILD.bazel b/core/internal/network/BUILD.bazel
index af82b8b..e0530d5 100644
--- a/core/internal/network/BUILD.bazel
+++ b/core/internal/network/BUILD.bazel
@@ -7,11 +7,13 @@
     visibility = ["//:__subpackages__"],
     deps = [
         "//core/internal/common/supervisor:go_default_library",
-        "//core/internal/network/dhcp:go_default_library",
         "//core/internal/network/dns:go_default_library",
+        "//core/pkg/dhcp4c:go_default_library",
+        "//core/pkg/dhcp4c/callback:go_default_library",
         "//core/pkg/logtree:go_default_library",
         "@com_github_google_nftables//:go_default_library",
         "@com_github_google_nftables//expr:go_default_library",
+        "@com_github_insomniacslk_dhcp//dhcpv4:go_default_library",
         "@com_github_vishvananda_netlink//:go_default_library",
         "@org_golang_x_sys//unix:go_default_library",
     ],
diff --git a/core/internal/network/dhcp/BUILD.bazel b/core/internal/network/dhcp/BUILD.bazel
deleted file mode 100644
index 8962e28..0000000
--- a/core/internal/network/dhcp/BUILD.bazel
+++ /dev/null
@@ -1,14 +0,0 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
-
-go_library(
-    name = "go_default_library",
-    srcs = ["dhcp.go"],
-    importpath = "git.monogon.dev/source/nexantic.git/core/internal/network/dhcp",
-    visibility = ["//core:__subpackages__"],
-    deps = [
-        "//core/internal/common/supervisor:go_default_library",
-        "@com_github_insomniacslk_dhcp//dhcpv4:go_default_library",
-        "@com_github_insomniacslk_dhcp//dhcpv4/nclient4:go_default_library",
-        "@com_github_vishvananda_netlink//:go_default_library",
-    ],
-)
diff --git a/core/internal/network/dhcp/dhcp.go b/core/internal/network/dhcp/dhcp.go
deleted file mode 100644
index 9a62c7c..0000000
--- a/core/internal/network/dhcp/dhcp.go
+++ /dev/null
@@ -1,167 +0,0 @@
-// Copyright 2020 The Monogon Project Authors.
-//
-// SPDX-License-Identifier: Apache-2.0
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package dhcp
-
-import (
-	"context"
-	"fmt"
-	"net"
-
-	"git.monogon.dev/source/nexantic.git/core/internal/common/supervisor"
-
-	"github.com/insomniacslk/dhcp/dhcpv4"
-	"github.com/insomniacslk/dhcp/dhcpv4/nclient4"
-	"github.com/vishvananda/netlink"
-)
-
-type Client struct {
-	reqC chan *dhcpStatusReq
-}
-
-func New() *Client {
-	return &Client{
-		reqC: make(chan *dhcpStatusReq),
-	}
-}
-
-type dhcpStatusReq struct {
-	resC chan *Status
-	wait bool
-}
-
-func (r *dhcpStatusReq) fulfill(s *Status) {
-	go func() {
-		r.resC <- s
-	}()
-}
-
-// Status is the IPv4 configuration provisioned via DHCP for a given interface. It does not necessarily represent
-// a configuration that is active or even valid.
-type Status struct {
-	// Address is 'our' (the node's) IPv4 Address on the network.
-	Address net.IPNet
-	// Gateway is the default Gateway/router of this network, or 0.0.0.0 if none was given.
-	Gateway net.IP
-	// DNS is a list of IPv4 DNS servers to use.
-	DNS []net.IP
-}
-
-func (s *Status) String() string {
-	return fmt.Sprintf("Address: %s, Gateway: %s, DNS: %v", s.Address.String(), s.Gateway.String(), s.DNS)
-}
-
-func (c *Client) Run(iface netlink.Link) supervisor.Runnable {
-	return func(ctx context.Context) error {
-		logger := supervisor.Logger(ctx)
-
-		// Channel updated with Address once one gets assigned/updated
-		newC := make(chan *Status)
-		// Status requests waiting for configuration
-		waiters := []*dhcpStatusReq{}
-
-		// Start lease acquisition
-		// TODO(q3k): actually maintain the lease instead of hoping we never get
-		// kicked off.
-
-		client, err := nclient4.New(iface.Attrs().Name)
-		if err != nil {
-			return fmt.Errorf("nclient4.New: %w", err)
-		}
-
-		err = supervisor.Run(ctx, "client", func(ctx context.Context) error {
-			supervisor.Signal(ctx, supervisor.SignalHealthy)
-			lease, err := client.Request(ctx)
-			if err != nil {
-				// TODO(q3k): implement retry logic with full state machine
-				logger.Errorf("DHCP lease request failed: %v", err)
-				return err
-			}
-			newC <- parseAck(lease.ACK)
-			supervisor.Signal(ctx, supervisor.SignalDone)
-			return nil
-		})
-		if err != nil {
-			return err
-		}
-
-		supervisor.Signal(ctx, supervisor.SignalHealthy)
-
-		// State machine
-		// Two implicit states: WAITING -> ASSIGNED
-		// We start at WAITING, once we get a current config we move to ASSIGNED
-		// Once this becomes more complex (ie. has to handle link state changes)
-		// this should grow into a real state machine.
-		var current *Status
-		logger.Info("DHCP client WAITING")
-		for {
-			select {
-			case <-ctx.Done():
-				// TODO(q3k): don't leave waiters hanging
-				return err
-
-			case cfg := <-newC:
-				current = cfg
-				logger.Infof("DHCP client ASSIGNED IP %s", current.String())
-				for _, w := range waiters {
-					w.fulfill(current)
-				}
-				waiters = []*dhcpStatusReq{}
-
-			case r := <-c.reqC:
-				if current != nil || !r.wait {
-					r.fulfill(current)
-				} else {
-					waiters = append(waiters, r)
-				}
-			}
-		}
-	}
-}
-
-// parseAck turns an internal Status (from the dhcpv4 library) into a Status
-func parseAck(ack *dhcpv4.DHCPv4) *Status {
-	address := net.IPNet{IP: ack.YourIPAddr, Mask: ack.SubnetMask()}
-
-	// DHCP routers are optional - if none are provided, assume no router and set Gateway to 0.0.0.0
-	// (this makes Gateway.IsUnspecified() == true)
-	gateway, _, _ := net.ParseCIDR("0.0.0.0/0")
-	if routers := ack.Router(); len(routers) > 0 {
-		gateway = routers[0]
-	}
-	return &Status{
-		Address: address,
-		Gateway: gateway,
-		DNS:     ack.DNS(),
-	}
-}
-
-// Status returns the DHCP configuration requested from us by the local DHCP server.
-// If wait is true, this function will block until a DHCP configuration is available. Otherwise, a nil Status may be
-// returned to indicate that no configuration has been received yet.
-func (c *Client) Status(ctx context.Context, wait bool) (*Status, error) {
-	resC := make(chan *Status)
-	c.reqC <- &dhcpStatusReq{
-		resC: resC,
-		wait: wait,
-	}
-	select {
-	case <-ctx.Done():
-		return nil, ctx.Err()
-	case r := <-resC:
-		return r, nil
-	}
-}
diff --git a/core/internal/network/main.go b/core/internal/network/main.go
index 414d971..e0bac79 100644
--- a/core/internal/network/main.go
+++ b/core/internal/network/main.go
@@ -18,20 +18,24 @@
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
 	"os"
+	"sync"
+	"time"
 
 	"github.com/google/nftables"
 	"github.com/google/nftables/expr"
-
+	"github.com/insomniacslk/dhcp/dhcpv4"
 	"github.com/vishvananda/netlink"
 	"golang.org/x/sys/unix"
 
 	"git.monogon.dev/source/nexantic.git/core/internal/common/supervisor"
-	"git.monogon.dev/source/nexantic.git/core/internal/network/dhcp"
 	"git.monogon.dev/source/nexantic.git/core/internal/network/dns"
+	"git.monogon.dev/source/nexantic.git/core/pkg/dhcp4c"
+	dhcpcb "git.monogon.dev/source/nexantic.git/core/pkg/dhcp4c/callback"
 	"git.monogon.dev/source/nexantic.git/core/pkg/logtree"
 )
 
@@ -42,7 +46,16 @@
 
 type Service struct {
 	config Config
-	dhcp   *dhcp.Client
+	dhcp   *dhcp4c.Client
+
+	// nftConn is a shared file descriptor handle to nftables, automatically initialized on first use.
+	nftConn             nftables.Conn
+	natTable            *nftables.Table
+	natPostroutingChain *nftables.Chain
+
+	// These are a temporary hack pending the removal of the GetIP interface
+	ipLock       sync.Mutex
+	currentIPTmp net.IP
 
 	logger logtree.LeveledLogger
 }
@@ -54,7 +67,6 @@
 func New(config Config) *Service {
 	return &Service{
 		config: config,
-		dhcp:   dhcp.New(),
 	}
 }
 
@@ -81,27 +93,6 @@
 	return unix.Rename(resolvConfSwapPath, resolvConfPath)
 }
 
-func (s *Service) addNetworkRoutes(link netlink.Link, addr net.IPNet, gw net.IP) error {
-	if err := netlink.AddrReplace(link, &netlink.Addr{IPNet: &addr}); err != nil {
-		return fmt.Errorf("failed to add DHCP address to network interface \"%v\": %w", link.Attrs().Name, err)
-	}
-
-	if gw.IsUnspecified() {
-		s.logger.Infof("No default route set, only local network %s will be reachable", addr.String())
-		return nil
-	}
-
-	route := &netlink.Route{
-		Dst:   &net.IPNet{IP: net.IPv4(0, 0, 0, 0), Mask: net.IPv4Mask(0, 0, 0, 0)},
-		Gw:    gw,
-		Scope: netlink.SCOPE_UNIVERSE,
-	}
-	if err := netlink.RouteAdd(route); err != nil {
-		return fmt.Errorf("could not add default route: netlink.RouteAdd(%+v): %v", route, err)
-	}
-	return nil
-}
-
 // nfifname converts an interface name into 16 bytes padded with zeroes (for nftables)
 func nfifname(n string) []byte {
 	b := make([]byte, 16)
@@ -109,42 +100,48 @@
 	return b
 }
 
+func (s *Service) dhcpDNSCallback(old, new *dhcp4c.Lease) error {
+	oldServers := old.DNSServers()
+	newServers := new.DNSServers()
+	if newServers.Equal(oldServers) {
+		return nil // nothing to do
+	}
+	s.logger.Infof("Setting upstream DNS servers to %v", newServers)
+	s.config.CorednsRegistrationChan <- dns.NewUpstreamDirective(newServers)
+	return nil
+}
+
+// TODO(lorenz): Get rid of this once we have robust node resolution
+func (s *Service) getIPCallbackHack(old, new *dhcp4c.Lease) error {
+	if old == nil && new != nil {
+		s.ipLock.Lock()
+		s.currentIPTmp = new.AssignedIP
+		s.ipLock.Unlock()
+	}
+	return nil
+}
+
 func (s *Service) useInterface(ctx context.Context, iface netlink.Link) error {
-	err := supervisor.Run(ctx, "dhcp", s.dhcp.Run(iface))
+	netIface, err := net.InterfaceByIndex(iface.Attrs().Index)
+	if err != nil {
+		return fmt.Errorf("cannot create Go net.Interface from netlink.Link: %w", err)
+	}
+	s.dhcp, err = dhcp4c.NewClient(netIface)
+	if err != nil {
+		return fmt.Errorf("failed to create DHCP client on interface %v: %w", iface.Attrs().Name, err)
+	}
+	s.dhcp.VendorClassIdentifier = "com.nexantic.smalltown.v1"
+	s.dhcp.RequestedOptions = []dhcpv4.OptionCode{dhcpv4.OptionRouter, dhcpv4.OptionNameServer}
+	s.dhcp.LeaseCallback = dhcpcb.Compose(dhcpcb.ManageIP(iface), dhcpcb.ManageDefaultRoute(iface), s.dhcpDNSCallback, s.getIPCallbackHack)
+	err = supervisor.Run(ctx, "dhcp", s.dhcp.Run)
 	if err != nil {
 		return err
 	}
-	status, err := s.dhcp.Status(ctx, true)
-	if err != nil {
-		return fmt.Errorf("could not get DHCP Status: %w", err)
-	}
-
-	// We're currently never removing this directive just like we're not removing routes and IPs
-	s.config.CorednsRegistrationChan <- dns.NewUpstreamDirective(status.DNS)
-
-	if err := s.addNetworkRoutes(iface, status.Address, status.Gateway); err != nil {
-		s.logger.Warning("Failed to add routes: %v", err)
-	}
-
-	c := nftables.Conn{}
-
-	nat := c.AddTable(&nftables.Table{
-		Family: nftables.TableFamilyIPv4,
-		Name:   "nat",
-	})
-
-	postrouting := c.AddChain(&nftables.Chain{
-		Name:     "postrouting",
-		Hooknum:  nftables.ChainHookPostrouting,
-		Priority: nftables.ChainPriorityNATSource,
-		Table:    nat,
-		Type:     nftables.ChainTypeNAT,
-	})
 
 	// Masquerade/SNAT all traffic going out of the external interface
-	c.AddRule(&nftables.Rule{
-		Table: nat,
-		Chain: postrouting,
+	s.nftConn.AddRule(&nftables.Rule{
+		Table: s.natTable,
+		Chain: s.natPostroutingChain,
 		Exprs: []expr.Any{
 			&expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1},
 			&expr.Cmp{
@@ -156,7 +153,7 @@
 		},
 	})
 
-	if err := c.Flush(); err != nil {
+	if err := s.nftConn.Flush(); err != nil {
 		panic(err)
 	}
 
@@ -165,11 +162,24 @@
 
 // GetIP returns the current IP (and optionally waits for one to be assigned)
 func (s *Service) GetIP(ctx context.Context, wait bool) (*net.IP, error) {
-	status, err := s.dhcp.Status(ctx, wait)
-	if err != nil {
-		return nil, err
+	for {
+		var currentIP net.IP
+		s.ipLock.Lock()
+		currentIP = s.currentIPTmp
+		s.ipLock.Unlock()
+		if currentIP == nil {
+			if !wait {
+				return nil, errors.New("no IP available")
+			}
+			select {
+			case <-ctx.Done():
+				return nil, ctx.Err()
+			case <-time.After(1 * time.Second):
+				continue
+			}
+		}
+		return &currentIP, nil
 	}
-	return &status.Address.IP, nil
 }
 
 func (s *Service) Run(ctx context.Context) error {
@@ -178,6 +188,22 @@
 	supervisor.Run(ctx, "dns", dnsSvc.Run)
 	supervisor.Run(ctx, "interfaces", s.runInterfaces)
 
+	s.natTable = s.nftConn.AddTable(&nftables.Table{
+		Family: nftables.TableFamilyIPv4,
+		Name:   "nat",
+	})
+
+	s.natPostroutingChain = s.nftConn.AddChain(&nftables.Chain{
+		Name:     "postrouting",
+		Hooknum:  nftables.ChainHookPostrouting,
+		Priority: nftables.ChainPriorityNATSource,
+		Table:    s.natTable,
+		Type:     nftables.ChainTypeNAT,
+	})
+	if err := s.nftConn.Flush(); err != nil {
+		logger.Fatalf("Failed to set up nftables base chains: %v", err)
+	}
+
 	if err := ioutil.WriteFile("/proc/sys/net/ipv4/ip_forward", []byte("1\n"), 0644); err != nil {
 		logger.Fatalf("Failed to enable IPv4 forwarding: %v", err)
 	}