Skip to content

Commit cb1e0d7

Browse files
committed
tailcat: separate node and disco keys
Like Tailscale itself, use separate node and disco keys. Tailcat had lazily reused the node key as its disco key as an early implementation shortcut, and that shortcut was not cleaned up before release. Because disco frames expose the disco public key on direct UDP paths, reusing it also exposed the node public key embedded in the unlisted connection token. Derive an independent disco key, include its public half in new tokens, and reject legacy tokens when connecting. Thanks to Heyang Zhou (https://x.com/heyang_zhou) for the report! Reported-by: Heyang Zhou (@losfair)
1 parent bce78f3 commit cb1e0d7

7 files changed

Lines changed: 168 additions & 44 deletions

File tree

‎README.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -451,11 +451,12 @@ ConnBlob internally). It looks like `tcXYZ...` and is a `"tc"` prefix
451451
followed by base64-encoded [CBOR](https://cbor.io/) containing:
452452

453453
- The server's WireGuard public key (Curve25519, 32 bytes)
454+
- A separate path-discovery public key (Curve25519, 32 bytes)
454455
- DERP info. Either:
455456
1. a small integer referencing one of the default [Tailscale-run tailcat servers](https://tailcat.dev/derpmap.json), or
456457
2. full DERP server metadata, to either use a custom DERP server, or to avoid the client needing a potential round-trip to fetch the latest DERP map (the server's `--full-address` flag and the `tailcat resolve` subcommand produce this form)
457458

458-
A typical token with just an integer region ID is around 50 bytes. With embedded
459+
A typical token with just an integer region ID is around 95 bytes. With embedded
459460
DERP node details it's longer but self-contained.
460461

461462
### Network stack
@@ -483,8 +484,10 @@ without the control plane.
483484
It then waits for clients.
484485

485486
2. **Client parses the token** to learn the server's public key and
486-
DERP region. It generates its own ephemeral keypair and connects to
487-
the same DERP relay.
487+
path-discovery key, plus its DERP region. It generates its own ephemeral
488+
keypair and connects to the same DERP relay. The separate path-discovery
489+
key can appear in cleartext direct-path disco frames without revealing the
490+
WireGuard public key that acts as the unlisted connection capability.
488491

489492
3. **Discovery handshake.** The client sends a "**Meow**" ping message
490493
to the server through the

‎cmd/tailcat/tailcat.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,7 @@ func server(logf logger.Logf) {
736736
clearUnnecessaryRegionFields(reg)
737737
fmt.Fprintf(os.Stderr, "# Selected bootstrap relay region %v, %v\n", reg.RegionID, reg.RegionName)
738738

739-
ci = &tailcat.ConnInfo{ServerPublic: tailcat.NodePublic{NodePublic: priv.Public()}}
739+
ci = new(tailcat.ConnInfo)
740740
if embed {
741741
ci.Region = []*tailcfg.DERPRegion{reg}
742742
} else {
@@ -750,6 +750,8 @@ func server(logf logger.Logf) {
750750
Region: []*tailcfg.DERPRegion{reg},
751751
}
752752
}
753+
ci.ServerPublic = tailcat.NodePublic{NodePublic: priv.Public()}
754+
ci.ServerDiscoPublic = tailcat.DiscoPublicForNode(priv)
753755
connStr := ci.ConnBlob()
754756

755757
s := &tailcat.Server{Key: priv, Logf: logf, Region: reg}

‎tailcat.go‎

Lines changed: 86 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
//
88
// A [Server] listens for incoming clients via a DERP relay. Clients discover
99
// the server through a compact [ConnBlob] (connection blob) that encodes the
10-
// server's public key and DERP region. DERP is used only for the initial
10+
// server's WireGuard and path-discovery public keys and DERP region. DERP is used only for the initial
1111
// bootstrap; once both sides learn each other's endpoints, Tailscale's
1212
// magicsock layer upgrades to a direct peer-to-peer UDP path whenever possible,
1313
// just like the normal Tailscale data plane. DERP remains available as a
@@ -33,6 +33,8 @@ package tailcat
3333
import (
3434
"cmp"
3535
"context"
36+
"crypto/hmac"
37+
"crypto/sha256"
3638
"encoding/base64"
3739
"encoding/json"
3840
"errors"
@@ -136,11 +138,16 @@ type expandForServer struct{}
136138
// [ConnInfo]. A typical ConnBlob looks like "tcomFwWC…".
137139
type ConnBlob string
138140

139-
// ConnInfo describes how to reach a server: its public key and which DERP
140-
// relay region to use. It is serialized into a [ConnBlob] for exchange,
141+
// ConnInfo describes how to reach a server: its WireGuard and path-discovery
142+
// public keys and which DERP relay region to use. It is serialized into a [ConnBlob] for exchange,
141143
// via the wire types in wire.go.
142144
type ConnInfo struct {
143145
ServerPublic NodePublic // a key.NodePublic
146+
// ServerDiscoPublic is the server's public key for path discovery.
147+
// It is deliberately independent from ServerPublic: disco packets carry
148+
// this key in cleartext on direct UDP paths, while ServerPublic is the
149+
// unguessable part of the server's connection address.
150+
ServerDiscoPublic DiscoPublic // a key.DiscoPublic
144151

145152
// Region, if non-empty, lists the regions of a DERPMap.
146153
// Either Region or RegionID must be set. If Region is set
@@ -167,6 +174,31 @@ type NodePublic struct {
167174
key.NodePublic
168175
}
169176

177+
// DiscoPublic is a wrapper around key.DiscoPublic that uses its raw 32-byte
178+
// representation in connection blobs.
179+
type DiscoPublic struct {
180+
key.DiscoPublic
181+
}
182+
183+
// Equal reports whether a and b represent the same disco public key.
184+
func (a DiscoPublic) Equal(b DiscoPublic) bool {
185+
return a.DiscoPublic == b.DiscoPublic
186+
}
187+
188+
// MarshalBinary implements encoding.BinaryMarshaler.
189+
func (p DiscoPublic) MarshalBinary() ([]byte, error) {
190+
return p.DiscoPublic.AppendTo(nil), nil
191+
}
192+
193+
// UnmarshalBinary implements encoding.BinaryUnmarshaler.
194+
func (p *DiscoPublic) UnmarshalBinary(x []byte) error {
195+
if len(x) != key.DiscoPublicRawLen {
196+
return fmt.Errorf("invalid disco public key length %d, want %d", len(x), key.DiscoPublicRawLen)
197+
}
198+
p.DiscoPublic = key.DiscoPublicFromRaw32(go4mem.B(x))
199+
return nil
200+
}
201+
170202
// MarshalBinary implements encoding.BinaryMarshaler for CBOR serialization,
171203
// encoding the raw 32-byte key without the "nodekey:" text prefix.
172204
func (p NodePublic) MarshalBinary() ([]byte, error) {
@@ -203,6 +235,7 @@ func NewPrivateKey() *PrivateKey {
203235
Private: key.NewNode(),
204236
}
205237
ret.Public.ServerPublic = NodePublic{ret.Private.Public()}
238+
ret.Public.ServerDiscoPublic = DiscoPublic{discoPrivateForNode(ret.Private).Public()}
206239
return ret
207240
}
208241

@@ -211,16 +244,17 @@ func NewPrivateKey() *PrivateKey {
211244
// but there's no controlclient involved, because there's
212245
// no control plane.
213246
type locoBackend struct {
214-
sys tsd.System
215-
priv key.NodePrivate
216-
pub key.NodePublic
217-
addr netip.Addr
218-
addrPrefix netip.Prefix
219-
ns *netstack.Impl
220-
dm *tailcfg.DERPMap
221-
logf logger.Logf
222-
serverPub key.NodePublic // non-zero if we're a client (server's public key)
223-
isServer bool
247+
sys tsd.System
248+
priv key.NodePrivate
249+
pub key.NodePublic
250+
addr netip.Addr
251+
addrPrefix netip.Prefix
252+
ns *netstack.Impl
253+
dm *tailcfg.DERPMap
254+
logf logger.Logf
255+
serverPub key.NodePublic // non-zero if we're a client (server's public key)
256+
serverDiscoPub key.DiscoPublic // non-zero if we're a client (server's disco key)
257+
isServer bool
224258

225259
// discoPublic returns the node's disco public key, memoized to
226260
// avoid redoing the curve25519 derivation for every client that
@@ -675,9 +709,7 @@ func newLocoBackend(priv key.NodePrivate) *locoBackend {
675709
addr: addr,
676710
addrPrefix: addrPrefix,
677711
}
678-
lb.discoPublic = sync.OnceValue(func() key.DiscoPublic {
679-
return nodePrivateAsDiscoPrivate(lb.priv).Public()
680-
})
712+
lb.discoPublic = sync.OnceValue(func() key.DiscoPublic { return discoPrivateForNode(lb.priv).Public() })
681713
return lb
682714
}
683715

@@ -689,6 +721,7 @@ func (lb *locoBackend) connBlob() ConnBlob {
689721
}
690722
var ci ConnInfo
691723
ci.ServerPublic = NodePublic{lb.pub}
724+
ci.ServerDiscoPublic = DiscoPublic{lb.discoPublic()}
692725
for _, r := range lb.dm.Regions {
693726
ci.Region = append(ci.Region, r)
694727
}
@@ -712,6 +745,9 @@ func (ci *ConnInfo) ConnBlob() ConnBlob {
712745
ServerPublic: ci.ServerPublic,
713746
RegionID: ci.RegionID,
714747
}
748+
if !ci.ServerDiscoPublic.IsZero() {
749+
w.ServerDiscoPublic = &ci.ServerDiscoPublic
750+
}
715751
for _, r := range ci.Region {
716752
wr := wireRegionOf(r)
717753

@@ -806,6 +842,9 @@ func ParseConnBlob(cb ConnBlob) (ConnInfo, error) {
806842
ServerPublic: w.ServerPublic,
807843
RegionID: w.RegionID,
808844
}
845+
if w.ServerDiscoPublic != nil {
846+
ci.ServerDiscoPublic = *w.ServerDiscoPublic
847+
}
809848
for _, wr := range w.Region {
810849
ci.Region = append(ci.Region, wr.derpRegion())
811850
}
@@ -1180,7 +1219,7 @@ func (b *locoBackend) advertiseEndpoints() {
11801219
return
11811220
}
11821221
payload := (&disco.CallMeMaybe{MyNumber: eps}).AppendMarshal(nil)
1183-
discoPriv := nodePrivateAsDiscoPrivate(b.priv)
1222+
discoPriv := discoPrivateForNode(b.priv)
11841223
mc := b.sys.MagicSock.Get()
11851224
regionID := b.derpRegionID()
11861225
for _, p := range peers {
@@ -1261,7 +1300,7 @@ func (lb *locoBackend) Start() error {
12611300
Name: "server.tailcat.",
12621301
User: 100,
12631302
Key: lb.serverPub,
1264-
DiscoKey: nodePublicAsDiscoPublic(lb.serverPub),
1303+
DiscoKey: lb.serverDiscoPub,
12651304
Addresses: []netip.Prefix{serverAddrPrefix},
12661305
AllowedIPs: []netip.Prefix{serverAddrPrefix, allIPv6},
12671306
HomeDERP: derpRegion,
@@ -1423,13 +1462,13 @@ func createEngine(logf logger.Logf, lb *locoBackend) (err error) {
14231462
if lb.isServer {
14241463
conf.DERPAppName = "tailcat-server"
14251464
}
1426-
// Both sides force their disco key to be derived from their node
1427-
// key. For the server, that lets clients predict its disco key
1428-
// from the ConnBlob without extra round trips. And knowing our own
1429-
// disco private key is what lets either side seal the
1465+
// Both sides deterministically derive a separate disco key from their
1466+
// node private key. The public disco key, unlike the node key, is safe to
1467+
// expose in direct-path disco frames. Knowing our own disco private key
1468+
// is what lets either side seal the
14301469
// call-me-maybe messages that advertise our UDP endpoints (see
14311470
// locoBackend.advertiseEndpoints).
1432-
conf.ForceDiscoKey = nodePrivateAsDiscoPrivate(lb.priv)
1471+
conf.ForceDiscoKey = discoPrivateForNode(lb.priv)
14331472
netns.SetEnabled(false)
14341473
e, err := wgengine.NewUserspaceEngine(logf, conf)
14351474
if err != nil {
@@ -1524,11 +1563,15 @@ func (c *Client) initLocked() error {
15241563
if err != nil {
15251564
return err
15261565
}
1566+
if ci.ServerDiscoPublic.IsZero() {
1567+
return errors.New("legacy server address lacks a separate disco key; generate a new address with an updated tailcat server")
1568+
}
15271569

15281570
lb := newLocoBackend(c.nodeKeyLocked())
15291571
lb.logf = logf
15301572
lb.dm = &tailcfg.DERPMap{}
15311573
lb.serverPub = ci.ServerPublic.NodePublic
1574+
lb.serverDiscoPub = ci.ServerDiscoPublic.DiscoPublic
15321575

15331576
sys := &lb.sys
15341577
bus := eventbus.New()
@@ -1876,18 +1919,25 @@ func (s *Server) Status() *ipnstate.Status {
18761919
return s.lb.Status()
18771920
}
18781921

1879-
// nodePrivateAsDiscoPrivate converts a NodePrivate to a DiscoPrivate by
1880-
// reusing the same raw key bytes. This is used in tailcat where the server
1881-
// uses its node key as the disco key.
1882-
func nodePrivateAsDiscoPrivate(k key.NodePrivate) key.DiscoPrivate {
1883-
raw := k.Raw32()
1884-
return key.DiscoPrivateFromRaw32(go4mem.B(raw[:]))
1885-
}
1886-
1887-
// nodePublicAsDiscoPublic converts a NodePublic to a DiscoPublic by
1888-
// reusing the same raw key bytes. This is used in tailcat where the
1889-
// server's node public key doubles as its disco public key.
1890-
func nodePublicAsDiscoPublic(k key.NodePublic) key.DiscoPublic {
1922+
// discoPrivateForNode deterministically derives a path-discovery key from a
1923+
// node private key. Keeping the two public keys unlinkable is a security
1924+
// boundary: disco frames expose the disco public key on the local network,
1925+
// while knowledge of the node public key grants access to an open server.
1926+
func discoPrivateForNode(k key.NodePrivate) key.DiscoPrivate {
18911927
raw := k.Raw32()
1892-
return key.DiscoPublicFromRaw32(go4mem.B(raw[:]))
1928+
mac := hmac.New(sha256.New, raw[:])
1929+
mac.Write([]byte("github.com/tailscale/tailcat disco key v1"))
1930+
discoRaw := mac.Sum(nil)
1931+
// Canonicalize the Curve25519 scalar, as key.NewDisco does.
1932+
discoRaw[0] &= 248
1933+
discoRaw[31] &= 127
1934+
discoRaw[31] |= 64
1935+
return key.DiscoPrivateFromRaw32(go4mem.B(discoRaw))
1936+
}
1937+
1938+
// DiscoPublicForNode returns the path-discovery public key derived from a
1939+
// node private key. Code constructing a [ConnInfo] directly must include it
1940+
// as ConnInfo.ServerDiscoPublic.
1941+
func DiscoPublicForNode(k key.NodePrivate) DiscoPublic {
1942+
return DiscoPublic{discoPrivateForNode(k).Public()}
18931943
}

‎tailcat_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"net/http"
1414
"net/http/httptest"
1515
"net/netip"
16+
"strings"
1617
"sync/atomic"
1718
"testing"
1819
"time"
@@ -424,6 +425,51 @@ func TestConnBlob(t *testing.T) {
424425
}
425426
}
426427

428+
func TestConnBlobSeparateDiscoKey(t *testing.T) {
429+
priv := key.NewNode()
430+
discoPub := DiscoPublicForNode(priv)
431+
ci := ConnInfo{
432+
ServerPublic: NodePublic{priv.Public()},
433+
ServerDiscoPublic: discoPub,
434+
RegionID: 10,
435+
}
436+
got, err := ParseConnBlob(ci.ConnBlob())
437+
if err != nil {
438+
t.Fatal(err)
439+
}
440+
if !got.ServerDiscoPublic.Equal(discoPub) {
441+
t.Fatalf("disco public key changed in round trip: got %v, want %v", got.ServerDiscoPublic, discoPub)
442+
}
443+
if got.ServerDiscoPublic.Raw32() == got.ServerPublic.Raw32() {
444+
t.Fatal("server disco public key exposes the server node public key")
445+
}
446+
if again := DiscoPublicForNode(priv); !again.Equal(discoPub) {
447+
t.Fatal("disco key derivation is not stable")
448+
}
449+
// Reproduce the report's reconstruction strategy: treating the public
450+
// key visible in a direct-path disco frame as the server node key must
451+
// not recover the connection address.
452+
reconstructed := (&ConnInfo{
453+
ServerPublic: NodePublic{key.NodePublicFromRaw32(mem.B(discoPub.AppendTo(nil)))},
454+
RegionID: ci.RegionID,
455+
}).ConnBlob()
456+
if reconstructed == ci.ConnBlob() {
457+
t.Fatal("disco public key can reconstruct the connection address")
458+
}
459+
}
460+
461+
func TestClientRejectsLegacyConnBlob(t *testing.T) {
462+
legacy := (&ConnInfo{
463+
ServerPublic: NodePublic{key.NewNode().Public()},
464+
RegionID: 10,
465+
}).ConnBlob()
466+
c := NewClient(legacy)
467+
_, err := c.Ping(context.Background())
468+
if err == nil || !strings.Contains(err.Error(), "legacy server address") {
469+
t.Fatalf("Ping error = %v; want legacy address rejection", err)
470+
}
471+
}
472+
427473
func TestParseConnBlobMalformedPublicKey(t *testing.T) {
428474
for name, keyBytes := range map[string][]byte{
429475
"short": make([]byte, key.NodePublicRawLen-1),
@@ -458,6 +504,27 @@ func TestParseConnBlobMalformedPublicKey(t *testing.T) {
458504
}
459505
}
460506

507+
func TestParseConnBlobMalformedDiscoPublicKey(t *testing.T) {
508+
for name, keyBytes := range map[string][]byte{
509+
"short": make([]byte, key.DiscoPublicRawLen-1),
510+
"long": make([]byte, key.DiscoPublicRawLen+1),
511+
} {
512+
t.Run(name, func(t *testing.T) {
513+
raw, err := cbor.Marshal(map[string][]byte{
514+
"p": key.NewNode().Public().AppendTo(nil),
515+
"k": keyBytes,
516+
})
517+
if err != nil {
518+
t.Fatal(err)
519+
}
520+
cb := ConnBlob("tc" + base64.RawURLEncoding.EncodeToString(raw))
521+
if _, err := ParseConnBlob(cb); err == nil {
522+
t.Fatal("ParseConnBlob unexpectedly accepted malformed disco public key")
523+
}
524+
})
525+
}
526+
}
527+
461528
// TestFetchDERPMapMemoryCache verifies the default in-memory DERP map
462529
// cache: a second fetch of the same URL within the freshness window
463530
// makes no network request.

‎web/main_js.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func tailcatListen(this js.Value, args []js.Value) any {
8888
if keyJSON == "" {
8989
// Pin the picked region so a persisted key keeps the
9090
// same address across page loads.
91-
pk.Public = tailcat.ConnInfo{ServerPublic: pk.Public.ServerPublic, RegionID: reg.RegionID}
91+
pk.Public.RegionID = reg.RegionID
9292
}
9393
blob := pk.Public.ConnBlob()
9494
keyOut, err := json.Marshal(pk)

‎wire.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ import (
2424

2525
// wireConnInfo is the wire form of [ConnInfo].
2626
type wireConnInfo struct {
27-
ServerPublic NodePublic `cbor:"p" json:"ServerPublic"`
28-
Region []*wireRegion `cbor:"r,omitempty" json:"Region,omitempty"`
29-
RegionID int `cbor:"i,omitempty" json:"RegionID,omitempty"`
27+
ServerPublic NodePublic `cbor:"p" json:"ServerPublic"`
28+
ServerDiscoPublic *DiscoPublic `cbor:"k,omitempty" json:"ServerDiscoPublic,omitempty"`
29+
Region []*wireRegion `cbor:"r,omitempty" json:"Region,omitempty"`
30+
RegionID int `cbor:"i,omitempty" json:"RegionID,omitempty"`
3031
}
3132

3233
// wireRegion is the wire form of [tailcfg.DERPRegion].

‎wire_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
// ConnBlob wire format.
2222
var wireFieldNames = map[string]string{
2323
"p": "ServerPublic",
24+
"k": "ServerDiscoPublic",
2425
"r": "Region",
2526
"i": "RegionID",
2627
"c": "RegionCode",

0 commit comments

Comments
 (0)