From 4f8c1a8ef405004e2c72ff944143ef2ffd96a42d Mon Sep 17 00:00:00 2001 From: Ying Li Date: Wed, 18 Nov 2015 00:29:18 -0800 Subject: [PATCH] Server check that the root.json's timestamp key ID is valid. If the client sends a root.json with an invalid timestamp key ID, possibly because they are pushing an existing repo to a new server, then the server should reject the update. Signed-off-by: Ying Li --- server/handlers/validation.go | 46 ++++++-- server/handlers/validation_test.go | 176 ++++++++++++++++++++++++----- 2 files changed, 188 insertions(+), 34 deletions(-) diff --git a/server/handlers/validation.go b/server/handlers/validation.go index 86d0524895..1356a87d32 100644 --- a/server/handlers/validation.go +++ b/server/handlers/validation.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/data" @@ -94,10 +95,11 @@ func validateUpdate(gun string, updates []storage.MetaUpdate, store storage.Meta if rootUpdate, ok := roles[rootRole]; ok { // if root is present, validate its integrity, possibly // against a previous root - if root, err = validateRoot(gun, oldRootJSON, rootUpdate.Data); err != nil { + if root, err = validateRoot(gun, oldRootJSON, rootUpdate.Data, store); err != nil { logrus.Error("ErrBadRoot: ", err.Error()) return ErrBadRoot{msg: err.Error()} } + // setting root will update keys db if err = repo.SetRoot(root); err != nil { logrus.Error("ErrValidation: ", err.Error()) @@ -252,7 +254,9 @@ func hierarchyOK(roles map[string]storage.MetaUpdate) error { return nil } -func validateRoot(gun string, oldRoot, newRoot []byte) (*data.SignedRoot, error) { +func validateRoot(gun string, oldRoot, newRoot []byte, store storage.MetaStore) ( + *data.SignedRoot, error) { + var parsedOldRoot *data.SignedRoot parsedNewRoot := &data.SignedRoot{} @@ -271,23 +275,32 @@ func validateRoot(gun string, oldRoot, newRoot []byte) (*data.SignedRoot, error) if err != nil { return nil, err } - if err := checkRoot(parsedOldRoot, parsedNewRoot); err != nil { + + // Don't update if a timestamp key doesn't exist. + algo, keyBytes, err := store.GetTimestampKey(gun) + if err != nil || algo == "" || keyBytes == nil { + return nil, fmt.Errorf("no timestamp key for %s", gun) + } + timestampKey := data.NewPublicKey(algo, keyBytes) + + if err := checkRoot(parsedOldRoot, parsedNewRoot, timestampKey); err != nil { // TODO(david): how strict do we want to be here about old signatures // for rotations? Should the user have to provide a flag // which gets transmitted to force a root update without // correct old key signatures. return nil, err } + if !data.ValidTUFType(parsedNewRoot.Signed.Type, data.CanonicalRootRole) { return nil, fmt.Errorf("root has wrong type") } return parsedNewRoot, nil } -// checkRoot returns true if no rotation, or a valid -// rotation has taken place, and the threshold number of signatures -// are valid. -func checkRoot(oldRoot, newRoot *data.SignedRoot) error { +// checkRoot errors if an invalid valid rotation has taken place, if the +// threshold number of signatures is invalid valid, if there are an invalid +// number of roles and keys, or if the timestamp keys are invalid +func checkRoot(oldRoot, newRoot *data.SignedRoot, timestampKey data.PublicKey) error { rootRole := data.RoleName(data.CanonicalRootRole) targetsRole := data.RoleName(data.CanonicalTargetsRole) snapshotRole := data.RoleName(data.CanonicalSnapshotRole) @@ -360,6 +373,9 @@ func checkRoot(oldRoot, newRoot *data.SignedRoot) error { if err != nil { return err } + + var timestampKeyIDs []string + // at a minimum, check the 4 required roles are present for _, r := range []string{rootRole, targetsRole, snapshotRole, timestampRole} { role, ok := root.Signed.Roles[r] @@ -372,6 +388,20 @@ func checkRoot(oldRoot, newRoot *data.SignedRoot) error { if len(role.KeyIDs) < role.Threshold { return fmt.Errorf("%s role has insufficient number of keys", r) } + + if r == timestampRole { + timestampKeyIDs = role.KeyIDs + } } - return nil + + // ensure that at least one of the timestamp keys specified in the role + // actually exists + + for _, keyID := range timestampKeyIDs { + if timestampKey.ID() == keyID { + return nil + } + } + return fmt.Errorf("none of the following timestamp keys exist: %s", + strings.Join(timestampKeyIDs, ", ")) } diff --git a/server/handlers/validation_test.go b/server/handlers/validation_test.go index aa7312b392..d3dcfd232f 100644 --- a/server/handlers/validation_test.go +++ b/server/handlers/validation_test.go @@ -1,9 +1,13 @@ package handlers import ( + "crypto/rand" + "fmt" "testing" + "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/keys" "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/testutils" "github.com/stretchr/testify/assert" @@ -11,8 +15,25 @@ import ( "github.com/docker/notary/server/storage" ) +func copyTimestampKey(t *testing.T, fromKeyDB *keys.KeyDB, + toStore storage.MetaStore, gun string) { + + role := fromKeyDB.GetRole(data.CanonicalTimestampRole) + assert.NotNil(t, role, "No timestamp role in the KeyDB") + assert.Len(t, role.KeyIDs, 1, fmt.Sprintf( + "Expected 1 timestamp key in timestamp role, got %d", len(role.KeyIDs))) + + pubTimestampKey := fromKeyDB.GetKey(role.KeyIDs[0]) + assert.NotNil(t, pubTimestampKey, + "Timestamp key specified by KeyDB role not in KeysDB") + + err := toStore.SetTimestampKey(gun, pubTimestampKey.Algorithm(), + pubTimestampKey.Public()) + assert.NoError(t, err) +} + func TestValidateEmptyNew(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -42,13 +63,13 @@ func TestValidateEmptyNew(t *testing.T) { Data: timestamp, }, } - + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.NoError(t, err) } func TestValidateNoNewRoot(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -82,13 +103,13 @@ func TestValidateNoNewRoot(t *testing.T) { Data: timestamp, }, } - + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.NoError(t, err) } func TestValidateNoNewTargets(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -122,13 +143,13 @@ func TestValidateNoNewTargets(t *testing.T) { Data: timestamp, }, } - + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.NoError(t, err) } func TestValidateOnlySnapshot(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -160,13 +181,13 @@ func TestValidateOnlySnapshot(t *testing.T) { Data: snapshot, }, } - + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.NoError(t, err) } func TestValidateOldRoot(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -205,13 +226,13 @@ func TestValidateOldRoot(t *testing.T) { Data: timestamp, }, } - + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.NoError(t, err) } func TestValidateRootRotation(t *testing.T) { - _, repo, crypto := testutils.EmptyRepo() + kdb, repo, crypto := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -278,12 +299,13 @@ func TestValidateRootRotation(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.NoError(t, err) } func TestValidateNoRoot(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -309,13 +331,14 @@ func TestValidateNoRoot(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrValidation{}, err) } func TestValidateSnapshotMissing(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -336,16 +359,104 @@ func TestValidateSnapshotMissing(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadHierarchy{}, err) } +// If there is no timestamp key in the store, validation fails. This could +// happen if pushing an existing repository from one server to another that +// does not have the repo. +func TestValidateRootNoTimestampKey(t *testing.T) { + _, oldRepo, _ := testutils.EmptyRepo() + + r, tg, sn, ts, err := testutils.Sign(oldRepo) + assert.NoError(t, err) + root, targets, snapshot, _, err := testutils.Serialize(r, tg, sn, ts) + assert.NoError(t, err) + + store := storage.NewMemStorage() + updates := []storage.MetaUpdate{ + { + Role: "root", + Version: 1, + Data: root, + }, + { + Role: "targets", + Version: 1, + Data: targets, + }, + { + Role: "snapshot", + Version: 1, + Data: snapshot, + }, + } + + // sanity check - no timestamp keys for the GUN + _, _, err = store.GetTimestampKey("testGUN") + assert.Error(t, err) + assert.IsType(t, &storage.ErrNoKey{}, err) + + // do not copy the targets key to the storage, and try to update the root + err = validateUpdate("testGUN", updates, store) + assert.Error(t, err) + assert.IsType(t, ErrBadRoot{}, err) + + // there should still be no timestamp keys - one should not have been + // created + _, _, err = store.GetTimestampKey("testGUN") + assert.Error(t, err) +} + +// If the timestamp key in the store does not match the timestamp key in +// the root.json, validation fails. This could happen if pushing an existing +// repository from one server to another that had already initialized the same +// repo. +func TestValidateRootInvalidTimestampKey(t *testing.T) { + _, oldRepo, _ := testutils.EmptyRepo() + + r, tg, sn, ts, err := testutils.Sign(oldRepo) + assert.NoError(t, err) + root, targets, snapshot, _, err := testutils.Serialize(r, tg, sn, ts) + assert.NoError(t, err) + + store := storage.NewMemStorage() + updates := []storage.MetaUpdate{ + { + Role: "root", + Version: 1, + Data: root, + }, + { + Role: "targets", + Version: 1, + Data: targets, + }, + { + Role: "snapshot", + Version: 1, + Data: snapshot, + }, + } + + key, err := trustmanager.GenerateECDSAKey(rand.Reader) + assert.NoError(t, err) + err = store.SetTimestampKey("testGUN", key.Algorithm(), key.Public()) + assert.NoError(t, err) + + err = validateUpdate("testGUN", updates, store) + assert.Error(t, err) + assert.IsType(t, ErrBadRoot{}, err) +} + // ### Role missing negative tests ### // These tests remove a role from the Root file and // check for a ErrBadRoot func TestValidateRootRoleMissing(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() delete(repo.Root.Signed.Roles, "root") @@ -378,13 +489,14 @@ func TestValidateRootRoleMissing(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadRoot{}, err) } func TestValidateTargetsRoleMissing(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() delete(repo.Root.Signed.Roles, "targets") @@ -417,13 +529,14 @@ func TestValidateTargetsRoleMissing(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadRoot{}, err) } func TestValidateSnapshotRoleMissing(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() delete(repo.Root.Signed.Roles, "snapshot") @@ -456,6 +569,7 @@ func TestValidateSnapshotRoleMissing(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadRoot{}, err) @@ -465,7 +579,7 @@ func TestValidateSnapshotRoleMissing(t *testing.T) { // ### Signature missing negative tests ### func TestValidateRootSigMissing(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() delete(repo.Root.Signed.Roles, "snapshot") @@ -501,13 +615,14 @@ func TestValidateRootSigMissing(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadRoot{}, err) } func TestValidateTargetsSigMissing(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -541,13 +656,14 @@ func TestValidateTargetsSigMissing(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadTargets{}, err) } func TestValidateSnapshotSigMissing(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -581,6 +697,7 @@ func TestValidateSnapshotSigMissing(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadSnapshot{}, err) @@ -590,7 +707,7 @@ func TestValidateSnapshotSigMissing(t *testing.T) { // ### Corrupted metadata negative tests ### func TestValidateRootCorrupt(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -624,13 +741,14 @@ func TestValidateRootCorrupt(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadRoot{}, err) } func TestValidateTargetsCorrupt(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -664,13 +782,14 @@ func TestValidateTargetsCorrupt(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadTargets{}, err) } func TestValidateSnapshotCorrupt(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -704,6 +823,7 @@ func TestValidateSnapshotCorrupt(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadSnapshot{}, err) @@ -713,7 +833,7 @@ func TestValidateSnapshotCorrupt(t *testing.T) { // ### Snapshot size mismatch negative tests ### func TestValidateRootModifiedSize(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -751,13 +871,14 @@ func TestValidateRootModifiedSize(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadRoot{}, err) } func TestValidateTargetsModifiedSize(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -792,6 +913,7 @@ func TestValidateTargetsModifiedSize(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadSnapshot{}, err) @@ -801,7 +923,7 @@ func TestValidateTargetsModifiedSize(t *testing.T) { // ### Snapshot hash mismatch negative tests ### func TestValidateRootModifiedHash(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -840,13 +962,14 @@ func TestValidateRootModifiedHash(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadSnapshot{}, err) } func TestValidateTargetsModifiedHash(t *testing.T) { - _, repo, _ := testutils.EmptyRepo() + kdb, repo, _ := testutils.EmptyRepo() store := storage.NewMemStorage() r, tg, sn, ts, err := testutils.Sign(repo) @@ -885,6 +1008,7 @@ func TestValidateTargetsModifiedHash(t *testing.T) { }, } + copyTimestampKey(t, kdb, store, "testGUN") err = validateUpdate("testGUN", updates, store) assert.Error(t, err) assert.IsType(t, ErrBadSnapshot{}, err)