Merge remote-tracking branch 'github/master'

cmd/dhcp4/dhcp4.go

@@ -37,6 +37,7 @@ import (
 	"github.com/google/gopacket/layers"
 	"github.com/google/renameio"
 	"github.com/jpillora/backoff"
+	rtr7dhcp4 "github.com/rtr7/dhcp4"
 	"github.com/rtr7/router7/internal/dhcp4"
 	"github.com/rtr7/router7/internal/netconfig"
 	"github.com/rtr7/router7/internal/notify"
@@ -136,15 +137,36 @@ func logic() error {
 		Min:    10 * time.Second,
 		Max:    1 * time.Minute,
 	}
+	var lastSuccess time.Time
+	if st, err := os.Stat(ackFn); err == nil {
+		lastSuccess = st.ModTime()
+	}
+	log.Printf("last success: %v", lastSuccess)
 ObtainOrRenew:
 	for c.ObtainOrRenew() {
 		if err := c.Err(); err != nil {
 			dur := backoff.Duration()
+			// Drop the lease if we do not get a reply from the DHCP server.
+			// I observed this in practice where over a period of days,
+			// the dhcp4 client would hang like this:
+			//
+			// dhcp4.go:140: Temporary error: DHCP: read packet
+			// 42:66:f1:f1:bd:e7: i/o timeout (waiting 1m0s)
+			//
+			// For brief periods of time, we probably want to paper over such
+			// issues, but after the lease expired, we should start the DHCP
+			// exchange from scratch.
+			if c.Ack != nil && time.Since(lastSuccess) > rtr7dhcp4.LeaseFromACK(c.Ack).RenewalTime {
+				log.Printf("Temporary error: %v (dropping lease and retrying)", err)
+				c.Ack = nil
+				continue
+			}
 			log.Printf("Temporary error: %v (waiting %v)", err, dur)
 			time.Sleep(dur)
 			continue
 		}
 		backoff.Reset()
+		lastSuccess = time.Now()
 		log.Printf("lease: %+v", c.Config())
 		b, err := json.Marshal(c.Config())
 		if err != nil {
@@ -190,6 +212,7 @@ ObtainOrRenew:
 				// Still not healthy? Drop DHCP lease and start from scratch.
 				log.Printf("unhealthy for 5 cycles, starting over without lease")
 				c.Ack = nil
+				continue ObtainOrRenew
 
 			case <-usr2:
 				log.Printf("SIGUSR2 received, sending DHCPRELEASE")

go.mod

@@ -26,10 +26,10 @@ require (
 	github.com/rtr7/dhcp4 v0.0.0-20220302171438-18c84d089b46
 	github.com/vishvananda/netlink v1.2.1-beta.2
 	github.com/vishvananda/netns v0.0.4
-	golang.org/x/crypto v0.21.0
+	golang.org/x/crypto v0.31.0
 	golang.org/x/net v0.23.0
 	golang.org/x/sync v0.7.0
-	golang.org/x/sys v0.19.0
+	golang.org/x/sys v0.28.0
 	golang.org/x/time v0.0.0-20220722155302-e5dcc9cfc0b9
 	golang.zx2c4.com/wireguard/wgctrl v0.0.0-20220504211119-3d4a969bb56b
 )

go.sum

@@ -131,8 +131,8 @@ gitlab.com/golang-commonmark/puny v0.0.0-20191124015043-9f83538fa04f/go.mod h1:T
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
-golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA=
+golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
-golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
+golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
 golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY=
 golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
@@ -183,11 +183,11 @@ golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210525143221-35b2ab0089ea/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
+golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
-golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
-golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8=
+golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q=
-golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58=
+golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=

integration/netconfig/netconfig_test.go

@@ -148,13 +148,13 @@ func goldenNftablesRules(additionalForwarding bool) string {
 	add := ""
 	if additionalForwarding {
 		add = `
-		ip daddr != 127.0.0.0/8 ip daddr != 10.0.0.0/24 fib daddr type 2 tcp dport 8045 dnat to 192.168.42.22:8045`
+		ip daddr != 127.0.0.0/8 ip daddr != 192.168.42.0/24 fib daddr type 2 tcp dport 8045 dnat to 192.168.42.22:8045`
 	}
 	return `table ip nat {
 	chain router7-portforwardings {
-		ip daddr != 127.0.0.0/8 ip daddr != 10.0.0.0/24 fib daddr type 2 tcp dport 8080 dnat to 192.168.42.23:9999` + add + `
+		ip daddr != 127.0.0.0/8 ip daddr != 192.168.42.0/24 fib daddr type 2 tcp dport 8080 dnat to 192.168.42.23:9999` + add + `
-		ip daddr != 127.0.0.0/8 ip daddr != 10.0.0.0/24 fib daddr type 2 tcp dport 8040-8060 dnat to 192.168.42.99:8040-8060
+		ip daddr != 127.0.0.0/8 ip daddr != 192.168.42.0/24 fib daddr type 2 tcp dport 8040-8060 dnat to 192.168.42.99:8040-8060
-		ip daddr != 127.0.0.0/8 ip daddr != 10.0.0.0/24 fib daddr type 2 udp dport 53 dnat to 192.168.42.99:53
+		ip daddr != 127.0.0.0/8 ip daddr != 192.168.42.0/24 fib daddr type 2 udp dport 53 dnat to 192.168.42.99:53
 	}
 
 	chain prerouting {

internal/netconfig/netconfig.go

@@ -32,7 +32,6 @@ import (
 	"github.com/google/nftables"
 	"github.com/google/nftables/binaryutil"
 	"github.com/google/nftables/expr"
-	"github.com/google/renameio"
 	"github.com/mdlayher/ethtool"
 	"github.com/vishvananda/netlink"
 	"golang.org/x/sys/unix"
@@ -440,6 +439,43 @@ func applyInterfaceFEC(details InterfaceDetails) error {
 	return nil
 }
 
+func createResolvConfIfMissing(root, contents string) error {
+	fn := filepath.Join(root, "tmp", "resolv.conf")
+
+	// Explicitly check for the file's existance
+	// just so that we can avoid printing an error
+	// in the normal case (file exists).
+	st, err := os.Lstat(fn)
+	if err == nil {
+		if st.Mode()&os.ModeSymlink != 0 {
+			// File is a symbolic link (at boot, gokrazy links /tmp/resolv.conf to /proc/net/pnp).
+			// Delete the link and fallthrough to create the file.
+			if err := os.Remove(fn); err != nil {
+				return err
+			}
+		} else {
+			return nil // regular file already exists, do not overwrite
+		}
+	} else if !os.IsNotExist(err) {
+		return err // unexpected error
+	}
+
+	// /tmp/resolv.conf does not exist yet, create it.
+
+	// This is os.WriteFile, but with O_EXCL set
+	// so that we do not accidentally clobber the file
+	// in case another process (e.g. tailscaled) just wrote it.
+	f, err := os.OpenFile(fn, os.O_WRONLY|os.O_CREATE|os.O_TRUNC|os.O_EXCL, 0644)
+	if err != nil {
+		return err
+	}
+	_, err = f.Write([]byte(contents))
+	if err1 := f.Close(); err1 != nil && err == nil {
+		err = err1
+	}
+	return err
+}
+
 func applyInterfaces(dir, root string, cfg InterfaceConfig) error {
 	byName := make(map[string]InterfaceDetails)
 	byHardwareAddr := make(map[string]InterfaceDetails)
@@ -531,12 +567,9 @@ func applyInterfaces(dir, root string, cfg InterfaceConfig) error {
 			}
 
 			if details.Name == "lan0" {
-				b := []byte("nameserver " + addr.IP.String() + "\n")
+				// Use dnsd for the system's own DNS resolution.
-				fn := filepath.Join(root, "tmp", "resolv.conf")
+				resolvConf := "nameserver " + addr.IP.String() + "\n"
-				if err := os.Remove(fn); err != nil && !os.IsNotExist(err) {
+				if err := createResolvConfIfMissing(root, resolvConf); err != nil {
-					return err
-				}
-				if err := renameio.WriteFile(fn, b, 0644); err != nil {
 					return err
 				}
 			}
@@ -587,7 +620,7 @@ func nfifname(n string) []byte {
 //
 // Instead, it uses “fib daddr type local” to match all locally-configured IP
 // addresses and then excludes the loopback and LAN IP addresses.
-func matchUplinkIP() []expr.Any {
+func matchUplinkIP(lan0ip net.IP) []expr.Any {
 	return []expr.Any{
 		// [ payload load 4b @ network header + 16 => reg 1 ]
 		&expr.Payload{
@@ -630,7 +663,9 @@ func matchUplinkIP() []expr.Any {
 		&expr.Cmp{
 			Op:       expr.CmpOpNeq,
 			Register: 1,
-			Data:     []byte{0x0a, 0x00, 0x00, 0x00},
+			// Turn the lan0 IP address (e.g. 192.168.42.1)
+			// into a netmask like 192.168.42.0/24.
+			Data: []byte{lan0ip[0], lan0ip[1], lan0ip[2], 0},
 		},
 
 		// [ fib daddr type => reg 1 ]
@@ -648,7 +683,7 @@ func matchUplinkIP() []expr.Any {
 	}
 }
 
-func portForwardExpr(ifname string, proto uint8, portMin, portMax uint16, dest net.IP, dportMin, dportMax uint16) []expr.Any {
+func portForwardExpr(lan0ip net.IP, proto uint8, portMin, portMax uint16, dest net.IP, dportMin, dportMax uint16) []expr.Any {
 	var cmp []expr.Any
 	if portMin == portMax {
 		cmp = []expr.Any{
@@ -675,7 +710,7 @@ func portForwardExpr(ifname string, proto uint8, portMin, portMax uint16, dest n
 			},
 		}
 	}
-	ex := append(matchUplinkIP(),
+	ex := append(matchUplinkIP(lan0ip),
 		// [ meta load l4proto => reg 1 ]
 		&expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1},
 		// [ cmp eq reg 1 0x00000006 ]
@@ -785,6 +820,15 @@ func applyPortForwardings(dir, ifname string, c *nftables.Conn, nat *nftables.Ta
 		return err
 	}
 
+	lan0ip, err := LinkAddress(dir, "lan0")
+	if err != nil {
+		return err
+	}
+	lan0ip = lan0ip.To4()
+	if got, want := len(lan0ip), net.IPv4len; got != want {
+		return fmt.Errorf("lan0 does not have an IPv4 address configured: len %d != %d", got, want)
+	}
+
 	for _, fw := range cfg.Forwardings {
 		for _, proto := range strings.Split(fw.Proto, ",") {
 			var p uint8
@@ -809,7 +853,7 @@ func applyPortForwardings(dir, ifname string, c *nftables.Conn, nat *nftables.Ta
 			c.AddRule(&nftables.Rule{
 				Table: nat,
 				Chain: prerouting,
-				Exprs: portForwardExpr(ifname, p, min, max, net.ParseIP(fw.DestAddr), dmin, dmax),
+				Exprs: portForwardExpr(lan0ip, p, min, max, net.ParseIP(fw.DestAddr), dmin, dmax),
 			})
 		}
 	}