From 566bd3ce67a42af061fe89086c03b943f657b65c Mon Sep 17 00:00:00 2001 From: Ying Li Date: Tue, 27 Oct 2015 12:32:02 -0700 Subject: [PATCH] Combine the nonRootKeyStore with the rootKeyStore, and move the abstracting over the root keys directory from non-root keys directory from keystoremanager to keystore, since we're eliminating keystoremanager. Maintain the two separate directories, though, because one can't tell whether there is an old-style separate-directories structure, or if someone has a GUN that starts with tuf_keys. Signed-off-by: Ying Li --- client/client.go | 2 +- client/client_root_validation_test.go | 2 +- client/client_test.go | 6 ++--- cmd/notary/keys.go | 29 +++++++++------------ cmd/notary/tuf.go | 7 +++++- keystoremanager/keystoremanager.go | 36 ++++++--------------------- trustmanager/keyfilestore.go | 32 +++++++++++++++++++++--- trustmanager/keyfilestore_test.go | 8 +++--- 8 files changed, 63 insertions(+), 59 deletions(-) diff --git a/client/client.go b/client/client.go index 2d8643d073..b7ae899e78 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/client/client_root_validation_test.go b/client/client_root_validation_test.go index 08184f7b6a..a5b4d8ea5b 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 b226475550..d434ac4c7c 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/cmd/notary/keys.go b/cmd/notary/keys.go index 0002fa4cd8..1beab11fd1 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 b3cd58e348..5bc0db1829 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() + var keysMap 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/keystoremanager.go b/keystoremanager/keystoremanager.go index bf39d7faa5..097436bbdd 100644 --- a/keystoremanager/keystoremanager.go +++ b/keystoremanager/keystoremanager.go @@ -20,9 +20,7 @@ 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 } @@ -62,15 +60,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 +93,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 +143,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 +151,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/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 51f0c4f2d5..1ed7fcf324 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 @@ -133,11 +138,12 @@ func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached } cachedKeys[name] = &cachedKey{alias: alias, key: privKey} - return s.Add(name+"_"+alias, pemPrivKey) + return s.Add(filepath.Join(getSubdir(alias), name+"_"+alias), pemPrivKey) } 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 { @@ -164,7 +170,9 @@ func getKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached return nil, "", err } - keyBytes, err := s.Get(name + "_" + keyAlias) + filename := name + "_" + keyAlias + var keyBytes []byte + keyBytes, err = s.Get(filepath.Join(getSubdir(keyAlias), filename)) if err != nil { return nil, "", err } @@ -208,6 +216,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 +238,18 @@ 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 +} + +func getSubdir(alias string) string { + if alias == "root" { + return rootKeysSubdir + } + return nonRootKeysSubdir } diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 9dbaf8e1bc..ff92aef41e 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -31,7 +31,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 +92,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 +155,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 +262,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)