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 <ying.li@docker.com>

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.",
+	Short: "Rotate 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.",
+	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()
+		return fmt.Errorf("key rotation not supported for %s keys", k.rotateKeyRole)
-		fatalf(`key rotation not supported for %s keys`, 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
 	}

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")
+}

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())

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") {
+	if config.IsSet("remote_server.skipTLSVerify") {
-		insecureSkipVerify = mainViper.GetBool("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(config, base, gun, readOnly)
-	return tokenAuth(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 {