tka: support rotating node-keys in node-key signatures

Signed-off-by: Tom DNetto <tom@tailscale.com>

tka/key_test.go

@@ -11,6 +11,7 @@ import (
 	"math/rand"
 	"testing"
 
+	"tailscale.com/types/key"
 	"tailscale.com/types/tkatype"
 )
 
@@ -64,3 +65,34 @@ func TestVerify25519(t *testing.T) {
 		t.Error("signature verification with different key did not fail")
 	}
 }
+
+func TestNLPrivate(t *testing.T) {
+	p := key.NewNLPrivate()
+	pub := p.Public()
+
+	// Test that key.NLPrivate implements Signer by making a new
+	// authority.
+	k := Key{Kind: Key25519, Public: pub.Verifier(), Votes: 1}
+	_, aum, err := Create(&Mem{}, State{
+		Keys:               []Key{k},
+		DisablementSecrets: [][]byte{bytes.Repeat([]byte{1}, 32)},
+	}, p)
+	if err != nil {
+		t.Fatalf("Create() failed: %v", err)
+	}
+
+	// Make sure the generated genesis AUM was signed.
+	if got, want := len(aum.Signatures), 1; got != want {
+		t.Fatalf("len(signatures) = %d, want %d", got, want)
+	}
+	sigHash := aum.SigHash()
+	if ok := ed25519.Verify(pub.Verifier(), sigHash[:], aum.Signatures[0].Signature); !ok {
+		t.Error("signature did not verify")
+	}
+
+	// We manually compute the keyID, so make sure its consistent with
+	// tka.Key.ID().
+	if !bytes.Equal(k.ID(), p.KeyID()) {
+		t.Errorf("private.KeyID() & tka KeyID differ: %x != %x", k.ID(), p.KeyID())
+	}
+}

tka/sig.go

@@ -13,6 +13,7 @@ import (
 	"github.com/fxamacker/cbor/v2"
 	"github.com/hdevalence/ed25519consensus"
 	"golang.org/x/crypto/blake2s"
+	"tailscale.com/types/key"
 	"tailscale.com/types/tkatype"
 )
 
