ipn/ipnlocal: allow multiple signature chains from the same SigCredential

Detection of duplicate Network Lock signature chains added in
01847e0123dee3b7a6f9645155da69270f01155e failed to account for chains
originating with a SigCredential signature, which is used for wrapped
auth keys. This results in erroneous removal of signatures that
originate from the same re-usable auth key.

This change ensures that multiple nodes created by the same re-usable
auth key are not getting filtered out by the network lock.

Updates tailscale/corp#19764

Signed-off-by: Anton Tolchanov <anton@tailscale.com>

ipn/ipnlocal/network-lock.go

@@ -142,8 +142,9 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
 //   - 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
+//     wrapping pubkey of the initial SigDirect signature (e.g. if a node is
-//     just one of them, marking the others as obsolete.
+//     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.
@@ -165,6 +166,13 @@ type sigRotationDetails struct {
 func (r *rotationTracker) addRotationDetails(np key.NodePublic, d *tka.RotationDetails) {
 	r.obsolete.Make()
 	r.obsolete.AddSlice(d.PrevNodeKeys)
+	if d.InitialSig.SigKind != tka.SigDirect {
+		// Only enforce uniqueness of chains originating from a SigDirect
+		// signature. Chains that begin with a SigCredential can legitimately
+		// start from the same wrapping pubkey when multiple nodes join the
+		// network using the same reusable auth key.
+		return
+	}
 	rd := sigRotationDetails{
 		np:          np,
 		numPrevKeys: len(d.PrevNodeKeys),
@@ -172,7 +180,7 @@ func (r *rotationTracker) addRotationDetails(np key.NodePublic, d *tka.RotationD
 	if r.byWrappingKey == nil {
 		r.byWrappingKey = make(map[string][]sigRotationDetails)
 	}
-	wp := string(d.WrappingPubkey)
+	wp := string(d.InitialSig.WrappingPubkey)
 	r.byWrappingKey[wp] = append(r.byWrappingKey[wp], rd)
 }
 

ipn/ipnlocal/network-lock_test.go

@@ -609,7 +609,9 @@ func TestTKAFilterNetmap(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	n6, n6Sig := nodeFromAuthKey(preauth)
+	// Two nodes created using the same auth key, both should be valid.
+	n60, n60Sig := nodeFromAuthKey(preauth)
+	n61, n61Sig := nodeFromAuthKey(preauth)
 
 	nm := &netmap.NetworkMap{
 		Peers: nodeViews([]*tailcfg.Node{
@@ -619,7 +621,8 @@ func TestTKAFilterNetmap(t *testing.T) {
 			{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},
-			{ID: 6, Key: n6.Public(), KeySignature: n6Sig},
+			{ID: 60, Key: n60.Public(), KeySignature: n60Sig},
+			{ID: 61, Key: n61.Public(), KeySignature: n61Sig},
 		}),
 	}
 
@@ -628,7 +631,8 @@ func TestTKAFilterNetmap(t *testing.T) {
 	want := nodeViews([]*tailcfg.Node{
 		{ID: 1, Key: n1.Public(), KeySignature: n1GoodSig.Serialize()},
 		{ID: 51, Key: n5Rotated.Public(), KeySignature: n5RotatedSig},
-		{ID: 6, Key: n6.Public(), KeySignature: n6Sig},
+		{ID: 60, Key: n60.Public(), KeySignature: n60Sig},
+		{ID: 61, Key: n61.Public(), KeySignature: n61Sig},
 	})
 	nodePubComparer := cmp.Comparer(func(x, y key.NodePublic) bool {
 		return x.Raw32() == y.Raw32()
@@ -1206,6 +1210,14 @@ func TestRotationTracker(t *testing.T) {
 		raw32 := [32]byte{idx}
 		return key.NodePublicFromRaw32(go4mem.B(raw32[:]))
 	}
+
+	rd := func(initialKind tka.SigKind, wrappingKey []byte, prevKeys ...key.NodePublic) *tka.RotationDetails {
+		return &tka.RotationDetails{
+			InitialSig:   &tka.NodeKeySignature{SigKind: initialKind, WrappingPubkey: wrappingKey},
+			PrevNodeKeys: prevKeys,
+		}
+	}
+
 	n1, n2, n3, n4, n5 := newNK(1), newNK(2), newNK(3), newNK(4), newNK(5)
 
 	pk1, pk2, pk3 := []byte{1}, []byte{2}, []byte{3}
@@ -1225,46 +1237,46 @@ type addDetails struct {
 		{
 			name: "single_prev_key",
 			addDetails: []addDetails{
-				{np: n1, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n2}, WrappingPubkey: pk1}},
+				{np: n1, details: rd(tka.SigDirect, pk1, n2)},
 			},
 			want: set.SetOf([]key.NodePublic{n2}),
 		},
 		{
 			name: "several_prev_keys",
 			addDetails: []addDetails{
-				{np: n1, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n2}, WrappingPubkey: pk1}},
+				{np: n1, details: rd(tka.SigDirect, pk1, n2)},
-				{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n4}, WrappingPubkey: pk2}},
+				{np: n3, details: rd(tka.SigDirect, pk2, n4)},
-				{np: n2, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n3, n4}, WrappingPubkey: pk1}},
+				{np: n2, details: rd(tka.SigDirect, pk1, n3, n4)},
 			},
 			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: n2, details: rd(tka.SigDirect, pk3, n1)},
-				{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
+				{np: n3, details: rd(tka.SigDirect, pk3, n1, n2)},
-				{np: n4, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2, n3}, WrappingPubkey: pk3}},
+				{np: n4, details: rd(tka.SigDirect, pk3, n1, n2, n3)},
-				{np: n5, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n4}, WrappingPubkey: pk3}},
+				{np: n5, details: rd(tka.SigDirect, pk3, n4)},
 			},
 			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: n2, details: rd(tka.SigDirect, pk3, n1)},
