From 7e6a1ef4f1511e03d24d19f6b3a51d4504cac8f7 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 1 Nov 2021 20:55:52 -0700 Subject: [PATCH] tailcfg: use key.NodePublic in wire protocol types. Updates #3206. Signed-off-by: David Anderson --- control/controlclient/direct.go | 6 ++-- ipn/ipnlocal/local.go | 5 ++- tailcfg/tailcfg.go | 10 +++--- tailcfg/tailcfg_clone.go | 2 +- tailcfg/tailcfg_test.go | 8 ++--- tstest/integration/integration_test.go | 2 +- tstest/integration/testcontrol/testcontrol.go | 10 +++--- types/netmap/netmap_test.go | 36 +++++++++---------- wgengine/magicsock/magicsock.go | 10 +++--- wgengine/magicsock/magicsock_test.go | 14 ++++---- wgengine/pendopen.go | 4 +-- wgengine/userspace.go | 2 +- wgengine/userspace_test.go | 2 +- wgengine/wgcfg/nmcfg/nmcfg.go | 2 +- 14 files changed, 56 insertions(+), 57 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index fa74cae3b..b92de5072 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -357,8 +357,8 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new now := time.Now().Round(time.Second) request := tailcfg.RegisterRequest{ Version: 1, - OldNodeKey: oldNodeKey.AsNodeKey(), - NodeKey: tryingNewKey.Public().AsNodeKey(), + OldNodeKey: oldNodeKey, + NodeKey: tryingNewKey.Public(), Hostinfo: hostinfo, Followup: opt.URL, Timestamp: &now, @@ -595,7 +595,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm request := &tailcfg.MapRequest{ Version: tailcfg.CurrentMapRequestVersion, KeepAlive: c.keepAlive, - NodeKey: persist.PrivateNodeKey.Public().AsNodeKey(), + NodeKey: persist.PrivateNodeKey.Public(), DiscoKey: c.discoPubKey, Endpoints: epStrs, EndpointTypes: epTypes, diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 11dab36ae..9aa9d0a24 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -26,7 +26,6 @@ import ( "time" "github.com/go-multierror/multierror" - "go4.org/mem" "inet.af/netaddr" "tailscale.com/client/tailscale/apitype" "tailscale.com/control/controlclient" @@ -389,7 +388,7 @@ func (b *LocalBackend) populatePeerStatusLocked(sb *ipnstate.StatusBuilder) { tailscaleIPs = append(tailscaleIPs, addr.IP()) } } - sb.AddPeer(key.NodePublicFromRaw32(mem.B(p.Key[:])), &ipnstate.PeerStatus{ + sb.AddPeer(p.Key, &ipnstate.PeerStatus{ InNetworkMap: true, ID: p.StableID, UserID: p.User, @@ -2782,7 +2781,7 @@ func (b *LocalBackend) SetDNS(ctx context.Context, name, value string) error { b.mu.Lock() cc := b.cc if prefs := b.prefs; prefs != nil { - req.NodeKey = prefs.Persist.PrivateNodeKey.Public().AsNodeKey() + req.NodeKey = prefs.Persist.PrivateNodeKey.Public() } b.mu.Unlock() if cc == nil { diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index d895cdbb8..09408e2fc 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -166,7 +166,7 @@ type Node struct { // Sharer, if non-zero, is the user who shared this node, if different than User. Sharer UserID `json:",omitempty"` - Key NodeKey + Key key.NodePublic KeyExpiry time.Time Machine key.MachinePublic DiscoKey DiscoKey @@ -638,8 +638,8 @@ func (st SignatureType) String() string { type RegisterRequest struct { _ structs.Incomparable Version int // currently 1 - NodeKey NodeKey - OldNodeKey NodeKey + NodeKey key.NodePublic + OldNodeKey key.NodePublic Auth struct { _ structs.Incomparable // One of Provider/LoginName, Oauth2Token, or AuthKey is set. @@ -756,7 +756,7 @@ type MapRequest struct { Compress string // "zstd" or "" (no compression) KeepAlive bool // whether server should send keep-alives back to us - NodeKey NodeKey + NodeKey key.NodePublic DiscoKey DiscoKey IncludeIPv6 bool `json:",omitempty"` // include IPv6 endpoints in returned Node Endpoints (for Version 4 clients) Stream bool // if true, multiple MapResponse objects are returned @@ -1284,7 +1284,7 @@ type SetDNSRequest struct { Version int // NodeKey is the client's current node key. - NodeKey NodeKey + NodeKey key.NodePublic // Name is the domain name for which to create a record. // For ACME DNS-01 challenges, it should be one of the domains diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 53db74262..6315bb85b 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -72,7 +72,7 @@ var _NodeCloneNeedsRegeneration = Node(struct { Name string User UserID Sharer UserID - Key key.NodeKey + Key key.NodePublic KeyExpiry time.Time Machine key.MachinePublic DiscoKey DiscoKey diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index edd730d2b..1b4bc510b 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -264,13 +264,13 @@ func TestNodeEqual(t *testing.T) { true, }, { - &Node{Key: n1.AsNodeKey()}, - &Node{Key: key.NewNode().Public().AsNodeKey()}, + &Node{Key: n1}, + &Node{Key: key.NewNode().Public()}, false, }, { - &Node{Key: n1.AsNodeKey()}, - &Node{Key: n1.AsNodeKey()}, + &Node{Key: n1}, + &Node{Key: n1}, true, }, { diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 321072172..18a7e0aa5 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -315,7 +315,7 @@ func TestAddPingRequest(t *testing.T) { t.Fatalf("expected 1 node, got %d nodes", len(nodes)) } - nodeKey := nodes[0].Key.AsNodePublic() + nodeKey := nodes[0].Key // Check that we get at least one ping reply after 10 tries. for try := 1; try <= 10; try++ { diff --git a/tstest/integration/testcontrol/testcontrol.go b/tstest/integration/testcontrol/testcontrol.go index a674381a4..4d3e3f869 100644 --- a/tstest/integration/testcontrol/testcontrol.go +++ b/tstest/integration/testcontrol/testcontrol.go @@ -286,7 +286,7 @@ func (s *Server) AddFakeNode() { StableID: tailcfg.StableNodeID(fmt.Sprintf("TESTCTRL%08x", id)), User: tailcfg.UserID(id), Machine: mk, - Key: nk.AsNodeKey(), + Key: nk, MachineAuthorized: true, DiscoKey: dk, Addresses: []netaddr.IPPrefix{addr}, @@ -434,7 +434,7 @@ func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key. // some follow-ups? For now all are successes. } - nk := req.NodeKey.AsNodePublic() + nk := req.NodeKey user, login := s.getUser(nk) s.mu.Lock() @@ -538,7 +538,7 @@ func (s *Server) UpdateNode(n *tailcfg.Node) (peersToUpdate []tailcfg.NodeID) { if n.Key.IsZero() { panic("zero nodekey") } - s.nodes[n.Key.AsNodePublic()] = n.Clone() + s.nodes[n.Key] = n.Clone() for _, n2 := range s.nodes { if n.ID != n2.ID { peersToUpdate = append(peersToUpdate, n2.ID) @@ -581,7 +581,7 @@ func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey key.Machi jitter := time.Duration(rand.Intn(8000)) * time.Millisecond keepAlive := 50*time.Second + jitter - node := s.Node(req.NodeKey.AsNodePublic()) + node := s.Node(req.NodeKey) if node == nil { http.Error(w, "node not found", 400) return @@ -693,7 +693,7 @@ var keepAliveMsg = &struct { // // No updates to s are done here. func (s *Server) MapResponse(req *tailcfg.MapRequest) (res *tailcfg.MapResponse, err error) { - nk := req.NodeKey.AsNodePublic() + nk := req.NodeKey node := s.Node(nk) if node == nil { // node key rotated away (once test server supports that) diff --git a/types/netmap/netmap_test.go b/types/netmap/netmap_test.go index 1bece7522..c0693bc2f 100644 --- a/types/netmap/netmap_test.go +++ b/types/netmap/netmap_test.go @@ -43,12 +43,12 @@ func TestNetworkMapConcise(t *testing.T) { NodeKey: testNodeKey(1), Peers: []*tailcfg.Node{ { - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, { - Key: testNodeKey(3).AsNodeKey(), + Key: testNodeKey(3), DERP: "127.3.3.40:4", Endpoints: []string{"10.2.0.100:12", "10.1.0.100:12345"}, }, @@ -98,7 +98,7 @@ func TestConciseDiffFrom(t *testing.T) { NodeKey: testNodeKey(1), Peers: []*tailcfg.Node{ { - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -108,7 +108,7 @@ func TestConciseDiffFrom(t *testing.T) { NodeKey: testNodeKey(1), Peers: []*tailcfg.Node{ { - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -122,7 +122,7 @@ func TestConciseDiffFrom(t *testing.T) { NodeKey: testNodeKey(1), Peers: []*tailcfg.Node{ { - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -132,7 +132,7 @@ func TestConciseDiffFrom(t *testing.T) { NodeKey: testNodeKey(2), Peers: []*tailcfg.Node{ { - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -147,7 +147,7 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -158,19 +158,19 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 1, - Key: testNodeKey(1).AsNodeKey(), + Key: testNodeKey(1), DERP: "127.3.3.40:1", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, { ID: 3, - Key: testNodeKey(3).AsNodeKey(), + Key: testNodeKey(3), DERP: "127.3.3.40:3", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -185,19 +185,19 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 1, - Key: testNodeKey(1).AsNodeKey(), + Key: testNodeKey(1), DERP: "127.3.3.40:1", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, { ID: 3, - Key: testNodeKey(3).AsNodeKey(), + Key: testNodeKey(3), DERP: "127.3.3.40:3", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -208,7 +208,7 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, @@ -223,7 +223,7 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "1.1.1.1:1"}, }, @@ -234,7 +234,7 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "1.1.1.1:2"}, }, @@ -249,7 +249,7 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:41641", "1.1.1.1:41641"}, DiscoKey: testDiscoKey("f00f00f00f"), @@ -262,7 +262,7 @@ func TestConciseDiffFrom(t *testing.T) { Peers: []*tailcfg.Node{ { ID: 2, - Key: testNodeKey(2).AsNodeKey(), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:41641", "1.1.1.1:41641"}, DiscoKey: testDiscoKey("ba4ba4ba4b"), diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 4b0680340..d02e03220 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -898,7 +898,7 @@ func (c *Conn) Ping(peer *tailcfg.Node, res *ipnstate.PingResult, cb func(*ipnst } } - ep, ok := c.peerMap.endpointForNodeKey(peer.Key.AsNodePublic()) + ep, ok := c.peerMap.endpointForNodeKey(peer.Key) if !ok { res.Err = "unknown peer" cb(res) @@ -2256,7 +2256,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) { // we'll fall through to the next pass, which allocates but can // handle full set updates. for _, n := range nm.Peers { - if ep, ok := c.peerMap.endpointForNodeKey(n.Key.AsNodePublic()); ok { + if ep, ok := c.peerMap.endpointForNodeKey(n.Key); ok { ep.updateFromNode(n) c.peerMap.upsertEndpoint(ep) // maybe update discokey mappings in peerMap continue @@ -2264,7 +2264,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) { ep := &endpoint{ c: c, - publicKey: n.Key.AsNodePublic(), + publicKey: n.Key, sentPing: map[stun.TxID]sentPing{}, endpointState: map[netaddr.IPPort]*endpointState{}, } @@ -2272,7 +2272,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) { ep.discoKey = key.DiscoPublicFromRaw32(mem.B(n.DiscoKey[:])) ep.discoShort = n.DiscoKey.ShortString() } - ep.wgEndpoint = key.NodePublicFromRaw32(mem.B(n.Key[:])).UntypedHexString() + ep.wgEndpoint = n.Key.UntypedHexString() ep.initFakeUDPAddr() c.logf("magicsock: created endpoint key=%s: disco=%s; %v", n.Key.ShortString(), n.DiscoKey.ShortString(), logger.ArgWriter(func(w *bufio.Writer) { const derpPrefix = "127.3.3.40:" @@ -2309,7 +2309,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) { if c.peerMap.nodeCount() != len(nm.Peers) { keep := make(map[key.NodePublic]bool, len(nm.Peers)) for _, n := range nm.Peers { - keep[n.Key.AsNodePublic()] = true + keep[n.Key] = true } c.peerMap.forEachEndpoint(func(ep *endpoint) { if !keep[ep.publicKey] { diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 9100de472..b9854b0bb 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -258,7 +258,7 @@ func meshStacks(logf logger.Logf, mutateNetmap func(idx int, nm *netmap.NetworkM peer := &tailcfg.Node{ ID: tailcfg.NodeID(i + 1), Name: fmt.Sprintf("node%d", i+1), - Key: peer.privateKey.Public().AsNodeKey(), + Key: peer.privateKey.Public(), DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(peer.conn.DiscoPublicKey()), Addresses: addrs, AllowedIPs: addrs, @@ -285,7 +285,7 @@ func meshStacks(logf logger.Logf, mutateNetmap func(idx int, nm *netmap.NetworkM m.conn.SetNetworkMap(nm) peerSet := make(map[key.NodePublic]struct{}, len(nm.Peers)) for _, peer := range nm.Peers { - peerSet[key.NodePublicFromRaw32(mem.B(peer.Key[:]))] = struct{}{} + peerSet[peer.Key] = struct{}{} } m.conn.UpdatePeers(peerSet) wg, err := nmcfg.WGCfg(nm, logf, netmap.AllowSingleHosts, "") @@ -1136,11 +1136,11 @@ func TestDiscoMessage(t *testing.T) { peer1Pub := c.DiscoPublicKey() peer1Priv := c.discoPrivate n := &tailcfg.Node{ - Key: key.NewNode().Public().AsNodeKey(), + Key: key.NewNode().Public(), DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(peer1Pub), } c.peerMap.upsertEndpoint(&endpoint{ - publicKey: n.Key.AsNodePublic(), + publicKey: n.Key, discoKey: key.DiscoPublicFromRaw32(mem.B(n.DiscoKey[:])), }) @@ -1232,7 +1232,7 @@ func addTestEndpoint(tb testing.TB, conn *Conn, sendConn net.PacketConn) (key.No conn.SetNetworkMap(&netmap.NetworkMap{ Peers: []*tailcfg.Node{ { - Key: nodeKey.AsNodeKey(), + Key: nodeKey, DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(discoKey), Endpoints: []string{sendConn.LocalAddr().String()}, }, @@ -1410,7 +1410,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) { conn.SetNetworkMap(&netmap.NetworkMap{ Peers: []*tailcfg.Node{ { - Key: nodeKey1.AsNodeKey(), + Key: nodeKey1, DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(discoKey), Endpoints: []string{"192.168.1.2:345"}, }, @@ -1425,7 +1425,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) { conn.SetNetworkMap(&netmap.NetworkMap{ Peers: []*tailcfg.Node{ { - Key: nodeKey2.AsNodeKey(), + Key: nodeKey2, DiscoKey: tailcfg.DiscoKeyFromDiscoPublic(discoKey), Endpoints: []string{"192.168.1.2:345"}, }, diff --git a/wgengine/pendopen.go b/wgengine/pendopen.go index e78cec17f..574c73435 100644 --- a/wgengine/pendopen.go +++ b/wgengine/pendopen.go @@ -178,7 +178,7 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) { var ps *ipnstate.PeerStatusLite if st, err := e.getStatus(); err == nil { for _, v := range st.Peers { - if v.NodeKey == n.Key.AsNodePublic() { + if v.NodeKey == n.Key { v := v // copy ps = &v } @@ -231,7 +231,7 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) { e.logf("open-conn-track: timeout opening %v to node %v; online=%v, lastRecv=%v", flow, n.Key.ShortString(), online, - e.magicConn.LastRecvActivityOfNodeKey(n.Key.AsNodePublic())) + e.magicConn.LastRecvActivityOfNodeKey(n.Key)) } func durFmt(t time.Time) string { diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 15aa2cd0d..969851f5b 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -1471,7 +1471,7 @@ func (e *userspaceEngine) peerForIP(ip netaddr.IP) (n *tailcfg.Node, isSelf bool // call. But TODO(bradfitz): add a lookup map to netmap.NetworkMap. if !bestKey.IsZero() { for _, p := range nm.Peers { - if p.Key.AsNodePublic() == bestKey { + if p.Key == bestKey { return p, false, nil } } diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go index 91393df44..e1550f454 100644 --- a/wgengine/userspace_test.go +++ b/wgengine/userspace_test.go @@ -100,7 +100,7 @@ func TestUserspaceEngineReconfig(t *testing.T) { nm := &netmap.NetworkMap{ Peers: []*tailcfg.Node{ &tailcfg.Node{ - Key: nkFromHex(nodeHex).AsNodeKey(), + Key: nkFromHex(nodeHex), }, }, } diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go index 58e66e34d..215ba26e8 100644 --- a/wgengine/wgcfg/nmcfg/nmcfg.go +++ b/wgengine/wgcfg/nmcfg/nmcfg.go @@ -73,7 +73,7 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags, continue } cfg.Peers = append(cfg.Peers, wgcfg.Peer{ - PublicKey: key.NodePublicFromRaw32(mem.B(peer.Key[:])), + PublicKey: peer.Key, DiscoKey: key.DiscoPublicFromRaw32(mem.B(peer.DiscoKey[:])), }) cpeer := &cfg.Peers[len(cfg.Peers)-1]