From 01847e0123dee3b7a6f9645155da69270f01155e Mon Sep 17 00:00:00 2001 From: Anton Tolchanov Date: Thu, 9 May 2024 07:23:03 +0100 Subject: [PATCH] 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 --- control/controlclient/direct.go | 41 +-------- ipn/ipnlocal/network-lock.go | 101 ++++++++++++++++++++- ipn/ipnlocal/network-lock_test.go | 141 ++++++++++++++++++++++++++++-- tka/sig.go | 75 ++++++++++++++++ tka/sig_test.go | 141 ++++++++++++++++++++++++++++++ tka/tka.go | 21 +++-- 6 files changed, 464 insertions(+), 56 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 24adec882..49879710e 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -558,7 +558,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new var nodeKeySignature tkatype.MarshaledSignature if !oldNodeKey.IsZero() && opt.OldNodeKeySignature != nil { - if nodeKeySignature, err = resignNKS(persist.NetworkLockKey, tryingNewKey.Public(), opt.OldNodeKeySignature); err != nil { + if nodeKeySignature, err = tka.ResignNKS(persist.NetworkLockKey, tryingNewKey.Public(), opt.OldNodeKeySignature); err != nil { c.logf("Failed re-signing node-key signature: %v", err) } } else if isWrapped { @@ -729,45 +729,6 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new return false, resp.AuthURL, nil, nil } -// resignNKS re-signs a node-key signature for a new node-key. -// -// This only matters on network-locked tailnets, because node-key signatures are -// how other nodes know that a node-key is authentic. When the node-key is -// rotated then the existing signature becomes invalid, so this function is -// responsible for generating a new wrapping signature to certify the new node-key. -// -// The signature itself is a SigRotation signature, which embeds the old signature -// and certifies the new node-key as a replacement for the old by signing the new -// signature with RotationPubkey (which is the node's own network-lock key). -func resignNKS(priv key.NLPrivate, nodeKey key.NodePublic, oldNKS tkatype.MarshaledSignature) (tkatype.MarshaledSignature, error) { - var oldSig tka.NodeKeySignature - if err := oldSig.Unserialize(oldNKS); err != nil { - return nil, fmt.Errorf("decoding NKS: %w", err) - } - - nk, err := nodeKey.MarshalBinary() - if err != nil { - return nil, fmt.Errorf("marshalling node-key: %w", err) - } - - if bytes.Equal(nk, oldSig.Pubkey) { - // The old signature is valid for the node-key we are using, so just - // use it verbatim. - return oldNKS, nil - } - - newSig := tka.NodeKeySignature{ - SigKind: tka.SigRotation, - Pubkey: nk, - Nested: &oldSig, - } - if newSig.Signature, err = priv.SignNKS(newSig.SigHash()); err != nil { - return nil, fmt.Errorf("signing NKS: %w", err) - } - - return newSig.Serialize(), nil -} - // newEndpoints acquires c.mu and sets the local port and endpoints and reports // whether they've changed. // diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index a07ca6faf..fff54231e 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -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. // diff --git a/ipn/ipnlocal/network-lock_test.go b/ipn/ipnlocal/network-lock_test.go index 34f739b0f..38a7327bb 100644 --- a/ipn/ipnlocal/network-lock_test.go +++ b/ipn/ipnlocal/network-lock_test.go @@ -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) + } + }) + } +} diff --git a/tka/sig.go b/tka/sig.go index 189645394..34f2ed167 100644 --- a/tka/sig.go +++ b/tka/sig.go @@ -304,3 +304,78 @@ func (s *NodeKeySignature) verifySignature(nodeKey key.NodePublic, verificationK return fmt.Errorf("unhandled signature type: %v", s.SigKind) } } + +// RotationDetails holds additional information about a nodeKeySignature +// of kind SigRotation. +type RotationDetails struct { + // PrevNodeKeys is a list of node keys which have been rotated out. + PrevNodeKeys []key.NodePublic + + // WrappingPubkey is the public key which has been authorized to sign + // this rotating signature. + WrappingPubkey []byte +} + +// rotationDetails returns the RotationDetails for a SigRotation signature. +func (s *NodeKeySignature) rotationDetails() (*RotationDetails, error) { + if s.SigKind != SigRotation { + return nil, nil + } + + sri := &RotationDetails{} + nested := s.Nested + for nested != nil { + if len(nested.Pubkey) > 0 { + var nestedPub key.NodePublic + if err := nestedPub.UnmarshalBinary(nested.Pubkey); err != nil { + return nil, fmt.Errorf("nested pubkey: %v", err) + } + sri.PrevNodeKeys = append(sri.PrevNodeKeys, nestedPub) + } + if nested.SigKind != SigRotation { + break + } + nested = nested.Nested + } + sri.WrappingPubkey = nested.WrappingPubkey + return sri, nil +} + +// ResignNKS re-signs a node-key signature for a new node-key. +// +// This only matters on network-locked tailnets, because node-key signatures are +// how other nodes know that a node-key is authentic. When the node-key is +// rotated then the existing signature becomes invalid, so this function is +// responsible for generating a new wrapping signature to certify the new node-key. +// +// The signature itself is a SigRotation signature, which embeds the old signature +// and certifies the new node-key as a replacement for the old by signing the new +// signature with RotationPubkey (which is the node's own network-lock key). +func ResignNKS(priv key.NLPrivate, nodeKey key.NodePublic, oldNKS tkatype.MarshaledSignature) (tkatype.MarshaledSignature, error) { + var oldSig NodeKeySignature + if err := oldSig.Unserialize(oldNKS); err != nil { + return nil, fmt.Errorf("decoding NKS: %w", err) + } + + nk, err := nodeKey.MarshalBinary() + if err != nil { + return nil, fmt.Errorf("marshalling node-key: %w", err) + } + + if bytes.Equal(nk, oldSig.Pubkey) { + // The old signature is valid for the node-key we are using, so just + // use it verbatim. + return oldNKS, nil + } + + newSig := NodeKeySignature{ + SigKind: SigRotation, + Pubkey: nk, + Nested: &oldSig, + } + if newSig.Signature, err = priv.SignNKS(newSig.SigHash()); err != nil { + return nil, fmt.Errorf("signing NKS: %w", err) + } + + return newSig.Serialize(), nil +} diff --git a/tka/sig_test.go b/tka/sig_test.go index 819b6aa83..305eb64eb 100644 --- a/tka/sig_test.go +++ b/tka/sig_test.go @@ -5,6 +5,7 @@ package tka import ( "crypto/ed25519" + "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -298,3 +299,143 @@ func TestSigSerializeUnserialize(t *testing.T) { t.Errorf("unmarshalled version differs (-want, +got):\n%s", diff) } } + +func TestNodeKeySignatureRotationDetails(t *testing.T) { + // Trusted network lock key + pub, priv := testingKey25519(t, 1) + k := Key{Kind: Key25519, Public: pub, Votes: 2} + + // 'credential' key (the one being delegated to) + cPub, cPriv := testingKey25519(t, 2) + + n1, n2, n3 := key.NewNode(), key.NewNode(), key.NewNode() + n1pub, _ := n1.Public().MarshalBinary() + n2pub, _ := n2.Public().MarshalBinary() + n3pub, _ := n3.Public().MarshalBinary() + + tests := []struct { + name string + nodeKey key.NodePublic + sigFn func() NodeKeySignature + want *RotationDetails + }{ + { + name: "SigDirect", + nodeKey: n1.Public(), + sigFn: func() NodeKeySignature { + s := NodeKeySignature{ + SigKind: SigDirect, + KeyID: pub, + Pubkey: n1pub, + } + sigHash := s.SigHash() + s.Signature = ed25519.Sign(priv, sigHash[:]) + return s + }, + want: nil, + }, + { + name: "SigWrappedCredential", + nodeKey: n1.Public(), + sigFn: func() NodeKeySignature { + nestedSig := NodeKeySignature{ + SigKind: SigCredential, + KeyID: pub, + WrappingPubkey: cPub, + } + sigHash := nestedSig.SigHash() + nestedSig.Signature = ed25519.Sign(priv, sigHash[:]) + + sig := NodeKeySignature{ + SigKind: SigRotation, + Pubkey: n1pub, + Nested: &nestedSig, + } + sigHash = sig.SigHash() + sig.Signature = ed25519.Sign(cPriv, sigHash[:]) + return sig + }, + want: &RotationDetails{ + WrappingPubkey: cPub, + }, + }, + { + name: "SigRotation", + nodeKey: n2.Public(), + sigFn: func() NodeKeySignature { + nestedSig := NodeKeySignature{ + SigKind: SigDirect, + Pubkey: n1pub, + KeyID: pub, + WrappingPubkey: cPub, + } + sigHash := nestedSig.SigHash() + nestedSig.Signature = ed25519.Sign(priv, sigHash[:]) + + sig := NodeKeySignature{ + SigKind: SigRotation, + Pubkey: n2pub, + Nested: &nestedSig, + } + sigHash = sig.SigHash() + sig.Signature = ed25519.Sign(cPriv, sigHash[:]) + return sig + }, + want: &RotationDetails{ + WrappingPubkey: cPub, + PrevNodeKeys: []key.NodePublic{n1.Public()}, + }, + }, + { + name: "SigRotationNestedTwice", + nodeKey: n3.Public(), + sigFn: func() NodeKeySignature { + initialSig := NodeKeySignature{ + SigKind: SigDirect, + Pubkey: n1pub, + KeyID: pub, + WrappingPubkey: cPub, + } + sigHash := initialSig.SigHash() + initialSig.Signature = ed25519.Sign(priv, sigHash[:]) + + prevRotation := NodeKeySignature{ + SigKind: SigRotation, + Pubkey: n2pub, + Nested: &initialSig, + } + sigHash = prevRotation.SigHash() + prevRotation.Signature = ed25519.Sign(cPriv, sigHash[:]) + + sig := NodeKeySignature{ + SigKind: SigRotation, + Pubkey: n3pub, + Nested: &prevRotation, + } + sigHash = sig.SigHash() + sig.Signature = ed25519.Sign(cPriv, sigHash[:]) + + return sig + }, + want: &RotationDetails{ + WrappingPubkey: cPub, + PrevNodeKeys: []key.NodePublic{n2.Public(), n1.Public()}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sig := tt.sigFn() + if err := sig.verifySignature(tt.nodeKey, k); err != nil { + t.Fatalf("verifySignature(node) failed: %v", err) + } + got, err := sig.rotationDetails() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("rotationDetails() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/tka/tka.go b/tka/tka.go index 22145a008..04b712660 100644 --- a/tka/tka.go +++ b/tka/tka.go @@ -668,25 +668,36 @@ func (a *Authority) Inform(storage Chonk, updates []AUM) error { // NodeKeyAuthorized checks if the provided nodeKeySignature authorizes // the given node key. func (a *Authority) NodeKeyAuthorized(nodeKey key.NodePublic, nodeKeySignature tkatype.MarshaledSignature) error { + _, err := a.NodeKeyAuthorizedWithDetails(nodeKey, nodeKeySignature) + return err +} + +// NodeKeyAuthorized checks if the provided nodeKeySignature authorizes +// the given node key, and returns RotationDetails if the signature is +// a valid rotation signature. +func (a *Authority) NodeKeyAuthorizedWithDetails(nodeKey key.NodePublic, nodeKeySignature tkatype.MarshaledSignature) (*RotationDetails, error) { var decoded NodeKeySignature if err := decoded.Unserialize(nodeKeySignature); err != nil { - return fmt.Errorf("unserialize: %v", err) + return nil, fmt.Errorf("unserialize: %v", err) } if decoded.SigKind == SigCredential { - return errors.New("credential signatures cannot authorize nodes on their own") + return nil, errors.New("credential signatures cannot authorize nodes on their own") } kID, err := decoded.authorizingKeyID() if err != nil { - return err + return nil, err } key, err := a.state.GetKey(kID) if err != nil { - return fmt.Errorf("key: %v", err) + return nil, fmt.Errorf("key: %v", err) } - return decoded.verifySignature(nodeKey, key) + if err := decoded.verifySignature(nodeKey, key); err != nil { + return nil, err + } + return decoded.rotationDetails() } // KeyTrusted returns true if the given keyID is trusted by the tailnet