cmd/tailscale, client, ipn, tailcfg: add network lock modify command

Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>

client/tailscale/localclient.go

@@ -748,6 +748,30 @@ func (lc *LocalClient) NetworkLockInit(ctx context.Context, keys []tka.Key) (*ip
 	return pr, nil
 }
 
+// NetworkLockModify adds and/or removes key(s) to the tailnet key authority.
+func (lc *LocalClient) NetworkLockModify(ctx context.Context, addKeys, removeKeys []tka.Key) (*ipnstate.NetworkLockStatus, error) {
+	var b bytes.Buffer
+	type modifyRequest struct {
+		AddKeys    []tka.Key
+		RemoveKeys []tka.Key
+	}
+
+	if err := json.NewEncoder(&b).Encode(modifyRequest{AddKeys: addKeys, RemoveKeys: removeKeys}); err != nil {
+		return nil, err
+	}
+
+	body, err := lc.send(ctx, "POST", "/localapi/v0/tka/modify", 200, &b)
+	if err != nil {
+		return nil, fmt.Errorf("error: %w", err)
+	}
+
+	pr := new(ipnstate.NetworkLockStatus)
+	if err := json.Unmarshal(body, pr); err != nil {
+		return nil, err
+	}
+	return pr, nil
+}
+
 // tailscaledConnectHint gives a little thing about why tailscaled (or
 // platform equivalent) is not answering localapi connections.
 //

cmd/tailscale/cli/network-lock.go

