Skip to content

Commit a34089b

Browse files
keyurbodarbradfitz
authored andcommitted
tailcat: reject malformed ConnBlob public keys
1 parent cdf35e5 commit a34089b

2 files changed

Lines changed: 39 additions & 0 deletions

File tree

tailcat.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ func (p NodePublic) MarshalBinary() ([]byte, error) {
174174

175175
// UnmarshalBinary implements encoding.BinaryUnmarshaler for CBOR deserialization.
176176
func (p *NodePublic) UnmarshalBinary(x []byte) error {
177+
if len(x) != key.NodePublicRawLen {
178+
return fmt.Errorf("invalid node public key length %d, want %d", len(x), key.NodePublicRawLen)
179+
}
177180
p.NodePublic = key.NodePublicFromRaw32(go4mem.B(x))
178181
return nil
179182
}

tailcat_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package tailcat
55

66
import (
77
"context"
8+
"encoding/base64"
89
"errors"
910
"fmt"
1011
"io"
@@ -16,6 +17,7 @@ import (
1617
"testing"
1718
"time"
1819

20+
"github.com/fxamacker/cbor/v2"
1921
"github.com/google/go-cmp/cmp"
2022
"go4.org/mem"
2123
"tailscale.com/tailcfg"
@@ -422,6 +424,40 @@ func TestConnBlob(t *testing.T) {
422424
}
423425
}
424426

427+
func TestParseConnBlobMalformedPublicKey(t *testing.T) {
428+
for name, keyBytes := range map[string][]byte{
429+
"short": make([]byte, key.NodePublicRawLen-1),
430+
"long": make([]byte, key.NodePublicRawLen+1),
431+
} {
432+
t.Run(name, func(t *testing.T) {
433+
raw, err := cbor.Marshal(map[string][]byte{"p": keyBytes})
434+
if err != nil {
435+
t.Fatal(err)
436+
}
437+
cb := ConnBlob("tc" + base64.RawURLEncoding.EncodeToString(raw))
438+
assertParseError := func(name string, parse func() error) {
439+
t.Helper()
440+
defer func() {
441+
if r := recover(); r != nil {
442+
t.Fatalf("%s panicked: %v", name, r)
443+
}
444+
}()
445+
if err := parse(); err == nil {
446+
t.Errorf("%s unexpectedly accepted malformed public key", name)
447+
}
448+
}
449+
assertParseError("ParseConnBlob", func() error {
450+
_, err := ParseConnBlob(cb)
451+
return err
452+
})
453+
assertParseError("ParseConnBlobRaw", func() error {
454+
_, err := ParseConnBlobRaw(cb)
455+
return err
456+
})
457+
})
458+
}
459+
}
460+
425461
// TestFetchDERPMapMemoryCache verifies the default in-memory DERP map
426462
// cache: a second fetch of the same URL within the freshness window
427463
// makes no network request.

0 commit comments

Comments
 (0)