From 1dc0dc9f2311d32e97e327e648e815c6334e3b2f Mon Sep 17 00:00:00 2001 From: Evan Cordell Date: Fri, 6 Jan 2017 08:55:39 -0500 Subject: [PATCH] Additional root rotation tests Signed-off-by: Evan Cordell --- client/client_update_test.go | 115 +++++++++++++++++- client/tufclient.go | 25 ++-- server/handlers/validation_test.go | 42 +++++++ server/storage/rethinkdb.go | 2 +- server/storage/rethinkdb_models.go | 8 +- signer/client/signer_trust.go | 6 - signer/keydbstore/rethink_keydbstore.go | 10 -- .../keydbstore/rethink_realkeydbstore_test.go | 4 - signer/keydbstore/sql_keydbstore.go | 9 -- signer/keydbstore/sql_keydbstore_test.go | 4 - tuf/builder.go | 4 - tuf/signed/ed25519.go | 8 -- tuf/signed/ed25519_test.go | 4 - tuf/signed/interface.go | 8 +- tuf/signed/sign_test.go | 11 -- 15 files changed, 174 insertions(+), 86 deletions(-) diff --git a/client/client_update_test.go b/client/client_update_test.go index a9208f56a6..e318cde426 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -1600,7 +1600,7 @@ func TestValidateRootRotationWithOldRole(t *testing.T) { // A valid root role is signed by the current root role keys and the previous root role keys func TestRootRoleInvariant(t *testing.T) { - // start with a repo with a root with 2 keys, optionally signing 1 + // start with a repo _, serverSwizzler := newServerSwizzler(t) ts := readOnlyServer(t, serverSwizzler.MetadataCache, http.StatusNotFound, "docker.com/notary") defer ts.Close() @@ -1679,6 +1679,119 @@ func TestRootRoleInvariant(t *testing.T) { requireRootSignatures(t, serverSwizzler, 3) } +// All intermediate roots must be signed by the previous root role +func TestBadIntermediateTransitions(t *testing.T) { + // start with a repo + _, serverSwizzler := newServerSwizzler(t) + ts := readOnlyServer(t, serverSwizzler.MetadataCache, http.StatusNotFound, "docker.com/notary") + defer ts.Close() + + repo := newBlankRepo(t, ts.URL) + defer os.RemoveAll(repo.baseDir) + + // --- setup so that the root starts with a role with 1 keys, and threshold of 1 + rootBytes, err := serverSwizzler.MetadataCache.GetSized(data.CanonicalRootRole, store.NoSizeLimit) + require.NoError(t, err) + signedRoot := data.SignedRoot{} + require.NoError(t, json.Unmarshal(rootBytes, &signedRoot)) + + // generate keys for testing + threeKeys := make([]data.PublicKey, 3) + keyIDs := make([]string, len(threeKeys)) + for i := 0; i < len(threeKeys); i++ { + threeKeys[i], err = testutils.CreateKey( + serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalRootRole, data.ECDSAKey) + require.NoError(t, err) + keyIDs[i] = threeKeys[i].ID() + } + + // increment the root version and sign with the first key only + signedRoot.Signed.Version++ + signedRoot.Signed.Keys[keyIDs[0]] = threeKeys[0] + signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[0]} + signedRoot.Signed.Roles[data.CanonicalRootRole].Threshold = 1 + signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[0]}) + + require.NoError(t, repo.Update(false)) + + // increment the root version and sign with the second key only + signedRoot.Signed.Version++ + delete(signedRoot.Signed.Keys, keyIDs[0]) + signedRoot.Signed.Keys[keyIDs[1]] = threeKeys[1] + signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[1]} + signedRoot.Signed.Roles[data.CanonicalRootRole].Threshold = 1 + signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[1]}) + + // increment the root version and sign with all three keys + signedRoot.Signed.Version++ + signedRoot.Signed.Keys[keyIDs[0]] = threeKeys[0] + signedRoot.Signed.Keys[keyIDs[1]] = threeKeys[1] + signedRoot.Signed.Keys[keyIDs[2]] = threeKeys[2] + signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[0], keyIDs[1], keyIDs[2]} + signedRoot.Signed.Roles[data.CanonicalRootRole].Threshold = 1 + signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[1]}) + requireRootSignatures(t, serverSwizzler, 1) + + // Update fails because version 1 -> 2 is invalid. + require.Error(t, repo.Update(false)) +} + +// All intermediate roots must be signed by the previous root role +func TestExpiredIntermediateTransitions(t *testing.T) { + // start with a repo + _, serverSwizzler := newServerSwizzler(t) + ts := readOnlyServer(t, serverSwizzler.MetadataCache, http.StatusNotFound, "docker.com/notary") + defer ts.Close() + + repo := newBlankRepo(t, ts.URL) + defer os.RemoveAll(repo.baseDir) + + // --- setup so that the root starts with a role with 1 keys, and threshold of 1 + rootBytes, err := serverSwizzler.MetadataCache.GetSized(data.CanonicalRootRole, store.NoSizeLimit) + require.NoError(t, err) + signedRoot := data.SignedRoot{} + require.NoError(t, json.Unmarshal(rootBytes, &signedRoot)) + + // generate keys for testing + threeKeys := make([]data.PublicKey, 3) + keyIDs := make([]string, len(threeKeys)) + for i := 0; i < len(threeKeys); i++ { + threeKeys[i], err = testutils.CreateKey( + serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalRootRole, data.ECDSAKey) + require.NoError(t, err) + keyIDs[i] = threeKeys[i].ID() + } + + // increment the root version and sign with the first key only + signedRoot.Signed.Version++ + signedRoot.Signed.Keys[keyIDs[0]] = threeKeys[0] + signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[0]} + signedRoot.Signed.Roles[data.CanonicalRootRole].Threshold = 1 + signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[0]}) + + require.NoError(t, repo.Update(false)) + + // increment the root version and sign with the first and second keys, but set metadata to be expired. + signedRoot.Signed.Version++ + signedRoot.Signed.Keys[keyIDs[1]] = threeKeys[1] + signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[0], keyIDs[1]} + signedRoot.Signed.Roles[data.CanonicalRootRole].Threshold = 1 + signedRoot.Signed.Expires = time.Now().AddDate(0, -1, 0) + signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[0], threeKeys[1]}) + + // increment the root version and sign with all three keys + signedRoot.Signed.Version++ + signedRoot.Signed.Keys[keyIDs[2]] = threeKeys[2] + signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[0], keyIDs[1], keyIDs[2]} + signedRoot.Signed.Roles[data.CanonicalRootRole].Threshold = 1 + signedRoot.Signed.Expires = time.Now().AddDate(0, 1, 0) + signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, threeKeys[:3]) + requireRootSignatures(t, serverSwizzler, 3) + + // Update succeeds despite version 2 being expired. + require.NoError(t, repo.Update(false)) +} + // TestDownloadTargetsLarge: Check that we can download very large targets metadata files, // which may be caused by adding a large number of targets. // This test is slow, so it will not run in short mode. diff --git a/client/tufclient.go b/client/tufclient.go index ced0800b88..726a3adc83 100644 --- a/client/tufclient.go +++ b/client/tufclient.go @@ -83,22 +83,24 @@ func (c *TUFClient) update() error { // updateRoot checks if there is a newer version of the root available, and if so // downloads all intermediate root files to allow proper key rotation. func (c *TUFClient) updateRoot() error { - // Get Current Root Version + // Get current root version currentRootConsistentInfo := c.oldBuilder.GetConsistentInfo(data.CanonicalRootRole) currentVersion := c.oldBuilder.GetLoadedVersion(currentRootConsistentInfo.RoleName) - // Get New Root Version + // Get new root version raw, err := c.downloadRoot() - // Return any non-rotation error. Rotation errors are okay since we haven't yet downloaded - // all intermediate root files - if _, ok := err.(*trustpinning.ErrRootRotationFail); err != nil && !ok { - return err - } - - // No error updating root - we were at most 1 version behind - if err == nil { + switch err.(type) { + case *trustpinning.ErrRootRotationFail: + // Rotation errors are okay since we haven't yet downloaded + // all intermediate root files + break + case nil: + // No error updating root - we were at most 1 version behind return nil + default: + // Return any non-rotation error. + return err } // Load current version into newBuilder @@ -161,9 +163,6 @@ func (c *TUFClient) updateRootVersions(fromVersion, toVersion int) error { return err } logrus.Debugf("successfully verified downloaded %s", versionedRole) - if err := c.cache.Set(data.CanonicalRootRole, raw); err != nil { - logrus.Debugf("unable to write %s to cache: %s", versionedRole, err) - } } return nil } diff --git a/server/handlers/validation_test.go b/server/handlers/validation_test.go index 4399563e31..c22e103ad8 100644 --- a/server/handlers/validation_test.go +++ b/server/handlers/validation_test.go @@ -631,6 +631,48 @@ func TestRootRotationNotSignedWithOldKeysForOldRole(t *testing.T) { require.NoError(t, err) } +// A root rotation must increment the root version by 1 +func TestRootRotationVersionIncrement(t *testing.T) { + gun := "docker.com/notary" + repo, crypto, err := testutils.EmptyRepo(gun) + require.NoError(t, err) + serverCrypto := testutils.CopyKeys(t, crypto, data.CanonicalTimestampRole) + store := storage.NewMemStorage() + + r, tg, sn, ts, err := testutils.Sign(repo) + require.NoError(t, err) + root, targets, snapshot, timestamp, err := getUpdates(r, tg, sn, ts) + require.NoError(t, err) + + // set the original root in the store + updates := []storage.MetaUpdate{root, targets, snapshot, timestamp} + require.NoError(t, store.UpdateMany(gun, updates)) + + // rotate the root key, sign with both keys, and update - update should succeed + newRootKey, err := testutils.CreateKey(crypto, gun, data.CanonicalRootRole, data.ECDSAKey) + require.NoError(t, err) + + require.NoError(t, repo.ReplaceBaseKeys(data.CanonicalRootRole, newRootKey)) + r, _, sn, _, err = testutils.Sign(repo) + require.NoError(t, err) + root, _, snapshot, _, err = getUpdates(r, tg, sn, ts) + require.NoError(t, err) + snapshot.Version = repo.Snapshot.Signed.Version + + // Wrong root version + root.Version = repo.Root.Signed.Version + 1 + + _, err = validateUpdate(serverCrypto, gun, []storage.MetaUpdate{root, snapshot}, store) + require.Error(t, err) + require.Contains(t, err.Error(), "Root modifications must increment the version") + + // correct root version + root.Version = root.Version - 1 + updates, err = validateUpdate(serverCrypto, gun, []storage.MetaUpdate{root, snapshot}, store) + require.NoError(t, err) + require.NoError(t, store.UpdateMany(gun, updates)) +} + // An update is not valid without the root metadata. func TestValidateNoRoot(t *testing.T) { gun := "docker.com/notary" diff --git a/server/storage/rethinkdb.go b/server/storage/rethinkdb.go index 9f8d70ef70..2686fe04b9 100644 --- a/server/storage/rethinkdb.go +++ b/server/storage/rethinkdb.go @@ -216,7 +216,7 @@ func (rdb RethinkDB) GetCurrent(gun, role string) (created *time.Time, data []by func (rdb RethinkDB) GetChecksum(gun, role, checksum string) (created *time.Time, data []byte, err error) { var file RDBTUFFile res, err := gorethink.DB(rdb.dbName).Table(file.TableName(), gorethink.TableOpts{ReadMode: "majority"}).GetAllByIndex( - rdbGunRoleSha256Idx, []string{gun, role, checksum}, + rdbGunRoleSHA256Idx, []string{gun, role, checksum}, ).Run(rdb.sess) if err != nil { return nil, nil, err diff --git a/server/storage/rethinkdb_models.go b/server/storage/rethinkdb_models.go index ea29e15bf3..56295c5907 100644 --- a/server/storage/rethinkdb_models.go +++ b/server/storage/rethinkdb_models.go @@ -6,9 +6,9 @@ import ( // These consts are the index names we've defined for RethinkDB const ( - rdbSha256Idx = "sha256" + rdbSHA256Idx = "sha256" rdbGunRoleIdx = "gun_role" - rdbGunRoleSha256Idx = "gun_role_sha256" + rdbGunRoleSHA256Idx = "gun_role_sha256" rdbGunRoleVersionIdx = "gun_role_version" ) @@ -18,11 +18,11 @@ var ( Name: RDBTUFFile{}.TableName(), PrimaryKey: "gun_role_version", SecondaryIndexes: map[string][]string{ - rdbSha256Idx: nil, + rdbSHA256Idx: nil, "gun": nil, "timestamp_checksum": nil, rdbGunRoleIdx: {"gun", "role"}, - rdbGunRoleSha256Idx: {"gun", "role", "sha256"}, + rdbGunRoleSHA256Idx: {"gun", "role", "sha256"}, }, // this configuration guarantees linearizability of individual atomic operations on individual documents Config: map[string]string{ diff --git a/signer/client/signer_trust.go b/signer/client/signer_trust.go index 9c4856fd42..ee2aaef227 100644 --- a/signer/client/signer_trust.go +++ b/signer/client/signer_trust.go @@ -14,7 +14,6 @@ import ( "github.com/docker/notary" pb "github.com/docker/notary/proto" - "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" "golang.org/x/net/context" "google.golang.org/grpc" @@ -204,11 +203,6 @@ func (trust *NotarySigner) GetKey(keyid string) data.PublicKey { return pubKey } -// GetKeyInfo returns the gun and role given a keyID -func (trust *NotarySigner) GetKeyInfo(keyid string) (trustmanager.KeyInfo, error) { - return trustmanager.KeyInfo{}, errors.New("Getting KeyInfo from NotarySigner is not supported") -} - func (trust *NotarySigner) getKeyInfo(keyid string) (data.PublicKey, string, error) { keyInfo, err := trust.kmClient.GetKeyInfo(context.Background(), &pb.KeyID{ID: keyid}) if err != nil { diff --git a/signer/keydbstore/rethink_keydbstore.go b/signer/keydbstore/rethink_keydbstore.go index da80e39fc2..1198c8ee5d 100644 --- a/signer/keydbstore/rethink_keydbstore.go +++ b/signer/keydbstore/rethink_keydbstore.go @@ -215,16 +215,6 @@ func (rdb *RethinkDBKeyStore) GetKey(keyID string) data.PublicKey { return data.NewPublicKey(dbPrivateKey.Algorithm, dbPrivateKey.Public) } -// GetKeyInfo returns the gun and role given a KeyID, and does not activate the key -func (rdb *RethinkDBKeyStore) GetKeyInfo(keyID string) (trustmanager.KeyInfo, error) { - dbPrivateKey, _, err := rdb.getKey(keyID) - if err != nil { - return trustmanager.KeyInfo{}, err - } - - return trustmanager.KeyInfo{Gun: dbPrivateKey.Gun, Role: dbPrivateKey.Role}, nil -} - // ListKeys always returns nil. This method is here to satisfy the CryptoService interface func (rdb RethinkDBKeyStore) ListKeys(role string) []string { return nil diff --git a/signer/keydbstore/rethink_realkeydbstore_test.go b/signer/keydbstore/rethink_realkeydbstore_test.go index 8bdc6a70f1..f58ffed926 100644 --- a/signer/keydbstore/rethink_realkeydbstore_test.go +++ b/signer/keydbstore/rethink_realkeydbstore_test.go @@ -107,15 +107,11 @@ func requireExpectedRDBKeys(t *testing.T, dbStore *RethinkDBKeyStore, expectedKe } for _, key := range expectedKeys { - rdbKeyInfo, err := dbStore.GetKeyInfo(key.ID()) - require.NoError(t, err) rdbKey, ok := result[key.ID()] require.True(t, ok) require.NotNil(t, rdbKey) require.Equal(t, key.Public(), rdbKey.Public) require.Equal(t, key.Algorithm(), rdbKey.Algorithm) - require.Equal(t, rdbKey.Gun, rdbKeyInfo.Gun) - require.Equal(t, rdbKey.Role, rdbKeyInfo.Role) // because we have to manually set the created and modified times require.True(t, rdbKey.CreatedAt.Equal(rdbNow)) diff --git a/signer/keydbstore/sql_keydbstore.go b/signer/keydbstore/sql_keydbstore.go index 4c9120db39..b6493e6157 100644 --- a/signer/keydbstore/sql_keydbstore.go +++ b/signer/keydbstore/sql_keydbstore.go @@ -228,15 +228,6 @@ func (s *SQLKeyDBStore) GetKey(keyID string) data.PublicKey { return data.NewPublicKey(privKey.Algorithm, []byte(privKey.Public)) } -// GetKeyInfo returns role and GUN info of a key by ID -func (s *SQLKeyDBStore) GetKeyInfo(keyID string) (trustmanager.KeyInfo, error) { - privKey, _, err := s.getKey(keyID, false) - if err != nil { - return trustmanager.KeyInfo{}, err - } - return trustmanager.KeyInfo{Gun: privKey.Gun, Role: privKey.Role}, nil -} - // HealthCheck verifies that DB exists and is query-able func (s *SQLKeyDBStore) HealthCheck() error { dbPrivateKey := GormPrivateKey{} diff --git a/signer/keydbstore/sql_keydbstore_test.go b/signer/keydbstore/sql_keydbstore_test.go index 8f5ca91747..d7ca0f806c 100644 --- a/signer/keydbstore/sql_keydbstore_test.go +++ b/signer/keydbstore/sql_keydbstore_test.go @@ -82,15 +82,11 @@ func requireExpectedGORMKeys(t *testing.T, dbStore *SQLKeyDBStore, expectedKeys } for _, key := range expectedKeys { - gormKeyInfo, err := dbStore.GetKeyInfo(key.ID()) - require.NoError(t, err) gormKey, ok := result[key.ID()] require.True(t, ok) require.NotNil(t, gormKey) require.Equal(t, string(key.Public()), gormKey.Public) require.Equal(t, key.Algorithm(), gormKey.Algorithm) - require.Equal(t, gormKey.Gun, gormKeyInfo.Gun) - require.Equal(t, gormKey.Role, gormKeyInfo.Role) } return result diff --git a/tuf/builder.go b/tuf/builder.go index 17ea33026b..d4f0d8e9c0 100644 --- a/tuf/builder.go +++ b/tuf/builder.go @@ -427,10 +427,6 @@ func (rb *repoBuilder) GenerateTimestamp(prev *data.SignedTimestamp) ([]byte, in return sgndJSON, rb.repo.Timestamp.Signed.Version, nil } -func (rb *repoBuilder) setPrevRoot(root *data.SignedRoot) { - rb.prevRoot = root -} - // loadRoot loads a root if one has not been loaded func (rb *repoBuilder) loadRoot(content []byte, minVersion int, allowExpired, skipChecksum bool) error { roleName := data.CanonicalRootRole diff --git a/tuf/signed/ed25519.go b/tuf/signed/ed25519.go index 475280c707..7a70739e40 100644 --- a/tuf/signed/ed25519.go +++ b/tuf/signed/ed25519.go @@ -102,14 +102,6 @@ func (e *Ed25519) GetKey(keyID string) data.PublicKey { return nil } -// GetKeyInfo returns a key info based on the ID -func (e *Ed25519) GetKeyInfo(keyID string) (trustmanager.KeyInfo, error) { - if k, ok := e.keys[keyID]; ok { - return trustmanager.KeyInfo{Gun: "", Role: k.role}, nil - } - return trustmanager.KeyInfo{}, trustmanager.ErrKeyNotFound{KeyID: keyID} -} - // GetPrivateKey returns a single private key and role if present, based on the ID func (e *Ed25519) GetPrivateKey(keyID string) (data.PrivateKey, string, error) { if k, ok := e.keys[keyID]; ok { diff --git a/tuf/signed/ed25519_test.go b/tuf/signed/ed25519_test.go index d8b156ceec..31a70ed5aa 100644 --- a/tuf/signed/ed25519_test.go +++ b/tuf/signed/ed25519_test.go @@ -35,10 +35,6 @@ func TestGetKeys(t *testing.T) { require.Equal(t, tskey.Algorithm(), pubKey.Algorithm()) require.Equal(t, tskey.ID(), pubKey.ID()) - keyInfo, err := c.GetKeyInfo(tskey.ID()) - require.NoError(t, err) - require.Equal(t, c.keys[tskey.ID()].role, keyInfo.Role) - privKey, role, err := c.GetPrivateKey(tskey.ID()) require.NoError(t, err) require.Equal(t, data.CanonicalTimestampRole, role) diff --git a/tuf/signed/interface.go b/tuf/signed/interface.go index c53cbd9124..9f64a47e66 100644 --- a/tuf/signed/interface.go +++ b/tuf/signed/interface.go @@ -1,9 +1,6 @@ package signed -import ( - "github.com/docker/notary/trustmanager" - "github.com/docker/notary/tuf/data" -) +import "github.com/docker/notary/tuf/data" // KeyService provides management of keys locally. It will never // accept or provide private keys. Communication between the KeyService @@ -19,9 +16,6 @@ type KeyService interface { // GetKey retrieves the public key if present, otherwise it returns nil GetKey(keyID string) data.PublicKey - // GetKeyInfo retrieves the KeyInfo for a keyID - GetKeyInfo(keyID string) (trustmanager.KeyInfo, error) - // GetPrivateKey retrieves the private key and role if present and retrievable, // otherwise it returns nil and an error GetPrivateKey(keyID string) (data.PrivateKey, string, error) diff --git a/tuf/signed/sign_test.go b/tuf/signed/sign_test.go index b7bb851862..5ede9c9abe 100644 --- a/tuf/signed/sign_test.go +++ b/tuf/signed/sign_test.go @@ -60,10 +60,6 @@ func (mts *FailingCryptoService) GetKey(keyID string) data.PublicKey { return nil } -func (mts *FailingCryptoService) GetKeyInfo(keyID string) (trustmanager.KeyInfo, error) { - return trustmanager.KeyInfo{}, nil -} - func (mts *FailingCryptoService) GetPrivateKey(keyID string) (data.PrivateKey, string, error) { return nil, "", trustmanager.ErrKeyNotFound{KeyID: keyID} } @@ -92,13 +88,6 @@ func (mts *MockCryptoService) GetKey(keyID string) data.PublicKey { return nil } -func (mts *MockCryptoService) GetKeyInfo(keyID string) (trustmanager.KeyInfo, error) { - if keyID == mts.testKey.ID() { - return trustmanager.KeyInfo{Gun: "gun", Role: "testRole"}, nil - } - return trustmanager.KeyInfo{}, nil -} - func (mts *MockCryptoService) ListKeys(role string) []string { return []string{mts.testKey.ID()} }