Allow certificate to be provided to key rotation of root role

This will allow user to rotate a repository's root key to a pinned trust, make trust pinning more useful. - add `--rootcert` flag to key rotation - add `-y` flag to key rotate to allow auto-confirmation of rotating root keys (no user interaction required) - allow mismatched key-certificate pair to be provided. an example usage would be : The PR includes the following: `notary key rotate [GUN] root --key path/to/key.key --rootcert path/to/rootcert.pem` related issues: notaryproject#1144, notaryproject#1118, notaryproject#731 Signed-off-by: Chen Yuechuan-XJQW46 <[email protected]>
cyc115 · Oct 10, 2017 · 8a3a3a2 · 8a3a3a2
1 parent fab4c67
commit 8a3a3a2
Show file tree

Hide file tree

Showing 8 changed files with 462 additions and 64 deletions.
diff --git a/client/client.go b/client/client.go
@@ -1221,25 +1221,35 @@ func (r *repository) bootstrapClient(checkInitialized bool) (*tufClient, error)
 // 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 *repository) RotateKey(role data.RoleName, serverManagesKey bool, keyList []string) error {
+func (r *repository) rotateKey(role data.RoleName, serverManagesKey bool, keyList []string, certs []data.PublicKey) error {
+	if len(certs) > 0 && role != data.CanonicalRootRole {
+		return fmt.Errorf("rotate with certificate only support root role")
+	}
 	if err := checkRotationInput(role, serverManagesKey); err != nil {
 		return err
 	}
 
-	pubKeyList, err := r.pubKeyListForRotation(role, serverManagesKey, keyList)
+	pubKeys, err := r.pubKeyListForRotation(role, serverManagesKey, keyList, certs)
 	if err != nil {
 		return err
 	}
 
 	cl := changelist.NewMemChangelist()
-	if err := r.rootFileKeyChange(cl, role, changelist.ActionCreate, pubKeyList); err != nil {
+	if err := r.rootFileKeyChange(cl, role, changelist.ActionCreate, pubKeys); err != nil {
 		return err
 	}
 	return r.publish(cl)
 }
+func (r *repository) RotateKey(role data.RoleName, serverManagesKey bool, keyList []string) error {
+	return r.rotateKey(role, serverManagesKey, keyList, nil)
+}
 
 // Given a set of new keys to rotate to and a set of keys to drop, returns the list of current keys to use
-func (r *repository) pubKeyListForRotation(role data.RoleName, serverManaged bool, newKeys []string) (pubKeyList data.KeyList, err error) {
+func (r *repository) RotateKeyWithCert(role data.RoleName, serverManagesKey bool, keyList []string, certs []data.PublicKey) error {
+	return r.rotateKey(role, serverManagesKey, keyList, certs)
+}
+
+func (r *repository) pubKeyListForRotation(role data.RoleName, serverManaged bool, newKeys []string, pubKeys data.KeyList) (pubKeyList data.KeyList, err error) {
 	var pubKey data.PublicKey
 
 	// If server manages the key being rotated, request a rotation and return the new key
@@ -1255,32 +1265,68 @@ func (r *repository) pubKeyListForRotation(role data.RoleName, serverManaged boo
 	}
 
 	// If no new keys are passed in, we generate one
-	if len(newKeys) == 0 {
+	if len(newKeys) == 0 && len(pubKeys) == 0 {
 		pubKeyList = make(data.KeyList, 0, 1)
 		pubKey, err = r.GetCryptoService().Create(role, r.gun, data.ECDSAKey)
 		pubKeyList = append(pubKeyList, pubKey)
 	}
+
+	// If a list of keys to rotate to are provided, we add those
+	if len(newKeys) > 0 || len(pubKeys) > 0 {
+		pubKeyList, err = r.pubKeyListFromPrivAndPubKeys(newKeys, pubKeys)
+	}
 	if err != nil {
 		return nil, fmt.Errorf("unable to generate key: %s", err)
 	}
 
 	// If a list of keys to rotate to are provided, we add those
-	if len(newKeys) > 0 {
-		pubKeyList = make(data.KeyList, 0, len(newKeys))
-		for _, keyID := range newKeys {
-			pubKey = r.GetCryptoService().GetKey(keyID)
+	if pubKeyList, err = r.pubKeysToCerts(role, pubKeyList); err != nil {
+		return nil, err
+	}
+
+	return pubKeyList, nil
+}
+
+// pubKeyListFromPrivAndPubKeys returns a list of consolidated public keys obtained by
+// merging the pubKeys with the public keys of the private keys
+func (r *repository) pubKeyListFromPrivAndPubKeys(privKIDs []string, pubKeys data.KeyList) (pubKeyList data.KeyList, err error) {
+
+	// if pubKeys are not provided, generate pubKeyList from provided private keys
+	if len(pubKeys) == 0 {
+		pubKeyList = make(data.KeyList, 0, len(privKIDs))
+
+		for _, keyID := range privKIDs {
+			pubKey := r.GetCryptoService().GetKey(keyID)
 			if pubKey == nil {
 				return nil, fmt.Errorf("unable to find key: %s", keyID)
 			}
 			pubKeyList = append(pubKeyList, pubKey)
 		}
+		return pubKeyList, nil
 	}
 
 	// Convert to certs (for root keys)
-	if pubKeyList, err = r.pubKeysToCerts(role, pubKeyList); err != nil {
-		return nil, err
+	pubKeyIDs := make(map[string]bool)
+	// list pubKeys
+	for _, k := range pubKeys {
+		kid, err := utils.CanonicalKeyID(k)
+		fmt.Println("key id : " + kid)
+		if err != nil {
+			return nil, fmt.Errorf("public key is invalid, %v", err)
+		}
+		pubKeyIDs[kid] = true
+		pubKeyList = append(pubKeyList, k)
 	}
 
+	for _, id := range privKIDs {
+		if exist := pubKeyIDs[id]; !exist {
+			pubKey := r.GetCryptoService().GetKey(id)
+			if pubKey == nil {
+				return nil, fmt.Errorf("unable to find key: %s", id)
+			}
+			pubKeyList = append(pubKeyList, pubKey)
+		}
+	}
 	return pubKeyList, nil
 }
 
@@ -1291,16 +1337,19 @@ func (r *repository) pubKeysToCerts(role data.RoleName, pubKeyList data.KeyList)
 	}
 
 	for i, pubKey := range pubKeyList {
-		privKey, loadedRole, err := r.GetCryptoService().GetPrivateKey(pubKey.ID())
-		if err != nil {
-			return nil, err
-		}
-		if loadedRole != role {
-			return nil, fmt.Errorf("attempted to load root key but given %s key instead", loadedRole)
-		}
-		pubKey, err = rootCertKey(r.gun, privKey)
-		if err != nil {
-			return nil, err
+		// convert to x509 if pubKey is not already one
+		if !utils.IsX509Key(pubKey) {
+			privKey, loadedRole, err := r.GetCryptoService().GetPrivateKey(pubKey.ID())
+			if err != nil {
+				return nil, fmt.Errorf("error converting public key with id %v to private key: %v", pubKey.ID(), err)
+			}
+			if loadedRole != role {
+				return nil, fmt.Errorf("attempted to load root key but given %s key instead", loadedRole)
+			}
+			pubKey, err = rootCertKey(r.gun, privKey)
+			if err != nil {
+				return nil, err
+			}
 		}
 		pubKeyList[i] = pubKey
 	}

diff --git a/client/client_test.go b/client/client_test.go
@@ -2965,6 +2965,53 @@ func rootRoleCertID(t *testing.T, repo *repository) string {
 	return rootKeys[0]
 }
 
+func TestRotateKeyHelper(t *testing.T) {
+	ts := fullTestServer(t)
+	defer ts.Close()
+
+	var gun data.GUN
+	gun = "docker/test"
+	r, _ := initializeRepo(t, data.ECDSAKey, string(gun), ts.URL, false)
+	defer os.RemoveAll(r.baseDir)
+
+	r.Publish()
+
+	pubs := make([]data.PublicKey, 7)
+	for i := range pubs {
+		pubs[i], _ = r.GetCryptoService().Create(data.CanonicalRootRole, gun, data.ECDSAKey)
+	}
+
+	err := r.RotateKey(data.CanonicalRootRole, false, []string{pubs[0].ID()})
+	require.NoError(t, err, "rotate key with 1 key failed:")
+
+	err = r.RotateKey(data.CanonicalRootRole, false, nil)
+	require.NoError(t, err, "rotate with nil as key list failed")
+
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, []string{pubs[1].ID()}, []data.PublicKey{pubs[1]})
+	require.NoError(t, err, "rotate with matching key/cert pair failed")
+
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, nil, []data.PublicKey{pubs[2]})
+	require.NoError(t, err, "rotate with 1 key and no cert failed")
+
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, nil, nil)
+	require.NoError(t, err, "rotate with no key and no cert failed")
+
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, []string{}, nil)
+	require.NoError(t, err, "rotate with empty list of keys and certs")
+
+	_ = r.GetCryptoService().RemoveKey(pubs[4].ID()) // remove pubs[4] from crypto
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, []string{}, []data.PublicKey{pubs[4]})
+	require.Error(t, err, "cert only, key is not in keystore, expect to fail")
+
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, []string{pubs[3].ID()}, []data.PublicKey{pubs[1]})
+	require.NoError(t, err, "rotate with non-matching key cert pair should be allowed as importing multiple keys")
+
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, []string{"juiceshop"}, []data.PublicKey{pubs[1]})
+	require.Error(t, err, "rotate with cert and invalid key id")
+
+	err = r.RotateKeyWithCert(data.CanonicalRootRole, false, []string{pubs[5].ID(), pubs[6].ID()}, []data.PublicKey{pubs[6], pubs[5]})
+	require.NoError(t, err, "rotate with 2 unordered cert/key pairs failed")
+}
 func TestRotateRootKey(t *testing.T) {
 	ts := fullTestServer(t)
 	defer ts.Close()

diff --git a/client/helpers.go b/client/helpers.go
@@ -237,12 +237,12 @@ func getRemoteKey(role data.RoleName, remote store.RemoteStore) (data.PublicKey,
 func rotateRemoteKey(role data.RoleName, remote store.RemoteStore) (data.PublicKey, error) {
 	rawPubKey, err := remote.RotateKey(role)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to rotate remote key: %v", err)
 	}
 
 	pubKey, err := data.UnmarshalPublicKey(rawPubKey)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to rotate remote key: %v", err)
 	}
 
 	return pubKey, nil

diff --git a/client/interface.go b/client/interface.go
@@ -40,7 +40,7 @@ type Repository interface {
 
 	// Key Operations
 	RotateKey(role data.RoleName, serverManagesKey bool, keyList []string) error
-
+	RotateKeyWithCert(role data.RoleName, serverManagesKey bool, keyList []string, certs []data.PublicKey) error
 	GetCryptoService() signed.CryptoService
 	SetLegacyVersions(int)
 	GetGUN() data.GUN