Error (and add tests for this) if the root in the server store is corrupt

Signed-off-by: Ying Li <ying.li@docker.com>

server/handlers/validation.go

@@ -55,7 +55,7 @@ func validateUpdate(cs signed.CryptoService, gun string, updates []storage.MetaU
 		// against a previous root
 		if root, err = validateRoot(gun, oldRootJSON, rootUpdate.Data); err != nil {
 			logrus.Error("ErrBadRoot: ", err.Error())
-			return nil, validation.ErrBadRoot{Msg: err.Error()}
+			return nil, err
 		}
 
 		// setting root will update keys db
@@ -71,7 +71,7 @@ func validateUpdate(cs signed.CryptoService, gun string, updates []storage.MetaU
 		}
 		parsedOldRoot := &data.SignedRoot{}
 		if err := json.Unmarshal(oldRootJSON, parsedOldRoot); err != nil {
-			return nil, validation.ErrValidation{Msg: "pre-existing root is corrupted and no root provided in update."}
+			return nil, fmt.Errorf("pre-existing root is corrupt")
 		}
 		if err = repo.SetRoot(parsedOldRoot); err != nil {
 			logrus.Error("ErrValidation: ", err.Error())
@@ -384,23 +384,23 @@ func validateRoot(gun string, oldRoot, newRoot []byte) (
 	parsedNewSigned := &data.Signed{}
 	err := json.Unmarshal(newRoot, parsedNewSigned)
 	if err != nil {
-		return nil, err
+		return nil, validation.ErrBadRoot{Msg: err.Error()}
 	}
 
 	// validates the structure of the root metadata
 	parsedNewRoot, err := data.RootFromSigned(parsedNewSigned)
 	if err != nil {
-		return nil, err
+		return nil, validation.ErrBadRoot{Msg: err.Error()}
 	}
 
 	newRootRole, _ := parsedNewRoot.BuildBaseRole(data.CanonicalRootRole)
 	if err != nil { // should never happen, since the root metadata has been validated
-		return nil, err
+		return nil, validation.ErrBadRoot{Msg: err.Error()}
 	}
 
 	newTimestampRole, err := parsedNewRoot.BuildBaseRole(data.CanonicalTimestampRole)
 	if err != nil { // should never happen, since the root metadata has been validated
-		return nil, err
+		return nil, validation.ErrBadRoot{Msg: err.Error()}
 	}
 	// According to the TUF spec, any role may have more than one signing
 	// key and require a threshold signature.  However, notary-server
@@ -417,7 +417,7 @@ func validateRoot(gun string, oldRoot, newRoot []byte) (
 	}
 
 	if err := signed.VerifyRoot(parsedNewSigned, newRootRole.Threshold, newRootRole.Keys); err != nil {
-		return nil, err
+		return nil, validation.ErrBadRoot{Msg: err.Error()}
 	}
 
 	return parsedNewRoot, nil
@@ -429,13 +429,13 @@ func checkAgainstOldRoot(oldRoot []byte, newRootRole data.BaseRole, newSigned *d
 	err := json.Unmarshal(oldRoot, parsedOldRoot)
 	if err != nil {
 		logrus.Warn("Old root could not be parsed, and cannot be used to check the new root.")
-		return nil
+		return err
 	}
 
 	oldRootRole, err := parsedOldRoot.BuildBaseRole(data.CanonicalRootRole)
 	if err != nil {
 		logrus.Warn("Old root does not have a valid root role, and cannot be used to check the new root.")
-		return nil
+		return err
 	}
 
 	// if the set of keys has changed between the old root and new root, then a root
@@ -453,8 +453,9 @@ func checkAgainstOldRoot(oldRoot []byte, newRootRole data.BaseRole, newSigned *d
 
 	if rotation {
 		if err := signed.VerifyRoot(newSigned, oldRootRole.Threshold, oldRootRole.Keys); err != nil {
-			return fmt.Errorf("rotation detected and new root was not signed with at least %d old keys",
+			return validation.ErrBadRoot{Msg: fmt.Sprintf(
-				oldRootRole.Threshold)
+				"rotation detected and new root was not signed with at least %d old keys",
+				oldRootRole.Threshold)}
 		}
 	}
 

server/handlers/validation_test.go

@@ -6,6 +6,7 @@ import (
 	"path"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/docker/notary/cryptoservice"
 	"github.com/docker/notary/passphrase"
@@ -27,22 +28,22 @@ type getFailStore struct {
 
 // GetCurrent returns the current metadata, or an error depending on whether
 // getFailStore is configured to return an error for this role
-func (f getFailStore) GetCurrent(gun, tufRole string) ([]byte, error) {
+func (f getFailStore) GetCurrent(gun, tufRole string) (*time.Time, []byte, error) {
 	err := f.errsToReturn[tufRole]
 	if err == nil {
 		return f.MetaStore.GetCurrent(gun, tufRole)
 	}
-	return nil, err
+	return nil, nil, err
 }
 
 // GetChecksum returns the metadata with this checksum, or an error depending on
 // whether getFailStore is configured to return an error for this role
-func (f getFailStore) GetChecksum(gun, tufRole, checksum string) ([]byte, error) {
+func (f getFailStore) GetChecksum(gun, tufRole, checksum string) (*time.Time, []byte, error) {
 	err := f.errsToReturn[tufRole]
 	if err == nil {
 		return f.MetaStore.GetChecksum(gun, tufRole, checksum)
 	}
-	return nil, err
+	return nil, nil, err
 }
 
 func copyKeys(t *testing.T, from signed.CryptoService, roles ...string) signed.CryptoService {
@@ -278,6 +279,81 @@ func TestValidateOldRoot(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestValidateOldRootCorrupt(t *testing.T) {
+	repo, cs, err := testutils.EmptyRepo("docker.com/notary")
+	assert.NoError(t, err)
+	store := storage.NewMemStorage()
+
+	r, tg, sn, ts, err := testutils.Sign(repo)
+	assert.NoError(t, err)
+	root, targets, snapshot, timestamp, err := getUpdates(r, tg, sn, ts)
+	assert.NoError(t, err)
+
+	badRoot := storage.MetaUpdate{
+		Version: root.Version,
+		Role:    root.Role,
+		Data:    root.Data[1:],
+	}
+	store.UpdateCurrent("testGUN", badRoot)
+	updates := []storage.MetaUpdate{root, targets, snapshot, timestamp}
+
+	serverCrypto := copyKeys(t, cs, data.CanonicalTimestampRole)
+	_, err = validateUpdate(serverCrypto, "testGUN", updates, store)
+	assert.Error(t, err)
+	assert.IsType(t, &json.SyntaxError{}, err)
+}
+
+func TestValidateOldRootCorruptRootRole(t *testing.T) {
+	repo, cs, err := testutils.EmptyRepo("docker.com/notary")
+	assert.NoError(t, err)
+	store := storage.NewMemStorage()
+
+	r, tg, sn, ts, err := testutils.Sign(repo)
+	assert.NoError(t, err)
+	root, targets, snapshot, timestamp, err := getUpdates(r, tg, sn, ts)
+	assert.NoError(t, err)
+
+	// so a valid root, but missing the root role
+	signedRoot, err := data.RootFromSigned(r)
+	assert.NoError(t, err)
+	delete(signedRoot.Signed.Roles, data.CanonicalRootRole)
+	badRootJSON, err := json.Marshal(signedRoot)
+	assert.NoError(t, err)
+	badRoot := storage.MetaUpdate{
+		Version: root.Version,
+		Role:    root.Role,
+		Data:    badRootJSON,
+	}
+	store.UpdateCurrent("testGUN", badRoot)
+	updates := []storage.MetaUpdate{root, targets, snapshot, timestamp}
+
+	serverCrypto := copyKeys(t, cs, data.CanonicalTimestampRole)
+	_, err = validateUpdate(serverCrypto, "testGUN", updates, store)
+	assert.Error(t, err)
+	assert.IsType(t, data.ErrInvalidRole{}, err)
+}
+
+func TestValidateRootGetCurrentRootBroken(t *testing.T) {
+	repo, cs, err := testutils.EmptyRepo("docker.com/notary")
+	assert.NoError(t, err)
+	store := getFailStore{
+		MetaStore:    storage.NewMemStorage(),
+		errsToReturn: map[string]error{data.CanonicalRootRole: data.ErrNoSuchRole{}},
+	}
+
+	r, tg, sn, ts, err := testutils.Sign(repo)
+	assert.NoError(t, err)
+	root, targets, snapshot, timestamp, err := getUpdates(r, tg, sn, ts)
+	assert.NoError(t, err)
+
+	updates := []storage.MetaUpdate{root, targets, snapshot, timestamp}
+
+	serverCrypto := copyKeys(t, cs, data.CanonicalTimestampRole)
+	_, err = validateUpdate(serverCrypto, "testGUN", updates, store)
+	assert.Error(t, err)
+	assert.IsType(t, data.ErrNoSuchRole{}, err)
+}
+
 func TestValidateRootRotation(t *testing.T) {
 	repo, crypto, err := testutils.EmptyRepo("docker.com/notary")
 	assert.NoError(t, err)
@@ -324,6 +400,48 @@ func TestValidateRootRotation(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestInvalidRootRotation(t *testing.T) {
+	repo, crypto, err := testutils.EmptyRepo("docker.com/notary")
+	assert.NoError(t, err)
+	store := storage.NewMemStorage()
+
+	r, tg, sn, ts, err := testutils.Sign(repo)
+	assert.NoError(t, err)
+	root, targets, snapshot, timestamp, err := getUpdates(r, tg, sn, ts)
+	assert.NoError(t, err)
+
+	store.UpdateCurrent("testGUN", root)
+
+	rootKey, err := crypto.Create("root", data.ED25519Key)
+	assert.NoError(t, err)
+	rootRole, err := data.NewRole("root", 1, []string{rootKey.ID()}, nil)
+	assert.NoError(t, err)
+
+	repo.Root.Signed.Roles["root"] = &rootRole.RootRole
+	repo.Root.Signed.Keys[rootKey.ID()] = rootKey
+
+	r, err = repo.SignRoot(data.DefaultExpires(data.CanonicalRootRole))
+	assert.NoError(t, err)
+	err = signed.Sign(crypto, r, rootKey)
+	assert.NoError(t, err)
+
+	rt, err := data.RootFromSigned(r)
+	assert.NoError(t, err)
+	repo.SetRoot(rt)
+
+	sn, err = repo.SignSnapshot(data.DefaultExpires(data.CanonicalSnapshotRole))
+	assert.NoError(t, err)
+	root, targets, snapshot, timestamp, err = getUpdates(r, tg, sn, ts)
+	assert.NoError(t, err)
+
+	updates := []storage.MetaUpdate{root, targets, snapshot, timestamp}
+
+	serverCrypto := copyKeys(t, crypto, data.CanonicalTimestampRole)
+	_, err = validateUpdate(serverCrypto, "testGUN", updates, store)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "new root was not signed with at least 1 old keys")
+}
+
 func TestValidateNoRoot(t *testing.T) {
 	repo, cs, err := testutils.EmptyRepo("docker.com/notary")
 	assert.NoError(t, err)