diff --git a/tka/aum.go b/tka/aum.go index f3de57bf3..fd6aaa8d4 100644 --- a/tka/aum.go +++ b/tka/aum.go @@ -7,7 +7,6 @@ package tka import ( "bytes" "crypto/ed25519" - "encoding/binary" "errors" "fmt" @@ -122,13 +121,6 @@ type AUM struct { Signatures []Signature `cbor:"23,keyasint,omitempty"` } -// Upper bound on checkpoint elements, chosen arbitrarily. Intended to -// cap out insanely large AUMs. -const ( - maxDisablementSecrets = 32 - maxKeys = 512 -) - // StaticValidate returns a nil error if the AUM is well-formed. func (a *AUM) StaticValidate() error { if a.Key != nil { @@ -146,34 +138,9 @@ func (a *AUM) StaticValidate() error { } if a.State != nil { - if a.State.LastAUMHash != nil { - return errors.New("checkpoint state cannot specify a parent AUM") - } - if len(a.State.DisablementSecrets) == 0 { - return errors.New("at least one disablement secret required") - } - if numDS := len(a.State.DisablementSecrets); numDS > maxDisablementSecrets { - return fmt.Errorf("too many disablement secrets (%d, max %d)", numDS, maxDisablementSecrets) - } - for i, ds := range a.State.DisablementSecrets { - if len(ds) != disablementLength { - return fmt.Errorf("disablement[%d]: invalid length (got %d, want %d)", i, len(ds), disablementLength) - } + if err := a.State.staticValidateCheckpoint(); err != nil { + return fmt.Errorf("checkpoint state: %v", err) } - // TODO(tom): Check for duplicate disablement secrets. - - if len(a.State.Keys) == 0 { - return errors.New("at least one key is required") - } - if numKeys := len(a.State.Keys); numKeys > maxKeys { - return fmt.Errorf("too many keys (%d, max %d)", numKeys, maxKeys) - } - for i, k := range a.State.Keys { - if err := k.StaticValidate(); err != nil { - return fmt.Errorf("key[%d]: %v", i, err) - } - } - // TODO(tom): Check for duplicate keys. } switch a.MessageKind { @@ -213,7 +180,7 @@ func (a *AUM) StaticValidate() error { return errors.New("DisableNL AUMs must specify a disablement secret") } if a.KeyID != nil || a.State != nil || a.Key != nil || a.Votes != nil || a.Meta != nil { - return errors.New("DisableNL AUMs may only a disablement secret") + return errors.New("DisableNL AUMs may only specify a disablement secret") } } @@ -304,18 +271,17 @@ func (a *AUM) Weight(state State) uint { // signatures with the same key do not result in 2x // the weight. // - // We use the first 8 bytes as the key for this map, - // because KeyIDs are either a blake2s hash or - // the 25519 public key, both of which approximate - // random distribution. - seenKeys := make(map[uint64]struct{}, 6) + // Despite the wire encoding being []byte, all KeyIDs are + // 32 bytes. As such, we use that as the key for the map, + // because map keys cannot be slices. + seenKeys := make(map[[32]byte]struct{}, 6) for _, sig := range a.Signatures { - if len(sig.KeyID) < 8 { - // Invalid, don't count it - continue + if len(sig.KeyID) != 32 { + panic("unexpected: keyIDs are 32 bytes") } - keyID := binary.LittleEndian.Uint64(sig.KeyID) + var keyID [32]byte + copy(keyID[:], sig.KeyID) key, err := state.GetKey(sig.KeyID) if err != nil { diff --git a/tka/scenario_test.go b/tka/scenario_test.go index 2d39f7a22..43bcaae8f 100644 --- a/tka/scenario_test.go +++ b/tka/scenario_test.go @@ -113,28 +113,35 @@ outer: return strings.Join(out, ",") } -func (s *scenarioTest) syncBetween(n1, n2 *scenarioNode) { +func (s *scenarioTest) syncBetween(n1, n2 *scenarioNode) error { o1, err := n1.A.SyncOffer() if err != nil { - s.t.Fatal(err) + return err } o2, err := n2.A.SyncOffer() if err != nil { - s.t.Fatal(err) + return err } aumsFrom1, err := n1.A.MissingAUMs(o2) if err != nil { - s.t.Fatal(err) + return err } aumsFrom2, err := n2.A.MissingAUMs(o1) if err != nil { - s.t.Fatal(err) + return err } if err := n2.A.Inform(aumsFrom1); err != nil { - s.t.Fatal(err) + return err } if err := n1.A.Inform(aumsFrom2); err != nil { + return err + } + return nil +} + +func (s *scenarioTest) testSyncsBetween(n1, n2 *scenarioNode) { + if err := s.syncBetween(n1, n2); err != nil { s.t.Fatal(err) } } @@ -201,7 +208,7 @@ func TestScenarioHelpers(t *testing.T) { t.Errorf("chained AUM was not signed: %v", err) } - s.syncBetween(control, n) + s.testSyncsBetween(control, n) s.checkHaveConsensus(control, n) } @@ -217,13 +224,13 @@ func TestNormalPropergation(t *testing.T) { "L2": newTestchain(t, `L3 -> L4`), }) // Can control haz the updates? - s.syncBetween(control, n1) + s.testSyncsBetween(control, n1) s.checkHaveConsensus(control, n1) // A new node came online, can the new node learn everything // just via control? n2 := s.mkNode("n2") - s.syncBetween(control, n2) + s.testSyncsBetween(control, n2) s.checkHaveConsensus(control, n2) // So by virtue of syncing with control n2 should be at the same @@ -252,7 +259,7 @@ func TestForkingPropergation(t *testing.T) { "L2": newTestchain(t, `L3 -> L4`), }) // Can control haz the updates? - s.syncBetween(control, n1) + s.testSyncsBetween(control, n1) s.checkHaveConsensus(control, n1) // Ooooo what about a forking update? @@ -264,11 +271,11 @@ func TestForkingPropergation(t *testing.T) { optKey("key2", key, priv), optTemplate("removeKey1", AUM{MessageKind: AUMRemoveKey, KeyID: s.defaultKey.ID()})), }) - s.syncBetween(control, n2) + s.testSyncsBetween(control, n2) s.checkHaveConsensus(control, n2) // No wozzles propergating from n2->CTRL, what about CTRL->n1? - s.syncBetween(control, n1) + s.testSyncsBetween(control, n1) s.checkHaveConsensus(n1, n2) if _, err := n1.A.state.GetKey(s.defaultKey.ID()); err != ErrNoSuchKey { @@ -278,3 +285,104 @@ func TestForkingPropergation(t *testing.T) { t.Errorf("key2 was not trusted: %v", err) } } + +func TestInvalidAUMPropergationRejected(t *testing.T) { + s := testScenario(t, ` + G -> L1 -> L2 + G.template = genesis + `) + control := s.mkNode("control") + + // Construct an invalid L4 AUM, and manually apply it to n1, + // resulting in a corrupted Authority. + n1 := s.mkNodeWithForks("n1", true, map[string]*testChain{ + "L2": newTestchain(t, `L3`), + }) + l3 := n1.AUMs["L3"] + l3H := l3.Hash() + l4 := AUM{MessageKind: AUMAddKey, PrevAUMHash: l3H[:]} + l4.sign25519(s.defaultPriv) + l4H := l4.Hash() + n1.A.storage.CommitVerifiedAUMs([]AUM{l4}) + n1.A.state.LastAUMHash = &l4H + + // Does control nope out with syncing? + if err := s.syncBetween(control, n1); err == nil { + t.Error("sync with invalid AUM was successful") + } + + // Control should not have accepted ANY of the updates, even + // though L3 was well-formed. + l2 := control.AUMs["L2"] + l2H := l2.Hash() + if control.A.Head() != l2H { + t.Errorf("head was %x, expected %x", control.A.Head(), l2H) + } +} + +func TestUnsignedAUMPropergationRejected(t *testing.T) { + s := testScenario(t, ` + G -> L1 -> L2 + G.template = genesis + `) + control := s.mkNode("control") + + // Construct an unsigned L4 AUM, and manually apply it to n1, + // resulting in a corrupted Authority. + n1 := s.mkNodeWithForks("n1", true, map[string]*testChain{ + "L2": newTestchain(t, `L3`), + }) + l3 := n1.AUMs["L3"] + l3H := l3.Hash() + l4 := AUM{MessageKind: AUMNoOp, PrevAUMHash: l3H[:]} + l4H := l4.Hash() + n1.A.storage.CommitVerifiedAUMs([]AUM{l4}) + n1.A.state.LastAUMHash = &l4H + + // Does control nope out with syncing? + if err := s.syncBetween(control, n1); err == nil || err.Error() != "update 1 invalid: unsigned AUM" { + t.Errorf("sync with unsigned AUM was successful (err = %v)", err) + } + + // Control should not have accepted ANY of the updates, even + // though L3 was well-formed. + l2 := control.AUMs["L2"] + l2H := l2.Hash() + if control.A.Head() != l2H { + t.Errorf("head was %x, expected %x", control.A.Head(), l2H) + } +} + +func TestBadSigAUMPropergationRejected(t *testing.T) { + s := testScenario(t, ` + G -> L1 -> L2 + G.template = genesis + `) + control := s.mkNode("control") + + // Construct a otherwise-valid L4 AUM but mess up the signature. + n1 := s.mkNodeWithForks("n1", true, map[string]*testChain{ + "L2": newTestchain(t, `L3`), + }) + l3 := n1.AUMs["L3"] + l3H := l3.Hash() + l4 := AUM{MessageKind: AUMNoOp, PrevAUMHash: l3H[:]} + l4.sign25519(s.defaultPriv) + l4.Signatures[0].Signature[3] = 42 + l4H := l4.Hash() + n1.A.storage.CommitVerifiedAUMs([]AUM{l4}) + n1.A.state.LastAUMHash = &l4H + + // Does control nope out with syncing? + if err := s.syncBetween(control, n1); err == nil || err.Error() != "update 1 invalid: signature 0: invalid signature" { + t.Errorf("sync with unsigned AUM was successful (err = %v)", err) + } + + // Control should not have accepted ANY of the updates, even + // though L3 was well-formed. + l2 := control.AUMs["L2"] + l2H := l2.Hash() + if control.A.Head() != l2H { + t.Errorf("head was %x, expected %x", control.A.Head(), l2H) + } +} diff --git a/tka/state.go b/tka/state.go index c1351a0e5..b19ad3c6f 100644 --- a/tka/state.go +++ b/tka/state.go @@ -147,6 +147,9 @@ func (s State) applyVerifiedAUM(update AUM) (State, error) { return update.State.cloneForUpdate(&update), nil case AUMAddKey: + if update.Key == nil { + return State{}, errors.New("no key to add provided") + } if _, err := s.GetKey(update.Key.ID()); err == nil { return State{}, errors.New("key already exists") } @@ -202,3 +205,58 @@ func (s State) applyVerifiedAUM(update AUM) (State, error) { return State{}, fmt.Errorf("unhandled message: %v", update.MessageKind) } } + +// Upper bound on checkpoint elements, chosen arbitrarily. Intended to +// cap out insanely large AUMs. +const ( + maxDisablementSecrets = 32 + maxKeys = 512 +) + +// staticValidateCheckpoint validates that the state is well-formed for +// inclusion in a checkpoint AUM. +func (s *State) staticValidateCheckpoint() error { + if s.LastAUMHash != nil { + return errors.New("cannot specify a parent AUM") + } + if len(s.DisablementSecrets) == 0 { + return errors.New("at least one disablement secret required") + } + if numDS := len(s.DisablementSecrets); numDS > maxDisablementSecrets { + return fmt.Errorf("too many disablement secrets (%d, max %d)", numDS, maxDisablementSecrets) + } + for i, ds := range s.DisablementSecrets { + if len(ds) != disablementLength { + return fmt.Errorf("disablement[%d]: invalid length (got %d, want %d)", i, len(ds), disablementLength) + } + for j, ds2 := range s.DisablementSecrets { + if i == j { + continue + } + if bytes.Equal(ds, ds2) { + return fmt.Errorf("disablement[%d]: duplicates disablement[%d]", i, j) + } + } + } + + if len(s.Keys) == 0 { + return errors.New("at least one key is required") + } + if numKeys := len(s.Keys); numKeys > maxKeys { + return fmt.Errorf("too many keys (%d, max %d)", numKeys, maxKeys) + } + for i, k := range s.Keys { + if err := k.StaticValidate(); err != nil { + return fmt.Errorf("key[%d]: %v", i, err) + } + for j, k2 := range s.Keys { + if i == j { + continue + } + if bytes.Equal(k.ID(), k2.ID()) { + return fmt.Errorf("key[%d]: duplicates key[%d]", i, j) + } + } + } + return nil +}