fixups as per Riyaz's review

Signed-off-by: Avi Vaid <[email protected]>

trustmanager/keystore.go

@@ -152,7 +152,7 @@ func (s *GenericKeyStore) AddKey(keyInfo KeyInfo, privKey data.PrivateKey) error
 	}
 
 	if chosenPassphrase != "" {
-		pemPrivKey, err = utils.EncryptPrivateKey(privKey, keyInfo.Role, "", chosenPassphrase)
+		pemPrivKey, err = utils.EncryptPrivateKey(privKey, keyInfo.Role, keyInfo.Gun, chosenPassphrase)
 	} else {
 		pemPrivKey, err = utils.KeyToPEM(privKey, keyInfo.Role)
 	}

utils/keys.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/notary"
+	tufdata "github.com/docker/notary/tuf/data"
 	"github.com/docker/notary/tuf/utils"
 	"io"
 	"io/ioutil"
@@ -27,7 +28,7 @@ type Importer interface {
 // ExportKeysByGUN exports all keys filtered to a GUN
 func ExportKeysByGUN(to io.Writer, s Exporter, gun string) error {
 	keys := s.ListFiles()
-	sort.Strings(keys) // ensure consistent. ListFiles has no order guarantee
+	sort.Strings(keys) // ensure consistency. ListFiles has no order guarantee
 	for _, k := range keys {
 		dir := filepath.Dir(k)
 		if dir == gun { // must be full GUN match
@@ -110,7 +111,7 @@ func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun
 		}
 		if rawPath := block.Headers["path"]; rawPath != "" {
 			pathWOFileName := strings.TrimSuffix(rawPath, filepath.Base(rawPath))
-			if strings.Contains(pathWOFileName, notary.NonRootKeysSubdir) {
+			if strings.HasPrefix(pathWOFileName, notary.NonRootKeysSubdir) {
 				gunName := strings.TrimPrefix(pathWOFileName, notary.NonRootKeysSubdir)
 				block.Headers["gun"] = gunName[1:(len(gunName) - 1)] //removes the slashes
 			}
@@ -132,12 +133,12 @@ func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun
 
 			decodedKey, err := utils.ParsePEMPrivateKey(pem.EncodeToMemory(block), "")
 			if err != nil {
-				logrus.Info("failed to import key to store: Invalid key generated, key may be encrypted and not contains path header")
+				logrus.Info("failed to import key to store: Invalid key generated, key may be encrypted and does not contain path header")
 				continue
 			}
 			keyID := decodedKey.ID()
 
-			if block.Headers["role"] == "root" {
+			if block.Headers["role"] == tufdata.CanonicalRootRole {
 				// does not make sense for root keys to have GUNs, so import it without the GUN even if specified
 				loc = filepath.Join(notary.RootKeysSubdir, keyID)
 			} else {

utils/keys_test.go

@@ -251,13 +251,15 @@ func TestImportKeys(t *testing.T) {
 
 	bFinal, bRest := pem.Decode(s.data["ankh"])
 	require.Equal(t, b.Bytes, bFinal.Bytes)
-	require.Equal(t, "", bFinal.Headers["path"])                       // path header is stripped during import
+	_, ok := bFinal.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 	require.Equal(t, notary.DefaultImportRole, bFinal.Headers["role"]) // if no role is specified we assume it is a delegation key
 	require.Len(t, bRest, 0)
 
 	cFinal, cRest := pem.Decode(s.data["morpork"])
 	require.Equal(t, c.Bytes, cFinal.Bytes)
-	require.Equal(t, "", bFinal.Headers["path"])
+	_, ok = cFinal.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 	require.Len(t, cRest, 0)
 }
 
@@ -367,16 +369,19 @@ func TestImportKeys2InOneFile(t *testing.T) {
 
 	bFinal, bRest := pem.Decode(s.data["ankh"])
 	require.Equal(t, b.Bytes, bFinal.Bytes)
-	require.Equal(t, "", bFinal.Headers["path"]) // path header is stripped during import
+	_, ok := bFinal.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 
 	b2Final, b2Rest := pem.Decode(bRest)
 	require.Equal(t, b2.Bytes, b2Final.Bytes)
-	require.Equal(t, "", b2Final.Headers["path"]) // path header is stripped during import
+	_, ok = b2Final.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 	require.Len(t, b2Rest, 0)
 
 	cFinal, cRest := pem.Decode(s.data["morpork"])
 	require.Equal(t, c.Bytes, cFinal.Bytes)
-	require.Equal(t, "", bFinal.Headers["path"])
+	_, ok = cFinal.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 	require.Len(t, cRest, 0)
 }
 
@@ -419,16 +424,19 @@ func TestImportKeys2InOneFileNoPath(t *testing.T) {
 
 	bFinal, bRest := pem.Decode(s.data["ankh"])
 	require.Equal(t, b.Bytes, bFinal.Bytes)
-	require.Equal(t, "", bFinal.Headers["path"]) // path header is stripped during import
+	_, ok := bFinal.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 
 	b2Final, b2Rest := pem.Decode(bRest)
 	require.Equal(t, b2.Bytes, b2Final.Bytes)
-	require.Equal(t, "", b2Final.Headers["path"]) // path header is stripped during import
+	_, ok = b2Final.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 	require.Len(t, b2Rest, 0)
 
 	cFinal, cRest := pem.Decode(s.data["morpork"])
 	require.Equal(t, c.Bytes, cFinal.Bytes)
-	require.Equal(t, "", bFinal.Headers["path"])
+	_, ok = cFinal.Headers["path"]
+	require.False(t, ok, "expected no path header, should have been removed at import")
 	require.Len(t, cRest, 0)
 
 	require.Len(t, s.data, 2)