ipn/{ipnlocal,localapi}: actually renew certs before expiry (#8731)

While our `shouldStartDomainRenewal` check is correct, `getCertPEM`
would always bail if the existing cert is not expired. Add the same
`shouldStartDomainRenewal` check to `getCertPEM` to make it proceed with
renewal when existing certs are still valid but should be renewed.

The extra check is expensive (ARI request towards LetsEncrypt), so cache
the last check result for 1hr to not degrade `tailscale serve`
performance.

Also, asynchronous renewal is great for `tailscale serve` but confusing
for `tailscale cert`. Add an explicit flag to `GetCertPEM` to force a
synchronous renewal for `tailscale cert`.

Fixes #8725

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
(cherry picked from commit c1ecae13ab)

VERSION.txt

@@ -1 +1 @@
-1.45.0
+1.46.1

cmd/derper/depaware.txt

@@ -77,20 +77,6 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         google.golang.org/protobuf/runtime/protoimpl                 from github.com/golang/protobuf/proto+
         google.golang.org/protobuf/types/descriptorpb                from google.golang.org/protobuf/reflect/protodesc
         google.golang.org/protobuf/types/known/timestamppb           from github.com/prometheus/client_golang/prometheus+
-   L    gvisor.dev/gvisor/pkg/abi                                    from gvisor.dev/gvisor/pkg/abi/linux
-   L 💣 gvisor.dev/gvisor/pkg/abi/linux                              from tailscale.com/util/linuxfw
-   L    gvisor.dev/gvisor/pkg/bits                                   from gvisor.dev/gvisor/pkg/abi/linux
-   L    gvisor.dev/gvisor/pkg/context                                from gvisor.dev/gvisor/pkg/abi/linux
-   L 💣 gvisor.dev/gvisor/pkg/gohacks                                from gvisor.dev/gvisor/pkg/abi/linux+
-   L 💣 gvisor.dev/gvisor/pkg/hostarch                               from gvisor.dev/gvisor/pkg/abi/linux+
-   L    gvisor.dev/gvisor/pkg/linewriter                             from gvisor.dev/gvisor/pkg/log
-   L    gvisor.dev/gvisor/pkg/log                                    from gvisor.dev/gvisor/pkg/context
-   L    gvisor.dev/gvisor/pkg/marshal                                from gvisor.dev/gvisor/pkg/abi/linux+
-   L 💣 gvisor.dev/gvisor/pkg/marshal/primitive                      from gvisor.dev/gvisor/pkg/abi/linux
-   L 💣 gvisor.dev/gvisor/pkg/state                                  from gvisor.dev/gvisor/pkg/abi/linux+
-   L    gvisor.dev/gvisor/pkg/state/wire                             from gvisor.dev/gvisor/pkg/state
-   L 💣 gvisor.dev/gvisor/pkg/sync                                   from gvisor.dev/gvisor/pkg/linewriter+
-   L    gvisor.dev/gvisor/pkg/waiter                                 from gvisor.dev/gvisor/pkg/context
         nhooyr.io/websocket                                          from tailscale.com/cmd/derper+
         nhooyr.io/websocket/internal/errd                            from nhooyr.io/websocket
         nhooyr.io/websocket/internal/xsync                           from nhooyr.io/websocket
@@ -156,7 +142,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         tailscale.com/util/dnsname                                   from tailscale.com/hostinfo+
         tailscale.com/util/httpm                                     from tailscale.com/client/tailscale
         tailscale.com/util/lineread                                  from tailscale.com/hostinfo+
-   L 💣 tailscale.com/util/linuxfw                                   from tailscale.com/net/netns
+   L    tailscale.com/util/linuxfw                                   from tailscale.com/net/netns
         tailscale.com/util/mak                                       from tailscale.com/syncs+
         tailscale.com/util/multierr                                  from tailscale.com/health+
         tailscale.com/util/set                                       from tailscale.com/health+

cmd/tailscale/depaware.txt

@@ -55,20 +55,6 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         go4.org/netipx                                               from tailscale.com/wgengine/filter
    W 💣 golang.zx2c4.com/wireguard/windows/tunnel/winipcfg           from tailscale.com/net/interfaces+
         gopkg.in/yaml.v2                                             from sigs.k8s.io/yaml
-   L    gvisor.dev/gvisor/pkg/abi                                    from gvisor.dev/gvisor/pkg/abi/linux
-   L 💣 gvisor.dev/gvisor/pkg/abi/linux                              from tailscale.com/util/linuxfw
-   L    gvisor.dev/gvisor/pkg/bits                                   from gvisor.dev/gvisor/pkg/abi/linux
-   L    gvisor.dev/gvisor/pkg/context                                from gvisor.dev/gvisor/pkg/abi/linux
-   L 💣 gvisor.dev/gvisor/pkg/gohacks                                from gvisor.dev/gvisor/pkg/abi/linux+
-   L 💣 gvisor.dev/gvisor/pkg/hostarch                               from gvisor.dev/gvisor/pkg/abi/linux+
-   L    gvisor.dev/gvisor/pkg/linewriter                             from gvisor.dev/gvisor/pkg/log
-   L    gvisor.dev/gvisor/pkg/log                                    from gvisor.dev/gvisor/pkg/context
-   L    gvisor.dev/gvisor/pkg/marshal                                from gvisor.dev/gvisor/pkg/abi/linux+
-   L 💣 gvisor.dev/gvisor/pkg/marshal/primitive                      from gvisor.dev/gvisor/pkg/abi/linux
-   L 💣 gvisor.dev/gvisor/pkg/state                                  from gvisor.dev/gvisor/pkg/abi/linux+
-   L    gvisor.dev/gvisor/pkg/state/wire                             from gvisor.dev/gvisor/pkg/state
-   L 💣 gvisor.dev/gvisor/pkg/sync                                   from gvisor.dev/gvisor/pkg/linewriter+
-   L    gvisor.dev/gvisor/pkg/waiter                                 from gvisor.dev/gvisor/pkg/context
         k8s.io/client-go/util/homedir                                from tailscale.com/cmd/tailscale/cli
         nhooyr.io/websocket                                          from tailscale.com/derp/derphttp+
         nhooyr.io/websocket/internal/errd                            from nhooyr.io/websocket
@@ -148,7 +134,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/util/groupmember                               from tailscale.com/cmd/tailscale/cli
         tailscale.com/util/httpm                                     from tailscale.com/client/tailscale
         tailscale.com/util/lineread                                  from tailscale.com/net/interfaces+
-   L 💣 tailscale.com/util/linuxfw                                   from tailscale.com/net/netns
+   L    tailscale.com/util/linuxfw                                   from tailscale.com/net/netns
         tailscale.com/util/mak                                       from tailscale.com/net/netcheck+
         tailscale.com/util/multierr                                  from tailscale.com/control/controlhttp+
         tailscale.com/util/must                                      from tailscale.com/cmd/tailscale/cli

cmd/tailscaled/depaware.txt

@@ -162,18 +162,13 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         go4.org/netipx                                               from tailscale.com/ipn/ipnlocal+
    W 💣 golang.zx2c4.com/wintun                                      from github.com/tailscale/wireguard-go/tun+
    W 💣 golang.zx2c4.com/wireguard/windows/tunnel/winipcfg           from tailscale.com/net/dns+
-   L    gvisor.dev/gvisor/pkg/abi                                    from gvisor.dev/gvisor/pkg/abi/linux
-   L 💣 gvisor.dev/gvisor/pkg/abi/linux                              from tailscale.com/util/linuxfw
         gvisor.dev/gvisor/pkg/atomicbitops                           from gvisor.dev/gvisor/pkg/tcpip+
-        gvisor.dev/gvisor/pkg/bits                                   from gvisor.dev/gvisor/pkg/bufferv2+
+        gvisor.dev/gvisor/pkg/bits                                   from gvisor.dev/gvisor/pkg/bufferv2
      💣 gvisor.dev/gvisor/pkg/bufferv2                               from gvisor.dev/gvisor/pkg/tcpip+
-        gvisor.dev/gvisor/pkg/context                                from gvisor.dev/gvisor/pkg/refs+
+        gvisor.dev/gvisor/pkg/context                                from gvisor.dev/gvisor/pkg/refs
      💣 gvisor.dev/gvisor/pkg/gohacks                                from gvisor.dev/gvisor/pkg/state/wire+
-   L 💣 gvisor.dev/gvisor/pkg/hostarch                               from gvisor.dev/gvisor/pkg/abi/linux+
         gvisor.dev/gvisor/pkg/linewriter                             from gvisor.dev/gvisor/pkg/log
         gvisor.dev/gvisor/pkg/log                                    from gvisor.dev/gvisor/pkg/context+
-   L    gvisor.dev/gvisor/pkg/marshal                                from gvisor.dev/gvisor/pkg/abi/linux+
-   L 💣 gvisor.dev/gvisor/pkg/marshal/primitive                      from gvisor.dev/gvisor/pkg/abi/linux
         gvisor.dev/gvisor/pkg/rand                                   from gvisor.dev/gvisor/pkg/tcpip/network/hash+
         gvisor.dev/gvisor/pkg/refs                                   from gvisor.dev/gvisor/pkg/bufferv2+
      💣 gvisor.dev/gvisor/pkg/sleep                                  from gvisor.dev/gvisor/pkg/tcpip/transport/tcp
@@ -333,7 +328,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
      💣 tailscale.com/util/hashx                                     from tailscale.com/util/deephash
         tailscale.com/util/httpm                                     from tailscale.com/client/tailscale+
         tailscale.com/util/lineread                                  from tailscale.com/hostinfo+
-   L 💣 tailscale.com/util/linuxfw                                   from tailscale.com/net/netns+
+   L    tailscale.com/util/linuxfw                                   from tailscale.com/net/netns+
         tailscale.com/util/mak                                       from tailscale.com/control/controlclient+
         tailscale.com/util/multierr                                  from tailscale.com/control/controlclient+
         tailscale.com/util/must                                      from tailscale.com/logpolicy

cmd/tailscaled/tailscaled_test.go

@@ -3,10 +3,32 @@
 
 package main // import "tailscale.com/cmd/tailscaled"
 
-import "testing"
+import (
+	"testing"
+
+	"tailscale.com/tstest/deptest"
+)
 
 func TestNothing(t *testing.T) {
 	// This test does nothing on purpose, so we can run
 	// GODEBUG=memprofilerate=1 go test -v -run=Nothing -memprofile=prof.mem
 	// without any errors about no matching tests.
 }
+
+func TestDeps(t *testing.T) {
+	deptest.DepChecker{
+		GOOS:   "darwin",
+		GOARCH: "arm64",
+		BadDeps: map[string]string{
+			"gvisor.dev/gvisor/pkg/hostarch": "will crash on non-4K page sizes; see https://github.com/tailscale/tailscale/issues/8658",
+		},
+	}.Check(t)
+
+	deptest.DepChecker{
+		GOOS:   "linux",
+		GOARCH: "arm64",
+		BadDeps: map[string]string{
+			"gvisor.dev/gvisor/pkg/hostarch": "will crash on non-4K page sizes; see https://github.com/tailscale/tailscale/issues/8658",
+		},
+	}.Check(t)
+}

ipn/ipnlocal/cert.go

@@ -53,8 +53,8 @@ var (
 	// populate the on-disk cache and the rest should use that.
 	acmeMu sync.Mutex
 
-	renewMu        sync.Mutex // lock order: don't hold acmeMu and renewMu at the same time
-	lastRenewCheck = map[string]time.Time{}
+	renewMu     sync.Mutex // lock order: acmeMu before renewMu
+	renewCertAt = map[string]time.Time{}
 )
 
 // certDir returns (creating if needed) the directory in which cached
@@ -80,9 +80,15 @@ func (b *LocalBackend) certDir() (string, error) {
 
 var acmeDebug = envknob.RegisterBool("TS_DEBUG_ACME")
 
-// getCertPEM gets the KeyPair for domain, either from cache, via the ACME
-// process, or from cache and kicking off an async ACME renewal.
-func (b *LocalBackend) GetCertPEM(ctx context.Context, domain string) (*TLSCertKeyPair, error) {
+// GetCertPEM gets the TLSCertKeyPair for domain, either from cache or via the
+// ACME process. ACME process is used for new domain certs, existing expired
+// certs or existing certs that should get renewed due to upcoming expiry.
+//
+// syncRenewal changes renewal behavior for existing certs that are still valid
+// but need renewal. When syncRenewal is set, the method blocks until a new
+// cert is issued. When syncRenewal is not set, existing cert is returned right
+// away and renewal is kicked off in a background goroutine.
+func (b *LocalBackend) GetCertPEM(ctx context.Context, domain string, syncRenewal bool) (*TLSCertKeyPair, error) {
 	if !validLookingCertDomain(domain) {
 		return nil, errors.New("invalid domain")
 	}
@@ -105,12 +111,15 @@ func (b *LocalBackend) GetCertPEM(ctx context.Context, domain string) (*TLSCertK
 		shouldRenew, err := b.shouldStartDomainRenewal(cs, domain, now, pair)
 		if err != nil {
 			logf("error checking for certificate renewal: %v", err)
-		} else if shouldRenew {
+		} else if !shouldRenew {
+			return pair, nil
+		}
+		if !syncRenewal {
 			logf("starting async renewal")
 			// Start renewal in the background.
 			go b.getCertPEM(context.Background(), cs, logf, traceACME, domain, now)
 		}
-		return pair, nil
+		// Synchronous renewal happens below.
 	}
 
 	pair, err := b.getCertPEM(ctx, cs, logf, traceACME, domain, now)
@@ -124,37 +133,43 @@ func (b *LocalBackend) GetCertPEM(ctx context.Context, domain string) (*TLSCertK
 func (b *LocalBackend) shouldStartDomainRenewal(cs certStore, domain string, now time.Time, pair *TLSCertKeyPair) (bool, error) {
 	renewMu.Lock()
 	defer renewMu.Unlock()
-	if last, ok := lastRenewCheck[domain]; ok && now.Sub(last) < time.Minute {
-		// We checked very recently. Don't bother reparsing &
-		// validating the x509 cert.
-		return false, nil
+	if renewAt, ok := renewCertAt[domain]; ok {
+		return now.After(renewAt), nil
 	}
-	lastRenewCheck[domain] = now
 
-	renew, err := b.shouldStartDomainRenewalByARI(cs, now, pair)
+	renewTime, err := b.domainRenewalTimeByARI(cs, pair)
 	if err != nil {
 		// Log any ARI failure and fall back to checking for renewal by expiry.
 		b.logf("acme: ARI check failed: %v; falling back to expiry-based check", err)
-	} else {
-		return renew, nil
+		renewTime, err = b.domainRenewalTimeByExpiry(pair)
+		if err != nil {
+			return false, err
+		}
 	}
 
-	return b.shouldStartDomainRenewalByExpiry(now, pair)
+	renewCertAt[domain] = renewTime
+	return now.After(renewTime), nil
 }
 
-func (b *LocalBackend) shouldStartDomainRenewalByExpiry(now time.Time, pair *TLSCertKeyPair) (bool, error) {
+func (b *LocalBackend) domainRenewed(domain string) {
+	renewMu.Lock()
+	defer renewMu.Unlock()
+	delete(renewCertAt, domain)
+}
+
+func (b *LocalBackend) domainRenewalTimeByExpiry(pair *TLSCertKeyPair) (time.Time, error) {
 	block, _ := pem.Decode(pair.CertPEM)
 	if block == nil {
-		return false, fmt.Errorf("parsing certificate PEM")
+		return time.Time{}, fmt.Errorf("parsing certificate PEM")
 	}
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {
-		return false, fmt.Errorf("parsing certificate: %w", err)
+		return time.Time{}, fmt.Errorf("parsing certificate: %w", err)
 	}
 
 	certLifetime := cert.NotAfter.Sub(cert.NotBefore)
 	if certLifetime < 0 {
-		return false, fmt.Errorf("negative certificate lifetime %v", certLifetime)
+		return time.Time{}, fmt.Errorf("negative certificate lifetime %v", certLifetime)
 	}
 
 	// Per https://github.com/tailscale/tailscale/issues/8204, check
@@ -163,36 +178,32 @@ func (b *LocalBackend) shouldStartDomainRenewalByExpiry(now time.Time, pair *TLS
 	// Encrypt.
 	renewalDuration := certLifetime * 2 / 3
 	renewAt := cert.NotBefore.Add(renewalDuration)
-
-	if now.After(renewAt) {
-		return true, nil
-	}
-	return false, nil
+	return renewAt, nil
 }
 
-func (b *LocalBackend) shouldStartDomainRenewalByARI(cs certStore, now time.Time, pair *TLSCertKeyPair) (bool, error) {
+func (b *LocalBackend) domainRenewalTimeByARI(cs certStore, pair *TLSCertKeyPair) (time.Time, error) {
 	var blocks []*pem.Block
 	rest := pair.CertPEM
 	for len(rest) > 0 {
 		var block *pem.Block
 		block, rest = pem.Decode(rest)
 		if block == nil {
-			return false, fmt.Errorf("parsing certificate PEM")
+			return time.Time{}, fmt.Errorf("parsing certificate PEM")
 		}
 		blocks = append(blocks, block)
 	}
 	if len(blocks) < 2 {
-		return false, fmt.Errorf("could not parse certificate chain from certStore, got %d PEM block(s)", len(blocks))
+		return time.Time{}, fmt.Errorf("could not parse certificate chain from certStore, got %d PEM block(s)", len(blocks))
 	}
 	ac, err := acmeClient(cs)
 	if err != nil {
-		return false, err
+		return time.Time{}, err
 	}
 	ctx, cancel := context.WithTimeout(b.ctx, 5*time.Second)
 	defer cancel()
 	ri, err := ac.FetchRenewalInfo(ctx, blocks[0].Bytes, blocks[1].Bytes)
 	if err != nil {
-		return false, fmt.Errorf("failed to fetch renewal info from ACME server: %w", err)
+		return time.Time{}, fmt.Errorf("failed to fetch renewal info from ACME server: %w", err)
 	}
 	if acmeDebug() {
 		b.logf("acme: ARI response: %+v", ri)
@@ -203,7 +214,7 @@ func (b *LocalBackend) shouldStartDomainRenewalByARI(cs certStore, now time.Time
 	// https://datatracker.ietf.org/doc/draft-ietf-acme-ari/
 	start, end := ri.SuggestedWindow.Start, ri.SuggestedWindow.End
 	renewTime := start.Add(time.Duration(insecurerand.Int63n(int64(end.Sub(start)))))
-	return now.After(renewTime), nil
+	return renewTime, nil
 }
 
 // certStore provides a way to perist and retrieve TLS certificates.
@@ -371,8 +382,18 @@ func (b *LocalBackend) getCertPEM(ctx context.Context, cs certStore, logf logger
 	acmeMu.Lock()
 	defer acmeMu.Unlock()
 
+	// In case this method was triggered multiple times in parallel (when
+	// serving incoming requests), check whether one of the other goroutines
+	// already renewed the cert before us.
 	if p, err := getCertPEMCached(cs, domain, now); err == nil {
-		return p, nil
+		// shouldStartDomainRenewal caches its result so it's OK to call this
+		// frequently.
+		shouldRenew, err := b.shouldStartDomainRenewal(cs, domain, now, p)
+		if err != nil {
+			logf("error checking for certificate renewal: %v", err)
+		} else if !shouldRenew {
+			return p, nil
+		}
 	} else if !errors.Is(err, ipn.ErrStateNotExist) && !errors.Is(err, errCertExpired) {
 		return nil, err
 	}
@@ -509,6 +530,7 @@ func (b *LocalBackend) getCertPEM(ctx context.Context, cs certStore, logf logger
 	if err := cs.WriteCert(domain, certPEM.Bytes()); err != nil {
 		return nil, err
 	}
+	b.domainRenewed(domain)
 
 	return &TLSCertKeyPair{CertPEM: certPEM.Bytes(), KeyPEM: privPEM.Bytes()}, nil
 }

ipn/ipnlocal/cert_js.go

@@ -12,6 +12,6 @@ type TLSCertKeyPair struct {
 	CertPEM, KeyPEM []byte
 }
 
-func (b *LocalBackend) GetCertPEM(ctx context.Context, domain string) (*TLSCertKeyPair, error) {
+func (b *LocalBackend) GetCertPEM(ctx context.Context, domain string, syncRenewal bool) (*TLSCertKeyPair, error) {
 	return nil, errors.New("not implemented for js/wasm")
 }

ipn/ipnlocal/cert_test.go

@@ -112,7 +112,7 @@ func TestShouldStartDomainRenewal(t *testing.T) {
 	reset := func() {
 		renewMu.Lock()
 		defer renewMu.Unlock()
-		maps.Clear(lastRenewCheck)
+		maps.Clear(renewCertAt)
 	}
 
 	mustMakePair := func(template *x509.Certificate) *TLSCertKeyPair {
@@ -178,7 +178,7 @@ func TestShouldStartDomainRenewal(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			reset()
 
-			ret, err := b.shouldStartDomainRenewalByExpiry(now, mustMakePair(&x509.Certificate{
+			ret, err := b.domainRenewalTimeByExpiry(mustMakePair(&x509.Certificate{
 				SerialNumber: big.NewInt(2019),
 				Subject:      subject,
 				NotBefore:    tt.notBefore,
@@ -192,8 +192,9 @@ func TestShouldStartDomainRenewal(t *testing.T) {
 					t.Errorf("got err=%q, want %q", err.Error(), tt.wantErr)
 				}
 			} else {
-				if ret != tt.want {
-					t.Errorf("got ret=%v, want %v", ret, tt.want)
+				renew := now.After(ret)
+				if renew != tt.want {
+					t.Errorf("got renew=%v (ret=%v), want renew %v", renew, ret, tt.want)
 				}
 			}
 		})

ipn/ipnlocal/serve.go

@@ -372,7 +372,7 @@ func (b *LocalBackend) tcpHandlerForServe(dport uint16, srcAddr netip.AddrPort)
 					GetCertificate: func(hi *tls.ClientHelloInfo) (*tls.Certificate, error) {
 						ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 						defer cancel()
-						pair, err := b.GetCertPEM(ctx, sni)
+						pair, err := b.GetCertPEM(ctx, sni, false)
 						if err != nil {
 							return nil, err
 						}
@@ -675,7 +675,7 @@ func (b *LocalBackend) getTLSServeCertForPort(port uint16) func(hi *tls.ClientHe
 
 		ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 		defer cancel()
-		pair, err := b.GetCertPEM(ctx, hi.ServerName)
+		pair, err := b.GetCertPEM(ctx, hi.ServerName, false)
 		if err != nil {
 			return nil, err
 		}

ipn/localapi/cert.go

@@ -23,7 +23,7 @@ func (h *Handler) serveCert(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "internal handler config wired wrong", 500)
 		return
 	}
-	pair, err := h.b.GetCertPEM(r.Context(), domain)
+	pair, err := h.b.GetCertPEM(r.Context(), domain, true)
 	if err != nil {
 		// TODO(bradfitz): 500 is a little lazy here. The errors returned from
 		// GetCertPEM (and everywhere) should carry info info to get whether

ssh/tailssh/incubator.go

@@ -270,7 +270,12 @@ func beIncubator(args []string) error {
 		if err != nil {
 			return err
 		}
-		return server.Serve()
+		// TODO(https://github.com/pkg/sftp/pull/554): Revert the check for io.EOF,
+		// when sftp is patched to report clean termination.
+		if err := server.Serve(); err != nil && err != io.EOF {
+			return err
+		}
+		return nil
 	}
 
 	cmd := exec.Command(ia.cmdName, ia.cmdArgs...)

tka/key.go

@@ -145,6 +145,9 @@ func signatureVerify(s *tkatype.Signature, aumDigest tkatype.AUMSigHash, key Key
 	//            so we should use the public contained in the state machine.
 	switch key.Kind {
 	case Key25519:
+		if len(key.Public) != ed25519.PublicKeySize {
+			return fmt.Errorf("ed25519 key has wrong length: %d", len(key.Public))
+		}
 		if ed25519consensus.Verify(ed25519.PublicKey(key.Public), aumDigest[:], s.Signature) {
 			return nil
 		}

tka/sig.go

@@ -225,6 +225,9 @@ func (s *NodeKeySignature) verifySignature(nodeKey key.NodePublic, verificationK
 		if !ok {
 			return errors.New("missing rotation key")
 		}
+		if len(verifyPub) != ed25519.PublicKeySize {
+			return fmt.Errorf("bad rotation key length: %d", len(verifyPub))
+		}
 		if !ed25519.Verify(ed25519.PublicKey(verifyPub[:]), sigHash[:], s.Signature) {
 			return errors.New("invalid signature")
 		}
@@ -249,6 +252,9 @@ func (s *NodeKeySignature) verifySignature(nodeKey key.NodePublic, verificationK
 		}
 		switch verificationKey.Kind {
 		case Key25519:
+			if len(verificationKey.Public) != ed25519.PublicKeySize {
+				return fmt.Errorf("ed25519 key has wrong length: %d", len(verificationKey.Public))
+			}
 			if ed25519consensus.Verify(ed25519.PublicKey(verificationKey.Public), sigHash[:], s.Signature) {
 				return nil
 			}

tstest/deptest/deptest.go

@@ -0,0 +1,56 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// The deptest package contains a shared implementation of negative
+// dependency tests for other packages, making sure we don't start
+// depending on certain packages.
+package deptest
+
+import (
+	"encoding/json"
+	"os"
+	"os/exec"
+	"runtime"
+	"testing"
+)
+
+type DepChecker struct {
+	GOOS    string
+	GOARCH  string
+	BadDeps map[string]string // package => why
+}
+
+func (c DepChecker) Check(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		// Slow and avoid caring about "go.exe" etc.
+		t.Skip("skipping dep tests on windows hosts")
+	}
+	t.Helper()
+	cmd := exec.Command("go", "list", "-json", ".")
+	var extraEnv []string
+	if c.GOOS != "" {
+		extraEnv = append(extraEnv, "GOOS="+c.GOOS)
+	}
+	if c.GOARCH != "" {
+		extraEnv = append(extraEnv, "GOARCH="+c.GOARCH)
+	}
+	cmd.Env = append(os.Environ(), extraEnv...)
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+	var res struct {
+		Deps []string
+	}
+	if err := json.Unmarshal(out, &res); err != nil {
+		t.Fatal(err)
+	}
+
+	for _, dep := range res.Deps {
+		if why, ok := c.BadDeps[dep]; ok {
+			t.Errorf("package %q is not allowed as a dependency (env: %q); reason: %s", dep, extraEnv, why)
+		}
+	}
+	t.Logf("got %d dependencies", len(res.Deps))
+
+}

tstest/iosdeps/iosdeps_test.go

@@ -1,38 +1,21 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-// No need to run this on Windows where CI's slow enough. Then we don't need to
-// worry about "go.exe" etc.
-
-//go:build !windows
-
 package iosdeps
 
 import (
-	"encoding/json"
-	"os"
-	"os/exec"
 	"testing"
+
+	"tailscale.com/tstest/deptest"
 )
 
 func TestDeps(t *testing.T) {
-	cmd := exec.Command("go", "list", "-json", ".")
-	cmd.Env = append(os.Environ(), "GOOS=ios", "GOARCH=arm64")
-	out, err := cmd.Output()
-	if err != nil {
-		t.Fatal(err)
-	}
-	var res struct {
-		Deps []string
-	}
-	if err := json.Unmarshal(out, &res); err != nil {
-		t.Fatal(err)
-	}
-	for _, dep := range res.Deps {
-		switch dep {
-		case "text/template", "html/template":
-			t.Errorf("package %q is not allowed as a dependency on iOS", dep)
-		}
-	}
-	t.Logf("got %d dependencies", len(res.Deps))
+	deptest.DepChecker{
+		GOOS:   "ios",
+		GOARCH: "arm64",
+		BadDeps: map[string]string{
+			"text/template": "linker bloat (MethodByName)",
+			"html/template": "linker bloat (MethodByName)",
+		},
+	}.Check(t)
 }

tstest/jsdeps/jsdeps_test.go

@@ -1,38 +1,24 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-// No need to run this on Windows where CI's slow enough. Then we don't need to
-// worry about "go.exe" etc.
-
-//go:build !windows
-
 package jsdeps
 
 import (
-	"encoding/json"
-	"os"
-	"os/exec"
 	"testing"
+
+	"tailscale.com/tstest/deptest"
 )
 
 func TestDeps(t *testing.T) {
-	cmd := exec.Command("go", "list", "-json", ".")
-	cmd.Env = append(os.Environ(), "GOOS=js", "GOARCH=wasm")
-	out, err := cmd.Output()
-	if err != nil {
-		t.Fatal(err)
-	}
-	var res struct {
-		Deps []string
-	}
-	if err := json.Unmarshal(out, &res); err != nil {
-		t.Fatal(err)
-	}
-	for _, dep := range res.Deps {
-		switch dep {
-		case "runtime/pprof", "golang.org/x/net/http2/h2c", "net/http/pprof", "golang.org/x/net/proxy", "github.com/tailscale/goupnp":
-			t.Errorf("package %q is not allowed as a dependency on JS", dep)
-		}
-	}
-	t.Logf("got %d dependencies", len(res.Deps))
+	deptest.DepChecker{
+		GOOS:   "js",
+		GOARCH: "wasm",
+		BadDeps: map[string]string{
+			"runtime/pprof":               "bloat",
+			"golang.org/x/net/http2/h2c":  "bloat",
+			"net/http/pprof":              "bloat",
+			"golang.org/x/net/proxy":      "bloat",
+			"github.com/tailscale/goupnp": "bloat, which can't work anyway in wasm",
+		},
+	}.Check(t)
 }

util/linuxfw/iptables.go

@@ -7,94 +7,13 @@
 package linuxfw
 
 import (
-	"encoding/hex"
-	"errors"
-	"fmt"
-	"net"
-	"net/netip"
-	"strings"
-	"unsafe"
-
-	"github.com/josharian/native"
-	"golang.org/x/sys/unix"
-	linuxabi "gvisor.dev/gvisor/pkg/abi/linux"
-	"tailscale.com/net/netaddr"
 	"tailscale.com/types/logger"
 )
 
-type sockLen uint32
-
-var (
-	iptablesChainNames = map[int]string{
-		linuxabi.NF_INET_PRE_ROUTING:  "PREROUTING",
-		linuxabi.NF_INET_LOCAL_IN:     "INPUT",
-		linuxabi.NF_INET_FORWARD:      "FORWARD",
-		linuxabi.NF_INET_LOCAL_OUT:    "OUTPUT",
-		linuxabi.NF_INET_POST_ROUTING: "POSTROUTING",
-	}
-	iptablesStandardChains = (func() map[string]bool {
-		ret := make(map[string]bool)
-		for _, v := range iptablesChainNames {
-			ret[v] = true
-		}
-		return ret
-	})()
-)
-
 // DebugNetfilter prints debug information about iptables rules to the
 // provided log function.
 func DebugIptables(logf logger.Logf) error {
-	for _, table := range []string{"filter", "nat", "raw"} {
-		type chainAndEntry struct {
-			chain string
-			entry *entry
-		}
-
-		// Collect all entries first so we can resolve jumps
-		var (
-			lastChain    string
-			ces          []chainAndEntry
-			chainOffsets = make(map[int]string)
-		)
-		err := enumerateIptablesTable(logf, table, func(chain string, entry *entry) error {
-			if chain != lastChain {
-				chainOffsets[entry.Offset] = chain
-				lastChain = chain
-			}
-
-			ces = append(ces, chainAndEntry{
-				chain: lastChain,
-				entry: entry,
-			})
-			return nil
-		})
-		if err != nil {
-			return err
-		}
-
-		lastChain = ""
-		for _, ce := range ces {
-			if ce.chain != lastChain {
-				logf("iptables: table=%s chain=%s", table, ce.chain)
-				lastChain = ce.chain
-			}
-
-			// Fixup jump
-			if std, ok := ce.entry.Target.Data.(standardTarget); ok {
-				if strings.HasPrefix(std.Verdict, "JUMP(") {
-					var off int
-					if _, err := fmt.Sscanf(std.Verdict, "JUMP(%d)", &off); err == nil {
-						if jt, ok := chainOffsets[off]; ok {
-							std.Verdict = "JUMP(" + jt + ")"
-							ce.entry.Target.Data = std
-						}
-					}
-				}
-			}
-
-			logf("iptables:   entry=%+v", ce.entry)
-		}
-	}
+	// unused.
 	return nil
 }
 
@@ -106,721 +25,5 @@ func DebugIptables(logf logger.Logf) error {
 // syscall fails); when there are no iptables rules, it is valid for this
 // function to return 0, nil.
 func DetectIptables() (int, error) {
-	dummyLog := func(string, ...any) {}
-
-	var (
-		validRules int
-		firstErr   error
-	)
-	for _, table := range []string{"filter", "nat", "raw"} {
-		err := enumerateIptablesTable(dummyLog, table, func(chain string, entry *entry) error {
-			// If we have any rules other than basic 'ACCEPT' entries in a
-			// standard chain, then we consider this a valid rule.
-			switch {
-			case !iptablesStandardChains[chain]:
-				validRules++
-			case entry.Target.Name != "standard":
-				validRules++
-			case entry.Target.Name == "standard" && entry.Target.Data.(standardTarget).Verdict != "ACCEPT":
-				validRules++
-			}
-			return nil
-		})
-		if err != nil && firstErr == nil {
-			firstErr = err
-		}
-	}
-
-	return validRules, firstErr
-}
-
-func enumerateIptablesTable(logf logger.Logf, table string, cb func(string, *entry) error) error {
-	ln, err := net.Listen("tcp4", ":0")
-	if err != nil {
-		return err
-	}
-	defer ln.Close()
-
-	tcpLn := ln.(*net.TCPListener)
-	conn, err := tcpLn.SyscallConn()
-	if err != nil {
-		return err
-	}
-
-	var tableName linuxabi.TableName
-	copy(tableName[:], []byte(table))
-
-	tbl := linuxabi.IPTGetinfo{
-		Name: tableName,
-	}
-	slt := sockLen(linuxabi.SizeOfIPTGetinfo)
-
-	var ctrlErr error
-	err = conn.Control(func(fd uintptr) {
-		_, _, errno := unix.Syscall6(
-			unix.SYS_GETSOCKOPT,
-			fd,
-			uintptr(unix.SOL_IP),
-			linuxabi.IPT_SO_GET_INFO,
-			uintptr(unsafe.Pointer(&tbl)),
-			uintptr(unsafe.Pointer(&slt)),
-			0,
-		)
-		if errno != 0 {
-			ctrlErr = errno
-			return
-		}
-	})
-	if err != nil {
-		return err
-	}
-	if ctrlErr != nil {
-		return ctrlErr
-	}
-
-	if tbl.Size < 1 {
-		return nil
-	}
-
-	// Allocate enough space to be able to get all iptables information.
-	entsBuf := make([]byte, linuxabi.SizeOfIPTGetEntries+tbl.Size)
-	entsHdr := (*linuxabi.IPTGetEntries)(unsafe.Pointer(&entsBuf[0]))
-	entsHdr.Name = tableName
-	entsHdr.Size = tbl.Size
-
-	slt = sockLen(len(entsBuf))
-
-	err = conn.Control(func(fd uintptr) {
-		_, _, errno := unix.Syscall6(
-			unix.SYS_GETSOCKOPT,
-			fd,
-			uintptr(unix.SOL_IP),
-			linuxabi.IPT_SO_GET_ENTRIES,
-			uintptr(unsafe.Pointer(&entsBuf[0])),
-			uintptr(unsafe.Pointer(&slt)),
-			0,
-		)
-		if errno != 0 {
-			ctrlErr = errno
-			return
-		}
-	})
-	if err != nil {
-		return err
-	}
-	if ctrlErr != nil {
-		return ctrlErr
-	}
-
-	// Skip header
-	entsBuf = entsBuf[linuxabi.SizeOfIPTGetEntries:]
-
-	var (
-		totalOffset  int
-		currentChain string
-	)
-	for len(entsBuf) > 0 {
-		parser := entryParser{
-			buf:             entsBuf,
-			logf:            logf,
-			checkExtraBytes: true,
-		}
-		entry, err := parser.parseEntry(entsBuf)
-		if err != nil {
-			logf("iptables: err=%v", err)
-			break
-		}
-		entry.Offset += totalOffset
-
-		// Don't pass 'ERROR' nodes to our caller
-		if entry.Target.Name == "ERROR" {
-			if parser.offset == len(entsBuf) {
-				// all done
-				break
-			}
-
-			// New user-defined chain
-			currentChain = entry.Target.Data.(errorTarget).ErrorName
-		} else {
-			// Detect if we're at a new chain based on the hook
-			// offsets we fetched earlier.
-			for i, he := range tbl.HookEntry {
-				if int(he) == totalOffset {
-					currentChain = iptablesChainNames[i]
-				}
-			}
-
-			// Now that we have everything, call our callback.
-			if err := cb(currentChain, &entry); err != nil {
-				return err
-			}
-		}
-
-		entsBuf = entsBuf[parser.offset:]
-		totalOffset += parser.offset
-	}
-	return nil
-}
-
-// TODO(andrew): convert to use cstruct
-type entryParser struct {
-	buf    []byte
-	offset int
-
-	logf logger.Logf
-
-	// Set to 'true' to print debug messages about unused bytes returned
-	// from the kernel
-	checkExtraBytes bool
-}
-
-func (p *entryParser) haveLen(ln int) bool {
-	if len(p.buf)-p.offset < ln {
-		return false
-	}
-	return true
-}
-
-func (p *entryParser) assertLen(ln int) error {
-	if !p.haveLen(ln) {
-		return fmt.Errorf("need %d bytes: %w", ln, errBufferTooSmall)
-	}
-	return nil
-}
-
-func (p *entryParser) getBytes(amt int) []byte {
-	ret := p.buf[p.offset : p.offset+amt]
-	p.offset += amt
-	return ret
-}
-
-func (p *entryParser) getByte() byte {
-	ret := p.buf[p.offset]
-	p.offset += 1
-	return ret
-}
-
-func (p *entryParser) get4() (ret [4]byte) {
-	ret[0] = p.buf[p.offset+0]
-	ret[1] = p.buf[p.offset+1]
-	ret[2] = p.buf[p.offset+2]
-	ret[3] = p.buf[p.offset+3]
-	p.offset += 4
-	return
-}
-
-func (p *entryParser) setOffset(off, max int) error {
-	// We can't go back
-	if off < p.offset {
-		return fmt.Errorf("invalid target offset (%d < %d): %w", off, p.offset, errMalformed)
-	}
-
-	// Ensure we don't go beyond our maximum, if given
-	if max >= 0 && off >= max {
-		return fmt.Errorf("invalid target offset (%d >= %d): %w", off, max, errMalformed)
-	}
-
-	// If we aren't already at this offset, move forward
-	if p.offset < off {
-		if p.checkExtraBytes {
-			extraData := p.buf[p.offset:off]
-			diff := off - p.offset
-			p.logf("%d bytes (%d, %d) are unused: %s", diff, p.offset, off, hex.EncodeToString(extraData))
-		}
-
-		p.offset = off
-	}
-	return nil
-}
-
-var (
-	errBufferTooSmall = errors.New("buffer too small")
-	errMalformed      = errors.New("data malformed")
-)
-
-type entry struct {
-	Offset      int
-	IP          iptip
-	NFCache     uint32
-	PacketCount uint64
-	ByteCount   uint64
-	Matches     []match
-	Target      target
-}
-
-func (e entry) String() string {
-	var sb strings.Builder
-	sb.WriteString("{")
-
-	fmt.Fprintf(&sb, "Offset:%d IP:%v PacketCount:%d ByteCount:%d", e.Offset, e.IP, e.PacketCount, e.ByteCount)
-	if len(e.Matches) > 0 {
-		fmt.Fprintf(&sb, " Matches:%v", e.Matches)
-	}
-	fmt.Fprintf(&sb, " Target:%v", e.Target)
-
-	sb.WriteString("}")
-	return sb.String()
-}
-
-func (p *entryParser) parseEntry(b []byte) (entry, error) {
-	startOff := p.offset
-
-	iptip, err := p.parseIPTIP()
-	if err != nil {
-		return entry{}, fmt.Errorf("parsing IPTIP: %w", err)
-	}
-
-	ret := entry{
-		Offset: startOff,
-		IP:     iptip,
-	}
-
-	// Must have space for the rest of the members
-	if err := p.assertLen(28); err != nil {
-		return entry{}, err
-	}
-
-	ret.NFCache = native.Endian.Uint32(p.getBytes(4))
-	targetOffset := int(native.Endian.Uint16(p.getBytes(2)))
-	nextOffset := int(native.Endian.Uint16(p.getBytes(2)))
-	/* unused field: Comeback = */ p.getBytes(4)
-	ret.PacketCount = native.Endian.Uint64(p.getBytes(8))
-	ret.ByteCount = native.Endian.Uint64(p.getBytes(8))
-
-	// Must have at least enough space in our buffer to get to the target;
-	// doing this here means we can avoid bounds checks in parseMatches
-	if err := p.assertLen(targetOffset - p.offset); err != nil {
-		return entry{}, err
-	}
-
-	// Matches are stored between the end of the entry structure and the
-	// start of the 'targets' structure.
-	ret.Matches, err = p.parseMatches(targetOffset)
-	if err != nil {
-		return entry{}, err
-	}
-
-	if targetOffset > 0 {
-		if err := p.setOffset(targetOffset, nextOffset); err != nil {
-			return entry{}, err
-		}
-
-		ret.Target, err = p.parseTarget(nextOffset)
-		if err != nil {
-			return entry{}, fmt.Errorf("parsing target: %w", err)
-		}
-	}
-
-	if err := p.setOffset(nextOffset, -1); err != nil {
-		return entry{}, err
-	}
-
-	return ret, nil
-}
-
-type iptip struct {
-	Src                 netip.Addr
-	Dst                 netip.Addr
-	SrcMask             netip.Addr
-	DstMask             netip.Addr
-	InputInterface      string
-	OutputInterface     string
-	InputInterfaceMask  []byte
-	OutputInterfaceMask []byte
-	Protocol            uint16
-	Flags               uint8
-	InverseFlags        uint8
-}
-
-var protocolNames = map[uint16]string{
-	unix.IPPROTO_ESP:    "esp",
-	unix.IPPROTO_GRE:    "gre",
-	unix.IPPROTO_ICMP:   "icmp",
-	unix.IPPROTO_ICMPV6: "icmpv6",
-	unix.IPPROTO_IGMP:   "igmp",
-	unix.IPPROTO_IP:     "ip",
-	unix.IPPROTO_IPIP:   "ipip",
-	unix.IPPROTO_IPV6:   "ip6",
-	unix.IPPROTO_RAW:    "raw",
-	unix.IPPROTO_TCP:    "tcp",
-	unix.IPPROTO_UDP:    "udp",
-}
-
-func (ip iptip) String() string {
-	var sb strings.Builder
-	sb.WriteString("{")
-
-	formatAddrMask := func(addr, mask netip.Addr) string {
-		if pref, ok := netaddr.FromStdIPNet(&net.IPNet{
-			IP:   addr.AsSlice(),
-			Mask: mask.AsSlice(),
-		}); ok {
-			return fmt.Sprint(pref)
-		}
-		return fmt.Sprintf("%s/%s", addr, mask)
-	}
-
-	fmt.Fprintf(&sb, "Src:%s", formatAddrMask(ip.Src, ip.SrcMask))
-	fmt.Fprintf(&sb, ", Dst:%s", formatAddrMask(ip.Dst, ip.DstMask))
-
-	translateMask := func(mask []byte) string {
-		var ret []byte
-		for _, b := range mask {
-			if b != 0 {
-				ret = append(ret, 'X')
-			} else {
-				ret = append(ret, '.')
-			}
-		}
-		return string(ret)
-	}
-
-	if ip.InputInterface != "" {
-		fmt.Fprintf(&sb, ", InputInterface:%s/%s", ip.InputInterface, translateMask(ip.InputInterfaceMask))
-	}
-	if ip.OutputInterface != "" {
-		fmt.Fprintf(&sb, ", OutputInterface:%s/%s", ip.OutputInterface, translateMask(ip.OutputInterfaceMask))
-	}
-	if nm, ok := protocolNames[ip.Protocol]; ok {
-		fmt.Fprintf(&sb, ", Protocol:%s", nm)
-	} else {
-		fmt.Fprintf(&sb, ", Protocol:%d", ip.Protocol)
-	}
-
-	if ip.Flags != 0 {
-		fmt.Fprintf(&sb, ", Flags:%d", ip.Flags)
-	}
-	if ip.InverseFlags != 0 {
-		fmt.Fprintf(&sb, ", InverseFlags:%d", ip.InverseFlags)
-	}
-
-	sb.WriteString("}")
-	return sb.String()
-}
-
-func (p *entryParser) parseIPTIP() (iptip, error) {
-	if err := p.assertLen(84); err != nil {
-		return iptip{}, err
-	}
-
-	var ret iptip
-
-	ret.Src = netip.AddrFrom4(p.get4())
-	ret.Dst = netip.AddrFrom4(p.get4())
-	ret.SrcMask = netip.AddrFrom4(p.get4())
-	ret.DstMask = netip.AddrFrom4(p.get4())
-
-	const IFNAMSIZ = 16
-	ret.InputInterface = unix.ByteSliceToString(p.getBytes(IFNAMSIZ))
-	ret.OutputInterface = unix.ByteSliceToString(p.getBytes(IFNAMSIZ))
-
-	ret.InputInterfaceMask = p.getBytes(IFNAMSIZ)
-	ret.OutputInterfaceMask = p.getBytes(IFNAMSIZ)
-
-	ret.Protocol = native.Endian.Uint16(p.getBytes(2))
-	ret.Flags = p.getByte()
-	ret.InverseFlags = p.getByte()
-	return ret, nil
-}
-
-type match struct {
-	Name     string
-	Revision int
-	Data     any
-	RawData  []byte
-}
-
-func (m match) String() string {
-	return fmt.Sprintf("{Name:%s, Data:%v}", m.Name, m.Data)
-}
-
-type matchTCP struct {
-	SourcePortRange [2]uint16
-	DestPortRange   [2]uint16
-	Option          byte
-	FlagMask        byte
-	FlagCompare     byte
-	InverseFlags    byte
-}
-
-func (m matchTCP) String() string {
-	var sb strings.Builder
-	sb.WriteString("{")
-
-	fmt.Fprintf(&sb, "SrcPort:%s, DstPort:%s",
-		formatPortRange(m.SourcePortRange),
-		formatPortRange(m.DestPortRange))
-
-	// TODO(andrew): format semantically
-	if m.Option != 0 {
-		fmt.Fprintf(&sb, ", Option:%d", m.Option)
-	}
-	if m.FlagMask != 0 {
-		fmt.Fprintf(&sb, ", FlagMask:%d", m.FlagMask)
-	}
-	if m.FlagCompare != 0 {
-		fmt.Fprintf(&sb, ", FlagCompare:%d", m.FlagCompare)
-	}
-	if m.InverseFlags != 0 {
-		fmt.Fprintf(&sb, ", InverseFlags:%d", m.InverseFlags)
-	}
-
-	sb.WriteString("}")
-	return sb.String()
-}
-
-func (p *entryParser) parseMatches(maxOffset int) ([]match, error) {
-	const XT_EXTENSION_MAXNAMELEN = 29
-	const structSize = 2 + XT_EXTENSION_MAXNAMELEN + 1
-
-	var ret []match
-	for {
-		// If we don't have space for a single match structure, we're done
-		if p.offset+structSize > maxOffset {
-			break
-		}
-
-		var curr match
-
-		matchSize := int(native.Endian.Uint16(p.getBytes(2)))
-		curr.Name = unix.ByteSliceToString(p.getBytes(XT_EXTENSION_MAXNAMELEN))
-		curr.Revision = int(p.getByte())
-
-		// The data size is the total match size minus what we've already consumed.
-		dataLen := matchSize - structSize
-		dataEnd := p.offset + dataLen
-
-		// If we don't have space for the match data, then there's something wrong
-		if dataEnd > maxOffset {
-			return nil, fmt.Errorf("out of space for match (%d > max %d): %w", dataEnd, maxOffset, errMalformed)
-		} else if dataEnd > len(p.buf) {
-			return nil, fmt.Errorf("out of space for match (%d > buf %d): %w", dataEnd, len(p.buf), errMalformed)
-		}
-
-		curr.RawData = p.getBytes(dataLen)
-
-		// TODO(andrew): more here; UDP, etc.
-		switch curr.Name {
-		case "tcp":
-			/*
-			   struct xt_tcp {
-			       __u16 spts[2];  // Source port range.
-			       __u16 dpts[2];  // Destination port range.
-			       __u8 option;    // TCP Option iff non-zero
-			       __u8 flg_mask;  // TCP flags mask byte
-			       __u8 flg_cmp;   // TCP flags compare byte
-			       __u8 invflags;  // Inverse flags
-			   };
-			*/
-			if len(curr.RawData) >= 12 {
-				curr.Data = matchTCP{
-					SourcePortRange: [...]uint16{
-						native.Endian.Uint16(curr.RawData[0:2]),
-						native.Endian.Uint16(curr.RawData[2:4]),
-					},
-					DestPortRange: [...]uint16{
-						native.Endian.Uint16(curr.RawData[4:6]),
-						native.Endian.Uint16(curr.RawData[6:8]),
-					},
-					Option:       curr.RawData[8],
-					FlagMask:     curr.RawData[9],
-					FlagCompare:  curr.RawData[10],
-					InverseFlags: curr.RawData[11],
-				}
-			}
-		}
-
-		ret = append(ret, curr)
-	}
-	return ret, nil
-}
-
-type target struct {
-	Name     string
-	Revision int
-	Data     any
-	RawData  []byte
-}
-
-func (t target) String() string {
-	return fmt.Sprintf("{Name:%s, Data:%v}", t.Name, t.Data)
-}
-
-func (p *entryParser) parseTarget(nextOffset int) (target, error) {
-	const XT_EXTENSION_MAXNAMELEN = 29
-	const structSize = 2 + XT_EXTENSION_MAXNAMELEN + 1
-
-	if err := p.assertLen(structSize); err != nil {
-		return target{}, err
-	}
-
-	var ret target
-
-	targetSize := int(native.Endian.Uint16(p.getBytes(2)))
-	ret.Name = unix.ByteSliceToString(p.getBytes(XT_EXTENSION_MAXNAMELEN))
-	ret.Revision = int(p.getByte())
-
-	if targetSize > structSize {
-		dataLen := targetSize - structSize
-		if err := p.assertLen(dataLen); err != nil {
-			return target{}, err
-		}
-
-		ret.RawData = p.getBytes(dataLen)
-	}
-
-	// Special case; matches what iptables does
-	if ret.Name == "" {
-		ret.Name = "standard"
-	}
-
-	switch ret.Name {
-	case "standard":
-		if len(ret.RawData) >= 4 {
-			verdict := int32(native.Endian.Uint32(ret.RawData))
-
-			var info string
-			switch verdict {
-			case -1:
-				info = "DROP"
-			case -2:
-				info = "ACCEPT"
-			case -4:
-				info = "QUEUE"
-			case -5:
-				info = "RETURN"
-			case int32(nextOffset):
-				info = "FALLTHROUGH"
-			default:
-				info = fmt.Sprintf("JUMP(%d)", verdict)
-			}
-			ret.Data = standardTarget{Verdict: info}
-		}
-
-	case "ERROR":
-		ret.Data = errorTarget{
-			ErrorName: unix.ByteSliceToString(ret.RawData),
-		}
-
-	case "REJECT":
-		if len(ret.RawData) >= 4 {
-			ret.Data = rejectTarget{
-				With: rejectWith(native.Endian.Uint32(ret.RawData)),
-			}
-		}
-
-	case "MARK":
-		if len(ret.RawData) >= 8 {
-			mark := native.Endian.Uint32(ret.RawData[0:4])
-			mask := native.Endian.Uint32(ret.RawData[4:8])
-
-			var mode markMode
-			switch {
-			case mark == 0:
-				mode = markModeAnd
-				mark = ^mask
-
-			case mark == mask:
-				mode = markModeOr
-
-			case mask == 0:
-				mode = markModeXor
-
-			case mask == 0xffffffff:
-				mode = markModeSet
-
-			default:
-				// TODO(andrew): handle xset?
-			}
-
-			ret.Data = markTarget{
-				Mark: mark,
-				Mode: mode,
-			}
-		}
-	}
-
-	return ret, nil
-}
-
-// Various types for things in iptables-land follow.
-
-type standardTarget struct {
-	Verdict string
-}
-
-type errorTarget struct {
-	ErrorName string
-}
-
-type rejectWith int
-
-const (
-	rwIPT_ICMP_NET_UNREACHABLE rejectWith = iota
-	rwIPT_ICMP_HOST_UNREACHABLE
-	rwIPT_ICMP_PROT_UNREACHABLE
-	rwIPT_ICMP_PORT_UNREACHABLE
-	rwIPT_ICMP_ECHOREPLY
-	rwIPT_ICMP_NET_PROHIBITED
-	rwIPT_ICMP_HOST_PROHIBITED
-	rwIPT_TCP_RESET
-	rwIPT_ICMP_ADMIN_PROHIBITED
-)
-
-func (rw rejectWith) String() string {
-	switch rw {
-	case rwIPT_ICMP_NET_UNREACHABLE:
-		return "icmp-net-unreachable"
-	case rwIPT_ICMP_HOST_UNREACHABLE:
-		return "icmp-host-unreachable"
-	case rwIPT_ICMP_PROT_UNREACHABLE:
-		return "icmp-prot-unreachable"
-	case rwIPT_ICMP_PORT_UNREACHABLE:
-		return "icmp-port-unreachable"
-	case rwIPT_ICMP_ECHOREPLY:
-		return "icmp-echo-reply"
-	case rwIPT_ICMP_NET_PROHIBITED:
-		return "icmp-net-prohibited"
-	case rwIPT_ICMP_HOST_PROHIBITED:
-		return "icmp-host-prohibited"
-	case rwIPT_TCP_RESET:
-		return "tcp-reset"
-	case rwIPT_ICMP_ADMIN_PROHIBITED:
-		return "icmp-admin-prohibited"
-	default:
-		return "UNKNOWN"
-	}
-}
-
-type rejectTarget struct {
-	With rejectWith
-}
-
-type markMode byte
-
-const (
-	markModeSet markMode = iota
-	markModeAnd
-	markModeOr
-	markModeXor
-)
-
-func (mm markMode) String() string {
-	switch mm {
-	case markModeSet:
-		return "set"
-	case markModeAnd:
-		return "and"
-	case markModeOr:
-		return "or"
-	case markModeXor:
-		return "xor"
-	default:
-		return "UNKNOWN"
-	}
-}
-
-type markTarget struct {
-	Mode markMode
-	Mark uint32
+	panic("unused")
 }

util/linuxfw/linuxfw_struct_linux_test.go

@@ -1,20 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-// TODO(#8502): add support for more architectures
-//go:build linux && (arm64 || amd64)
-
-package linuxfw
-
-import (
-	"testing"
-	"unsafe"
-
-	"tailscale.com/util/linuxfw/linuxfwtest"
-)
-
-func TestSizes(t *testing.T) {
-	linuxfwtest.TestSizes(t, &linuxfwtest.SizeInfo{
-		SizeofSocklen: unsafe.Sizeof(sockLen(0)),
-	})
-}