Revert "ipn,types/persist: store disallowed TKA's in prefs, lock local-disable"

This reverts commit f1130421f0.

It was submitted with failing tests (go generate checks)

Requires a lot of API changes to fix so rolling back instead of
forward.

Change-Id: I024e8885c0ed44675d3028a662f386dda811f2ad
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

client/tailscale/localclient.go

@@ -853,21 +853,6 @@ func (lc *LocalClient) NetworkLockLog(ctx context.Context, maxEntries int) ([]ip
 	return decodeJSON[[]ipnstate.NetworkLockUpdate](body)
 }
 
-// NetworkLockForceLocalDisable forcibly shuts down network lock on this node.
-func (lc *LocalClient) NetworkLockForceLocalDisable(ctx context.Context) error {
-	// This endpoint expects an empty JSON stanza as the payload.
-	var b bytes.Buffer
-	if err := json.NewEncoder(&b).Encode(struct{}{}); err != nil {
-		return err
-	}
-
-	if _, err := lc.send(ctx, "POST", "/localapi/v0/tka/force-local-disable", 200, &b); err != nil {
-		return fmt.Errorf("error: %w", err)
-	}
-	return nil
-}
-
-
 // SetServeConfig sets or replaces the serving settings.
 // If config is nil, settings are cleared and serving is disabled.
 func (lc *LocalClient) SetServeConfig(ctx context.Context, config *ipn.ServeConfig) error {

cmd/tailscale/cli/network-lock.go

@@ -37,7 +37,6 @@ var netlockCmd = &ffcli.Command{
 		nlDisableCmd,
 		nlDisablementKDFCmd,
 		nlLogCmd,
-		nlLocalDisableCmd,
 	},
 	Exec: runNetworkLockStatus,
 }
@@ -349,17 +348,6 @@ func runNetworkLockDisable(ctx context.Context, args []string) error {
 	return localClient.NetworkLockDisable(ctx, secrets[0])
 }
 
