ipn,cmd/tailscale: implement resigning nodes on tka key removal

Signed-off-by: Tom DNetto <tom@tailscale.com>

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{}

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 <public-key>...",
+	ShortUsage: "remove [--re-sign=false] <public-key>...",
 	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 {
+	Exec:       runNetworkLockRemove,
-		return runNetworkLockModify(ctx, nil, args)
+	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

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
+}

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())
+				}
+			}
+		})
+	}
+}

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)