From 8a3a3a2014379986dca01780dd60cfccdffd00df Mon Sep 17 00:00:00 2001 From: Chen Yuechuan-XJQW46 Date: Tue, 10 Oct 2017 10:50:39 -0500 Subject: [PATCH] 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: #1144, #1118, #731 Signed-off-by: Chen Yuechuan-XJQW46 --- client/client.go | 91 ++++++++--- client/client_test.go | 47 ++++++ client/helpers.go | 4 +- client/interface.go | 2 +- cmd/notary/integration_test.go | 265 +++++++++++++++++++++++++++++++-- cmd/notary/keys.go | 88 ++++++++--- cmd/notary/tuf.go | 17 +-- tuf/utils/x509.go | 12 +- 8 files changed, 462 insertions(+), 64 deletions(-) diff --git a/client/client.go b/client/client.go index 81411d244..5046a1c9e 100644 --- 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 index f56d1fae1..5da62a5c2 100644 --- 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 index 186547eec..9297451e4 100644 --- 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 index ca09fb4eb..e6b8a7b20 100644 --- 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 diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index d11d389ca..6c6c6a3e6 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -257,7 +257,7 @@ i85wnaTwOgWv8n6q3tavmnIA/v2QqsTpmI+bhwrPNKQ= "--rootkey", privKeyFilename, "--rootcert", certFilename) require.NoError(t, err) - require.Contains(t, output, "Root key found") + require.Contains(t, output, "Root key found, using:") // === no rootkey specified: look up in keystore === // this requires the previous test to inject private key in keystore output, err = runCommand(t, tempDir, @@ -1420,17 +1420,17 @@ func getUniqueKeys(t *testing.T, tempDir string) ([]string, []string) { // List keys, parses the output, and asserts something about the number of root // keys and number of signing keys, as well as returning them. func assertNumKeys(t *testing.T, tempDir string, numRoot, numSigning int, - rootOnDisk bool) ([]string, []string) { + rootOnDisk bool, msg ...interface{}) ([]string, []string) { root, signing := getUniqueKeys(t, tempDir) - require.Len(t, root, numRoot) - require.Len(t, signing, numSigning) + require.Len(t, root, numRoot, msg...) + require.Len(t, signing, numSigning, msg...) for _, rootKeyID := range root { _, err := os.Stat(filepath.Join( tempDir, notary.PrivDir, rootKeyID+".key")) // os.IsExist checks to see if the error is because a file already // exists, and hence it isn't actually the right function to use here - require.Equal(t, rootOnDisk, !os.IsNotExist(err)) + require.Equal(t, rootOnDisk, !os.IsNotExist(err), msg...) // this function is declared is in the build-tagged setup files verifyRootKeyOnHardware(t, rootKeyID) @@ -1575,9 +1575,9 @@ func TestKeyRotation(t *testing.T) { require.NoError(t, err) badKeyFile.Close() - _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String(), "--key", "123") + _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String(), "--key", "123", "-y") require.Error(t, err) - _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String(), "--key", badKeyFile.Name()) + _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String(), "--key", badKeyFile.Name(), "-y") require.Error(t, err) // create encrypted root keys @@ -1598,16 +1598,16 @@ func TestKeyRotation(t *testing.T) { require.NoError(t, err) // rotate the root key - _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String(), "--key", encryptedPEMKeyFilename1, "--key", encryptedPEMKeyFilename2) + _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String(), "--key", encryptedPEMKeyFilename1, "--key", encryptedPEMKeyFilename2, "-y") require.NoError(t, err) // 3 root keys - 1 prev, 1 new assertNumKeys(t, tempDir, 3, 2, true) // rotate the root key again - _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String()) + _, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", data.CanonicalRootRole.String(), "-y") require.NoError(t, err) // 3 root keys, 2 prev, 1 new - assertNumKeys(t, tempDir, 3, 2, true) + assertNumKeys(t, tempDir, 4, 2, true) // publish using the new keys output := assertSuccessfullyPublish( @@ -1616,9 +1616,252 @@ func TestKeyRotation(t *testing.T) { require.True(t, strings.Contains(string(output), target)) } +func nonMatchingKeyPairFilenames(t *testing.T, dir string) (string, string) { + nonMatchingKeyStr := `-----BEGIN EC PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-256-CBC,fd6e6735232efbc1a851549d12b8203d +role: root + +Z6u+cAZOEmeoieyQHt6Lp8ZmLWPiyGXT0wTkfYMnGxZ+EX+6sBeu9CWgx+3kOCWQ +qXuLmBjJ4ZwL/lZejeLLefF7jILA0oDLJtNH1L0oP7H/i7DUtNv+7Jvnci986Rx0 +i85wnaTwOgWv8n6q3tavmnIA/v2QqsTpmI+bhwrPNKQ= +-----END EC PRIVATE KEY-----` + + privFn := filepath.Join(dir, "non-matching-priv.key") + require.NoError(t, ioutil.WriteFile(privFn, []byte(nonMatchingKeyStr), 0644)) + + _, pub := matchingKeyPairFilenames(t, dir) + return privFn, pub +} + +//matchingKeyPairFilenames Write a matching key pair to dir and return filename of private key and public key +func matchingKeyPairFilenames(t *testing.T, dir string) (privFn string, pubFn string) { + // key pairs + privStr := `-----BEGIN EC PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-256-CBC,c9ccb4ef1effa1a080030c9c36942e8e +role: root +gun: +path: root_keys/ab7fc77dba46f5d6264c80d6630727d17f8d285fb0eff0fd7c5620b44e6cd0fc +role: root + +3pCHAMGD2QJDr8BAojd01wa4nzhct0Brk6olIAoaL9yRfV5jRguidu1UaoA22Tan +9zOatIkxIgqkEP+P3+prIipbXJPbr9I9zVdWxhANSEhmQ95jmlk9syi/xeJT2oXB +6+u84t59l0mRpuAisdC9AGkw7Cz2T5U51lhyCWjLDqE= +-----END EC PRIVATE KEY-----` + + certStr := `-----BEGIN CERTIFICATE----- +MIIBWDCB/6ADAgECAhBKKoVsRNJdGsGh6tPWnE4rMAoGCCqGSM49BAMCMBMxETAP +BgNVBAMTCGRvY2tlci8qMB4XDTE3MDQyODIwMTczMFoXDTI3MDQyNjIwMTczMFow +EzERMA8GA1UEAxMIZG9ja2VyLyowWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQQ +6RhA8sX/kWedbPPFzNqOMI+AnWOQV+u0+FQfeNO+k/Uf0LBnKhHEPSwSBuuwLPon +w+nR0YTdv3lFaM7x9nOUozUwMzAOBgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYI +KwYBBQUHAwMwDAYDVR0TAQH/BAIwADAKBggqhkjOPQQDAgNIADBFAiA+eHPInhLJ +HgP8nha+UqdYgq8ZCOlhdGTJhSdHd4sCuQIhAPXqQeWhDLA3/Pf8B7G3ZwWpPbZ8 +adLwkjqoeEKMaAXf +-----END CERTIFICATE-----` + privFn = filepath.Join(dir, "priv.key") + pubFn = filepath.Join(dir, "pub.crt") + require.NoError(t, ioutil.WriteFile(privFn, []byte(privStr), 0644)) + require.NoError(t, ioutil.WriteFile(pubFn, []byte(certStr), 0644)) + return +} + +// genKeyPairFN creates a ECDSA private key and write the key (in PKCS8 format) and its corresponding public key (in x509 certificate format) to disk. +func genKeyPairFN(t *testing.T, dir, filePrefix string, gun data.GUN) (keyFN string, certFN string) { + keyFN = filepath.Join(dir, filePrefix+".key") + certFN = filepath.Join(dir, filePrefix+".crt") + + //generate key + priv, err := utils.GenerateECDSAKey(rand.Reader) + require.NoError(t, err) + + pemBytes, err := utils.ConvertPrivateKeyToPKCS8(priv, data.CanonicalRootRole, gun, "") + require.NoError(t, err) + + //generate cert + startTime := time.Now() + endTime := startTime.AddDate(0, 0, 1) + cert, err := cryptoservice.GenerateCertificate(priv, gun, startTime, endTime) + require.NoError(t, err) + + require.NoError(t, ioutil.WriteFile(keyFN, pemBytes, 0644)) + require.NoError(t, ioutil.WriteFile(certFN, utils.CertToPEM(cert), 0644)) + return +} + +func TestKeyRotationWithDuplicatedKeysAndCerts(t *testing.T) { + setUp(t) + + tempDir := tempDirWithConfig(t, `{}`) + defer os.RemoveAll(tempDir) + + tempfiles := make([]string, 2) + for i := 0; i < 2; i++ { + tempFile, err := ioutil.TempFile("", "targetfile") + require.NoError(t, err) + tempFile.Close() + tempfiles[i] = tempFile.Name() + defer os.Remove(tempFile.Name()) + } + + server := setupServer() + defer server.Close() + + gun := "docker/repoName" + target := "target" + + // privKey and privKey2 are the same in this case + privKey, cert := matchingKeyPairFilenames(t, tempDir) + privKey2, cert2 := matchingKeyPairFilenames(t, tempDir) + + // initialize a repo, should have signing keys and no new root key + _, err := runCommand(t, tempDir, "-s", server.URL, "init", gun) + require.NoError(t, err) + assertNumKeys(t, tempDir, 1, 2, true) + assertSuccessfullyPublish(t, tempDir, server.URL, gun, target, tempfiles[0]) + out, err := runCommand(t, tempDir, "-s", server.URL, "key", "rotate", gun, data.CanonicalRootRole.String(), "--rootcert", cert, "--rootcert", cert2, "--key", privKey, "--key", privKey2, "-y") + require.NoError(t, err, "should successfully rotate") + assertNumKeys(t, tempDir, 2, 2, true, out) + +} + +func TestRootKeyRotationWithCert(t *testing.T) { + setUp(t) + + tempDir := tempDirWithConfig(t, `{}`) + defer os.RemoveAll(tempDir) + + tempfiles := make([]string, 3) + for i := 0; i < 3; i++ { + tempFile, err := ioutil.TempFile("", "targetfile") + require.NoError(t, err) + tempFile.Close() + tempfiles[i] = tempFile.Name() + defer os.Remove(tempFile.Name()) + } + + server := setupServer() + defer server.Close() + + gun := "docker/repoName" + target := "target" + privKey, cert := matchingKeyPairFilenames(t, tempDir) + nonMatchingKey, cert2 := nonMatchingKeyPairFilenames(t, tempDir) + + // starts out with no keys + assertNumKeys(t, tempDir, 0, 0, true) + + // initialize a repo, should have signing keys and no new root key + out, err := runCommand(t, tempDir, "-s", server.URL, "init", gun) + require.NoError(t, err) + assertNumKeys(t, tempDir, 1, 2, true, out) + assertSuccessfullyPublish(t, tempDir, server.URL, gun, target, tempfiles[0]) + + // add another root key to key store for test case 6: rotate with --rootcert flag only + privKey2, _ := genKeyPairFN(t, tempDir, "credentials2", data.GUN(gun)) + _, err = runCommand(t, tempDir, "-s", server.URL, "key", "import", privKey2) + require.NoError(t, err, "importing key failed") + assertNumKeys(t, tempDir, 2, 2, true, out) + + // confirm 2 root keys and 2 signing keys are present. Do not make assumption where the root keys are located. + // this is because keys are either imported into yubikey or to disk if yubikey is not present + root, signing := getUniqueKeys(t, tempDir) + require.Len(t, root, 2, "unexpected number of root keys") + require.Len(t, signing, 2, "unexpected number of signing keys") + + // begin test + testCases := []struct { + name string + keyArgs []string + assertPublish bool + useTargetRole bool + targetName string + targetFile string + expectedRootKeys int + expectedSigningKeys int + reqErr bool + }{ + { + name: "1. test key rotation with 1 imported key and 1 matching certificate", + keyArgs: []string{"--key", privKey, "--rootcert", cert}, + assertPublish: true, + targetName: target + "2", + targetFile: tempfiles[1], + expectedRootKeys: 3, + expectedSigningKeys: 2, + }, + { + name: "2. rotate with the same key", + keyArgs: []string{"--key", privKey, "--rootcert", cert}, + expectedRootKeys: 3, + expectedSigningKeys: 2, + }, + { + name: "3. test key rotation with key and a non matching certificate", + keyArgs: []string{"--key", nonMatchingKey, "--rootcert", cert}, + expectedRootKeys: 4, + expectedSigningKeys: 2, + reqErr: false, + }, + { + name: "4. test key rotation with imported key and non matching number of certificates", + keyArgs: []string{"--key", nonMatchingKey, "--key", privKey, "--rootcert", cert}, + expectedRootKeys: 4, + expectedSigningKeys: 2, + reqErr: false, + }, + { + name: "5. test key rotation with target role", + keyArgs: []string{"--key", privKey, "--rootcert", cert}, + useTargetRole: true, + expectedRootKeys: 4, + expectedSigningKeys: 2, + reqErr: true, + }, + { + name: "6. test key rotation with a certificate corresponding to an unrecognized key", + keyArgs: []string{"--rootcert", cert2}, + expectedRootKeys: 4, + expectedSigningKeys: 2, + assertPublish: true, + targetName: target + "3", + targetFile: tempfiles[2], + }, + } + + for _, tc := range testCases { + role := data.CanonicalRootRole.String() + if tc.useTargetRole { + role = data.CanonicalTargetsRole.String() + } + + // generate runCommand arguments + rotateArgs := []string{"key", "rotate", gun, role, "-s", server.URL, "-y"} + rotateArgs = append(rotateArgs, tc.keyArgs...) + fmt.Printf("command arguments: %v", rotateArgs) + _, err := runCommand(t, tempDir, rotateArgs...) + + // switch between error and no error + if tc.reqErr { + require.Error(t, err, tc.name+": failed, error is expected but not obtained") + } else { + require.NoError(t, err, tc.name+" failed, error: %v", err) + } + + root, signing := getUniqueKeys(t, tempDir) + require.Len(t, root, tc.expectedRootKeys, "test case: "+tc.name) + require.Len(t, signing, tc.expectedSigningKeys, "test case: "+tc.name) + + if tc.assertPublish { + output := assertSuccessfullyPublish(t, tempDir, server.URL, gun, tc.targetName, tc.targetFile) + require.True(t, strings.Contains(string(output), tc.targetName)) + } + } +} + // Tests rotating non-root keys func TestKeyRotationNonRoot(t *testing.T) { - // -- setup -- setUp(t) tempDir := tempDirWithConfig(t, "{}") diff --git a/cmd/notary/keys.go b/cmd/notary/keys.go index e78d1a992..84b0ea481 100644 --- a/cmd/notary/keys.go +++ b/cmd/notary/keys.go @@ -71,7 +71,7 @@ var cmdKeyPasswdTemplate = usageTemplate{ var cmdKeyImportTemplate = usageTemplate{ Use: "import pemfile [ pemfile ... ]", Short: "Imports all keys from all provided .pem files", - Long: "Imports all keys from all provided .pem files by reading each PEM block from the file and writing that block to a unique object in the local keystore. A Yubikey will be the prefferred import location for root keys if present.", + Long: "Imports all keys from all provided .pem files by reading each PEM block from the file and writing that block to a unique object in the local keystore. A Yubikey will be the preferred import location for root keys if present.", } var cmdKeyExportTemplate = usageTemplate{ @@ -91,6 +91,8 @@ type keyCommander struct { rotateKeyFiles []string legacyVersions int input io.Reader + rootCert []string + autoConfirm bool importRole string generateRole string @@ -130,6 +132,8 @@ func (k *keyCommander) GetCommand() *cobra.Command { nil, "New key(s) to rotate to. If not specified, one will be generated.", ) + cmdRotateKey.Flags().StringSliceVar(&k.rootCert, "rootcert", nil, "Public key(s) to rotate to, potentially corresponding to private key(s)") + cmdRotateKey.Flags().BoolVarP(&k.autoConfirm, "yes", "y", false, "Auto-confirm rotating root role") cmd.AddCommand(cmdRotateKey) cmdKeysImport := cmdKeyImportTemplate.ToCommand(k.importKeys) @@ -317,21 +321,7 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error { return err } - var keyList []string - - for _, keyFile := range k.rotateKeyFiles { - privKey, err := readKey(rotateKeyRole, keyFile, k.getRetriever()) - if err != nil { - return err - } - err = nRepo.GetCryptoService().AddKey(rotateKeyRole, gun, privKey) - if err != nil { - return fmt.Errorf("Error importing key: %v", err) - } - keyList = append(keyList, privKey.ID()) - } - - if rotateKeyRole == data.CanonicalRootRole { + if !k.autoConfirm && rotateKeyRole == data.CanonicalRootRole { cmd.Print("Warning: you are about to rotate your root key.\n\n" + "You must use your old key to sign this root rotation.\n" + "Are you sure you want to proceed? (yes/no) ") @@ -341,14 +331,78 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error { return nil } } + + retriver := k.getRetriever() + keyList, err := preparePrivateKeyListForRotation(k.rotateKeyFiles, gun, rotateKeyRole, &nRepo, &retriver) + if err != nil { + return err + } + + certList, err := preparePubKeyListForRotation(rotateKeyRole, k.rootCert) + if err != nil { + return err + } + nRepo.SetLegacyVersions(k.legacyVersions) - if err := nRepo.RotateKey(rotateKeyRole, k.rotateKeyServerManaged, keyList); err != nil { + if err := nRepo.RotateKeyWithCert(rotateKeyRole, k.rotateKeyServerManaged, keyList, certList); err != nil { return err } cmd.Printf("Successfully rotated %s key for repository %s\n", rotateKeyRole, gun) return nil } +// given a list of path to private keys, import them to the crypto service and return a list of key IDs +func preparePrivateKeyListForRotation(rotateKeyFiles []string, gun data.GUN, role data.RoleName, + repo *notaryclient.Repository, retriver *notary.PassRetriever) (keyList []string, err error) { + + crypto := (*repo).GetCryptoService() + for _, keyFile := range rotateKeyFiles { + privKey, err := readKey(role, keyFile, *retriver) + if err != nil { + return nil, err + } + err = crypto.AddKey(role, gun, privKey) + if err != nil { + return nil, fmt.Errorf("Error importing key: %v", err) + } + keyList = append(keyList, privKey.ID()) + } + + return +} + +// given a list of path to certificates, return a list of public keys +func preparePubKeyListForRotation(role data.RoleName, pubKeys []string) (certList data.KeyList, err error) { + if role == data.CanonicalRootRole { + for _, c := range pubKeys { + crt, err := importRootCert(c) + if err != nil { + return nil, err + } + certList = append(certList, crt[0]) + } + } + return +} + +// checkRootCert validates the cli flags argument. +// cases result in no err: +// 1. 0 key 0 cert +// 2. n keys m cert +// 3. n keys 0 cert +// 4. n cert (stored key) +func checkRootCert(k *keyCommander, rotateKeyRole data.RoleName) error { + if len(k.rootCert) != 0 { + //role is not "root" then error + if rotateKeyRole != data.CanonicalRootRole { + return fmt.Errorf("error: importing certificate is only supported for root role") + } else if len(k.rotateKeyFiles) != 0 && len(k.rootCert) < len(k.rotateKeyFiles) { + return fmt.Errorf("error: each key must have a matching certificate but received %v keys and %v certificates", len(k.rotateKeyFiles), len(k.rootCert)) + } + } + return nil +} + func removeKeyInteractively(keyStores []trustmanager.KeyStore, keyID string, in io.Reader, out io.Writer) error { diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 2b2ab0978..e8faf1227 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -410,8 +410,8 @@ func (t *tufCommander) tufDeleteGUN(cmd *cobra.Command, args []string) error { // importRootKey imports the root key from path then adds the key to repo // returns key ids func importRootKey(cmd *cobra.Command, rootKey string, nRepo notaryclient.Repository, retriever notary.PassRetriever) ([]string, error) { - var rootKeyList []string + rootKeyList := nRepo.GetCryptoService().ListKeys(data.CanonicalRootRole) if rootKey != "" { privKey, err := readKey(data.CanonicalRootRole, rootKey, retriever) if err != nil { @@ -423,8 +423,6 @@ func importRootKey(cmd *cobra.Command, rootKey string, nRepo notaryclient.Reposi return nil, fmt.Errorf("Error importing key: %v", err) } rootKeyList = []string{privKey.ID()} - } else { - rootKeyList = nRepo.GetCryptoService().ListKeys(data.CanonicalRootRole) } if len(rootKeyList) > 0 { @@ -444,10 +442,6 @@ func importRootKey(cmd *cobra.Command, rootKey string, nRepo notaryclient.Reposi func importRootCert(certFilePath string) ([]data.PublicKey, error) { publicKeys := make([]data.PublicKey, 0, 1) - if certFilePath == "" { - return publicKeys, nil - } - // read certificate from file certPEM, err := ioutil.ReadFile(certFilePath) if err != nil { @@ -491,9 +485,12 @@ func (t *tufCommander) tufInit(cmd *cobra.Command, args []string) error { return err } - rootCerts, err := importRootCert(t.rootCert) - if err != nil { - return err + var rootCerts []data.PublicKey + if t.rootCert != "" { + rootCerts, err = importRootCert(t.rootCert) + if err != nil { + return err + } } // if key is not defined but cert is, then clear the key to to allow key to be searched in keystore diff --git a/tuf/utils/x509.go b/tuf/utils/x509.go index 2a6cf9777..81dccb452 100644 --- a/tuf/utils/x509.go +++ b/tuf/utils/x509.go @@ -29,11 +29,19 @@ func CanonicalKeyID(k data.PublicKey) (string, error) { if k == nil { return "", errors.New("public key is nil") } + if IsX509Key(k) { + return X509PublicKeyID(k) + } + return k.ID(), nil +} + +// IsX509Key returns true if k is a x509 RSA/ECDSA TUF key, otherwise return false +func IsX509Key(k data.PublicKey) bool { switch k.Algorithm() { case data.ECDSAx509Key, data.RSAx509Key: - return X509PublicKeyID(k) + return true default: - return k.ID(), nil + return false } }