From ca1623e17bc017a9144697ef5286b12eee3008b8 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Wed, 9 Dec 2015 00:24:10 -0800 Subject: [PATCH] Update CLI rotate key command to optionally rotate a single key. This makes it possible to delegate snapshots key management to the server, and to reclaim the responsibility. Signed-off-by: Ying Li --- cmd/notary/keys.go | 33 +++++-- cmd/notary/keys_test.go | 185 ++++++++++++++++++++++++++++++++++++++++ cmd/notary/main.go | 1 - cmd/notary/tuf.go | 38 +++++---- 4 files changed, 231 insertions(+), 26 deletions(-) diff --git a/cmd/notary/keys.go b/cmd/notary/keys.go index 22d7d7adad..e690a630ad 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" @@ -62,8 +63,8 @@ var cmdKeyListTemplate = usageTemplate{ 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.", + 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 cmdKeyGenerateRootKeyTemplate = usageTemplate{ @@ -106,11 +107,12 @@ type keyCommander struct { // these need to be set configGetter func() *viper.Viper retriever passphrase.Retriever - remoteServer string // these are for command line parsing - no need to set keysExportRootChangePassphrase bool keysExportGUN string + rotateKeyRole string + rotateKeyServerManaged bool } func (k *keyCommander) GetCommand() *cobra.Command { @@ -120,7 +122,6 @@ func (k *keyCommander) GetCommand() *cobra.Command { cmd.AddCommand(cmdKeysRestoreTemplate.ToCommand(k.keysRestore)) cmd.AddCommand(cmdKeyImportRootTemplate.ToCommand(k.keysImportRoot)) cmd.AddCommand(cmdKeyRemoveTemplate.ToCommand(k.keyRemove)) - cmd.AddCommand(cmdRotateKeyTemplate.ToCommand(k.keysRotate)) cmdKeysBackup := cmdKeysBackupTemplate.ToCommand(k.keysBackup) cmdKeysBackup.Flags().StringVarP( @@ -132,6 +133,18 @@ func (k *keyCommander) GetCommand() *cobra.Command { &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", "snapshots". `+ + `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 } @@ -344,8 +357,7 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error { case data.CanonicalTargetsRole: rolesToRotate = []string{data.CanonicalTargetsRole} default: - cmd.Usage() - fatalf(`key rotation not supported for %s keys`, rotateKeyRole) + return fmt.Errorf("key rotation not supported for %s keys", k.rotateKeyRole) } if k.rotateKeyServerManaged && rotateKeyRole != data.CanonicalSnapshotRole { return fmt.Errorf( @@ -355,8 +367,15 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error { config := k.configGetter() gun := args[0] + 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, k.remoteServer, nil, retriever) + config.GetString("trust_dir"), gun, getRemoteTrustServer(config), + rt, k.retriever) if err != nil { return err } 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 e22f5436a4..efd9eb97cc 100644 --- a/cmd/notary/main.go +++ b/cmd/notary/main.go @@ -118,7 +118,6 @@ func setupCommand(notaryCmd *cobra.Command) { cmdKeyGenerator := &keyCommander{ configGetter: parseConfig, retriever: retriever, - remoteServer: remoteTrustServer, } notaryCmd.AddCommand(cmdKeyGenerator.GetCommand()) 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 {