Repository: docker/notary
Pull request #242 (2015-10-27 23:31:42)

Unify root nonroot keystore
This leaves handling the `root_keys` and `tuf_keys` subdirectories to the `KeyFileStore` rather than having `KeyStoreManager` (which is going to be removed in the future) have a non-root `KeyFileStore` and a root `KeyFileStore`.

We cannot do away with the `root_keys` and `tuf_keys` subdirectory completely because we still have to be able to read repos in that format.  And if we see a `tuf_keys` subdirectory, we can't be sure if it's the old-style file structure, or if someone added a GUN that started with `tuf_keys`.

This also makes the following changes:
1. `KeyStore` now has `ImportKey` and `ExportKey` functions, so each `KeyStore` can decide for itself whether and how to import/export keys.
2. After discussion with @endophage, importing a root key now requires you to decrypt the key, to make sure that it's a valid key.  This makes the `ImportKey` function in `KeyStore` less ugly, because you don't have to provide the key ID.

Not sure how importing/exporting will work in the future if there are multiple `KeyStore`'s.


Patch:
diff --git a/client/client.go b/client/client.go
index 2d8643d0..b7ae899e 100644
--- a/client/client.go
+++ b/client/client.go
@@ -109,7 +109,7 @@ func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper,
 		return nil, err
 	}
 
