From 8f7fddd5d5b987ba9404159897cb4586bce5b42c Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 21 Dec 2015 16:10:48 -0800 Subject: [PATCH 1/8] breaking up low level storage into logical files Signed-off-by: David Lawrence (github: endophage) --- trustmanager/filestore.go | 97 ------------------------------------- trustmanager/memorystore.go | 71 +++++++++++++++++++++++++++ trustmanager/store.go | 36 ++++++++++++++ 3 files changed, 107 insertions(+), 97 deletions(-) create mode 100644 trustmanager/memorystore.go create mode 100644 trustmanager/store.go diff --git a/trustmanager/filestore.go b/trustmanager/filestore.go index e5172997b6..46c291fc09 100644 --- a/trustmanager/filestore.go +++ b/trustmanager/filestore.go @@ -1,45 +1,13 @@ package trustmanager import ( - "errors" "fmt" - "github.com/docker/notary" "io/ioutil" "os" "path/filepath" "strings" - "sync" ) -const ( - visible = notary.PubCertPerms - private = notary.PrivKeyPerms -) - -var ( - // ErrPathOutsideStore indicates that the returned path would be - // outside the store - ErrPathOutsideStore = errors.New("path outside file store") -) - -// LimitedFileStore implements the bare bones primitives (no hierarchy) -type LimitedFileStore interface { - Add(fileName string, data []byte) error - Remove(fileName string) error - Get(fileName string) ([]byte, error) - ListFiles() []string -} - -// FileStore is the interface for full-featured FileStores -type FileStore interface { - LimitedFileStore - - RemoveDir(directoryName string) error - GetPath(fileName string) (string, error) - ListDir(directoryName string) []string - BaseDir() string -} - // SimpleFileStore implements FileStore type SimpleFileStore struct { baseDir string @@ -212,68 +180,3 @@ func createDirectory(dir string, perms os.FileMode) error { dir = dir + "/" return os.MkdirAll(dir, perms) } - -// MemoryFileStore is an implementation of LimitedFileStore that keeps -// the contents in memory. -type MemoryFileStore struct { - sync.Mutex - - files map[string][]byte -} - -// NewMemoryFileStore creates a MemoryFileStore -func NewMemoryFileStore() *MemoryFileStore { - return &MemoryFileStore{ - files: make(map[string][]byte), - } -} - -// ErrMemFileNotFound is returned for a nonexistent "file" in the memory file -// store -var ErrMemFileNotFound = errors.New("key not found in memory file store") - -// Add writes data to a file with a given name -func (f *MemoryFileStore) Add(name string, data []byte) error { - f.Lock() - defer f.Unlock() - - f.files[name] = data - return nil -} - -// Remove removes a file identified by name -func (f *MemoryFileStore) Remove(name string) error { - f.Lock() - defer f.Unlock() - - if _, present := f.files[name]; !present { - return ErrMemFileNotFound - } - delete(f.files, name) - - return nil -} - -// Get returns the data given a file name -func (f *MemoryFileStore) Get(name string) ([]byte, error) { - f.Lock() - defer f.Unlock() - - fileData, present := f.files[name] - if !present { - return nil, ErrMemFileNotFound - } - - return fileData, nil -} - -// ListFiles lists all the files inside of a store -func (f *MemoryFileStore) ListFiles() []string { - var list []string - - for name := range f.files { - list = append(list, name) - } - - return list -} diff --git a/trustmanager/memorystore.go b/trustmanager/memorystore.go new file mode 100644 index 0000000000..3509035946 --- /dev/null +++ b/trustmanager/memorystore.go @@ -0,0 +1,71 @@ +package trustmanager + +import ( + "errors" + "sync" +) + +// MemoryFileStore is an implementation of LimitedFileStore that keeps +// the contents in memory. +type MemoryFileStore struct { + sync.Mutex + + files map[string][]byte +} + +// NewMemoryFileStore creates a MemoryFileStore +func NewMemoryFileStore() *MemoryFileStore { + return &MemoryFileStore{ + files: make(map[string][]byte), + } +} + +// ErrMemFileNotFound is returned for a nonexistent "file" in the memory file +// store +var ErrMemFileNotFound = errors.New("key not found in memory file store") + +// Add writes data to a file with a given name +func (f *MemoryFileStore) Add(name string, data []byte) error { + f.Lock() + defer f.Unlock() + + f.files[name] = data + return nil +} + +// Remove removes a file identified by name +func (f *MemoryFileStore) Remove(name string) error { + f.Lock() + defer f.Unlock() + + if _, present := f.files[name]; !present { + return ErrMemFileNotFound + } + delete(f.files, name) + + return nil +} + +// Get returns the data given a file name +func (f *MemoryFileStore) Get(name string) ([]byte, error) { + f.Lock() + defer f.Unlock() + + fileData, present := f.files[name] + if !present { + return nil, ErrMemFileNotFound + } + + return fileData, nil +} + +// ListFiles lists all the files inside of a store +func (f *MemoryFileStore) ListFiles() []string { + var list []string + + for name := range f.files { + list = append(list, name) + } + + return list +} diff --git a/trustmanager/store.go b/trustmanager/store.go new file mode 100644 index 0000000000..f43c6d219b --- /dev/null +++ b/trustmanager/store.go @@ -0,0 +1,36 @@ +package trustmanager + +import ( + "errors" + + "github.com/docker/notary" +) + +const ( + visible = notary.PubCertPerms + private = notary.PrivKeyPerms +) + +var ( + // ErrPathOutsideStore indicates that the returned path would be + // outside the store + ErrPathOutsideStore = errors.New("path outside file store") +) + +// LimitedFileStore implements the bare bones primitives (no hierarchy) +type LimitedFileStore interface { + Add(fileName string, data []byte) error + Remove(fileName string) error + Get(fileName string) ([]byte, error) + ListFiles() []string +} + +// FileStore is the interface for full-featured FileStores +type FileStore interface { + LimitedFileStore + + RemoveDir(directoryName string) error + GetPath(fileName string) (string, error) + ListDir(directoryName string) []string + BaseDir() string +} From 1f329868e8f3f77c514607f8004e20462d30c63b Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 21 Dec 2015 18:10:53 -0800 Subject: [PATCH 2/8] making filestores consistent so you can Get, Remove, etc... the paths returned by ListFiles Signed-off-by: David Lawrence (github: endophage) --- cryptoservice/import_export.go | 15 +- trustmanager/filestore.go | 13 +- trustmanager/filestore_test.go | 251 ++++++++++++++------------------- trustmanager/keyfilestore.go | 14 +- trustmanager/memorystore.go | 10 +- trustmanager/store.go | 16 +++ trustmanager/x509utils.go | 13 +- 7 files changed, 161 insertions(+), 171 deletions(-) diff --git a/cryptoservice/import_export.go b/cryptoservice/import_export.go index 4f5bd4ca82..f1d454e151 100644 --- a/cryptoservice/import_export.go +++ b/cryptoservice/import_export.go @@ -261,12 +261,12 @@ func moveKeysByGUN(oldKeyStore, newKeyStore trustmanager.KeyStore, gun string) e func moveKeys(oldKeyStore, newKeyStore trustmanager.KeyStore) error { for f := range oldKeyStore.ListKeys() { - privateKey, alias, err := oldKeyStore.GetKey(f) + privateKey, role, err := oldKeyStore.GetKey(f) if err != nil { return err } - err = newKeyStore.AddKey(f, alias, privateKey) + err = newKeyStore.AddKey(f, role, privateKey) if err != nil { return err @@ -278,7 +278,10 @@ func moveKeys(oldKeyStore, newKeyStore trustmanager.KeyStore) error { func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileStore) error { for _, relKeyPath := range newKeyStore.ListFiles() { - fullKeyPath := filepath.Join(newKeyStore.BaseDir(), relKeyPath) + fullKeyPath, err := newKeyStore.GetPath(relKeyPath) + if err != nil { + return err + } fi, err := os.Lstat(fullKeyPath) if err != nil { @@ -290,7 +293,11 @@ func addKeysToArchive(zipWriter *zip.Writer, newKeyStore *trustmanager.KeyFileSt return err } - infoHeader.Name = relKeyPath + relPath, err := filepath.Rel(newKeyStore.BaseDir(), fullKeyPath) + if err != nil { + return err + } + infoHeader.Name = relPath zipFileEntryWriter, err := zipWriter.CreateHeader(infoHeader) if err != nil { diff --git a/trustmanager/filestore.go b/trustmanager/filestore.go index 46c291fc09..b970159e22 100644 --- a/trustmanager/filestore.go +++ b/trustmanager/filestore.go @@ -23,6 +23,10 @@ func NewSimpleFileStore(baseDir string, fileExt string) (*SimpleFileStore, error return nil, err } + if !strings.HasPrefix(fileExt, ".") { + fileExt = "." + fileExt + } + return &SimpleFileStore{ baseDir: baseDir, fileExt: fileExt, @@ -36,6 +40,10 @@ func NewPrivateSimpleFileStore(baseDir string, fileExt string) (*SimpleFileStore return nil, err } + if !strings.HasPrefix(fileExt, ".") { + fileExt = "." + fileExt + } + return &SimpleFileStore{ baseDir: baseDir, fileExt: fileExt, @@ -144,7 +152,8 @@ func (f *SimpleFileStore) list(path string) []string { if err != nil { return err } - files = append(files, fp) + trimmed := strings.TrimSuffix(fp, f.fileExt) + files = append(files, trimmed) } return nil }) @@ -153,7 +162,7 @@ func (f *SimpleFileStore) list(path string) []string { // genFileName returns the name using the right extension func (f *SimpleFileStore) genFileName(name string) string { - return fmt.Sprintf("%s.%s", name, f.fileExt) + return fmt.Sprintf("%s%s", name, f.fileExt) } // BaseDir returns the base directory of the filestore diff --git a/trustmanager/filestore_test.go b/trustmanager/filestore_test.go index 0b64d7d89e..e96bc04621 100644 --- a/trustmanager/filestore_test.go +++ b/trustmanager/filestore_test.go @@ -1,9 +1,9 @@ package trustmanager import ( - "bytes" "crypto/rand" "fmt" + "github.com/stretchr/testify/assert" "io/ioutil" "os" "path/filepath" @@ -14,18 +14,16 @@ import ( func TestAddFile(t *testing.T) { testData := []byte("This test data should be part of the file.") testName := "docker.com/notary/certificate" - testExt := "crt" + testExt := ".crt" perms := os.FileMode(0755) // 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) defer os.RemoveAll(tempBaseDir) // Since we're generating this manually we need to add the extension '.' - expectedFilePath := filepath.Join(tempBaseDir, testName+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, testName+testExt) // Create our SimpleFileStore store := &SimpleFileStore{ @@ -36,40 +34,29 @@ func TestAddFile(t *testing.T) { // Call the Add function err = store.Add(testName, testData) - if err != nil { - t.Fatalf("failed to add file to store: %v", err) - } + assert.NoError(t, err) // Check to see if file exists b, err := ioutil.ReadFile(expectedFilePath) - if err != nil { - t.Fatalf("expected file not found: %v", err) - } - - if !bytes.Equal(b, testData) { - t.Fatalf("unexpected content in the file: %s", expectedFilePath) - } + assert.NoError(t, err) + assert.Equal(t, testData, b, "unexpected content in the file: %s", expectedFilePath) } func TestRemoveFile(t *testing.T) { testName := "docker.com/notary/certificate" - testExt := "crt" + testExt := ".crt" perms := os.FileMode(0755) // 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) defer os.RemoveAll(tempBaseDir) // Since we're generating this manually we need to add the extension '.' - expectedFilePath := filepath.Join(tempBaseDir, testName+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, testName+testExt) _, err = generateRandomFile(expectedFilePath, perms) - if err != nil { - t.Fatalf("failed to generate random file: %v", err) - } + assert.NoError(t, err) // Create our SimpleFileStore store := &SimpleFileStore{ @@ -80,36 +67,28 @@ func TestRemoveFile(t *testing.T) { // Call the Remove function err = store.Remove(testName) - if err != nil { - t.Fatalf("failed to remove file from store: %v", err) - } + assert.NoError(t, err) // Check to see if file exists _, err = os.Stat(expectedFilePath) - if err == nil { - t.Fatalf("expected not to find file: %s", expectedFilePath) - } + assert.Error(t, err) } func TestRemoveDir(t *testing.T) { testName := "docker.com/diogomonica/" - testExt := "key" + testExt := ".key" perms := os.FileMode(0700) // 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) defer os.RemoveAll(tempBaseDir) // Since we're generating this manually we need to add the extension '.' - expectedFilePath := filepath.Join(tempBaseDir, testName+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, testName+testExt) _, err = generateRandomFile(expectedFilePath, perms) - if err != nil { - t.Fatalf("failed to generate random file: %v", err) - } + assert.NoError(t, err) // Create our SimpleFileStore store := &SimpleFileStore{ @@ -120,16 +99,12 @@ func TestRemoveDir(t *testing.T) { // Call the RemoveDir function err = store.RemoveDir(testName) - if err != nil { - t.Fatalf("failed to remove directory: %v", err) - } + assert.NoError(t, err) expectedDirectory := filepath.Dir(expectedFilePath) // Check to see if file exists _, err = os.Stat(expectedDirectory) - if err == nil { - t.Fatalf("expected not to find directory: %s", expectedDirectory) - } + assert.Error(t, err) } func TestListFiles(t *testing.T) { @@ -139,9 +114,7 @@ func TestListFiles(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) defer os.RemoveAll(tempBaseDir) var expectedFilePath string @@ -151,9 +124,7 @@ func TestListFiles(t *testing.T) { expectedFilename := testName + strconv.Itoa(i) + "." + testExt expectedFilePath = filepath.Join(tempBaseDir, expectedFilename) _, err = generateRandomFile(expectedFilePath, perms) - if err != nil { - t.Fatalf("failed to generate random file: %v", err) - } + assert.NoError(t, err) } // Create our SimpleFileStore @@ -165,9 +136,7 @@ func TestListFiles(t *testing.T) { // Call the List function. Expect 10 files files := store.ListFiles() - if len(files) != 10 { - t.Fatalf("expected 10 files in listing, got: %d", len(files)) - } + assert.Len(t, files, 10) } func TestListDir(t *testing.T) { @@ -177,9 +146,7 @@ func TestListDir(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) defer os.RemoveAll(tempBaseDir) var expectedFilePath string @@ -189,9 +156,7 @@ func TestListDir(t *testing.T) { fileName := fmt.Sprintf("%s-%s.%s", testName, strconv.Itoa(i), testExt) expectedFilePath = filepath.Join(tempBaseDir, fileName) _, err = generateRandomFile(expectedFilePath, perms) - if err != nil { - t.Fatalf("failed to generate random file: %v", err) - } + assert.NoError(t, err) } // Create our SimpleFileStore @@ -203,21 +168,15 @@ func TestListDir(t *testing.T) { // Call the ListDir function files := store.ListDir("docker.com/") - if len(files) != 10 { - t.Fatalf("expected 10 files in listing, got: %d", len(files)) - } + assert.Len(t, files, 10) files = store.ListDir("docker.com/notary") - if len(files) != 10 { - t.Fatalf("expected 10 files in listing, got: %d", len(files)) - } + assert.Len(t, files, 10) files = store.ListDir("fakedocker.com/") - if len(files) != 0 { - t.Fatalf("expected 0 files in listing, got: %d", len(files)) - } + assert.Len(t, files, 0) } func TestGetPath(t *testing.T) { - testExt := "crt" + testExt := ".crt" perms := os.FileMode(0755) // Create our SimpleFileStore @@ -231,24 +190,14 @@ func TestGetPath(t *testing.T) { secondPath := "/docker.io/testing-dashes/@#$%^&().crt" 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) - } + assert.Equal(t, firstPath, result, "unexpected error from GetPath: %v", err) 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) - } + assert.Equal(t, secondPath, result, "unexpected error from GetPath: %v", err) } func TestGetPathProtection(t *testing.T) { - testExt := "crt" + testExt := ".crt" perms := os.FileMode(0755) // Create our SimpleFileStore @@ -259,22 +208,19 @@ func TestGetPathProtection(t *testing.T) { } // 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") - } + _, err := store.GetPath("../../etc/passwd") + assert.Error(t, err) + assert.Equal(t, ErrPathOutsideStore, err) + + _, err = store.GetPath("private/../../../etc/passwd") + assert.Error(t, err) + assert.Equal(t, ErrPathOutsideStore, err) // 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) - } + assert.NoError(t, err) + assert.Equal(t, expected, result) // Repeat tests with a relative baseDir relStore := &SimpleFileStore{ @@ -284,43 +230,35 @@ func TestGetPathProtection(t *testing.T) { } // 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") - } + _, err = relStore.GetPath("../../etc/passwd") + assert.Error(t, err) + assert.Equal(t, ErrPathOutsideStore, err) + _, err = relStore.GetPath("private/../../../etc/passwd") + assert.Error(t, err) + assert.Equal(t, ErrPathOutsideStore, err) // 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) - } + assert.NoError(t, err) + assert.Equal(t, expected, result) } func TestGetData(t *testing.T) { testName := "docker.com/notary/certificate" - testExt := "crt" + testExt := ".crt" perms := os.FileMode(0755) // 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) defer os.RemoveAll(tempBaseDir) // Since we're generating this manually we need to add the extension '.' - expectedFilePath := filepath.Join(tempBaseDir, testName+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, testName+testExt) expectedData, err := generateRandomFile(expectedFilePath, perms) - if err != nil { - t.Fatalf("failed to generate random file: %v", err) - } + assert.NoError(t, err) // Create our SimpleFileStore store := &SimpleFileStore{ @@ -329,13 +267,8 @@ func TestGetData(t *testing.T) { perms: perms, } testData, err := store.Get(testName) - if err != nil { - t.Fatalf("failed to get data from: %s", testName) - - } - if !bytes.Equal(testData, expectedData) { - t.Fatalf("unexpected content for the file: %s", expectedFilePath) - } + assert.NoError(t, err, "failed to get data from: %s", testName) + assert.Equal(t, expectedData, testData, "unexpected content for the file: %s", expectedFilePath) } func TestCreateDirectory(t *testing.T) { @@ -343,9 +276,7 @@ func TestCreateDirectory(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) defer os.RemoveAll(tempBaseDir) dirPath := filepath.Join(tempBaseDir, testDir) @@ -355,19 +286,13 @@ func TestCreateDirectory(t *testing.T) { // Check to see if file exists fi, err := os.Stat(dirPath) - if err != nil { - t.Fatalf("expected find directory: %s", dirPath) - } + assert.NoError(t, err) // Check to see if it is a directory - if !fi.IsDir() { - t.Fatalf("expected to be directory: %s", dirPath) - } + assert.True(t, fi.IsDir(), "expected to be directory: %s", dirPath) // Check to see if the permissions match - if fi.Mode().String() != "drwxr-xr-x" { - t.Fatalf("permissions are wrong for: %s. Got: %s", dirPath, fi.Mode().String()) - } + assert.Equal(t, "drwxr-xr-x", fi.Mode().String(), "permissions are wrong for: %s. Got: %s", dirPath, fi.Mode().String()) } func TestCreatePrivateDirectory(t *testing.T) { @@ -375,9 +300,7 @@ func TestCreatePrivateDirectory(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) defer os.RemoveAll(tempBaseDir) dirPath := filepath.Join(tempBaseDir, testDir) @@ -387,19 +310,13 @@ func TestCreatePrivateDirectory(t *testing.T) { // Check to see if file exists fi, err := os.Stat(dirPath) - if err != nil { - t.Fatalf("expected find directory: %s", dirPath) - } + assert.NoError(t, err) // Check to see if it is a directory - if !fi.IsDir() { - t.Fatalf("expected to be directory: %s", dirPath) - } + assert.True(t, fi.IsDir(), "expected to be directory: %s", dirPath) // Check to see if the permissions match - if fi.Mode().String() != "drwx------" { - t.Fatalf("permissions are wrong for: %s. Got: %s", dirPath, fi.Mode().String()) - } + assert.Equal(t, "drwx------", fi.Mode().String(), "permissions are wrong for: %s. Got: %s", dirPath, fi.Mode().String()) } func generateRandomFile(filePath string, perms os.FileMode) ([]byte, error) { @@ -416,3 +333,47 @@ func generateRandomFile(filePath string, perms os.FileMode) ([]byte, error) { return rndBytes, nil } + +func TestFileStoreConsistency(t *testing.T) { + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + assert.NoError(t, err) + defer os.RemoveAll(tempBaseDir) + + s, err := NewPrivateSimpleFileStore(tempBaseDir, "txt") + assert.NoError(t, err) + + file1Data := make([]byte, 20) + _, err = rand.Read(file1Data) + assert.NoError(t, err) + + file2Data := make([]byte, 20) + _, err = rand.Read(file2Data) + assert.NoError(t, err) + + file3Data := make([]byte, 20) + _, err = rand.Read(file3Data) + assert.NoError(t, err) + + file1Path := "file1" + file2Path := "path/file2" + file3Path := "long/path/file3" + + s.Add(file1Path, file1Data) + s.Add(file2Path, file2Data) + s.Add(file3Path, file3Data) + + paths := map[string][]byte{ + file1Path: file1Data, + file2Path: file2Data, + file3Path: file3Data, + } + for _, p := range s.ListFiles() { + _, ok := paths[p] + assert.True(t, ok, fmt.Sprintf("returned path not found: %s", p)) + d, err := s.Get(p) + assert.NoError(t, err) + assert.Equal(t, paths[p], d) + } + +} diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 58edaea1fd..d087afe910 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -54,10 +54,10 @@ func (s *KeyFileStore) Name() string { } // 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 { +func (s *KeyFileStore) AddKey(name, role string, privKey data.PrivateKey) error { s.Lock() defer s.Unlock() - return addKey(s, s.Retriever, s.cachedKeys, name, alias, privKey) + return addKey(s, s.Retriever, s.cachedKeys, name, role, privKey) } // GetKey returns the PrivateKey given a KeyID @@ -153,7 +153,7 @@ func (s *KeyMemoryStore) ImportKey(pemBytes []byte, alias string) error { return importKey(s, s.Retriever, s.cachedKeys, alias, pemBytes) } -func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error { +func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, name, role string, privKey data.PrivateKey) error { var ( chosenPassphrase string @@ -162,7 +162,7 @@ func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached ) for attempts := 0; ; attempts++ { - chosenPassphrase, giveup, err = passphraseRetriever(name, alias, true, attempts) + chosenPassphrase, giveup, err = passphraseRetriever(name, role, true, attempts) if err != nil { continue } @@ -175,7 +175,7 @@ func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached break } - return encryptAndAddKey(s, chosenPassphrase, cachedKeys, name, alias, privKey) + return encryptAndAddKey(s, chosenPassphrase, cachedKeys, name, role, privKey) } func getKeyAlias(s LimitedFileStore, keyID string) (string, error) { @@ -234,9 +234,7 @@ func listKeys(s LimitedFileStore) map[string]string { f = strings.TrimPrefix(f, nonRootKeysSubdir+"/") } - // Remove the extension from the full filename - // abcde_root.key becomes abcde_root - keyIDFull := strings.TrimSpace(strings.TrimSuffix(f, filepath.Ext(f))) + keyIDFull := strings.TrimSpace(f) // If the key does not have a _, it is malformed underscoreIndex := strings.LastIndex(keyIDFull, "_") diff --git a/trustmanager/memorystore.go b/trustmanager/memorystore.go index 3509035946..ffb825fa4a 100644 --- a/trustmanager/memorystore.go +++ b/trustmanager/memorystore.go @@ -1,7 +1,7 @@ package trustmanager import ( - "errors" + "os" "sync" ) @@ -20,10 +20,6 @@ func NewMemoryFileStore() *MemoryFileStore { } } -// ErrMemFileNotFound is returned for a nonexistent "file" in the memory file -// store -var ErrMemFileNotFound = errors.New("key not found in memory file store") - // Add writes data to a file with a given name func (f *MemoryFileStore) Add(name string, data []byte) error { f.Lock() @@ -39,7 +35,7 @@ func (f *MemoryFileStore) Remove(name string) error { defer f.Unlock() if _, present := f.files[name]; !present { - return ErrMemFileNotFound + return os.ErrNotExist } delete(f.files, name) @@ -53,7 +49,7 @@ func (f *MemoryFileStore) Get(name string) ([]byte, error) { fileData, present := f.files[name] if !present { - return nil, ErrMemFileNotFound + return nil, os.ErrNotExist } return fileData, nil diff --git a/trustmanager/store.go b/trustmanager/store.go index f43c6d219b..6672449cf9 100644 --- a/trustmanager/store.go +++ b/trustmanager/store.go @@ -19,9 +19,25 @@ var ( // LimitedFileStore implements the bare bones primitives (no hierarchy) type LimitedFileStore interface { + // Add writes a file to the specified location, returning an error if this + // is not possible (reasons may include permissions errors). The path is cleaned + // before being made absolute against the store's base dir. Add(fileName string, data []byte) error + + // Remove deletes a file from the store relative to the store's base directory. + // The path is cleaned before being made absolute to ensure no path traversal + // outside the base directory is possible. Remove(fileName string) error + + // Get returns the file content found at fileName relative to the base directory + // of the file store. The path is cleaned before being made absolute to ensure + // path traversal outside the store is not possible. If the file is not found + // an error to that effect is returned. Get(fileName string) ([]byte, error) + + // ListFiles returns a list of paths relative to the base directory of the + // filestore. Any of these paths must be retrievable via the + // LimitedFileStore.Get method. ListFiles() []string } diff --git a/trustmanager/x509utils.go b/trustmanager/x509utils.go index 917515e809..09b65d6258 100644 --- a/trustmanager/x509utils.go +++ b/trustmanager/x509utils.go @@ -15,7 +15,6 @@ import ( "math/big" "net/http" "net/url" - "path/filepath" "time" "github.com/Sirupsen/logrus" @@ -117,11 +116,15 @@ func fingerprintCert(cert *x509.Certificate) (CertID, error) { // loadCertsFromDir receives a store AddCertFromFile for each certificate found func loadCertsFromDir(s *X509FileStore) error { - certFiles := s.fileStore.ListFiles() - for _, f := range certFiles { + for _, f := range s.fileStore.ListFiles() { // ListFiles returns relative paths - fullPath := filepath.Join(s.fileStore.BaseDir(), f) - err := s.AddCertFromFile(fullPath) + data, err := s.fileStore.Get(f) + if err != nil { + // the filestore told us it had a file that it then couldn't serve. + // this is a serious problem so error immediately + return err + } + err = s.AddCertFromPEM(data) if err != nil { if _, ok := err.(*ErrCertValidation); ok { logrus.Debugf("ignoring certificate, did not pass validation: %s", f) From 6d5b8ff54a07c70eb16e5760f8f7373644af6f0c Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 21 Dec 2015 23:00:06 -0800 Subject: [PATCH 3/8] add role into PEM headers Signed-off-by: David Lawrence (github: endophage) --- cmd/notary/integration_test.go | 2 +- cryptoservice/import_export_test.go | 2 +- trustmanager/keyfilestore.go | 10 +++++----- trustmanager/keyfilestore_test.go | 6 ++++-- trustmanager/x509utils.go | 19 ++++++++++++++++--- trustmanager/x509utils_test.go | 12 ++++++------ trustmanager/yubikey/yubikeystore_test.go | 8 ++++---- 7 files changed, 37 insertions(+), 22 deletions(-) diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index ef257325e3..aaefadeb6d 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -465,7 +465,7 @@ func TestClientKeyImportExportRootOnly(t *testing.T) { privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err) - pemBytes, err := trustmanager.EncryptPrivateKey(privKey, testPassphrase) + pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "root", testPassphrase) assert.NoError(t, err) nBytes, err := tempFile.Write(pemBytes) diff --git a/cryptoservice/import_export_test.go b/cryptoservice/import_export_test.go index e6e762581f..5b4399b5cc 100644 --- a/cryptoservice/import_export_test.go +++ b/cryptoservice/import_export_test.go @@ -328,7 +328,7 @@ func TestImportExportRootKey(t *testing.T) { 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) + decryptedPEMBytes, err := trustmanager.KeyToPEM(privKey, "root") assert.NoError(t, err, "could not convert key to PEM") err = cs2.ImportRootKey(bytes.NewReader(decryptedPEMBytes)) diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index d087afe910..bd9612a472 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -333,7 +333,7 @@ func GetPasswdDecryptBytes(passphraseRetriever passphrase.Retriever, pemBytes [] return privKey, passwd, nil } -func encryptAndAddKey(s LimitedFileStore, passwd string, cachedKeys map[string]*cachedKey, name, alias string, privKey data.PrivateKey) error { +func encryptAndAddKey(s LimitedFileStore, passwd string, cachedKeys map[string]*cachedKey, name, role string, privKey data.PrivateKey) error { var ( pemPrivKey []byte @@ -341,17 +341,17 @@ func encryptAndAddKey(s LimitedFileStore, passwd string, cachedKeys map[string]* ) if passwd != "" { - pemPrivKey, err = EncryptPrivateKey(privKey, passwd) + pemPrivKey, err = EncryptPrivateKey(privKey, role, passwd) } else { - pemPrivKey, err = KeyToPEM(privKey) + pemPrivKey, err = KeyToPEM(privKey, role) } if err != nil { return err } - cachedKeys[name] = &cachedKey{alias: alias, key: privKey} - return s.Add(filepath.Join(getSubdir(alias), name+"_"+alias), pemPrivKey) + cachedKeys[name] = &cachedKey{alias: role, key: privKey} + return s.Add(filepath.Join(getSubdir(role), name+"_"+role), pemPrivKey) } func importKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, alias string, pemBytes []byte) error { diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index dabfb98e62..87cf24dad3 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -55,6 +55,8 @@ func TestAddKey(t *testing.T) { func TestGet(t *testing.T) { testData := []byte(`-----BEGIN RSA PRIVATE KEY----- +role: root + MIIEogIBAAKCAQEAyUIXjsrWRrvPa4Bzp3VJ6uOUGPay2fUpSV8XzNxZxIG/Opdr +k3EQi1im6WOqF3Y5AS1UjYRxNuRN+cAZeo3uS1pOTuoSupBXuchVw8s4hZJ5vXn TRmGb+xY7tZ1ZVgPfAZDib9sRSUsL/gC+aSyprAjG/YBdbF06qKbfOfsoCEYW1OQ @@ -109,7 +111,7 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0= privKey, _, err := store.GetKey(testName) assert.NoError(t, err, "failed to get key from store") - pemPrivKey, err := KeyToPEM(privKey) + pemPrivKey, err := KeyToPEM(privKey, testAlias) assert.NoError(t, err, "failed to convert key to PEM") assert.Equal(t, testData, pemPrivKey) } @@ -513,7 +515,7 @@ func assertExportKeySuccess( func assertImportKeySuccess( t *testing.T, s KeyStore, expectedKey data.PrivateKey) { - pemBytes, err := EncryptPrivateKey(expectedKey, cannedPassphrase) + pemBytes, err := EncryptPrivateKey(expectedKey, "root", cannedPassphrase) assert.NoError(t, err) err = s.ImportKey(pemBytes, "root") diff --git a/trustmanager/x509utils.go b/trustmanager/x509utils.go index 09b65d6258..4fbbff987e 100644 --- a/trustmanager/x509utils.go +++ b/trustmanager/x509utils.go @@ -414,18 +414,26 @@ func blockType(k data.PrivateKey) (string, error) { } // KeyToPEM returns a PEM encoded key from a Private Key -func KeyToPEM(privKey data.PrivateKey) ([]byte, error) { +func KeyToPEM(privKey data.PrivateKey, role string) ([]byte, error) { bt, err := blockType(privKey) if err != nil { return nil, err } - return pem.EncodeToMemory(&pem.Block{Type: bt, Bytes: privKey.Private()}), nil + block := &pem.Block{ + Type: bt, + Headers: map[string]string{ + "role": role, + }, + Bytes: privKey.Private(), + } + + return pem.EncodeToMemory(block), nil } // EncryptPrivateKey returns an encrypted PEM key given a Privatekey // and a passphrase -func EncryptPrivateKey(key data.PrivateKey, passphrase string) ([]byte, error) { +func EncryptPrivateKey(key data.PrivateKey, role, passphrase string) ([]byte, error) { bt, err := blockType(key) if err != nil { return nil, err @@ -443,6 +451,11 @@ func EncryptPrivateKey(key data.PrivateKey, passphrase string) ([]byte, error) { return nil, err } + if encryptedPEMBlock.Headers == nil { + encryptedPEMBlock.Headers = make(map[string]string) + } + encryptedPEMBlock.Headers["role"] = role + return pem.EncodeToMemory(encryptedPEMBlock), nil } diff --git a/trustmanager/x509utils_test.go b/trustmanager/x509utils_test.go index cba6e84a6d..738a4f1c0c 100644 --- a/trustmanager/x509utils_test.go +++ b/trustmanager/x509utils_test.go @@ -69,15 +69,15 @@ func TestKeyOperations(t *testing.T) { rsaKey, err := GenerateRSAKey(rand.Reader, 512) // Encode our ED private key - edPEM, err := KeyToPEM(edKey) + edPEM, err := KeyToPEM(edKey, "root") assert.NoError(t, err) // Encode our EC private key - ecPEM, err := KeyToPEM(ecKey) + ecPEM, err := KeyToPEM(ecKey, "root") assert.NoError(t, err) // Encode our RSA private key - rsaPEM, err := KeyToPEM(rsaKey) + rsaPEM, err := KeyToPEM(rsaKey, "root") assert.NoError(t, err) // Check to see if ED key it is encoded @@ -108,15 +108,15 @@ func TestKeyOperations(t *testing.T) { assert.Equal(t, rsaKey.Private(), decodedRSAKey.Private()) // Encrypt our ED Key - encryptedEDKey, err := EncryptPrivateKey(edKey, "ponies") + encryptedEDKey, err := EncryptPrivateKey(edKey, "root", "ponies") assert.NoError(t, err) // Encrypt our EC Key - encryptedECKey, err := EncryptPrivateKey(ecKey, "ponies") + encryptedECKey, err := EncryptPrivateKey(ecKey, "root", "ponies") assert.NoError(t, err) // Encrypt our RSA Key - encryptedRSAKey, err := EncryptPrivateKey(rsaKey, "ponies") + encryptedRSAKey, err := EncryptPrivateKey(rsaKey, "root", "ponies") assert.NoError(t, err) // Check to see if ED key it is encrypted diff --git a/trustmanager/yubikey/yubikeystore_test.go b/trustmanager/yubikey/yubikeystore_test.go index 621b8dd11e..95d45af8bd 100644 --- a/trustmanager/yubikey/yubikeystore_test.go +++ b/trustmanager/yubikey/yubikeystore_test.go @@ -339,7 +339,7 @@ func TestYubiImportNewKey(t *testing.T) { privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err) - pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "root", "passphrase") assert.NoError(t, err) err = store.ImportKey(pemBytes, "root") @@ -388,7 +388,7 @@ func TestYubiImportExistingKey(t *testing.T) { assert.NotNil(t, k) // import the key, which should have already been added to the yubikey - pemBytes, err := trustmanager.EncryptPrivateKey(key, "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(key, "root", "passphrase") assert.NoError(t, err) err = newStore.ImportKey(pemBytes, "root") assert.NoError(t, err) @@ -418,7 +418,7 @@ func TestYubiImportNonRootKey(t *testing.T) { privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err) - pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "root", "passphrase") assert.NoError(t, err) err = store.ImportKey(pemBytes, privKey.ID()) @@ -690,7 +690,7 @@ func TestYubiImportKeyCleansUpOnError(t *testing.T) { privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err) - pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "root", "passphrase") assert.NoError(t, err) var _importkey = func() error { return store.ImportKey(pemBytes, "root") } From f2ec72b5b6e615f59cead320a91f0cba17b52c8b Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Tue, 22 Dec 2015 03:00:26 -0800 Subject: [PATCH 4/8] aliases removed from file names Signed-off-by: David Lawrence (github: endophage) --- cmd/notary/integration_test.go | 2 +- cryptoservice/import_export_test.go | 16 +++---- trustmanager/keyfilestore.go | 71 +++++++++++++++++++---------- trustmanager/keyfilestore_test.go | 9 ++-- 4 files changed, 59 insertions(+), 39 deletions(-) diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index aaefadeb6d..9392cd1c92 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -223,7 +223,7 @@ func assertNumKeys(t *testing.T, tempDir string, numRoot, numSigning int, assert.Len(t, signing, numSigning) for _, rootKeyID := range root { _, err := os.Stat(filepath.Join( - tempDir, "private", "root_keys", rootKeyID+"_root.key")) + tempDir, "private", "root_keys", rootKeyID+".key")) // os.IsExist checks to see if the error is because a file already // exist, and hence doesn't actually the right funciton to use here assert.Equal(t, rootOnDisk, !os.IsNotExist(err)) diff --git a/cryptoservice/import_export_test.go b/cryptoservice/import_export_test.go index 5b4399b5cc..98ab514d00 100644 --- a/cryptoservice/import_export_test.go +++ b/cryptoservice/import_export_test.go @@ -80,13 +80,13 @@ func TestImportExportZip(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") + relKeyPath := filepath.Join("tuf_keys", privKeyName+".key") passphraseByFile[relKeyPath] = exportPassphrase } // Add root key to the map. This will use the export passphrase because it // will be reencrypted. - relRootKey := filepath.Join("root_keys", rootKeyID+"_root.key") + relRootKey := filepath.Join("root_keys", rootKeyID+".key") passphraseByFile[relRootKey] = exportPassphrase // Iterate through the files in the archive, checking that the files @@ -145,7 +145,7 @@ func TestImportExportZip(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") + relKeyPath := filepath.Join("tuf_keys", privKeyName+".key") privKeyFileName := filepath.Join(tempBaseDir2, "private", relKeyPath) _, err = os.Stat(privKeyFileName) assert.NoError(t, err, "missing private key for role %s: %s", alias, privKeyName) @@ -154,7 +154,7 @@ func TestImportExportZip(t *testing.T) { // Look for keys in root_keys // There should be a file named after the key ID of the root key we // passed in. - rootKeyFilename := rootKeyID + "_root.key" + rootKeyFilename := rootKeyID + ".key" _, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename)) assert.NoError(t, err, "missing root key") } @@ -205,7 +205,7 @@ func TestImportExportGUN(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") + relKeyPath := filepath.Join("tuf_keys", privKeyName+".key") passphraseByFile[relKeyPath] = exportPassphrase } @@ -270,7 +270,7 @@ func TestImportExportGUN(t *testing.T) { if alias == "root" { continue } - relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key") + relKeyPath := filepath.Join("tuf_keys", privKeyName+".key") privKeyFileName := filepath.Join(tempBaseDir2, "private", relKeyPath) _, err = os.Stat(privKeyFileName) assert.NoError(t, err) @@ -318,7 +318,7 @@ func TestImportExportRootKey(t *testing.T) { // 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 + "_root.key" + rootKeyFilename := rootKeyID + ".key" _, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename)) assert.NoError(t, err, "missing root key") @@ -386,7 +386,7 @@ func TestImportExportRootKeyReencrypt(t *testing.T) { // 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 + "_root.key" + rootKeyFilename := rootKeyID + ".key" _, err = os.Stat(filepath.Join(tempBaseDir2, "private", "root_keys", rootKeyFilename)) assert.NoError(t, err, "missing root key") diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index bd9612a472..1eb6556c6d 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -1,11 +1,13 @@ package trustmanager import ( + "encoding/pem" "fmt" "path/filepath" "strings" "sync" + "github.com/Sirupsen/logrus" "github.com/docker/notary/passphrase" "github.com/docker/notary/tuf/data" ) @@ -179,14 +181,23 @@ func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached } func getKeyAlias(s LimitedFileStore, keyID string) (string, error) { - files := s.ListFiles() - name := strings.TrimSpace(strings.TrimSuffix(filepath.Base(keyID), filepath.Ext(keyID))) - for _, file := range files { + for _, file := range s.ListFiles() { filename := filepath.Base(file) if strings.HasPrefix(filename, name) { + d, err := s.Get(file) + if err != nil { + return "", err + } + block, _ := pem.Decode(d) + if block != nil { + if role, ok := block.Headers["role"]; ok { + return role, nil + } + } + aliasPlusDotKey := strings.TrimPrefix(filename, name+"_") retVal := strings.TrimSuffix(aliasPlusDotKey, "."+keyExtension) return retVal, nil @@ -208,14 +219,13 @@ func getKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached return nil, "", err } - var retErr error // See if the key is encrypted. If its encrypted we'll fail to parse the private key privKey, err := ParsePEMPrivateKey(keyBytes, "") if err != nil { - privKey, _, retErr = GetPasswdDecryptBytes(passphraseRetriever, keyBytes, name, string(keyAlias)) - } - if retErr != nil { - return nil, "", retErr + privKey, _, err = GetPasswdDecryptBytes(passphraseRetriever, keyBytes, name, string(keyAlias)) + if err != nil { + return nil, "", err + } } cachedKeys[name] = &cachedKey{alias: keyAlias, key: privKey} return privKey, keyAlias, nil @@ -228,26 +238,39 @@ func listKeys(s LimitedFileStore) map[string]string { for _, f := range s.ListFiles() { // Remove the prefix of the directory from the filename - if f[:len(rootKeysSubdir)] == rootKeysSubdir { - f = strings.TrimPrefix(f, rootKeysSubdir+"/") + var keyIDFull string + if strings.HasPrefix(f, rootKeysSubdir+"/") { + keyIDFull = strings.TrimPrefix(f, rootKeysSubdir+"/") } else { - f = strings.TrimPrefix(f, nonRootKeysSubdir+"/") + keyIDFull = strings.TrimPrefix(f, nonRootKeysSubdir+"/") } - keyIDFull := strings.TrimSpace(f) + keyIDFull = strings.TrimSpace(keyIDFull) // If the key does not have a _, it is malformed underscoreIndex := strings.LastIndex(keyIDFull, "_") if underscoreIndex == -1 { - continue + keyID := keyIDFull + d, err := s.Get(f) + if err != nil { + logrus.Error(err) + continue + } + block, _ := pem.Decode(d) + if block == nil { + continue + } + if role, ok := block.Headers["role"]; ok { + keyIDMap[keyID] = role + } + } else { + // The keyID is the first part of the keyname + // The KeyAlias is the second part of the keyname + // in a key named abcde_root, abcde is the keyID and root is the KeyAlias + keyID := keyIDFull[:underscoreIndex] + keyAlias := keyIDFull[underscoreIndex+1:] + keyIDMap[keyID] = keyAlias } - - // The keyID is the first part of the keyname - // The KeyAlias is the second part of the keyname - // in a key named abcde_root, abcde is the keyID and root is the KeyAlias - keyID := keyIDFull[:underscoreIndex] - keyAlias := keyIDFull[underscoreIndex+1:] - keyIDMap[keyID] = keyAlias } return keyIDMap } @@ -262,8 +285,7 @@ func removeKey(s LimitedFileStore, cachedKeys map[string]*cachedKey, name string delete(cachedKeys, name) // being in a subdirectory is for backwards compatibliity - filename := name + "_" + keyAlias - err = s.Remove(filepath.Join(getSubdir(keyAlias), filename)) + err = s.Remove(filepath.Join(getSubdir(keyAlias), name)) if err != nil { return err } @@ -286,9 +308,8 @@ func getRawKey(s LimitedFileStore, name string) ([]byte, string, error) { return nil, "", err } - filename := name + "_" + keyAlias var keyBytes []byte - keyBytes, err = s.Get(filepath.Join(getSubdir(keyAlias), filename)) + keyBytes, err = s.Get(filepath.Join(getSubdir(keyAlias), name)) if err != nil { return nil, "", err } @@ -351,7 +372,7 @@ func encryptAndAddKey(s LimitedFileStore, passwd string, cachedKeys map[string]* } cachedKeys[name] = &cachedKey{alias: role, key: privKey} - return s.Add(filepath.Join(getSubdir(role), name+"_"+role), pemPrivKey) + return s.Add(filepath.Join(getSubdir(role), name), pemPrivKey) } func importKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, alias string, pemBytes []byte) error { diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 87cf24dad3..fad34f3087 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -26,7 +26,6 @@ var passphraseRetriever = func(keyID string, alias string, createNew bool, numAt func TestAddKey(t *testing.T) { testName := "docker.com/notary/root" testExt := "key" - testAlias := "root" // Temporary directory where test files will be created tempBaseDir, err := ioutil.TempDir("", "notary-test-") @@ -34,7 +33,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, privDir, rootKeysSubdir, testName+"_"+testAlias+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir, testName+"."+testExt) // Create our store store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever) @@ -97,7 +96,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, privDir, rootKeysSubdir, testName+"_"+testAlias+"."+testExt) + filePath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir, testName+"."+testExt) os.MkdirAll(filepath.Dir(filePath), perms) err = ioutil.WriteFile(filePath, testData, perms) @@ -215,7 +214,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, privDir, rootKeysSubdir, privKey.ID()+"_"+testAlias+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, privDir, rootKeysSubdir, privKey.ID()+"."+testExt) // Get file description, open file fp, err := os.OpenFile(expectedFilePath, os.O_WRONLY, 0600) @@ -322,7 +321,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, privDir, nonRootKeysSubdir, testName+"_"+testAlias+"."+testExt) + expectedFilePath := filepath.Join(tempBaseDir, privDir, nonRootKeysSubdir, testName+"."+testExt) // Create our store store, err := NewKeyFileStore(tempBaseDir, passphraseRetriever) From e516dd88f2515b592d809ec5e0ba3bedc8bc0480 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Tue, 22 Dec 2015 13:58:29 -0800 Subject: [PATCH 5/8] cleaning up tests by converting t.Fatal to assert.___ Signed-off-by: David Lawrence (github: endophage) --- client/client_test.go | 10 ++++------ cryptoservice/import_export_test.go | 24 ++++++------------------ server/server_test.go | 18 +++++++++--------- 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 1c868be8cf..4aa032639a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1064,12 +1064,10 @@ func testValidateRootKey(t *testing.T, rootType string) { assert.NotEmpty(t, keyids) for _, keyid := range keyids { - if key, ok := decodedRoot.Keys[keyid]; !ok { - t.Fatal("key id not found in keys") - } else { - _, err := trustmanager.LoadCertFromPEM(key.Public()) - assert.NoError(t, err, "key is not a valid cert") - } + key, ok := decodedRoot.Keys[keyid] + assert.True(t, ok, "key id not found in keys") + _, err := trustmanager.LoadCertFromPEM(key.Public()) + assert.NoError(t, err, "key is not a valid cert") } } diff --git a/cryptoservice/import_export_test.go b/cryptoservice/import_export_test.go index 98ab514d00..5b48216277 100644 --- a/cryptoservice/import_export_test.go +++ b/cryptoservice/import_export_test.go @@ -93,9 +93,7 @@ func TestImportExportZip(t *testing.T) { // 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) - } + assert.True(t, present, "unexpected file %s in zip file", f.Name) delete(passphraseByFile, f.Name) @@ -114,9 +112,7 @@ func TestImportExportZip(t *testing.T) { 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) - } + assert.Len(t, passphraseByFile, 0) // Create new repo to test import tempBaseDir2, err := ioutil.TempDir("", "notary-test-") @@ -199,9 +195,7 @@ func TestImportExportGUN(t *testing.T) { privKeyMap := cs.ListAllKeys() for privKeyName := range privKeyMap { _, alias, err := cs.GetPrivateKey(privKeyName) - if err != nil { - t.Fatalf("privKey %s has no alias", privKeyName) - } + assert.NoError(t, err, "privKey %s has no alias", privKeyName) if alias == "root" { continue } @@ -215,9 +209,7 @@ func TestImportExportGUN(t *testing.T) { for _, f := range zipReader.File { expectedPassphrase, present := passphraseByFile[f.Name] - if !present { - t.Fatalf("unexpected file %s in zip file", f.Name) - } + assert.True(t, present, "unexpected file %s in zip file", f.Name) delete(passphraseByFile, f.Name) @@ -236,9 +228,7 @@ func TestImportExportGUN(t *testing.T) { 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) - } + assert.Len(t, passphraseByFile, 0) // Create new repo to test import tempBaseDir2, err := ioutil.TempDir("", "notary-test-") @@ -264,9 +254,7 @@ func TestImportExportGUN(t *testing.T) { continue } _, alias, err := cs2.GetPrivateKey(privKeyName) - if err != nil { - t.Fatalf("privKey %s has no alias", privKeyName) - } + assert.NoError(t, err, "privKey %s has no alias", privKeyName) if alias == "root" { continue } diff --git a/server/server_test.go b/server/server_test.go index 978d952b51..8b847b5c71 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -25,9 +25,7 @@ func TestRunBadAddr(t *testing.T) { "", nil, ) - if err == nil { - t.Fatal("Passed bad addr, Run should have failed") - } + assert.Error(t, err, "Passed bad addr, Run should have failed") } func TestRunReservedPort(t *testing.T) { @@ -42,12 +40,14 @@ func TestRunReservedPort(t *testing.T) { nil, ) - if _, ok := err.(*net.OpError); !ok { - t.Fatalf("Received unexpected err: %s", err.Error()) - } - if !strings.Contains(err.Error(), "bind: permission denied") { - t.Fatalf("Received unexpected err: %s", err.Error()) - } + assert.Error(t, err) + assert.IsType(t, &net.OpError{}, err) + assert.True( + t, + strings.Contains(err.Error(), "bind: permission denied"), + "Received unexpected err: %s", + err.Error(), + ) } func TestMetricsEndpoint(t *testing.T) { From 2bf5d4b09a730997f1c3f35218ecc8260ab7cf03 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Tue, 22 Dec 2015 14:22:42 -0800 Subject: [PATCH 6/8] test for legacy keys and some bugfixes for same Signed-off-by: David Lawrence (github: endophage) --- trustmanager/filestore_test.go | 37 +++++++++++-------- trustmanager/keyfilestore.go | 38 +++++++++++++------- trustmanager/keyfilestore_test.go | 60 +++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 27 deletions(-) diff --git a/trustmanager/filestore_test.go b/trustmanager/filestore_test.go index e96bc04621..0623c2384c 100644 --- a/trustmanager/filestore_test.go +++ b/trustmanager/filestore_test.go @@ -340,9 +340,16 @@ func TestFileStoreConsistency(t *testing.T) { assert.NoError(t, err) defer os.RemoveAll(tempBaseDir) + tempBaseDir2, err := ioutil.TempDir("", "notary-test-") + assert.NoError(t, err) + defer os.RemoveAll(tempBaseDir2) + s, err := NewPrivateSimpleFileStore(tempBaseDir, "txt") assert.NoError(t, err) + s2, err := NewPrivateSimpleFileStore(tempBaseDir2, ".txt") + assert.NoError(t, err) + file1Data := make([]byte, 20) _, err = rand.Read(file1Data) assert.NoError(t, err) @@ -359,21 +366,23 @@ func TestFileStoreConsistency(t *testing.T) { file2Path := "path/file2" file3Path := "long/path/file3" - s.Add(file1Path, file1Data) - s.Add(file2Path, file2Data) - s.Add(file3Path, file3Data) + for _, s := range []LimitedFileStore{s, s2} { + s.Add(file1Path, file1Data) + s.Add(file2Path, file2Data) + s.Add(file3Path, file3Data) - paths := map[string][]byte{ - file1Path: file1Data, - file2Path: file2Data, - file3Path: file3Data, - } - for _, p := range s.ListFiles() { - _, ok := paths[p] - assert.True(t, ok, fmt.Sprintf("returned path not found: %s", p)) - d, err := s.Get(p) - assert.NoError(t, err) - assert.Equal(t, paths[p], d) + paths := map[string][]byte{ + file1Path: file1Data, + file2Path: file2Data, + file3Path: file3Data, + } + for _, p := range s.ListFiles() { + _, ok := paths[p] + assert.True(t, ok, fmt.Sprintf("returned path not found: %s", p)) + d, err := s.Get(p) + assert.NoError(t, err) + assert.Equal(t, paths[p], d) + } } } diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 1eb6556c6d..9e1c53d7ee 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -180,7 +180,11 @@ func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cached return encryptAndAddKey(s, chosenPassphrase, cachedKeys, name, role, privKey) } -func getKeyAlias(s LimitedFileStore, keyID string) (string, error) { +// getKeyRole finds the role for the given keyID. It attempts to look +// both in the newer format PEM headers, and also in the legacy filename +// format. It returns: the role, whether it was found in the legacy format +// (true == legacy), and an error +func getKeyRole(s LimitedFileStore, keyID string) (string, bool, error) { name := strings.TrimSpace(strings.TrimSuffix(filepath.Base(keyID), filepath.Ext(keyID))) for _, file := range s.ListFiles() { @@ -189,22 +193,21 @@ func getKeyAlias(s LimitedFileStore, keyID string) (string, error) { if strings.HasPrefix(filename, name) { d, err := s.Get(file) if err != nil { - return "", err + return "", false, err } block, _ := pem.Decode(d) if block != nil { if role, ok := block.Headers["role"]; ok { - return role, nil + return role, false, nil } } - aliasPlusDotKey := strings.TrimPrefix(filename, name+"_") - retVal := strings.TrimSuffix(aliasPlusDotKey, "."+keyExtension) - return retVal, nil + role := strings.TrimPrefix(filename, name+"_") + return role, true, nil } } - return "", &ErrKeyNotFound{KeyID: keyID} + return "", false, &ErrKeyNotFound{KeyID: keyID} } // GetKey returns the PrivateKey given a KeyID @@ -247,7 +250,8 @@ func listKeys(s LimitedFileStore) map[string]string { keyIDFull = strings.TrimSpace(keyIDFull) - // If the key does not have a _, it is malformed + // If the key does not have a _, we'll attempt to + // read it as a PEM underscoreIndex := strings.LastIndex(keyIDFull, "_") if underscoreIndex == -1 { keyID := keyIDFull @@ -277,15 +281,19 @@ func listKeys(s LimitedFileStore) map[string]string { // RemoveKey removes the key from the keyfilestore func removeKey(s LimitedFileStore, cachedKeys map[string]*cachedKey, name string) error { - keyAlias, err := getKeyAlias(s, name) + role, legacy, err := getKeyRole(s, name) if err != nil { return err } delete(cachedKeys, name) + if legacy { + name = name + "_" + role + } + // being in a subdirectory is for backwards compatibliity - err = s.Remove(filepath.Join(getSubdir(keyAlias), name)) + err = s.Remove(filepath.Join(getSubdir(role), name)) if err != nil { return err } @@ -303,17 +311,21 @@ func getSubdir(alias string) string { // Given a key ID, gets the bytes and alias belonging to that key if the key // exists func getRawKey(s LimitedFileStore, name string) ([]byte, string, error) { - keyAlias, err := getKeyAlias(s, name) + role, legacy, err := getKeyRole(s, name) if err != nil { return nil, "", err } + if legacy { + name = name + "_" + role + } + var keyBytes []byte - keyBytes, err = s.Get(filepath.Join(getSubdir(keyAlias), name)) + keyBytes, err = s.Get(filepath.Join(getSubdir(role), name)) if err != nil { return nil, "", err } - return keyBytes, keyAlias, nil + return keyBytes, role, nil } // GetPasswdDecryptBytes gets the password to decript the given pem bytes. diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index fad34f3087..05f26572df 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -115,6 +115,66 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0= assert.Equal(t, testData, pemPrivKey) } +// TestGetLegacyKey ensures we can still load keys where the role +// is stored as part of the filename (i.e. _.key +func TestGetLegacyKey(t *testing.T) { + testData := []byte(`-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAyUIXjsrWRrvPa4Bzp3VJ6uOUGPay2fUpSV8XzNxZxIG/Opdr ++k3EQi1im6WOqF3Y5AS1UjYRxNuRN+cAZeo3uS1pOTuoSupBXuchVw8s4hZJ5vXn +TRmGb+xY7tZ1ZVgPfAZDib9sRSUsL/gC+aSyprAjG/YBdbF06qKbfOfsoCEYW1OQ +82JqHzQH514RFYPTnEGpvfxWaqmFQLmv0uMxV/cAYvqtrGkXuP0+a8PknlD2obw5 +0rHE56Su1c3Q42S7L51K38tpbgWOSRcTfDUWEj5v9wokkNQvyKBwbS996s4EJaZd +7r6M0h1pHnuRxcSaZLYRwgOe1VNGg2VfWzgd5QIDAQABAoIBAF9LGwpygmj1jm3R +YXGd+ITugvYbAW5wRb9G9mb6wspnwNsGTYsz/UR0ZudZyaVw4jx8+jnV/i3e5PC6 +QRcAgqf8l4EQ/UuThaZg/AlT1yWp9g4UyxNXja87EpTsGKQGwTYxZRM4/xPyWOzR +mt8Hm8uPROB9aA2JG9npaoQG8KSUj25G2Qot3ukw/IOtqwN/Sx1EqF0EfCH1K4KU +a5TrqlYDFmHbqT1zTRec/BTtVXNsg8xmF94U1HpWf3Lpg0BPYT7JiN2DPoLelRDy +a/A+a3ZMRNISL5wbq/jyALLOOyOkIqa+KEOeW3USuePd6RhDMzMm/0ocp5FCwYfo +k4DDeaECgYEA0eSMD1dPGo+u8UTD8i7ZsZCS5lmXLNuuAg5f5B/FGghD8ymPROIb +dnJL5QSbUpmBsYJ+nnO8RiLrICGBe7BehOitCKi/iiZKJO6edrfNKzhf4XlU0HFl +jAOMa975pHjeCoZ1cXJOEO9oW4SWTCyBDBSqH3/ZMgIOiIEk896lSmkCgYEA9Xf5 +Jqv3HtQVvjugV/axAh9aI8LMjlfFr9SK7iXpY53UdcylOSWKrrDok3UnrSEykjm7 +UL3eCU5jwtkVnEXesNn6DdYo3r43E6iAiph7IBkB5dh0yv3vhIXPgYqyTnpdz4pg +3yPGBHMPnJUBThg1qM7k6a2BKHWySxEgC1DTMB0CgYAGvdmF0J8Y0k6jLzs/9yNE +4cjmHzCM3016gW2xDRgumt9b2xTf+Ic7SbaIV5qJj6arxe49NqhwdESrFohrKaIP +kM2l/o2QaWRuRT/Pvl2Xqsrhmh0QSOQjGCYVfOb10nAHVIRHLY22W4o1jk+piLBo +a+1+74NRaOGAnu1J6/fRKQKBgAF180+dmlzemjqFlFCxsR/4G8s2r4zxTMXdF+6O +3zKuj8MbsqgCZy7e8qNeARxwpCJmoYy7dITNqJ5SOGSzrb2Trn9ClP+uVhmR2SH6 +AlGQlIhPn3JNzI0XVsLIloMNC13ezvDE/7qrDJ677EQQtNEKWiZh1/DrsmHr+irX +EkqpAoGAJWe8PC0XK2RE9VkbSPg9Ehr939mOLWiHGYTVWPttUcum/rTKu73/X/mj +WxnPWGtzM1pHWypSokW90SP4/xedMxludvBvmz+CTYkNJcBGCrJumy11qJhii9xp +EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0= +-----END RSA PRIVATE KEY----- +`) + testName := "docker.com/notary/root" + testExt := "key" + testAlias := "root" + perms := os.FileMode(0755) + + emptyPassphraseRetriever := func(string, string, bool, int) (string, bool, error) { return "", false, nil } + + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + 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, privDir, rootKeysSubdir, testName+"_"+testAlias+"."+testExt) + + os.MkdirAll(filepath.Dir(filePath), perms) + err = ioutil.WriteFile(filePath, testData, perms) + assert.NoError(t, err, "failed to write test file") + + // Create our store + store, err := NewKeyFileStore(tempBaseDir, emptyPassphraseRetriever) + assert.NoError(t, err, "failed to create new key filestore") + + // Call the GetKey function + _, role, err := store.GetKey(testName) + assert.NoError(t, err, "failed to get key from store") + assert.Equal(t, testAlias, role) +} + func TestListKeys(t *testing.T) { testName := "docker.com/notary/root" perms := os.FileMode(0755) From fa788cb2a92ab3f6b74d8402ad0ca8bf8b324ce6 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Tue, 22 Dec 2015 14:49:43 -0800 Subject: [PATCH 7/8] make x509 certs viable as delegated public key object Signed-off-by: David Lawrence (github: endophage) --- client/client.go | 2 +- client/client_test.go | 88 +++++++++++++++++++++++++++++++++++++++++++ tuf/tuf.go | 14 +++++-- 3 files changed, 100 insertions(+), 4 deletions(-) diff --git a/client/client.go b/client/client.go index 537673a49e..9d511af31d 100644 --- a/client/client.go +++ b/client/client.go @@ -367,7 +367,7 @@ func (r *NotaryRepository) RemoveDelegation(name string) error { // AddTarget creates new changelist entries to add a target to the given roles // in the repository when the changelist gets appied at publish time. -// If roles are unspecified, the default role is "target". +// If roles are unspecified, the default role is "targets". func (r *NotaryRepository) AddTarget(target *Target, roles ...string) error { cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) diff --git a/client/client_test.go b/client/client_test.go index 4aa032639a..5b82789118 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -15,6 +15,7 @@ import ( "sort" "strings" "testing" + "time" "github.com/Sirupsen/logrus" ctxu "github.com/docker/distribution/context" @@ -1581,6 +1582,93 @@ func TestPublishDelegations(t *testing.T) { } } +// Publishing delegations works so long as the delegation parent exists by the +// time that delegation addition change is applied. Most of the tests for +// applying delegation changes in in helpers_test.go (applyTargets tests), so +// this is just a sanity test to make sure Publish calls it correctly and +// no fallback happens. +func TestPublishDelegationsX509(t *testing.T) { + var tempDirs [2]string + for i := 0; i < 2; i++ { + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + defer os.RemoveAll(tempBaseDir) + tempDirs[i] = tempBaseDir + } + + gun := "docker.com/notary" + ts := fullTestServer(t) + defer ts.Close() + + repo1, _ := initializeRepo(t, data.ECDSAKey, tempDirs[0], gun, ts.URL, false) + delgKey, err := repo1.CryptoService.Create("targets/a", data.ECDSAKey) + assert.NoError(t, err, "error creating delegation key") + + start := time.Now() + privKey, _, err := repo1.CryptoService.GetPrivateKey(delgKey.ID()) + assert.NoError(t, err) + cert, err := cryptoservice.GenerateCertificate( + privKey, "targets/a", start, start.AddDate(1, 0, 0), + ) + assert.NoError(t, err) + delgCert := data.NewECDSAx509PublicKey(trustmanager.CertToPEM(cert)) + + // This should publish fine, even though targets/a/b is dependent upon + // targets/a, because these should execute in order + for _, delgName := range []string{"targets/a", "targets/a/b", "targets/c"} { + assert.NoError(t, + repo1.AddDelegation(delgName, 1, []data.PublicKey{delgCert}, []string{""}), + "error creating delegation") + } + assert.Len(t, getChanges(t, repo1), 3, "wrong number of changelist files found") + assert.NoError(t, repo1.Publish()) + assert.Len(t, getChanges(t, repo1), 0, "wrong number of changelist files found") + + // this should not publish, because targets/z doesn't exist + assert.NoError(t, + repo1.AddDelegation("targets/z/y", 1, []data.PublicKey{delgCert}, []string{""}), + "error creating delegation") + assert.Len(t, getChanges(t, repo1), 1, "wrong number of changelist files found") + assert.Error(t, repo1.Publish()) + assert.Len(t, getChanges(t, repo1), 1, "wrong number of changelist files found") + + // Create a new repo and pull from the server + repo2, err := NewNotaryRepository(tempDirs[1], gun, ts.URL, + http.DefaultTransport, passphraseRetriever) + assert.NoError(t, err, "error creating repository: %s", err) + + // pull + _, err = repo2.ListTargets() + assert.NoError(t, err, "unable to pull repo") + + for _, repo := range []*NotaryRepository{repo1, repo2} { + // targets should have delegations targets/a and targets/c + targets := repo.tufRepo.Targets[data.CanonicalTargetsRole] + assert.Len(t, targets.Signed.Delegations.Roles, 2) + assert.Len(t, targets.Signed.Delegations.Keys, 1) + + _, ok := targets.Signed.Delegations.Keys[delgCert.ID()] + assert.True(t, ok) + + foundRoleNames := make(map[string]bool) + for _, r := range targets.Signed.Delegations.Roles { + foundRoleNames[r.Name] = true + } + assert.True(t, foundRoleNames["targets/a"]) + assert.True(t, foundRoleNames["targets/c"]) + + // targets/a should have delegation targets/a/b only + a := repo.tufRepo.Targets["targets/a"] + assert.Len(t, a.Signed.Delegations.Roles, 1) + assert.Len(t, a.Signed.Delegations.Keys, 1) + + _, ok = a.Signed.Delegations.Keys[delgCert.ID()] + assert.True(t, ok) + + assert.Equal(t, "targets/a/b", a.Signed.Delegations.Roles[0].Name) + } +} + // If a changelist specifies a particular role to push targets to, and there // is no such role, publish will try to publish to its parent. If the parent // doesn't work, it falls back on its parent, and so forth, and eventually diff --git a/tuf/tuf.go b/tuf/tuf.go index aed73621f0..6aa94cc032 100644 --- a/tuf/tuf.go +++ b/tuf/tuf.go @@ -530,9 +530,17 @@ func (tr *Repo) VerifyCanSign(roleName string) error { } for _, keyID := range role.KeyIDs { - p, _, err := tr.cryptoService.GetPrivateKey(keyID) - if err == nil && p != nil { - return nil + k := tr.keysDB.GetKey(keyID) + canonicalID, err := utils.CanonicalKeyID(k) + check := []string{keyID} + if err == nil { + check = append(check, canonicalID) + } + for _, id := range check { + p, _, err := tr.cryptoService.GetPrivateKey(id) + if err == nil && p != nil { + return nil + } } } return signed.ErrNoKeys{KeyIDs: role.KeyIDs} From 0465365fb6fb9d1f5f995e0f19f772f5334fbe09 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Wed, 23 Dec 2015 09:44:48 -0800 Subject: [PATCH 8/8] Return an error if unable to encrypt a key as a valid PEM file Also address review comments and fix semantic conflict after rebase. Signed-off-by: Ying Li --- client/client_test.go | 18 +++++------------- trustmanager/keyfilestore.go | 3 +-- trustmanager/x509utils.go | 2 +- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 5b82789118..d4d7126040 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1588,19 +1588,12 @@ func TestPublishDelegations(t *testing.T) { // this is just a sanity test to make sure Publish calls it correctly and // no fallback happens. func TestPublishDelegationsX509(t *testing.T) { - var tempDirs [2]string - for i := 0; i < 2; i++ { - tempBaseDir, err := ioutil.TempDir("", "notary-test-") - assert.NoError(t, err, "failed to create a temporary directory: %s", err) - defer os.RemoveAll(tempBaseDir) - tempDirs[i] = tempBaseDir - } - - gun := "docker.com/notary" ts := fullTestServer(t) defer ts.Close() - repo1, _ := initializeRepo(t, data.ECDSAKey, tempDirs[0], gun, ts.URL, false) + repo1, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, false) + defer os.RemoveAll(repo1.baseDir) + delgKey, err := repo1.CryptoService.Create("targets/a", data.ECDSAKey) assert.NoError(t, err, "error creating delegation key") @@ -1633,9 +1626,8 @@ func TestPublishDelegationsX509(t *testing.T) { assert.Len(t, getChanges(t, repo1), 1, "wrong number of changelist files found") // Create a new repo and pull from the server - repo2, err := NewNotaryRepository(tempDirs[1], gun, ts.URL, - http.DefaultTransport, passphraseRetriever) - assert.NoError(t, err, "error creating repository: %s", err) + repo2 := newRepoToTestRepo(t, repo1) + defer os.RemoveAll(repo2.baseDir) // pull _, err = repo2.ListTargets() diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 9e1c53d7ee..0f9d821327 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -254,7 +254,6 @@ func listKeys(s LimitedFileStore) map[string]string { // read it as a PEM underscoreIndex := strings.LastIndex(keyIDFull, "_") if underscoreIndex == -1 { - keyID := keyIDFull d, err := s.Get(f) if err != nil { logrus.Error(err) @@ -265,7 +264,7 @@ func listKeys(s LimitedFileStore) map[string]string { continue } if role, ok := block.Headers["role"]; ok { - keyIDMap[keyID] = role + keyIDMap[keyIDFull] = role } } else { // The keyID is the first part of the keyname diff --git a/trustmanager/x509utils.go b/trustmanager/x509utils.go index 4fbbff987e..6b0bc76258 100644 --- a/trustmanager/x509utils.go +++ b/trustmanager/x509utils.go @@ -452,7 +452,7 @@ func EncryptPrivateKey(key data.PrivateKey, role, passphrase string) ([]byte, er } if encryptedPEMBlock.Headers == nil { - encryptedPEMBlock.Headers = make(map[string]string) + return nil, fmt.Errorf("unable to encrypt key - invalid PEM file produced") } encryptedPEMBlock.Headers["role"] = role