cmd/{derp,derpprobe},prober,derp: add mesh support to derpprobe (#15414)

Add mesh key support to derpprobe for
probing derpers with verify set to true.

Move MeshKey checking to central point for code reuse.

Fix a bad error fmt msg.

Fixes tailscale/corp#27294
Fixes tailscale/corp#25756

Signed-off-by: Mike O'Driscoll <mikeo@tailscale.com>

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,21 @@ type clientInfo struct {
 	IsProber bool `json:",omitempty"`
 }
 
+// Equal reports if two clientInfo values are equal.
+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 false
-	}
-	return s.meshKey.Equal(k)
+
+	return s.meshKey.Equal(info.MeshKey)
 }
 
 // verifyClient checks whether the client is allowed to connect to the derper,

derp/derp_test.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"reflect"
 	"strconv"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -33,21 +34,53 @@ import (
 	"tailscale.com/tstest"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/must"
 )
 
 func TestClientInfoUnmarshal(t *testing.T) {
-	for i, in := range []string{
-		`{"Version":5,"MeshKey":"abc"}`,
-		`{"version":5,"meshKey":"abc"}`,
+	for i, in := range map[string]struct {
+		json    string
+		want    *clientInfo
+		wantErr string
+	}{
+		"empty": {
+			json: `{}`,
+			want: &clientInfo{},
+		},
+		"valid": {
+			json: `{"Version":5,"MeshKey":"6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8"}`,
+			want: &clientInfo{MeshKey: must.Get(key.ParseDERPMesh("6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8")), Version: 5},
+		},
+		"validLowerMeshKey": {
+			json: `{"version":5,"meshKey":"6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8"}`,
+			want: &clientInfo{MeshKey: must.Get(key.ParseDERPMesh("6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8")), Version: 5},
+		},
+		"invalidMeshKeyToShort": {
+			json:    `{"version":5,"meshKey":"abcdefg"}`,
+			wantErr: "invalid mesh key",
+		},
+		"invalidMeshKeyToLong": {
+			json:    `{"version":5,"meshKey":"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"}`,
+			wantErr: "invalid mesh key",
+		},
 	} {
-		var got clientInfo
-		if err := json.Unmarshal([]byte(in), &got); err != nil {
-			t.Fatalf("[%d]: %v", i, err)
-		}
-		want := clientInfo{Version: 5, MeshKey: "abc"}
-		if got != want {
-			t.Errorf("[%d]: got %+v; want %+v", i, got, want)
-		}
+		t.Run(i, func(t *testing.T) {
+			t.Parallel()
+			var got clientInfo
+			err := json.Unmarshal([]byte(in.json), &got)
+			if in.wantErr != "" {
+				if err == nil || !strings.Contains(err.Error(), in.wantErr) {
+					t.Errorf("Unmarshal(%q) = %v, want error containing %q", in.json, err, in.wantErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("Unmarshal(%q) = %v, want no error", in.json, err)
+			}
+			if !got.Equal(in.want) {
+				t.Errorf("Unmarshal(%q) = %+v, want %+v", in.json, got, in.want)
+			}
+		})
 	}
 }
 
@@ -1681,43 +1714,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 {