Merge pull request #388 from docker/cleanup

Rebased cleanup/remove PEM headers

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"))

client/client_test.go

@@ -15,6 +15,7 @@ import (
 	"sort"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/Sirupsen/logrus"
 	ctxu "github.com/docker/distribution/context"
@@ -1064,12 +1065,10 @@ func testValidateRootKey(t *testing.T, rootType string) {
 	assert.NotEmpty(t, keyids)
 
 	for _, keyid := range keyids {
-		if key, ok := decodedRoot.Keys[keyid]; !ok {
+		key, ok := decodedRoot.Keys[keyid]
-			t.Fatal("key id not found in keys")
+		assert.True(t, ok, "key id not found in keys")
-		} else {
+		_, err := trustmanager.LoadCertFromPEM(key.Public())
-			_, err := trustmanager.LoadCertFromPEM(key.Public())
+		assert.NoError(t, err, "key is not a valid cert")
-			assert.NoError(t, err, "key is not a valid cert")
-		}
 	}
 }
 
@@ -1603,6 +1602,85 @@ 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) {
+	ts := fullTestServer(t)
+	defer ts.Close()
+
+	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")
+
+	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 := newRepoToTestRepo(t, repo1)
+	defer os.RemoveAll(repo2.baseDir)
+
+	// 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

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))
@@ -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)

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 {

cryptoservice/import_export_test.go

@@ -80,22 +80,20 @@ 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
 	// exist and are encrypted with the expected passphrase.
 	for _, f := range zipReader.File {
 		expectedPassphrase, present := passphraseByFile[f.Name]
-		if !present {
+		assert.True(t, present, "unexpected file %s in zip file", f.Name)
-			t.Fatalf("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 {
+	assert.Len(t, passphraseByFile, 0)
-		t.Fatalf("%s not found in zip", fileNotFound)
-	}
 
 	// Create new repo to test import
 	tempBaseDir2, err := ioutil.TempDir("", "notary-test-")
@@ -145,7 +141,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 +150,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")
 }
@@ -199,13 +195,11 @@ func TestImportExportGUN(t *testing.T) {
 	privKeyMap := cs.ListAllKeys()
 	for privKeyName := range privKeyMap {
 		_, alias, err := cs.GetPrivateKey(privKeyName)
-		if err != nil {
+		assert.NoError(t, err, "privKey %s has no alias", privKeyName)
-			t.Fatalf("privKey %s has no alias", privKeyName)
-		}
 		if alias == "root" {
 			continue
 		}
-		relKeyPath := filepath.Join("tuf_keys", privKeyName+"_"+alias+".key")
+		relKeyPath := filepath.Join("tuf_keys", privKeyName+".key")
 
 		passphraseByFile[relKeyPath] = exportPassphrase
 	}
@@ -215,9 +209,7 @@ func TestImportExportGUN(t *testing.T) {
 	for _, f := range zipReader.File {
 
 		expectedPassphrase, present := passphraseByFile[f.Name]
-		if !present {
+		assert.True(t, present, "unexpected file %s in zip file", f.Name)
-			t.Fatalf("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 {
+	assert.Len(t, passphraseByFile, 0)
-		t.Fatalf("%s not found in zip", fileNotFound)
-	}
 
 	// Create new repo to test import
 	tempBaseDir2, err := ioutil.TempDir("", "notary-test-")
@@ -264,13 +254,11 @@ func TestImportExportGUN(t *testing.T) {
 			continue
 		}
 		_, alias, err := cs2.GetPrivateKey(privKeyName)
-		if err != nil {
+		assert.NoError(t, err, "privKey %s has no alias", privKeyName)
-			t.Fatalf("privKey %s has no alias", privKeyName)
-		}
 		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 +306,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")
 
@@ -328,7 +316,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))
@@ -386,7 +374,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")
 

server/server_test.go

@@ -25,9 +25,7 @@ func TestRunBadAddr(t *testing.T) {
 		"",
 		nil,
 	)
-	if err == nil {
+	assert.Error(t, err, "Passed bad addr, Run should have failed")
-		t.Fatal("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 {
+	assert.Error(t, err)
-		t.Fatalf("Received unexpected err: %s", err.Error())
+	assert.IsType(t, &net.OpError{}, err)
-	}
+	assert.True(
-	if !strings.Contains(err.Error(), "bind: permission denied") {
+		t,
-		t.Fatalf("Received unexpected err: %s", err.Error())
+		strings.Contains(err.Error(), "bind: permission denied"),
-	}
+		"Received unexpected err: %s",
+		err.Error(),
+	)
 }
 
 func TestMetricsEndpoint(t *testing.T) {

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
@@ -55,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,
@@ -68,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,
@@ -176,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
 	})
@@ -185,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
@@ -212,68 +189,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
-}

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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to add file to store: %v", err)
-	}
 
 	// Check to see if file exists
 	b, err := ioutil.ReadFile(expectedFilePath)
-	if err != nil {
+	assert.NoError(t, err)
-		t.Fatalf("expected file not found: %v", err)
+	assert.Equal(t, testData, b, "unexpected content in the file: %s", expectedFilePath)
-	}
-
-	if !bytes.Equal(b, testData) {
-		t.Fatalf("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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to generate random file: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to remove file from store: %v", err)
-	}
 
 	// Check to see if file exists
 	_, err = os.Stat(expectedFilePath)
-	if err == nil {
+	assert.Error(t, err)
-		t.Fatalf("expected not to find file: %s", expectedFilePath)
-	}
 }
 
 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to generate random file: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to remove directory: %v", err)
