From f1ab11e961a0fd4d52778b0cfcef922e85a78a60 Mon Sep 17 00:00:00 2001 From: Tom DNetto Date: Wed, 30 Nov 2022 10:34:59 -0800 Subject: [PATCH] ipn/ipnlocal,tailcfg: introduce capability to gate TKA init paths Previously, `TAILSCALE_USE_WIP_CODE` was needed to hit a bunch of the TKA paths. With this change: - Enablement codepaths (NetworkLockInit) and initialization codepaths (tkaBootstrapFromGenesisLocked via tkaSyncIfNeeded) require either the WIP envknob or CapabilityTailnetLockAlpha. - Normal operation codepaths (tkaSyncIfNeeded, tkaFilterNetmapLocked) require TKA to be initialized, or either-or the envknob / capability. - Auxillary commands (ie: changing tka keys) require TKA to be initialized. The end result is that it shouldn't be possible to initialize TKA (or subsequently use any of its features) without being sent the capability or setting the envknob on tailscaled yourself. I've also pulled out a bunch of unnecessary checks for CanSupportNetworkLock(). Signed-off-by: Tom DNetto --- ipn/ipnlocal/local.go | 3 +++ ipn/ipnlocal/network-lock.go | 46 ++++++++++++++++++++---------------- tailcfg/tailcfg.go | 9 ++++++- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 0e6c5da05..6992f6378 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -162,6 +162,7 @@ type LocalBackend struct { tka *tkaState state ipn.State capFileSharing bool // whether netMap contains the file sharing capability + capTailnetLock bool // whether netMap contains the tailnet lock capability // hostinfo is mutated in-place while mu is held. hostinfo *tailcfg.Hostinfo // netMap is not mutated in-place once set. @@ -869,6 +870,8 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { } // Prefs will be written out; this is not safe unless locked or cloned. if st.NetMap != nil { + b.capTailnetLock = hasCapability(st.NetMap, tailcfg.CapabilityTailnetLockAlpha) + b.mu.Unlock() // respect locking rules for tkaSyncIfNeeded if err := b.tkaSyncIfNeeded(st.NetMap, prefs.View()); err != nil { b.logf("[v1] TKA sync error: %v", err) diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index f4c4b8d19..1b1375d6f 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -46,13 +46,21 @@ type tkaState struct { filtered []ipnstate.TKAFilteredPeer } +// permitTKAInitLocked returns true if tailnet lock initialization may +// occur. +// b.mu must be held. +func (b *LocalBackend) permitTKAInitLocked() bool { + return envknob.UseWIPCode() || b.capTailnetLock +} + // tkaFilterNetmapLocked checks the signatures on each node key, dropping // nodes from the netmap whose signature does not verify. // // b.mu must be held. func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) { - if !envknob.UseWIPCode() { - return // Feature-flag till network-lock is in Alpha. + // TODO(tom): Remove this guard for 1.35 and later. + if b.tka == nil && !b.permitTKAInitLocked() { + return } if b.tka == nil { return // TKA not enabled. @@ -61,7 +69,7 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) { var toDelete map[int]bool // peer index => true for i, p := range nm.Peers { if p.UnsignedPeerAPIOnly { - // Not subject to TKA. + // Not subject to tailnet lock. continue } if len(p.KeySignature) == 0 { @@ -123,18 +131,18 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) { // tkaSyncIfNeeded immediately takes b.takeSyncLock which is held throughout, // and may take b.mu as required. func (b *LocalBackend) tkaSyncIfNeeded(nm *netmap.NetworkMap, prefs ipn.PrefsView) error { - if !envknob.UseWIPCode() { - // If the feature flag is not enabled, pretend we don't exist. - return nil - } - - b.logf("tkaSyncIfNeeded: enabled=%v, head=%v", nm.TKAEnabled, nm.TKAHead) - b.tkaSyncLock.Lock() // take tkaSyncLock to make this function an exclusive section. defer b.tkaSyncLock.Unlock() b.mu.Lock() // take mu to protect access to synchronized fields. defer b.mu.Unlock() + // TODO(tom): Remove this guard for 1.35 and later. + if b.tka == nil && !b.permitTKAInitLocked() { + return nil + } + + b.logf("tkaSyncIfNeeded: enabled=%v, head=%v", nm.TKAEnabled, nm.TKAHead) + ourNodeKey := prefs.Persist().PublicNodeKey() isEnabled := b.tka != nil @@ -352,10 +360,6 @@ func (b *LocalBackend) tkaBootstrapFromGenesisLocked(g tkatype.MarshaledAUM, per // CanSupportNetworkLock returns nil if tailscaled is able to operate // a local tailnet key authority (and hence enforce network lock). func (b *LocalBackend) CanSupportNetworkLock() error { - if !envknob.UseWIPCode() { - return errors.New("this feature is not yet complete, a later release may support this functionality") - } - if b.tka != nil { // If the TKA is being used, it is supported. return nil @@ -453,6 +457,13 @@ func (b *LocalBackend) NetworkLockInit(keys []tka.Key, disablementValues [][]byt var ourNodeKey key.NodePublic var nlPriv key.NLPrivate b.mu.Lock() + + // TODO(tom): Remove this guard for 1.35 and later. + if !b.permitTKAInitLocked() { + b.mu.Unlock() + return errors.New("this feature is not yet complete, a later release may support this functionality") + } + if p := b.pm.CurrentPrefs(); p.Valid() && p.Persist().Valid() && !p.Persist().PrivateNodeKey().IsZero() { ourNodeKey = p.Persist().PublicNodeKey() nlPriv = p.Persist().NetworkLockKey() @@ -631,9 +642,6 @@ func (b *LocalBackend) NetworkLockModify(addKeys, removeKeys []tka.Key) (err err return errors.New("no node-key: is tailscale logged in?") } - if err := b.CanSupportNetworkLock(); err != nil { - return err - } var nlPriv key.NLPrivate if p := b.pm.CurrentPrefs(); p.Valid() && p.Persist().Valid() { nlPriv = p.Persist().NetworkLockKey() @@ -690,10 +698,6 @@ func (b *LocalBackend) NetworkLockModify(addKeys, removeKeys []tka.Key) (err err // NetworkLockDisable disables network-lock using the provided disablement secret. func (b *LocalBackend) NetworkLockDisable(secret []byte) error { - if err := b.CanSupportNetworkLock(); err != nil { - return err - } - var ( ourNodeKey key.NodePublic head tka.AUMHash diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 38caff99f..290151fd1 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -87,7 +87,8 @@ type CapabilityVersion int // - 48: 2022-11-02: Node.UnsignedPeerAPIOnly // - 49: 2022-11-03: Client understands EarlyNoise // - 50: 2022-11-14: Client understands CapabilityIngress -const CurrentCapabilityVersion CapabilityVersion = 50 +// - 51: 2022-11-30: Client understands CapabilityTailnetLockAlpha +const CurrentCapabilityVersion CapabilityVersion = 51 type StableID string @@ -1707,6 +1708,12 @@ const ( CapabilityDataPlaneAuditLogs = "https://tailscale.com/cap/data-plane-audit-logs" // feature enabled CapabilityDebug = "https://tailscale.com/cap/debug" // exposes debug endpoints over the PeerAPI + // CapabilityTailnetLockAlpha indicates the node is in the tailnet lock alpha, + // and initialization of tailnet lock may proceed. + // + // TODO(tom): Remove this for 1.35 and later. + CapabilityTailnetLockAlpha = "https://tailscale.com/cap/tailnet-lock-alpha" + // Inter-node capabilities as specified in the MapResponse.PacketFilter[].CapGrants. // CapabilityFileSharingTarget grants the current node the ability to send