diff --git a/tka/key_test.go b/tka/key_test.go index dc7c73ee6..e6676aef8 100644 --- a/tka/key_test.go +++ b/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()) + } +} diff --git a/tka/sig.go b/tka/sig.go index 177c52ec3..4f881e755 100644 --- a/tka/sig.go +++ b/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) } } diff --git a/tka/sig_test.go b/tka/sig_test.go index c757e8049..7fc1a579d 100644 --- a/tka/sig_test.go +++ b/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[:]) diff --git a/tka/tka.go b/tka/tka.go index 5c32229be..58ff116d3 100644 --- a/tka/tka.go +++ b/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 diff --git a/types/key/nl_test.go b/types/key/nl_test.go index a8dc43ca1..f8e36ee57 100644 --- a/types/key/nl_test.go +++ b/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()) - } }