-	}
 
 	expectedDirectory := filepath.Dir(expectedFilePath)
 	// Check to see if file exists
 	_, err = os.Stat(expectedDirectory)
-	if err == nil {
+	assert.Error(t, err)
-		t.Fatalf("expected not to find directory: %s", expectedDirectory)
-	}
 }
 
 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+		assert.NoError(t, err)
-			t.Fatalf("failed to generate random file: %v", 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 {
+	assert.Len(t, files, 10)
-		t.Fatalf("expected 10 files in listing, got: %d", len(files))
-	}
 }
 
 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+		assert.NoError(t, err)
-			t.Fatalf("failed to generate random file: %v", 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 {
+	assert.Len(t, files, 10)
-		t.Fatalf("expected 10 files in listing, got: %d", len(files))
-	}
 	files = store.ListDir("docker.com/notary")
-	if len(files) != 10 {
+	assert.Len(t, files, 10)
-		t.Fatalf("expected 10 files in listing, got: %d", len(files))
-	}
 	files = store.ListDir("fakedocker.com/")
-	if len(files) != 0 {
+	assert.Len(t, files, 0)
-		t.Fatalf("expected 0 files in listing, got: %d", len(files))
-	}
 }
 
 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 {
+	assert.Equal(t, firstPath, result, "unexpected error from GetPath: %v", err)
-		t.Fatalf("unexpected error from GetPath: %v", err)
-	}
-	if result != firstPath {
-		t.Fatalf("Expecting: %s", firstPath)
-	}
 
 	result, err = store.GetPath("/docker.io/testing-dashes/@#$%^&()")
