diff --git a/client/client.go b/client/client.go index af5e5985aa..c498a62ddb 100644 --- a/client/client.go +++ b/client/client.go @@ -628,21 +628,32 @@ func (r *NotaryRepository) bootstrapClient() (*tufclient.Client, error) { ), nil } -// RotateKeys removes all existing keys associated with role and adds -// the keys specified by keyIDs to the role. These changes are staged -// in a changelist until publish is called. -func (r *NotaryRepository) RotateKeys() error { - for _, role := range []string{"targets", "snapshot"} { - key, err := r.CryptoService.Create(role, data.ECDSAKey) - if err != nil { - return err - } - err = r.rootFileKeyChange(role, changelist.ActionCreate, key) - if err != nil { - return err - } +// 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. +// These changes are staged in a changelist until publish is called. +func (r *NotaryRepository) RotateKey(role string, serverManagesKey bool) error { + if role == data.CanonicalRootRole || role == data.CanonicalTimestampRole { + return fmt.Errorf( + "notary does not currently support rotating the %s key", role) } - return nil + if serverManagesKey && role == data.CanonicalTargetsRole { + return ErrInvalidRemoteRole{Role: data.CanonicalTargetsRole} + } + + var ( + pubKey data.PublicKey + err error + ) + if serverManagesKey { + pubKey, err = getRemoteKey(r.baseURL, r.gun, role, r.roundTrip) + } else { + pubKey, err = r.CryptoService.Create(role, data.ECDSAKey) + } + if err != nil { + return err + } + + return r.rootFileKeyChange(role, changelist.ActionCreate, pubKey) } func (r *NotaryRepository) rootFileKeyChange(role, action string, key data.PublicKey) error { diff --git a/client/client_test.go b/client/client_test.go index 3c095db5a4..9bbba350be 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1020,73 +1020,94 @@ func TestPublishSnapshotLocalKeysCreatedFirst(t *testing.T) { assert.False(t, requestMade) } +// Rotate invalid roles, or attempt to delegate target signing to the server +func TestRotateKeyInvalidRole(t *testing.T) { + tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-") + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + defer os.RemoveAll(tempBaseDir) + + gun := "docker.com/notary" + ts, _, _ := simpleTestServer(t) + defer ts.Close() + + repo, _ := initializeRepo(t, data.ECDSAKey, tempBaseDir, gun, ts.URL, false) + + // the equivalent of: (root, true), (root, false), (timestamp, true), + // (timestamp, false), (targets, true) + for role := range data.ValidRoles { + if role == data.CanonicalSnapshotRole { + continue + } + for _, serverManagesKey := range []bool{true, false} { + if role == data.CanonicalTargetsRole && !serverManagesKey { + continue + } + err = repo.RotateKey(role, serverManagesKey) + assert.Error(t, err, + "Rotating a %s key with server-managing the key as %v should fail", + role, serverManagesKey) + } + } +} + // Rotates the keys. After the rotation, downloading the latest metadata // and assert that the keys have changed -func assertRotationSuccessful(t *testing.T, repo *NotaryRepository) { - // capture targets + snapshot key IDs - targetsKeyIDs := repo.tufRepo.Root.Signed.Roles["targets"].KeyIDs - snapshotKeyIDs := repo.tufRepo.Root.Signed.Roles["snapshot"].KeyIDs - assert.Len(t, targetsKeyIDs, 1) - assert.Len(t, snapshotKeyIDs, 1) +func assertRotationSuccessful(t *testing.T, repo *NotaryRepository, + keysToRotate map[string]bool) { + + oldKeyIDs := make(map[string][]string) + for role := range keysToRotate { + keyIDs := repo.tufRepo.Root.Signed.Roles[role].KeyIDs + oldKeyIDs[role] = keyIDs + } // Do rotation - repo.RotateKeys() + for role, serverManaged := range keysToRotate { + assert.NoError(t, repo.RotateKey(role, serverManaged)) + } // Publish err := repo.Publish() assert.NoError(t, err) - // Get root.json. Check targets + snapshot keys have changed - // and that they match those found in the changelist. + // Get root.json. Check keys have changed. _, err = repo.GetTargetByName("latest") // force a pull assert.NoError(t, err) - newTargetsKeyIDs := repo.tufRepo.Root.Signed.Roles["targets"].KeyIDs - newSnapshotKeyIDs := repo.tufRepo.Root.Signed.Roles["snapshot"].KeyIDs - assert.Len(t, newTargetsKeyIDs, 1) - assert.Len(t, newSnapshotKeyIDs, 1) - assert.NotEqual(t, targetsKeyIDs[0], newTargetsKeyIDs[0]) - assert.NotEqual(t, snapshotKeyIDs[0], newSnapshotKeyIDs[0]) + + for role, isRemoteKey := range keysToRotate { + keyIDs := repo.tufRepo.Root.Signed.Roles[role].KeyIDs + assert.Len(t, keyIDs, 1) + + // the new key is not the same as any of the old keys, and the + // old keys have been removed not just from the TUF file, but + // from the cryptoservice + for _, oldKeyID := range oldKeyIDs[role] { + assert.NotEqual(t, oldKeyID, keyIDs[0]) + _, _, err := repo.CryptoService.GetPrivateKey(oldKeyID) + assert.Error(t, err) + } + + // the new key is present in the cryptoservice, or not present if remote + key, _, err := repo.CryptoService.GetPrivateKey(keyIDs[0]) + if isRemoteKey { + assert.Error(t, err) + assert.Nil(t, key) + } else { + assert.NoError(t, err) + assert.NotNil(t, key) + } + } // Confirm changelist dir empty after publishing changes changes := getChanges(t, repo) assert.Len(t, changes, 0, "wrong number of changelist files found") } -// Initialize a repo -// Publish some content (so that the server has a root.json) -// Rotate keys -// Download the latest metadata and assert that the keys have changed. -func TestRotateAfterPublish(t *testing.T) { - // Temporary directory where test files will be created - tempBaseDir, err := ioutil.TempDir("", "notary-test-") - defer os.RemoveAll(tempBaseDir) - - assert.NoError(t, err, "failed to create a temporary directory: %s", err) - - gun := "docker.com/notary" - - ts := fullTestServer(t) - defer ts.Close() - - repo, _ := initializeRepo(t, data.ECDSAKey, tempBaseDir, gun, ts.URL, false) - - // Adding a target will allow us to confirm the repository is still valid after - // rotating the keys. - addTarget(t, repo, "latest", "../fixtures/intermediate-ca.crt") - - // Publish - err = repo.Publish() - assert.NoError(t, err) - - repo.GetTargetByName("latest") // force a pull - assertRotationSuccessful(t, repo) -} - // Initialize repo to have the server sign snapshots (remote snapshot key) // Without downloading a server-signed snapshot file, rotate keys so that // snapshots are locally signed (local snapshot key) // Assert that we can publish. -func TestPublishAfterChangedFromRemoteKeyToLocalKey(t *testing.T) { +func TestRotateBeforePublishFromRemoteKeyToLocalKey(t *testing.T) { // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") defer os.RemoveAll(tempBaseDir) @@ -1101,12 +1122,74 @@ func TestPublishAfterChangedFromRemoteKeyToLocalKey(t *testing.T) { // Adding a target will allow us to confirm the repository is still valid // after rotating the keys. addTarget(t, repo, "latest", "../fixtures/intermediate-ca.crt") - assertRotationSuccessful(t, repo) + assertRotationSuccessful(t, repo, map[string]bool{ + data.CanonicalTargetsRole: false, + data.CanonicalSnapshotRole: false}) +} + +// Initialize a repo, locally signed snapshots +// Publish some content (so that the server has a root.json), and download root.json +// Rotate keys +// Download the latest metadata and assert that the keys have changed. +func TestRotateKeyAfterPublishNoServerManagementChange(t *testing.T) { + // rotate a single target key + testRotateKeySuccess(t, false, map[string]bool{data.CanonicalTargetsRole: false}) + testRotateKeySuccess(t, false, map[string]bool{data.CanonicalSnapshotRole: false}) + // rotate two at once before publishing + testRotateKeySuccess(t, false, map[string]bool{ + data.CanonicalSnapshotRole: false, + data.CanonicalTargetsRole: false}) +} + +// Tests rotating keys when there's a change from locally managed keys to +// remotely managed keys and vice versa +// Before rotating, publish some content (so that the server has a root.json), +// and download root.json +func TestRotateKeyAfterPublishServerManagementChange(t *testing.T) { + // delegate snapshot key management to the server + testRotateKeySuccess(t, false, map[string]bool{ + data.CanonicalSnapshotRole: true, + data.CanonicalTargetsRole: false, + }) + // reclaim snapshot key management from the server + testRotateKeySuccess(t, true, map[string]bool{ + data.CanonicalSnapshotRole: false, + data.CanonicalTargetsRole: false, + }) +} + +func testRotateKeySuccess(t *testing.T, serverManagesSnapshotInit bool, + keysToRotate map[string]bool) { + + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + defer os.RemoveAll(tempBaseDir) + + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + + gun := "docker.com/notary" + ts := fullTestServer(t) + defer ts.Close() + + repo, _ := initializeRepo(t, data.ECDSAKey, tempBaseDir, gun, ts.URL, + serverManagesSnapshotInit) + + // Adding a target will allow us to confirm the repository is still valid after + // rotating the keys. + addTarget(t, repo, "latest", "../fixtures/intermediate-ca.crt") + + // Publish + err = repo.Publish() + assert.NoError(t, err) + + // Get root.json and capture targets + snapshot key IDs + repo.GetTargetByName("latest") // force a pull + assertRotationSuccessful(t, repo, keysToRotate) } // If there is no local cache, notary operations return the remote error code func TestRemoteServerUnavailableNoLocalCache(t *testing.T) { - tempBaseDir, err := ioutil.TempDir("", "notary-test-") + tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-") assert.NoError(t, err, "failed to create a temporary directory: %s", err) defer os.RemoveAll(tempBaseDir) diff --git a/cmd/notary/keys.go b/cmd/notary/keys.go index b028a606ca..ddaf53cdb1 100644 --- a/cmd/notary/keys.go +++ b/cmd/notary/keys.go @@ -5,6 +5,7 @@ import ( "bufio" "fmt" "io" + "net/http" "os" "path/filepath" "strconv" @@ -17,112 +18,161 @@ import ( "github.com/docker/notary/tuf/data" "github.com/spf13/cobra" + "github.com/spf13/viper" ) -func init() { - cmdKey.AddCommand(cmdKeyList) - cmdKey.AddCommand(cmdKeyGenerateRootKey) - - cmdKeysBackup.Flags().StringVarP(&keysExportGUN, "gun", "g", "", "Globally Unique Name to export keys for") - cmdKey.AddCommand(cmdKeysBackup) - cmdKey.AddCommand(cmdKeyExportRoot) - cmdKeyExportRoot.Flags().BoolVarP(&keysExportRootChangePassphrase, "change-passphrase", "p", false, "Set a new passphrase for the key being exported") - cmdKey.AddCommand(cmdKeysRestore) - cmdKey.AddCommand(cmdKeyImportRoot) - cmdKey.AddCommand(cmdRotateKey) - cmdKey.AddCommand(cmdKeyRemove) +type usageTemplate struct { + Use string + Short string + Long string } -var cmdKey = &cobra.Command{ +type cobraRunE func(cmd *cobra.Command, args []string) error + +func (u usageTemplate) ToCommand(run cobraRunE) *cobra.Command { + c := cobra.Command{ + Use: u.Use, + Short: u.Short, + Long: u.Long, + } + if run != nil { + // newer versions of cobra support a run function that returns an error, + // but in the meantime, this should help ease the transition + c.Run = func(cmd *cobra.Command, args []string) { + err := run(cmd, args) + if err != nil { + cmd.Usage() + fatalf(err.Error()) + } + } + } + return &c +} + +var cmdKeyTemplate = usageTemplate{ Use: "key", Short: "Operates on keys.", Long: `Operations on private keys.`, } -var cmdKeyList = &cobra.Command{ +var cmdKeyListTemplate = usageTemplate{ Use: "list", Short: "Lists keys.", Long: "Lists all keys known to notary.", - Run: keysList, } -var cmdRotateKey = &cobra.Command{ +var cmdRotateKeyTemplate = usageTemplate{ Use: "rotate [ GUN ]", - Short: "Rotate all the signing (non-root) keys for the given Globally Unique Name.", - Long: "Removes all old signing (non-root) keys for the given Globally Unique Name, and generates new ones. This only makes local changes - please use then `notary publish` to push the key rotation changes to the remote server.", - Run: keysRotate, + Short: "Rotate the signing (non-root) keys for the given Globally Unique Name.", + Long: "Removes all the old signing (non-root) keys for the given Globally Unique Name, and generates new ones. This only makes local changes - please use then `notary publish` to push the key rotation changes to the remote server.", } -var cmdKeyGenerateRootKey = &cobra.Command{ +var cmdKeyGenerateRootKeyTemplate = usageTemplate{ Use: "generate [ algorithm ]", Short: "Generates a new root key with a given algorithm.", Long: "Generates a new root key with a given algorithm. If hardware key storage (e.g. a Yubikey) is available, the key will be stored both on hardware and on disk (so that it can be backed up). Please make sure to back up and then remove this on-key disk immediately afterwards.", - Run: keysGenerateRootKey, } -var keysExportGUN string - -var cmdKeysBackup = &cobra.Command{ +var cmdKeysBackupTemplate = usageTemplate{ Use: "backup [ zipfilename ]", Short: "Backs up all your on-disk keys to a ZIP file.", Long: "Backs up all of your accessible of keys. The keys are reencrypted with a new passphrase. The output is a ZIP file. If the --gun option is passed, only signing keys and no root keys will be backed up. Does not work on keys that are only in hardware (e.g. Yubikeys).", - Run: keysBackup, } -var keysExportRootChangePassphrase bool - -var cmdKeyExportRoot = &cobra.Command{ +var cmdKeyExportRootTemplate = usageTemplate{ Use: "export [ keyID ] [ pemfilename ]", Short: "Export a root key on disk to a PEM file.", Long: "Exports a single root key on disk, without reencrypting. The output is a PEM file. Does not work on keys that are only in hardware (e.g. Yubikeys).", - Run: keysExportRoot, } -var cmdKeysRestore = &cobra.Command{ +var cmdKeysRestoreTemplate = usageTemplate{ Use: "restore [ zipfilename ]", Short: "Restore multiple keys from a ZIP file.", Long: "Restores one or more keys from a ZIP file. If hardware key storage (e.g. a Yubikey) is available, root keys will be imported into the hardware, but not backed up to disk in the same location as the other, non-root keys.", - Run: keysRestore, } -var cmdKeyImportRoot = &cobra.Command{ +var cmdKeyImportRootTemplate = usageTemplate{ Use: "import [ pemfilename ]", Short: "Imports a root key from a PEM file.", Long: "Imports a single root key from a PEM file. If a hardware key storage (e.g. Yubikey) is available, the root key will be imported into the hardware but not backed up on disk again.", - Run: keysImportRoot, } -var cmdKeyRemove = &cobra.Command{ +var cmdKeyRemoveTemplate = usageTemplate{ Use: "remove [ keyID ]", Short: "Removes the key with the given keyID.", Long: "Removes the key with the given keyID. If the key is stored in more than one location, you will be asked which one to remove.", - Run: keyRemove, } -func keysList(cmd *cobra.Command, args []string) { +type keyCommander struct { + // these need to be set + configGetter func() *viper.Viper + retriever passphrase.Retriever + + // these are for command line parsing - no need to set + keysExportRootChangePassphrase bool + keysExportGUN string + rotateKeyRole string + rotateKeyServerManaged bool +} + +func (k *keyCommander) GetCommand() *cobra.Command { + cmd := cmdKeyTemplate.ToCommand(nil) + cmd.AddCommand(cmdKeyListTemplate.ToCommand(k.keysList)) + cmd.AddCommand(cmdKeyGenerateRootKeyTemplate.ToCommand(k.keysGenerateRootKey)) + cmd.AddCommand(cmdKeysRestoreTemplate.ToCommand(k.keysRestore)) + cmd.AddCommand(cmdKeyImportRootTemplate.ToCommand(k.keysImportRoot)) + cmd.AddCommand(cmdKeyRemoveTemplate.ToCommand(k.keyRemove)) + + cmdKeysBackup := cmdKeysBackupTemplate.ToCommand(k.keysBackup) + cmdKeysBackup.Flags().StringVarP( + &k.keysExportGUN, "gun", "g", "", "Globally Unique Name to export keys for") + cmd.AddCommand(cmdKeysBackup) + + cmdKeyExportRoot := cmdKeyExportRootTemplate.ToCommand(k.keysExportRoot) + cmdKeyExportRoot.Flags().BoolVarP( + &k.keysExportRootChangePassphrase, "change-passphrase", "p", false, + "Set a new passphrase for the key being exported") + cmd.AddCommand(cmdKeyExportRoot) + + cmdRotateKey := cmdRotateKeyTemplate.ToCommand(k.keysRotate) + cmdRotateKey.Flags().BoolVarP(&k.rotateKeyServerManaged, "server-managed", "r", + false, "Signing and key management will be handled by the remote server. "+ + "(no key will be generated or stored locally) "+ + "Can only be used in conjunction with --key-type.") + cmdRotateKey.Flags().StringVarP(&k.rotateKeyRole, "key-type", "t", "", + `Key type to rotate. Supported values: "targets", "snapshot". `+ + `If not provided, both targets and snapshot keys will be rotated, `+ + `and the new keys will be locally generated and stored.`) + cmd.AddCommand(cmdRotateKey) + + return cmd +} + +func (k *keyCommander) keysList(cmd *cobra.Command, args []string) error { if len(args) > 0 { - cmd.Usage() - os.Exit(1) + return fmt.Errorf("") } - parseConfig() + config := k.configGetter() + ks, err := k.getKeyStores(config, true) + if err != nil { + return err + } - stores := getKeyStores(cmd, mainViper.GetString("trust_dir"), retriever, true) cmd.Println("") - prettyPrintKeys(stores, cmd.Out()) + prettyPrintKeys(ks, cmd.Out()) cmd.Println("") + return nil } -func keysGenerateRootKey(cmd *cobra.Command, args []string) { +func (k *keyCommander) keysGenerateRootKey(cmd *cobra.Command, args []string) error { // We require one or no arguments (since we have a default value), but if the // user passes in more than one argument, we error out. if len(args) > 1 { - cmd.Usage() - fatalf("Please provide only one Algorithm as an argument to generate (rsa, ecdsa)") + return fmt.Errorf( + "Please provide only one Algorithm as an argument to generate (rsa, ecdsa)") } - parseConfig() - // If no param is given to generate, generates an ecdsa key by default algorithm := data.ECDSAKey @@ -137,49 +187,50 @@ func keysGenerateRootKey(cmd *cobra.Command, args []string) { } if !allowedCiphers[strings.ToLower(algorithm)] { - fatalf("Algorithm not allowed, possible values are: RSA, ECDSA") + return fmt.Errorf("Algorithm not allowed, possible values are: RSA, ECDSA") } - parseConfig() - - cs := cryptoservice.NewCryptoService( - "", - getKeyStores(cmd, mainViper.GetString("trust_dir"), retriever, true)..., - ) + config := k.configGetter() + ks, err := k.getKeyStores(config, true) + if err != nil { + return err + } + cs := cryptoservice.NewCryptoService("", ks...) pubKey, err := cs.Create(data.CanonicalRootRole, algorithm) if err != nil { - fatalf("Failed to create a new root key: %v", err) + return fmt.Errorf("Failed to create a new root key: %v", err) } cmd.Printf("Generated new %s root key with keyID: %s\n", algorithm, pubKey.ID()) + return nil } // keysBackup exports a collection of keys to a ZIP file -func keysBackup(cmd *cobra.Command, args []string) { +func (k *keyCommander) keysBackup(cmd *cobra.Command, args []string) error { if len(args) < 1 { - cmd.Usage() - fatalf("Must specify output filename for export") + return fmt.Errorf("Must specify output filename for export") } - parseConfig() + config := k.configGetter() + ks, err := k.getKeyStores(config, false) + if err != nil { + return err + } exportFilename := args[0] - cs := cryptoservice.NewCryptoService( - "", - getKeyStores(cmd, mainViper.GetString("trust_dir"), retriever, false)..., - ) + cs := cryptoservice.NewCryptoService("", ks...) exportFile, err := os.Create(exportFilename) if err != nil { - fatalf("Error creating output file: %v", err) + return fmt.Errorf("Error creating output file: %v", err) } // Must use a different passphrase retriever to avoid caching the // unlocking passphrase and reusing that. exportRetriever := getRetriever() - if keysExportGUN != "" { - err = cs.ExportKeysByGUN(exportFile, keysExportGUN, exportRetriever) + if k.keysExportGUN != "" { + err = cs.ExportKeysByGUN(exportFile, k.keysExportGUN, exportRetriever) } else { err = cs.ExportAllKeys(exportFile, exportRetriever) } @@ -188,38 +239,36 @@ func keysBackup(cmd *cobra.Command, args []string) { if err != nil { os.Remove(exportFilename) - fatalf("Error exporting keys: %v", err) + return fmt.Errorf("Error exporting keys: %v", err) } + return nil } // keysExportRoot exports a root key by ID to a PEM file -func keysExportRoot(cmd *cobra.Command, args []string) { +func (k *keyCommander) keysExportRoot(cmd *cobra.Command, args []string) error { if len(args) < 2 { - cmd.Usage() - fatalf("Must specify key ID and output filename for export") + return fmt.Errorf("Must specify key ID and output filename for export") } - parseConfig() - keyID := args[0] exportFilename := args[1] if len(keyID) != idSize { - fatalf("Please specify a valid root key ID") + return fmt.Errorf("Please specify a valid root key ID") } - parseConfig() - - cs := cryptoservice.NewCryptoService( - "", - getKeyStores(cmd, mainViper.GetString("trust_dir"), retriever, false)..., - ) + config := k.configGetter() + ks, err := k.getKeyStores(config, true) + if err != nil { + return err + } + cs := cryptoservice.NewCryptoService("", ks...) exportFile, err := os.Create(exportFilename) if err != nil { - fatalf("Error creating output file: %v", err) + return fmt.Errorf("Error creating output file: %v", err) } - if keysExportRootChangePassphrase { + if k.keysExportRootChangePassphrase { // Must use a different passphrase retriever to avoid caching the // unlocking passphrase and reusing that. exportRetriever := getRetriever() @@ -230,83 +279,112 @@ func keysExportRoot(cmd *cobra.Command, args []string) { exportFile.Close() if err != nil { os.Remove(exportFilename) - fatalf("Error exporting root key: %v", err) + return fmt.Errorf("Error exporting root key: %v", err) } + return nil } // keysRestore imports keys from a ZIP file -func keysRestore(cmd *cobra.Command, args []string) { +func (k *keyCommander) keysRestore(cmd *cobra.Command, args []string) error { if len(args) < 1 { - cmd.Usage() - fatalf("Must specify input filename for import") + return fmt.Errorf("Must specify input filename for import") } importFilename := args[0] - parseConfig() - - cs := cryptoservice.NewCryptoService( - "", - getKeyStores(cmd, mainViper.GetString("trust_dir"), retriever, true)..., - ) + config := k.configGetter() + ks, err := k.getKeyStores(config, true) + if err != nil { + return err + } + cs := cryptoservice.NewCryptoService("", ks...) zipReader, err := zip.OpenReader(importFilename) if err != nil { - fatalf("Opening file for import: %v", err) + return fmt.Errorf("Opening file for import: %v", err) } defer zipReader.Close() err = cs.ImportKeysZip(zipReader.Reader) if err != nil { - fatalf("Error importing keys: %v", err) + return fmt.Errorf("Error importing keys: %v", err) } + return nil } // keysImportRoot imports a root key from a PEM file -func keysImportRoot(cmd *cobra.Command, args []string) { +func (k *keyCommander) keysImportRoot(cmd *cobra.Command, args []string) error { if len(args) != 1 { - cmd.Usage() - fatalf("Must specify input filename for import") + return fmt.Errorf("Must specify input filename for import") } - parseConfig() - - cs := cryptoservice.NewCryptoService( - "", - getKeyStores(cmd, mainViper.GetString("trust_dir"), retriever, true)..., - ) + config := k.configGetter() + ks, err := k.getKeyStores(config, true) + if err != nil { + return err + } + cs := cryptoservice.NewCryptoService("", ks...) importFilename := args[0] importFile, err := os.Open(importFilename) if err != nil { - fatalf("Opening file for import: %v", err) + return fmt.Errorf("Opening file for import: %v", err) } defer importFile.Close() err = cs.ImportRootKey(importFile) if err != nil { - fatalf("Error importing root key: %v", err) + return fmt.Errorf("Error importing root key: %v", err) } + return nil } -func keysRotate(cmd *cobra.Command, args []string) { +func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error { if len(args) < 1 { - cmd.Usage() - fatalf("Must specify a GUN and target") + return fmt.Errorf("Must specify a GUN") } - parseConfig() + rotateKeyRole := strings.ToLower(k.rotateKeyRole) + + var rolesToRotate []string + switch rotateKeyRole { + case "": + rolesToRotate = []string{data.CanonicalSnapshotRole, data.CanonicalTargetsRole} + case data.CanonicalSnapshotRole: + rolesToRotate = []string{data.CanonicalSnapshotRole} + case data.CanonicalTargetsRole: + rolesToRotate = []string{data.CanonicalTargetsRole} + default: + return fmt.Errorf("key rotation not supported for %s keys", k.rotateKeyRole) + } + if k.rotateKeyServerManaged && rotateKeyRole != data.CanonicalSnapshotRole { + return fmt.Errorf( + "remote signing/key management is only supported for the snapshot key") + } + + config := k.configGetter() gun := args[0] - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, remoteTrustServer, nil, retriever) + var rt http.RoundTripper + if k.rotateKeyServerManaged { + // this does not actually push the changes, just creates the keys, but + // it creates a key remotely so it needs a transport + rt = getTransport(config, gun, true) + } + nRepo, err := notaryclient.NewNotaryRepository( + config.GetString("trust_dir"), gun, getRemoteTrustServer(config), + rt, k.retriever) if err != nil { - fatalf(err.Error()) + return err } - if err := nRepo.RotateKeys(); err != nil { - fatalf(err.Error()) + for _, role := range rolesToRotate { + if err := nRepo.RotateKey(role, k.rotateKeyServerManaged); err != nil { + return err + } } + return nil } func removeKeyInteractively(keyStores []trustmanager.KeyStore, keyID string, @@ -382,40 +460,43 @@ func removeKeyInteractively(keyStores []trustmanager.KeyStore, keyID string, } // keyRemove deletes a private key based on ID -func keyRemove(cmd *cobra.Command, args []string) { +func (k *keyCommander) keyRemove(cmd *cobra.Command, args []string) error { if len(args) < 1 { - cmd.Usage() - fatalf("must specify the key ID of the key to remove") + return fmt.Errorf("must specify the key ID of the key to remove") } - parseConfig() + config := k.configGetter() + ks, err := k.getKeyStores(config, true) + if err != nil { + return err + } keyID := args[0] // This is an invalid ID if len(keyID) != idSize { - fatalf("invalid key ID provided: %s", keyID) + return fmt.Errorf("invalid key ID provided: %s", keyID) } - stores := getKeyStores(cmd, mainViper.GetString("trust_dir"), retriever, true) cmd.Println("") - err := removeKeyInteractively(stores, keyID, os.Stdin, cmd.Out()) + err = removeKeyInteractively(ks, keyID, os.Stdin, + cmd.Out()) cmd.Println("") - if err != nil { - fatalf(err.Error()) - } + return err } -func getKeyStores(cmd *cobra.Command, directory string, - ret passphrase.Retriever, withHardware bool) []trustmanager.KeyStore { +func (k *keyCommander) getKeyStores( + config *viper.Viper, withHardware bool) ([]trustmanager.KeyStore, error) { - fileKeyStore, err := trustmanager.NewKeyFileStore(directory, ret) + directory := config.GetString("trust_dir") + fileKeyStore, err := trustmanager.NewKeyFileStore(directory, k.retriever) if err != nil { - fatalf("Failed to create private key store in directory: %s", directory) + return nil, fmt.Errorf( + "Failed to create private key store in directory: %s", directory) } ks := []trustmanager.KeyStore{fileKeyStore} if withHardware { - yubiStore, err := getYubiKeyStore(fileKeyStore, ret) + yubiStore, err := getYubiKeyStore(fileKeyStore, k.retriever) if err == nil && yubiStore != nil { // Note that the order is important, since we want to prioritize // the yubikey store @@ -423,5 +504,5 @@ func getKeyStores(cmd *cobra.Command, directory string, } } - return ks + return ks, nil } diff --git a/cmd/notary/keys_test.go b/cmd/notary/keys_test.go index 1b791aa9a1..111778cce0 100644 --- a/cmd/notary/keys_test.go +++ b/cmd/notary/keys_test.go @@ -3,12 +3,21 @@ package main import ( "bytes" "crypto/rand" + "fmt" "io/ioutil" + "net/http" + "net/http/httptest" + "os" "strings" "testing" + "github.com/docker/notary/client" "github.com/docker/notary/passphrase" "github.com/docker/notary/trustmanager" + "github.com/docker/notary/tuf/data" + "github.com/jfrazelle/go/canonical/json" + "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" ) @@ -216,3 +225,179 @@ func TestRemoveMultikeysRemoveOnlyChosenKey(t *testing.T) { } } } + +// Non-roles, root, and timestamp can't be rotated +func TestRotateKeyInvalidRoles(t *testing.T) { + invalids := []string{ + data.CanonicalRootRole, + data.CanonicalTimestampRole, + "notevenARole", + } + for _, role := range invalids { + for _, serverManaged := range []bool{true, false} { + k := &keyCommander{ + configGetter: viper.New, + retriever: passphrase.ConstantRetriever("pass"), + rotateKeyRole: role, + rotateKeyServerManaged: serverManaged, + } + err := k.keysRotate(&cobra.Command{}, []string{"gun"}) + assert.Error(t, err) + assert.Contains(t, err.Error(), + fmt.Sprintf("key rotation not supported for %s keys", role)) + } + } +} + +// Cannot rotate a targets key and require that the server manage it +func TestRotateKeyTargetCannotBeServerManaged(t *testing.T) { + k := &keyCommander{ + configGetter: viper.New, + retriever: passphrase.ConstantRetriever("pass"), + rotateKeyRole: data.CanonicalTargetsRole, + rotateKeyServerManaged: true, + } + err := k.keysRotate(&cobra.Command{}, []string{"gun"}) + assert.Error(t, err) + assert.Contains(t, err.Error(), + "remote signing/key management is only supported for the snapshot key") +} + +// rotate key must be provided with a gun +func TestRotateKeyNoGUN(t *testing.T) { + k := &keyCommander{ + configGetter: viper.New, + retriever: passphrase.ConstantRetriever("pass"), + rotateKeyRole: data.CanonicalTargetsRole, + } + err := k.keysRotate(&cobra.Command{}, []string{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Must specify a GUN") +} + +// initialize a repo with keys, so they can be rotated +func setUpRepo(t *testing.T, tempBaseDir, gun string, ret passphrase.Retriever) ( + *httptest.Server, map[string]string) { + + // server that always returns 200 (and a key) + key, err := trustmanager.GenerateECDSAKey(rand.Reader) + assert.NoError(t, err) + pubKey := data.PublicKeyFromPrivate(key) + jsonBytes, err := json.MarshalCanonical(&pubKey) + assert.NoError(t, err) + keyJSON := string(jsonBytes) + ts := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, keyJSON) + })) + + repo, err := client.NewNotaryRepository( + tempBaseDir, gun, ts.URL, http.DefaultTransport, ret) + assert.NoError(t, err, "error creating repo: %s", err) + + rootPubKey, err := repo.CryptoService.Create("root", data.ECDSAKey) + assert.NoError(t, err, "error generating root key: %s", err) + + err = repo.Initialize(rootPubKey.ID()) + assert.NoError(t, err) + + return ts, repo.CryptoService.ListAllKeys() +} + +// The command line uses NotaryRepository's RotateKey - this is just testing +// that the correct config variables are passed for the client to request a key +// from the remote server. +func TestRotateKeyRemoteServerManagesKey(t *testing.T) { + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-") + defer os.RemoveAll(tempBaseDir) + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + gun := "docker.com/notary" + + ret := passphrase.ConstantRetriever("pass") + + ts, initialKeys := setUpRepo(t, tempBaseDir, gun, ret) + defer ts.Close() + + k := &keyCommander{ + configGetter: func() *viper.Viper { + v := viper.New() + v.SetDefault("trust_dir", tempBaseDir) + v.SetDefault("remote_server.url", ts.URL) + return v + }, + retriever: ret, + rotateKeyRole: data.CanonicalSnapshotRole, + rotateKeyServerManaged: true, + } + err = k.keysRotate(&cobra.Command{}, []string{gun}) + assert.NoError(t, err) + + repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, nil, ret) + assert.NoError(t, err, "error creating repo: %s", err) + + cl, err := repo.GetChangelist() + assert.NoError(t, err, "unable to get changelist: %v", err) + assert.Len(t, cl.List(), 1) + // no keys have been created, since a remote key was specified + assert.Equal(t, initialKeys, repo.CryptoService.ListAllKeys()) +} + +// The command line uses NotaryRepository's RotateKey - this is just testing +// that the correct config variables are passed for the client to rotate +// both the targets and snapshot key, and create them locally +func TestRotateKeyBothKeys(t *testing.T) { + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-") + defer os.RemoveAll(tempBaseDir) + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + gun := "docker.com/notary" + + ret := passphrase.ConstantRetriever("pass") + + ts, initialKeys := setUpRepo(t, tempBaseDir, gun, ret) + // we won't need this anymore since we are creating keys locally + ts.Close() + + k := &keyCommander{ + configGetter: func() *viper.Viper { + v := viper.New() + v.SetDefault("trust_dir", tempBaseDir) + // won't need a remote server URL, since we are creating local keys + return v + }, + retriever: ret, + } + err = k.keysRotate(&cobra.Command{}, []string{gun}) + assert.NoError(t, err) + + repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, nil, ret) + assert.NoError(t, err, "error creating repo: %s", err) + + cl, err := repo.GetChangelist() + assert.NoError(t, err, "unable to get changelist: %v", err) + assert.Len(t, cl.List(), 2) + + // two new keys have been created, and the old keys should still be there + newKeys := repo.CryptoService.ListAllKeys() + for keyID, role := range initialKeys { + r, ok := newKeys[keyID] + assert.True(t, ok, "original key %s missing", keyID) + assert.Equal(t, role, r) + delete(newKeys, keyID) + } + // there should be 2 keys left + assert.Len(t, newKeys, 2) + // one for each role + var targetsFound, snapshotFound bool + for _, role := range newKeys { + switch role { + case data.CanonicalTargetsRole: + targetsFound = true + case data.CanonicalSnapshotRole: + snapshotFound = true + } + } + assert.True(t, targetsFound, "targets key was not created") + assert.True(t, snapshotFound, "snapshot key was not created") +} diff --git a/cmd/notary/main.go b/cmd/notary/main.go index 64afff1470..efd9eb97cc 100644 --- a/cmd/notary/main.go +++ b/cmd/notary/main.go @@ -37,7 +37,7 @@ func init() { retriever = getPassphraseRetriever() } -func parseConfig() { +func parseConfig() *viper.Viper { if verbose { logrus.SetLevel(logrus.DebugLevel) logrus.SetOutput(os.Stderr) @@ -94,6 +94,8 @@ func parseConfig() { mainViper.Set("trust_dir", expandedTrustDir) } logrus.Debugf("Using the following trust directory: %s", mainViper.GetString("trust_dir")) + + return mainViper } func setupCommand(notaryCmd *cobra.Command) { @@ -113,7 +115,12 @@ func setupCommand(notaryCmd *cobra.Command) { notaryCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Verbose output") notaryCmd.PersistentFlags().StringVarP(&remoteTrustServer, "server", "s", "", "Remote trust server location") - notaryCmd.AddCommand(cmdKey) + cmdKeyGenerator := &keyCommander{ + configGetter: parseConfig, + retriever: retriever, + } + + notaryCmd.AddCommand(cmdKeyGenerator.GetCommand()) notaryCmd.AddCommand(cmdCert) notaryCmd.AddCommand(cmdTufInit) notaryCmd.AddCommand(cmdTufList) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 4f59076f00..e67fed333f 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -23,6 +23,7 @@ import ( "github.com/docker/notary/tuf/data" "github.com/docker/notary/utils" "github.com/spf13/cobra" + "github.com/spf13/viper" ) var cmdTufList = &cobra.Command{ @@ -94,7 +95,7 @@ func tufAdd(cmd *cobra.Command, args []string) { // no online operations are performed by add so the transport argument // should be nil - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), nil, retriever) + nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), nil, retriever) if err != nil { fatalf(err.Error()) } @@ -121,7 +122,7 @@ func tufInit(cmd *cobra.Command, args []string) { parseConfig() gun := args[0] - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), getTransport(gun, false), retriever) + nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), getTransport(mainViper, gun, false), retriever) if err != nil { fatalf(err.Error()) } @@ -157,7 +158,7 @@ func tufList(cmd *cobra.Command, args []string) { parseConfig() gun := args[0] - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), getTransport(gun, true), retriever) + nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), getTransport(mainViper, gun, true), retriever) if err != nil { fatalf(err.Error()) } @@ -181,7 +182,7 @@ func tufLookup(cmd *cobra.Command, args []string) { gun := args[0] targetName := args[1] - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), getTransport(gun, true), retriever) + nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), getTransport(mainViper, gun, true), retriever) if err != nil { fatalf(err.Error()) } @@ -203,7 +204,7 @@ func tufStatus(cmd *cobra.Command, args []string) { parseConfig() gun := args[0] - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), nil, retriever) + nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), nil, retriever) if err != nil { fatalf(err.Error()) } @@ -237,7 +238,7 @@ func tufPublish(cmd *cobra.Command, args []string) { cmd.Println("Pushing changes to", gun) - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), getTransport(gun, false), retriever) + nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), getTransport(mainViper, gun, false), retriever) if err != nil { fatalf(err.Error()) } @@ -260,7 +261,7 @@ func tufRemove(cmd *cobra.Command, args []string) { // no online operation are performed by remove so the transport argument // should be nil. - repo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), nil, retriever) + repo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), nil, retriever) if err != nil { fatalf(err.Error()) } @@ -288,7 +289,7 @@ func verify(cmd *cobra.Command, args []string) { gun := args[0] targetName := args[1] - nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(), getTransport(gun, true), retriever) + nRepo, err := notaryclient.NewNotaryRepository(mainViper.GetString("trust_dir"), gun, getRemoteTrustServer(mainViper), getTransport(mainViper, gun, true), retriever) if err != nil { fatalf(err.Error()) } @@ -353,9 +354,9 @@ func (ps passwordStore) Basic(u *url.URL) (string, string) { return username, password } -func getTransport(gun string, readOnly bool) http.RoundTripper { +func getTransport(config *viper.Viper, gun string, readOnly bool) http.RoundTripper { // Attempt to get a root CA from the config file. Nil is the host defaults. - rootCAFile := mainViper.GetString("remote_server.root_ca") + rootCAFile := config.GetString("remote_server.root_ca") if rootCAFile != "" { // If we haven't been given an Absolute path, we assume it's relative // from the configuration directory (~/.notary by default) @@ -365,8 +366,8 @@ func getTransport(gun string, readOnly bool) http.RoundTripper { } insecureSkipVerify := false - if mainViper.IsSet("remote_server.skipTLSVerify") { - insecureSkipVerify = mainViper.GetBool("remote_server.skipTLSVerify") + if config.IsSet("remote_server.skipTLSVerify") { + insecureSkipVerify = config.GetBool("remote_server.skipTLSVerify") } tlsConfig, err := utils.ConfigureClientTLS(&utils.ClientTLSOpts{ RootCAFile: rootCAFile, @@ -387,18 +388,19 @@ func getTransport(gun string, readOnly bool) http.RoundTripper { TLSClientConfig: tlsConfig, DisableKeepAlives: true, } - - return tokenAuth(base, gun, readOnly) + return tokenAuth(config, base, gun, readOnly) } -func tokenAuth(baseTransport *http.Transport, gun string, readOnly bool) http.RoundTripper { +func tokenAuth(config *viper.Viper, baseTransport *http.Transport, gun string, + readOnly bool) http.RoundTripper { + // TODO(dmcgowan): add notary specific headers authTransport := transport.NewTransport(baseTransport) pingClient := &http.Client{ Transport: authTransport, Timeout: 5 * time.Second, } - trustServerURL := getRemoteTrustServer() + trustServerURL := getRemoteTrustServer(config) endpoint, err := url.Parse(trustServerURL) if err != nil { fatalf("Could not parse remote trust server url (%s): %s", trustServerURL, err.Error()) @@ -433,9 +435,9 @@ func tokenAuth(baseTransport *http.Transport, gun string, readOnly bool) http.Ro return transport.NewTransport(baseTransport, modifier) } -func getRemoteTrustServer() string { +func getRemoteTrustServer(config *viper.Viper) string { if remoteTrustServer == "" { - configRemote := mainViper.GetString("remote_server.url") + configRemote := config.GetString("remote_server.url") if configRemote != "" { remoteTrustServer = configRemote } else {