From fe0c57fc09c20c4e08c350393d94cb5f72767aa3 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Fri, 27 Sep 2024 17:11:50 +0200 Subject: [PATCH 1/6] dhcp4: fix drop-lease-and-restart logic The code should immediately attempt obtaining a lease from scratch instead of remaining stuck in the wait-until-renew loop. --- cmd/dhcp4/dhcp4.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/dhcp4/dhcp4.go b/cmd/dhcp4/dhcp4.go index 67d3dc6..4f3436e 100644 --- a/cmd/dhcp4/dhcp4.go +++ b/cmd/dhcp4/dhcp4.go @@ -187,6 +187,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") From ed7523c3111ab098fe3eaeef710d7167b8b7abbf Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 12 Dec 2024 08:24:06 +0100 Subject: [PATCH 2/6] build(deps): bump golang.org/x/crypto from 0.21.0 to 0.31.0 (#88) Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.21.0 to 0.31.0. - [Commits](https://github.com/golang/crypto/compare/v0.21.0...v0.31.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 4 ++-- go.sum | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 933c1ff..1ca4fef 100644 --- a/go.mod +++ b/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 ) diff --git a/go.sum b/go.sum index 54d7fab..1d1087d 100644 --- a/go.sum +++ b/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.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= +golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U= +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.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= +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.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= +golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q= +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= From af27264a0323c7367966cd4b39772fd41431c4c4 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sat, 21 Dec 2024 16:07:56 +0100 Subject: [PATCH 3/6] dhcp4: drop expired lease on server error (faster time to recovery) netconfigd still keeps the address configured for as long as possible, but dhcp4 now more quickly returns to a from-scratch DHCP exchange. --- cmd/dhcp4/dhcp4.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/cmd/dhcp4/dhcp4.go b/cmd/dhcp4/dhcp4.go index 4f3436e..7235fbf 100644 --- a/cmd/dhcp4/dhcp4.go +++ b/cmd/dhcp4/dhcp4.go @@ -35,6 +35,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" @@ -133,15 +134,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 { From 07325dde93e5a788597cedbfa3a7a60f87c6a717 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sun, 12 Jan 2025 10:29:42 +0100 Subject: [PATCH 4/6] netconfigd: do not hardcode 10.0.0.0/24 netmask for hairpinning related to https://github.com/rtr7/router7/issues/53 --- integration/netconfig/netconfig_test.go | 8 ++++---- internal/netconfig/netconfig.go | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/integration/netconfig/netconfig_test.go b/integration/netconfig/netconfig_test.go index 4a630cb..08a4e64 100644 --- a/integration/netconfig/netconfig_test.go +++ b/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 != 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 != 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 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 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 udp dport 53 dnat to 192.168.42.99:53 } chain prerouting { diff --git a/internal/netconfig/netconfig.go b/internal/netconfig/netconfig.go index a73cb86..860492c 100644 --- a/internal/netconfig/netconfig.go +++ b/internal/netconfig/netconfig.go @@ -583,7 +583,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{ @@ -626,7 +626,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 ] @@ -644,7 +646,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{ @@ -671,7 +673,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 ] @@ -781,6 +783,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 @@ -805,7 +816,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), }) } } From 0f75b1cbef6d0ec4853a6a02d96d4b57ce478769 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sun, 26 Jan 2025 10:16:38 +0100 Subject: [PATCH 5/6] netconfigd: write /tmp/resolv.conf only once, do not clobber This fixes tailscale name resolution breaking again and again. --- internal/netconfig/netconfig.go | 38 +++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/internal/netconfig/netconfig.go b/internal/netconfig/netconfig.go index 860492c..9488dd0 100644 --- a/internal/netconfig/netconfig.go +++ b/internal/netconfig/netconfig.go @@ -30,7 +30,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" @@ -436,6 +435,34 @@ 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). + if _, err := os.Stat(fn); err == nil { + return nil // 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) @@ -527,12 +554,9 @@ func applyInterfaces(dir, root string, cfg InterfaceConfig) error { } if details.Name == "lan0" { - b := []byte("nameserver " + addr.IP.String() + "\n") - fn := filepath.Join(root, "tmp", "resolv.conf") - if err := os.Remove(fn); err != nil && !os.IsNotExist(err) { - return err - } - if err := renameio.WriteFile(fn, b, 0644); err != nil { + // Use dnsd for the system's own DNS resolution. + resolvConf := "nameserver " + addr.IP.String() + "\n" + if err := createResolvConfIfMissing(root, resolvConf); err != nil { return err } } From 13e1c1bbb47bbd04bd8dbda646ee213a15b18fe4 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 27 Jan 2025 08:26:03 +0100 Subject: [PATCH 6/6] netconfig: move /tmp/resolv.conf symlink out of the way Commit 0f75b1cbef6d0ec4853a6a02d96d4b57ce478769 was incomplete. --- internal/netconfig/netconfig.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/netconfig/netconfig.go b/internal/netconfig/netconfig.go index 9488dd0..7675453 100644 --- a/internal/netconfig/netconfig.go +++ b/internal/netconfig/netconfig.go @@ -441,8 +441,17 @@ func createResolvConfIfMissing(root, contents string) error { // Explicitly check for the file's existance // just so that we can avoid printing an error // in the normal case (file exists). - if _, err := os.Stat(fn); err == nil { - return nil // file already exists, do not overwrite + 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 }