From ebd1637e5076e1dbe06057fad1e4aa93024fec0f Mon Sep 17 00:00:00 2001 From: Tom DNetto Date: Wed, 21 Sep 2022 13:27:58 -0700 Subject: [PATCH] ipn/ipnlocal,tailcfg: Identify client using NodeKey in tka RPCs Updates https://github.com/tailscale/corp/pull/7024 Signed-off-by: Tom DNetto --- ipn/ipnlocal/network-lock.go | 37 ++++++++++++++----------- ipn/ipnlocal/network-lock_test.go | 45 ++++++++++++++++++++----------- tailcfg/tka.go | 39 +++++++++++++++++++-------- 3 files changed, 80 insertions(+), 41 deletions(-) diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index 416b02ad2..56ee9b1ff 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -55,9 +55,6 @@ func (b *LocalBackend) tkaSyncIfNeededLocked(nm *netmap.NetworkMap) error { // If the feature flag is not enabled, pretend we don't exist. return nil } - if nm.SelfNode == nil { - return errors.New("SelfNode missing") - } isEnabled := b.tka != nil wantEnabled := nm.TKAEnabled @@ -69,8 +66,9 @@ func (b *LocalBackend) tkaSyncIfNeededLocked(nm *netmap.NetworkMap) error { // Regardless of whether we are moving to disabled or enabled, we // need information from the tka bootstrap endpoint. + ourNodeKey := b.prefs.Persist.PrivateNodeKey.Public() b.mu.Unlock() - bs, err := b.tkaFetchBootstrap(nm.SelfNode.ID, ourHead) + bs, err := b.tkaFetchBootstrap(ourNodeKey, ourHead) b.mu.Lock() if err != nil { return fmt.Errorf("fetching bootstrap: %v", err) @@ -202,9 +200,15 @@ func (b *LocalBackend) NetworkLockInit(keys []tka.Key) error { if !b.CanSupportNetworkLock() { return errors.New("network-lock is not supported in this configuration. Did you supply a --statedir?") } - nm := b.NetMap() - if nm == nil { - return errors.New("no netmap: are you logged into tailscale?") + + var ourNodeKey key.NodePublic + b.mu.Lock() + if b.prefs != nil { + ourNodeKey = b.prefs.Persist.PrivateNodeKey.Public() + } + b.mu.Unlock() + if ourNodeKey.IsZero() { + return errors.New("no node-key: is tailscale logged in?") } // Generates a genesis AUM representing trust in the provided keys. @@ -226,7 +230,7 @@ func (b *LocalBackend) NetworkLockInit(keys []tka.Key) error { } // Phase 1/2 of initialization: Transmit the genesis AUM to Control. - initResp, err := b.tkaInitBegin(nm, genesisAUM) + initResp, err := b.tkaInitBegin(ourNodeKey, genesisAUM) if err != nil { return fmt.Errorf("tka init-begin RPC: %w", err) } @@ -247,7 +251,7 @@ func (b *LocalBackend) NetworkLockInit(keys []tka.Key) error { } // Finalize enablement by transmitting signature for all nodes to Control. - _, err = b.tkaInitFinish(nm, sigs) + _, err = b.tkaInitFinish(ourNodeKey, sigs) return err } @@ -270,10 +274,11 @@ func signNodeKey(nodeInfo tailcfg.TKASignInfo, signer key.NLPrivate) (*tka.NodeK return &sig, nil } -func (b *LocalBackend) tkaInitBegin(nm *netmap.NetworkMap, aum tka.AUM) (*tailcfg.TKAInitBeginResponse, error) { +func (b *LocalBackend) tkaInitBegin(ourNodeKey key.NodePublic, aum tka.AUM) (*tailcfg.TKAInitBeginResponse, error) { var req bytes.Buffer if err := json.NewEncoder(&req).Encode(tailcfg.TKAInitBeginRequest{ - NodeID: nm.SelfNode.ID, + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: ourNodeKey, GenesisAUM: aum.Serialize(), }); err != nil { return nil, fmt.Errorf("encoding request: %v", err) @@ -311,10 +316,11 @@ func (b *LocalBackend) tkaInitBegin(nm *netmap.NetworkMap, aum tka.AUM) (*tailcf } } -func (b *LocalBackend) tkaInitFinish(nm *netmap.NetworkMap, nks map[tailcfg.NodeID]tkatype.MarshaledSignature) (*tailcfg.TKAInitFinishResponse, error) { +func (b *LocalBackend) tkaInitFinish(ourNodeKey key.NodePublic, nks map[tailcfg.NodeID]tkatype.MarshaledSignature) (*tailcfg.TKAInitFinishResponse, error) { var req bytes.Buffer if err := json.NewEncoder(&req).Encode(tailcfg.TKAInitFinishRequest{ - NodeID: nm.SelfNode.ID, + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: ourNodeKey, Signatures: nks, }); err != nil { return nil, fmt.Errorf("encoding request: %v", err) @@ -354,9 +360,10 @@ func (b *LocalBackend) tkaInitFinish(nm *netmap.NetworkMap, nks map[tailcfg.Node // tkaFetchBootstrap sends a /machine/tka/bootstrap RPC to the control plane // over noise. This is used to get values necessary to enable or disable TKA. -func (b *LocalBackend) tkaFetchBootstrap(nodeID tailcfg.NodeID, head tka.AUMHash) (*tailcfg.TKABootstrapResponse, error) { +func (b *LocalBackend) tkaFetchBootstrap(ourNodeKey key.NodePublic, head tka.AUMHash) (*tailcfg.TKABootstrapResponse, error) { bootstrapReq := tailcfg.TKABootstrapRequest{ - NodeID: nodeID, + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: ourNodeKey, } if !head.IsZero() { head, err := head.MarshalText() diff --git a/ipn/ipnlocal/network-lock_test.go b/ipn/ipnlocal/network-lock_test.go index 5d5c5c80c..3b1d43287 100644 --- a/ipn/ipnlocal/network-lock_test.go +++ b/ipn/ipnlocal/network-lock_test.go @@ -17,10 +17,12 @@ "tailscale.com/control/controlclient" "tailscale.com/hostinfo" + "tailscale.com/ipn" "tailscale.com/tailcfg" "tailscale.com/tka" "tailscale.com/types/key" "tailscale.com/types/netmap" + "tailscale.com/types/persist" ) func fakeControlClient(t *testing.T, c *http.Client) *controlclient.Auto { @@ -62,6 +64,7 @@ func fakeNoiseServer(t *testing.T, handler http.HandlerFunc) (*httptest.Server, func TestTKAEnablementFlow(t *testing.T) { networkLockAvailable = func() bool { return true } // Enable the feature flag + nodePriv := key.NewNode() // Make a fake TKA authority, getting a usable genesis AUM which // our mock server can communicate. @@ -83,8 +86,11 @@ func TestTKAEnablementFlow(t *testing.T) { if err := json.NewDecoder(r.Body).Decode(body); err != nil { t.Fatal(err) } - if body.NodeID != 420 { - t.Errorf("bootstrap nodeID=%v, want 420", body.NodeID) + if body.Version != tailcfg.CurrentCapabilityVersion { + t.Errorf("bootstrap CapVer = %v, want %v", body.Version, tailcfg.CurrentCapabilityVersion) + } + if body.NodeKey != nodePriv.Public() { + t.Errorf("bootstrap nodeKey=%v, want %v", body.NodeKey, nodePriv.Public()) } if body.Head != "" { t.Errorf("bootstrap head=%s, want empty hash", body.Head) @@ -112,11 +118,13 @@ func TestTKAEnablementFlow(t *testing.T) { cc: cc, ccAuto: cc, logf: t.Logf, + prefs: &ipn.Prefs{ + Persist: &persist.Persist{PrivateNodeKey: nodePriv}, + }, } b.mu.Lock() err = b.tkaSyncIfNeededLocked(&netmap.NetworkMap{ - SelfNode: &tailcfg.Node{ID: 420}, TKAEnabled: true, TKAHead: tka.AUMHash{}, }) @@ -136,6 +144,7 @@ func TestTKADisablementFlow(t *testing.T) { networkLockAvailable = func() bool { return true } // Enable the feature flag temp := t.TempDir() os.Mkdir(filepath.Join(temp, "tka"), 0755) + nodePriv := key.NewNode() // Make a fake TKA authority, to seed local state. disablementSecret := bytes.Repeat([]byte{0xa5}, 32) @@ -153,6 +162,7 @@ func TestTKADisablementFlow(t *testing.T) { t.Fatalf("tka.Create() failed: %v", err) } + returnWrongSecret := false ts, client := fakeNoiseServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() switch r.URL.Path { @@ -161,14 +171,11 @@ func TestTKADisablementFlow(t *testing.T) { if err := json.NewDecoder(r.Body).Decode(body); err != nil { t.Fatal(err) } - var disablement []byte - switch body.NodeID { - case 42: - disablement = bytes.Repeat([]byte{0x42}, 32) // wrong secret - case 420: - disablement = disablementSecret - default: - t.Errorf("bootstrap nodeID=%v, wanted 42 or 420", body.NodeID) + if body.Version != tailcfg.CurrentCapabilityVersion { + t.Errorf("bootstrap CapVer = %v, want %v", body.Version, tailcfg.CurrentCapabilityVersion) + } + if body.NodeKey != nodePriv.Public() { + t.Errorf("nodeKey=%v, want %v", body.NodeKey, nodePriv.Public()) } var head tka.AUMHash if err := head.UnmarshalText([]byte(body.Head)); err != nil { @@ -178,6 +185,13 @@ func TestTKADisablementFlow(t *testing.T) { t.Errorf("reported head = %x, want %x", head, authority.Head()) } + var disablement []byte + if returnWrongSecret { + disablement = bytes.Repeat([]byte{0x42}, 32) // wrong secret + } else { + disablement = disablementSecret + } + w.WriteHeader(200) out := tailcfg.TKABootstrapResponse{ DisablementSecret: disablement, @@ -203,13 +217,15 @@ func TestTKADisablementFlow(t *testing.T) { authority: authority, storage: chonk, }, + prefs: &ipn.Prefs{ + Persist: &persist.Persist{PrivateNodeKey: nodePriv}, + }, } // Test that the wrong disablement secret does not shut down the authority. - // NodeID == 42 indicates this scenario to our mock server. + returnWrongSecret = true b.mu.Lock() err = b.tkaSyncIfNeededLocked(&netmap.NetworkMap{ - SelfNode: &tailcfg.Node{ID: 42}, TKAEnabled: false, TKAHead: authority.Head(), }) @@ -222,10 +238,9 @@ func TestTKADisablementFlow(t *testing.T) { } // Test the correct disablement secret shuts down the authority. - // NodeID == 420 indicates this scenario to our mock server. + returnWrongSecret = false b.mu.Lock() err = b.tkaSyncIfNeededLocked(&netmap.NetworkMap{ - SelfNode: &tailcfg.Node{ID: 420}, TKAEnabled: false, TKAHead: authority.Head(), }) diff --git a/tailcfg/tka.go b/tailcfg/tka.go index 39d46ec2c..b3f9f5611 100644 --- a/tailcfg/tka.go +++ b/tailcfg/tka.go @@ -12,9 +12,11 @@ // TKAInitBeginRequest submits a genesis AUM to seed the creation of the // tailnet's key authority. type TKAInitBeginRequest struct { - // NodeID is the node of the initiating client. - // It must match the machine key being used to communicate over noise. - NodeID NodeID + // Version is the client's capabilities. + Version CapabilityVersion + + // NodeKey is the client's current node key. + NodeKey key.NodePublic // GenesisAUM is the initial (genesis) AUM that the node generated // to bootstrap tailnet key authority state. @@ -58,8 +60,11 @@ type TKAInitBeginResponse struct { // This RPC finalizes initialization of the tailnet key authority // by submitting node-key signatures for all existing nodes. type TKAInitFinishRequest struct { - // NodeID is the node ID of the initiating client. - NodeID NodeID + // Version is the client's capabilities. + Version CapabilityVersion + + // NodeKey is the client's current node key. + NodeKey key.NodePublic // Signatures are serialized tka.NodeKeySignatures for all nodes // in the tailnet. @@ -101,8 +106,12 @@ type TKAInfo struct { // TKABootstrapRequest is sent by a node to get information necessary for // enabling or disabling the tailnet key authority. type TKABootstrapRequest struct { - // NodeID is the node ID of the initiating client. - NodeID NodeID + // Version is the client's capabilities. + Version CapabilityVersion + + // NodeKey is the client's current node key. + NodeKey key.NodePublic + // Head represents the node's head AUMHash (tka.Authority.Head), if // network lock is enabled. Head string @@ -122,8 +131,12 @@ type TKABootstrapResponse struct { // state (TKA). Values of type tka.AUMHash are encoded as strings in their // MarshalText form. type TKASyncOfferRequest struct { - // NodeID is the node ID of the initiating client. - NodeID NodeID + // Version is the client's capabilities. + Version CapabilityVersion + + // NodeKey is the client's current node key. + NodeKey key.NodePublic + // Head represents the node's head AUMHash (tka.Authority.Head). This // corresponds to tka.SyncOffer.Head. Head string @@ -151,8 +164,12 @@ type TKASyncOfferResponse struct { // TKASyncSendRequest encodes AUMs that a node believes the control plane // is missing. type TKASyncSendRequest struct { - // NodeID is the node ID of the initiating client. - NodeID NodeID + // Version is the client's capabilities. + Version CapabilityVersion + + // NodeKey is the client's current node key. + NodeKey key.NodePublic + // MissingAUMs encodes AUMs that the node believes the control plane // is missing. MissingAUMs []tkatype.MarshaledAUM