tka: Use strict decoding settings, implement Unserialize()

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

tka/aum.go

@@ -212,6 +212,10 @@ func (a *AUM) StaticValidate() error {
 }
 
 // Serialize returns the given AUM in a serialized format.
+//
+// We would implement encoding.BinaryMarshaler, except that would
+// unfortunately get called by the cbor marshaller resulting in infinite
+// recursion.
 func (a *AUM) Serialize() []byte {
 	// Why CBOR and not something like JSON?
 	//
@@ -243,6 +247,16 @@ func (a *AUM) Serialize() []byte {
 	return out.Bytes()
 }
 
+// Unserialize decodes bytes representing a marshaled AUM.
+//
+// We would implement encoding.BinaryUnmarshaler, except that would
+// unfortunately get called by the cbor unmarshaller resulting in infinite
+// recursion.
+func (a *AUM) Unserialize(data []byte) error {
+	dec, _ := cborDecOpts.DecMode()
+	return dec.Unmarshal(data, a)
+}
+
 // Hash returns a cryptographic digest of all AUM contents.
 func (a *AUM) Hash() AUMHash {
 	return blake2s.Sum256(a.Serialize())

tka/aum_test.go

@@ -8,7 +8,6 @@ import (
 	"bytes"
 	"testing"
 
-	"github.com/fxamacker/cbor/v2"
 	"github.com/google/go-cmp/cmp"
 	"golang.org/x/crypto/blake2s"
 	"tailscale.com/types/tkatype"
@@ -165,7 +164,7 @@ func TestSerialization(t *testing.T) {
 			}
 
 			var decodedAUM AUM
-			if err := cbor.Unmarshal(data, &decodedAUM); err != nil {
+			if err := decodedAUM.Unserialize(data); err != nil {
 				t.Fatalf("Unmarshal failed: %v", err)
 			}
 			if diff := cmp.Diff(tc.AUM, decodedAUM); diff != "" {

tka/sig.go

@@ -68,6 +68,10 @@ func (s NodeKeySignature) sigHash() [blake2s.Size]byte {
 }
 
 // Serialize returns the given NKS in a serialized format.
+//
+// We would implement encoding.BinaryMarshaler, except that would
+// unfortunately get called by the cbor marshaller resulting in infinite
+// recursion.
 func (s *NodeKeySignature) Serialize() tkatype.MarshaledSignature {
 	out := bytes.NewBuffer(make([]byte, 0, 128)) // 64byte sig + 32byte keyID + 32byte headroom
 	encoder, err := cbor.CTAP2EncOptions().EncMode()
@@ -83,6 +87,16 @@ func (s *NodeKeySignature) Serialize() tkatype.MarshaledSignature {
 	return out.Bytes()
 }
 
+// Unserialize decodes bytes representing a marshaled NKS.
+//
+// We would implement encoding.BinaryUnmarshaler, except that would
+// unfortunately get called by the cbor unmarshaller resulting in infinite
+// recursion.
+func (s *NodeKeySignature) Unserialize(data []byte) error {
+	dec, _ := cborDecOpts.DecMode()
+	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 {

tka/sig_test.go

@@ -7,6 +7,8 @@ package tka
 import (
 	"crypto/ed25519"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestSigDirect(t *testing.T) {
@@ -32,3 +34,24 @@ func TestSigDirect(t *testing.T) {
 		t.Fatalf("verifySignature() failed: %v", err)
 	}
 }
+
+func TestSigSerializeUnserialize(t *testing.T) {
+	nodeKeyPub := []byte{1, 2, 3, 4}
+	pub, priv := testingKey25519(t, 1)
+	key := Key{Kind: Key25519, Public: pub, Votes: 2}
+	sig := NodeKeySignature{
+		SigKind: SigDirect,
+		KeyID:   key.ID(),
+		Pubkey:  nodeKeyPub,
+	}
+	sigHash := sig.sigHash()
+	sig.Signature = ed25519.Sign(priv, sigHash[:])
+
+	var decoded NodeKeySignature
+	if err := decoded.Unserialize(sig.Serialize()); err != nil {
+		t.Fatalf("Unserialize() failed: %v", err)
+	}
+	if diff := cmp.Diff(sig, decoded); diff != "" {
+		t.Errorf("unmarshalled version differs (-want, +got):\n%s", diff)
+	}
+}

tka/tailchonk.go

@@ -261,8 +261,13 @@ func (c *FS) get(h AUMHash) (*fsHashInfo, error) {
 	}
 	defer f.Close()
 
+	m, err := cborDecOpts.DecMode()
+	if err != nil {
+		return nil, err
+	}
+
 	var out fsHashInfo
-	if err := cbor.NewDecoder(f).Decode(&out); err != nil {
+	if err := m.NewDecoder(f).Decode(&out); err != nil {
 		return nil, err
 	}
 	if out.AUM != nil && out.AUM.Hash() != h {
@@ -420,8 +425,13 @@ func (c *FS) commit(h AUMHash, updater func(*fsHashInfo)) error {
 		return fmt.Errorf("creating directory: %v", err)
 	}
 
+	m, err := cbor.CTAP2EncOptions().EncMode()
+	if err != nil {
+		return fmt.Errorf("cbor EncMode: %v", err)
+	}
+
 	var buff bytes.Buffer
-	if err := cbor.NewEncoder(&buff).Encode(toCommit); err != nil {
+	if err := m.NewEncoder(&buff).Encode(toCommit); err != nil {
 		return fmt.Errorf("encoding: %v", err)
 	}
 	return atomicfile.WriteFile(filepath.Join(dir, base), buff.Bytes(), 0644)

tka/tka.go

@@ -16,6 +16,18 @@ import (
 	"tailscale.com/types/tkatype"
 )
 
+// Strict settings for the CBOR decoder.
+var cborDecOpts = cbor.DecOptions{
+	DupMapKey:   cbor.DupMapKeyEnforcedAPF,
+	IndefLength: cbor.IndefLengthForbidden,
+	TagsMd:      cbor.TagsForbidden,
+
+	// Arbitrarily-chosen maximums.
+	MaxNestedLevels:  8,
+	MaxArrayElements: 4096,
+	MaxMapPairs:      1024,
+}
+
 // Authority is a Tailnet Key Authority. This type is the main coupling
 // point to the rest of the tailscale client.
 //
@@ -596,8 +608,8 @@ func (a *Authority) Inform(updates []AUM) error {
 // correctly by a trusted key.
 func (a *Authority) VerifySignature(nodeKeySignature tkatype.MarshaledSignature) error {
 	var decoded NodeKeySignature
-	if err := cbor.Unmarshal(nodeKeySignature, &decoded); err != nil {
+	if err := decoded.Unserialize(nodeKeySignature); err != nil {
-		return fmt.Errorf("unmarshal: %v", err)
+		return fmt.Errorf("unserialize: %v", err)
 	}
 	key, err := a.state.GetKey(decoded.KeyID)
 	if err != nil {
@@ -606,3 +618,10 @@ func (a *Authority) VerifySignature(nodeKeySignature tkatype.MarshaledSignature)
 
 	return decoded.verifySignature(key)
 }
+
+// KeyTrusted returns true if the given keyID is trusted by the tailnet
+// key authority.
+func (a *Authority) KeyTrusted(keyID tkatype.KeyID) bool {
+	_, err := a.state.GetKey(keyID)
+	return err == nil
+}

tka/tka_test.go

@@ -317,11 +317,11 @@ func TestCreateBootstrapAuthority(t *testing.T) {
 	}
 
 	// Both authorities should trust the key laid down in the genesis state.
-	if _, err := a1.state.GetKey(key.ID()); err != nil {
+	if !a1.KeyTrusted(key.ID()) {
-		t.Errorf("reading genesis key from a1: %v", err)
+		t.Error("a1 did not trust genesis key")
 	}
-	if _, err := a2.state.GetKey(key.ID()); err != nil {
+	if !a2.KeyTrusted(key.ID()) {
-		t.Errorf("reading genesis key from a2: %v", err)
+		t.Error("a2 did not trust genesis key")
 	}
 }