-var nlLocalDisableCmd = &ffcli.Command{
-	Name:       "local-disable",
-	ShortUsage: "local-disable",
-	ShortHelp:  "Disables the currently-active tailnet lock for this node",
-	Exec:       runNetworkLockLocalDisable,
-}
-
-func runNetworkLockLocalDisable(ctx context.Context, args []string) error {
-	return localClient.NetworkLockForceLocalDisable(ctx)
-}
-
 var nlDisablementKDFCmd = &ffcli.Command{
 	Name:       "disablement-kdf",
 	ShortUsage: "disablement-kdf <hex-encoded-disablement-secret>",

ipn/ipnlocal/network-lock.go

@@ -25,7 +25,6 @@ import (
 	"tailscale.com/tka"
 	"tailscale.com/types/key"
 	"tailscale.com/types/netmap"
-	"tailscale.com/types/persist"
 	"tailscale.com/types/tkatype"
 	"tailscale.com/util/mak"
 )
@@ -135,7 +134,7 @@ func (b *LocalBackend) tkaSyncIfNeeded(nm *netmap.NetworkMap, prefs ipn.PrefsVie
 		}
 
 		if wantEnabled && !isEnabled {
-			if err := b.tkaBootstrapFromGenesisLocked(bs.GenesisAUM, prefs.Persist()); err != nil {
+			if err := b.tkaBootstrapFromGenesisLocked(bs.GenesisAUM); err != nil {
 				return fmt.Errorf("bootstrap: %w", err)
 			}
 			isEnabled = true
@@ -279,7 +278,7 @@ func (b *LocalBackend) chonkPathLocked() string {
 // tailnet key authority, based on the given genesis AUM.
 //
 // b.mu must be held.
-func (b *LocalBackend) tkaBootstrapFromGenesisLocked(g tkatype.MarshaledAUM, persist *persist.Persist) error {
+func (b *LocalBackend) tkaBootstrapFromGenesisLocked(g tkatype.MarshaledAUM) error {
 	if err := b.CanSupportNetworkLock(); err != nil {
 		return err
 	}
@@ -289,19 +288,6 @@ func (b *LocalBackend) tkaBootstrapFromGenesisLocked(g tkatype.MarshaledAUM, per
 		return fmt.Errorf("reading genesis: %v", err)
 	}
 
-	if persist != nil && len(persist.DisallowedTKAStateIDs) > 0 {
-		if genesis.State == nil {
-			return errors.New("invalid genesis: missing State")
-		}
-		bootstrapStateID := fmt.Sprintf("%d:%d", genesis.State.StateID1, genesis.State.StateID2)
-
-		for _, stateID := range persist.DisallowedTKAStateIDs {
-			if stateID == bootstrapStateID {
-				return fmt.Errorf("TKA with stateID of %q is disallowed on this node", stateID)
-			}
-		}
-	}
-
 	chonkDir := b.chonkPathLocked()
 	if err := os.Mkdir(filepath.Dir(chonkDir), 0755); err != nil && !os.IsExist(err) {
 		return fmt.Errorf("creating chonk root dir: %v", err)
@@ -509,31 +495,6 @@ func (b *LocalBackend) NetworkLockKeyTrustedForTest(keyID tkatype.KeyID) bool {
 	return b.tka.authority.KeyTrusted(keyID)
 }
 
-// NetworkLockForceLocalDisable shuts down TKA locally, and denylists the current
-// TKA from being initialized locally in future.
-func (b *LocalBackend) NetworkLockForceLocalDisable() error {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-	if b.tka == nil {
-		return errNetworkLockNotActive
-	}
-
-	id1, id2 := b.tka.authority.StateIDs()
-	stateID := fmt.Sprintf("%d:%d", id1, id2)
-
-	newPrefs := b.pm.CurrentPrefs().AsStruct().Clone() // .Persist should always be initialized here.
-	newPrefs.Persist.DisallowedTKAStateIDs = append(newPrefs.Persist.DisallowedTKAStateIDs, stateID)
-	if err := b.pm.SetPrefs(newPrefs.View()); err != nil {
-		return fmt.Errorf("saving prefs: %w", err)
-	}
-
-	if err := os.RemoveAll(b.chonkPathLocked()); err != nil {
-		return fmt.Errorf("deleting TKA state: %w", err)
-	}
-	b.tka = nil
-	return nil
-}
-
 // NetworkLockSign signs the given node-key and submits it to the control plane.
 // rotationPublic, if specified, must be an ed25519 public key.
 func (b *LocalBackend) NetworkLockSign(nodeKey key.NodePublic, rotationPublic []byte) error {

ipn/ipnlocal/network-lock_test.go

@@ -778,103 +778,3 @@ func TestTKASign(t *testing.T) {
 		t.Errorf("NetworkLockSign() failed: %v", err)
 	}
 }
-
-func TestTKAForceDisable(t *testing.T) {
-	envknob.Setenv("TAILSCALE_USE_WIP_CODE", "1")
-	defer envknob.Setenv("TAILSCALE_USE_WIP_CODE", "")
-	nodePriv := key.NewNode()
-
-	// Make a fake TKA authority, to seed local state.
-	disablementSecret := bytes.Repeat([]byte{0xa5}, 32)
-	nlPriv := key.NewNLPrivate()
-	key := tka.Key{Kind: tka.Key25519, Public: nlPriv.Public().Verifier(), Votes: 2}
-
-	pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ""))
-	must.Do(pm.SetPrefs((&ipn.Prefs{
-		Persist: &persist.Persist{
-			PrivateNodeKey: nodePriv,
-			NetworkLockKey: nlPriv,
-		},
-	}).View()))
-
-	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, genesis, err := tka.Create(chonk, tka.State{
-		Keys:               []tka.Key{key},
-		DisablementSecrets: [][]byte{tka.DisablementKDF(disablementSecret)},
-	}, nlPriv)
-	if err != nil {
-		t.Fatalf("tka.Create() failed: %v", err)
-	}
-
-	ts, client := fakeNoiseServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		defer r.Body.Close()
-		switch r.URL.Path {
-		case "/machine/tka/bootstrap":
-			body := new(tailcfg.TKABootstrapRequest)
-			if err := json.NewDecoder(r.Body).Decode(body); err != nil {
-				t.Fatal(err)
-			}
-			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())
-			}
-
-			w.WriteHeader(200)
-			out := tailcfg.TKABootstrapResponse{
-				GenesisAUM: genesis.Serialize(),
-			}
-			if err := json.NewEncoder(w).Encode(out); 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(),
-	}
-
-	if err := b.NetworkLockForceLocalDisable(); err != nil {
-		t.Fatalf("NetworkLockForceLocalDisable() failed: %v", err)
-	}
-	if b.tka != nil {
-		t.Fatal("tka was not shut down")
-	}
-	if _, err := os.Stat(b.chonkPathLocked()); err == nil || !os.IsNotExist(err) {
-		t.Errorf("os.Stat(chonkDir) = %v, want ErrNotExist", err)
-	}
-
-	err = b.tkaSyncIfNeeded(&netmap.NetworkMap{
-		TKAEnabled: true,
-		TKAHead:    authority.Head(),
-	}, pm.CurrentPrefs())
-	if err != nil && err.Error() != "bootstrap: TKA with stateID of \"0:0\" is disallowed on this node" {
-		t.Errorf("tkaSyncIfNeededLocked() failed: %v", err)
-	}
-
-	if b.tka != nil {
-		t.Fatal("tka was re-initalized")
-	}
-}

ipn/localapi/localapi.go

@@ -89,7 +89,6 @@ var handler = map[string]localAPIHandler{
 	"tka/sign":                (*Handler).serveTKASign,
 	"tka/status":              (*Handler).serveTKAStatus,
 	"tka/disable":             (*Handler).serveTKADisable,
-	"tka/force-local-disable": (*Handler).serveTKALocalDisable,
 	"upload-client-metrics":   (*Handler).serveUploadClientMetrics,
 	"watch-ipn-bus":           (*Handler).serveWatchIPNBus,
 	"whois":                   (*Handler).serveWhoIs,
@@ -1244,30 +1243,6 @@ func (h *Handler) serveTKADisable(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(200)
 }
 
-func (h *Handler) serveTKALocalDisable(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
-	}
-
-	// Require a JSON stanza for the body as an additional CSRF protection.
-	var req struct{}
-	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		http.Error(w, "invalid JSON body", 400)
-		return
-	}
-
-	if err := h.b.NetworkLockForceLocalDisable(); err != nil {
-		http.Error(w, "network-lock local disable failed: "+err.Error(), http.StatusBadRequest)
-		return
-	}
-	w.WriteHeader(200)
-}
-
 func (h *Handler) serveTKALog(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodGet {
 		http.Error(w, "use GET", http.StatusMethodNotAllowed)

tka/tka.go

@@ -714,8 +714,3 @@ func (a *Authority) Keys() []Key {
 	}
 	return out
 }
-
-// StateIDs returns the stateIDs for this tailnet key authority.
-func (a *Authority) StateIDs() (uint64, uint64) {
-	return a.state.StateID1, a.state.StateID2
-}

types/persist/persist.go

@@ -7,7 +7,6 @@ package persist
 
 import (
 	"fmt"
-	"reflect"
 
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
@@ -40,12 +39,6 @@ type Persist struct {
 	UserProfile       tailcfg.UserProfile
 	NetworkLockKey    key.NLPrivate
 	NodeID            tailcfg.StableNodeID
-
-	// DisallowedTKAStateIDs stores the tka.State.StateID values which
-	// this node will not operate network lock on. This is used to
-	// prevent bootstrapping TKA onto a key authority which was forcibly
-	// disabled.
-	DisallowedTKAStateIDs []string
 }
 
 // PublicNodeKey returns the public key for the node key.
@@ -77,8 +70,7 @@ func (p *Persist) Equals(p2 *Persist) bool {
 		p.LoginName == p2.LoginName &&
 		p.UserProfile == p2.UserProfile &&
 		p.NetworkLockKey.Equal(p2.NetworkLockKey) &&
-		p.NodeID == p2.NodeID &&
-		reflect.DeepEqual(p.DisallowedTKAStateIDs, p2.DisallowedTKAStateIDs)
+		p.NodeID == p2.NodeID
 }
 
 func (p *Persist) Pretty() string {

types/persist/persist_clone.go

@@ -20,7 +20,6 @@ func (src *Persist) Clone() *Persist {
 	}
 	dst := new(Persist)
 	*dst = *src
-	dst.DisallowedTKAStateIDs = append(src.DisallowedTKAStateIDs[:0:0], src.DisallowedTKAStateIDs...)
 	return dst
 }
 
@@ -35,5 +34,4 @@ var _PersistCloneNeedsRegeneration = Persist(struct {
 	UserProfile                     tailcfg.UserProfile
 	NetworkLockKey                  key.NLPrivate
 	NodeID                          tailcfg.StableNodeID
-	DisallowedTKAStateIDs           []string
 }{})

types/persist/persist_test.go

@@ -22,7 +22,7 @@ func fieldsOf(t reflect.Type) (fields []string) {
 }
 
 func TestPersistEqual(t *testing.T) {
-	persistHandles := []string{"LegacyFrontendPrivateMachineKey", "PrivateNodeKey", "OldPrivateNodeKey", "Provider", "LoginName", "UserProfile", "NetworkLockKey", "NodeID", "DisallowedTKAStateIDs"}
+	persistHandles := []string{"LegacyFrontendPrivateMachineKey", "PrivateNodeKey", "OldPrivateNodeKey", "Provider", "LoginName", "UserProfile", "NetworkLockKey", "NodeID"}
 	if have := fieldsOf(reflect.TypeOf(Persist{})); !reflect.DeepEqual(have, persistHandles) {
 		t.Errorf("Persist.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
 			have, persistHandles)
@@ -133,11 +133,6 @@ func TestPersistEqual(t *testing.T) {
 			&Persist{NodeID: "abc"},
 			false,
 		},
-		{
-			&Persist{DisallowedTKAStateIDs: nil},
-			&Persist{DisallowedTKAStateIDs: []string{"0:0"}},
-			false,
-		},
 	}
 	for i, test := range tests {
 		if got := test.a.Equals(test.b); got != test.want {

types/persist/persist_view.go

@@ -13,7 +13,6 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 	"tailscale.com/types/structs"
-	"tailscale.com/types/views"
 )
 
 //go:generate go run tailscale.com/cmd/cloner  -clonefunc=false -type=Persist
@@ -73,9 +72,6 @@ func (v PersistView) LoginName() string                  { return v.ж.LoginName
 func (v PersistView) UserProfile() tailcfg.UserProfile   { return v.ж.UserProfile }
 func (v PersistView) NetworkLockKey() key.NLPrivate      { return v.ж.NetworkLockKey }
 func (v PersistView) NodeID() tailcfg.StableNodeID       { return v.ж.NodeID }
-func (v PersistView) DisallowedTKAStateIDs() views.Slice[string] {
-	return views.SliceOf(v.ж.DisallowedTKAStateIDs)
-}
 
 // A compilation failure here means this code must be regenerated, with the command at the top of this file.
 var _PersistViewNeedsRegeneration = Persist(struct {
@@ -88,5 +84,4 @@ var _PersistViewNeedsRegeneration = Persist(struct {
 	UserProfile                     tailcfg.UserProfile
 	NetworkLockKey                  key.NLPrivate
 	NodeID                          tailcfg.StableNodeID
-	DisallowedTKAStateIDs           []string
 }{})