@@ -17,11 +17,16 @@ import (
 )
 
 var netlockCmd = &ffcli.Command{
-	Name:        "lock",
-	ShortUsage:  "lock <sub-command> <arguments>",
-	ShortHelp:   "Manipulate the tailnet key authority",
-	Subcommands: []*ffcli.Command{nlInitCmd, nlStatusCmd},
-	Exec:        runNetworkLockStatus,
+	Name:       "lock",
+	ShortUsage: "lock <sub-command> <arguments>",
+	ShortHelp:  "Manipulate the tailnet key authority",
+	Subcommands: []*ffcli.Command{
+		nlInitCmd,
+		nlStatusCmd,
+		nlAddCmd,
+		nlRemoveCmd,
+	},
+	Exec: runNetworkLockStatus,
 }
 
 var nlInitCmd = &ffcli.Command{
@@ -41,29 +46,9 @@ func runNetworkLockInit(ctx context.Context, args []string) error {
 	}
 
 	// Parse the set of initially-trusted keys.
-	// Keys are specified using their key.NLPublic.MarshalText representation,
-	// with an optional '?<votes>' suffix.
-	var keys []tka.Key
-	for i, a := range args {
-		var key key.NLPublic
-		spl := strings.SplitN(a, "?", 2)
-		if err := key.UnmarshalText([]byte(spl[0])); err != nil {
-			return fmt.Errorf("parsing key %d: %v", i+1, err)
-		}
-
-		k := tka.Key{
-			Kind:   tka.Key25519,
-			Public: key.Verifier(),
-			Votes:  1,
-		}
-		if len(spl) > 1 {
-			votes, err := strconv.Atoi(spl[1])
-			if err != nil {
-				return fmt.Errorf("parsing key %d votes: %v", i+1, err)
-			}
-			k.Votes = uint(votes)
-		}
-		keys = append(keys, k)
+	keys, err := parseNLKeyArgs(args)
+	if err != nil {
+		return err
 	}
 
 	status, err := localClient.NetworkLockInit(ctx, keys)
@@ -99,3 +84,78 @@ func runNetworkLockStatus(ctx context.Context, args []string) error {
 	fmt.Printf("our public-key: %s\n", p)
 	return nil
 }
+
+var nlAddCmd = &ffcli.Command{
+	Name:       "add",
+	ShortUsage: "add <public-key>...",
+	ShortHelp:  "Adds one or more signing keys to the tailnet key authority",
+	Exec: func(ctx context.Context, args []string) error {
+		return runNetworkLockModify(ctx, args, nil)
+	},
+}
+
+var nlRemoveCmd = &ffcli.Command{
+	Name:       "remove",
+	ShortUsage: "remove <public-key>...",
+	ShortHelp:  "Removes one or more signing keys to the tailnet key authority",
+	Exec: func(ctx context.Context, args []string) error {
+		return runNetworkLockModify(ctx, nil, args)
+	},
+}
+
+// parseNLKeyArgs converts a slice of strings into a slice of tka.Key. The keys
+// should be specified using their key.NLPublic.MarshalText representation with
+// an optional '?<votes>' suffix. If any of the keys encounters an error, a nil
+// slice is returned along with an appropriate error.
+func parseNLKeyArgs(args []string) ([]tka.Key, error) {
+	var keys []tka.Key
+	for i, a := range args {
+		var nlpk key.NLPublic
+		spl := strings.SplitN(a, "?", 2)
+		if err := nlpk.UnmarshalText([]byte(spl[0])); err != nil {
+			return nil, fmt.Errorf("parsing key %d: %v", i+1, err)
+		}
+
+		k := tka.Key{
+			Kind:   tka.Key25519,
+			Public: nlpk.Verifier(),
+			Votes:  1,
+		}
+		if len(spl) > 1 {
+			votes, err := strconv.Atoi(spl[1])
+			if err != nil {
+				return nil, fmt.Errorf("parsing key %d votes: %v", i+1, err)
+			}
+			k.Votes = uint(votes)
+		}
+		keys = append(keys, k)
+	}
+	return keys, nil
+}
+
+func runNetworkLockModify(ctx context.Context, addArgs, removeArgs []string) error {
+	st, err := localClient.NetworkLockStatus(ctx)
+	if err != nil {
+		return fixTailscaledConnectError(err)
+	}
+	if st.Enabled {
+		return errors.New("network-lock is already enabled")
+	}
+
+	addKeys, err := parseNLKeyArgs(addArgs)
+	if err != nil {
+		return err
+	}
+	removeKeys, err := parseNLKeyArgs(removeArgs)
+	if err != nil {
+		return err
+	}
+
+	status, err := localClient.NetworkLockModify(ctx, addKeys, removeKeys)
+	if err != nil {
+		return err
+	}
+
+	fmt.Printf("Status: %+v\n\n", status)
+	return nil
+}

ipn/ipnlocal/network-lock.go

@@ -16,6 +16,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"golang.org/x/exp/slices"
 	"tailscale.com/envknob"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/logtail/backoff"
@@ -28,6 +29,11 @@ import (
 
 var networkLockAvailable = envknob.RegisterBool("TS_EXPERIMENTAL_NETWORK_LOCK")
 
+var (
+	errMissingNetmap        = errors.New("missing netmap: verify that you are logged in")
+	errNetworkLockNotActive = errors.New("network-lock is not active")
+)
+
 type tkaState struct {
 	authority *tka.Authority
 	storage   *tka.FS
@@ -202,8 +208,8 @@ func (b *LocalBackend) chonkPath() string {
 //
 // b.mu must be held.
 func (b *LocalBackend) tkaBootstrapFromGenesisLocked(g tkatype.MarshaledAUM) error {
-	if !b.CanSupportNetworkLock() {
-		return errors.New("network lock not supported in this configuration")
+	if err := b.CanSupportNetworkLock(); err != nil {
+		return err
 	}
 
 	var genesis tka.AUM
@@ -232,21 +238,26 @@ func (b *LocalBackend) tkaBootstrapFromGenesisLocked(g tkatype.MarshaledAUM) err
 	return nil
 }
 
-// CanSupportNetworkLock returns true if tailscaled is able to operate
+// CanSupportNetworkLock returns nil if tailscaled is able to operate
 // a local tailnet key authority (and hence enforce network lock).
-func (b *LocalBackend) CanSupportNetworkLock() bool {
-	if b.tka != nil {
-		// The TKA is being used, so yeah its supported.
-		return true
+func (b *LocalBackend) CanSupportNetworkLock() error {
+	if !networkLockAvailable() {
+		return errors.New("this feature is not yet complete, a later release may support this functionality")
 	}
 
-	if b.TailscaleVarRoot() != "" {
-		// Theres a var root (aka --statedir), so if network lock gets
-		// initialized we have somewhere to store our AUMs. Thats all
-		// we need.
-		return true
+	if b.tka != nil {
+		// If the TKA is being used, it is supported.
+		return nil
 	}
-	return false
+
+	if b.TailscaleVarRoot() == "" {
+		return errors.New("network-lock is not supported in this configuration, try setting --statedir")
+	}
+
+	// There's a var root (aka --statedir), so if network lock gets
+	// initialized we have somewhere to store our AUMs. That's all
+	// we need.
+	return nil
 }
 
 // NetworkLockStatus returns a structure describing the state of the
@@ -280,14 +291,8 @@ func (b *LocalBackend) NetworkLockStatus() *ipnstate.NetworkLockStatus {
 // The Finish RPC submits signatures for all these nodes, at which point
 // Control has everything it needs to atomically enable network lock.
 func (b *LocalBackend) NetworkLockInit(keys []tka.Key) error {
-	if b.tka != nil {
-		return errors.New("network-lock is already initialized")
-	}
-	if !networkLockAvailable() {
-		return errors.New("this is an experimental feature in your version of tailscale - Please upgrade to the latest to use this.")
-	}
-	if !b.CanSupportNetworkLock() {
-		return errors.New("network-lock is not supported in this configuration. Did you supply a --statedir?")
+	if err := b.CanSupportNetworkLock(); err != nil {
+		return err
 	}
 
 	var ourNodeKey key.NodePublic
@@ -344,6 +349,117 @@ func (b *LocalBackend) NetworkLockInit(keys []tka.Key) error {
 	return err
 }
 
+// NetworkLockModify adds and/or removes keys in the tailnet's key authority.
+func (b *LocalBackend) NetworkLockModify(addKeys, removeKeys []tka.Key) (err error) {
+	defer func() {
+		if err != nil {
+			err = fmt.Errorf("modify network-lock keys: %w", err)
+		}
+	}()
+
+	b.mu.Lock()
+	defer b.mu.Unlock()
+
+	if err := b.CanSupportNetworkLock(); err != nil {
+		return err
+	}
+	if b.tka == nil {
+		return errNetworkLockNotActive
+	}
+	nm := b.NetMap()
+	if nm == nil {
+		return errMissingNetmap
+	}
+
+	updater := b.tka.authority.NewUpdater(b.nlPrivKey)
+
+	for _, addKey := range addKeys {
+		if err := updater.AddKey(addKey); err != nil {
+			return err
+		}
+	}
+	for _, removeKey := range removeKeys {
+		if err := updater.RemoveKey(removeKey.ID()); err != nil {
+			return err
+		}
+	}
+
+	aums, err := updater.Finalize(b.tka.storage)
+	if err != nil {
+		return err
+	}
+
+	if len(aums) == 0 {
+		return nil
+	}
+
+	head, err := b.sendAUMsLocked(aums, true)
+	if err != nil {
+		return err
+	}
+
+	lastHead := aums[len(aums)-1].Hash()
+	if !slices.Equal(head[:], lastHead[:]) {
+		return errors.New("central tka head differs from submitted AUM, try again")
+	}
+
+	return nil
+}
+
+func (b *LocalBackend) sendAUMsLocked(aums []tka.AUM, interactive bool) (head tka.AUMHash, err error) {
+	// Submitting AUMs may block, so release the lock
+	b.mu.Unlock()
+	defer b.mu.Lock()
+
+	mAUMs := make([]tkatype.MarshaledAUM, len(aums))
+	for i := range aums {
+		mAUMs[i] = aums[i].Serialize()
+	}
+
+	var req bytes.Buffer
+	if err := json.NewEncoder(&req).Encode(tailcfg.TKASyncSendRequest{
+		MissingAUMs: mAUMs,
+		Interactive: interactive,
+	}); err != nil {
+		return head, err
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
+	bo := backoff.NewBackoff("tka-submit", b.logf, 5*time.Second)
+	var res *http.Response
+	for {
+		if err := ctx.Err(); err != nil {
+			return head, err
+		}
+		req, err := http.NewRequestWithContext(ctx, "GET", "https://unused/machine/tka/sync/send", &req)
+		if err != nil {
+			return head, err
+		}
+		res, err = b.DoNoiseRequest(req)
+		bo.BackOff(ctx, err)
+		if err == nil {
+			break
+		}
+	}
+	defer res.Body.Close()
+
+	if res.StatusCode != 200 {
+		body, _ := io.ReadAll(res.Body)
+		return head, fmt.Errorf("submit status %d: %s", res.StatusCode, string(body))
+	}
+	a := new(tailcfg.TKASyncSendResponse)
+	if err := json.NewDecoder(res.Body).Decode(a); err != nil {
+		return head, err
+	}
+
+	if err := head.UnmarshalText([]byte(a.Head)); err != nil {
+		return head, err
+	}
+
+	return head, nil
+}
+
 func signNodeKey(nodeInfo tailcfg.TKASignInfo, signer key.NLPrivate) (*tka.NodeKeySignature, error) {
 	p, err := nodeInfo.NodePublic.MarshalBinary()
 	if err != nil {

ipn/localapi/localapi.go

@@ -156,6 +156,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		h.serveTkaStatus(w, r)
 	case "/localapi/v0/tka/init":
 		h.serveTkaInit(w, r)
+	case "/localapi/v0/tka/modify":
+		h.serveTkaModify(w, r)
 	case "/":
 		io.WriteString(w, "tailscaled\n")
 	default:
@@ -855,6 +857,40 @@ func (h *Handler) serveTkaInit(w http.ResponseWriter, r *http.Request) {
 	w.Write(j)
 }
 
+func (h *Handler) serveTkaModify(w http.ResponseWriter, r *http.Request) {
+	if !h.PermitWrite {
+		http.Error(w, "network-lock modify access denied", http.StatusForbidden)
+		return
+	}
+	if r.Method != http.MethodPost {
+		http.Error(w, "use POST", http.StatusMethodNotAllowed)
+		return
+	}
+
+	type modifyRequest struct {
+		AddKeys    []tka.Key
+		RemoveKeys []tka.Key
+	}
+	var req modifyRequest
+	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+		http.Error(w, "invalid JSON body", 400)
+		return
+	}
+
+	if err := h.b.NetworkLockModify(req.AddKeys, req.RemoveKeys); err != nil {
+		http.Error(w, "network-lock modify failed: "+err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	j, err := json.MarshalIndent(h.b.NetworkLockStatus(), "", "\t")
+	if err != nil {
+		http.Error(w, "JSON encoding error", 500)
+		return
+	}
+	w.Header().Set("Content-Type", "application/json")
+	w.Write(j)
+}
+
 func defBool(a string, def bool) bool {
 	if a == "" {
 		return def

tailcfg/tka.go

@@ -173,6 +173,11 @@ type TKASyncSendRequest struct {
 	// MissingAUMs encodes AUMs that the node believes the control plane
 	// is missing.
 	MissingAUMs []tkatype.MarshaledAUM
+	// Interactive is true if additional error checking should be performed as
+	// the request is on behalf of an interactive operation (e.g., an
+	// administrator publishing new changes) as opposed to an automatic
+	// synchronization that may be reporting lost data.
+	Interactive bool
 }
 
 // TKASyncSendResponse encodes the control plane's response to a node