From 0532eb30dbba7bc5a467c29c929ca22eff3a5439 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 2 Nov 2021 14:41:56 -0700 Subject: [PATCH] all: replace tailcfg.DiscoKey with key.DiscoPublic. Signed-off-by: David Anderson --- control/controlclient/direct.go | 4 +- ipn/ipnlocal/local.go | 2 +- net/tstun/wrap.go | 15 ++++---- net/tstun/wrap_test.go | 8 ++-- tailcfg/tailcfg.go | 37 +------------------ tailcfg/tailcfg_clone.go | 2 +- tailcfg/tailcfg_test.go | 17 --------- tstest/integration/testcontrol/testcontrol.go | 2 +- types/netmap/netmap_test.go | 9 +++-- util/deephash/deephash_test.go | 11 +++--- wgengine/magicsock/magicsock.go | 7 ++-- wgengine/magicsock/magicsock_test.go | 16 ++++---- wgengine/userspace.go | 6 +-- wgengine/watchdog.go | 3 +- wgengine/wgcfg/nmcfg/nmcfg.go | 4 +- wgengine/wgengine.go | 3 +- 16 files changed, 51 insertions(+), 95 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index b92de5072..a81aea27a 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -60,7 +60,7 @@ type Direct struct { keepAlive bool logf logger.Logf linkMon *monitor.Mon // or nil - discoPubKey tailcfg.DiscoKey + discoPubKey key.DiscoPublic getMachinePrivKey func() (key.MachinePrivate, error) debugFlags []string keepSharerAndUserSplit bool @@ -88,7 +88,7 @@ type Options struct { AuthKey string // optional node auth key for auto registration TimeNow func() time.Time // time.Now implementation used by Client Hostinfo *tailcfg.Hostinfo // non-nil passes ownership, nil means to use default using os.Hostname, etc - DiscoPublicKey tailcfg.DiscoKey + DiscoPublicKey key.DiscoPublic NewDecompressor func() (Decompressor, error) KeepAlive bool Logf logger.Logf diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index d9434acfc..3345d000f 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -847,7 +847,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error { }) } - var discoPublic tailcfg.DiscoKey + var discoPublic key.DiscoPublic if controlclient.Debug.Disco { discoPublic = b.e.DiscoPublicKey() } diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index f07e4909e..0bba78a39 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -7,7 +7,6 @@ package tstun import ( - "bytes" "errors" "fmt" "io" @@ -17,14 +16,15 @@ import ( "sync/atomic" "time" + "go4.org/mem" "golang.zx2c4.com/wireguard/device" "golang.zx2c4.com/wireguard/tun" "inet.af/netaddr" "tailscale.com/disco" "tailscale.com/net/packet" - "tailscale.com/tailcfg" "tailscale.com/tstime/mono" "tailscale.com/types/ipproto" + "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/pad32" "tailscale.com/wgengine/filter" @@ -79,7 +79,7 @@ type Wrapper struct { destIPActivity atomic.Value // of map[netaddr.IP]func() destMACAtomic atomic.Value // of [6]byte - discoKey atomic.Value // of tailcfg.DiscoKey + discoKey atomic.Value // of key.DiscoPublic // buffer stores the oldest unconsumed packet from tdev. // It is made a static buffer in order to avoid allocations. @@ -204,7 +204,7 @@ func (t *Wrapper) SetDestIPActivityFuncs(m map[netaddr.IP]func()) { // // It is only used for filtering out bogus traffic when network // stack(s) get confused; see Issue 1526. -func (t *Wrapper) SetDiscoKey(k tailcfg.DiscoKey) { +func (t *Wrapper) SetDiscoKey(k key.DiscoPublic) { t.discoKey.Store(k) } @@ -216,12 +216,13 @@ func (t *Wrapper) isSelfDisco(p *packet.Parsed) bool { return false } pkt := p.Payload() - discoSrc, ok := disco.Source(pkt) + discobs, ok := disco.Source(pkt) if !ok { return false } - selfDiscoPub, ok := t.discoKey.Load().(tailcfg.DiscoKey) - return ok && bytes.Equal(selfDiscoPub[:], discoSrc) + discoSrc := key.DiscoPublicFromRaw32(mem.B(discobs)) + selfDiscoPub, ok := t.discoKey.Load().(key.DiscoPublic) + return ok && selfDiscoPub == discoSrc } func (t *Wrapper) Close() error { diff --git a/net/tstun/wrap_test.go b/net/tstun/wrap_test.go index 3d13567e8..5957d8cd9 100644 --- a/net/tstun/wrap_test.go +++ b/net/tstun/wrap_test.go @@ -13,14 +13,15 @@ import ( "testing" "unsafe" + "go4.org/mem" "golang.zx2c4.com/wireguard/tun/tuntest" "inet.af/netaddr" "tailscale.com/disco" "tailscale.com/net/packet" - "tailscale.com/tailcfg" "tailscale.com/tstest" "tailscale.com/tstime/mono" "tailscale.com/types/ipproto" + "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/wgengine/filter" ) @@ -493,7 +494,7 @@ func TestPeerAPIBypass(t *testing.T) { // Issue 1526: drop disco frames from ourselves. func TestFilterDiscoLoop(t *testing.T) { var memLog tstest.MemLogger - discoPub := tailcfg.DiscoKey{1: 1, 2: 2} + discoPub := key.DiscoPublicFromRaw32(mem.B([]byte{1: 1, 2: 2, 31: 0})) tw := &Wrapper{logf: memLog.Logf} tw.SetDiscoKey(discoPub) uh := packet.UDP4Header{ @@ -505,7 +506,8 @@ func TestFilterDiscoLoop(t *testing.T) { SrcPort: 9, DstPort: 10, } - discoPayload := fmt.Sprintf("%s%s%s", disco.Magic, discoPub[:], [disco.NonceLen]byte{}) + discobs := discoPub.Raw32() + discoPayload := fmt.Sprintf("%s%s%s", disco.Magic, discobs[:], [disco.NonceLen]byte{}) pkt := make([]byte, uh.Len()+len(discoPayload)) uh.Marshal(pkt) copy(pkt[uh.Len():], discoPayload) diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index d1dc77942..8a6dfc748 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -14,7 +14,6 @@ import ( "strings" "time" - "go4.org/mem" "inet.af/netaddr" "tailscale.com/types/dnstype" "tailscale.com/types/key" @@ -79,19 +78,6 @@ func (u StableNodeID) IsZero() bool { return u == "" } -// DiscoKey is the curve25519 public key for path discovery key. -// It's never written to disk or reused between network start-ups. -type DiscoKey [32]byte - -// DiscoKeyFromNodePublic returns k converted to a DiscoKey. -// -// Deprecated: exists only as a compatibility bridge while DiscoKey -// gets removed from the codebase. Do not introduce new uses that -// aren't related to #3206. -func DiscoKeyFromDiscoPublic(k key.DiscoPublic) DiscoKey { - return k.Raw32() -} - // User is an IPN user. // // A user can have multiple logins associated with it (e.g. gmail and github oauth). @@ -163,7 +149,7 @@ type Node struct { Key key.NodePublic KeyExpiry time.Time Machine key.MachinePublic - DiscoKey DiscoKey + DiscoKey key.DiscoPublic Addresses []netaddr.IPPrefix // IP addresses of this Node directly AllowedIPs []netaddr.IPPrefix // range of IP addresses to route to this node Endpoints []string `json:",omitempty"` // IP+port (public via STUN, and local LANs) @@ -751,7 +737,7 @@ type MapRequest struct { Compress string // "zstd" or "" (no compression) KeepAlive bool // whether server should send keep-alives back to us NodeKey key.NodePublic - DiscoKey DiscoKey + DiscoKey key.DiscoPublic IncludeIPv6 bool `json:",omitempty"` // include IPv6 endpoints in returned Node Endpoints (for Version 4 clients) Stream bool // if true, multiple MapResponse objects are returned Hostinfo *Hostinfo @@ -1138,25 +1124,6 @@ func keyMarshalText(prefix string, k [32]byte) []byte { return appendKey(nil, prefix, k) } -func (k DiscoKey) String() string { return fmt.Sprintf("discokey:%x", k[:]) } -func (k DiscoKey) MarshalText() ([]byte, error) { - dk := key.DiscoPublicFromRaw32(mem.B(k[:])) - return dk.MarshalText() -} -func (k *DiscoKey) UnmarshalText(text []byte) error { - var dk key.DiscoPublic - if err := dk.UnmarshalText(text); err != nil { - return err - } - dk.AppendTo(k[:0]) - return nil -} -func (k DiscoKey) ShortString() string { return fmt.Sprintf("d:%x", k[:8]) } -func (k DiscoKey) AppendTo(b []byte) []byte { return appendKey(b, "discokey:", k) } - -// IsZero reports whether k is the zero value. -func (k DiscoKey) IsZero() bool { return k == DiscoKey{} } - func (id ID) String() string { return fmt.Sprintf("id:%x", int64(id)) } func (id UserID) String() string { return fmt.Sprintf("userid:%x", int64(id)) } func (id LoginID) String() string { return fmt.Sprintf("loginid:%x", int64(id)) } diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 6315bb85b..c3e1f0bea 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -75,7 +75,7 @@ var _NodeCloneNeedsRegeneration = Node(struct { Key key.NodePublic KeyExpiry time.Time Machine key.MachinePublic - DiscoKey DiscoKey + DiscoKey key.DiscoPublic Addresses []netaddr.IPPrefix AllowedIPs []netaddr.IPPrefix Endpoints []string diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 1b4bc510b..071a88a90 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -407,14 +407,6 @@ func TestNetInfoFields(t *testing.T) { } } -func TestDiscoKeyMarshal(t *testing.T) { - var k1, k2 DiscoKey - for i := range k1 { - k1[i] = byte(i) - } - testKey(t, "discokey:", k1, &k2) -} - type keyIn interface { String() string MarshalText() ([]byte, error) @@ -542,15 +534,6 @@ func TestAppendKeyAllocs(t *testing.T) { } } -func TestDiscoKeyAppend(t *testing.T) { - d := DiscoKey{1: 1, 2: 2} - got := string(d.AppendTo([]byte("foo"))) - want := "foodiscokey:0001020000000000000000000000000000000000000000000000000000000000" - if got != want { - t.Errorf("got %q; want %q", got, want) - } -} - func TestRegisterRequestNilClone(t *testing.T) { var nilReq *RegisterRequest got := nilReq.Clone() diff --git a/tstest/integration/testcontrol/testcontrol.go b/tstest/integration/testcontrol/testcontrol.go index 4d3e3f869..2b2f73b14 100644 --- a/tstest/integration/testcontrol/testcontrol.go +++ b/tstest/integration/testcontrol/testcontrol.go @@ -276,7 +276,7 @@ func (s *Server) AddFakeNode() { } nk := key.NewNode().Public() mk := key.NewMachine().Public() - dk := tailcfg.DiscoKeyFromDiscoPublic(key.NewDisco().Public()) + dk := key.NewDisco().Public() r := nk.Raw32() id := int64(binary.LittleEndian.Uint64(r[:])) ip := netaddr.IPv4(r[0], r[1], r[2], r[3]) diff --git a/types/netmap/netmap_test.go b/types/netmap/netmap_test.go index c0693bc2f..adf29e8e4 100644 --- a/types/netmap/netmap_test.go +++ b/types/netmap/netmap_test.go @@ -22,13 +22,16 @@ func testNodeKey(b byte) (ret key.NodePublic) { return key.NodePublicFromRaw32(mem.B(bs[:])) } -func testDiscoKey(hexPrefix string) (ret tailcfg.DiscoKey) { +func testDiscoKey(hexPrefix string) (ret key.DiscoPublic) { b, err := hex.DecodeString(hexPrefix) if err != nil { panic(err) } - copy(ret[:], b) - return + // this function is used with short hexes, so zero-extend the raw + // value. + var bs [32]byte + copy(bs[:], b) + return key.DiscoPublicFromRaw32(mem.B(bs[:])) } func TestNetworkMapConcise(t *testing.T) { diff --git a/util/deephash/deephash_test.go b/util/deephash/deephash_test.go index 3319d0b3f..3309ccfc3 100644 --- a/util/deephash/deephash_test.go +++ b/util/deephash/deephash_test.go @@ -15,6 +15,7 @@ import ( "runtime" "testing" + "go4.org/mem" "inet.af/netaddr" "tailscale.com/tailcfg" "tailscale.com/types/dnstype" @@ -163,11 +164,11 @@ func getVal() []interface{} { dnsname.FQDN("d."): {netaddr.MustParseIPPort("8.8.8.8:13"), netaddr.MustParseIPPort("9.9.9.9:24")}, dnsname.FQDN("e."): {netaddr.MustParseIPPort("8.8.8.8:14"), netaddr.MustParseIPPort("9.9.9.9:25")}, }, - map[tailcfg.DiscoKey]bool{ - {1: 1}: true, - {1: 2}: false, - {2: 3}: true, - {3: 4}: false, + map[key.DiscoPublic]bool{ + key.DiscoPublicFromRaw32(mem.B([]byte{1: 1, 31: 0})): true, + key.DiscoPublicFromRaw32(mem.B([]byte{1: 2, 31: 0})): false, + key.DiscoPublicFromRaw32(mem.B([]byte{1: 3, 31: 0})): true, + key.DiscoPublicFromRaw32(mem.B([]byte{1: 4, 31: 0})): false, }, &tailcfg.MapResponse{ DERPMap: &tailcfg.DERPMap{ diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index d02e03220..2ab58397f 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -2269,7 +2269,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) { endpointState: map[netaddr.IPPort]*endpointState{}, } if !n.DiscoKey.IsZero() { - ep.discoKey = key.DiscoPublicFromRaw32(mem.B(n.DiscoKey[:])) + ep.discoKey = n.DiscoKey ep.discoShort = n.DiscoKey.ShortString() } ep.wgEndpoint = n.Key.UntypedHexString() @@ -3609,10 +3609,9 @@ func (de *endpoint) updateFromNode(n *tailcfg.Node) { de.mu.Lock() defer de.mu.Unlock() - tnk := key.DiscoPublicFromRaw32(mem.B(n.DiscoKey[:])) - if de.discoKey != tnk { + if de.discoKey != n.DiscoKey { de.c.logf("[v1] magicsock: disco: node %s changed from discokey %s to %s", de.publicKey.ShortString(), de.discoKey, n.DiscoKey) - de.discoKey = tnk + de.discoKey = n.DiscoKey de.discoShort = de.discoKey.ShortString() de.resetLocked() } diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index b9854b0bb..7b8a3f0d3 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -259,7 +259,7 @@ func meshStacks(logf logger.Logf, mutateNetmap func(idx int, nm *netmap.NetworkM ID: tailcfg.NodeID(i + 1), Name: fmt.Sprintf("node%d", i+1), Key: peer.privateKey.Public(), - DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(peer.conn.DiscoPublicKey()), + DiscoKey: peer.conn.DiscoPublicKey(), Addresses: addrs, AllowedIPs: addrs, Endpoints: epStrings(eps[i]), @@ -618,7 +618,7 @@ func TestNoDiscoKey(t *testing.T) { removeDisco := func(idx int, nm *netmap.NetworkMap) { for _, p := range nm.Peers { - p.DiscoKey = tailcfg.DiscoKey{} + p.DiscoKey = key.DiscoPublic{} } } @@ -680,7 +680,7 @@ func TestDiscokeyChange(t *testing.T) { } mu.Lock() defer mu.Unlock() - nm.Peers[0].DiscoKey = tailcfg.DiscoKeyFromDiscoPublic(m1DiscoKey) + nm.Peers[0].DiscoKey = m1DiscoKey } cleanupMesh := meshStacks(t.Logf, setm1Key, m1, m2) @@ -1137,11 +1137,11 @@ func TestDiscoMessage(t *testing.T) { peer1Priv := c.discoPrivate n := &tailcfg.Node{ Key: key.NewNode().Public(), - DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(peer1Pub), + DiscoKey: peer1Pub, } c.peerMap.upsertEndpoint(&endpoint{ publicKey: n.Key, - discoKey: key.DiscoPublicFromRaw32(mem.B(n.DiscoKey[:])), + discoKey: n.DiscoKey, }) const payload = "why hello" @@ -1233,7 +1233,7 @@ func addTestEndpoint(tb testing.TB, conn *Conn, sendConn net.PacketConn) (key.No Peers: []*tailcfg.Node{ { Key: nodeKey, - DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(discoKey), + DiscoKey: discoKey, Endpoints: []string{sendConn.LocalAddr().String()}, }, }, @@ -1411,7 +1411,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) { Peers: []*tailcfg.Node{ { Key: nodeKey1, - DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(discoKey), + DiscoKey: discoKey, Endpoints: []string{"192.168.1.2:345"}, }, }, @@ -1426,7 +1426,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) { Peers: []*tailcfg.Node{ { Key: nodeKey2, - DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(discoKey), + DiscoKey: discoKey, Endpoints: []string{"192.168.1.2:345"}, }, }, diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 969851f5b..32d50896b 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -331,7 +331,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) closePool.add(e.magicConn) e.magicConn.SetNetworkUp(e.linkMon.InterfaceState().AnyInterfaceUp()) - tsTUNDev.SetDiscoKey(tailcfg.DiscoKeyFromDiscoPublic(e.magicConn.DiscoPublicKey())) + tsTUNDev.SetDiscoKey(e.magicConn.DiscoPublicKey()) if conf.RespondToPing { e.tundev.PostFilterIn = echoRespondToAll @@ -1230,8 +1230,8 @@ func (e *userspaceEngine) SetNetworkMap(nm *netmap.NetworkMap) { } } -func (e *userspaceEngine) DiscoPublicKey() tailcfg.DiscoKey { - return tailcfg.DiscoKeyFromDiscoPublic(e.magicConn.DiscoPublicKey()) +func (e *userspaceEngine) DiscoPublicKey() key.DiscoPublic { + return e.magicConn.DiscoPublicKey() } func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) { diff --git a/wgengine/watchdog.go b/wgengine/watchdog.go index d040b2ae8..2385148c8 100644 --- a/wgengine/watchdog.go +++ b/wgengine/watchdog.go @@ -17,6 +17,7 @@ import ( "tailscale.com/net/dns" "tailscale.com/net/tstun" "tailscale.com/tailcfg" + "tailscale.com/types/key" "tailscale.com/types/netmap" "tailscale.com/wgengine/filter" "tailscale.com/wgengine/magicsock" @@ -112,7 +113,7 @@ func (e *watchdogEngine) AddNetworkMapCallback(callback NetworkMapCallback) func e.watchdog("AddNetworkMapCallback", func() { fn = e.wrap.AddNetworkMapCallback(callback) }) return func() { e.watchdog("RemoveNetworkMapCallback", fn) } } -func (e *watchdogEngine) DiscoPublicKey() (k tailcfg.DiscoKey) { +func (e *watchdogEngine) DiscoPublicKey() (k key.DiscoPublic) { e.watchdog("DiscoPublicKey", func() { k = e.wrap.DiscoPublicKey() }) return k } diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go index 215ba26e8..e8cdd16ee 100644 --- a/wgengine/wgcfg/nmcfg/nmcfg.go +++ b/wgengine/wgcfg/nmcfg/nmcfg.go @@ -10,11 +10,9 @@ import ( "fmt" "strings" - "go4.org/mem" "inet.af/netaddr" "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" - "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/netmap" "tailscale.com/wgengine/wgcfg" @@ -74,7 +72,7 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags, } cfg.Peers = append(cfg.Peers, wgcfg.Peer{ PublicKey: peer.Key, - DiscoKey: key.DiscoPublicFromRaw32(mem.B(peer.DiscoKey[:])), + DiscoKey: peer.DiscoKey, }) cpeer := &cfg.Peers[len(cfg.Peers)-1] if peer.KeepAlive { diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index 22d891215..44e51e639 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -11,6 +11,7 @@ import ( "tailscale.com/ipn/ipnstate" "tailscale.com/net/dns" "tailscale.com/tailcfg" + "tailscale.com/types/key" "tailscale.com/types/netmap" "tailscale.com/wgengine/filter" "tailscale.com/wgengine/monitor" @@ -127,7 +128,7 @@ type Engine interface { // DiscoPublicKey gets the public key used for path discovery // messages. - DiscoPublicKey() tailcfg.DiscoKey + DiscoPublicKey() key.DiscoPublic // UpdateStatus populates the network state using the provided // status builder.