-	if err != nil {
+	assert.Equal(t, secondPath, result, "unexpected error from GetPath: %v", err)
-		t.Fatalf("unexpected error from GetPath: %v", err)
-	}
-	if result != secondPath {
-		t.Fatalf("Expecting: %s", secondPath)
-	}
 }
 
 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 {
+	_, err := store.GetPath("../../etc/passwd")
-		t.Fatalf("expected ErrPathOutsideStore error from GetPath")
+	assert.Error(t, err)
-	}
+	assert.Equal(t, ErrPathOutsideStore, err)
-	if _, err := store.GetPath("private/../../../etc/passwd"); err != ErrPathOutsideStore {
+
-		t.Fatalf("expected ErrPathOutsideStore error from GetPath")
+	_, 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 {
+	assert.NoError(t, err)
-		t.Fatalf("unexpected error from GetPath: %v", err)
+	assert.Equal(t, expected, result)
-	}
-	if result != expected {
-		t.Fatalf("Expecting: %s (got: %s)", 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 {
+	_, err = relStore.GetPath("../../etc/passwd")
-		t.Fatalf("expected ErrPathOutsideStore error from GetPath")
+	assert.Error(t, err)
-	}
+	assert.Equal(t, ErrPathOutsideStore, err)
-	if _, err := relStore.GetPath("private/../../../etc/passwd"); err != ErrPathOutsideStore {
+	_, err = relStore.GetPath("private/../../../etc/passwd")
-		t.Fatalf("expected ErrPathOutsideStore error from GetPath")
+	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 {
+	assert.NoError(t, err)
-		t.Fatalf("unexpected error from GetPath: %v", err)
+	assert.Equal(t, expected, result)
-	}
-	if result != expected {
-		t.Fatalf("Expecting: %s (got: %s)", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to generate random file: %v", 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 {
+	assert.NoError(t, err, "failed to get data from: %s", testName)
-		t.Fatalf("failed to get data from: %s", testName)
+	assert.Equal(t, expectedData, testData, "unexpected content for the file: %s", expectedFilePath)
-
-	}
-	if !bytes.Equal(testData, expectedData) {
-		t.Fatalf("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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("expected find directory: %s", dirPath)
-	}
 
 	// Check to see if it is a directory
-	if !fi.IsDir() {
+	assert.True(t, fi.IsDir(), "expected to be directory: %s", dirPath)
-		t.Fatalf("expected to be directory: %s", dirPath)
-	}
 
 	// Check to see if the permissions match
-	if fi.Mode().String() != "drwxr-xr-x" {
+	assert.Equal(t, "drwxr-xr-x", fi.Mode().String(), "permissions are wrong for: %s. Got: %s", dirPath, fi.Mode().String())
-		t.Fatalf("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 {
+	assert.NoError(t, err)
-		t.Fatalf("failed to create a temporary directory: %v", 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 {
+	assert.NoError(t, err)
-		t.Fatalf("expected find directory: %s", dirPath)
-	}
 
 	// Check to see if it is a directory
-	if !fi.IsDir() {
+	assert.True(t, fi.IsDir(), "expected to be directory: %s", dirPath)
-		t.Fatalf("expected to be directory: %s", dirPath)
-	}
 
 	// Check to see if the permissions match
-	if fi.Mode().String() != "drwx------" {
+	assert.Equal(t, "drwx------", fi.Mode().String(), "permissions are wrong for: %s. Got: %s", dirPath, fi.Mode().String())
-		t.Fatalf("permissions are wrong for: %s. Got: %s", dirPath, fi.Mode().String())
-	}
 }
 
 func generateRandomFile(filePath string, perms os.FileMode) ([]byte, error) {
@@ -416,3 +333,56 @@ 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)
+
+	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)
+
+	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"
+
+	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)
+		}
+	}
+
+}

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"
 )
@@ -54,10 +56,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 +155,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 +164,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,25 +177,37 @@ 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) {
+// getKeyRole finds the role for the given keyID. It attempts to look
-	files := s.ListFiles()
+// 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 files {
+	for _, file := range s.ListFiles() {
 		filename := filepath.Base(file)
 
 		if strings.HasPrefix(filename, name) {
-			aliasPlusDotKey := strings.TrimPrefix(filename, name+"_")
+			d, err := s.Get(file)
-			retVal := strings.TrimSuffix(aliasPlusDotKey, "."+keyExtension)
+			if err != nil {
-			return retVal, nil
+				return "", false, err
+			}
+			block, _ := pem.Decode(d)
+			if block != nil {
+				if role, ok := block.Headers["role"]; ok {
+					return role, false, 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
@@ -208,14 +222,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))
+		privKey, _, err = GetPasswdDecryptBytes(passphraseRetriever, keyBytes, name, string(keyAlias))
-	}
+		if err != nil {
-	if retErr != nil {
+			return nil, "", err
-		return nil, "", retErr
+		}
 	}
 	cachedKeys[name] = &cachedKey{alias: keyAlias, key: privKey}
 	return privKey, keyAlias, nil
@@ -228,44 +241,58 @@ 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 {
+		var keyIDFull string
-			f = strings.TrimPrefix(f, rootKeysSubdir+"/")
+		if strings.HasPrefix(f, rootKeysSubdir+"/") {
+			keyIDFull = strings.TrimPrefix(f, rootKeysSubdir+"/")
 		} else {
-			f = strings.TrimPrefix(f, nonRootKeysSubdir+"/")
+			keyIDFull = strings.TrimPrefix(f, nonRootKeysSubdir+"/")
 		}
 
-		// Remove the extension from the full filename
+		keyIDFull = strings.TrimSpace(keyIDFull)
-		// abcde_root.key becomes abcde_root
-		keyIDFull := strings.TrimSpace(strings.TrimSuffix(f, filepath.Ext(f)))
 
-		// 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 {
-			continue
+			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[keyIDFull] = 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
 }
 
 // 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
-	filename := name + "_" + keyAlias
+	err = s.Remove(filepath.Join(getSubdir(role), name))
-	err = s.Remove(filepath.Join(getSubdir(keyAlias), filename))
 	if err != nil {
 		return err
 	}
@@ -283,18 +310,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
 	}
 
-	filename := name + "_" + keyAlias
+	if legacy {
+		name = name + "_" + role
+	}
+
 	var keyBytes []byte
-	keyBytes, err = s.Get(filepath.Join(getSubdir(keyAlias), filename))
+	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.
@@ -335,7 +365,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
@@ -343,17 +373,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}
+	cachedKeys[name] = &cachedKey{alias: role, key: privKey}
-	return s.Add(filepath.Join(getSubdir(alias), name+"_"+alias), 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 {

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)
@@ -55,6 +54,71 @@ 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
+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+"."+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
+	privKey, _, err := store.GetKey(testName)
+	assert.NoError(t, err, "failed to get key from store")
+
+	pemPrivKey, err := KeyToPEM(privKey, testAlias)
+	assert.NoError(t, err, "failed to convert key to PEM")
+	assert.Equal(t, testData, pemPrivKey)
+}
+
+// TestGetLegacyKey ensures we can still load keys where the role
+// is stored as part of the filename (i.e. <hexID>_<role>.key
+func TestGetLegacyKey(t *testing.T) {
+	testData := []byte(`-----BEGIN RSA PRIVATE KEY-----
 MIIEogIBAAKCAQEAyUIXjsrWRrvPa4Bzp3VJ6uOUGPay2fUpSV8XzNxZxIG/Opdr
 +k3EQi1im6WOqF3Y5AS1UjYRxNuRN+cAZeo3uS1pOTuoSupBXuchVw8s4hZJ5vXn
 TRmGb+xY7tZ1ZVgPfAZDib9sRSUsL/gC+aSyprAjG/YBdbF06qKbfOfsoCEYW1OQ
@@ -106,12 +170,9 @@ EMl3eFOJXjIch/wIesRSN+2dGOsl7neercjMh1i9RvpCwHDx/E0=
 	assert.NoError(t, err, "failed to create new key filestore")
 
 	// Call the GetKey function
-	privKey, _, err := store.GetKey(testName)
+	_, role, err := store.GetKey(testName)
 	assert.NoError(t, err, "failed to get key from store")
-
+	assert.Equal(t, testAlias, role)
-	pemPrivKey, err := KeyToPEM(privKey)
-	assert.NoError(t, err, "failed to convert key to PEM")
-	assert.Equal(t, testData, pemPrivKey)
 }
 
 func TestListKeys(t *testing.T) {
@@ -213,7 +274,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)
@@ -320,7 +381,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)
@@ -513,7 +574,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")

trustmanager/memorystore.go

@@ -0,0 +1,67 @@
+package trustmanager
+
+import (
+	"os"
+	"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),
+	}
+}
+
+// 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 os.ErrNotExist
+	}
+	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, os.ErrNotExist
+	}
+
+	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
+}

trustmanager/store.go

@@ -0,0 +1,52 @@
+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 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
+}
+
+// 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
+}

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 s.fileStore.ListFiles() {
-	for _, f := range certFiles {
 		// ListFiles returns relative paths
-		fullPath := filepath.Join(s.fileStore.BaseDir(), f)
+		data, err := s.fileStore.Get(f)
-		err := s.AddCertFromFile(fullPath)
+		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)
@@ -411,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
@@ -440,6 +451,11 @@ func EncryptPrivateKey(key data.PrivateKey, passphrase string) ([]byte, error) {
 		return nil, err
 	}
 
+	if encryptedPEMBlock.Headers == nil {
+		return nil, fmt.Errorf("unable to encrypt key - invalid PEM file produced")
+	}
+	encryptedPEMBlock.Headers["role"] = role
+
 	return pem.EncodeToMemory(encryptedPEMBlock), nil
 }
 

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

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") }

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)
+		k := tr.keysDB.GetKey(keyID)
-		if err == nil && p != nil {
+		canonicalID, err := utils.CanonicalKeyID(k)
-			return nil
+		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}