refactoring and adding tests for EC key types

Signed-off-by: Riyaz Faizullabhoy <[email protected]>
Upstream-commit: 2d8cc3cd80204de38354610b92ab5c933cac3e0f
Component: cli

components/cli/cli/command/trust/key_generate.go

@@ -19,15 +19,24 @@ import (
 	"github.com/spf13/cobra"
 )
 
+type keyGenerateOptions struct {
+	name      string
+	directory string
+}
+
 func newKeyGenerateCommand(dockerCli command.Streams) *cobra.Command {
+	options := keyGenerateOptions{}
 	cmd := &cobra.Command{
 		Use:   "generate NAME [NAME...]",
 		Short: "Generate and load a signing key-pair",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return setupPassphraseAndGenerateKeys(dockerCli, args[0])
+			options.name = args[0]
+			return setupPassphraseAndGenerateKeys(dockerCli, options)
 		},
 	}
+	flags := cmd.Flags()
+	flags.StringVarP(&options.directory, "dir", "d", "", "Directory to generate key in, defaults to current directory")
 	return cmd
 }
 
@@ -48,60 +57,74 @@ func validateKeyArgs(keyName string, cwdPath string) error {
 	return nil
 }
 
-func setupPassphraseAndGenerateKeys(streams command.Streams, keyName string) error {
-	// always use a fresh passphrase for each key generation
-	freshPassRetGetter := func() notary.PassRetriever { return trust.GetPassphraseRetriever(streams.In(), streams.Out()) }
-	cwd, err := os.Getwd()
-	if err != nil {
-		return err
+func setupPassphraseAndGenerateKeys(streams command.Streams, opts keyGenerateOptions) error {
+	targetDir := opts.directory
+	// if the target dir is empty, default to CWD
+	if targetDir == "" {
+		cwd, err := os.Getwd()
+		if err != nil {
+			return err
+		}
+		targetDir = cwd
 	}
-	return validateAndGenerateKey(streams, keyName, cwd, freshPassRetGetter)
+	return validateAndGenerateKey(streams, opts.name, targetDir)
 }
 
-func validateAndGenerateKey(streams command.Streams, keyName string, workingDir string, passphraseGetter func() notary.PassRetriever) error {
+func validateAndGenerateKey(streams command.Streams, keyName string, workingDir string) error {
+	freshPassRetGetter := func() notary.PassRetriever { return trust.GetPassphraseRetriever(streams.In(), streams.Out()) }
 	if err := validateKeyArgs(keyName, workingDir); err != nil {
 		return err
 	}
 	fmt.Fprintf(streams.Out(), "\nGenerating key for %s...\n", keyName)
-	freshPassRet := passphraseGetter()
-	if err := generateKey(keyName, workingDir, trust.GetTrustDirectory(), freshPassRet); err != nil {
+	// Automatically load the private key to local storage for use
+	privKeyFileStore, err := trustmanager.NewKeyFileStore(trust.GetTrustDirectory(), freshPassRetGetter())
+	if err != nil {
+		return err
+	}
+
+	pubPEM, err := generateKeyAndOutputPubPEM(keyName, privKeyFileStore)
+	if err != nil {
 		fmt.Fprintf(streams.Out(), err.Error())
 		return fmt.Errorf("Error generating key for: %s", keyName)
 	}
-	pubFileName := strings.Join([]string{keyName, "pub"}, ".")
-	fmt.Fprintf(streams.Out(), "Successfully generated and loaded private key. Corresponding public key available: %s\n", pubFileName)
 
-	return nil
-}
-
-func generateKey(keyName, pubDir, privTrustDir string, passRet notary.PassRetriever) error {
-	privKey, err := tufutils.GenerateKey(data.ECDSAKey)
+	// Output the public key to a file in the CWD or specified dir
+	writtenPubFile, err := writePubKeyPEMToDir(pubPEM, keyName, workingDir)
 	if err != nil {
 		return err
 	}
+	fmt.Fprintf(streams.Out(), "Successfully generated and loaded private key. Corresponding public key available: %s\n", writtenPubFile)
 
-	// Automatically load the private key to local storage for use
-	privKeyFileStore, err := trustmanager.NewKeyFileStore(privTrustDir, passRet)
+	return nil
+}
+
+func generateKeyAndOutputPubPEM(keyName string, privKeyStore trustmanager.KeyStore) (pem.Block, error) {
+	privKey, err := tufutils.GenerateKey(data.ECDSAKey)
 	if err != nil {
-		return err
+		return pem.Block{}, err
 	}
 
-	privKeyFileStore.AddKey(trustmanager.KeyInfo{Role: data.RoleName(keyName)}, privKey)
+	privKeyStore.AddKey(trustmanager.KeyInfo{Role: data.RoleName(keyName)}, privKey)
 	if err != nil {
-		return err
+		return pem.Block{}, err
 	}
 
 	pubKey := data.PublicKeyFromPrivate(privKey)
-	pubPEM := pem.Block{
+	return pem.Block{
 		Type: "PUBLIC KEY",
 		Headers: map[string]string{
 			"role": keyName,
 		},
 		Bytes: pubKey.Public(),
-	}
+	}, nil
+}
 
-	// Output the public key to a file in the CWD
+func writePubKeyPEMToDir(pubPEM pem.Block, keyName, workingDir string) (string, error) {
+	// Output the public key to a file in the CWD or specified dir
 	pubFileName := strings.Join([]string{keyName, "pub"}, ".")
-	pubFilePath := filepath.Join(pubDir, pubFileName)
-	return ioutil.WriteFile(pubFilePath, pem.EncodeToMemory(&pubPEM), notary.PrivNoExecPerms)
+	pubFilePath := filepath.Join(workingDir, pubFileName)
+	if err := ioutil.WriteFile(pubFilePath, pem.EncodeToMemory(&pubPEM), notary.PrivNoExecPerms); err != nil {
+		return "", fmt.Errorf("Error writing public key to location: %s", pubFilePath)
+	}
+	return pubFilePath, nil
 }

components/cli/cli/command/trust/key_generate_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/cli/internal/test/testutil"
 	"github.com/docker/notary"
 	"github.com/docker/notary/passphrase"
+	"github.com/docker/notary/trustmanager"
 	tufutils "github.com/docker/notary/tuf/utils"
 	"github.com/stretchr/testify/assert"
 )
@@ -60,27 +61,17 @@ func TestGenerateKeySuccess(t *testing.T) {
 	cannedPasswordRetriever := passphrase.ConstantRetriever(passwd)
 	// generate a single key
 	keyName := "alice"
-	assert.NoError(t, generateKey(keyName, pubKeyCWD, privKeyStorageDir, cannedPasswordRetriever))
-
-	// check that the public key exists:
-	expectedPubKeyPath := filepath.Join(pubKeyCWD, keyName+".pub")
-	_, err = os.Stat(expectedPubKeyPath)
+	privKeyFileStore, err := trustmanager.NewKeyFileStore(privKeyStorageDir, cannedPasswordRetriever)
 	assert.NoError(t, err)
-	// check that the public key is the only file output in CWD
-	cwdKeyFiles, err := ioutil.ReadDir(pubKeyCWD)
+
+	pubKeyPEM, err := generateKeyAndOutputPubPEM(keyName, privKeyFileStore)
 	assert.NoError(t, err)
-	assert.Len(t, cwdKeyFiles, 1)
 
-	// verify the key header is set with the specified name
-	from, _ := os.OpenFile(expectedPubKeyPath, os.O_RDONLY, notary.PrivExecPerms)
-	defer from.Close()
-	fromBytes, _ := ioutil.ReadAll(from)
-	keyPEM, _ := pem.Decode(fromBytes)
-	assert.Equal(t, keyName, keyPEM.Headers["role"])
+	assert.Equal(t, keyName, pubKeyPEM.Headers["role"])
 	// the default GUN is empty
-	assert.Equal(t, "", keyPEM.Headers["gun"])
+	assert.Equal(t, "", pubKeyPEM.Headers["gun"])
 	// assert public key header
-	assert.Equal(t, "PUBLIC KEY", keyPEM.Type)
+	assert.Equal(t, "PUBLIC KEY", pubKeyPEM.Type)
 
 	// check that an appropriate ~/<trust_dir>/private/<key_id>.key file exists
 	expectedPrivKeyDir := filepath.Join(privKeyStorageDir, notary.PrivDir)
@@ -95,16 +86,28 @@ func TestGenerateKeySuccess(t *testing.T) {
 	// verify the key content
 	privFrom, _ := os.OpenFile(privKeyFilePath, os.O_RDONLY, notary.PrivExecPerms)
 	defer privFrom.Close()
-	fromBytes, _ = ioutil.ReadAll(privFrom)
-	keyPEM, _ = pem.Decode(fromBytes)
-	assert.Equal(t, keyName, keyPEM.Headers["role"])
+	fromBytes, _ := ioutil.ReadAll(privFrom)
+	privKeyPEM, _ := pem.Decode(fromBytes)
+	assert.Equal(t, keyName, privKeyPEM.Headers["role"])
 	// the default GUN is empty
-	assert.Equal(t, "", keyPEM.Headers["gun"])
+	assert.Equal(t, "", privKeyPEM.Headers["gun"])
 	// assert encrypted header
-	assert.Equal(t, "ENCRYPTED PRIVATE KEY", keyPEM.Type)
+	assert.Equal(t, "ENCRYPTED PRIVATE KEY", privKeyPEM.Type)
 	// check that the passphrase matches
-	_, err = tufutils.ParsePKCS8ToTufKey(keyPEM.Bytes, []byte(passwd))
+	_, err = tufutils.ParsePKCS8ToTufKey(privKeyPEM.Bytes, []byte(passwd))
+	assert.NoError(t, err)
+
+	// check that the public key exists at the correct path if we use the helper:
+	returnedPath, err := writePubKeyPEMToDir(pubKeyPEM, keyName, pubKeyCWD)
+	assert.NoError(t, err)
+	expectedPubKeyPath := filepath.Join(pubKeyCWD, keyName+".pub")
+	assert.Equal(t, returnedPath, expectedPubKeyPath)
+	_, err = os.Stat(expectedPubKeyPath)
 	assert.NoError(t, err)
+	// check that the public key is the only file output in CWD
+	cwdKeyFiles, err := ioutil.ReadDir(pubKeyCWD)
+	assert.NoError(t, err)
+	assert.Len(t, cwdKeyFiles, 1)
 }
 
 func TestValidateKeyArgs(t *testing.T) {

components/cli/cli/command/trust/key_load.go

@@ -1,10 +1,10 @@
 package trust
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path/filepath"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
@@ -42,7 +42,7 @@ func newKeyLoadCommand(dockerCli command.Streams) *cobra.Command {
 
 func loadPrivKey(streams command.Streams, keyPath string, options keyLoadOptions) error {
 	trustDir := trust.GetTrustDirectory()
-	keyFileStore, err := storage.NewPrivateKeyFileStorage(filepath.Join(trustDir, notary.PrivDir), notary.KeyExtension)
+	keyFileStore, err := storage.NewPrivateKeyFileStorage(trustDir, notary.KeyExtension)
 	if err != nil {
 		return err
 	}
@@ -52,39 +52,43 @@ func loadPrivKey(streams command.Streams, keyPath string, options keyLoadOptions
 
 	// Always use a fresh passphrase retriever for each import
 	passRet := trust.GetPassphraseRetriever(streams.In(), streams.Out())
-	if err := loadPrivKeyFromPath(privKeyImporters, keyPath, options.keyName, passRet); err != nil {
+	keyBytes, err := getPrivKeyBytesFromPath(keyPath)
+	if err != nil {
+		return fmt.Errorf("error reading key from %s: %s", keyPath, err)
+	}
+	if err := loadPrivKeyBytesToStore(keyBytes, privKeyImporters, keyPath, options.keyName, passRet); err != nil {
 		return fmt.Errorf("error importing key from %s: %s", keyPath, err)
 	}
 	fmt.Fprintf(streams.Out(), "Successfully imported key from %s\n", keyPath)
 	return nil
 }
 
-func loadPrivKeyFromPath(privKeyImporters []utils.Importer, keyPath, keyName string, passRet notary.PassRetriever) error {
+func getPrivKeyBytesFromPath(keyPath string) ([]byte, error) {
 	fileInfo, err := os.Stat(keyPath)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	if fileInfo.Mode() != ownerReadOnlyPerms && fileInfo.Mode() != ownerReadAndWritePerms {
-		return fmt.Errorf("private key permission from %s should be set to 400 or 600", keyPath)
+		return nil, fmt.Errorf("private key permission from %s should be set to 400 or 600", keyPath)
 	}
 
 	from, err := os.OpenFile(keyPath, os.O_RDONLY, notary.PrivExecPerms)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	defer from.Close()
 
 	keyBytes, err := ioutil.ReadAll(from)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	if _, _, err := tufutils.ExtractPrivateKeyAttributes(keyBytes); err != nil {
+	return keyBytes, nil
+}
+
+func loadPrivKeyBytesToStore(privKeyBytes []byte, privKeyImporters []utils.Importer, keyPath, keyName string, passRet notary.PassRetriever) error {
+	if _, _, err := tufutils.ExtractPrivateKeyAttributes(privKeyBytes); err != nil {
 		return fmt.Errorf("provided file %s is not a supported private key - to add a signer's public key use docker trust signer add", keyPath)
 	}
-	// Rewind the file pointer
-	if _, err := from.Seek(0, 0); err != nil {
-		return err
-	}
-
-	return utils.ImportKeys(from, privKeyImporters, keyName, "", passRet)
+	// Make a reader, rewind the file pointer
+	return utils.ImportKeys(bytes.NewReader(privKeyBytes), privKeyImporters, keyName, "", passRet)
 }