-				{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
+				{np: n3, details: rd(tka.SigDirect, pk3, n1, n2)},
-				{np: n4, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
+				{np: n4, details: rd(tka.SigDirect, pk3, n1, n2)},
-				{np: n5, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
+				{np: n5, details: rd(tka.SigDirect, pk3, n1, n2)},
 			},
 			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: n2, details: rd(tka.SigDirect, pk3, n1)},
-				{np: n3, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
+				{np: n3, details: rd(tka.SigDirect, pk3, n1, n2)},
-				{np: n4, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2}, WrappingPubkey: pk3}},
+				{np: n4, details: rd(tka.SigDirect, pk3, n1, n2)},
-				{np: n5, details: &tka.RotationDetails{PrevNodeKeys: []key.NodePublic{n1, n2, n3}, WrappingPubkey: pk3}},
+				{np: n5, details: rd(tka.SigDirect, pk3, n1, n2, n3)},
 			},
 			want: set.SetOf([]key.NodePublic{n1, n2, n3, n4}),
 		},

tka/sig.go

@@ -313,9 +313,9 @@ 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
+	// InitialSig is the first signature in the chain which led to
 	// this rotating signature.
-	WrappingPubkey []byte
+	InitialSig *NodeKeySignature
 }
 
 // rotationDetails returns the RotationDetails for a SigRotation signature.
@@ -339,7 +339,7 @@ func (s *NodeKeySignature) rotationDetails() (*RotationDetails, error) {
 		}
 		nested = nested.Nested
 	}
-	sri.WrappingPubkey = nested.WrappingPubkey
+	sri.InitialSig = nested
 	return sri, nil
 }
 

tka/sig_test.go

@@ -356,9 +356,13 @@ func TestNodeKeySignatureRotationDetails(t *testing.T) {
 				return sig
 			},
 			want: &RotationDetails{
+				InitialSig: &NodeKeySignature{
+					SigKind:        SigCredential,
+					KeyID:          pub,
 					WrappingPubkey: cPub,
 				},
 			},
+		},
 		{
 			name:    "SigRotation",
 			nodeKey: n2.Public(),
@@ -382,7 +386,12 @@ func TestNodeKeySignatureRotationDetails(t *testing.T) {
 				return sig
 			},
 			want: &RotationDetails{
+				InitialSig: &NodeKeySignature{
+					SigKind:        SigDirect,
+					Pubkey:         n1pub,
+					KeyID:          pub,
 					WrappingPubkey: cPub,
+				},
 				PrevNodeKeys: []key.NodePublic{n1.Public()},
 			},
 		},
@@ -418,13 +427,23 @@ func TestNodeKeySignatureRotationDetails(t *testing.T) {
 				return sig
 			},
 			want: &RotationDetails{
+				InitialSig: &NodeKeySignature{
+					SigKind:        SigDirect,
+					Pubkey:         n1pub,
+					KeyID:          pub,
 					WrappingPubkey: cPub,
+				},
 				PrevNodeKeys: []key.NodePublic{n2.Public(), n1.Public()},
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			if tt.want != nil {
+				initialHash := tt.want.InitialSig.SigHash()
+				tt.want.InitialSig.Signature = ed25519.Sign(priv, initialHash[:])
+			}
+
 			sig := tt.sigFn()
 			if err := sig.verifySignature(tt.nodeKey, k); err != nil {
 				t.Fatalf("verifySignature(node) failed: %v", err)