From 8628b57a963a6e90485a615a14a6f05906464fdd Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 11 Nov 2015 15:48:12 -0800 Subject: [PATCH] private subdir should be added by keyfilestore, rather than all over the place Signed-off-by: David Lawrence (github: endophage) --- client/repo.go | 3 +-- client/repo_pkcs11.go | 6 ++--- cmd/notary/cert.go | 12 +++------ cmd/notary/keys.go | 6 ++--- const.go | 2 -- cryptoservice/import_export.go | 35 ++++++++----------------- cryptoservice/import_export_test.go | 30 ++++++++++----------- keystoremanager/keystoremanager_test.go | 8 ++---- trustmanager/keyfilestore.go | 2 ++ trustmanager/keyfilestore_test.go | 10 +++---- 10 files changed, 44 insertions(+), 70 deletions(-) diff --git a/client/repo.go b/client/repo.go index 97cba741af..43f4ed78ce 100644 --- a/client/repo.go +++ b/client/repo.go @@ -20,8 +20,7 @@ import ( // (usually ~/.docker/trust/). func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper, retriever passphrase.Retriever) (*NotaryRepository, error) { - keysPath := filepath.Join(baseDir, notary.PrivDir) - fileKeyStore, err := trustmanager.NewKeyFileStore(keysPath, retriever) + fileKeyStore, err := trustmanager.NewKeyFileStore(baseDir, retriever) if err != nil { return nil, fmt.Errorf("failed to create private key store in directory: %s", keysPath) } diff --git a/client/repo_pkcs11.go b/client/repo_pkcs11.go index ed82bd555b..933accc439 100644 --- a/client/repo_pkcs11.go +++ b/client/repo_pkcs11.go @@ -7,7 +7,6 @@ import ( "net/http" "path/filepath" - "github.com/docker/notary" "github.com/docker/notary/cryptoservice" "github.com/docker/notary/keystoremanager" "github.com/docker/notary/passphrase" @@ -22,10 +21,9 @@ import ( func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper, retriever passphrase.Retriever) (*NotaryRepository, error) { - keysPath := filepath.Join(baseDir, notary.PrivDir) - fileKeyStore, err := trustmanager.NewKeyFileStore(keysPath, retriever) + fileKeyStore, err := trustmanager.NewKeyFileStore(baseDir, retriever) if err != nil { - return nil, fmt.Errorf("failed to create private key store in directory: %s", keysPath) + return nil, fmt.Errorf("failed to create private key store in directory: %s", baseDir) } keyStoreManager, err := keystoremanager.NewKeyStoreManager(baseDir, fileKeyStore) diff --git a/cmd/notary/cert.go b/cmd/notary/cert.go index 95ae44e03a..a21e6979f3 100644 --- a/cmd/notary/cert.go +++ b/cmd/notary/cert.go @@ -4,10 +4,8 @@ import ( "crypto/x509" "math" "os" - "path/filepath" "time" - "github.com/docker/notary" "github.com/docker/notary/keystoremanager" "github.com/docker/notary/trustmanager" @@ -56,10 +54,9 @@ func certRemove(cmd *cobra.Command, args []string) { parseConfig() trustDir := mainViper.GetString("trust_dir") - keysPath := filepath.Join(trustDir, notary.PrivDir) - fileKeyStore, err := trustmanager.NewKeyFileStore(keysPath, retriever) + fileKeyStore, err := trustmanager.NewKeyFileStore(trustDir, retriever) if err != nil { - fatalf("Failed to create private key store in directory: %s", keysPath) + fatalf("Failed to create private key store in directory: %s", trustDir) } keyStoreManager, err := keystoremanager.NewKeyStoreManager(trustDir, fileKeyStore) if err != nil { @@ -125,10 +122,9 @@ func certList(cmd *cobra.Command, args []string) { parseConfig() trustDir := mainViper.GetString("trust_dir") - keysPath := filepath.Join(trustDir, notary.PrivDir) - fileKeyStore, err := trustmanager.NewKeyFileStore(keysPath, retriever) + fileKeyStore, err := trustmanager.NewKeyFileStore(trustDir, retriever) if err != nil { - fatalf("Failed to create private key store in directory: %s", keysPath) + fatalf("Failed to create private key store in directory: %s", trustDir) } keyStoreManager, err := keystoremanager.NewKeyStoreManager(trustDir, fileKeyStore) if err != nil { diff --git a/cmd/notary/keys.go b/cmd/notary/keys.go index e8d2554c00..b3e378c200 100644 --- a/cmd/notary/keys.go +++ b/cmd/notary/keys.go @@ -7,7 +7,6 @@ import ( "sort" "strings" - "github.com/docker/notary" notaryclient "github.com/docker/notary/client" "github.com/docker/notary/cryptoservice" "github.com/docker/notary/passphrase" @@ -341,10 +340,9 @@ func keysRotate(cmd *cobra.Command, args []string) { func getKeyStores(cmd *cobra.Command, directory string, ret passphrase.Retriever, withHardware bool) []trustmanager.KeyStore { - keysPath := filepath.Join(directory, notary.PrivDir) - fileKeyStore, err := trustmanager.NewKeyFileStore(keysPath, ret) + fileKeyStore, err := trustmanager.NewKeyFileStore(directory, ret) if err != nil { - fatalf("Failed to create private key store in directory: %s", keysPath) + fatalf("Failed to create private key store in directory: %s", directory) } ks := []trustmanager.KeyStore{fileKeyStore} diff --git a/const.go b/const.go index 41d80de393..3c22c05024 100644 --- a/const.go +++ b/const.go @@ -2,8 +2,6 @@ package notary // application wide constants const ( - BackupDir = "backup" - PrivDir = "private" PrivKeyPerms = 0700 PubCertPerms = 0755 ) diff --git a/cryptoservice/import_export.go b/cryptoservice/import_export.go index 66405b22e3..4f5bd4ca82 100644 --- a/cryptoservice/import_export.go +++ b/cryptoservice/import_export.go @@ -11,7 +11,6 @@ import ( "path/filepath" "strings" - "github.com/docker/notary" "github.com/docker/notary/passphrase" "github.com/docker/notary/trustmanager" ) @@ -72,8 +71,7 @@ func (cs *CryptoService) ExportRootKeyReencrypt(dest io.Writer, keyID string, ne tempBaseDir, err := ioutil.TempDir("", "notary-key-export-") defer os.RemoveAll(tempBaseDir) - tempKeysPath := filepath.Join(tempBaseDir, notary.PrivDir) - tempKeyStore, err := trustmanager.NewKeyFileStore(tempKeysPath, newPassphraseRetriever) + tempKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, newPassphraseRetriever) if err != nil { return err } @@ -127,8 +125,7 @@ func (cs *CryptoService) ExportAllKeys(dest io.Writer, newPassphraseRetriever pa defer os.RemoveAll(tempBaseDir) // Create temporary keystore to use as a staging area - tempKeysPath := filepath.Join(tempBaseDir, notary.PrivDir) - tempKeyStore, err := trustmanager.NewKeyFileStore(tempKeysPath, newPassphraseRetriever) + tempKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, newPassphraseRetriever) if err != nil { return err } @@ -141,7 +138,7 @@ func (cs *CryptoService) ExportAllKeys(dest io.Writer, newPassphraseRetriever pa zipWriter := zip.NewWriter(dest) - if err := addKeysToArchive(zipWriter, tempKeyStore, notary.PrivDir); err != nil { + if err := addKeysToArchive(zipWriter, tempKeyStore); err != nil { return err } @@ -176,21 +173,12 @@ func (cs *CryptoService) ImportKeysZip(zipReader zip.Reader) error { // Note that using / as a separator is okay here - the zip // package guarantees that the separator will be / - if strings.HasPrefix(fNameTrimmed, notary.PrivDir) { - if fNameTrimmed[len(fNameTrimmed)-5:] == "_root" { - if err = checkRootKeyIsEncrypted(fileBytes); err != nil { - return err - } + if fNameTrimmed[len(fNameTrimmed)-5:] == "_root" { + if err = checkRootKeyIsEncrypted(fileBytes); err != nil { + return err } - keyName := strings.TrimPrefix(fNameTrimmed, notary.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. - continue } + newKeys[fNameTrimmed] = fileBytes } for keyName, pemBytes := range newKeys { @@ -224,8 +212,7 @@ func (cs *CryptoService) ExportKeysByGUN(dest io.Writer, gun string, passphraseR defer os.RemoveAll(tempBaseDir) // Create temporary keystore to use as a staging area - tempKeysPath := filepath.Join(tempBaseDir, notary.PrivDir) - tempKeyStore, err := trustmanager.NewKeyFileStore(tempKeysPath, passphraseRetriever) + tempKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever) if err != nil { return err } @@ -242,7 +229,7 @@ func (cs *CryptoService) ExportKeysByGUN(dest io.Writer, gun string, passphraseR return ErrNoKeysFoundForGUN } - if err := addKeysToArchive(zipWriter, tempKeyStore, notary.PrivDir); err != nil { + if err := addKeysToArchive(zipWriter, tempKeyStore); err != nil { return err } @@ -289,7 +276,7 @@ func moveKeys(oldKeyStore, newKeyStore trustmanager.KeyStore) error { return nil } -func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileStore, subDir string) error { +func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileStore) error { for _, relKeyPath := range newKeyStore.ListFiles() { fullKeyPath := filepath.Join(newKeyStore.BaseDir(), relKeyPath) @@ -303,7 +290,7 @@ func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileSt return err } - infoHeader.Name = filepath.Join(subDir, relKeyPath) + infoHeader.Name = relKeyPath zipFileEntryWriter, err := zipWriter.CreateHeader(infoHeader) if err != nil { diff --git a/cryptoservice/import_export_test.go b/cryptoservice/import_export_test.go index 6b9ced61b4..e6e762581f 100644 --- a/cryptoservice/import_export_test.go +++ b/cryptoservice/import_export_test.go @@ -48,7 +48,7 @@ func TestImportExportZip(t *testing.T) { defer os.RemoveAll(tempBaseDir) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir, "private"), newPassphraseRetriever) + fileStore, err := trustmanager.NewKeyFileStore(tempBaseDir, newPassphraseRetriever) cs := NewCryptoService(gun, fileStore) pubKey, err := cs.Create(data.CanonicalRootRole, data.ECDSAKey) assert.NoError(t, err) @@ -80,13 +80,13 @@ func TestImportExportZip(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key") + relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") passphraseByFile[relKeyPath] = exportPassphrase } // Add root key to the map. This will use the export passphrase because it // will be reencrypted. - relRootKey := filepath.Join("private", "root_keys", rootKeyID+"_root.key") + relRootKey := filepath.Join("root_keys", rootKeyID+"_root.key") passphraseByFile[relRootKey] = exportPassphrase // Iterate through the files in the archive, checking that the files @@ -123,7 +123,7 @@ func TestImportExportZip(t *testing.T) { defer os.RemoveAll(tempBaseDir2) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore2, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir2, "private"), newPassphraseRetriever) + fileStore2, err := trustmanager.NewKeyFileStore(tempBaseDir2, newPassphraseRetriever) assert.NoError(t, err) cs2 := NewCryptoService(gun, fileStore2) @@ -145,8 +145,8 @@ func TestImportExportZip(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key") - privKeyFileName := filepath.Join(tempBaseDir2, relKeyPath) + relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") + privKeyFileName := filepath.Join(tempBaseDir2, "private", relKeyPath) _, err = os.Stat(privKeyFileName) assert.NoError(t, err, "missing private key for role %s: %s", alias, privKeyName) } @@ -167,7 +167,7 @@ func TestImportExportGUN(t *testing.T) { defer os.RemoveAll(tempBaseDir) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir, "private"), newPassphraseRetriever) + fileStore, err := trustmanager.NewKeyFileStore(tempBaseDir, newPassphraseRetriever) cs := NewCryptoService(gun, fileStore) _, err = cs.Create(data.CanonicalRootRole, data.ECDSAKey) _, err = cs.Create(data.CanonicalTargetsRole, data.ECDSAKey) @@ -205,7 +205,7 @@ func TestImportExportGUN(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key") + relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") passphraseByFile[relKeyPath] = exportPassphrase } @@ -245,7 +245,7 @@ func TestImportExportGUN(t *testing.T) { defer os.RemoveAll(tempBaseDir2) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore2, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir2, "private"), newPassphraseRetriever) + fileStore2, err := trustmanager.NewKeyFileStore(tempBaseDir2, newPassphraseRetriever) cs2 := NewCryptoService(gun, fileStore2) // Reopen the zip file for importing @@ -270,8 +270,8 @@ func TestImportExportGUN(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key") - privKeyFileName := filepath.Join(tempBaseDir2, relKeyPath) + relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") + privKeyFileName := filepath.Join(tempBaseDir2, "private", relKeyPath) _, err = os.Stat(privKeyFileName) assert.NoError(t, err) } @@ -285,7 +285,7 @@ func TestImportExportRootKey(t *testing.T) { defer os.RemoveAll(tempBaseDir) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir, "private"), oldPassphraseRetriever) + fileStore, err := trustmanager.NewKeyFileStore(tempBaseDir, oldPassphraseRetriever) cs := NewCryptoService(gun, fileStore) pubKey, err := cs.Create(data.CanonicalRootRole, data.ECDSAKey) assert.NoError(t, err) @@ -305,7 +305,7 @@ func TestImportExportRootKey(t *testing.T) { defer os.RemoveAll(tempBaseDir2) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore2, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir2, "private"), oldPassphraseRetriever) + fileStore2, err := trustmanager.NewKeyFileStore(tempBaseDir2, oldPassphraseRetriever) cs2 := NewCryptoService(gun, fileStore2) keyReader, err := os.Open(tempKeyFilePath) @@ -353,7 +353,7 @@ func TestImportExportRootKeyReencrypt(t *testing.T) { defer os.RemoveAll(tempBaseDir) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir, "private"), oldPassphraseRetriever) + fileStore, err := trustmanager.NewKeyFileStore(tempBaseDir, oldPassphraseRetriever) cs := NewCryptoService(gun, fileStore) pubKey, err := cs.Create(data.CanonicalRootRole, data.ECDSAKey) assert.NoError(t, err) @@ -373,7 +373,7 @@ func TestImportExportRootKeyReencrypt(t *testing.T) { defer os.RemoveAll(tempBaseDir2) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - fileStore2, err := trustmanager.NewKeyFileStore(filepath.Join(tempBaseDir2, "private"), newPassphraseRetriever) + fileStore2, err := trustmanager.NewKeyFileStore(tempBaseDir2, newPassphraseRetriever) cs2 := NewCryptoService(gun, fileStore2) keyReader, err := os.Open(tempKeyFilePath) diff --git a/keystoremanager/keystoremanager_test.go b/keystoremanager/keystoremanager_test.go index 1c5d3e316f..9304b5971c 100644 --- a/keystoremanager/keystoremanager_test.go +++ b/keystoremanager/keystoremanager_test.go @@ -6,11 +6,9 @@ import ( "encoding/json" "io/ioutil" "os" - "path/filepath" "testing" "text/template" - "github.com/docker/notary" "github.com/docker/notary/cryptoservice" "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" @@ -123,8 +121,7 @@ func TestValidateRoot(t *testing.T) { defer os.RemoveAll(tempBaseDir) assert.NoError(t, err, "failed to create a temporary directory: %s", err) - keysPath := filepath.Join(tempBaseDir, notary.PrivDir) - fileKeyStore, err := trustmanager.NewKeyFileStore(keysPath, passphraseRetriever) + fileKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever) assert.NoError(t, err) // Create a FileStoreManager @@ -235,8 +232,7 @@ func filestoreWithTwoCerts(t *testing.T, gun, keyAlg string) ( tempBaseDir, err := ioutil.TempDir("", "notary-test-") assert.NoError(t, err, "failed to create a temporary directory: %s", err) - keysPath := filepath.Join(tempBaseDir, notary.PrivDir) - fileKeyStore, err := trustmanager.NewKeyFileStore(keysPath, passphraseRetriever) + fileKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever) assert.NoError(t, err) // Create a FileStoreManager diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index b8148c58ca..58edaea1fd 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -13,6 +13,7 @@ import ( const ( rootKeysSubdir = "root_keys" nonRootKeysSubdir = "tuf_keys" + privDir = "private" ) // KeyFileStore persists and manages private keys on disk @@ -34,6 +35,7 @@ type KeyMemoryStore struct { // NewKeyFileStore returns a new KeyFileStore creating a private directory to // hold the keys. func NewKeyFileStore(baseDir string, passphraseRetriever passphrase.Retriever) (*KeyFileStore, error) { + baseDir = filepath.Join(baseDir, privDir) fileStore, err := NewPrivateSimpleFileStore(baseDir, keyExtension) if err != nil { return nil, err diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 16e39f31f5..dabfb98e62 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -34,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, rootKeysSubdir, testName+"_"+testAlias+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir, testName+"_"+testAlias+"."+testExt) // Create our store store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever) @@ -95,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, rootKeysSubdir, testName+"_"+testAlias+"."+testExt) + filePath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir, testName+"_"+testAlias+"."+testExt) os.MkdirAll(filepath.Dir(filePath), perms) err = ioutil.WriteFile(filePath, testData, perms) @@ -159,7 +159,7 @@ func TestListKeys(t *testing.T) { assert.Equal(t, role, "targets") // Write an invalid filename to the directory - filePath := filepath.Join(tempBaseDir, rootKeysSubdir, "fakekeyname.key") + filePath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir, "fakekeyname.key") err = ioutil.WriteFile(filePath, []byte("data"), perms) assert.NoError(t, err, "failed to write test file") @@ -213,7 +213,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, rootKeysSubdir, privKey.ID()+"_"+testAlias+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir, privKey.ID()+"_"+testAlias+"."+testExt) // Get file description, open file fp, err := os.OpenFile(expectedFilePath, os.O_WRONLY, 0600) @@ -320,7 +320,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, nonRootKeysSubdir, testName+"_"+testAlias+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, privDir, nonRootKeysSubdir, testName+"_"+testAlias+"."+testExt) // Create our store store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever)