From 878a8a083d31bc588d2a68b97e0d3a18edab3abd Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 14 Jul 2015 17:34:00 -0700 Subject: [PATCH 1/7] Add ExportAllKeys function This allows all keys to be exported to a zip file. Keys that were already encrypted are kept as-is, and keys that weren't encrypted are encrypted with the specified passphrase. Also add a unit test that creates the zip file and checks the expected keys all exist, and are all encrypted with the expected passphrase. Signed-off-by: Aaron Lehmann --- keystoremanager/export_test.go | 116 ++++++++++++++++++++++++ keystoremanager/import_export.go | 147 +++++++++++++++++++++++++++++++ trustmanager/filestore.go | 6 ++ 3 files changed, 269 insertions(+) create mode 100644 keystoremanager/export_test.go create mode 100644 keystoremanager/import_export.go diff --git a/keystoremanager/export_test.go b/keystoremanager/export_test.go new file mode 100644 index 0000000000..be72f06078 --- /dev/null +++ b/keystoremanager/export_test.go @@ -0,0 +1,116 @@ +package keystoremanager_test + +import ( + "archive/zip" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/docker/notary/client" + "github.com/docker/notary/trustmanager" + "github.com/endophage/gotuf/data" + "github.com/stretchr/testify/assert" +) + +const timestampECDSAKeyJSON = ` +{"keytype":"ecdsa","keyval":{"public":"MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEgl3rzMPMEKhS1k/AX16MM4PdidpjJr+z4pj0Td+30QnpbOIARgpyR1PiFztU8BZlqG3cUazvFclr2q/xHvfrqw==","private":"MHcCAQEEIDqtcdzU7H3AbIPSQaxHl9+xYECt7NpK7B1+6ep5cv9CoAoGCCqGSM49AwEHoUQDQgAEgl3rzMPMEKhS1k/AX16MM4PdidpjJr+z4pj0Td+30QnpbOIARgpyR1PiFztU8BZlqG3cUazvFclr2q/xHvfrqw=="}}` + +func createTestServer(t *testing.T) (*httptest.Server, *http.ServeMux) { + mux := http.NewServeMux() + // TUF will request /v2/docker.com/notary/_trust/tuf/timestamp.key + // Return a canned timestamp.key + mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/timestamp.key", func(w http.ResponseWriter, r *http.Request) { + // Also contains the private key, but for the purpose of this + // test, we don't care + fmt.Fprint(w, timestampECDSAKeyJSON) + }) + + ts := httptest.NewServer(mux) + + return ts, mux +} + +func TestExportKeys(t *testing.T) { + gun := "docker.com/notary" + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + defer os.RemoveAll(tempBaseDir) + + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + + ts, _ := createTestServer(t) + defer ts.Close() + + repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport) + assert.NoError(t, err, "error creating repo: %s", err) + + rootKeyID, err := repo.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), "oldPassphrase") + assert.NoError(t, err, "error generating root key: %s", err) + + rootCryptoService, err := repo.KeyStoreManager.GetRootCryptoService(rootKeyID, "oldPassphrase") + assert.NoError(t, err, "error retrieving root key: %s", err) + + err = repo.Initialize(rootCryptoService) + assert.NoError(t, err, "error creating repository: %s", err) + + tempZipFile, err := ioutil.TempFile("", "notary-test-export-") + tempZipFilePath := tempZipFile.Name() + defer os.Remove(tempZipFilePath) + + err = repo.KeyStoreManager.ExportAllKeys(tempZipFile, "exportPassphrase") + tempZipFile.Close() + assert.NoError(t, err) + + // Check the contents of the zip file + zipReader, err := zip.OpenReader(tempZipFilePath) + assert.NoError(t, err, "could not open zip file") + defer zipReader.Close() + + // Map of files to expect in the zip file, with the passphrases + passphraseByFile := make(map[string]string) + + // Add keys in private to the map. These should use the new passphrase + // because they were formerly unencrypted. + privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListFiles(false) + for _, privKeyName := range privKeyList { + relName := strings.TrimPrefix(privKeyName, tempBaseDir+string(filepath.Separator)) + passphraseByFile[relName] = "exportPassphrase" + } + + // Add root key to the map. This will use the old passphrase because it + // won't be reencrypted. + relRootKey := filepath.Join("private", "root_keys", rootCryptoService.ID()+".key") + passphraseByFile[relRootKey] = "oldPassphrase" + + // Iterate through the files in the archive, checking that the files + // exist and are encrypted with the expected passphrase. + for _, f := range zipReader.File { + expectedPassphrase, present := passphraseByFile[f.Name] + if !present { + t.Fatalf("unexpected file %s in zip file", f.Name) + } + + delete(passphraseByFile, f.Name) + + rc, err := f.Open() + assert.NoError(t, err, "could not open file inside zip archive") + + pemBytes, err := ioutil.ReadAll(rc) + assert.NoError(t, err, "could not read file from zip") + + _, err = trustmanager.ParsePEMPrivateKey(pemBytes, expectedPassphrase) + assert.NoError(t, err, "PEM not encrypted with the expected passphrase") + + rc.Close() + } + + // Are there any keys that didn't make it to the zip? + for fileNotFound, _ := range passphraseByFile { + t.Fatalf("%s not found in zip", fileNotFound) + } +} diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go new file mode 100644 index 0000000000..c46c2704c0 --- /dev/null +++ b/keystoremanager/import_export.go @@ -0,0 +1,147 @@ +package keystoremanager + +import ( + "archive/zip" + "crypto/x509" + "encoding/pem" + "errors" + "io" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/docker/notary/trustmanager" +) + +func moveKeysWithNewPassphrase(oldKeyStore, newKeyStore *trustmanager.KeyFileStore, outputPassphrase string) error { + // List all files but no symlinks + for _, f := range oldKeyStore.ListFiles(false) { + fullKeyPath := strings.TrimSpace(strings.TrimSuffix(f, filepath.Ext(f))) + relKeyPath := strings.TrimPrefix(fullKeyPath, oldKeyStore.BaseDir()) + relKeyPath = strings.TrimPrefix(relKeyPath, string(filepath.Separator)) + + pemBytes, err := oldKeyStore.Get(relKeyPath) + if err != nil { + return err + } + + block, _ := pem.Decode(pemBytes) + if block == nil { + return errors.New("no valid private key found") + } + + if !x509.IsEncryptedPEMBlock(block) { + // Key is not encrypted. Parse it, and add it + // to the temporary store as an encrypted key. + privKey, err := trustmanager.ParsePEMPrivateKey(pemBytes, "") + if err != nil { + return err + } + err = newKeyStore.AddEncryptedKey(relKeyPath, privKey, outputPassphrase) + } else { + // Encrypted key - pass it through without + // decrypting + err = newKeyStore.Add(relKeyPath, pemBytes) + } + + if err != nil { + return err + } + } + + return nil +} + +func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileStore, tempBaseDir string, dedup map[string]struct{}) error { + // List all files but no symlinks + for _, fullKeyPath := range newKeyStore.ListFiles(false) { + if _, present := dedup[fullKeyPath]; present { + continue + } + dedup[fullKeyPath] = struct{}{} + + relKeyPath := strings.TrimPrefix(fullKeyPath, tempBaseDir) + relKeyPath = strings.TrimPrefix(relKeyPath, string(filepath.Separator)) + + fi, err := os.Stat(fullKeyPath) + if err != nil { + return err + } + + infoHeader, err := zip.FileInfoHeader(fi) + if err != nil { + return err + } + + infoHeader.Name = relKeyPath + zipFileEntryWriter, err := zipWriter.CreateHeader(infoHeader) + if err != nil { + return err + } + + fileContents, err := ioutil.ReadFile(fullKeyPath) + if err != nil { + return err + } + if _, err = zipFileEntryWriter.Write(fileContents); err != nil { + return err + } + } + + return nil +} + +// ExportAllKeys exports all keys to an io.Writer in zip format. +// outputPassphrase is the new passphrase to use to encrypt the existing keys. +// If blank, the keys will not be encrypted. Note that keys which are already +// encrypted are not re-encrypted. They will be included in the zip with their +// original encryption. +func (km *KeyStoreManager) ExportAllKeys(dest io.Writer, outputPassphrase string) error { + tempBaseDir, err := ioutil.TempDir("", "notary-key-export-") + defer os.RemoveAll(tempBaseDir) + + // Create temporary keystores to use as a staging area + tempNonRootKeysPath := filepath.Join(tempBaseDir, privDir) + tempNonRootKeyStore, err := trustmanager.NewKeyFileStore(tempNonRootKeysPath) + if err != nil { + return err + } + + tempRootKeysPath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir) + tempRootKeyStore, err := trustmanager.NewKeyFileStore(tempRootKeysPath) + if err != nil { + return err + } + + if err := moveKeysWithNewPassphrase(km.rootKeyStore, tempRootKeyStore, outputPassphrase); err != nil { + return err + } + if err := moveKeysWithNewPassphrase(km.nonRootKeyStore, tempNonRootKeyStore, outputPassphrase); err != nil { + return err + } + + zipWriter := zip.NewWriter(dest) + + // Root and non-root stores overlap, so we need to dedup files + dedup := make(map[string]struct{}) + + if err := addKeysToArchive(zipWriter, tempRootKeyStore, tempBaseDir, dedup); err != nil { + return err + } + if err := addKeysToArchive(zipWriter, tempNonRootKeyStore, tempBaseDir, dedup); err != nil { + return err + } + + zipWriter.Close() + + return nil +} + +// ImportZip imports keys from a zip file provided as an io.Reader. The keys +// in the root_keys directory are left encrypted, but the other keys are +// decrypted with the specified passphrase. +func (km *KeyStoreManager) ImportZip(zip io.Reader, passphrase string) error { + // TODO(aaronl) + return nil +} diff --git a/trustmanager/filestore.go b/trustmanager/filestore.go index 3701ebece6..8c57020124 100644 --- a/trustmanager/filestore.go +++ b/trustmanager/filestore.go @@ -20,6 +20,7 @@ type FileStore interface { ListFiles(symlinks bool) []string ListDir(directoryName string, symlinks bool) []string Link(src, dst string) error + BaseDir() string } // SimpleFileStore implements FileStore @@ -165,6 +166,11 @@ func (f *SimpleFileStore) Link(oldname, newname string) error { ) } +// BaseDir returns the base directory of the filestore +func (f *SimpleFileStore) BaseDir() string { + return f.baseDir +} + // CreateDirectory uses createDirectory to create a chmod 755 Directory func CreateDirectory(dir string) error { return createDirectory(dir, visible) From eb8a7a0e2519e6cfde6300a574353dab70cb5b0a Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 15 Jul 2015 11:02:55 -0700 Subject: [PATCH 2/7] Add ImportKeysZip and test coverage This function reads a zip file and populates the keystores with the keys in the zip file. Root keys are left encrypted, and non-root keys are decrypted before being added to the keystore. The unit test first exports a repo's keys to a zip file, then imports it into another repo. It checks that all the correct keys exist in the new repo after the import operation. Signed-off-by: Aaron Lehmann --- keystoremanager/import_export.go | 75 ++++++++++++++++++- .../{export_test.go => import_export_test.go} | 75 +++++++++++++++++-- 2 files changed, 138 insertions(+), 12 deletions(-) rename keystoremanager/{export_test.go => import_export_test.go} (58%) diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go index c46c2704c0..b36fdf112b 100644 --- a/keystoremanager/import_export.go +++ b/keystoremanager/import_export.go @@ -11,7 +11,9 @@ import ( "path/filepath" "strings" + "github.com/Sirupsen/logrus" "github.com/docker/notary/trustmanager" + "github.com/endophage/gotuf/data" ) func moveKeysWithNewPassphrase(oldKeyStore, newKeyStore *trustmanager.KeyFileStore, outputPassphrase string) error { @@ -138,10 +140,75 @@ func (km *KeyStoreManager) ExportAllKeys(dest io.Writer, outputPassphrase string return nil } -// ImportZip imports keys from a zip file provided as an io.Reader. The keys -// in the root_keys directory are left encrypted, but the other keys are +// ImportKeysZip imports keys from a zip file provided as an io.ReaderAt. The +// keys in the root_keys directory are left encrypted, but the other keys are // decrypted with the specified passphrase. -func (km *KeyStoreManager) ImportZip(zip io.Reader, passphrase string) error { - // TODO(aaronl) +func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader, passphrase string) 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]*data.PrivateKey) + + // Iterate through the files in the archive. Don't add the keys + for _, f := range zipReader.File { + fNameTrimmed := strings.TrimSuffix(f.Name, filepath.Ext(f.Name)) + // Note that using / as a separator is okay here - the zip + // package guarantees that the separator will be / + keysPrefix := privDir + "/" + + if !strings.HasPrefix(fNameTrimmed, keysPrefix) { + // This path inside the zip archive doesn't start with + // "private". That's unexpected, because all keys + // should be in that subdirectory. 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: %s", keysPrefix, f.Name) + continue + } + fNameTrimmed = strings.TrimPrefix(fNameTrimmed, keysPrefix) + + rc, err := f.Open() + if err != nil { + return err + } + + pemBytes, err := ioutil.ReadAll(rc) + if err != nil { + 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 / + rootKeysPrefix := rootKeysSubdir + "/" + if strings.HasPrefix(fNameTrimmed, rootKeysPrefix) { + // Root keys are preserved without decrypting + keyName := strings.TrimPrefix(fNameTrimmed, rootKeysPrefix) + newRootKeys[keyName] = pemBytes + } else { + // Non-root keys need to be decrypted + key, err := trustmanager.ParsePEMPrivateKey(pemBytes, passphrase) + if err != nil { + return err + } + newNonRootKeys[fNameTrimmed] = key + } + + rc.Close() + } + + for keyName, pemBytes := range newRootKeys { + if err := km.rootKeyStore.Add(keyName, pemBytes); err != nil { + return err + } + } + + for keyName, privKey := range newNonRootKeys { + if err := km.nonRootKeyStore.AddKey(keyName, privKey); err != nil { + return err + } + } + return nil } diff --git a/keystoremanager/export_test.go b/keystoremanager/import_export_test.go similarity index 58% rename from keystoremanager/export_test.go rename to keystoremanager/import_export_test.go index be72f06078..d270098205 100644 --- a/keystoremanager/export_test.go +++ b/keystoremanager/import_export_test.go @@ -35,8 +35,11 @@ func createTestServer(t *testing.T) (*httptest.Server, *http.ServeMux) { return ts, mux } -func TestExportKeys(t *testing.T) { +func TestImportExportZip(t *testing.T) { gun := "docker.com/notary" + oldPassphrase := "oldPassphrase" + exportPassphrase := "exportPassphrase" + // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") defer os.RemoveAll(tempBaseDir) @@ -49,10 +52,10 @@ func TestExportKeys(t *testing.T) { repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport) assert.NoError(t, err, "error creating repo: %s", err) - rootKeyID, err := repo.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), "oldPassphrase") + rootKeyID, err := repo.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), oldPassphrase) assert.NoError(t, err, "error generating root key: %s", err) - rootCryptoService, err := repo.KeyStoreManager.GetRootCryptoService(rootKeyID, "oldPassphrase") + rootCryptoService, err := repo.KeyStoreManager.GetRootCryptoService(rootKeyID, oldPassphrase) assert.NoError(t, err, "error retrieving root key: %s", err) err = repo.Initialize(rootCryptoService) @@ -62,14 +65,13 @@ func TestExportKeys(t *testing.T) { tempZipFilePath := tempZipFile.Name() defer os.Remove(tempZipFilePath) - err = repo.KeyStoreManager.ExportAllKeys(tempZipFile, "exportPassphrase") + err = repo.KeyStoreManager.ExportAllKeys(tempZipFile, exportPassphrase) tempZipFile.Close() assert.NoError(t, err) // Check the contents of the zip file zipReader, err := zip.OpenReader(tempZipFilePath) assert.NoError(t, err, "could not open zip file") - defer zipReader.Close() // Map of files to expect in the zip file, with the passphrases passphraseByFile := make(map[string]string) @@ -79,13 +81,13 @@ func TestExportKeys(t *testing.T) { privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListFiles(false) for _, privKeyName := range privKeyList { relName := strings.TrimPrefix(privKeyName, tempBaseDir+string(filepath.Separator)) - passphraseByFile[relName] = "exportPassphrase" + passphraseByFile[relName] = exportPassphrase } // Add root key to the map. This will use the old passphrase because it // won't be reencrypted. relRootKey := filepath.Join("private", "root_keys", rootCryptoService.ID()+".key") - passphraseByFile[relRootKey] = "oldPassphrase" + passphraseByFile[relRootKey] = oldPassphrase // Iterate through the files in the archive, checking that the files // exist and are encrypted with the expected passphrase. @@ -109,8 +111,65 @@ func TestExportKeys(t *testing.T) { rc.Close() } + zipReader.Close() + // Are there any keys that didn't make it to the zip? - for fileNotFound, _ := range passphraseByFile { + for fileNotFound := range passphraseByFile { t.Fatalf("%s not found in zip", fileNotFound) } + + // Create new repo to test import + tempBaseDir2, err := ioutil.TempDir("", "notary-test-") + defer os.RemoveAll(tempBaseDir2) + + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + + repo2, err := client.NewNotaryRepository(tempBaseDir2, gun, ts.URL, http.DefaultTransport) + assert.NoError(t, err, "error creating repo: %s", err) + + rootKeyID2, err := repo2.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), "oldPassphrase") + assert.NoError(t, err, "error generating root key: %s", err) + + rootCryptoService2, err := repo2.KeyStoreManager.GetRootCryptoService(rootKeyID2, "oldPassphrase") + assert.NoError(t, err, "error retrieving root key: %s", err) + + err = repo2.Initialize(rootCryptoService2) + assert.NoError(t, err, "error creating repository: %s", err) + + // Reopen the zip file for importing + zipReader, err = zip.OpenReader(tempZipFilePath) + assert.NoError(t, err, "could not open zip file") + + // First try with an incorrect passphrase + err = repo2.KeyStoreManager.ImportKeysZip(zipReader.Reader, "wrongpassphrase") + // Don't use EqualError here because occasionally decrypting with the + // wrong passphrase returns a parse error + assert.Error(t, err) + zipReader.Close() + + // Reopen the zip file for importing + zipReader, err = zip.OpenReader(tempZipFilePath) + assert.NoError(t, err, "could not open zip file") + + // Now try with a valid passphrase. This time it should succeed. + err = repo2.KeyStoreManager.ImportKeysZip(zipReader.Reader, exportPassphrase) + assert.NoError(t, err) + zipReader.Close() + + // Look for repo's keys in repo2 + + // Look for keys in private. The filenames should match the key IDs + // in the repo's private key store. + for _, privKeyName := range privKeyList { + privKeyRel := strings.TrimPrefix(privKeyName, tempBaseDir) + _, err := os.Stat(filepath.Join(tempBaseDir2, privKeyRel)) + assert.NoError(t, err, "missing private key: %s", privKeyName) + } + + // Look for keys in root_keys + // There should be a file named after the key ID of the root key we + // passed in. + rootKeyFilename := rootCryptoService.ID() + ".key" + _, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename)) + assert.NoError(t, err, "missing root key") } From 20633e3e12568b74dabaa26113787b1f7ad3dbd6 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 15 Jul 2015 11:49:02 -0700 Subject: [PATCH 3/7] Make FileStore only allow operations on files inside the store Paths that abuse .. shouldn't be able to escape from the filestore. This is especially important when importing keys from zip files that could have "creative" paths encoded in the zip. Add test coverage for this protection. Signed-off-by: Aaron Lehmann --- trustmanager/filestore.go | 59 ++++++++++++++++++++--------- trustmanager/filestore_test.go | 69 +++++++++++++++++++++++++++++++++- trustmanager/x509filestore.go | 6 ++- 3 files changed, 113 insertions(+), 21 deletions(-) diff --git a/trustmanager/filestore.go b/trustmanager/filestore.go index 8c57020124..8bc4a2fdd7 100644 --- a/trustmanager/filestore.go +++ b/trustmanager/filestore.go @@ -1,14 +1,24 @@ package trustmanager import ( + "errors" "fmt" "io/ioutil" "os" "path/filepath" + "strings" ) -const visible os.FileMode = 0755 -const private os.FileMode = 0700 +const ( + visible os.FileMode = 0755 + private os.FileMode = 0700 +) + +var ( + // ErrPathOutsideStore indicates that the returned path would be + // outside the store + ErrPathOutsideStore = errors.New("path outside file store") +) // FileStore is the interface for all FileStores type FileStore interface { @@ -16,7 +26,7 @@ type FileStore interface { Remove(fileName string) error RemoveDir(directoryName string) error Get(fileName string) ([]byte, error) - GetPath(fileName string) string + GetPath(fileName string) (string, error) ListFiles(symlinks bool) []string ListDir(directoryName string, symlinks bool) []string Link(src, dst string) error @@ -32,6 +42,8 @@ type SimpleFileStore struct { // NewSimpleFileStore creates a directory with 755 permissions func NewSimpleFileStore(baseDir string, fileExt string) (FileStore, error) { + baseDir = filepath.Clean(baseDir) + if err := CreateDirectory(baseDir); err != nil { return nil, err } @@ -58,7 +70,10 @@ func NewPrivateSimpleFileStore(baseDir string, fileExt string) (FileStore, error // Add writes data to a file with a given name func (f *SimpleFileStore) Add(name string, data []byte) error { - filePath := f.genFilePath(name) + filePath, err := f.GetPath(name) + if err != nil { + return err + } createDirectory(filepath.Dir(filePath), f.perms) return ioutil.WriteFile(filePath, data, f.perms) } @@ -66,7 +81,10 @@ func (f *SimpleFileStore) Add(name string, data []byte) error { // Remove removes a file identified by name func (f *SimpleFileStore) Remove(name string) error { // Attempt to remove - filePath := f.genFilePath(name) + filePath, err := f.GetPath(name) + if err != nil { + return err + } return os.Remove(filePath) } @@ -90,7 +108,10 @@ func (f *SimpleFileStore) RemoveDir(name string) error { // Get returns the data given a file name func (f *SimpleFileStore) Get(name string) ([]byte, error) { - filePath := f.genFilePath(name) + filePath, err := f.GetPath(name) + if err != nil { + return nil, err + } data, err := ioutil.ReadFile(filePath) if err != nil { return nil, err @@ -100,8 +121,14 @@ func (f *SimpleFileStore) Get(name string) ([]byte, error) { } // GetPath returns the full final path of a file with a given name -func (f *SimpleFileStore) GetPath(name string) string { - return f.genFilePath(name) +func (f *SimpleFileStore) GetPath(name string) (string, error) { + fileName := f.genFileName(name) + fullPath := filepath.Clean(filepath.Join(f.baseDir, fileName)) + + if !strings.HasPrefix(fullPath, f.baseDir) { + return "", ErrPathOutsideStore + } + return fullPath, nil } // ListFiles lists all the files inside of a store @@ -144,12 +171,6 @@ func (f *SimpleFileStore) list(path string, symlinks bool) []string { return files } -// genFilePath returns the full path with extension given a file name -func (f *SimpleFileStore) genFilePath(name string) string { - fileName := f.genFileName(name) - return filepath.Join(f.baseDir, fileName) -} - // genFileName returns the name using the right extension func (f *SimpleFileStore) genFileName(name string) string { return fmt.Sprintf("%s.%s", name, f.fileExt) @@ -160,10 +181,12 @@ func (f *SimpleFileStore) genFileName(name string) string { // We use full path for the source and local for the destination to use relative // path for the symlink func (f *SimpleFileStore) Link(oldname, newname string) error { - return os.Symlink( - f.genFileName(oldname), - f.genFilePath(newname), - ) + newnamePath, err := f.GetPath(newname) + if err != nil { + return err + } + + return os.Symlink(f.genFileName(oldname), newnamePath) } // BaseDir returns the base directory of the filestore diff --git a/trustmanager/filestore_test.go b/trustmanager/filestore_test.go index 9dc20486b4..797fc342b0 100644 --- a/trustmanager/filestore_test.go +++ b/trustmanager/filestore_test.go @@ -286,14 +286,79 @@ func TestGetPath(t *testing.T) { firstPath := "diogomonica.com/openvpn/0xdeadbeef.crt" secondPath := "/docker.io/testing-dashes/@#$%^&().crt" - if store.GetPath("diogomonica.com/openvpn/0xdeadbeef") != firstPath { + + result, err := store.GetPath("diogomonica.com/openvpn/0xdeadbeef") + if err != nil { + t.Fatalf("unexpected error from GetPath: %v", err) + } + if result != firstPath { t.Fatalf("Expecting: %s", firstPath) } - if store.GetPath("/docker.io/testing-dashes/@#$%^&()") != secondPath { + + result, err = store.GetPath("/docker.io/testing-dashes/@#$%^&()") + if err != nil { + t.Fatalf("unexpected error from GetPath: %v", err) + } + if result != secondPath { t.Fatalf("Expecting: %s", secondPath) } } +func TestGetPathProtection(t *testing.T) { + testExt := "crt" + perms := os.FileMode(0755) + + // Create our SimpleFileStore + store := &SimpleFileStore{ + baseDir: "/path/to/filestore/", + fileExt: testExt, + perms: perms, + } + + // Should deny requests for paths outside the filestore + if _, err := store.GetPath("../../etc/passwd"); err != ErrPathOutsideStore { + t.Fatalf("expected ErrPathOutsideStore error from GetPath") + } + if _, err := store.GetPath("private/../../../etc/passwd"); err != ErrPathOutsideStore { + t.Fatalf("expected ErrPathOutsideStore error from GetPath") + } + + // Convoluted paths should work as long as they end up inside the store + expected := "/path/to/filestore/filename.crt" + result, err := store.GetPath("private/../../filestore/./filename") + if err != nil { + t.Fatalf("unexpected error from GetPath: %v", err) + } + if result != expected { + t.Fatalf("Expecting: %s (got: %s)", expected, result) + } + + // Repeat tests with a relative baseDir + relStore := &SimpleFileStore{ + baseDir: "relative/file/path", + fileExt: testExt, + perms: perms, + } + + // Should deny requests for paths outside the filestore + if _, err := relStore.GetPath("../../etc/passwd"); err != ErrPathOutsideStore { + t.Fatalf("expected ErrPathOutsideStore error from GetPath") + } + if _, err := relStore.GetPath("private/../../../etc/passwd"); err != ErrPathOutsideStore { + t.Fatalf("expected ErrPathOutsideStore error from GetPath") + } + + // Convoluted paths should work as long as they end up inside the store + expected = "relative/file/path/filename.crt" + result, err = relStore.GetPath("private/../../path/./filename") + if err != nil { + t.Fatalf("unexpected error from GetPath: %v", err) + } + if result != expected { + t.Fatalf("Expecting: %s (got: %s)", expected, result) + } +} + func TestGetData(t *testing.T) { testName := "docker.com/notary/certificate" testExt := "crt" diff --git a/trustmanager/x509filestore.go b/trustmanager/x509filestore.go index e4cb5fb018..553565356b 100644 --- a/trustmanager/x509filestore.go +++ b/trustmanager/x509filestore.go @@ -85,7 +85,11 @@ func (s X509FileStore) addNamedCert(cert *x509.Certificate) error { certBytes := CertToPEM(cert) // Save the file to disk if not already there. - if _, err := os.Stat(s.fileStore.GetPath(fileName)); os.IsNotExist(err) { + filePath, err := s.fileStore.GetPath(fileName) + if err != nil { + return err + } + if _, err := os.Stat(filePath); os.IsNotExist(err) { if err := s.fileStore.Add(fileName, certBytes); err != nil { return err } From 479333ca7bf8fd9a76c358292e27b5ec9a07039e Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 15 Jul 2015 13:34:32 -0700 Subject: [PATCH 4/7] Add ExportRootKey and ImportRootKey functions Also add a unit test Signed-off-by: Aaron Lehmann --- keystoremanager/import_export.go | 30 ++++++++++++ keystoremanager/import_export_test.go | 69 ++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go index b36fdf112b..da48fbbd23 100644 --- a/keystoremanager/import_export.go +++ b/keystoremanager/import_export.go @@ -16,6 +16,36 @@ import ( "github.com/endophage/gotuf/data" ) +// 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) + if err != nil { + return err + } + + _, err = dest.Write(pemBytes) + return err +} + +// ImportRootKey imports a root in PEM format key from an io.Reader +// The key's existing encryption is preserved. The keyID parameter is +// necessary because otherwise we'd need the passphrase to decrypt the key +// in order to compute the ID. +func (km *KeyStoreManager) ImportRootKey(source io.Reader, keyID string) error { + pemBytes, err := ioutil.ReadAll(source) + if err != nil { + return err + } + + err = km.rootKeyStore.Add(keyID, pemBytes) + if err != nil { + return err + } + + return err +} + func moveKeysWithNewPassphrase(oldKeyStore, newKeyStore *trustmanager.KeyFileStore, outputPassphrase string) error { // List all files but no symlinks for _, f := range oldKeyStore.ListFiles(false) { diff --git a/keystoremanager/import_export_test.go b/keystoremanager/import_export_test.go index d270098205..0d63e37b8c 100644 --- a/keystoremanager/import_export_test.go +++ b/keystoremanager/import_export_test.go @@ -69,7 +69,7 @@ func TestImportExportZip(t *testing.T) { tempZipFile.Close() assert.NoError(t, err) - // Check the contents of the zip file + // Reopen the zip file for importing zipReader, err := zip.OpenReader(tempZipFilePath) assert.NoError(t, err, "could not open zip file") @@ -173,3 +173,70 @@ func TestImportExportZip(t *testing.T) { _, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename)) assert.NoError(t, err, "missing root key") } + +func TestImportExportRootKey(t *testing.T) { + gun := "docker.com/notary" + oldPassphrase := "oldPassphrase" + + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + defer os.RemoveAll(tempBaseDir) + + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + + ts, _ := createTestServer(t) + defer ts.Close() + + repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport) + assert.NoError(t, err, "error creating repo: %s", err) + + rootKeyID, err := repo.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), oldPassphrase) + assert.NoError(t, err, "error generating root key: %s", err) + + rootCryptoService, err := repo.KeyStoreManager.GetRootCryptoService(rootKeyID, oldPassphrase) + assert.NoError(t, err, "error retrieving root key: %s", err) + + err = repo.Initialize(rootCryptoService) + assert.NoError(t, err, "error creating repository: %s", err) + + tempKeyFile, err := ioutil.TempFile("", "notary-test-export-") + tempKeyFilePath := tempKeyFile.Name() + defer os.Remove(tempKeyFilePath) + + err = repo.KeyStoreManager.ExportRootKey(tempKeyFile, rootKeyID) + assert.NoError(t, err) + tempKeyFile.Close() + + // Create new repo to test import + tempBaseDir2, err := ioutil.TempDir("", "notary-test-") + defer os.RemoveAll(tempBaseDir2) + + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + + repo2, err := client.NewNotaryRepository(tempBaseDir2, gun, ts.URL, http.DefaultTransport) + assert.NoError(t, err, "error creating repo: %s", err) + + rootKeyID2, err := repo2.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), "oldPassphrase") + assert.NoError(t, err, "error generating root key: %s", err) + + rootCryptoService2, err := repo2.KeyStoreManager.GetRootCryptoService(rootKeyID2, "oldPassphrase") + assert.NoError(t, err, "error retrieving root key: %s", err) + + err = repo2.Initialize(rootCryptoService2) + assert.NoError(t, err, "error creating repository: %s", err) + + // Check the contents of the zip file + keyReader, err := os.Open(tempKeyFilePath) + assert.NoError(t, err, "could not open key file") + + err = repo2.KeyStoreManager.ImportRootKey(keyReader, rootKeyID) + assert.NoError(t, err) + keyReader.Close() + + // Look for repo's root key in repo2 + // There should be a file named after the key ID of the root key we + // imported. + rootKeyFilename := rootKeyID + ".key" + _, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename)) + assert.NoError(t, err, "missing root key") +} From 0ffb2c69d907a084876502c67bdefb819a2bd0c8 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 15 Jul 2015 14:34:54 -0700 Subject: [PATCH 5/7] Add a check that a root key being imported is encrypted Add unit test coverage that makes this check fail. Also add unit test coverage for making sure trying to import something that isn't PEM fails in the expected way. Signed-off-by: Aaron Lehmann --- keystoremanager/import_export.go | 37 ++++++++++++++++++++++++--- keystoremanager/import_export_test.go | 22 ++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go index da48fbbd23..8dc2ee420b 100644 --- a/keystoremanager/import_export.go +++ b/keystoremanager/import_export.go @@ -16,6 +16,16 @@ import ( "github.com/endophage/gotuf/data" ) +var ( + // ErrNoValidPrivateKey is returned if a key being imported doesn't + // look like a private key + ErrNoValidPrivateKey = errors.New("no valid private key found") + + // ErrRootKeyNotEncrypted is returned if a root key being imported is + // unencrypted + ErrRootKeyNotEncrypted = errors.New("only encrypted root keys may be imported") +) + // ExportRootKey exports the specified root key to an io.Writer in PEM format. // The key's existing encryption is preserved. func (km *KeyStoreManager) ExportRootKey(dest io.Writer, keyID string) error { @@ -28,6 +38,21 @@ func (km *KeyStoreManager) ExportRootKey(dest io.Writer, keyID string) error { return err } +// checkRootKeyIsEncrypted makes sure the root key is encrypted. We have +// internal assumptions that depend on this. +func checkRootKeyIsEncrypted(pemBytes []byte) error { + block, _ := pem.Decode(pemBytes) + if block == nil { + return ErrNoValidPrivateKey + } + + if !x509.IsEncryptedPEMBlock(block) { + return ErrRootKeyNotEncrypted + } + + return nil +} + // ImportRootKey imports a root in PEM format key from an io.Reader // The key's existing encryption is preserved. The keyID parameter is // necessary because otherwise we'd need the passphrase to decrypt the key @@ -38,8 +63,11 @@ func (km *KeyStoreManager) ImportRootKey(source io.Reader, keyID string) error { return err } - err = km.rootKeyStore.Add(keyID, pemBytes) - if err != nil { + if err = checkRootKeyIsEncrypted(pemBytes); err != nil { + return err + } + + if err = km.rootKeyStore.Add(keyID, pemBytes); err != nil { return err } @@ -60,7 +88,7 @@ func moveKeysWithNewPassphrase(oldKeyStore, newKeyStore *trustmanager.KeyFileSto block, _ := pem.Decode(pemBytes) if block == nil { - return errors.New("no valid private key found") + return ErrNoValidPrivateKey } if !x509.IsEncryptedPEMBlock(block) { @@ -213,6 +241,9 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader, passphrase string // package guarantees that the separator will be / rootKeysPrefix := rootKeysSubdir + "/" if strings.HasPrefix(fNameTrimmed, rootKeysPrefix) { + if err = checkRootKeyIsEncrypted(pemBytes); err != nil { + return err + } // Root keys are preserved without decrypting keyName := strings.TrimPrefix(fNameTrimmed, rootKeysPrefix) newRootKeys[keyName] = pemBytes diff --git a/keystoremanager/import_export_test.go b/keystoremanager/import_export_test.go index 0d63e37b8c..d08ab7fe9f 100644 --- a/keystoremanager/import_export_test.go +++ b/keystoremanager/import_export_test.go @@ -2,6 +2,7 @@ package keystoremanager_test import ( "archive/zip" + "bytes" "fmt" "io/ioutil" "net/http" @@ -12,6 +13,7 @@ import ( "testing" "github.com/docker/notary/client" + "github.com/docker/notary/keystoremanager" "github.com/docker/notary/trustmanager" "github.com/endophage/gotuf/data" "github.com/stretchr/testify/assert" @@ -216,10 +218,10 @@ func TestImportExportRootKey(t *testing.T) { repo2, err := client.NewNotaryRepository(tempBaseDir2, gun, ts.URL, http.DefaultTransport) assert.NoError(t, err, "error creating repo: %s", err) - rootKeyID2, err := repo2.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), "oldPassphrase") + rootKeyID2, err := repo2.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), oldPassphrase) assert.NoError(t, err, "error generating root key: %s", err) - rootCryptoService2, err := repo2.KeyStoreManager.GetRootCryptoService(rootKeyID2, "oldPassphrase") + rootCryptoService2, err := repo2.KeyStoreManager.GetRootCryptoService(rootKeyID2, oldPassphrase) assert.NoError(t, err, "error retrieving root key: %s", err) err = repo2.Initialize(rootCryptoService2) @@ -239,4 +241,20 @@ func TestImportExportRootKey(t *testing.T) { rootKeyFilename := rootKeyID + ".key" _, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename)) assert.NoError(t, err, "missing root key") + + // Try to import a decrypted version of the root key and make sure it + // doesn't succeed + pemBytes, err := ioutil.ReadFile(tempKeyFilePath) + assert.NoError(t, err, "could not read key file") + privKey, err := trustmanager.ParsePEMPrivateKey(pemBytes, oldPassphrase) + assert.NoError(t, err, "could not decrypt key file") + decryptedPEMBytes, err := trustmanager.KeyToPEM(privKey) + assert.NoError(t, err, "could not convert key to PEM") + + err = repo2.KeyStoreManager.ImportRootKey(bytes.NewReader(decryptedPEMBytes), rootKeyID) + assert.EqualError(t, err, keystoremanager.ErrRootKeyNotEncrypted.Error()) + + // Try to import garbage and make sure it doesn't succeed + err = repo2.KeyStoreManager.ImportRootKey(strings.NewReader("this is not PEM"), rootKeyID) + assert.EqualError(t, err, keystoremanager.ErrNoValidPrivateKey.Error()) } From 6d3d98b8735b9e09d92053e041d9355a4c19f3d0 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 15 Jul 2015 15:07:41 -0700 Subject: [PATCH 6/7] Move non-root keys to tuf_keys subdirectory This subdirectory is at the same level as root_keys. It avoids having rootKeyStore and nonRootKeyStore overlap. Previously, the base directory for rootKeyStore was .../private/root_keys and the base directory for nonRootKeyStore was .../private. This commit also removes deduplicating logic in ExportAllKeys, which is no longer needed now that the stores don't overlap. Signed-off-by: Aaron Lehmann --- client/client_test.go | 2 +- keystoremanager/import_export.go | 51 +++++++++++++----------------- keystoremanager/keystoremanager.go | 12 ++++--- 3 files changed, 30 insertions(+), 35 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 18cbe02615..5df2cd1034 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -75,7 +75,7 @@ func testInitRepo(t *testing.T, rootType data.KeyAlgorithm) { // Inspect contents of the temporary directory expectedDirs := []string{ "private", - filepath.Join("private", gun), + filepath.Join("private", "tuf_keys", gun), filepath.Join("private", "root_keys"), "trusted_certificates", filepath.Join("trusted_certificates", gun), diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go index 8dc2ee420b..c17452c984 100644 --- a/keystoremanager/import_export.go +++ b/keystoremanager/import_export.go @@ -113,14 +113,9 @@ func moveKeysWithNewPassphrase(oldKeyStore, newKeyStore *trustmanager.KeyFileSto return nil } -func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileStore, tempBaseDir string, dedup map[string]struct{}) error { +func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileStore, tempBaseDir string) error { // List all files but no symlinks for _, fullKeyPath := range newKeyStore.ListFiles(false) { - if _, present := dedup[fullKeyPath]; present { - continue - } - dedup[fullKeyPath] = struct{}{} - relKeyPath := strings.TrimPrefix(fullKeyPath, tempBaseDir) relKeyPath = strings.TrimPrefix(relKeyPath, string(filepath.Separator)) @@ -162,7 +157,7 @@ func (km *KeyStoreManager) ExportAllKeys(dest io.Writer, outputPassphrase string defer os.RemoveAll(tempBaseDir) // Create temporary keystores to use as a staging area - tempNonRootKeysPath := filepath.Join(tempBaseDir, privDir) + tempNonRootKeysPath := filepath.Join(tempBaseDir, privDir, nonRootKeysSubdir) tempNonRootKeyStore, err := trustmanager.NewKeyFileStore(tempNonRootKeysPath) if err != nil { return err @@ -183,13 +178,10 @@ func (km *KeyStoreManager) ExportAllKeys(dest io.Writer, outputPassphrase string zipWriter := zip.NewWriter(dest) - // Root and non-root stores overlap, so we need to dedup files - dedup := make(map[string]struct{}) - - if err := addKeysToArchive(zipWriter, tempRootKeyStore, tempBaseDir, dedup); err != nil { + if err := addKeysToArchive(zipWriter, tempRootKeyStore, tempBaseDir); err != nil { return err } - if err := addKeysToArchive(zipWriter, tempNonRootKeyStore, tempBaseDir, dedup); err != nil { + if err := addKeysToArchive(zipWriter, tempNonRootKeyStore, tempBaseDir); err != nil { return err } @@ -208,23 +200,14 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader, passphrase string newRootKeys := make(map[string][]byte) newNonRootKeys := make(map[string]*data.PrivateKey) + // Note that using / as a separator is okay here - the zip package + // guarantees that the separator will be / + rootKeysPrefix := privDir + "/" + rootKeysSubdir + "/" + nonRootKeysPrefix := privDir + "/" + nonRootKeysSubdir + "/" + // Iterate through the files in the archive. Don't add the keys for _, f := range zipReader.File { fNameTrimmed := strings.TrimSuffix(f.Name, filepath.Ext(f.Name)) - // Note that using / as a separator is okay here - the zip - // package guarantees that the separator will be / - keysPrefix := privDir + "/" - - if !strings.HasPrefix(fNameTrimmed, keysPrefix) { - // This path inside the zip archive doesn't start with - // "private". That's unexpected, because all keys - // should be in that subdirectory. 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: %s", keysPrefix, f.Name) - continue - } - fNameTrimmed = strings.TrimPrefix(fNameTrimmed, keysPrefix) rc, err := f.Open() if err != nil { @@ -239,21 +222,31 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader, passphrase string // 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 / - rootKeysPrefix := rootKeysSubdir + "/" if strings.HasPrefix(fNameTrimmed, rootKeysPrefix) { if err = checkRootKeyIsEncrypted(pemBytes); err != nil { + rc.Close() return err } // Root keys are preserved without decrypting keyName := strings.TrimPrefix(fNameTrimmed, rootKeysPrefix) newRootKeys[keyName] = pemBytes - } else { + } else if strings.HasPrefix(fNameTrimmed, nonRootKeysPrefix) { // Non-root keys need to be decrypted key, err := trustmanager.ParsePEMPrivateKey(pemBytes, passphrase) if err != nil { + rc.Close() return err } - newNonRootKeys[fNameTrimmed] = key + keyName := strings.TrimPrefix(fNameTrimmed, nonRootKeysPrefix) + newNonRootKeys[keyName] = key + } else { + // This path inside the zip archive doesn't look like a + // root key or a non-root key. 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 } rc.Close() diff --git a/keystoremanager/keystoremanager.go b/keystoremanager/keystoremanager.go index d27be29d3c..cd607d6db1 100644 --- a/keystoremanager/keystoremanager.go +++ b/keystoremanager/keystoremanager.go @@ -29,16 +29,18 @@ type KeyStoreManager struct { } const ( - trustDir = "trusted_certificates" - privDir = "private" - rootKeysSubdir = "root_keys" - rsaRootKeySize = 4096 // Used for new root keys + trustDir = "trusted_certificates" + privDir = "private" + rootKeysSubdir = "root_keys" + nonRootKeysSubdir = "tuf_keys" + rsaRootKeySize = 4096 // Used for new root keys ) // NewKeyStoreManager returns an initialized KeyStoreManager, or an error // if it fails to create the KeyFileStores or load certificates func NewKeyStoreManager(baseDir string) (*KeyStoreManager, error) { - nonRootKeyStore, err := trustmanager.NewKeyFileStore(filepath.Join(baseDir, privDir)) + nonRootKeysPath := filepath.Join(baseDir, privDir, nonRootKeysSubdir) + nonRootKeyStore, err := trustmanager.NewKeyFileStore(nonRootKeysPath) if err != nil { return nil, err } From e5a42d4df92a596c0a80d66fc59abb571b62a8b8 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 15 Jul 2015 15:37:00 -0700 Subject: [PATCH 7/7] Add ExportKeysByGUN function It exports the keys for a particular GUN to a zip, encrypted with a specified passphrase. Signed-off-by: Aaron Lehmann --- client/client.go | 2 +- client/client_test.go | 26 ++--- keystoremanager/import_export.go | 82 ++++++++++++++++ keystoremanager/import_export_test.go | 134 +++++++++++++++++++++++++- 4 files changed, 229 insertions(+), 15 deletions(-) diff --git a/client/client.go b/client/client.go index 8835cb17f4..9ec4142ff0 100644 --- a/client/client.go +++ b/client/client.go @@ -95,7 +95,7 @@ func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper) (*N gun: gun, baseDir: baseDir, baseURL: baseURL, - tufRepoPath: filepath.Join(baseDir, tufDir, gun), + tufRepoPath: filepath.Join(baseDir, tufDir, filepath.FromSlash(gun)), cryptoService: cryptoService, roundTrip: rt, KeyStoreManager: keyStoreManager, diff --git a/client/client_test.go b/client/client_test.go index 5df2cd1034..7e3312b14d 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -75,13 +75,13 @@ func testInitRepo(t *testing.T, rootType data.KeyAlgorithm) { // Inspect contents of the temporary directory expectedDirs := []string{ "private", - filepath.Join("private", "tuf_keys", gun), + filepath.Join("private", "tuf_keys", filepath.FromSlash(gun)), filepath.Join("private", "root_keys"), "trusted_certificates", - filepath.Join("trusted_certificates", gun), + filepath.Join("trusted_certificates", filepath.FromSlash(gun)), "tuf", - filepath.Join("tuf", gun, "metadata"), - filepath.Join("tuf", gun, "targets"), + filepath.Join("tuf", filepath.FromSlash(gun), "metadata"), + filepath.Join("tuf", filepath.FromSlash(gun), "targets"), } for _, dir := range expectedDirs { fi, err := os.Stat(filepath.Join(tempBaseDir, dir)) @@ -118,16 +118,16 @@ func testInitRepo(t *testing.T, rootType data.KeyAlgorithm) { assert.Equal(t, rootKeyFilename, actualDest, "symlink to root key has wrong destination") // There should be a trusted certificate - _, err = os.Stat(filepath.Join(tempBaseDir, "trusted_certificates", gun, certID+".crt")) + _, err = os.Stat(filepath.Join(tempBaseDir, "trusted_certificates", filepath.FromSlash(gun), certID+".crt")) assert.NoError(t, err, "missing trusted certificate") // Sanity check the TUF metadata files. Verify that they exist, the JSON is // well-formed, and the signatures exist. For the root.json file, also check // that the root, snapshot, and targets key IDs are present. expectedTUFMetadataFiles := []string{ - filepath.Join("tuf", gun, "metadata", "root.json"), - filepath.Join("tuf", gun, "metadata", "snapshot.json"), - filepath.Join("tuf", gun, "metadata", "targets.json"), + filepath.Join("tuf", filepath.FromSlash(gun), "metadata", "root.json"), + filepath.Join("tuf", filepath.FromSlash(gun), "metadata", "snapshot.json"), + filepath.Join("tuf", filepath.FromSlash(gun), "metadata", "targets.json"), } for _, filename := range expectedTUFMetadataFiles { fullPath := filepath.Join(tempBaseDir, filename) @@ -225,7 +225,7 @@ func testAddListTarget(t *testing.T, rootType data.KeyAlgorithm) { assert.NoError(t, err, "error adding target") // Look for the changelist file - changelistDirPath := filepath.Join(tempBaseDir, "tuf", gun, "changelist") + changelistDirPath := filepath.Join(tempBaseDir, "tuf", filepath.FromSlash(gun), "changelist") changelistDir, err := os.Open(changelistDirPath) assert.NoError(t, err, "could not open changelist directory") @@ -299,7 +299,7 @@ func testAddListTarget(t *testing.T, rootType data.KeyAlgorithm) { // Apply the changelist. Normally, this would be done by Publish // load the changelist for this repo - cl, err := changelist.NewFileChangelist(filepath.Join(tempBaseDir, "tuf", gun, "changelist")) + cl, err := changelist.NewFileChangelist(filepath.Join(tempBaseDir, "tuf", filepath.FromSlash(gun), "changelist")) assert.NoError(t, err, "could not open changelist") // apply the changelist to the repo @@ -309,10 +309,10 @@ func testAddListTarget(t *testing.T, rootType data.KeyAlgorithm) { var tempKey data.PrivateKey json.Unmarshal([]byte(timestampECDSAKeyJSON), &tempKey) - repo.KeyStoreManager.NonRootKeyStore().AddKey(filepath.Join(gun, tempKey.ID()), &tempKey) + repo.KeyStoreManager.NonRootKeyStore().AddKey(filepath.Join(filepath.FromSlash(gun), tempKey.ID()), &tempKey) mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/root.json", func(w http.ResponseWriter, r *http.Request) { - rootJSONFile := filepath.Join(tempBaseDir, "tuf", gun, "metadata", "root.json") + rootJSONFile := filepath.Join(tempBaseDir, "tuf", filepath.FromSlash(gun), "metadata", "root.json") rootFileBytes, err := ioutil.ReadFile(rootJSONFile) assert.NoError(t, err) fmt.Fprint(w, string(rootFileBytes)) @@ -401,7 +401,7 @@ func testValidateRootKey(t *testing.T, rootType data.KeyAlgorithm) { err = repo.Initialize(rootCryptoService) assert.NoError(t, err, "error creating repository: %s", err) - rootJSONFile := filepath.Join(tempBaseDir, "tuf", gun, "metadata", "root.json") + rootJSONFile := filepath.Join(tempBaseDir, "tuf", filepath.FromSlash(gun), "metadata", "root.json") jsonBytes, err := ioutil.ReadFile(rootJSONFile) assert.NoError(t, err, "error reading TUF metadata file %s: %s", rootJSONFile, err) diff --git a/keystoremanager/import_export.go b/keystoremanager/import_export.go index c17452c984..18e2f47ad4 100644 --- a/keystoremanager/import_export.go +++ b/keystoremanager/import_export.go @@ -24,6 +24,14 @@ var ( // ErrRootKeyNotEncrypted is returned if a root key being imported is // unencrypted ErrRootKeyNotEncrypted = errors.New("only encrypted root keys may be imported") + + // ErrNonRootKeyEncrypted is returned if a non-root key is found to + // be encrypted while exporting + ErrNonRootKeyEncrypted = errors.New("found encrypted non-root key") + + // ErrNoKeysFoundForGUN is returned if no keys are found for the + // specified GUN during export + ErrNoKeysFoundForGUN = errors.New("no keys found for specified GUN") ) // ExportRootKey exports the specified root key to an io.Writer in PEM format. @@ -266,3 +274,77 @@ func (km *KeyStoreManager) ImportKeysZip(zipReader zip.Reader, passphrase string return nil } + +func moveKeysByGUN(oldKeyStore, newKeyStore *trustmanager.KeyFileStore, gun, outputPassphrase string) error { + // List all files but no symlinks + for _, f := range oldKeyStore.ListFiles(false) { + fullKeyPath := strings.TrimSpace(strings.TrimSuffix(f, filepath.Ext(f))) + relKeyPath := strings.TrimPrefix(fullKeyPath, oldKeyStore.BaseDir()) + relKeyPath = strings.TrimPrefix(relKeyPath, string(filepath.Separator)) + + // Skip keys that aren't associated with this GUN + if !strings.HasPrefix(relKeyPath, filepath.FromSlash(gun)) { + continue + } + + pemBytes, err := oldKeyStore.Get(relKeyPath) + if err != nil { + return err + } + + block, _ := pem.Decode(pemBytes) + if block == nil { + return ErrNoValidPrivateKey + } + + if x509.IsEncryptedPEMBlock(block) { + return ErrNonRootKeyEncrypted + } + + // Key is not encrypted. Parse it, and add it + // to the temporary store as an encrypted key. + privKey, err := trustmanager.ParsePEMPrivateKey(pemBytes, "") + if err != nil { + return err + } + err = newKeyStore.AddEncryptedKey(relKeyPath, privKey, outputPassphrase) + if err != nil { + return err + } + } + + return nil +} + +// ExportKeysByGUN exports all keys associated with a specified GUN to an +// io.Writer in zip format. outputPassphrase is the new passphrase to use to +// encrypt the keys. If blank, the keys will not be encrypted. +func (km *KeyStoreManager) ExportKeysByGUN(dest io.Writer, gun, outputPassphrase string) error { + tempBaseDir, err := ioutil.TempDir("", "notary-key-export-") + defer os.RemoveAll(tempBaseDir) + + // Create temporary keystore to use as a staging area + tempNonRootKeysPath := filepath.Join(tempBaseDir, privDir, nonRootKeysSubdir) + tempNonRootKeyStore, err := trustmanager.NewKeyFileStore(tempNonRootKeysPath) + if err != nil { + return err + } + + if err := moveKeysByGUN(km.nonRootKeyStore, tempNonRootKeyStore, gun, outputPassphrase); err != nil { + return err + } + + zipWriter := zip.NewWriter(dest) + + if len(tempNonRootKeyStore.ListKeys()) == 0 { + return ErrNoKeysFoundForGUN + } + + if err := addKeysToArchive(zipWriter, tempNonRootKeyStore, tempBaseDir); err != nil { + return err + } + + zipWriter.Close() + + return nil +} diff --git a/keystoremanager/import_export_test.go b/keystoremanager/import_export_test.go index d08ab7fe9f..08dc3cb437 100644 --- a/keystoremanager/import_export_test.go +++ b/keystoremanager/import_export_test.go @@ -78,7 +78,7 @@ func TestImportExportZip(t *testing.T) { // Map of files to expect in the zip file, with the passphrases passphraseByFile := make(map[string]string) - // Add keys in private to the map. These should use the new passphrase + // Add non-root keys to the map. These should use the new passphrase // because they were formerly unencrypted. privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListFiles(false) for _, privKeyName := range privKeyList { @@ -176,6 +176,138 @@ func TestImportExportZip(t *testing.T) { assert.NoError(t, err, "missing root key") } +func TestImportExportGUN(t *testing.T) { + gun := "docker.com/notary" + oldPassphrase := "oldPassphrase" + exportPassphrase := "exportPassphrase" + + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + defer os.RemoveAll(tempBaseDir) + + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + + ts, _ := createTestServer(t) + defer ts.Close() + + repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport) + assert.NoError(t, err, "error creating repo: %s", err) + + rootKeyID, err := repo.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), oldPassphrase) + assert.NoError(t, err, "error generating root key: %s", err) + + rootCryptoService, err := repo.KeyStoreManager.GetRootCryptoService(rootKeyID, oldPassphrase) + assert.NoError(t, err, "error retrieving root key: %s", err) + + err = repo.Initialize(rootCryptoService) + assert.NoError(t, err, "error creating repository: %s", err) + + tempZipFile, err := ioutil.TempFile("", "notary-test-export-") + tempZipFilePath := tempZipFile.Name() + defer os.Remove(tempZipFilePath) + + err = repo.KeyStoreManager.ExportKeysByGUN(tempZipFile, gun, exportPassphrase) + assert.NoError(t, err) + + // With an invalid GUN, this should return an error + err = repo.KeyStoreManager.ExportKeysByGUN(tempZipFile, "does.not.exist/in/repository", exportPassphrase) + assert.EqualError(t, err, keystoremanager.ErrNoKeysFoundForGUN.Error()) + + tempZipFile.Close() + + // Reopen the zip file for importing + zipReader, err := zip.OpenReader(tempZipFilePath) + assert.NoError(t, err, "could not open zip file") + + // Map of files to expect in the zip file, with the passphrases + passphraseByFile := make(map[string]string) + + // Add keys non-root keys to the map. These should use the new passphrase + // because they were formerly unencrypted. + privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListFiles(false) + for _, privKeyName := range privKeyList { + relName := strings.TrimPrefix(privKeyName, tempBaseDir+string(filepath.Separator)) + passphraseByFile[relName] = exportPassphrase + } + + // Iterate through the files in the archive, checking that the files + // exist and are encrypted with the expected passphrase. + for _, f := range zipReader.File { + expectedPassphrase, present := passphraseByFile[f.Name] + if !present { + t.Fatalf("unexpected file %s in zip file", f.Name) + } + + delete(passphraseByFile, f.Name) + + rc, err := f.Open() + assert.NoError(t, err, "could not open file inside zip archive") + + pemBytes, err := ioutil.ReadAll(rc) + assert.NoError(t, err, "could not read file from zip") + + _, err = trustmanager.ParsePEMPrivateKey(pemBytes, expectedPassphrase) + assert.NoError(t, err, "PEM not encrypted with the expected passphrase") + + rc.Close() + } + + zipReader.Close() + + // Are there any keys that didn't make it to the zip? + for fileNotFound := range passphraseByFile { + t.Fatalf("%s not found in zip", fileNotFound) + } + + // Create new repo to test import + tempBaseDir2, err := ioutil.TempDir("", "notary-test-") + defer os.RemoveAll(tempBaseDir2) + + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + + repo2, err := client.NewNotaryRepository(tempBaseDir2, gun, ts.URL, http.DefaultTransport) + assert.NoError(t, err, "error creating repo: %s", err) + + rootKeyID2, err := repo2.KeyStoreManager.GenRootKey(data.ECDSAKey.String(), "oldPassphrase") + assert.NoError(t, err, "error generating root key: %s", err) + + rootCryptoService2, err := repo2.KeyStoreManager.GetRootCryptoService(rootKeyID2, "oldPassphrase") + assert.NoError(t, err, "error retrieving root key: %s", err) + + err = repo2.Initialize(rootCryptoService2) + assert.NoError(t, err, "error creating repository: %s", err) + + // Reopen the zip file for importing + zipReader, err = zip.OpenReader(tempZipFilePath) + assert.NoError(t, err, "could not open zip file") + + // First try with an incorrect passphrase + err = repo2.KeyStoreManager.ImportKeysZip(zipReader.Reader, "wrongpassphrase") + // Don't use EqualError here because occasionally decrypting with the + // wrong passphrase returns a parse error + assert.Error(t, err) + zipReader.Close() + + // Reopen the zip file for importing + zipReader, err = zip.OpenReader(tempZipFilePath) + assert.NoError(t, err, "could not open zip file") + + // Now try with a valid passphrase. This time it should succeed. + err = repo2.KeyStoreManager.ImportKeysZip(zipReader.Reader, exportPassphrase) + assert.NoError(t, err) + zipReader.Close() + + // Look for repo's non-root keys in repo2 + + // Look for keys in private. The filenames should match the key IDs + // in the repo's private key store. + for _, privKeyName := range privKeyList { + privKeyRel := strings.TrimPrefix(privKeyName, tempBaseDir) + _, err := os.Stat(filepath.Join(tempBaseDir2, privKeyRel)) + assert.NoError(t, err, "missing private key: %s", privKeyName) + } +} + func TestImportExportRootKey(t *testing.T) { gun := "docker.com/notary" oldPassphrase := "oldPassphrase"