fixup! cmd/{derp,derpprobe},prober,derp: add mesh support to derpprobe

cmd/derpprobe/derpprobe.go

@@ -20,6 +20,7 @@ import (
 	"tailscale.com/derp"
 	"tailscale.com/prober"
 	"tailscale.com/tsweb"
+	"tailscale.com/types/key"
 	"tailscale.com/version"
 
 	// Support for prometheus varz in tsweb
@@ -121,7 +122,7 @@ func main() {
 	log.Fatal(http.ListenAndServe(*listen, mux))
 }
 
-func getMeshKey() (string, error) {
+func getMeshKey() (key.DERPMesh, error) {
 	var meshKey string
 
 	if *dev {
@@ -160,12 +161,12 @@ func getMeshKey() (string, error) {
 		log.Println("Got mesh key from static file")
 		meshKey = b.GetString()
 	}
-
 	if meshKey == "" {
 		log.Printf("No mesh key found, mesh key is empty")
+		return key.DERPMesh{}, nil
 	}
 
-	return derp.CheckMeshKey(meshKey)
+	return key.ParseDERPMesh(meshKey)
 }
 
 type overallStatus struct {

derp/derp_client.go

@@ -165,7 +165,7 @@ type clientInfo struct {
 	// trusted clients.  It's required to subscribe to the
 	// connection list & forward packets. It's empty for regular
 	// users.
-	MeshKey string `json:"meshKey,omitempty"`
+	MeshKey key.DERPMesh `json:"meshKey,omitempty,omitzero"`
 
 	// Version is the DERP protocol version that the client was built with.
 	// See the ProtocolVersion const.
@@ -179,10 +179,20 @@ type clientInfo struct {
 	IsProber bool `json:",omitempty"`
 }
 
+func (c *clientInfo) Equal(other *clientInfo) bool {
+	if c == nil || other == nil {
+		return c == other
+	}
+	if c.Version != other.Version || c.CanAckPings != other.CanAckPings || c.IsProber != other.IsProber {
+		return false
+	}
+	return c.MeshKey.Equal(other.MeshKey)
+}
+
 func (c *Client) sendClientKey() error {
 	msg, err := json.Marshal(clientInfo{
 		Version:     ProtocolVersion,
-		MeshKey:     c.meshKey.String(),
+		MeshKey:     c.meshKey,
 		CanAckPings: c.canAckPings,
 		IsProber:    c.isProber,
 	})

derp/derp_server.go

@@ -1364,14 +1364,11 @@ func (s *Server) isMeshPeer(info *clientInfo) bool {
 	// Since mesh keys are a fixed length, we don’t need to be concerned
 	// about timing attacks on client mesh keys that are the wrong length.
 	// See https://github.com/tailscale/corp/issues/28720
-	if info == nil || info.MeshKey == "" {
+	if info == nil || info.MeshKey.IsZero() {
 		return false
 	}
-	k, err := key.ParseDERPMesh(info.MeshKey)
+
-	if err != nil {
+	return s.meshKey.Equal(info.MeshKey)
-		return false
-	}
-	return s.meshKey.Equal(k)
 }
 
 // verifyClient checks whether the client is allowed to connect to the derper,

derp/derp_test.go

@@ -36,16 +36,20 @@ import (
 )
 
 func TestClientInfoUnmarshal(t *testing.T) {
+	m, err := key.ParseDERPMesh("6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8")
+	if err != nil {
+		t.Fatalf("ParseDERPMesh failed: %v", err)
+	}
 	for i, in := range []string{
-		`{"Version":5,"MeshKey":"abc"}`,
+		`{"Version":5,"MeshKey":"6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8"}`,
-		`{"version":5,"meshKey":"abc"}`,
+		`{"version":5,"meshKey":"6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8"}`,
 	} {
 		var got clientInfo
 		if err := json.Unmarshal([]byte(in), &got); err != nil {
 			t.Fatalf("[%d]: %v", i, err)
 		}
-		want := clientInfo{Version: 5, MeshKey: "abc"}
+		want := clientInfo{Version: 5, MeshKey: m}
-		if got != want {
+		if !got.Equal(&want) {
 			t.Errorf("[%d]: got %+v; want %+v", i, got, want)
 		}
 	}
@@ -1681,43 +1685,43 @@ func TestIsMeshPeer(t *testing.T) {
 		t.Fatal(err)
 	}
 	for name, tt := range map[string]struct {
-		info       *clientInfo
 		want       bool
+		meshKey    string
 		wantAllocs float64
 	}{
 		"nil": {
-			info:       nil,
 			want:       false,
 			wantAllocs: 0,
 		},
-		"empty": {
-			info:       &clientInfo{MeshKey: ""},
-			want:       false,
-			wantAllocs: 0,
-		},
-		"invalid": {
-			info:       &clientInfo{MeshKey: "invalid"},
-			want:       false,
-			wantAllocs: 2, // error message
-		},
 		"mismatch": {
-			info:       &clientInfo{MeshKey: "0badf00d00000000000000000000000000000000000000000000000000000000"},
+			meshKey:    "6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8",
 			want:       false,
 			wantAllocs: 1,
 		},
 		"match": {
-			info:       &clientInfo{MeshKey: testMeshKey},
+			meshKey:    testMeshKey,
 			want:       true,
-			wantAllocs: 1,
+			wantAllocs: 0,
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
 			var got bool
+			var mKey key.DERPMesh
+			if tt.meshKey != "" {
+				mKey, err = key.ParseDERPMesh(tt.meshKey)
+				if err != nil {
+					t.Fatalf("ParseDERPMesh(%q) failed: %v", tt.meshKey, err)
+				}
+			}
+
+			info := clientInfo{
+				MeshKey: mKey,
+			}
 			allocs := testing.AllocsPerRun(1, func() {
-				got = s.isMeshPeer(tt.info)
+				got = s.isMeshPeer(&info)
 			})
 			if got != tt.want {
-				t.Fatalf("got %t, want %t: info = %#v", got, tt.want, tt.info)
+				t.Fatalf("got %t, want %t: info = %#v", got, tt.want, info)
 			}
 
 			if allocs != tt.wantAllocs && tt.want {

derp/mesh_key.go

@@ -4,9 +4,7 @@
 package derp
 
 import (
-	"fmt"
 	"regexp"
-	"strings"
 )
 
 var (
@@ -14,20 +12,3 @@ var (
 	// which must be a 64-character hexadecimal string (lowercase only).
 	ValidMeshKey = regexp.MustCompile(`^[0-9a-f]{64}$`)
 )
-
-// CheckMeshKey checks if the provided key is a valid mesh key.
-// It trims any leading or trailing whitespace and returns an error if the key
-// does not match the expected format. If the key is empty or valid, it returns
-// the trimmed key and a nil error. The key must be a 64-character
-// hexadecimal string (lowercase only).
-func CheckMeshKey(key string) (string, error) {
-	if key == "" {
-		return key, nil
-	}
-
-	key = strings.TrimSpace(key)
-	if !ValidMeshKey.MatchString(key) {
-		return "", fmt.Errorf("key must contain exactly 64 hex digits")
-	}
-	return key, nil
-}

derp/mesh_key_test.go

@@ -1,52 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-package derp
-
-import "testing"
-
-func TestCheckMeshKey(t *testing.T) {
-	testCases := []struct {
-		name    string
-		input   string
-		want    string
-		wantErr bool
-	}{
-		{
-			name:    "KeyOkay",
-			input:   "f1ffafffffffffffffffffffffffffffffffffffffffffffffffff2ffffcfff6",
-			want:    "f1ffafffffffffffffffffffffffffffffffffffffffffffffffff2ffffcfff6",
-			wantErr: false,
-		},
-		{
-			name:    "TrimKeyOkay",
-			input:   "  f1ffafffffffffffffffffffffffffffffffffffffffffffffffff2ffffcfff6  ",
-			want:    "f1ffafffffffffffffffffffffffffffffffffffffffffffffffff2ffffcfff6",
-			wantErr: false,
-		},
-		{
-			name:    "NotAKey",
-			input:   "zzthisisnotakey",
-			want:    "",
-			wantErr: true,
-		},
-	}
-	for _, tt := range testCases {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			k, err := CheckMeshKey(tt.input)
-			if err != nil && !tt.wantErr {
-				t.Errorf("unexpected error: %v", err)
-			}
-			if err == nil && tt.wantErr {
-				t.Errorf("expected error but got none")
-			}
-			if k != tt.want {
-				t.Errorf("got: %s doesn't match expected: %s", k, tt.want)
-			}
-
-		})
-	}
-
-}

prober/derp.go

@@ -144,12 +144,8 @@ func WithRegionCodeOrID(regionCode string) DERPOpt {
 	}
 }
 
-func WithMeshKey(meshKey string) DERPOpt {
+func WithMeshKey(meshKey key.DERPMesh) DERPOpt {
 	return func(d *derpProber) {
-		meshKey, err := key.ParseDERPMesh(meshKey)
-		if err != nil {
-			log.Fatalf("failed to parse DERP mesh key: %v", err)
-		}
 		d.meshKey = meshKey
 	}
 }

types/key/derp.go

@@ -6,6 +6,7 @@ package key
 import (
 	"crypto/subtle"
 	"encoding/hex"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"strings"
@@ -23,14 +24,40 @@ type DERPMesh struct {
 	k [32]byte             // 64-digit hexadecimal numbers fit in 32 bytes
 }
 
+func (k DERPMesh) MarshalJSON() ([]byte, error) {
+	return json.Marshal(k.String())
+}
+
+func (k *DERPMesh) UnmarshalJSON(data []byte) error {
+	var s string
+	json.Unmarshal(data, &s)
+
+	if len(s) != 64 {
+		return fmt.Errorf("%w: JSON Unmarshal failure, mesh key must be a 64-digit hexadecimal number", ErrInvalidMeshKey)
+	}
+
+	decoded, err := hex.DecodeString(s)
+	if err != nil {
+		return fmt.Errorf("%w: JSON Unmarshal failure, %v", ErrInvalidMeshKey, err)
+	}
+
+	if len(decoded) != 32 {
+		return fmt.Errorf("%w: JSON Unmarshal failure, mesh key must be 32 bytes", ErrInvalidMeshKey)
+	}
+
+	copy(k.k[:], decoded)
+
+	return nil
+}
+
 // DERPMeshFromRaw32 parses a 32-byte raw value as a DERP mesh key.
-func DERPMeshFromRaw32(raw mem.RO) DERPMesh {
+func DERPMeshFromRaw32(raw mem.RO) (DERPMesh, error) {
 	if raw.Len() != 32 {
-		panic("input has wrong size")
+		return DERPMesh{}, fmt.Errorf("%w: must be 32 bytes", ErrInvalidMeshKey)
 	}
 	var ret DERPMesh
 	raw.Copy(ret.k[:])
-	return ret
+	return ret, nil
 }
 
 // ParseDERPMesh parses a DERP mesh key from a string.
@@ -45,7 +72,7 @@ func ParseDERPMesh(key string) (DERPMesh, error) {
 	if err != nil {
 		return DERPMesh{}, fmt.Errorf("%w: %v", ErrInvalidMeshKey, err)
 	}
-	return DERPMeshFromRaw32(mem.B(decoded)), nil
+	return DERPMeshFromRaw32(mem.B(decoded))
 }
 
 // IsZero reports whether k is the zero value.

types/key/derp_test.go

@@ -119,7 +119,10 @@ func TestDERPMesh(t *testing.T) {
 				t.Fatalf("decoded %x, want %x", k.k, tt.hex)
 			}
 
-			h := DERPMeshFromRaw32(mem.B(tt.hex))
+			h, err := DERPMeshFromRaw32(mem.B(tt.hex))
+			if err != nil {
+				t.Fatalf("DERPMeshFromRaw32(%x): %v", tt.hex, err)
+			}
 			if k.Equal(h) != tt.equal {
 				if tt.equal {
 					t.Fatalf("%v != %v", k, h)
@@ -131,3 +134,12 @@ func TestDERPMesh(t *testing.T) {
 		})
 	}
 }
+
+func TestRaw32Invalid(t *testing.T) {
+	t.Parallel()
+
+	_, err := DERPMeshFromRaw32(mem.B(make([]byte, 31)))
+	if !errors.Is(err, ErrInvalidMeshKey) {
+		t.Fatalf("DERPMeshFromRaw32(31 bytes): %v, want ErrInvalidMeshKey", err)
+	}
+}