ipn/ipnlocal: discard node keys that have been rotated out

A non-signing node can be allowed to re-sign its new node keys following
key renewal/rotation (e.g. via `tailscale up --force-reauth`). To be
able to do this, node's TLK is written into WrappingPubkey field of the
initial SigDirect signature, signed by a signing node.

The intended use of this field implies that, for each WrappingPubkey, we
typically expect to have at most one active node with a signature
tracing back to that key. Multiple valid signatures referring to the
same WrappingPubkey can occur if a client's state has been cloned, but
it's something we explicitly discourage and don't support:
https://tailscale.com/s/clone

This change propagates rotation details (wrapping public key, a list
of previous node keys that have been rotated out) to netmap processing,
and adds tracking of obsolete node keys that, when found, will get
filtered out.

Updates tailscale/corp#19764

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
This commit is contained in:
Anton Tolchanov
2024-05-09 07:23:03 +01:00
committed by Anton Tolchanov
parent 42cfbf427c
commit 01847e0123
6 changed files with 464 additions and 56 deletions

View File

@@ -18,6 +18,7 @@ import (
"net/netip"
"os"
"path/filepath"
"slices"
"time"
"tailscale.com/health/healthmsg"
@@ -27,10 +28,12 @@ import (
"tailscale.com/tailcfg"
"tailscale.com/tka"
"tailscale.com/types/key"
"tailscale.com/types/logger"
"tailscale.com/types/netmap"
"tailscale.com/types/persist"
"tailscale.com/types/tkatype"
"tailscale.com/util/mak"
"tailscale.com/util/set"
)
// TODO(tom): RPC retry/backoff was broken and has been removed. Fix?
@@ -66,6 +69,7 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
return // TKA not enabled.
}
tracker := rotationTracker{logf: b.logf}
var toDelete map[int]bool // peer index => true
for i, p := range nm.Peers {
if p.UnsignedPeerAPIOnly() {
@@ -76,21 +80,32 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
b.logf("Network lock is dropping peer %v(%v) due to missing signature", p.ID(), p.StableID())
mak.Set(&toDelete, i, true)
} else {
if err := b.tka.authority.NodeKeyAuthorized(p.Key(), p.KeySignature().AsSlice()); err != nil {
details, err := b.tka.authority.NodeKeyAuthorizedWithDetails(p.Key(), p.KeySignature().AsSlice())
if err != nil {
b.logf("Network lock is dropping peer %v(%v) due to failed signature check: %v", p.ID(), p.StableID(), err)
mak.Set(&toDelete, i, true)
continue
}
if details != nil {
// Rotation details are returned when the node key is signed by a valid SigRotation signature.
tracker.addRotationDetails(p.Key(), details)
}
}
}
obsoleteByRotation := tracker.obsoleteKeys()
// nm.Peers is ordered, so deletion must be order-preserving.
if len(toDelete) > 0 {
if len(toDelete) > 0 || len(obsoleteByRotation) > 0 {
peers := make([]tailcfg.NodeView, 0, len(nm.Peers))
filtered := make([]ipnstate.TKAFilteredPeer, 0, len(toDelete))
filtered := make([]ipnstate.TKAFilteredPeer, 0, len(toDelete)+len(obsoleteByRotation))
for i, p := range nm.Peers {
if !toDelete[i] {
if !toDelete[i] && !obsoleteByRotation.Contains(p.Key()) {
peers = append(peers, p)
} else {
if obsoleteByRotation.Contains(p.Key()) {
b.logf("Network lock is dropping peer %v(%v) due to key rotation", p.ID(), p.StableID())
}
// Record information about the node we filtered out.
fp := ipnstate.TKAFilteredPeer{
Name: p.Name(),
@@ -122,6 +137,84 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
}
}
// rotationTracker determines the set of node keys that are made obsolete by key
// rotation.
// - for each SigRotation signature, all previous node keys referenced by the
// nested signatures are marked as obsolete.
// - if there are multiple SigRotation signatures tracing back to the same
// wrapping pubkey (e.g. if a node is cloned with all its keys), we keep
// just one of them, marking the others as obsolete.
type rotationTracker struct {
// obsolete is the set of node keys that are obsolete due to key rotation.
// users of rotationTracker should use the obsoleteKeys method for complete results.
obsolete set.Set[key.NodePublic]
// byWrappingKey keeps track of rotation details per wrapping pubkey.
byWrappingKey map[string][]sigRotationDetails
logf logger.Logf
}
// sigRotationDetails holds information about a node key signed by a SigRotation.
type sigRotationDetails struct {
np key.NodePublic
numPrevKeys int
}
// addRotationDetails records the rotation signature details for a node key.
func (r *rotationTracker) addRotationDetails(np key.NodePublic, d *tka.RotationDetails) {
r.obsolete.Make()
r.obsolete.AddSlice(d.PrevNodeKeys)
rd := sigRotationDetails{
np: np,
numPrevKeys: len(d.PrevNodeKeys),
}
if r.byWrappingKey == nil {
r.byWrappingKey = make(map[string][]sigRotationDetails)
}
wp := string(d.WrappingPubkey)
r.byWrappingKey[wp] = append(r.byWrappingKey[wp], rd)
}
// obsoleteKeys returns the set of node keys that are obsolete due to key rotation.
func (r *rotationTracker) obsoleteKeys() set.Set[key.NodePublic] {
for _, v := range r.byWrappingKey {
// If there are multiple rotation signatures with the same wrapping
// pubkey, we need to decide which one is the "latest", and keep it.
// The signature with the largest number of previous keys is likely to
// be the latest, unless it has been marked as obsolete (rotated out) by
// another signature (which might happen in the future if we start
// compacting long rotated signature chains).
slices.SortStableFunc(v, func(a, b sigRotationDetails) int {
// Group all obsolete keys after non-obsolete keys.
if ao, bo := r.obsolete.Contains(a.np), r.obsolete.Contains(b.np); ao != bo {
if ao {
return 1
}
return -1
}
// Sort by decreasing number of previous keys.
return b.numPrevKeys - a.numPrevKeys
})
// If there are several signatures with the same number of previous
// keys, we cannot determine which one is the latest, so all of them are
// rejected for safety.
if len(v) >= 2 && v[0].numPrevKeys == v[1].numPrevKeys {
r.logf("at least two nodes (%s and %s) have equally valid rotation signatures with the same wrapping pubkey, rejecting", v[0].np, v[1].np)
for _, rd := range v {
r.obsolete.Add(rd.np)
}
} else {
// The first key in v is the one with the longest chain of previous
// keys, so it must be the newest one. Mark all older keys as obsolete.
for _, rd := range v[1:] {
r.obsolete.Add(rd.np)
}
}
}
return r.obsolete
}
// tkaSyncIfNeeded examines TKA info reported from the control plane,
// performing the steps necessary to synchronize local tka state.
//

View File

@@ -13,8 +13,11 @@ import (
"net/http/httptest"
"os"
"path/filepath"
"reflect"
"testing"
go4mem "go4.org/mem"
"github.com/google/go-cmp/cmp"
"tailscale.com/control/controlclient"
"tailscale.com/health"
@@ -30,6 +33,7 @@ import (
"tailscale.com/types/persist"
"tailscale.com/types/tkatype"
"tailscale.com/util/must"
"tailscale.com/util/set"
)
type observerFunc func(controlclient.Status)
@@ -563,18 +567,32 @@ func TestTKAFilterNetmap(t *testing.T) {
}
n4Sig.Signature[3] = 42 // mess up the signature
n4Sig.Signature[4] = 42 // mess up the signature
n5GoodSig, err := signNodeKey(tailcfg.TKASignInfo{NodePublic: n5.Public()}, nlPriv)
n5nl := key.NewNLPrivate()
n5InitialSig, err := signNodeKey(tailcfg.TKASignInfo{NodePublic: n5.Public(), RotationPubkey: n5nl.Public().Verifier()}, nlPriv)
if err != nil {
t.Fatal(err)
}
resign := func(nl key.NLPrivate, currentSig tkatype.MarshaledSignature) (key.NodePrivate, tkatype.MarshaledSignature) {
nk := key.NewNode()
sig, err := tka.ResignNKS(nl, nk.Public(), currentSig)
if err != nil {
t.Fatal(err)
}
return nk, sig
}
n5Rotated, n5RotatedSig := resign(n5nl, n5InitialSig.Serialize())
nm := &netmap.NetworkMap{
Peers: nodeViews([]*tailcfg.Node{
{ID: 1, Key: n1.Public(), KeySignature: n1GoodSig.Serialize()},
{ID: 2, Key: n2.Public(), KeySignature: nil}, // missing sig
{ID: 3, Key: n3.Public(), KeySignature: n1GoodSig.Serialize()}, // someone elses sig
{ID: 4, Key: n4.Public(), KeySignature: n4Sig.Serialize()}, // messed-up signature
{ID: 5, Key: n5.Public(), KeySignature: n5GoodSig.Serialize()},
{ID: 2, Key: n2.Public(), KeySignature: nil}, // missing sig
{ID: 3, Key: n3.Public(), KeySignature: n1GoodSig.Serialize()}, // someone elses sig
{ID: 4, Key: n4.Public(), KeySignature: n4Sig.Serialize()}, // messed-up signature
{ID: 50, Key: n5.Public(), KeySignature: n5InitialSig.Serialize()}, // rotated
{ID: 51, Key: n5Rotated.Public(), KeySignature: n5RotatedSig},
}),
}
@@ -586,12 +604,39 @@ func TestTKAFilterNetmap(t *testing.T) {
want := nodeViews([]*tailcfg.Node{
{ID: 1, Key: n1.Public(), KeySignature: n1GoodSig.Serialize()},
{ID: 5, Key: n5.Public(), KeySignature: n5GoodSig.Serialize()},
{ID: 51, Key: n5Rotated.Public(), KeySignature: n5RotatedSig},
})
nodePubComparer := cmp.Comparer(func(x, y key.NodePublic) bool {
return x.Raw32() == y.Raw32()
})
if diff := cmp.Diff(nm.Peers, want, nodePubComparer); diff != "" {
if diff := cmp.Diff(want, nm.Peers, nodePubComparer); diff != "" {
t.Errorf("filtered netmap differs (-want, +got):\n%s", diff)
}
// Create two more node signatures using the same wrapping key as n5.
// Since they have the same rotation chain, both will be filtered out.
n7, n7Sig := resign(n5nl, n5RotatedSig)
n8, n8Sig := resign(n5nl, n5RotatedSig)
nm = &netmap.NetworkMap{
Peers: nodeViews([]*tailcfg.Node{
{ID: 1, Key: n1.Public(), KeySignature: n1GoodSig.Serialize()},
{ID: 2, Key: n2.Public(), KeySignature: nil}, // missing sig
{ID: 3, Key: n3.Public(), KeySignature: n1GoodSig.Serialize()}, // someone elses sig
{ID: 4, Key: n4.Public(), KeySignature: n4Sig.Serialize()}, // messed-up signature
{ID: 50, Key: n5.Public(), KeySignature: n5InitialSig.Serialize()}, // rotated
{ID: 51, Key: n5Rotated.Public(), KeySignature: n5RotatedSig}, // rotated
{ID: 7, Key: n7.Public(), KeySignature: n7Sig}, // same rotation chain as n8
{ID: 8, Key: n8.Public(), KeySignature: n8Sig}, // same rotation chain as n7
}),
}
b.tkaFilterNetmapLocked(nm)
want = nodeViews([]*tailcfg.Node{
{ID: 1, Key: n1.Public(), KeySignature: n1GoodSig.Serialize()},
})
if diff := cmp.Diff(want, nm.Peers, nodePubComparer); diff != "" {
t.Errorf("filtered netmap differs (-want, +got):\n%s", diff)
}
}
@@ -1130,3 +1175,85 @@ func TestTKARecoverCompromisedKeyFlow(t *testing.T) {
t.Errorf("NetworkLockSubmitRecoveryAUM() failed: %v", err)
}
}
func TestRotationTracker(t *testing.T) {
newNK := func(idx byte) key.NodePublic {
// single-byte public key to make it human-readable in tests.
raw32 := [32]byte{idx}
return key.NodePublicFromRaw32(go4mem.B(raw32[:]))
}
n1, n2, n3, n4, n5 := newNK(1), newNK(2), newNK(3), newNK(4), newNK(5)
pk1, pk2, pk3 := []byte{1}, []byte{2}, []byte{3}
type addDetails struct {
np key.NodePublic
details *tka.RotationDetails
}
tests := []struct {
name string
addDetails []addDetails
want set.Set[key.NodePublic]
}{
{
name: "empty",
want: nil,
},
{
name: "single_prev_key",
addDetails: []addDetails{
{np: n1, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n2}, WrappingPubkey: pk1}},
},
want: set.SetOf([]key.NodePublic{n2}),
},
{
name: "several_prev_keys",
addDetails: []addDetails{
{np: n1, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n2}, WrappingPubkey: pk1}},
{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n4}, WrappingPubkey: pk2}},
{np: n2, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n3, n4}, WrappingPubkey: pk1}},
},
want: set.SetOf([]key.NodePublic{n2, n3, n4}),
},
{
name: "several_per_pubkey_latest_wins",
addDetails: []addDetails{
{np: n2, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1}, WrappingPubkey: pk3}},
{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
{np: n4, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2, n3}, WrappingPubkey: pk3}},
{np: n5, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n4}, WrappingPubkey: pk3}},
},
want: set.SetOf([]key.NodePublic{n1, n2, n3, n4}),
},
{
name: "several_per_pubkey_same_chain_length_all_rejected",
addDetails: []addDetails{
{np: n2, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1}, WrappingPubkey: pk3}},
{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
{np: n4, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
{np: n5, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
},
want: set.SetOf([]key.NodePublic{n1, n2, n3, n4, n5}),
},
{
name: "several_per_pubkey_longest_wins",
addDetails: []addDetails{
{np: n2, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1}, WrappingPubkey: pk3}},
{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
{np: n4, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
{np: n5, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2, n3}, WrappingPubkey: pk3}},
},
want: set.SetOf([]key.NodePublic{n1, n2, n3, n4}),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := &rotationTracker{logf: t.Logf}
for _, ad := range tt.addDetails {
r.addRotationDetails(ad.np, ad.details)
}
if got := r.obsoleteKeys(); !reflect.DeepEqual(got, tt.want) {
t.Errorf("rotationTracker.obsoleteKeys() = %v, want %v", got, tt.want)
}
})
}
}