-	cryptoService := cryptoservice.NewCryptoService(gun, keyStoreManager.NonRootKeyStore())
+	cryptoService := cryptoservice.NewCryptoService(gun, keyStoreManager.KeyStore)
 
 	nRepo := &NotaryRepository{
 		gun:             gun,
diff --git a/cmd/notary/keys.go b/cmd/notary/keys.go
index 0002fa4c..1beab11f 100644
--- a/cmd/notary/keys.go
+++ b/cmd/notary/keys.go
@@ -11,7 +11,6 @@ import (
 	notaryclient "github.com/docker/notary/client"
 	"github.com/docker/notary/keystoremanager"
 	"github.com/docker/notary/pkg/passphrase"
-	"github.com/docker/notary/trustmanager"
 
 	"github.com/spf13/cobra"
 )
@@ -137,15 +136,7 @@ func keysRemoveKey(cmd *cobra.Command, args []string) {
 	}
 
 	// Choose the correct filestore to remove the key from
-	var keyStoreToRemove *trustmanager.KeyFileStore
-	var keyMap map[string]string
-	if keyRemoveRoot {
-		keyStoreToRemove = keyStoreManager.RootKeyStore()
-		keyMap = keyStoreManager.RootKeyStore().ListKeys()
-	} else {
-		keyStoreToRemove = keyStoreManager.NonRootKeyStore()
-		keyMap = keyStoreManager.NonRootKeyStore().ListKeys()
-	}
+	keyMap := keyStoreManager.KeyStore.ListKeys()
 
 	// Attempt to find the full GUN to the key in the map
 	// This is irrelevant for removing root keys, but does no harm
@@ -162,7 +153,7 @@ func keysRemoveKey(cmd *cobra.Command, args []string) {
 	}
 
 	// Attempt to remove the key
-	err = keyStoreToRemove.RemoveKey(keyWithGUN)
+	err = keyStoreManager.KeyStore.RemoveKey(keyWithGUN)
 	if err != nil {
 		fatalf("failed to remove key with key ID: %s, %v", keyID, err)
 	}
@@ -181,18 +172,20 @@ func keysList(cmd *cobra.Command, args []string) {
 		fatalf("failed to create a new truststore manager with directory: %s", trustDir)
 	}
 
+	// Get a map of all the keys/roles
+	keysMap := keyStoreManager.KeyStore.ListKeys()
+
 	fmt.Println("")
 	fmt.Println("# Root keys: ")
-	for k := range keyStoreManager.RootKeyStore().ListKeys() {
-		fmt.Println(k)
+	for k, v := range keysMap {
+		if v == "root" {
+			fmt.Println(k)
+		}
 	}
 
 	fmt.Println("")
 	fmt.Println("# Signing keys: ")
 
-	// Get a map of all the keys/roles
-	keysMap := keyStoreManager.NonRootKeyStore().ListKeys()
-
 	// Get a list of all the keys
 	var sortedKeys []string
 	for k := range keysMap {
@@ -203,7 +196,9 @@ func keysList(cmd *cobra.Command, args []string) {
 
 	// Print a sorted list of the key/role
 	for _, k := range sortedKeys {
-		printKey(k, keysMap[k])
+		if keysMap[k] != "root" {
+			printKey(k, keysMap[k])
+		}
 	}
 }
 
diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go
index b3cd58e3..29113b39 100644
--- a/cmd/notary/tuf.go
+++ b/cmd/notary/tuf.go
@@ -123,7 +123,12 @@ func tufInit(cmd *cobra.Command, args []string) {
 		fatalf(err.Error())
 	}
 
-	keysMap := nRepo.KeyStoreManager.RootKeyStore().ListKeys()
+	keysMap := make(map[string]string)
+	for k, role := range nRepo.KeyStoreManager.KeyStore.ListKeys() {
+		if role == "root" {
+			keysMap[k] = role
+		}
+	}
 
 	var rootKeyID string
 	if len(keysMap) < 1 {
diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go
index 05624ad1..649a15fb 100644
--- a/keystoremanager/import_export.go
+++ b/keystoremanager/import_export.go
@@ -11,7 +11,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/Sirupsen/logrus"
 	"github.com/docker/notary/pkg/passphrase"
 	"github.com/docker/notary/trustmanager"
 )
@@ -35,19 +34,24 @@ var (
 // 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 {
-	pemBytes, err := km.rootKeyStore.Get(keyID + "_root")
+	pemBytes, err := km.KeyStore.ExportKey(keyID)
 	if err != nil {
 		return err
 	}
-
-	_, err = dest.Write(pemBytes)
-	return err
+	nBytes, err := dest.Write(pemBytes)
+	if err != nil {
+		return err
+	}
+	if nBytes != len(pemBytes) {
+		return errors.New("Unable to finish writing exported key.")
+	}
+	return nil
 }
 
 // ExportRootKeyReencrypt exports the specified root key to an io.Writer in
 // PEM format. The key is reencrypted with a new passphrase.
 func (km *KeyStoreManager) ExportRootKeyReencrypt(dest io.Writer, keyID string, newPassphraseRetriever passphrase.Retriever) error {
-	privateKey, alias, err := km.rootKeyStore.GetKey(keyID)
+	privateKey, alias, err := km.KeyStore.GetKey(keyID)
 	if err != nil {
 		return err
 	}
@@ -56,25 +60,29 @@ func (km *KeyStoreManager) ExportRootKeyReencrypt(dest io.Writer, keyID string,
 	tempBaseDir, err := ioutil.TempDir("", "notary-key-export-")
 	defer os.RemoveAll(tempBaseDir)
 
-	privRootKeysSubdir := filepath.Join(privDir, rootKeysSubdir)
-	tempRootKeysPath := filepath.Join(tempBaseDir, privRootKeysSubdir)
-	tempRootKeyStore, err := trustmanager.NewKeyFileStore(tempRootKeysPath, newPassphraseRetriever)
+	tempKeysPath := filepath.Join(tempBaseDir, privDir)
+	tempKeyStore, err := trustmanager.NewKeyFileStore(tempKeysPath, newPassphraseRetriever)
 	if err != nil {
 		return err
 	}
 
-	err = tempRootKeyStore.AddKey(keyID, alias, privateKey)
+	err = tempKeyStore.AddKey(keyID, alias, privateKey)
 	if err != nil {
 		return err
 	}
 
-	pemBytes, err := tempRootKeyStore.Get(keyID + "_" + alias)
+	pemBytes, err := tempKeyStore.ExportKey(keyID)
 	if err != nil {
 		return err
 	}
-
-	_, err = dest.Write(pemBytes)
-	return err
+	nBytes, err := dest.Write(pemBytes)
+	if err != nil {
+		return err
+	}
+	if nBytes != len(pemBytes) {
+		return errors.New("Unable to finish writing exported key.")
+	}
+	return nil
 }
 
 // checkRootKeyIsEncrypted makes sure the root key is encrypted. We have
@@ -106,7 +114,7 @@ func (km *KeyStoreManager) ImportRootKey(source io.Reader, keyID string) error {
 		return err
 	}
 
-	if err = km.rootKeyStore.Add(keyID+"_root", pemBytes); err != nil {
+	if err = km.KeyStore.ImportKey(pemBytes, "root"); err != nil {
 		return err
 	}
 
@@ -170,35 +178,20 @@ func (km *KeyStoreManager) ExportAllKeys(dest io.Writer, newPassphraseRetriever
 	tempBaseDir, err := ioutil.TempDir("", "notary-key-export-")
 	defer os.RemoveAll(tempBaseDir)
 
-	privNonRootKeysSubdir := filepath.Join(privDir, nonRootKeysSubdir)
-	privRootKeysSubdir := filepath.Join(privDir, rootKeysSubdir)
-
-	// Create temporary keystores to use as a staging area
-	tempNonRootKeysPath := filepath.Join(tempBaseDir, privNonRootKeysSubdir)
-	tempNonRootKeyStore, err := trustmanager.NewKeyFileStore(tempNonRootKeysPath, newPassphraseRetriever)
-	if err != nil {
-		return err
-	}
-
-	tempRootKeysPath := filepath.Join(tempBaseDir, privRootKeysSubdir)
-	tempRootKeyStore, err := trustmanager.NewKeyFileStore(tempRootKeysPath, newPassphraseRetriever)
+	// Create temporary keystore to use as a staging area
+	tempKeysPath := filepath.Join(tempBaseDir, privDir)
+	tempKeyStore, err := trustmanager.NewKeyFileStore(tempKeysPath, newPassphraseRetriever)
 	if err != nil {
 		return err
 	}
 
-	if err := moveKeys(km.rootKeyStore, tempRootKeyStore); err != nil {
-		return err
-	}
-	if err := moveKeys(km.nonRootKeyStore, tempNonRootKeyStore); err != nil {
+	if err := moveKeys(km.KeyStore, tempKeyStore); err != nil {
 		return err
 	}
 
 	zipWriter := zip.NewWriter(dest)
 
-	if err := addKeysToArchive(zipWriter, tempRootKeyStore, privRootKeysSubdir); err != nil {
-		return err
-	}
-	if err := addKeysToArchive(zipWriter, tempNonRootKeyStore, privNonRootKeysSubdir); err != nil {
+	if err := addKeysToArchive(zipWriter, tempKeyStore, privDir); err != nil {
 		return err
 	}
 
@@ -214,13 +207,7 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader) error {
 	// Temporarily store the keys in maps, so we can bail early if there's
 	// an error (for example, wrong passphrase), without leaving the key
 	// store in an inconsistent state
-	newRootKeys := make(map[string][]byte)
-	newNonRootKeys := make(map[string][]byte)
-
-	// Note that using / as a separator is okay here - the zip package
-	// guarantees that the separator will be /
-	rootKeysPrefix := privDir + "/" + rootKeysSubdir + "/"
-	nonRootKeysPrefix := privDir + "/" + nonRootKeysSubdir + "/"
+	newKeys := make(map[string][]byte)
 
 	// Iterate through the files in the archive. Don't add the keys
 	for _, f := range zipReader.File {
@@ -236,29 +223,22 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader) error {
 			return nil
 		}
 
-		// Is this in the root_keys directory?
 		// Note that using / as a separator is okay here - the zip
 		// package guarantees that the separator will be /
-		if strings.HasPrefix(fNameTrimmed, rootKeysPrefix) {
-			if err = checkRootKeyIsEncrypted(fileBytes); err != nil {
-				rc.Close()
-				return err
+		if strings.HasPrefix(fNameTrimmed, privDir) {
+			if fNameTrimmed[len(fNameTrimmed)-5:] == "_root" {
+				if err = checkRootKeyIsEncrypted(fileBytes); err != nil {
+					rc.Close()
+					return err
+				}
 			}
-			// Root keys are preserved without decrypting
-			keyName := strings.TrimPrefix(fNameTrimmed, rootKeysPrefix)
-			newRootKeys[keyName] = fileBytes
-
-		} else if strings.HasPrefix(fNameTrimmed, nonRootKeysPrefix) {
-			// Nonroot keys are preserved without decrypting
-			keyName := strings.TrimPrefix(fNameTrimmed, nonRootKeysPrefix)
-			newNonRootKeys[keyName] = fileBytes
-
+			keyName := strings.TrimPrefix(fNameTrimmed, privDir)
+			newKeys[keyName] = fileBytes
 		} else {
 			// This path inside the zip archive doesn't look like a
 			// root key, non-root key, or alias. To avoid adding a file
 			// to the filestore that we won't be able to use, skip
 			// this file in the import.
-			logrus.Warnf("skipping import of key with a path that doesn't begin with %s or %s: %s", rootKeysPrefix, nonRootKeysPrefix, f.Name)
 			rc.Close()
 			continue
 		}
@@ -266,14 +246,8 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader) error {
 		rc.Close()
 	}
 
-	for keyName, pemBytes := range newRootKeys {
-		if err := km.rootKeyStore.Add(keyName, pemBytes); err != nil {
-			return err
-		}
-	}
-
-	for keyName, pemBytes := range newNonRootKeys {
-		if err := km.nonRootKeyStore.Add(keyName, pemBytes); err != nil {
+	for keyName, pemBytes := range newKeys {
+		if err := km.KeyStore.Add(keyName, pemBytes); err != nil {
 			return err
 		}
 	}
@@ -309,26 +283,24 @@ func (km *KeyStoreManager) ExportKeysByGUN(dest io.Writer, gun string, passphras
 	tempBaseDir, err := ioutil.TempDir("", "notary-key-export-")
 	defer os.RemoveAll(tempBaseDir)
 
-	privNonRootKeysSubdir := filepath.Join(privDir, nonRootKeysSubdir)
-
 	// Create temporary keystore to use as a staging area
-	tempNonRootKeysPath := filepath.Join(tempBaseDir, privNonRootKeysSubdir)
-	tempNonRootKeyStore, err := trustmanager.NewKeyFileStore(tempNonRootKeysPath, passphraseRetriever)
+	tempKeysPath := filepath.Join(tempBaseDir, privDir)
+	tempKeyStore, err := trustmanager.NewKeyFileStore(tempKeysPath, passphraseRetriever)
 	if err != nil {
 		return err
 	}
 
-	if err := moveKeysByGUN(km.nonRootKeyStore, tempNonRootKeyStore, gun); err != nil {
+	if err := moveKeysByGUN(km.KeyStore, tempKeyStore, gun); err != nil {
 		return err
 	}
 
 	zipWriter := zip.NewWriter(dest)
 
-	if len(tempNonRootKeyStore.ListKeys()) == 0 {
+	if len(tempKeyStore.ListKeys()) == 0 {
 		return ErrNoKeysFoundForGUN
 	}
 
-	if err := addKeysToArchive(zipWriter, tempNonRootKeyStore, privNonRootKeysSubdir); err != nil {
+	if err := addKeysToArchive(zipWriter, tempKeyStore, privDir); err != nil {
 		return err
 	}
 
diff --git a/keystoremanager/keystoremanager.go b/keystoremanager/keystoremanager.go
index bf39d7fa..fc7917e9 100644
--- a/keystoremanager/keystoremanager.go
+++ b/keystoremanager/keystoremanager.go
@@ -20,19 +20,15 @@ import (
 // KeyStoreManager is an abstraction around the root and non-root key stores,
 // and related CA stores
 type KeyStoreManager struct {
-	rootKeyStore    *trustmanager.KeyFileStore
-	nonRootKeyStore *trustmanager.KeyFileStore
-
+	KeyStore                *trustmanager.KeyFileStore
 	trustedCAStore          trustmanager.X509Store
 	trustedCertificateStore trustmanager.X509Store
 }
 
 const (
-	trustDir          = "trusted_certificates"
-	privDir           = "private"
-	rootKeysSubdir    = "root_keys"
-	nonRootKeysSubdir = "tuf_keys"
-	rsaRootKeySize    = 4096 // Used for new root keys
+	trustDir       = "trusted_certificates"
+	privDir        = "private"
+	rsaRootKeySize = 4096 // Used for new root keys
 )
 
 // ErrValidationFail is returned when there is no valid trusted certificates
@@ -62,15 +58,8 @@ func (err ErrRootRotationFail) Error() string {
 // NewKeyStoreManager returns an initialized KeyStoreManager, or an error
 // if it fails to create the KeyFileStores or load certificates
 func NewKeyStoreManager(baseDir string, passphraseRetriever passphrase.Retriever) (*KeyStoreManager, error) {
-	nonRootKeysPath := filepath.Join(baseDir, privDir, nonRootKeysSubdir)
-	nonRootKeyStore, err := trustmanager.NewKeyFileStore(nonRootKeysPath, passphraseRetriever)
-	if err != nil {
-		return nil, err
-	}
-
-	// Load the keystore that will hold all of our encrypted Root Private Keys
-	rootKeysPath := filepath.Join(baseDir, privDir, rootKeysSubdir)
-	rootKeyStore, err := trustmanager.NewKeyFileStore(rootKeysPath, passphraseRetriever)
+	keysPath := filepath.Join(baseDir, privDir)
+	keyStore, err := trustmanager.NewKeyFileStore(keysPath, passphraseRetriever)
 	if err != nil {
 		return nil, err
 	}
@@ -102,25 +91,12 @@ func NewKeyStoreManager(baseDir string, passphraseRetriever passphrase.Retriever
 	}
 
 	return &KeyStoreManager{
-		rootKeyStore:            rootKeyStore,
-		nonRootKeyStore:         nonRootKeyStore,
+		KeyStore:                keyStore,
 		trustedCAStore:          trustedCAStore,
 		trustedCertificateStore: trustedCertificateStore,
 	}, nil
 }
 
-// RootKeyStore returns the root key store being managed by this
-// KeyStoreManager
-func (km *KeyStoreManager) RootKeyStore() *trustmanager.KeyFileStore {
-	return km.rootKeyStore
-}
-
-// NonRootKeyStore returns the non-root key store being managed by this
-// KeyStoreManager
-func (km *KeyStoreManager) NonRootKeyStore() *trustmanager.KeyFileStore {
-	return km.nonRootKeyStore
-}
-
 // TrustedCertificateStore returns the trusted certificate store being managed
 // by this KeyStoreManager
 func (km *KeyStoreManager) TrustedCertificateStore() trustmanager.X509Store {
@@ -165,7 +141,7 @@ func (km *KeyStoreManager) GenRootKey(algorithm string) (string, error) {
 	}
 
 	// Changing the root
-	km.rootKeyStore.AddKey(privKey.ID(), "root", privKey)
+	km.KeyStore.AddKey(privKey.ID(), "root", privKey)
 
 	return privKey.ID(), nil
 }
@@ -173,12 +149,12 @@ func (km *KeyStoreManager) GenRootKey(algorithm string) (string, error) {
 // GetRootCryptoService retrieves a root key and a cryptoservice to use with it
 // TODO(mccauley): remove this as its no longer needed once we have key caching in the keystores
 func (km *KeyStoreManager) GetRootCryptoService(rootKeyID string) (*cryptoservice.UnlockedCryptoService, error) {
-	privKey, _, err := km.rootKeyStore.GetKey(rootKeyID)
+	privKey, _, err := km.KeyStore.GetKey(rootKeyID)
 	if err != nil {
 		return nil, fmt.Errorf("could not get decrypted root key with keyID: %s, %v", rootKeyID, err)
 	}
 
-	cryptoService := cryptoservice.NewCryptoService("", km.rootKeyStore)
+	cryptoService := cryptoservice.NewCryptoService("", km.KeyStore)
 
 	return cryptoservice.NewUnlockedCryptoService(privKey, cryptoService), nil
 }
diff --git a/server/server.go b/server/server.go
index ead03695..cc8e29de 100644
--- a/server/server.go
+++ b/server/server.go
@@ -7,7 +7,6 @@ import (
 	"net/http"
 
 	"github.com/Sirupsen/logrus"
-	"github.com/docker/distribution/health"
 	"github.com/docker/distribution/registry/auth"
 	"github.com/endophage/gotuf/data"
 	"github.com/endophage/gotuf/signed"
@@ -75,11 +74,6 @@ func Run(ctx context.Context, addr, tlsCertFile, tlsKeyFile string, trust signed
 	r.Methods("GET").Path("/v2/{imageName:.*}/_trust/tuf/timestamp.json").Handler(hand(handlers.GetTimestampHandler, "pull"))
 	r.Methods("GET").Path("/v2/{imageName:.*}/_trust/tuf/timestamp.key").Handler(hand(handlers.GetTimestampKeyHandler, "push", "pull"))
 	r.Methods("DELETE").Path("/v2/{imageName:.*}/_trust/tuf/").Handler(hand(handlers.DeleteHandler, "push", "pull"))
-	r.Methods("GET").Path("/_notary_server/health").Handler(hand(
-		func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
-			health.StatusHandler(w, r)
-			return nil
-		}))
 	r.Methods("GET", "POST", "PUT", "HEAD", "DELETE").Path("/{other:.*}").Handler(hand(utils.NotFoundHandler))
 	svr := http.Server{
 		Addr:    addr,
diff --git a/signer/keydbstore.go b/signer/keydbstore.go
index a7fa5aa1..6ec5f409 100644
--- a/signer/keydbstore.go
+++ b/signer/keydbstore.go
@@ -2,6 +2,7 @@ package signer
 
 import (
 	"database/sql"
+	"errors"
 	"fmt"
 	"sync"
 
@@ -196,6 +197,16 @@ func (s *KeyDBStore) RotateKeyPassphrase(name, newPassphraseAlias string) error
 	return nil
 }
 
+// ExportKey is currently unimplemented and will always return an error
+func (s *KeyDBStore) ExportKey(name string) ([]byte, error) {
+	return nil, errors.New("Exporting from a KeyDBStore is not supported.")
+}
+
+// ImportKey is currently unimplemented and will always return an error
+func (s *KeyDBStore) ImportKey(pemBytes []byte, alias string) error {
+	return errors.New("Importing into a KeyDBStore is not supported")
+}
+
 // HealthCheck verifies that DB exists and is query-able
 func (s *KeyDBStore) HealthCheck() error {
 	dbPrivateKey := GormPrivateKey{}
diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go
index 51f0c4f2..6bd30ecf 100644
--- a/trustmanager/keyfilestore.go
+++ b/trustmanager/keyfilestore.go
@@ -9,6 +9,11 @@ import (
 	"github.com/endophage/gotuf/data"
 )
 
+const (
+	rootKeysSubdir    = "root_keys"
+	nonRootKeysSubdir = "tuf_keys"
+)
+
 // KeyFileStore persists and manages private keys on disk
 type KeyFileStore struct {
 	sync.Mutex
@@ -65,6 +70,22 @@ func (s *KeyFileStore) RemoveKey(name string) error {
 	return removeKey(s, s.cachedKeys, name)
 }
 
+// ExportKey exportes the encrypted bytes from the keystore and writes it to
+// dest.
+func (s *KeyFileStore) ExportKey(name string) ([]byte, error) {
+	keyBytes, _, err := getRawKey(s, name)
+	if err != nil {
+		return nil, err
+	}
+	return keyBytes, nil
+}
+
+// ImportKey imports the private key in the encrypted bytes into the keystore
+// with the given key ID and alias.
+func (s *KeyFileStore) ImportKey(pemBytes []byte, alias string) error {
+	return importKey(s, s.Retriever, s.cachedKeys, alias, pemBytes)
+}
+
 // NewKeyMemoryStore returns a new KeyMemoryStore which holds keys in memory
 func NewKeyMemoryStore(passphraseRetriever passphrase.Retriever) *KeyMemoryStore {
 	memStore := NewMemoryFileStore()
@@ -101,19 +122,33 @@ func (s *KeyMemoryStore) RemoveKey(name string) error {
 	return removeKey(s, s.cachedKeys, name)
 }
 
-func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error {
-	pemPrivKey, err := KeyToPEM(privKey)
+// ExportKey exportes the encrypted bytes from the keystore and writes it to
+// dest.
+func (s *KeyMemoryStore) ExportKey(name string) ([]byte, error) {
+	keyBytes, _, err := getRawKey(s, name)
 	if err != nil {
-		return err
+		return nil, err
 	}
+	return keyBytes, nil
+}
+
+// ImportKey imports the private key in the encrypted bytes into the keystore
+// with the given key ID and alias.
+func (s *KeyMemoryStore) ImportKey(pemBytes []byte, alias string) error {
+	return importKey(s, s.Retriever, s.cachedKeys, alias, pemBytes)
+}
 
-	attempts := 0
-	chosenPassphrase := ""
-	giveup := false
-	for {
+func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error {
+
+	var (
+		chosenPassphrase string
+		giveup           bool
+		err              error
+	)
+
+	for attempts := 0; ; attempts++ {
 		chosenPassphrase, giveup, err = passphraseRetriever(name, alias, true, attempts)
 		if err != nil {
-			attempts++
 			continue
 		}
 		if giveup {
@@ -125,19 +160,12 @@ func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached
 		break
 	}
 
-	if chosenPassphrase != "" {
-		pemPrivKey, err = EncryptPrivateKey(privKey, chosenPassphrase)
-		if err != nil {
-			return err
-		}
-	}
-
-	cachedKeys[name] = &cachedKey{alias: alias, key: privKey}
-	return s.Add(name+"_"+alias, pemPrivKey)
+	return encryptAndAddKey(s, chosenPassphrase, cachedKeys, name, alias, privKey)
 }
 
 func getKeyAlias(s LimitedFileStore, keyID string) (string, error) {
 	files := s.ListFiles()
+
 	name := strings.TrimSpace(strings.TrimSuffix(filepath.Base(keyID), filepath.Ext(keyID)))
 
 	for _, file := range files {
@@ -159,12 +187,8 @@ func getKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached
 	if ok {
 		return cachedKeyEntry.key, cachedKeyEntry.alias, nil
 	}
-	keyAlias, err := getKeyAlias(s, name)
-	if err != nil {
-		return nil, "", err
-	}
 
-	keyBytes, err := s.Get(name + "_" + keyAlias)
+	keyBytes, keyAlias, err := getRawKey(s, name)
 	if err != nil {
 		return nil, "", err
 	}
@@ -173,27 +197,7 @@ func getKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached
 	// See if the key is encrypted. If its encrypted we'll fail to parse the private key
 	privKey, err := ParsePEMPrivateKey(keyBytes, "")
 	if err != nil {
-		// We need to decrypt the key, lets get a passphrase
-		for attempts := 0; ; attempts++ {
-			passphrase, giveup, err := passphraseRetriever(name, string(keyAlias), false, attempts)
-			// Check if the passphrase retriever got an error or if it is telling us to give up
-			if giveup || err != nil {
-				return nil, "", ErrPasswordInvalid{}
-			}
-			if attempts > 10 {
-				return nil, "", ErrAttemptsExceeded{}
-			}
-
-			// Try to convert PEM encoded bytes back to a PrivateKey using the passphrase
-			privKey, err = ParsePEMPrivateKey(keyBytes, passphrase)
-			if err != nil {
-				retErr = ErrPasswordInvalid{}
-			} else {
-				// We managed to parse the PrivateKey. We've succeeded!
-				retErr = nil
-				break
-			}
-		}
+		privKey, _, retErr = getPasswdDecryptBytes(s, passphraseRetriever, keyBytes, name, string(keyAlias))
 	}
 	if retErr != nil {
 		return nil, "", retErr
@@ -208,6 +212,11 @@ func listKeys(s LimitedFileStore) map[string]string {
 	keyIDMap := make(map[string]string)
 
 	for _, f := range s.ListFiles() {
+		if f[:len(rootKeysSubdir)] == rootKeysSubdir {
+			f = strings.TrimPrefix(f, rootKeysSubdir+"/")
+		} else {
+			f = strings.TrimPrefix(f, nonRootKeysSubdir+"/")
+		}
 		keyIDFull := strings.TrimSpace(strings.TrimSuffix(f, filepath.Ext(f)))
 		keyID := keyIDFull[:strings.LastIndex(keyIDFull, "_")]
 		keyAlias := keyIDFull[strings.LastIndex(keyIDFull, "_")+1:]
@@ -225,5 +234,107 @@ func removeKey(s LimitedFileStore, cachedKeys map[string]*cachedKey, name string
 
 	delete(cachedKeys, name)
 
-	return s.Remove(name + "_" + keyAlias)
+	// being in a subdirectory is for backwards compatibliity
+	filename := name + "_" + keyAlias
+	err = s.Remove(filepath.Join(getSubdir(keyAlias), filename))
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+// Assumes 2 subdirectories, 1 containing root keys and 1 containing tuf keys
+func getSubdir(alias string) string {
+	if alias == "root" {
+		return rootKeysSubdir
+	}
+	return nonRootKeysSubdir
+}
+
+// Given a key ID, gets the bytes and alias belonging to that key if the key
+// exists
+func getRawKey(s LimitedFileStore, name string) ([]byte, string, error) {
+	keyAlias, err := getKeyAlias(s, name)
+	if err != nil {
+		return nil, "", err
+	}
+
+	filename := name + "_" + keyAlias
+	var keyBytes []byte
+	keyBytes, err = s.Get(filepath.Join(getSubdir(keyAlias), filename))
+	if err != nil {
+		return nil, "", err
+	}
+	return keyBytes, keyAlias, nil
+}
+
+// Get the password to decript the given pem bytes.  Return the password,
+// because it is useful for importing
+func getPasswdDecryptBytes(s LimitedFileStore, passphraseRetriever passphrase.Retriever, pemBytes []byte, name, alias string) (data.PrivateKey, string, error) {
+	var (
+		passwd  string
+		retErr  error
+		privKey data.PrivateKey
+	)
+	for attempts := 0; ; attempts++ {
+		var (
+			giveup bool
+			err    error
+		)
+		passwd, giveup, err = passphraseRetriever(name, alias, false, attempts)
+		// Check if the passphrase retriever got an error or if it is telling us to give up
+		if giveup || err != nil {
+			return nil, "", ErrPasswordInvalid{}
+		}
+		if attempts > 10 {
+			return nil, "", ErrAttemptsExceeded{}
+		}
+
+		// Try to convert PEM encoded bytes back to a PrivateKey using the passphrase
+		privKey, err = ParsePEMPrivateKey(pemBytes, passwd)
+		if err != nil {
+			retErr = ErrPasswordInvalid{}
+		} else {
+			// We managed to parse the PrivateKey. We've succeeded!
+			retErr = nil
+			break
+		}
+	}
+	if retErr != nil {
+		return nil, "", retErr
+	}
+	return privKey, passwd, nil
+}
+
+func encryptAndAddKey(s LimitedFileStore, passwd string, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error {
+
+	var (
+		pemPrivKey []byte
+		err        error
+	)
+
+	if passwd != "" {
+		pemPrivKey, err = EncryptPrivateKey(privKey, passwd)
+	} else {
+		pemPrivKey, err = KeyToPEM(privKey)
+	}
+
+	if err != nil {
+		return err
+	}
+
+	cachedKeys[name] = &cachedKey{alias: alias, key: privKey}
+	return s.Add(filepath.Join(getSubdir(alias), name+"_"+alias), pemPrivKey)
+}
+
+func importKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, alias string, pemBytes []byte) error {
+
+	privKey, passphrase, err := getPasswdDecryptBytes(s, passphraseRetriever, pemBytes, "imported", alias)
+
+	if err != nil {
+		return err
+	}
+
+	return encryptAndAddKey(
+		s, passphrase, cachedKeys, privKey.ID(), alias, privKey)
 }
diff --git a/trustmanager/keystore.go b/trustmanager/keystore.go
index ba5fb1a1..0eaf526e 100644
--- a/trustmanager/keystore.go
+++ b/trustmanager/keystore.go
@@ -44,6 +44,8 @@ type KeyStore interface {
 	GetKey(name string) (data.PrivateKey, string, error)
 	ListKeys() map[string]string
 	RemoveKey(name string) error
+	ExportKey(name string) ([]byte, error)
+	ImportKey(pemBytes []byte, alias string) error
 }
 
 type cachedKey struct {


Test patch:
diff --git a/client/client_root_validation_test.go b/client/client_root_validation_test.go
index 08184f7b..a5b4d8ea 100644
--- a/client/client_root_validation_test.go
+++ b/client/client_root_validation_test.go
@@ -66,7 +66,7 @@ func validateRootSuccessfully(t *testing.T, rootType data.KeyAlgorithm) {
 	var tempKey data.TUFKey
 	json.Unmarshal([]byte(timestampECDSAKeyJSON), &tempKey)
 
-	repo.KeyStoreManager.NonRootKeyStore().AddKey(filepath.Join(filepath.FromSlash(gun), tempKey.ID()), "root", &tempKey)
+	repo.KeyStoreManager.KeyStore.AddKey(filepath.Join(filepath.FromSlash(gun), tempKey.ID()), "root", &tempKey)
 
 	// Because ListTargets will clear this
 	savedTUFRepo := repo.tufRepo
diff --git a/client/client_test.go b/client/client_test.go
index b2264755..d434ac4c 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -94,9 +94,9 @@ func testInitRepo(t *testing.T, rootType data.KeyAlgorithm) {
 
 	// Look for keys in private. The filenames should match the key IDs
 	// in the private key store.
-	privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListFiles()
+	privKeyList := repo.KeyStoreManager.KeyStore.ListFiles()
 	for _, privKeyName := range privKeyList {
-		privKeyFileName := filepath.Join(repo.KeyStoreManager.NonRootKeyStore().BaseDir(), privKeyName)
+		privKeyFileName := filepath.Join(repo.KeyStoreManager.KeyStore.BaseDir(), privKeyName)
 		_, err := os.Stat(privKeyFileName)
 		assert.NoError(t, err, "missing private key: %s", privKeyName)
 	}
@@ -301,7 +301,7 @@ func testAddListTarget(t *testing.T, rootType data.KeyAlgorithm) {
 	var tempKey data.TUFKey
 	json.Unmarshal([]byte(timestampECDSAKeyJSON), &tempKey)
 
-	repo.KeyStoreManager.NonRootKeyStore().AddKey(filepath.Join(filepath.FromSlash(gun), tempKey.ID()), "nonroot", &tempKey)
+	repo.KeyStoreManager.KeyStore.AddKey(filepath.Join(filepath.FromSlash(gun), tempKey.ID()), "nonroot", &tempKey)
 
 	// Because ListTargets will clear this
 	savedTUFRepo := repo.tufRepo
diff --git a/keystoremanager/import_export_test.go b/keystoremanager/import_export_test.go
index 71a85cce..afceec09 100644
--- a/keystoremanager/import_export_test.go
+++ b/keystoremanager/import_export_test.go
@@ -83,11 +83,14 @@ func TestImportExportZip(t *testing.T) {
 
 	// Add non-root keys to the map. These should use the new passphrase
 	// because the passwords were chosen by the newPassphraseRetriever.
-	privKeyMap := repo.KeyStoreManager.NonRootKeyStore().ListKeys()
+	privKeyMap := repo.KeyStoreManager.KeyStore.ListKeys()
 	for privKeyName := range privKeyMap {
-		_, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName)
+		_, alias, err := repo.KeyStoreManager.KeyStore.GetKey(privKeyName)
 		assert.NoError(t, err, "privKey %s has no alias", privKeyName)
 
+		if alias == "root" {
+			continue
+		}
 		relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key")
 		passphraseByFile[relKeyPath] = exportPassphrase
 	}
@@ -156,9 +159,12 @@ func TestImportExportZip(t *testing.T) {
 	// Look for keys in private. The filenames should match the key IDs
 	// in the repo's private key store.
 	for privKeyName := range privKeyMap {
-		_, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName)
+		_, alias, err := repo.KeyStoreManager.KeyStore.GetKey(privKeyName)
 		assert.NoError(t, err, "privKey %s has no alias", privKeyName)
 
+		if alias == "root" {
+			continue
+		}
 		relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key")
 		privKeyFileName := filepath.Join(tempBaseDir2, relKeyPath)
 		_, err = os.Stat(privKeyFileName)
@@ -219,12 +225,15 @@ func TestImportExportGUN(t *testing.T) {
 
 	// Add keys non-root keys to the map. These should use the new passphrase
 	// because they were formerly unencrypted.
-	privKeyMap := repo.KeyStoreManager.NonRootKeyStore().ListKeys()
+	privKeyMap := repo.KeyStoreManager.KeyStore.ListKeys()
 	for privKeyName := range privKeyMap {
-		_, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName)
+		_, alias, err := repo.KeyStoreManager.KeyStore.GetKey(privKeyName)
 		if err != nil {
 			t.Fatalf("privKey %s has no alias", privKeyName)
 		}
+		if alias == "root" {
+			continue
+		}
 		relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key")
 
 		passphraseByFile[relKeyPath] = exportPassphrase
@@ -290,10 +299,13 @@ func TestImportExportGUN(t *testing.T) {
 	// Look for keys in private. The filenames should match the key IDs
 	// in the repo's private key store.
 	for privKeyName := range privKeyMap {
-		_, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName)
+		_, alias, err := repo.KeyStoreManager.KeyStore.GetKey(privKeyName)
 		if err != nil {
 			t.Fatalf("privKey %s has no alias", privKeyName)
 		}
+		if alias == "root" {
+			continue
+		}
 		relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key")
 		privKeyFileName := filepath.Join(tempBaseDir2, relKeyPath)
 		_, err = os.Stat(privKeyFileName)
@@ -381,7 +393,7 @@ func TestImportExportRootKey(t *testing.T) {
 	assert.EqualError(t, err, keystoremanager.ErrNoValidPrivateKey.Error())
 
 	// Should be able to unlock the root key with the old password
-	key, alias, err := repo2.KeyStoreManager.RootKeyStore().GetKey(rootKeyID)
+	key, alias, err := repo2.KeyStoreManager.KeyStore.GetKey(rootKeyID)
 	assert.NoError(t, err, "could not unlock root key")
 	assert.Equal(t, "root", alias)
 	assert.Equal(t, rootKeyID, key.ID())
@@ -452,7 +464,7 @@ func TestImportExportRootKeyReencrypt(t *testing.T) {
 	assert.NoError(t, err, "missing root key")
 
 	// Should be able to unlock the root key with the new password
-	key, alias, err := repo2.KeyStoreManager.RootKeyStore().GetKey(rootKeyID)
+	key, alias, err := repo2.KeyStoreManager.KeyStore.GetKey(rootKeyID)
 	assert.NoError(t, err, "could not unlock root key")
 	assert.Equal(t, "root", alias)
 	assert.Equal(t, rootKeyID, key.ID())
diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go
index 9dbaf8e1..ed1edb42 100644
--- a/trustmanager/keyfilestore_test.go
+++ b/trustmanager/keyfilestore_test.go
@@ -9,15 +9,18 @@ import (
 	"testing"
 
 	"github.com/docker/notary/pkg/passphrase"
+	"github.com/endophage/gotuf/data"
 	"github.com/stretchr/testify/assert"
 )
 
+const cannedPassphrase = "passphrase"
+
 var passphraseRetriever = func(keyID string, alias string, createNew bool, numAttempts int) (string, bool, error) {
 	if numAttempts > 5 {
 		giveup := true
 		return "", giveup, errors.New("passPhraseRetriever failed after too many requests")
 	}
-	return "passphrase", false, nil
+	return cannedPassphrase, false, nil
 }
 
 func TestAddKey(t *testing.T) {
@@ -31,7 +34,7 @@ func TestAddKey(t *testing.T) {
 	defer os.RemoveAll(tempBaseDir)
 
 	// Since we're generating this manually we need to add the extension '.'
-	expectedFilePath := filepath.Join(tempBaseDir, testName+"_"+testAlias+"."+testExt)
+	expectedFilePath := filepath.Join(tempBaseDir, rootKeysSubdir, testName+"_"+testAlias+"."+testExt)
 
 	// Create our store
 	store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever)
@@ -92,7 +95,7 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0=
 	defer os.RemoveAll(tempBaseDir)
 
 	// Since we're generating this manually we need to add the extension '.'
-	filePath := filepath.Join(tempBaseDir, testName+"_"+testAlias+"."+testExt)
+	filePath := filepath.Join(tempBaseDir, rootKeysSubdir, testName+"_"+testAlias+"."+testExt)
 
 	os.MkdirAll(filepath.Dir(filePath), perms)
 	err = ioutil.WriteFile(filePath, testData, perms)
@@ -155,7 +158,7 @@ func TestGetDecryptedWithTamperedCipherText(t *testing.T) {
 	assert.NoError(t, err, "failed to add key to store")
 
 	// Since we're generating this manually we need to add the extension '.'
-	expectedFilePath := filepath.Join(tempBaseDir, privKey.ID()+"_"+testAlias+"."+testExt)
+	expectedFilePath := filepath.Join(tempBaseDir, rootKeysSubdir, privKey.ID()+"_"+testAlias+"."+testExt)
 
 	// Get file description, open file
 	fp, err := os.OpenFile(expectedFilePath, os.O_WRONLY, 0600)
@@ -262,7 +265,7 @@ func TestRemoveKey(t *testing.T) {
 	defer os.RemoveAll(tempBaseDir)
 
 	// Since we're generating this manually we need to add the extension '.'
-	expectedFilePath := filepath.Join(tempBaseDir, testName+"_"+testAlias+"."+testExt)
+	expectedFilePath := filepath.Join(tempBaseDir, nonRootKeysSubdir, testName+"_"+testAlias+"."+testExt)
 
 	// Create our store
 	store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever)
@@ -345,3 +348,125 @@ func TestKeysAreCached(t *testing.T) {
 	}
 	assert.Equal(t, 2, numTimesCalled, "numTimesCalled should be 2 -- no additional call to passphraseRetriever")
 }
+
+// Exporting a key is successful (it is a valid key)
+func TestKeyFileStoreExportSuccess(t *testing.T) {
+	// Generate a new Private Key
+	privKey, err := GenerateECDSAKey(rand.Reader)
+	assert.NoError(t, err)
+
+	// Temporary directory where test files will be created
+	tempBaseDir, err := ioutil.TempDir("", "notary-test-")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tempBaseDir)
+
+	// Create our FileStore and add the key
+	store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever)
+	assert.NoError(t, err)
+	err = store.AddKey(privKey.ID(), "root", privKey)
+	assert.NoError(t, err)
+
+	assertExportKeySuccess(t, store, privKey)
+}
+
+// Exporting a key that doesn't exist fails (it is a valid key)
+func TestKeyFileStoreExportNonExistantFailure(t *testing.T) {
+	// Temporary directory where test files will be created
+	tempBaseDir, err := ioutil.TempDir("", "notary-test-")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tempBaseDir)
+
+	// Create empty FileStore
+	store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever)
+	assert.NoError(t, err)
+
+	_, err = store.ExportKey("12345")
+	assert.Error(t, err)
+}
+
+// Exporting a key is successful (it is a valid key)
+func TestKeyMemoryStoreExportSuccess(t *testing.T) {
+	// Generate a new Private Key
+	privKey, err := GenerateECDSAKey(rand.Reader)
+	assert.NoError(t, err)
+
+	// Create our MemoryStore and add key to it
+	store := NewKeyMemoryStore(passphraseRetriever)
+	assert.NoError(t, err)
+	err = store.AddKey(privKey.ID(), "root", privKey)
+	assert.NoError(t, err)
+
+	assertExportKeySuccess(t, store, privKey)
+}
+
+// Exporting a key that doesn't exist fails (it is a valid key)
+func TestKeyMemoryStoreExportNonExistantFailure(t *testing.T) {
+	store := NewKeyMemoryStore(passphraseRetriever)
+	_, err := store.ExportKey("12345")
+	assert.Error(t, err)
+}
+
+// Importing a key is successful
+func TestKeyFileStoreImportSuccess(t *testing.T) {
+	// Generate a new Private Key
+	privKey, err := GenerateECDSAKey(rand.Reader)
+	assert.NoError(t, err)
+
+	// Temporary directory where test files will be created
+	tempBaseDir, err := ioutil.TempDir("", "notary-test-")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tempBaseDir)
+
+	// Create our FileStore
+	store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever)
+	assert.NoError(t, err)
+
+	assertImportKeySuccess(t, store, privKey)
+}
+
+// Importing a key is successful
+func TestKeyMemoryStoreImportSuccess(t *testing.T) {
+	// Generate a new Private Key
+	privKey, err := GenerateECDSAKey(rand.Reader)
+	assert.NoError(t, err)
+
+	// Create our MemoryStore
+	store := NewKeyMemoryStore(passphraseRetriever)
+	assert.NoError(t, err)
+
+	assertImportKeySuccess(t, store, privKey)
+}
+
+// Given a keystore and expected key that is in the store, export the key
+// and assert that the exported key is the same and encrypted with the right
+// password.
+func assertExportKeySuccess(
+	t *testing.T, s KeyStore, expectedKey data.PrivateKey) {
+
+	pemBytes, err := s.ExportKey(expectedKey.ID())
+	assert.NoError(t, err)
+
+	reparsedKey, err := ParsePEMPrivateKey(pemBytes, cannedPassphrase)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedKey.Private(), reparsedKey.Private())
+	assert.Equal(t, expectedKey.Public(), reparsedKey.Public())
+}
+
+// Given a keystore and expected key, generate an encrypted PEM of the key
+// and assert that the then imported key is the same and encrypted with the
+// right password.
+func assertImportKeySuccess(
+	t *testing.T, s KeyStore, expectedKey data.PrivateKey) {
+
+	pemBytes, err := EncryptPrivateKey(expectedKey, cannedPassphrase)
+	assert.NoError(t, err)
+
+	err = s.ImportKey(pemBytes, "root")
+	assert.NoError(t, err)
+
+	reimportedKey, reimportedAlias, err := s.GetKey(expectedKey.ID())
+	assert.NoError(t, err)
+	assert.Equal(t, "root", reimportedAlias)
+	assert.Equal(t, expectedKey.Private(), reimportedKey.Private())
+	assert.Equal(t, expectedKey.Public(), reimportedKey.Public())
+}