From 77ba2ce66e3f253ccf936b3900373e051346e93d Mon Sep 17 00:00:00 2001 From: Mike O'Driscoll Date: Mon, 9 Jun 2025 15:16:37 -0400 Subject: [PATCH] fixup! cmd/{derp,derpprobe},prober,derp: add mesh support to derpprobe --- cmd/derpprobe/derpprobe.go | 7 ++--- derp/derp_client.go | 14 ++++++++-- derp/derp_server.go | 9 +++---- derp/derp_test.go | 46 ++++++++++++++++++--------------- derp/mesh_key.go | 19 -------------- derp/mesh_key_test.go | 52 -------------------------------------- prober/derp.go | 6 +---- types/key/derp.go | 35 ++++++++++++++++++++++--- types/key/derp_test.go | 14 +++++++++- 9 files changed, 89 insertions(+), 113 deletions(-) delete mode 100644 derp/mesh_key_test.go diff --git a/cmd/derpprobe/derpprobe.go b/cmd/derpprobe/derpprobe.go index 0c9c0a218..485aa04fc 100644 --- a/cmd/derpprobe/derpprobe.go +++ b/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 { diff --git a/derp/derp_client.go b/derp/derp_client.go index a9b92299c..3f8962e4a 100644 --- a/derp/derp_client.go +++ b/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, }) diff --git a/derp/derp_server.go b/derp/derp_server.go index 6f86c3ea4..c6a749485 100644 --- a/derp/derp_server.go +++ b/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, diff --git a/derp/derp_test.go b/derp/derp_test.go index 0093ee2b1..3d4f382ff 100644 --- a/derp/derp_test.go +++ b/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":"abc"}`, + `{"Version":5,"MeshKey":"6d529e9d4ef632d22d4a4214cb49da8f1ba1b72697061fb24e312984c35ec8d8"}`, + `{"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"} - if got != want { + want := clientInfo{Version: 5, MeshKey: m} + 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 { diff --git a/derp/mesh_key.go b/derp/mesh_key.go index e75182fa5..e7076e1d2 100644 --- a/derp/mesh_key.go +++ b/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 -} diff --git a/derp/mesh_key_test.go b/derp/mesh_key_test.go deleted file mode 100644 index 0a90a8b6f..000000000 --- a/derp/mesh_key_test.go +++ /dev/null @@ -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) - } - - }) - } - -} diff --git a/prober/derp.go b/prober/derp.go index 1bfcd6322..e21c8ce76 100644 --- a/prober/derp.go +++ b/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 } } diff --git a/types/key/derp.go b/types/key/derp.go index 1fe690189..c2d46fff5 100644 --- a/types/key/derp.go +++ b/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. diff --git a/types/key/derp_test.go b/types/key/derp_test.go index b91cbbf8c..3be972e65 100644 --- a/types/key/derp_test.go +++ b/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) + } +}