From 1421f4725851cc7c80a5953c0b4c200741931201 Mon Sep 17 00:00:00 2001 From: Nathan McCauley Date: Mon, 20 Jul 2015 01:40:55 -0700 Subject: [PATCH 1/5] keystore caching Signed-off-by: Nathan McCauley --- trustmanager/keyfilestore.go | 30 ++++++++---- trustmanager/keyfilestore_test.go | 77 +++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index ae941088bd..6a50ebf3eb 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -29,13 +29,15 @@ type KeyStore interface { // KeyFileStore persists and manages private keys on disk type KeyFileStore struct { SimpleFileStore - PassphraseRetriever passphrase.Retriever + PassphraseRetriever + cachedKeys map[string]data.PrivateKey } // KeyMemoryStore manages private keys in memory type KeyMemoryStore struct { MemoryFileStore - PassphraseRetriever passphrase.Retriever + PassphraseRetriever + cachedKeys map[string]data.PrivateKey } // NewKeyFileStore returns a new KeyFileStore creating a private directory to @@ -45,18 +47,19 @@ func NewKeyFileStore(baseDir string, passphraseRetriever passphrase.Retriever) ( if err != nil { return nil, err } + cachedKeys := make(map[string]data.PrivateKey) - return &KeyFileStore{*fileStore, passphraseRetriever}, nil + return &KeyFileStore{*fileStore, passphraseRetriever, cachedKeys}, nil } // AddKey stores the contents of a PEM-encoded private key as a PEM block func (s *KeyFileStore) AddKey(name, alias string, privKey data.PrivateKey) error { - return addKey(s, s.PassphraseRetriever, name, alias, privKey) + return addKey(s, s.PassphraseRetriever, s.cachedKeys, name, alias, privKey) } // GetKey returns the PrivateKey given a KeyID func (s *KeyFileStore) GetKey(name string) (data.PrivateKey, error) { - return getKey(s, s.PassphraseRetriever, name) + return getKey(s, s.PassphraseRetriever, s.cachedKeys, name) } // GetKeyAlias returns the PrivateKey's alias given a KeyID @@ -79,18 +82,19 @@ func (s *KeyFileStore) RemoveKey(name string) error { // NewKeyMemoryStore returns a new KeyMemoryStore which holds keys in memory func NewKeyMemoryStore(passphraseRetriever passphrase.Retriever) *KeyMemoryStore { memStore := NewMemoryFileStore() + cachedKeys := make(map[string]data.PrivateKey) - return &KeyMemoryStore{*memStore, passphraseRetriever} + return &KeyMemoryStore{*memStore, passphraseRetriever, cachedKeys} } // AddKey stores the contents of a PEM-encoded private key as a PEM block func (s *KeyMemoryStore) AddKey(name, alias string, privKey data.PrivateKey) error { - return addKey(s, s.PassphraseRetriever, name, alias, privKey) + return addKey(s, s.PassphraseRetriever, s.cachedKeys, name, alias, privKey) } // GetKey returns the PrivateKey given a KeyID func (s *KeyMemoryStore) GetKey(name string) (data.PrivateKey, error) { - return getKey(s, s.PassphraseRetriever, name) + return getKey(s, s.PassphraseRetriever, s.cachedKeys, name) } // GetKeyAlias returns the PrivateKey's alias given a KeyID @@ -110,7 +114,7 @@ func (s *KeyMemoryStore) RemoveKey(name string) error { return removeKey(s, name) } -func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, name, alias string, privKey data.PrivateKey) error { +func addKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]data.PrivateKey, name, alias string, privKey data.PrivateKey) error { pemPrivKey, err := KeyToPEM(privKey) if err != nil { return err @@ -141,6 +145,7 @@ func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, name, } } + cachedKeys[name] = privKey return s.Add(name+"_"+alias, pemPrivKey) } @@ -162,7 +167,11 @@ func getKeyAlias(s LimitedFileStore, keyID string) (string, error) { } // GetKey returns the PrivateKey given a KeyID -func getKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, name string) (data.PrivateKey, error) { +func getKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]data.PrivateKey, name string) (data.PrivateKey, error) { + cachedKey, ok := cachedKeys[name] + if ok { + return cachedKey, nil + } keyAlias, err := getKeyAlias(s, name) if err != nil { return nil, err @@ -195,6 +204,7 @@ func getKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, name s } } } + cachedKeys[name] = privKey return privKey, nil } diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index a04f026183..941ef09154 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" "testing" + "github.com/docker/notary/Godeps/_workspace/src/github.com/stretchr/testify/assert" ) var passphraseRetriever = func(keyID string, alias string, createNew bool, numAttempts int) (string, bool, error) { @@ -364,3 +365,79 @@ func TestRemoveKey(t *testing.T) { t.Fatalf("file should not exist %s", expectedFilePath) } } + +func TestKeysAreCached(t *testing.T) { + testName := "docker.com/notary/root" + testAlias := "alias" + + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + if err != nil { + t.Fatalf("failed to create a temporary directory: %v", err) + } + defer os.RemoveAll(tempBaseDir) + + + var countingPassphraseRetriever PassphraseRetriever + + numTimesCalled := 0 + countingPassphraseRetriever = func(keyId, alias string, createNew bool, attempts int) (passphrase string, giveup bool, err error) { + numTimesCalled++ + return "password", false, nil + } + + // Create our store + store, err := NewKeyFileStore(tempBaseDir, countingPassphraseRetriever) + if err != nil { + t.Fatalf("failed to create new key filestore: %v", err) + } + + privKey, err := GenerateRSAKey(rand.Reader, 512) + if err != nil { + t.Fatalf("could not generate private key: %v", err) + } + + // Call the AddKey function + err = store.AddKey(testName, testAlias, privKey) + if err != nil { + t.Fatalf("failed to add file to store: %v", err) + } + + assert.Equal(t, 1, numTimesCalled, "numTimesCalled should have been 1") + + // Call the AddKey function + privKey2, err := store.GetKey(testName) + if err != nil { + t.Fatalf("failed to add file to store: %v", err) + } + + assert.Equal(t, privKey.Public(), privKey2.Public(), "cachedPrivKey should be the same as the added privKey") + assert.Equal(t, privKey.Private(), privKey2.Private(), "cachedPrivKey should be the same as the added privKey") + assert.Equal(t, 1, numTimesCalled, "numTimesCalled should be 1 -- no additional call to passphraseRetriever") + + + // Create a new store + store2, err := NewKeyFileStore(tempBaseDir, countingPassphraseRetriever) + if err != nil { + t.Fatalf("failed to create new key filestore: %v", err) + } + + // Call the AddKey function + privKey3, err := store2.GetKey(testName) + if err != nil { + t.Fatalf("failed to add file to store: %v", err) + } + + assert.Equal(t, privKey2.Private(), privKey3.Private(), "privkey from store1 should be the same as privkey from store2") + assert.Equal(t, privKey2.Public(), privKey3.Public(), "privkey from store1 should be the same as privkey from store2") + assert.Equal(t, 2, numTimesCalled, "numTimesCalled should be 2 -- one additional call to passphraseRetriever") + + // Call the GetKey function a bunch of times + for i := 0; i < 10; i++ { + _, err := store2.GetKey(testName) + if err != nil { + t.Fatalf("failed to add file to store: %v", err) + } + } + assert.Equal(t, 2, numTimesCalled, "numTimesCalled should be 2 -- no additional call to passphraseRetriever") +} From 1aced67471c9da6ede5162a2adf155ae12b61408 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 20 Jul 2015 12:19:38 -0700 Subject: [PATCH 2/5] Improvements to keystore caching * RemoveKey must purge the cache entry * Add mutexes to KeyFileStore and KeyMemoryStore so the cachedKeys map is protected in the case that keystore operations happen from multiple goroutines * Change GetKey to return the alias along with the key. Remove GetKeyAlias. This simplifies the code flows that retrieve the alias (since they usually get the key and alias together). * Fix tests affected by key caching Signed-off-by: Aaron Lehmann --- cryptoservice/crypto_service.go | 4 +- keystoremanager/import_export.go | 14 +---- keystoremanager/import_export_test.go | 8 +-- keystoremanager/keystoremanager.go | 2 +- trustmanager/keyfilestore.go | 89 +++++++++++++++++---------- trustmanager/keyfilestore_test.go | 56 ++++++++--------- 6 files changed, 91 insertions(+), 82 deletions(-) diff --git a/cryptoservice/crypto_service.go b/cryptoservice/crypto_service.go index 061c0541d4..3a61cb95da 100644 --- a/cryptoservice/crypto_service.go +++ b/cryptoservice/crypto_service.go @@ -68,7 +68,7 @@ func (ccs *CryptoService) Create(role string, algorithm data.KeyAlgorithm) (data // GetKey returns a key by ID func (ccs *CryptoService) GetKey(keyID string) data.PublicKey { - key, err := ccs.keyStore.GetKey(keyID) + key, _, err := ccs.keyStore.GetKey(keyID) if err != nil { return nil } @@ -92,7 +92,7 @@ func (ccs *CryptoService) Sign(keyIDs []string, payload []byte) ([]data.Signatur var privKey data.PrivateKey var err error - privKey, err = ccs.keyStore.GetKey(keyName) + privKey, _, err = ccs.keyStore.GetKey(keyName) if err != nil { // Note that GetKey always fails on InitRepo. // InitRepo gets a signer that doesn't have access to diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go index 3c42ab8263..bd87244681 100644 --- a/keystoremanager/import_export.go +++ b/keystoremanager/import_export.go @@ -81,12 +81,7 @@ func (km *KeyStoreManager) ImportRootKey(source io.Reader, keyID string) error { func moveKeys(oldKeyStore, newKeyStore *trustmanager.KeyFileStore) error { // List all files but no symlinks for _, f := range oldKeyStore.ListKeys() { - pemBytes, err := oldKeyStore.GetKey(f) - if err != nil { - return err - } - - alias, err := oldKeyStore.GetKeyAlias(f) + pemBytes, alias, err := oldKeyStore.GetKey(f) if err != nil { return err } @@ -259,12 +254,7 @@ func moveKeysByGUN(oldKeyStore, newKeyStore *trustmanager.KeyFileStore, gun stri continue } - privKey, err := oldKeyStore.GetKey(relKeyPath) - if err != nil { - return err - } - - alias, err := oldKeyStore.GetKeyAlias(relKeyPath) + privKey, alias, err := oldKeyStore.GetKey(relKeyPath) if err != nil { return err } diff --git a/keystoremanager/import_export_test.go b/keystoremanager/import_export_test.go index 47247f95c9..4a62995796 100644 --- a/keystoremanager/import_export_test.go +++ b/keystoremanager/import_export_test.go @@ -85,7 +85,7 @@ func TestImportExportZip(t *testing.T) { // because the passwords were chosen by the newPassphraseRetriever. privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListKeys() for _, privKeyName := range privKeyList { - alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKeyAlias(privKeyName) + _, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName) assert.NoError(t, err, "privKey %s has no alias", privKeyName) relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key") @@ -156,7 +156,7 @@ 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 privKeyList { - alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKeyAlias(privKeyName) + _, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName) assert.NoError(t, err, "privKey %s has no alias", privKeyName) relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key") @@ -221,7 +221,7 @@ func TestImportExportGUN(t *testing.T) { // because they were formerly unencrypted. privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListKeys() for _, privKeyName := range privKeyList { - alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKeyAlias(privKeyName) + _, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName) if err != nil { t.Fatalf("privKey %s has no alias", privKeyName) } @@ -290,7 +290,7 @@ 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 privKeyList { - alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKeyAlias(privKeyName) + _, alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKey(privKeyName) if err != nil { t.Fatalf("privKey %s has no alias", privKeyName) } diff --git a/keystoremanager/keystoremanager.go b/keystoremanager/keystoremanager.go index 6f4c1d6b20..ecc4c1bcec 100644 --- a/keystoremanager/keystoremanager.go +++ b/keystoremanager/keystoremanager.go @@ -173,7 +173,7 @@ 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.rootKeyStore.GetKey(rootKeyID) if err != nil { return nil, fmt.Errorf("could not get decrypted root key with keyID: %s, %v", rootKeyID, err) } diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 6a50ebf3eb..545345a508 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -3,6 +3,7 @@ package trustmanager import ( "path/filepath" "strings" + "sync" "errors" "fmt" @@ -20,24 +21,36 @@ type KeyStore interface { LimitedFileStore AddKey(name, alias string, privKey data.PrivateKey) error - GetKey(name string) (data.PrivateKey, error) - GetKeyAlias(name string) (string, error) + GetKey(name string) (data.PrivateKey, string, error) ListKeys() []string RemoveKey(name string) error } +type cachedKey struct { + alias string + key data.PrivateKey +} + +// PassphraseRetriever is a callback function that should retrieve a passphrase +// for a given named key. If it should be treated as new passphrase (e.g. with +// confirmation), createNew will be true. Attempts is passed in so that implementers +// decide how many chances to give to a human, for example. +type PassphraseRetriever func(keyId, alias string, createNew bool, attempts int) (passphrase string, giveup bool, err error) + // KeyFileStore persists and manages private keys on disk type KeyFileStore struct { + sync.Mutex SimpleFileStore PassphraseRetriever - cachedKeys map[string]data.PrivateKey + cachedKeys map[string]*cachedKey } // KeyMemoryStore manages private keys in memory type KeyMemoryStore struct { + sync.Mutex MemoryFileStore PassphraseRetriever - cachedKeys map[string]data.PrivateKey + cachedKeys map[string]*cachedKey } // NewKeyFileStore returns a new KeyFileStore creating a private directory to @@ -47,26 +60,27 @@ func NewKeyFileStore(baseDir string, passphraseRetriever passphrase.Retriever) ( if err != nil { return nil, err } - cachedKeys := make(map[string]data.PrivateKey) + cachedKeys := make(map[string]*cachedKey) - return &KeyFileStore{*fileStore, passphraseRetriever, cachedKeys}, nil + return &KeyFileStore{SimpleFileStore: *fileStore, + PassphraseRetriever: passphraseRetriever, + cachedKeys: cachedKeys}, nil } // AddKey stores the contents of a PEM-encoded private key as a PEM block func (s *KeyFileStore) AddKey(name, alias string, privKey data.PrivateKey) error { + s.Lock() + defer s.Unlock() return addKey(s, s.PassphraseRetriever, s.cachedKeys, name, alias, privKey) } // GetKey returns the PrivateKey given a KeyID -func (s *KeyFileStore) GetKey(name string) (data.PrivateKey, error) { +func (s *KeyFileStore) GetKey(name string) (data.PrivateKey, string, error) { + s.Lock() + defer s.Unlock() return getKey(s, s.PassphraseRetriever, s.cachedKeys, name) } -// GetKeyAlias returns the PrivateKey's alias given a KeyID -func (s *KeyFileStore) GetKeyAlias(name string) (string, error) { - return getKeyAlias(s, name) -} - // ListKeys returns a list of unique PublicKeys present on the KeyFileStore. // There might be symlinks associating Certificate IDs to Public Keys, so this // method only returns the IDs that aren't symlinks @@ -76,32 +90,35 @@ func (s *KeyFileStore) ListKeys() []string { // RemoveKey removes the key from the keyfilestore func (s *KeyFileStore) RemoveKey(name string) error { - return removeKey(s, name) + s.Lock() + defer s.Unlock() + return removeKey(s, s.cachedKeys, name) } // NewKeyMemoryStore returns a new KeyMemoryStore which holds keys in memory func NewKeyMemoryStore(passphraseRetriever passphrase.Retriever) *KeyMemoryStore { memStore := NewMemoryFileStore() - cachedKeys := make(map[string]data.PrivateKey) + cachedKeys := make(map[string]*cachedKey) - return &KeyMemoryStore{*memStore, passphraseRetriever, cachedKeys} + return &KeyMemoryStore{MemoryFileStore: *memStore, + PassphraseRetriever: passphraseRetriever, + cachedKeys: cachedKeys} } // AddKey stores the contents of a PEM-encoded private key as a PEM block func (s *KeyMemoryStore) AddKey(name, alias string, privKey data.PrivateKey) error { + s.Lock() + defer s.Unlock() return addKey(s, s.PassphraseRetriever, s.cachedKeys, name, alias, privKey) } // GetKey returns the PrivateKey given a KeyID -func (s *KeyMemoryStore) GetKey(name string) (data.PrivateKey, error) { +func (s *KeyMemoryStore) GetKey(name string) (data.PrivateKey, string, error) { + s.Lock() + defer s.Unlock() return getKey(s, s.PassphraseRetriever, s.cachedKeys, name) } -// GetKeyAlias returns the PrivateKey's alias given a KeyID -func (s *KeyMemoryStore) GetKeyAlias(name string) (string, error) { - return getKeyAlias(s, name) -} - // ListKeys returns a list of unique PublicKeys present on the KeyFileStore. // There might be symlinks associating Certificate IDs to Public Keys, so this // method only returns the IDs that aren't symlinks @@ -111,10 +128,12 @@ func (s *KeyMemoryStore) ListKeys() []string { // RemoveKey removes the key from the keystore func (s *KeyMemoryStore) RemoveKey(name string) error { - return removeKey(s, name) + s.Lock() + defer s.Unlock() + return removeKey(s, s.cachedKeys, name) } -func addKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]data.PrivateKey, name, alias string, privKey data.PrivateKey) error { +func addKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error { pemPrivKey, err := KeyToPEM(privKey) if err != nil { return err @@ -145,7 +164,7 @@ func addKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedK } } - cachedKeys[name] = privKey + cachedKeys[name] = &cachedKey{alias: alias, key: privKey} return s.Add(name+"_"+alias, pemPrivKey) } @@ -167,19 +186,19 @@ func getKeyAlias(s LimitedFileStore, keyID string) (string, error) { } // GetKey returns the PrivateKey given a KeyID -func getKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]data.PrivateKey, name string) (data.PrivateKey, error) { - cachedKey, ok := cachedKeys[name] +func getKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]*cachedKey, name string) (data.PrivateKey, string, error) { + cachedKeyEntry, ok := cachedKeys[name] if ok { - return cachedKey, nil + return cachedKeyEntry.key, cachedKeyEntry.alias, nil } keyAlias, err := getKeyAlias(s, name) if err != nil { - return nil, err + return nil, "", err } keyBytes, err := s.Get(name + "_" + keyAlias) if err != nil { - return nil, err + return nil, "", err } // See if the key is encrypted. If its encrypted we'll fail to parse the private key @@ -190,10 +209,10 @@ func getKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedK 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, errors.New("obtaining passphrase failed") + return nil, "", errors.New("obtaining passphrase failed") } if attempts > 10 { - return nil, errors.New("maximum number of passphrase attempts exceeded") + return nil, "", errors.New("maximum number of passphrase attempts exceeded") } // Try to convert PEM encoded bytes back to a PrivateKey using the passphrase @@ -204,8 +223,8 @@ func getKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedK } } } - cachedKeys[name] = privKey - return privKey, nil + cachedKeys[name] = &cachedKey{alias: keyAlias, key: privKey} + return privKey, keyAlias, nil } // ListKeys returns a list of unique PublicKeys present on the KeyFileStore. @@ -223,11 +242,13 @@ func listKeys(s LimitedFileStore) []string { } // RemoveKey removes the key from the keyfilestore -func removeKey(s LimitedFileStore, name string) error { +func removeKey(s LimitedFileStore, cachedKeys map[string]*cachedKey, name string) error { keyAlias, err := getKeyAlias(s, name) if err != nil { return err } + delete(cachedKeys, name) + return s.Remove(name + "_" + keyAlias) } diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 941ef09154..9eae4e96ca 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -4,12 +4,12 @@ import ( "bytes" "crypto/rand" "errors" + "github.com/docker/notary/Godeps/_workspace/src/github.com/stretchr/testify/assert" "io/ioutil" "os" "path/filepath" "strings" "testing" - "github.com/docker/notary/Godeps/_workspace/src/github.com/stretchr/testify/assert" ) var passphraseRetriever = func(keyID string, alias string, createNew bool, numAttempts int) (string, bool, error) { @@ -121,7 +121,7 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0= } // Call the GetKey function - privKey, err := store.GetKey(testName) + privKey, _, err := store.GetKey(testName) if err != nil { t.Fatalf("failed to get file from store: %v", err) } @@ -155,13 +155,7 @@ func TestAddGetKeyMemStore(t *testing.T) { } // Check to see if file exists - retrievedKey, err := store.GetKey(testName) - if err != nil { - t.Fatalf("failed to get key from store: %v", err) - } - - // Check to see if alias exists - retrievedAlias, err := store.GetKeyAlias(testName) + retrievedKey, retrievedAlias, err := store.GetKey(testName) if err != nil { t.Fatalf("failed to get key from store: %v", err) } @@ -216,8 +210,11 @@ func TestGetDecryptedWithTamperedCipherText(t *testing.T) { // Tamper the file fp.WriteAt([]byte("a"), int64(1)) + // Recreate the KeyFileStore to avoid caching + store, err = NewKeyFileStore(tempBaseDir, passphraseRetriever) + // Try to decrypt the file - _, err = store.GetKey(privKey.ID()) + _, _, err = store.GetKey(privKey.ID()) if err == nil { t.Fatalf("expected error while decrypting the content due to invalid cipher text") } @@ -250,15 +247,15 @@ func TestGetDecryptedWithInvalidPassphrase(t *testing.T) { t.Fatalf("failed to create new key filestore: %v", err) } - testGetDecryptedWithInvalidPassphrase(t, fileStore) - - // Test with KeyMemoryStore - memStore := NewKeyMemoryStore(invalidPassphraseRetriever) + newFileStore, err := NewKeyFileStore(tempBaseDir, invalidPassphraseRetriever) if err != nil { - t.Fatalf("failed to create new key memorystore: %v", err) + t.Fatalf("failed to create new key filestore: %v", err) } - testGetDecryptedWithInvalidPassphrase(t, memStore) + testGetDecryptedWithInvalidPassphrase(t, fileStore, newFileStore) + + // Can't test with KeyMemoryStore because we cache the decrypted version of + // the key forever } func TestGetDecryptedWithConsistentlyInvalidPassphrase(t *testing.T) { @@ -283,17 +280,20 @@ func TestGetDecryptedWithConsistentlyInvalidPassphrase(t *testing.T) { t.Fatalf("failed to create new key filestore: %v", err) } - testGetDecryptedWithInvalidPassphrase(t, fileStore) - - // Test with KeyMemoryStore - memStore := NewKeyMemoryStore(consistentlyInvalidPassphraseRetriever) + newFileStore, err := NewKeyFileStore(tempBaseDir, consistentlyInvalidPassphraseRetriever) if err != nil { - t.Fatalf("failed to create new key memorystore: %v", err) + t.Fatalf("failed to create new key filestore: %v", err) } - testGetDecryptedWithInvalidPassphrase(t, memStore) + + testGetDecryptedWithInvalidPassphrase(t, fileStore, newFileStore) + + // Can't test with KeyMemoryStore because we cache the decrypted version of + // the key forever } -func testGetDecryptedWithInvalidPassphrase(t *testing.T, store KeyStore) { +// testGetDecryptedWithInvalidPassphrase takes two keystores so it can add to +// one and get from the other (to work around caching) +func testGetDecryptedWithInvalidPassphrase(t *testing.T, store KeyStore, newStore KeyStore) { testAlias := "root" // Generate a new random RSA Key @@ -309,7 +309,7 @@ func testGetDecryptedWithInvalidPassphrase(t *testing.T, store KeyStore) { } // Try to decrypt the file with an invalid passphrase - _, err = store.GetKey(privKey.ID()) + _, _, err = newStore.GetKey(privKey.ID()) if err == nil { t.Fatalf("expected error while decrypting the content due to invalid passphrase") } @@ -377,7 +377,6 @@ func TestKeysAreCached(t *testing.T) { } defer os.RemoveAll(tempBaseDir) - var countingPassphraseRetriever PassphraseRetriever numTimesCalled := 0 @@ -406,7 +405,7 @@ func TestKeysAreCached(t *testing.T) { assert.Equal(t, 1, numTimesCalled, "numTimesCalled should have been 1") // Call the AddKey function - privKey2, err := store.GetKey(testName) + privKey2, _, err := store.GetKey(testName) if err != nil { t.Fatalf("failed to add file to store: %v", err) } @@ -415,7 +414,6 @@ func TestKeysAreCached(t *testing.T) { assert.Equal(t, privKey.Private(), privKey2.Private(), "cachedPrivKey should be the same as the added privKey") assert.Equal(t, 1, numTimesCalled, "numTimesCalled should be 1 -- no additional call to passphraseRetriever") - // Create a new store store2, err := NewKeyFileStore(tempBaseDir, countingPassphraseRetriever) if err != nil { @@ -423,7 +421,7 @@ func TestKeysAreCached(t *testing.T) { } // Call the AddKey function - privKey3, err := store2.GetKey(testName) + privKey3, _, err := store2.GetKey(testName) if err != nil { t.Fatalf("failed to add file to store: %v", err) } @@ -434,7 +432,7 @@ func TestKeysAreCached(t *testing.T) { // Call the GetKey function a bunch of times for i := 0; i < 10; i++ { - _, err := store2.GetKey(testName) + _, _, err := store2.GetKey(testName) if err != nil { t.Fatalf("failed to add file to store: %v", err) } From 42ded6231cadf565f76fa353182acecbe82792ef Mon Sep 17 00:00:00 2001 From: Diogo Monica Date: Mon, 20 Jul 2015 13:29:26 -0700 Subject: [PATCH 3/5] Converted tests to testify and EC generation Signed-off-by: Diogo Monica --- client/client_root_validation_test.go | 5 +- trustmanager/keyfilestore_test.go | 214 +++++++------------------- 2 files changed, 63 insertions(+), 156 deletions(-) diff --git a/client/client_root_validation_test.go b/client/client_root_validation_test.go index 68612fb863..8aa3511d4c 100644 --- a/client/client_root_validation_test.go +++ b/client/client_root_validation_test.go @@ -36,7 +36,10 @@ const signedRSARootTemplate = `{"signed":{"_type":"Root","consistent_snapshot":f // We test this with both an RSA and ECDSA root key func TestValidateRoot(t *testing.T) { logrus.SetLevel(logrus.DebugLevel) - validateRootSuccessfully(t, data.RSAKey) + validateRootSuccessfully(t, data.ECDSAKey) + if !testing.Short() { + validateRootSuccessfully(t, data.RSAKey) + } } func validateRootSuccessfully(t *testing.T, rootType data.KeyAlgorithm) { diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 9eae4e96ca..4312edc285 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -1,15 +1,14 @@ package trustmanager import ( - "bytes" "crypto/rand" "errors" - "github.com/docker/notary/Godeps/_workspace/src/github.com/stretchr/testify/assert" "io/ioutil" "os" "path/filepath" - "strings" "testing" + + "github.com/docker/notary/Godeps/_workspace/src/github.com/stretchr/testify/assert" ) var passphraseRetriever = func(keyID string, alias string, createNew bool, numAttempts int) (string, bool, error) { @@ -27,9 +26,7 @@ func TestAddKey(t *testing.T) { // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") - if err != nil { - t.Fatalf("failed to create a temporary directory: %v", err) - } + assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) // Since we're generating this manually we need to add the extension '.' @@ -37,30 +34,19 @@ func TestAddKey(t *testing.T) { // Create our store store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") - privKey, err := GenerateRSAKey(rand.Reader, 512) - if err != nil { - t.Fatalf("could not generate private key: %v", err) - } + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err, "could not generate private key") // Call the AddKey function err = store.AddKey(testName, "root", privKey) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to add key to store") // Check to see if file exists b, err := ioutil.ReadFile(expectedFilePath) - if err != nil { - t.Fatalf("expected file not found: %v", err) - } - - if !strings.Contains(string(b), "-----BEGIN RSA PRIVATE KEY-----") { - t.Fatalf("expected private key content in the file: %s", expectedFilePath) - } + assert.NoError(t, err, "expected file not found") + assert.Contains(t, string(b), "-----BEGIN EC PRIVATE KEY-----") } func TestGet(t *testing.T) { @@ -101,39 +87,27 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0= // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") - if err != nil { - t.Fatalf("failed to create a temporary directory: %v", err) - } + assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) // Since we're generating this manually we need to add the extension '.' filePath := filepath.Join(tempBaseDir, testName+"_"+testAlias+"."+testExt) os.MkdirAll(filepath.Dir(filePath), perms) - if err = ioutil.WriteFile(filePath, testData, perms); err != nil { - t.Fatalf("Failed to write test file: %v", err) - } + err = ioutil.WriteFile(filePath, testData, perms) + assert.NoError(t, err, "failed to write test file") // Create our store store, err := NewKeyFileStore(tempBaseDir, emptyPassphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") // Call the GetKey function privKey, _, err := store.GetKey(testName) - if err != nil { - t.Fatalf("failed to get file from store: %v", err) - } + assert.NoError(t, err, "failed to get key from store") pemPrivKey, err := KeyToPEM(privKey) - if err != nil { - t.Fatalf("failed to convert key to PEM: %v", err) - } - - if !bytes.Equal(testData, pemPrivKey) { - t.Fatalf("unexpected content in the file: %s", filePath) - } + assert.NoError(t, err, "failed to convert key to PEM") + assert.Equal(t, testData, pemPrivKey) } func TestAddGetKeyMemStore(t *testing.T) { @@ -143,31 +117,20 @@ func TestAddGetKeyMemStore(t *testing.T) { // Create our store store := NewKeyMemoryStore(passphraseRetriever) - privKey, err := GenerateRSAKey(rand.Reader, 512) - if err != nil { - t.Fatalf("could not generate private key: %v", err) - } + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err, "could not generate private key") // Call the AddKey function err = store.AddKey(testName, testAlias, privKey) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to add key to store") // Check to see if file exists retrievedKey, retrievedAlias, err := store.GetKey(testName) - if err != nil { - t.Fatalf("failed to get key from store: %v", err) - } + assert.NoError(t, err, "failed to get key from store") - if retrievedAlias != testAlias { - t.Fatalf("retrievedAlias differs getAlias") - } - - if !bytes.Equal(retrievedKey.Public(), privKey.Public()) || - !bytes.Equal(retrievedKey.Private(), privKey.Private()) { - t.Fatalf("key contents differs after add/get") - } + assert.Equal(t, retrievedAlias, testAlias) + assert.Equal(t, retrievedKey.Public(), privKey.Public()) + assert.Equal(t, retrievedKey.Private(), privKey.Private()) } func TestGetDecryptedWithTamperedCipherText(t *testing.T) { testExt := "key" @@ -175,49 +138,38 @@ func TestGetDecryptedWithTamperedCipherText(t *testing.T) { // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") - if err != nil { - t.Fatalf("failed to create a temporary directory: %v", err) - } + assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) // Create our FileStore store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") // Generate a new Private Key - privKey, err := GenerateRSAKey(rand.Reader, 512) - if err != nil { - t.Fatalf("could not generate private key: %v", err) - } + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err, "could not generate private key") // Call the AddEncryptedKey function err = store.AddKey(privKey.ID(), testAlias, privKey) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + 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) // Get file description, open file fp, err := os.OpenFile(expectedFilePath, os.O_WRONLY, 0600) - if err != nil { - t.Fatalf("expected file not found: %v", err) - } + assert.NoError(t, err, "expected file not found") // Tamper the file fp.WriteAt([]byte("a"), int64(1)) // Recreate the KeyFileStore to avoid caching store, err = NewKeyFileStore(tempBaseDir, passphraseRetriever) + assert.NoError(t, err, "failed to create new key filestore") // Try to decrypt the file _, _, err = store.GetKey(privKey.ID()) - if err == nil { - t.Fatalf("expected error while decrypting the content due to invalid cipher text") - } + assert.Error(t, err, "expected error while decrypting the content due to invalid cipher text") } func TestGetDecryptedWithInvalidPassphrase(t *testing.T) { @@ -236,21 +188,15 @@ func TestGetDecryptedWithInvalidPassphrase(t *testing.T) { // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") - if err != nil { - t.Fatalf("failed to create a temporary directory: %v", err) - } + assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) // Test with KeyFileStore fileStore, err := NewKeyFileStore(tempBaseDir, invalidPassphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") newFileStore, err := NewKeyFileStore(tempBaseDir, invalidPassphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") testGetDecryptedWithInvalidPassphrase(t, fileStore, newFileStore) @@ -269,21 +215,15 @@ func TestGetDecryptedWithConsistentlyInvalidPassphrase(t *testing.T) { // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") - if err != nil { - t.Fatalf("failed to create a temporary directory: %v", err) - } + assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) // Test with KeyFileStore fileStore, err := NewKeyFileStore(tempBaseDir, consistentlyInvalidPassphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") newFileStore, err := NewKeyFileStore(tempBaseDir, consistentlyInvalidPassphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") testGetDecryptedWithInvalidPassphrase(t, fileStore, newFileStore) @@ -297,22 +237,16 @@ func testGetDecryptedWithInvalidPassphrase(t *testing.T, store KeyStore, newStor testAlias := "root" // Generate a new random RSA Key - privKey, err := GenerateRSAKey(rand.Reader, 512) - if err != nil { - t.Fatalf("could not generate private key: %v", err) - } + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err, "could not generate private key") // Call the AddKey function err = store.AddKey(privKey.ID(), testAlias, privKey) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to add key to store") // Try to decrypt the file with an invalid passphrase _, _, err = newStore.GetKey(privKey.ID()) - if err == nil { - t.Fatalf("expected error while decrypting the content due to invalid passphrase") - } + assert.Error(t, err, "expected error while decrypting the content due to invalid passphrase") } func TestRemoveKey(t *testing.T) { @@ -322,9 +256,7 @@ func TestRemoveKey(t *testing.T) { // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") - if err != nil { - t.Fatalf("failed to create a temporary directory: %v", err) - } + assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) // Since we're generating this manually we need to add the extension '.' @@ -332,38 +264,26 @@ func TestRemoveKey(t *testing.T) { // Create our store store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") - privKey, err := GenerateRSAKey(rand.Reader, 512) - if err != nil { - t.Fatalf("could not generate private key: %v", err) - } + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err, "could not generate private key") // Call the AddKey function err = store.AddKey(testName, testAlias, privKey) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to add key to store") // Check to see if file exists _, err = ioutil.ReadFile(expectedFilePath) - if err != nil { - t.Fatalf("expected file not found: %v", err) - } + assert.NoError(t, err, "expected file not found") // Call remove key err = store.RemoveKey(testName) - if err != nil { - t.Fatalf("unable to remove key: %v", err) - } + assert.NoError(t, err, "unable to remove key") // Check to see if file still exists _, err = ioutil.ReadFile(expectedFilePath) - if err == nil { - t.Fatalf("file should not exist %s", expectedFilePath) - } + assert.Error(t, err, "file should not exist") } func TestKeysAreCached(t *testing.T) { @@ -372,9 +292,7 @@ func TestKeysAreCached(t *testing.T) { // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") - if err != nil { - t.Fatalf("failed to create a temporary directory: %v", err) - } + assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) var countingPassphraseRetriever PassphraseRetriever @@ -387,28 +305,20 @@ func TestKeysAreCached(t *testing.T) { // Create our store store, err := NewKeyFileStore(tempBaseDir, countingPassphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") - privKey, err := GenerateRSAKey(rand.Reader, 512) - if err != nil { - t.Fatalf("could not generate private key: %v", err) - } + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err, "could not generate private key") // Call the AddKey function err = store.AddKey(testName, testAlias, privKey) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to add key to store") assert.Equal(t, 1, numTimesCalled, "numTimesCalled should have been 1") // Call the AddKey function privKey2, _, err := store.GetKey(testName) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to add key to store") assert.Equal(t, privKey.Public(), privKey2.Public(), "cachedPrivKey should be the same as the added privKey") assert.Equal(t, privKey.Private(), privKey2.Private(), "cachedPrivKey should be the same as the added privKey") @@ -416,15 +326,11 @@ func TestKeysAreCached(t *testing.T) { // Create a new store store2, err := NewKeyFileStore(tempBaseDir, countingPassphraseRetriever) - if err != nil { - t.Fatalf("failed to create new key filestore: %v", err) - } + assert.NoError(t, err, "failed to create new key filestore") - // Call the AddKey function + // Call the GetKey function privKey3, _, err := store2.GetKey(testName) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to get key from store") assert.Equal(t, privKey2.Private(), privKey3.Private(), "privkey from store1 should be the same as privkey from store2") assert.Equal(t, privKey2.Public(), privKey3.Public(), "privkey from store1 should be the same as privkey from store2") @@ -433,9 +339,7 @@ func TestKeysAreCached(t *testing.T) { // Call the GetKey function a bunch of times for i := 0; i < 10; i++ { _, _, err := store2.GetKey(testName) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err, "failed to get key from store") } assert.Equal(t, 2, numTimesCalled, "numTimesCalled should be 2 -- no additional call to passphraseRetriever") } From 4dfe45d64eb77cd005712c1a530218baba8dba9b Mon Sep 17 00:00:00 2001 From: Diogo Monica Date: Mon, 20 Jul 2015 13:32:06 -0700 Subject: [PATCH 4/5] Changing testify import Signed-off-by: Diogo Monica --- trustmanager/keyfilestore_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 4312edc285..65795d4467 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -8,7 +8,7 @@ import ( "path/filepath" "testing" - "github.com/docker/notary/Godeps/_workspace/src/github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" ) var passphraseRetriever = func(keyID string, alias string, createNew bool, numAttempts int) (string, bool, error) { From f7ea67cfabc63bbcbf1bce2e44bb15189e604404 Mon Sep 17 00:00:00 2001 From: Diogo Monica Date: Mon, 20 Jul 2015 13:46:01 -0700 Subject: [PATCH 5/5] Rebased from master Signed-off-by: Diogo Monica --- trustmanager/keyfilestore.go | 24 ++++++++++++------------ trustmanager/keyfilestore_test.go | 3 ++- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 545345a508..f92396508b 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -41,7 +41,7 @@ type PassphraseRetriever func(keyId, alias string, createNew bool, attempts int) type KeyFileStore struct { sync.Mutex SimpleFileStore - PassphraseRetriever + passphrase.Retriever cachedKeys map[string]*cachedKey } @@ -49,7 +49,7 @@ type KeyFileStore struct { type KeyMemoryStore struct { sync.Mutex MemoryFileStore - PassphraseRetriever + passphrase.Retriever cachedKeys map[string]*cachedKey } @@ -63,22 +63,22 @@ func NewKeyFileStore(baseDir string, passphraseRetriever passphrase.Retriever) ( cachedKeys := make(map[string]*cachedKey) return &KeyFileStore{SimpleFileStore: *fileStore, - PassphraseRetriever: passphraseRetriever, - cachedKeys: cachedKeys}, nil + Retriever: passphraseRetriever, + cachedKeys: cachedKeys}, nil } // AddKey stores the contents of a PEM-encoded private key as a PEM block func (s *KeyFileStore) AddKey(name, alias string, privKey data.PrivateKey) error { s.Lock() defer s.Unlock() - return addKey(s, s.PassphraseRetriever, s.cachedKeys, name, alias, privKey) + return addKey(s, s.Retriever, s.cachedKeys, name, alias, privKey) } // GetKey returns the PrivateKey given a KeyID func (s *KeyFileStore) GetKey(name string) (data.PrivateKey, string, error) { s.Lock() defer s.Unlock() - return getKey(s, s.PassphraseRetriever, s.cachedKeys, name) + return getKey(s, s.Retriever, s.cachedKeys, name) } // ListKeys returns a list of unique PublicKeys present on the KeyFileStore. @@ -101,22 +101,22 @@ func NewKeyMemoryStore(passphraseRetriever passphrase.Retriever) *KeyMemoryStore cachedKeys := make(map[string]*cachedKey) return &KeyMemoryStore{MemoryFileStore: *memStore, - PassphraseRetriever: passphraseRetriever, - cachedKeys: cachedKeys} + Retriever: passphraseRetriever, + cachedKeys: cachedKeys} } // AddKey stores the contents of a PEM-encoded private key as a PEM block func (s *KeyMemoryStore) AddKey(name, alias string, privKey data.PrivateKey) error { s.Lock() defer s.Unlock() - return addKey(s, s.PassphraseRetriever, s.cachedKeys, name, alias, privKey) + return addKey(s, s.Retriever, s.cachedKeys, name, alias, privKey) } // GetKey returns the PrivateKey given a KeyID func (s *KeyMemoryStore) GetKey(name string) (data.PrivateKey, string, error) { s.Lock() defer s.Unlock() - return getKey(s, s.PassphraseRetriever, s.cachedKeys, name) + return getKey(s, s.Retriever, s.cachedKeys, name) } // ListKeys returns a list of unique PublicKeys present on the KeyFileStore. @@ -133,7 +133,7 @@ func (s *KeyMemoryStore) RemoveKey(name string) error { return removeKey(s, s.cachedKeys, name) } -func addKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error { +func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error { pemPrivKey, err := KeyToPEM(privKey) if err != nil { return err @@ -186,7 +186,7 @@ func getKeyAlias(s LimitedFileStore, keyID string) (string, error) { } // GetKey returns the PrivateKey given a KeyID -func getKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, cachedKeys map[string]*cachedKey, name string) (data.PrivateKey, string, error) { +func getKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, name string) (data.PrivateKey, string, error) { cachedKeyEntry, ok := cachedKeys[name] if ok { return cachedKeyEntry.key, cachedKeyEntry.alias, nil diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 65795d4467..1205282c1c 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "testing" + "github.com/docker/notary/pkg/passphrase" "github.com/stretchr/testify/assert" ) @@ -295,7 +296,7 @@ func TestKeysAreCached(t *testing.T) { assert.NoError(t, err, "failed to create a temporary directory") defer os.RemoveAll(tempBaseDir) - var countingPassphraseRetriever PassphraseRetriever + var countingPassphraseRetriever passphrase.Retriever numTimesCalled := 0 countingPassphraseRetriever = func(keyId, alias string, createNew bool, attempts int) (passphrase string, giveup bool, err error) {