From f580f4484fe70540ba5b12a62d0e819b094036ff Mon Sep 17 00:00:00 2001 From: Tom DNetto Date: Thu, 25 Aug 2022 12:47:30 -0700 Subject: [PATCH] tka: move disablement logic out-of-band from AUMs It doesn't make a ton of sense for disablement to be communicated as an AUM, because any failure in the AUM or chain mechanism will mean disablement wont function. Instead, tracking of the disablement secrets remains inside the state machine, but actual disablement and communication of the disablement secret is done by the caller. Signed-off-by: Tom DNetto --- tka/aum.go | 35 +++++++++++------------------------ tka/aum_test.go | 25 ++++--------------------- tka/state.go | 8 -------- tka/state_test.go | 19 ++++--------------- tka/tailchonk_test.go | 4 ++-- tka/tka.go | 8 ++++++++ tka/tka_test.go | 29 +++++++++++++++++++++++++---- 7 files changed, 54 insertions(+), 74 deletions(-) diff --git a/tka/aum.go b/tka/aum.go index fef2b6b78..9c2daac6d 100644 --- a/tka/aum.go +++ b/tka/aum.go @@ -59,10 +59,6 @@ const ( // // Only the KeyID optional field may be set. AUMRemoveKey - // A DisableNL AUM describes the disablement of TKA. - // - // Only the DisablementSecret optional field may be set. - AUMDisableNL // A NoOp AUM carries no information and is used in tests. AUMNoOp // A UpdateKey AUM updates the metadata or votes of an existing key. @@ -84,8 +80,6 @@ func (k AUMKind) String() string { return "add-key" case AUMRemoveKey: return "remove-key" - case AUMDisableNL: - return "disable-nl" case AUMNoOp: return "no-op" case AUMCheckpoint: @@ -130,15 +124,10 @@ type AUM struct { // This field is used for Checkpoint AUMs. State *State `cbor:"5,keyasint,omitempty"` - // DisablementSecret is used to transmit a secret for disabling - // the TKA. - // This field is used for DisableNL AUMs. - DisablementSecret []byte `cbor:"6,keyasint,omitempty"` - // Votes and Meta describe properties of a key in the key authority. // These fields are used for UpdateKey AUMs. - Votes *uint `cbor:"7,keyasint,omitempty"` - Meta map[string]string `cbor:"8,keyasint,omitempty"` + Votes *uint `cbor:"6,keyasint,omitempty"` + Meta map[string]string `cbor:"7,keyasint,omitempty"` // Signatures lists the signatures over this AUM. // CBOR key 23 is the last key which can be encoded as a single byte. @@ -172,14 +161,14 @@ func (a *AUM) StaticValidate() error { if a.Key == nil { return errors.New("AddKey AUMs must contain a key") } - if a.KeyID != nil || a.DisablementSecret != nil || a.State != nil || a.Votes != nil || a.Meta != nil { + if a.KeyID != nil || a.State != nil || a.Votes != nil || a.Meta != nil { return errors.New("AddKey AUMs may only specify a Key") } case AUMRemoveKey: if len(a.KeyID) == 0 { return errors.New("RemoveKey AUMs must specify a key ID") } - if a.Key != nil || a.DisablementSecret != nil || a.State != nil || a.Votes != nil || a.Meta != nil { + if a.Key != nil || a.State != nil || a.Votes != nil || a.Meta != nil { return errors.New("RemoveKey AUMs may only specify a KeyID") } case AUMUpdateKey: @@ -189,23 +178,21 @@ func (a *AUM) StaticValidate() error { if a.Meta == nil && a.Votes == nil { return errors.New("UpdateKey AUMs must contain an update to votes or key metadata") } - if a.Key != nil || a.DisablementSecret != nil || a.State != nil { + if a.Key != nil || a.State != nil { return errors.New("UpdateKey AUMs may only specify KeyID, Votes, and Meta") } case AUMCheckpoint: if a.State == nil { return errors.New("Checkpoint AUMs must specify the state") } - if a.KeyID != nil || a.DisablementSecret != nil || a.Key != nil || a.Votes != nil || a.Meta != nil { + if a.KeyID != nil || a.Key != nil || a.Votes != nil || a.Meta != nil { return errors.New("Checkpoint AUMs may only specify State") } - case AUMDisableNL: - if len(a.DisablementSecret) == 0 { - 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 specify a disablement secret") - } + + case AUMNoOp: + default: + // TODO(tom): Ignore unknown AUMs for GA. + return fmt.Errorf("unknown AUM kind: %v", a.MessageKind) } return nil diff --git a/tka/aum_test.go b/tka/aum_test.go index 3f2449889..e7182f19a 100644 --- a/tka/aum_test.go +++ b/tka/aum_test.go @@ -62,16 +62,16 @@ func TestSerialization(t *testing.T) { []byte{ 0xa5, // major type 5 (map), 5 items 0x01, // |- major type 0 (int), value 1 (first key, MessageKind) - 0x05, // |- major type 0 (int), value 2 (first value, AUMUpdateKey) + 0x04, // |- major type 0 (int), value 4 (first value, AUMUpdateKey) 0x02, // |- major type 0 (int), value 2 (second key, PrevAUMHash) 0xf6, // |- major type 7 (val), value null (second value, nil) 0x04, // |- major type 0 (int), value 4 (third key, KeyID) 0x42, // |- major type 2 (byte string), 2 items 0x01, // |- major type 0 (int), value 1 (byte 1) 0x02, // |- major type 0 (int), value 2 (byte 2) - 0x07, // |- major type 0 (int), value 7 (fourth key, Votes) + 0x06, // |- major type 0 (int), value 6 (fourth key, Votes) 0x02, // |- major type 0 (int), value 2 (forth value, 2) - 0x08, // |- major type 0 (int), value 8 (fifth key, Meta) + 0x07, // |- major type 0 (int), value 7 (fifth key, Meta) 0xa1, // |- major type 5 (map), 1 item (map[string]string type) 0x61, // |- major type 3 (text string), value 1 (first key, one byte long) 0x61, // |- byte 'a' @@ -79,23 +79,6 @@ func TestSerialization(t *testing.T) { 0x62, // |- byte 'b' }, }, - { - "DisableNL", - AUM{MessageKind: AUMDisableNL, PrevAUMHash: []byte{1, 2}, DisablementSecret: []byte{3, 4}}, - []byte{ - 0xa3, // major type 5 (map), 3 items - 0x01, // |- major type 0 (int), value 1 (first key, MessageKind) - 0x03, // |- major type 0 (int), value 3 (first value, AUMDisableNL) - 0x02, // |- major type 0 (int), value 2 (second key, PrevAUMHash) - 0x42, // |- major type 2 (byte string), 2 items (second value) - 0x01, // |- major type 0 (int), value 1 (byte 1) - 0x02, // |- major type 0 (int), value 2 (byte 2) - 0x06, // |- major type 0 (int), value 6 (third key, DisablementSecret) - 0x42, // |- major type 2 (byte string), 2 items (third value) - 0x03, // |- major type 0 (int), value 3 (byte 3) - 0x04, // |- major type 0 (int), value 4 (byte 4) - }, - }, { "Checkpoint", AUM{MessageKind: AUMCheckpoint, PrevAUMHash: []byte{1, 2}, State: &State{ @@ -108,7 +91,7 @@ func TestSerialization(t *testing.T) { append([]byte{ 0xa3, // major type 5 (map), 3 items 0x01, // |- major type 0 (int), value 1 (first key, MessageKind) - 0x06, // |- major type 0 (int), value 6 (first value, AUMCheckpoint) + 0x05, // |- major type 0 (int), value 5 (first value, AUMCheckpoint) 0x02, // |- major type 0 (int), value 2 (second key, PrevAUMHash) 0x42, // |- major type 2 (byte string), 2 items (second value) 0x01, // |- major type 0 (int), value 1 (byte 1) diff --git a/tka/state.go b/tka/state.go index 80b93c583..d25dc0d00 100644 --- a/tka/state.go +++ b/tka/state.go @@ -192,14 +192,6 @@ func (s State) applyVerifiedAUM(update AUM) (State, error) { out.Keys = append(out.Keys[:idx], out.Keys[idx+1:]...) return out, nil - case AUMDisableNL: - // TODO(tom): We should handle this at a higher level than State. - if !s.checkDisablement(update.DisablementSecret) { - return State{}, errors.New("incorrect disablement secret") - } - // Valid disablement secret, lets reset - return State{}, nil - default: // TODO(tom): Instead of erroring, update lastHash and // continue (to preserve future compatibility). diff --git a/tka/state_test.go b/tka/state_test.go index d2aa16b25..85c9f9f7d 100644 --- a/tka/state_test.go +++ b/tka/state_test.go @@ -121,7 +121,7 @@ func TestApplyUpdatesChain(t *testing.T) { LastAUMHash: hashFromHex("53898e4311d0b6087fcbb871563868a16c629d9267df851fcfa7b52b31d2bd03"), }, State{ - LastAUMHash: hashFromHex("828fe04c16032cf3e0b021abca0b4d79924b0a18b2e627b308347aa87ce7c21c"), + LastAUMHash: hashFromHex("d55458a9c3ed6997439ba5a18b9b62d2c6e5e0c1bb4c61409e92a1281a3b458d"), Keys: []Key{{Kind: Key25519, Votes: 1, Meta: map[string]string{"a": "b"}, Public: []byte{1, 2, 3, 4}}}, }, }, @@ -139,15 +139,6 @@ func TestApplyUpdatesChain(t *testing.T) { LastAUMHash: hashFromHex("218165fe5f757304b9deaff4ac742890364f5f509e533c74e80e0ce35e44ee1d"), }, }, - { - "Disablement", - []AUM{{MessageKind: AUMDisableNL, DisablementSecret: []byte{1, 2, 3, 4}, PrevAUMHash: fromHex("53898e4311d0b6087fcbb871563868a16c629d9267df851fcfa7b52b31d2bd03")}}, - State{ - DisablementSecrets: [][]byte{disablementKDF([]byte{1, 2, 3, 4})}, - LastAUMHash: hashFromHex("53898e4311d0b6087fcbb871563868a16c629d9267df851fcfa7b52b31d2bd03"), - }, - State{}, - }, { "Checkpoint", []AUM{ @@ -159,7 +150,7 @@ func TestApplyUpdatesChain(t *testing.T) { State{DisablementSecrets: [][]byte{[]byte{1, 2, 3, 4}}}, State{ Keys: []Key{{Kind: Key25519, Public: []byte{1, 2, 3, 4}}}, - LastAUMHash: hashFromHex("2e34f7e21883c35c8e34ec06e735f7ed8a14c3ceeb11ccb18fcbc11d51c8dabb"), + LastAUMHash: hashFromHex("57343671da5eea3cfb502954e976e8028bffd3540b50a043b2a65a8d8d8217d0"), }, }, } @@ -177,10 +168,8 @@ func TestApplyUpdatesChain(t *testing.T) { // t.Logf("update[%d] end-state = %+v", i, state) updateHash := tc.Updates[i].Hash() - if tc.Updates[i].MessageKind != AUMDisableNL { - if got, want := *state.LastAUMHash, updateHash[:]; !bytes.Equal(got[:], want) { - t.Errorf("expected state.LastAUMHash = %x (update %d), got %x", want, i, got) - } + if got, want := *state.LastAUMHash, updateHash[:]; !bytes.Equal(got[:], want) { + t.Errorf("expected state.LastAUMHash = %x (update %d), got %x", want, i, got) } } diff --git a/tka/tailchonk_test.go b/tka/tailchonk_test.go index 3ca35dbe7..bb8b61fc6 100644 --- a/tka/tailchonk_test.go +++ b/tka/tailchonk_test.go @@ -147,10 +147,10 @@ func TestTailchonkFS_Commit(t *testing.T) { } dir, base := chonk.aumDir(aum.Hash()) - if got, want := dir, filepath.Join(chonk.base, "VU"); got != want { + if got, want := dir, filepath.Join(chonk.base, "PD"); got != want { t.Errorf("aum dir=%s, want %s", got, want) } - if want := "VU5G7NN5FGCWEWKC7SBZUSIGTQOR3VF52ED33GQIWLZU7GYPGN7Q"; base != want { + if want := "PD57DVP6GKC76OOZMXFFZUSOEFQXOLAVT7N2ZM5KB3HDIMCANF4A"; base != want { t.Errorf("aum base=%s, want %s", base, want) } if _, err := os.Stat(filepath.Join(dir, base)); err != nil { diff --git a/tka/tka.go b/tka/tka.go index ce5690134..61434592c 100644 --- a/tka/tka.go +++ b/tka/tka.go @@ -549,6 +549,14 @@ func Bootstrap(storage Chonk, bootstrap AUM) (*Authority, error) { return Open(storage) } +// ValidDisablement returns true if the disablement secret was correct. +// +// If this method returns true, the caller should shut down the authority +// and purge all network-lock state. +func (a *Authority) ValidDisablement(secret []byte) bool { + return a.state.checkDisablement(secret) +} + // Inform is called to tell the authority about new updates. Updates // should be ordered oldest to newest. An error is returned if any // of the updates could not be processed. diff --git a/tka/tka_test.go b/tka/tka_test.go index 361ae2a97..2215b5d33 100644 --- a/tka/tka_test.go +++ b/tka/tka_test.go @@ -35,10 +35,10 @@ func TestComputeChainCandidates(t *testing.T) { } want := []chain{ - {Oldest: c.AUMs["G1"], Head: c.AUMs["L1"], chainsThroughActive: true}, + {Oldest: c.AUMs["G2"], Head: c.AUMs["L4"]}, {Oldest: c.AUMs["G1"], Head: c.AUMs["L3"], chainsThroughActive: true}, + {Oldest: c.AUMs["G1"], Head: c.AUMs["L1"], chainsThroughActive: true}, {Oldest: c.AUMs["G1"], Head: c.AUMs["L2"], chainsThroughActive: true}, - {Oldest: c.AUMs["G2"], Head: c.AUMs["L4"]}, } if diff := cmp.Diff(want, got, cmp.AllowUnexported(chain{})); diff != "" { t.Errorf("chains differ (-want, +got):\n%s", diff) @@ -82,7 +82,7 @@ func TestForkResolutionSigWeight(t *testing.T) { | -> L2 G1.template = addKey - L1.hashSeed = 2 + L1.hashSeed = 11 L2.signedWith = key `, optTemplate("addKey", AUM{MessageKind: AUMAddKey, Key: &key}), @@ -295,6 +295,26 @@ func TestAuthorityHead(t *testing.T) { } } +func TestAuthorityValidDisablement(t *testing.T) { + pub, _ := testingKey25519(t, 1) + key := Key{Kind: Key25519, Public: pub, Votes: 2} + c := newTestchain(t, ` + G1 -> L1 + + G1.template = genesis + `, + optTemplate("genesis", AUM{MessageKind: AUMCheckpoint, State: &State{ + Keys: []Key{key}, + DisablementSecrets: [][]byte{disablementKDF([]byte{1, 2, 3})}, + }}), + ) + + a, _ := Open(c.Chonk()) + if valid := a.ValidDisablement([]byte{1, 2, 3}); !valid { + t.Error("ValidDisablement() returned false, want true") + } +} + func TestCreateBootstrapAuthority(t *testing.T) { pub, priv := testingKey25519(t, 1) key := Key{Kind: Key25519, Public: pub, Votes: 2} @@ -335,7 +355,8 @@ func TestAuthorityInformNonLinear(t *testing.T) { | -> L4 -> L5 G1.template = genesis - L2.hashSeed = 1 + L1.hashSeed = 3 + L2.hashSeed = 2 L4.hashSeed = 2 `, optTemplate("genesis", AUM{MessageKind: AUMCheckpoint, State: &State{