client: add support for rotating root keys

Signed-off-by: Evan Cordell <[email protected]>

buildscripts/testclient.py

@@ -248,9 +248,9 @@ def root_rotation_test(self, tempfile, tempdir):
         self.client.run(["list", self.repo_name], tempdir)
         with open(os.path.join(tempdir, "tuf", self.repo_name, "metadata", "root.json")) as root:
             root_json = json.load(root)
-            assert len(root_json["signed"]["keys"]) == old_root_num_keys + 1, (
+            assert len(root_json["signed"]["keys"]) == old_root_num_keys, (
                 "expected {0} base keys, but got {1}".format(
-                    old_root_num_keys + 1, len(root_json["signed"]["keys"])))
+                    old_root_num_keys, len(root_json["signed"]["keys"])))
 
             root_certs = root_json["signed"]["roles"]["root"]["keyids"]
 

client/client.go

@@ -804,7 +804,7 @@ func (r *NotaryRepository) Update(forWrite bool) error {
 		// notFound.Resource may include a checksum so when the role is root,
 		// it will be root or root.<checksum>. Therefore best we can
 		// do it match a "root." prefix
-		if notFound, ok := err.(store.ErrMetaNotFound); ok && strings.HasPrefix(notFound.Resource, data.CanonicalRootRole+".") {
+		if notFound, ok := err.(store.ErrMetaNotFound); ok && strings.Contains(notFound.Resource, data.CanonicalRootRole) {
 			return r.errRepositoryNotExist()
 		}
 		return err
@@ -823,8 +823,11 @@ func (r *NotaryRepository) Update(forWrite bool) error {
 // is initialized or not. If set to true, we will always attempt to download
 // and return an error if the remote repository errors.
 //
-// Populates a tuf.RepoBuilder with this root metadata (only use
-// TUFClient.Update to load the rest).
+// Populates a tuf.RepoBuilder with this root metadata. If the root metadata
+// downloaded is a newer version than what is on disk, then intermediate
+// versions will be downloaded and verified in order to rotate trusted keys
+// properly. Newer root metadata must always be signed with the previous
+// threshold and keys.
 //
 // Fails if the remote server is reachable and does not know the repo
 // (i.e. before the first r.Publish()), in which case the error is
@@ -903,8 +906,10 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*TUFClient, e
 	return NewTUFClient(oldBuilder, newBuilder, remote, r.fileStore), nil
 }
 
-// RotateKey removes all existing keys associated with the role, and either
-// creates and adds one new key or delegates managing the key to the server.
+// RotateKey removes all existing keys associated with the role. If no keys are
+// specified in keyList, then this creates and adds one new key or delegates
+// managing the key to the server. If key(s) are specified by keyList, then they are
+// used for signing the role.
 // These changes are staged in a changelist until publish is called.
 func (r *NotaryRepository) RotateKey(role string, serverManagesKey bool, keyList []string) error {
 	// We currently support remotely managing timestamp and snapshot keys
@@ -939,6 +944,11 @@ func (r *NotaryRepository) RotateKey(role string, serverManagesKey bool, keyList
 		pubKeyList = make(data.KeyList, 0, len(keyList))
 		for _, keyID := range keyList {
 			pubKey = r.CryptoService.GetKey(keyID)
+			if pubKey == nil {
+				errFmtMsg = "unable to find key: %s"
+				err = fmt.Errorf("no key with id %s could be found", keyID)
+				break
+			}
 			pubKeyList = append(pubKeyList, pubKey)
 		}
 	case !serverManagesKey && len(keyList) == 0:

client/client_update_test.go

@@ -66,14 +66,21 @@ func readOnlyServer(t *testing.T, cache store.MetadataStore, notFoundStatus int,
 	m := mux.NewRouter()
 	handler := func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
-		metaBytes, err := cache.GetSized(vars["role"], store.NoSizeLimit)
+		var role string
+		if vars["version"] != "" {
+			role = fmt.Sprintf("%s.%s", vars["version"], vars["role"])
+		} else {
+			role = vars["role"]
+		}
+		metaBytes, err := cache.GetSized(role, store.NoSizeLimit)
 		if _, ok := err.(store.ErrMetaNotFound); ok {
 			w.WriteHeader(notFoundStatus)
 		} else {
 			require.NoError(t, err)
 			w.Write(metaBytes)
 		}
 	}
+	m.HandleFunc(fmt.Sprintf("/v2/%s/_trust/tuf/{version:[0-9]+}.{role:.*}.json", gun), handler)
 	m.HandleFunc(fmt.Sprintf("/v2/%s/_trust/tuf/{role:.*}.{checksum:.*}.json", gun), handler)
 	m.HandleFunc(fmt.Sprintf("/v2/%s/_trust/tuf/{role:.*}.json", gun), handler)
 	return httptest.NewServer(m)
@@ -158,6 +165,10 @@ func TestUpdateSucceedsEvenIfCannotWriteExistingRepo(t *testing.T) {
 			require.NoError(t, err)
 
 			for r, expected := range serverMeta {
+				if r != data.CanonicalRootRole && strings.Contains(r, "root") {
+					// don't fetch versioned root roles here
+					continue
+				}
 				actual, err := repo.fileStore.GetSized(r, store.NoSizeLimit)
 				require.NoError(t, err, "problem getting repo metadata for %s", r)
 				if role == r {
@@ -1263,7 +1274,7 @@ func testUpdateRemoteCorruptValidChecksum(t *testing.T, opts updateOpts, expt sw
 		var expectedTypes []string
 		for _, expectErr := range expt.expectErrs {
 			expectedType := reflect.TypeOf(expectErr)
-			isExpectedType = isExpectedType || errType == expectedType
+			isExpectedType = isExpectedType || reflect.DeepEqual(errType, expectedType)
 			expectedTypes = append(expectedTypes, expectedType.String())
 		}
 		require.True(t, isExpectedType, "expected one of %v when %s: got %s",
@@ -1435,6 +1446,13 @@ func signSerializeAndUpdateRoot(t *testing.T, signedRoot data.SignedRoot,
 	require.NoError(t, serverSwizzler.UpdateTimestampHash())
 }
 
+func requireRootSignatures(t *testing.T, serverSwizzler *testutils.MetadataSwizzler, num int) {
+	updatedRootBytes, _ := serverSwizzler.MetadataCache.GetSized(data.CanonicalRootRole, -1)
+	updatedRoot := &data.SignedRoot{}
+	require.NoError(t, json.Unmarshal(updatedRootBytes, updatedRoot))
+	require.EqualValues(t, len(updatedRoot.Signatures), num)
+}
+
 // A valid root rotation only cares about the immediately previous old root keys,
 // whether or not there are old root roles, and cares that the role is satisfied
 // (for instance if the old role has 2 keys, either of which can sign, then it
@@ -1534,6 +1552,87 @@ func TestValidateRootRotationWithOldRole(t *testing.T) {
 	require.NoError(t, repo.Update(false))
 }
 
+// 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
+	_, 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))
+
+	// save the old role to prove that it is not needed for client updates
+	oldVersion := fmt.Sprintf("%v.%v", data.CanonicalRootRole, signedRoot.Signed.Version)
+	signedRoot.Signed.Roles[oldVersion] = &data.RootRole{
+		Threshold: 1,
+		KeyIDs:    signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs,
+	}
+
+	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()
+	}
+	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
+	// sign with the first key only
+	signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[0]})
+
+	// Load this root for the first time with 1 key
+	require.NoError(t, repo.Update(false))
+
+	// --- First root rotation: replace the first key with a different key
+	signedRoot.Signed.Version++
+	signedRoot.Signed.Keys[keyIDs[1]] = threeKeys[1]
+	signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[1]}
+
+	// --- If the current role is satisfied but the previous one is not, root rotation
+	// --- will fail.  Signing with just the second key will not satisfy the first role.
+	signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[1]})
+	require.Error(t, repo.Update(false))
+	requireRootSignatures(t, serverSwizzler, 1)
+
+	// --- If both the current and previous roles are satisfied, then the root rotation
+	// --- will succeed (signing with the first and second keys will satisfy both)
+	signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, threeKeys[:2])
+	require.NoError(t, repo.Update(false))
+	requireRootSignatures(t, serverSwizzler, 2)
+
+	// --- Second root rotation: replace the second key with a third
+	signedRoot.Signed.Version++
+	signedRoot.Signed.Keys[keyIDs[2]] = threeKeys[2]
+	signedRoot.Signed.Roles[data.CanonicalRootRole].KeyIDs = []string{keyIDs[2]}
+
+	// --- If the current role is satisfied but the previous one is not, root rotation
+	// --- will fail.  Signing with just the second key will not satisfy the first role.
+	signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, []data.PublicKey{threeKeys[2]})
+	require.Error(t, repo.Update(false))
+	requireRootSignatures(t, serverSwizzler, 1)
+
+	// --- If both the current and previous roles are satisfied, then the root rotation
+	// --- will succeed (signing with the second and third keys will satisfy both)
+	signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, threeKeys[1:])
+	require.NoError(t, repo.Update(false))
+	requireRootSignatures(t, serverSwizzler, 2)
+
+	// -- If signed with all previous roles, update will succeed
+	signSerializeAndUpdateRoot(t, signedRoot, serverSwizzler, threeKeys)
+	require.NoError(t, repo.Update(false))
+	requireRootSignatures(t, serverSwizzler, 3)
+}
+
 // 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.