@@ -21,9 +22,17 @@ type SigKind uint8
 
 const (
 	SigInvalid SigKind = iota
-	// SigDirect describes a signature over a specific node key, using
-	// the keyID specified.
+	// SigDirect describes a signature over a specific node key, signed
+	// by a key in the tailnet key authority referenced by the specified keyID.
 	SigDirect
+	// SigRotation describes a signature over a specific node key, signed
+	// by the rotation key authorized by a nested NodeKeySignature structure.
+	//
+	// While it is possible to nest rotations multiple times up to the CBOR
+	// nesting limit, it is intended that nodes simply regenerate their outer
+	// SigRotation signature and sign it again with their rotation key. That
+	// way, SigRotation nesting should only be 2 deep in the common case.
+	SigRotation
 )
 
 func (s SigKind) String() string {
@@ -32,6 +41,8 @@ func (s SigKind) String() string {
 		return "invalid"
 	case SigDirect:
 		return "direct"
+	case SigRotation:
+		return "rotation"
 	default:
 		return fmt.Sprintf("Sig?<%d>", int(s))
 	}
@@ -42,7 +53,7 @@ func (s SigKind) String() string {
 type NodeKeySignature struct {
 	// SigKind identifies the variety of signature.
 	SigKind SigKind `cbor:"1,keyasint"`
-	// Pubkey identifies the public key which is being certified.
+	// Pubkey identifies the public key which is being authorized.
 	Pubkey []byte `cbor:"2,keyasint"`
 
 	// KeyID identifies which key in the tailnet key authority should
@@ -50,9 +61,39 @@ type NodeKeySignature struct {
 	// SigCredential signature kinds.
 	KeyID []byte `cbor:"3,keyasint,omitempty"`
 
-	// Signature is the packed (R, S) ed25519 signature over the rest
-	// of the structure.
+	// Signature is the packed (R, S) ed25519 signature over all other
+	// fields of the structure.
 	Signature []byte `cbor:"4,keyasint,omitempty"`
+
+	// Nested describes a NodeKeySignature which authorizes the node-key
+	// used as Pubkey. Only used for SigRotation signatures.
+	Nested *NodeKeySignature `cbor:"5,keyasint,omitempty"`
+
+	// RotationPubkey specifies the ed25519 public key which may sign a
+	// SigRotation signature, which embeds this one.
+	//
+	// Intermediate SigRotation signatures may omit this value to use the
+	// parent one.
+	RotationPubkey []byte `cbor:"6,keyasint,omitempty"`
+}
+
+// rotationPublic returns the public key which must sign a SigRotation
+// signature that embeds this signature, if any.
+func (s NodeKeySignature) rotationPublic() (pub ed25519.PublicKey, ok bool) {
+	if len(s.RotationPubkey) > 0 {
+		return ed25519.PublicKey(s.RotationPubkey), true
+	}
+
+	switch s.SigKind {
+	case SigRotation:
+		if s.Nested == nil {
+			return nil, false
+		}
+		return s.Nested.rotationPublic()
+
+	default:
+		return nil, false
+	}
 }
 
 // SigHash returns the cryptographic digest which a signature
@@ -97,18 +138,56 @@ func (s *NodeKeySignature) Unserialize(data []byte) error {
 	return dec.Unmarshal(data, s)
 }
 
-// verifySignature checks that the NodeKeySignature is authentic and certified
-// by the given verificationKey.
-func (s *NodeKeySignature) verifySignature(verificationKey Key) error {
+// verifySignature checks that the NodeKeySignature is authentic, certified
+// by the given verificationKey, and authorizes the given nodeKey.
+func (s *NodeKeySignature) verifySignature(nodeKey key.NodePublic, verificationKey Key) error {
+	nodeBytes, err := nodeKey.MarshalBinary()
+	if err != nil {
+		return fmt.Errorf("marshalling pubkey: %v", err)
+	}
+	if !bytes.Equal(nodeBytes, s.Pubkey) {
+		return errors.New("signature does not authorize nodeKey")
+	}
+
 	sigHash := s.SigHash()
-	switch verificationKey.Kind {
-	case Key25519:
-		if ed25519consensus.Verify(ed25519.PublicKey(verificationKey.Public), sigHash[:], s.Signature) {
-			return nil
+	switch s.SigKind {
+	case SigRotation:
+		if s.Nested == nil {
+			return errors.New("nested signatures must nest a signature")
+		}
+
+		// Verify the signature using the nested rotation key.
+		verifyPub, ok := s.Nested.rotationPublic()
+		if !ok {
+			return errors.New("missing rotation key")
+		}
+		if !ed25519.Verify(ed25519.PublicKey(verifyPub[:]), sigHash[:], s.Signature) {
+			return errors.New("invalid signature")
+		}
+
+		// Recurse to verify the signature on the nested structure.
+		var nestedPub key.NodePublic
+		if err := nestedPub.UnmarshalBinary(s.Nested.Pubkey); err != nil {
+			return fmt.Errorf("nested pubkey: %v", err)
+		}
+		if err := s.Nested.verifySignature(nestedPub, verificationKey); err != nil {
+			return fmt.Errorf("nested: %v", err)
+		}
+		return nil
+
+	case SigDirect:
+		switch verificationKey.Kind {
+		case Key25519:
+			if ed25519consensus.Verify(ed25519.PublicKey(verificationKey.Public), sigHash[:], s.Signature) {
+				return nil
+			}
+			return errors.New("invalid signature")
+
+		default:
+			return fmt.Errorf("unhandled key type: %v", verificationKey.Kind)
 		}
-		return errors.New("invalid signature")
 
 	default:
-		return fmt.Errorf("unhandled key type: %v", verificationKey.Kind)
+		return fmt.Errorf("unhandled signature type: %v", s.SigKind)
 	}
 }

tka/sig_test.go

@@ -9,18 +9,20 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"tailscale.com/types/key"
 )
 
 func TestSigDirect(t *testing.T) {
-	nodeKeyPub := []byte{1, 2, 3, 4}
+	node := key.NewNode()
+	nodeKeyPub, _ := node.Public().MarshalBinary()
 
 	// Verification key (the key used to sign)
 	pub, priv := testingKey25519(t, 1)
-	key := Key{Kind: Key25519, Public: pub, Votes: 2}
+	k := Key{Kind: Key25519, Public: pub, Votes: 2}
 
 	sig := NodeKeySignature{
 		SigKind: SigDirect,
-		KeyID:   key.ID(),
+		KeyID:   k.ID(),
 		Pubkey:  nodeKeyPub,
 	}
 	sigHash := sig.SigHash()
@@ -30,9 +32,147 @@ func TestSigDirect(t *testing.T) {
 		t.Errorf("sigHash changed after signing: %x != %x", sig.SigHash(), sigHash)
 	}
 
-	if err := sig.verifySignature(key); err != nil {
+	if err := sig.verifySignature(node.Public(), k); err != nil {
 		t.Fatalf("verifySignature() failed: %v", err)
 	}
+
+	// Test verification fails when verifying for a different node
+	if err := sig.verifySignature(key.NewNode().Public(), k); err == nil {
+		t.Error("verifySignature() did not error for different nodekey")
+	}
+
+	// Test verification fails if the wrong verification key is provided
+	copy(k.Public, []byte{1, 2, 3, 4})
+	if err := sig.verifySignature(node.Public(), k); err == nil {
+		t.Error("verifySignature() did not error for wrong verification key")
+	}
+}
+
+func TestSigNested(t *testing.T) {
+	// Network-lock key (the key used to sign the nested sig)
+	pub, priv := testingKey25519(t, 1)
+	k := Key{Kind: Key25519, Public: pub, Votes: 2}
+	// Rotation key (the key used to sign the outer sig)
+	rPub, rPriv := testingKey25519(t, 2)
+	// The old node key which is being rotated out
+	oldNode := key.NewNode()
+	oldPub, _ := oldNode.Public().MarshalBinary()
+	// The new node key that is being rotated in
+	node := key.NewNode()
+	nodeKeyPub, _ := node.Public().MarshalBinary()
+
+	// The original signature for the old node key, signed by
+	// the network-lock key.
+	nestedSig := NodeKeySignature{
+		SigKind:        SigDirect,
+		KeyID:          k.ID(),
+		Pubkey:         oldPub,
+		RotationPubkey: rPub,
+	}
+	sigHash := nestedSig.SigHash()
+	nestedSig.Signature = ed25519.Sign(priv, sigHash[:])
+	if err := nestedSig.verifySignature(oldNode.Public(), k); err != nil {
+		t.Fatalf("verifySignature(oldNode) failed: %v", err)
+	}
+
+	// The signature authorizing the rotation, signed by the
+	// rotation key & embedding the original signature.
+	sig := NodeKeySignature{
+		SigKind: SigRotation,
+		KeyID:   k.ID(),
+		Pubkey:  nodeKeyPub,
+		Nested:  &nestedSig,
+	}
+	sigHash = sig.SigHash()
+	sig.Signature = ed25519.Sign(rPriv, sigHash[:])
+
+	if err := sig.verifySignature(node.Public(), k); err != nil {
+		t.Fatalf("verifySignature(node) failed: %v", err)
+	}
+
+	// Test verification fails if the wrong verification key is provided
+	kBad := Key{Kind: Key25519, Public: []byte{1, 2, 3, 4}, Votes: 2}
+	if err := sig.verifySignature(node.Public(), kBad); err == nil {
+		t.Error("verifySignature() did not error for wrong verification key")
+	}
+
+	// Test verification fails if the inner signature is invalid
+	tmp := make([]byte, ed25519.SignatureSize)
+	copy(tmp, nestedSig.Signature)
+	copy(nestedSig.Signature, []byte{1, 2, 3, 4})
+	if err := sig.verifySignature(node.Public(), k); err == nil {
+		t.Error("verifySignature(node) succeeded with bad inner signature")
+	}
+	copy(nestedSig.Signature, tmp)
+
+	// Test verification fails if the outer signature is invalid
+	copy(sig.Signature, []byte{1, 2, 3, 4})
+	if err := sig.verifySignature(node.Public(), k); err == nil {
+		t.Error("verifySignature(node) succeeded with bad outer signature")
+	}
+}
+
+func TestSigNested_DeepNesting(t *testing.T) {
+	// Network-lock key (the key used to sign the nested sig)
+	pub, priv := testingKey25519(t, 1)
+	k := Key{Kind: Key25519, Public: pub, Votes: 2}
+	// Rotation key (the key used to sign the outer sig)
+	rPub, rPriv := testingKey25519(t, 2)
+	// The old node key which is being rotated out
+	oldNode := key.NewNode()
+	oldPub, _ := oldNode.Public().MarshalBinary()
+
+	// The original signature for the old node key, signed by
+	// the network-lock key.
+	nestedSig := NodeKeySignature{
+		SigKind:        SigDirect,
+		KeyID:          k.ID(),
+		Pubkey:         oldPub,
+		RotationPubkey: rPub,
+	}
+	sigHash := nestedSig.SigHash()
+	nestedSig.Signature = ed25519.Sign(priv, sigHash[:])
+	if err := nestedSig.verifySignature(oldNode.Public(), k); err != nil {
+		t.Fatalf("verifySignature(oldNode) failed: %v", err)
+	}
+
+	outer := nestedSig
+	var lastNodeKey key.NodePrivate
+	for i := 0; i < 100; i++ {
+		lastNodeKey = key.NewNode()
+		nodeKeyPub, _ := lastNodeKey.Public().MarshalBinary()
+
+		tmp := outer
+		sig := NodeKeySignature{
+			SigKind: SigRotation,
+			KeyID:   k.ID(),
+			Pubkey:  nodeKeyPub,
+			Nested:  &tmp,
+		}
+		sigHash = sig.SigHash()
+		sig.Signature = ed25519.Sign(rPriv, sigHash[:])
+
+		outer = sig
+	}
+
+	if err := outer.verifySignature(lastNodeKey.Public(), k); err != nil {
+		t.Fatalf("verifySignature(lastNodeKey) failed: %v", err)
+	}
+
+	// Test verification fails if the inner signature is invalid
+	tmp := make([]byte, ed25519.SignatureSize)
+	copy(tmp, nestedSig.Signature)
+	copy(nestedSig.Signature, []byte{1, 2, 3, 4})
+	if err := outer.verifySignature(lastNodeKey.Public(), k); err == nil {
+		t.Error("verifySignature(lastNodeKey) succeeded with bad inner signature")
+	}
+	copy(nestedSig.Signature, tmp)
+
+	// Test verification fails if an intermediate signature is invalid
+	copy(outer.Nested.Nested.Signature, []byte{1, 2, 3, 4})
+	if err := outer.verifySignature(lastNodeKey.Public(), k); err == nil {
+		t.Error("verifySignature(lastNodeKey) succeeded with bad outer signature")
+	}
 }
 
 func TestSigSerializeUnserialize(t *testing.T) {
@@ -43,6 +183,11 @@ func TestSigSerializeUnserialize(t *testing.T) {
 		SigKind: SigDirect,
 		KeyID:   key.ID(),
 		Pubkey:  nodeKeyPub,
+		Nested: &NodeKeySignature{
+			SigKind: SigDirect,
+			KeyID:   key.ID(),
+			Pubkey:  nodeKeyPub,
+		},
 	}
 	sigHash := sig.SigHash()
 	sig.Signature = ed25519.Sign(priv, sigHash[:])

tka/tka.go

@@ -13,6 +13,7 @@ import (
 	"sort"
 
 	"github.com/fxamacker/cbor/v2"
+	"tailscale.com/types/key"
 	"tailscale.com/types/tkatype"
 )
 
@@ -23,7 +24,7 @@ var cborDecOpts = cbor.DecOptions{
 	TagsMd:      cbor.TagsForbidden,
 
 	// Arbitrarily-chosen maximums.
-	MaxNestedLevels:  8,
+	MaxNestedLevels:  16, // Most likely to be hit for SigRotation sigs.
 	MaxArrayElements: 4096,
 	MaxMapPairs:      1024,
 }
@@ -604,9 +605,9 @@ func (a *Authority) Inform(updates []AUM) error {
 	return nil
 }
 
-// VerifySignature returns true if the provided nodeKeySignature is signed
-// correctly by a trusted key.
-func (a *Authority) VerifySignature(nodeKeySignature tkatype.MarshaledSignature) error {
+// NodeKeyAuthorized checks if the provided nodeKeySignature authorizes
+// the given node key.
+func (a *Authority) NodeKeyAuthorized(nodeKey key.NodePublic, nodeKeySignature tkatype.MarshaledSignature) error {
 	var decoded NodeKeySignature
 	if err := decoded.Unserialize(nodeKeySignature); err != nil {
 		return fmt.Errorf("unserialize: %v", err)
@@ -616,7 +617,7 @@ func (a *Authority) VerifySignature(nodeKeySignature tkatype.MarshaledSignature)
 		return fmt.Errorf("key: %v", err)
 	}
 
-	return decoded.verifySignature(key)
+	return decoded.verifySignature(nodeKey, key)
 }
 
 // KeyTrusted returns true if the given keyID is trusted by the tailnet

types/key/nl_test.go

@@ -6,10 +6,7 @@ package key
 
 import (
 	"bytes"
-	"crypto/ed25519"
 	"testing"
-
-	"tailscale.com/tka"
 )
 
 func TestNLPrivate(t *testing.T) {
@@ -40,30 +37,4 @@ func TestNLPrivate(t *testing.T) {
 	if !bytes.Equal(decodedPub.k[:], pub.k[:]) {
 		t.Error("decoded and generated NLPublic bytes differ")
 	}
-
-	// Test that NLPrivate implements tka.Signer by making a new
-	// authority.
-	k := tka.Key{Kind: tka.Key25519, Public: pub.Verifier(), Votes: 1}
-	_, aum, err := tka.Create(&tka.Mem{}, tka.State{
-		Keys:               []tka.Key{k},
-		DisablementSecrets: [][]byte{bytes.Repeat([]byte{1}, 32)},
-	}, p)
-	if err != nil {
-		t.Fatalf("tka.Create() failed: %v", err)
-	}
-
-	// Make sure the generated genesis AUM was signed.
-	if got, want := len(aum.Signatures), 1; got != want {
-		t.Fatalf("len(signatures) = %d, want %d", got, want)
-	}
-	sigHash := aum.SigHash()
-	if ok := ed25519.Verify(pub.Verifier(), sigHash[:], aum.Signatures[0].Signature); !ok {
-		t.Error("signature did not verify")
-	}
-
-	// We manually compute the keyID, so make sure its consistent with
-	// tka.Key.ID().
-	if !bytes.Equal(k.ID(), p.KeyID()) {
-		t.Errorf("private.KeyID() & tka KeyID differ: %x != %x", k.ID(), p.KeyID())
-	}
 }