From e2d652ec4dc53dce6c828e0128126ac7db2d6b09 Mon Sep 17 00:00:00 2001 From: Tom DNetto Date: Wed, 1 Mar 2023 12:47:29 -0800 Subject: [PATCH] ipn,cmd/tailscale: implement resigning nodes on tka key removal Signed-off-by: Tom DNetto --- client/tailscale/localclient.go | 10 +++ cmd/tailscale/cli/network-lock.go | 72 +++++++++++++++- ipn/ipnlocal/network-lock.go | 94 +++++++++++++++++++++ ipn/ipnlocal/network-lock_test.go | 133 ++++++++++++++++++++++++++++++ ipn/localapi/localapi.go | 27 ++++++ 5 files changed, 332 insertions(+), 4 deletions(-) diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index 12eb907e1..90b4e9688 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -36,6 +36,7 @@ "tailscale.com/tailcfg" "tailscale.com/tka" "tailscale.com/types/key" + "tailscale.com/types/tkatype" ) // defaultLocalClient is the default LocalClient when using the legacy @@ -886,6 +887,15 @@ type signRequest struct { return nil } +// NetworkLockAffectedSigs returns all signatures signed by the specified keyID. +func (lc *LocalClient) NetworkLockAffectedSigs(ctx context.Context, keyID tkatype.KeyID) ([]tkatype.MarshaledSignature, error) { + body, err := lc.send(ctx, "POST", "/localapi/v0/tka/affected-sigs", 200, bytes.NewReader(keyID)) + if err != nil { + return nil, fmt.Errorf("error: %w", err) + } + return decodeJSON[[]tkatype.MarshaledSignature](body) +} + // NetworkLockLog returns up to maxEntries number of changes to network-lock state. func (lc *LocalClient) NetworkLockLog(ctx context.Context, maxEntries int) ([]ipnstate.NetworkLockUpdate, error) { v := url.Values{} diff --git a/cmd/tailscale/cli/network-lock.go b/cmd/tailscale/cli/network-lock.go index 053aff352..943929eec 100644 --- a/cmd/tailscale/cli/network-lock.go +++ b/cmd/tailscale/cli/network-lock.go @@ -267,14 +267,78 @@ func runNetworkLockStatus(ctx context.Context, args []string) error { }, } +var nlRemoveArgs struct { + resign bool +} + var nlRemoveCmd = &ffcli.Command{ Name: "remove", - ShortUsage: "remove ...", + ShortUsage: "remove [--re-sign=false] ...", ShortHelp: "Removes one or more trusted signing keys from tailnet lock", LongHelp: "Removes one or more trusted signing keys from tailnet lock", - Exec: func(ctx context.Context, args []string) error { - return runNetworkLockModify(ctx, nil, args) - }, + Exec: runNetworkLockRemove, + FlagSet: (func() *flag.FlagSet { + fs := newFlagSet("lock remove") + fs.BoolVar(&nlRemoveArgs.resign, "re-sign", true, "resign signatures which would be invalidated by removal of trusted signing keys") + return fs + })(), +} + +func runNetworkLockRemove(ctx context.Context, args []string) error { + removeKeys, _, err := parseNLArgs(args, true, false) + if err != nil { + return err + } + st, err := localClient.NetworkLockStatus(ctx) + if err != nil { + return fixTailscaledConnectError(err) + } + if !st.Enabled { + return errors.New("tailnet lock is not enabled") + } + + if nlRemoveArgs.resign { + // Validate we are not removing trust in ourselves while resigning. This is because + // we resign with our own key, so the signatures would be immediately invalid. + for _, k := range removeKeys { + kID, err := k.ID() + if err != nil { + return fmt.Errorf("computing KeyID for key %v: %w", k, err) + } + if bytes.Equal(st.PublicKey.KeyID(), kID) { + return errors.New("cannot remove local trusted signing key while resigning; run command on a different node or with --re-sign=false") + } + } + + // Resign affected signatures for each of the keys we are removing. + for _, k := range removeKeys { + kID, _ := k.ID() // err already checked above + sigs, err := localClient.NetworkLockAffectedSigs(ctx, kID) + if err != nil { + return fmt.Errorf("affected sigs for key %X: %w", kID, err) + } + + for _, sigBytes := range sigs { + var sig tka.NodeKeySignature + if err := sig.Unserialize(sigBytes); err != nil { + return fmt.Errorf("failed decoding signature: %w", err) + } + var nodeKey key.NodePublic + if err := nodeKey.UnmarshalBinary(sig.Pubkey); err != nil { + return fmt.Errorf("failed decoding pubkey for signature: %w", err) + } + + // Safety: NetworkLockAffectedSigs() verifies all signatures before + // successfully returning. + rotationKey, _ := sig.UnverifiedWrappingPublic() + if err := localClient.NetworkLockSign(ctx, nodeKey, []byte(rotationKey)); err != nil { + return fmt.Errorf("failed to sign %v: %w", nodeKey, err) + } + } + } + } + + return localClient.NetworkLockModify(ctx, nil, removeKeys) } // parseNLArgs parses a slice of strings into slices of tka.Key & disablement diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index c8b1da764..955ad3a76 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -784,6 +784,64 @@ func (b *LocalBackend) NetworkLockLog(maxEntries int) ([]ipnstate.NetworkLockUpd return out, nil } +// NetworkLockAffectedSigs returns the signatures which would be invalidated +// by removing trust in the specified KeyID. +func (b *LocalBackend) NetworkLockAffectedSigs(keyID tkatype.KeyID) ([]tkatype.MarshaledSignature, error) { + var ( + ourNodeKey key.NodePublic + err error + ) + b.mu.Lock() + if p := b.pm.CurrentPrefs(); p.Valid() && p.Persist().Valid() && !p.Persist().PrivateNodeKey().IsZero() { + ourNodeKey = p.Persist().PublicNodeKey() + } + if b.tka == nil { + err = errNetworkLockNotActive + } + b.mu.Unlock() + if err != nil { + return nil, err + } + + resp, err := b.tkaReadAffectedSigs(ourNodeKey, keyID) + if err != nil { + return nil, err + } + + b.mu.Lock() + defer b.mu.Unlock() + if b.tka == nil { + return nil, errNetworkLockNotActive + } + + // Confirm for ourselves tha the signatures would actually be invalidated + // by removal of trusted in the specified key. + for i, sigBytes := range resp.Signatures { + var sig tka.NodeKeySignature + if err := sig.Unserialize(sigBytes); err != nil { + return nil, fmt.Errorf("failed decoding signature %d: %w", i, err) + } + + sigKeyID, err := sig.UnverifiedAuthorizingKeyID() + if err != nil { + return nil, fmt.Errorf("extracting SigID from signature %d: %w", i, err) + } + if !bytes.Equal(keyID, sigKeyID) { + return nil, fmt.Errorf("got signature with keyID %X from request for %X", sigKeyID, keyID) + } + + var nodeKey key.NodePublic + if err := nodeKey.UnmarshalBinary(sig.Pubkey); err != nil { + return nil, fmt.Errorf("failed decoding pubkey for signature %d: %w", i, err) + } + if err := b.tka.authority.NodeKeyAuthorized(nodeKey, sigBytes); err != nil { + return nil, fmt.Errorf("signature %d is not valid: %w", i, err) + } + } + + return resp.Signatures, nil +} + func signNodeKey(nodeInfo tailcfg.TKASignInfo, signer key.NLPrivate) (*tka.NodeKeySignature, error) { p, err := nodeInfo.NodePublic.MarshalBinary() if err != nil { @@ -1110,3 +1168,39 @@ func (b *LocalBackend) tkaSubmitSignature(ourNodeKey key.NodePublic, sig tkatype return a, nil } + +func (b *LocalBackend) tkaReadAffectedSigs(ourNodeKey key.NodePublic, key tkatype.KeyID) (*tailcfg.TKASignaturesUsingKeyResponse, error) { + var encodedReq bytes.Buffer + if err := json.NewEncoder(&encodedReq).Encode(tailcfg.TKASignaturesUsingKeyRequest{ + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: ourNodeKey, + KeyID: key, + }); err != nil { + return nil, fmt.Errorf("encoding request: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, "GET", "https://unused/machine/tka/affected-sigs", &encodedReq) + if err != nil { + return nil, fmt.Errorf("req: %w", err) + } + resp, err := b.DoNoiseRequest(req) + if err != nil { + return nil, fmt.Errorf("resp: %w", err) + } + if resp.StatusCode != 200 { + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + return nil, fmt.Errorf("request returned (%d): %s", resp.StatusCode, string(body)) + } + a := new(tailcfg.TKASignaturesUsingKeyResponse) + err = json.NewDecoder(&io.LimitedReader{R: resp.Body, N: 1024 * 1024}).Decode(a) + resp.Body.Close() + if err != nil { + return nil, fmt.Errorf("decoding JSON: %w", err) + } + + return a, nil +} diff --git a/ipn/ipnlocal/network-lock_test.go b/ipn/ipnlocal/network-lock_test.go index daf1d0d06..31ff5adf0 100644 --- a/ipn/ipnlocal/network-lock_test.go +++ b/ipn/ipnlocal/network-lock_test.go @@ -7,6 +7,7 @@ "bytes" "context" "encoding/json" + "fmt" "net" "net/http" "net/http/httptest" @@ -877,3 +878,135 @@ func TestTKAForceDisable(t *testing.T) { t.Fatal("tka was re-initalized") } } + +func TestTKAAffectedSigs(t *testing.T) { + nodePriv := key.NewNode() + // toSign := key.NewNode() + nlPriv := key.NewNLPrivate() + + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + must.Do(pm.SetPrefs((&ipn.Prefs{ + Persist: &persist.Persist{ + PrivateNodeKey: nodePriv, + NetworkLockKey: nlPriv, + }, + }).View())) + + // Make a fake TKA authority, to seed local state. + disablementSecret := bytes.Repeat([]byte{0xa5}, 32) + tkaKey := tka.Key{Kind: tka.Key25519, Public: nlPriv.Public().Verifier(), Votes: 2} + + temp := t.TempDir() + tkaPath := filepath.Join(temp, "tka-profile", string(pm.CurrentProfile().ID)) + os.Mkdir(tkaPath, 0755) + chonk, err := tka.ChonkDir(tkaPath) + if err != nil { + t.Fatal(err) + } + authority, _, err := tka.Create(chonk, tka.State{ + Keys: []tka.Key{tkaKey}, + DisablementSecrets: [][]byte{tka.DisablementKDF(disablementSecret)}, + }, nlPriv) + if err != nil { + t.Fatalf("tka.Create() failed: %v", err) + } + + untrustedKey := key.NewNLPrivate() + tcs := []struct { + name string + makeSig func() *tka.NodeKeySignature + wantErr string + }{ + { + "no error", + func() *tka.NodeKeySignature { + sig, _ := signNodeKey(tailcfg.TKASignInfo{NodePublic: nodePriv.Public()}, nlPriv) + return sig + }, + "", + }, + { + "signature for different keyID", + func() *tka.NodeKeySignature { + sig, _ := signNodeKey(tailcfg.TKASignInfo{NodePublic: nodePriv.Public()}, untrustedKey) + return sig + }, + fmt.Sprintf("got signature with keyID %X from request for %X", untrustedKey.KeyID(), nlPriv.KeyID()), + }, + { + "invalid signature", + func() *tka.NodeKeySignature { + sig, _ := signNodeKey(tailcfg.TKASignInfo{NodePublic: nodePriv.Public()}, nlPriv) + copy(sig.Signature, []byte{1, 2, 3, 4, 5, 6}) // overwrite with trash to invalid signature + return sig + }, + "signature 0 is not valid: invalid signature", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + s := tc.makeSig() + ts, client := fakeNoiseServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + switch r.URL.Path { + case "/machine/tka/affected-sigs": + body := new(tailcfg.TKASignaturesUsingKeyRequest) + if err := json.NewDecoder(r.Body).Decode(body); err != nil { + t.Fatal(err) + } + if body.Version != tailcfg.CurrentCapabilityVersion { + t.Errorf("sign CapVer = %v, want %v", body.Version, tailcfg.CurrentCapabilityVersion) + } + if body.NodeKey != nodePriv.Public() { + t.Errorf("nodeKey = %v, want %v", body.NodeKey, nodePriv.Public()) + } + + w.WriteHeader(200) + if err := json.NewEncoder(w).Encode(tailcfg.TKASignaturesUsingKeyResponse{ + Signatures: []tkatype.MarshaledSignature{s.Serialize()}, + }); err != nil { + t.Fatal(err) + } + + default: + t.Errorf("unhandled endpoint path: %v", r.URL.Path) + w.WriteHeader(404) + } + })) + defer ts.Close() + cc := fakeControlClient(t, client) + b := LocalBackend{ + varRoot: temp, + cc: cc, + ccAuto: cc, + logf: t.Logf, + tka: &tkaState{ + authority: authority, + storage: chonk, + }, + pm: pm, + store: pm.Store(), + } + + sigs, err := b.NetworkLockAffectedSigs(nlPriv.KeyID()) + switch { + case tc.wantErr == "" && err != nil: + t.Errorf("NetworkLockAffectedSigs() failed: %v", err) + case tc.wantErr != "" && err == nil: + t.Errorf("NetworkLockAffectedSigs().err = nil, want %q", tc.wantErr) + case tc.wantErr != "" && err.Error() != tc.wantErr: + t.Errorf("NetworkLockAffectedSigs().err = %q, want %q", err.Error(), tc.wantErr) + } + + if tc.wantErr == "" { + if len(sigs) != 1 { + t.Fatalf("len(sigs) = %d, want 1", len(sigs)) + } + if !bytes.Equal(s.Serialize(), sigs[0]) { + t.Errorf("unexpected signature: got %v, want %v", sigs[0], s.Serialize()) + } + } + }) + } +} diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 64828adab..7294d88ed 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -99,6 +99,7 @@ "tka/status": (*Handler).serveTKAStatus, "tka/disable": (*Handler).serveTKADisable, "tka/force-local-disable": (*Handler).serveTKALocalDisable, + "tka/affected-sigs": (*Handler).serveTKAAffectedSigs, "upload-client-metrics": (*Handler).serveUploadClientMetrics, "watch-ipn-bus": (*Handler).serveWatchIPNBus, "whois": (*Handler).serveWhoIs, @@ -1601,6 +1602,32 @@ func (h *Handler) serveTKALog(w http.ResponseWriter, r *http.Request) { w.Write(j) } +func (h *Handler) serveTKAAffectedSigs(w http.ResponseWriter, r *http.Request) { + if r.Method != httpm.POST { + http.Error(w, "use POST", http.StatusMethodNotAllowed) + return + } + keyID, err := ioutil.ReadAll(http.MaxBytesReader(w, r.Body, 2048)) + if err != nil { + http.Error(w, "reading body", http.StatusBadRequest) + return + } + + sigs, err := h.b.NetworkLockAffectedSigs(keyID) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + j, err := json.MarshalIndent(sigs, "", "\t") + if err != nil { + http.Error(w, "JSON encoding error", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.Write(j) +} + // serveProfiles serves profile switching-related endpoints. Supported methods // and paths are: // - GET /profiles/: list all profiles (JSON-encoded array of ipn.LoginProfiles)