client/tufclient.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"encoding/json"
+	"fmt"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/notary"
@@ -47,7 +48,7 @@ func (c *TUFClient) Update() (*tuf.Repo, *tuf.Repo, error) {
 
 		c.newBuilder = c.newBuilder.BootstrapNewBuilder()
 
-		if err := c.downloadRoot(); err != nil {
+		if err := c.updateRoot(); err != nil {
 			logrus.Debug("Client Update (Root):", err)
 			return nil, nil, err
 		}
@@ -78,25 +79,81 @@ func (c *TUFClient) update() error {
 	return nil
 }
 
-// downloadRoot is responsible for downloading the root.json
-func (c *TUFClient) downloadRoot() 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 {
 	role := data.CanonicalRootRole
-	consistentInfo := c.newBuilder.GetConsistentInfo(role)
 
-	// We can't read an exact size for the root metadata without risking getting stuck in the TUF update cycle
-	// since it's possible that downloading timestamp/snapshot metadata may fail due to a signature mismatch
-	if !consistentInfo.ChecksumKnown() {
-		logrus.Debugf("Loading root with no expected checksum")
+	// Get Current Root Version
+	currentRootConsistentInfo := c.oldBuilder.GetConsistentInfo(role)
+	currentRoot, _ := c.cache.GetSized(role, -1)
+	c.oldBuilder.Load(currentRootConsistentInfo.RoleName, currentRoot, 1, true)
+	currentVersion := c.oldBuilder.GetLoadedVersion(currentRootConsistentInfo.RoleName)
+
+	// Get New Root Version
+	raw, err := c.remote.GetSized(data.CanonicalRootRole, -1)
+	if err != nil {
+		logrus.Debugf("error downloading root: %s", err)
+		return err
+	}
+	signedRoot := &data.Signed{}
+	if err := json.Unmarshal(raw, signedRoot); err != nil {
+		return err
+	}
+	newestRoot, err := data.RootFromSigned(signedRoot)
+	if err != nil {
+		return err
+	}
+	newestVersion := newestRoot.Signed.Version
+	if newestVersion-currentVersion < 2 {
+		// We're only one version behind, just download the new root
+		if err := c.downloadRoot(); err != nil {
+			return err
+		}
+	} else {
+		if err := c.updateRootVersions(currentRoot, currentVersion, newestVersion); err != nil {
+			return err
+		}
 
-		// get the cached root, if it exists, just for version checking
-		cachedRoot, _ := c.cache.GetSized(role, -1)
-		// prefer to download a new root
-		_, remoteErr := c.tryLoadRemote(consistentInfo, cachedRoot)
-		return remoteErr
 	}
+	return nil
+}
 
-	_, err := c.tryLoadCacheThenRemote(consistentInfo)
-	return err
+// updateRootVersions updates the root from it's current version to a target, rotating keys
+// as they are found
+func (c *TUFClient) updateRootVersions(currentRoot []byte, fromVersion, toVersion int) error {
+	if fromVersion == toVersion {
+		logrus.Debugf("finished updating root files")
+		return nil
+	}
+	nextVersion := fromVersion + 1
+	skipCheckExpiryAndChecksum := !(nextVersion == toVersion)
+
+	logrus.Debugf("updating root from version %d to version %d, up to %d", fromVersion, nextVersion, toVersion)
+
+	var versionedRole string
+	if nextVersion == toVersion {
+		logrus.Debugf("updating root to %d by requesting latest", toVersion)
+		versionedRole = data.CanonicalRootRole
+	} else {
+		versionedRole = fmt.Sprintf("%d.%s", nextVersion, data.CanonicalRootRole)
+	}
+
+	raw, err := c.remote.GetSized(versionedRole, -1)
+	if err != nil {
+		logrus.Debugf("error downloading %s: %s", versionedRole, err)
+		return err
+	}
+	// If loading the most recent version, check expiry
+	if err := c.newBuilder.LoadOptions(data.CanonicalRootRole, raw, nextVersion, skipCheckExpiryAndChecksum, skipCheckExpiryAndChecksum, true); err != nil {
+		logrus.Debugf("downloaded %s is invalid: %s", versionedRole, err)
+		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 c.updateRootVersions(raw, nextVersion, toVersion)
 }
 
 // downloadTimestamp is responsible for downloading the timestamp.json
@@ -198,6 +255,27 @@ func (c TUFClient) getTargetsFile(role data.DelegationRole, ci tuf.ConsistentInf
 	return tgs.GetValidDelegations(role), nil
 }
 
+// downloadRoot is responsible for downloading the root.json
+func (c *TUFClient) downloadRoot() error {
+	role := data.CanonicalRootRole
+	consistentInfo := c.newBuilder.GetConsistentInfo(role)
+
+	// We can't read an exact size for the root metadata without risking getting stuck in the TUF update cycle
+	// since it's possible that downloading timestamp/snapshot metadata may fail due to a signature mismatch
+	if !consistentInfo.ChecksumKnown() {
+		logrus.Debugf("Loading root with no expected checksum")
+
+		// get the cached root, if it exists, just for version checking
+		cachedRoot, _ := c.cache.GetSized(role, -1)
+		// prefer to download a new root
+		_, remoteErr := c.tryLoadRemote(consistentInfo, cachedRoot)
+		return remoteErr
+	}
+
+	_, err := c.tryLoadCacheThenRemote(consistentInfo)
+	return err
+}
+
 func (c *TUFClient) tryLoadCacheThenRemote(consistentInfo tuf.ConsistentInfo) ([]byte, error) {
 	cachedTS, err := c.cache.GetSized(consistentInfo.RoleName, consistentInfo.Length())
 	if err != nil {