Add a check that a root key being imported is encrypted

Add unit test coverage that makes this check fail. Also add unit test
coverage for making sure trying to import something that isn't PEM fails
in the expected way.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

keystoremanager/import_export.go

@@ -16,6 +16,16 @@ import (
 	"github.com/endophage/gotuf/data"
 )
 
+var (
+	// ErrNoValidPrivateKey is returned if a key being imported doesn't
+	// look like a private key
+	ErrNoValidPrivateKey = errors.New("no valid private key found")
+
+	// ErrRootKeyNotEncrypted is returned if a root key being imported is
+	// unencrypted
+	ErrRootKeyNotEncrypted = errors.New("only encrypted root keys may be imported")
+)
+
 // ExportRootKey exports the specified root key to an io.Writer in PEM format.
 // The key's existing encryption is preserved.
 func (km *KeyStoreManager) ExportRootKey(dest io.Writer, keyID string) error {
@@ -28,6 +38,21 @@ func (km *KeyStoreManager) ExportRootKey(dest io.Writer, keyID string) error {
 	return err
 }
 
+// checkRootKeyIsEncrypted makes sure the root key is encrypted. We have
+// internal assumptions that depend on this.
+func checkRootKeyIsEncrypted(pemBytes []byte) error {
+	block, _ := pem.Decode(pemBytes)
+	if block == nil {
+		return ErrNoValidPrivateKey
+	}
+
+	if !x509.IsEncryptedPEMBlock(block) {
+		return ErrRootKeyNotEncrypted
+	}
+
+	return nil
+}
+
 // ImportRootKey imports a root in PEM format key from an io.Reader
 // The key's existing encryption is preserved. The keyID parameter is
 // necessary because otherwise we'd need the passphrase to decrypt the key
@@ -38,8 +63,11 @@ func (km *KeyStoreManager) ImportRootKey(source io.Reader, keyID string) error {
 		return err
 	}
 
-	err = km.rootKeyStore.Add(keyID, pemBytes)
-	if err != nil {
+	if err = checkRootKeyIsEncrypted(pemBytes); err != nil {
+		return err
+	}
+
+	if err = km.rootKeyStore.Add(keyID, pemBytes); err != nil {
 		return err
 	}
 
@@ -60,7 +88,7 @@ func moveKeysWithNewPassphrase(oldKeyStore, newKeyStore *trustmanager.KeyFileSto
 
 		block, _ := pem.Decode(pemBytes)
 		if block == nil {
-			return errors.New("no valid private key found")
+			return ErrNoValidPrivateKey
 		}
 
 		if !x509.IsEncryptedPEMBlock(block) {
@@ -213,6 +241,9 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader, passphrase string
 		// package guarantees that the separator will be /
 		rootKeysPrefix := rootKeysSubdir + "/"
 		if strings.HasPrefix(fNameTrimmed, rootKeysPrefix) {
+			if err = checkRootKeyIsEncrypted(pemBytes); err != nil {
+				return err
+			}
 			// Root keys are preserved without decrypting
 			keyName := strings.TrimPrefix(fNameTrimmed, rootKeysPrefix)
 			newRootKeys[keyName] = pemBytes

keystoremanager/import_export_test.go

@@ -2,6 +2,7 @@ package keystoremanager_test
 
 import (
 	"archive/zip"
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -12,6 +13,7 @@ import (
 	"testing"
 
 	"github.com/docker/notary/client"
+	"github.com/docker/notary/keystoremanager"
 	"github.com/docker/notary/trustmanager"
 	"github.com/endophage/gotuf/data"
 	"github.com/stretchr/testify/assert"
@@ -216,10 +218,10 @@ func TestImportExportRootKey(t *testing.T) {
 	repo2, err := client.NewNotaryRepository(tempBaseDir2, gun, ts.URL, http.DefaultTransport)
 	assert.NoError(t, err, "error creating repo: %s", err)
 
-	rootKeyID2, err := repo2.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), "oldPassphrase")
+	rootKeyID2, err := repo2.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), oldPassphrase)
 	assert.NoError(t, err, "error generating root key: %s", err)
 
-	rootCryptoService2, err := repo2.KeyStoreManager.GetRootCryptoService(rootKeyID2, "oldPassphrase")
+	rootCryptoService2, err := repo2.KeyStoreManager.GetRootCryptoService(rootKeyID2, oldPassphrase)
 	assert.NoError(t, err, "error retrieving root key: %s", err)
 
 	err = repo2.Initialize(rootCryptoService2)
@@ -239,4 +241,20 @@ func TestImportExportRootKey(t *testing.T) {
 	rootKeyFilename := rootKeyID + ".key"
 	_, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename))
 	assert.NoError(t, err, "missing root key")
+
+	// Try to import a decrypted version of the root key and make sure it
+	// doesn't succeed
+	pemBytes, err := ioutil.ReadFile(tempKeyFilePath)
+	assert.NoError(t, err, "could not read key file")
+	privKey, err := trustmanager.ParsePEMPrivateKey(pemBytes, oldPassphrase)
+	assert.NoError(t, err, "could not decrypt key file")
+	decryptedPEMBytes, err := trustmanager.KeyToPEM(privKey)
+	assert.NoError(t, err, "could not convert key to PEM")
+
+	err = repo2.KeyStoreManager.ImportRootKey(bytes.NewReader(decryptedPEMBytes), rootKeyID)
+	assert.EqualError(t, err, keystoremanager.ErrRootKeyNotEncrypted.Error())
+
+	// Try to import garbage and make sure it doesn't succeed
+	err = repo2.KeyStoreManager.ImportRootKey(strings.NewReader("this is not PEM"), rootKeyID)
+	assert.EqualError(t, err, keystoremanager.ErrNoValidPrivateKey.Error())
 }