From 06eac9bbff50c0b7ff90d1bd637eb503f8f98ab7 Mon Sep 17 00:00:00 2001 From: Tom DNetto Date: Fri, 12 Aug 2022 13:13:38 -0700 Subject: [PATCH] tka: Use strict decoding settings, implement Unserialize() Signed-off-by: Tom DNetto --- tka/aum.go | 14 ++++++++++++++ tka/aum_test.go | 3 +-- tka/sig.go | 14 ++++++++++++++ tka/sig_test.go | 23 +++++++++++++++++++++++ tka/tailchonk.go | 14 ++++++++++++-- tka/tka.go | 23 +++++++++++++++++++++-- tka/tka_test.go | 8 ++++---- 7 files changed, 89 insertions(+), 10 deletions(-) diff --git a/tka/aum.go b/tka/aum.go index f83ceeeed..05bf9221b 100644 --- a/tka/aum.go +++ b/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()) diff --git a/tka/aum_test.go b/tka/aum_test.go index 78d178d8b..3ca9755d9 100644 --- a/tka/aum_test.go +++ b/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 != "" { diff --git a/tka/sig.go b/tka/sig.go index 0cb59ee9d..1f17b0c53 100644 --- a/tka/sig.go +++ b/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 { diff --git a/tka/sig_test.go b/tka/sig_test.go index 1a071f088..cf7ec8e91 100644 --- a/tka/sig_test.go +++ b/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) + } +} diff --git a/tka/tailchonk.go b/tka/tailchonk.go index 16be78ce7..94fffca72 100644 --- a/tka/tailchonk.go +++ b/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) diff --git a/tka/tka.go b/tka/tka.go index 58772bf45..5c32229be 100644 --- a/tka/tka.go +++ b/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 { - return fmt.Errorf("unmarshal: %v", err) + if err := decoded.Unserialize(nodeKeySignature); err != nil { + 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 +} diff --git a/tka/tka_test.go b/tka/tka_test.go index a72c433c8..0a801a8a0 100644 --- a/tka/tka_test.go +++ b/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 { - t.Errorf("reading genesis key from a1: %v", err) + if !a1.KeyTrusted(key.ID()) { + t.Error("a1 did not trust genesis key") } - if _, err := a2.state.GetKey(key.ID()); err != nil { - t.Errorf("reading genesis key from a2: %v", err) + if !a2.KeyTrusted(key.ID()) { + t.Error("a2 did not trust genesis key") } }