made changes as per comments on PR

Signed-off-by: Avi Vaid <avaid1996@gmail.com>

cmd/notary/integration_test.go

@@ -106,7 +106,7 @@ func TestInitWithRootKey(t *testing.T) {
 	// create encrypted root key
 	privKey, err := utils.GenerateECDSAKey(rand.Reader)
 	require.NoError(t, err)
-	encryptedPEMPrivKey, err := utils.EncryptPrivateKey(privKey, data.CanonicalRootRole, testPassphrase)
+	encryptedPEMPrivKey, err := utils.EncryptPrivateKey(privKey, data.CanonicalRootRole, "", testPassphrase)
 	require.NoError(t, err)
 	encryptedPEMKeyFilename := filepath.Join(tempDir, "encrypted_key.key")
 	err = ioutil.WriteFile(encryptedPEMKeyFilename, encryptedPEMPrivKey, 0644)
@@ -154,7 +154,7 @@ func TestInitWithRootKey(t *testing.T) {
 	// instead of using a new retriever, we create a new key with a different pass
 	badPassPrivKey, err := utils.GenerateECDSAKey(rand.Reader)
 	require.NoError(t, err)
-	badPassPEMPrivKey, err := utils.EncryptPrivateKey(badPassPrivKey, data.CanonicalRootRole, "bad_pass")
+	badPassPEMPrivKey, err := utils.EncryptPrivateKey(badPassPrivKey, data.CanonicalRootRole, "", "bad_pass")
 	require.NoError(t, err)
 	badPassPEMKeyFilename := filepath.Join(tempDir, "badpass_key.key")
 	err = ioutil.WriteFile(badPassPEMKeyFilename, badPassPEMPrivKey, 0644)

fixtures/precedence.example.com.key

@@ -0,0 +1,30 @@
+-----BEGIN RSA PRIVATE KEY-----
+role: root
+
+MIIEowIBAAKCAQEAmLYiYCTAWJBWAuxZLqVmV4FiUdGgEqoQvCbN73zF/mQfhq0C
+ITo6xSxs1QiGDOzUtkpzXzziSj4J5+et4JkFleeEKaMcHadeIsSlHGvVtXDv93oR
+3ydmfZO+ULRU8xHloqcLr1KrOP1daLfdMRbactd75UQgvw9XTsdeMVX5AlicSENV
+KV+AQXvVpv8PT10MSvlBFam4reXuY/SkeMbIaW5pFu6AQv3Zmftt2ta0CB9kb1mY
+d+OKru8Hnnq5aJw6R3GhP0TBd25P1PkiSxM2KGYZZk0W/NZqLK9/LTFKTNCv7VjC
+bysVo7HxCY0bQe/bDP82v7SnLtb3aZogfva4HQIDAQABAoIBAQCLPj+X5MrRtkIH
+BlTHGJ95mIr6yaYofpMlzEgoX1/1dnvcg/IWNA8UbE6L7Oq17FiEItyR8WTwhyLn
+JrO/wCd8qQ40HPrs+wf1sdJPWPATMfhMcizLihSE2mtFETkILcByD9iyszFWlIdQ
+jZ4NPaZP4rWgtf8Z1zYnqdf0Kk0T2imFya0qyoRLo40kxeb4p5K53JD7rPLQNyvO
+YeFXTuKxBrFEMs6/wFjl+TO4nfHQXQlgQp4MNd9L5fEQBj+TvGVX+zcQEmzxljK8
+zFNXyxvXgjBPD+0V7yRhTYjrUfZJ4RX1yKDpdsva6BXL7t9hNEg/aGnKRDYF3i5q
+WQz8csCBAoGBAMfdtAr3RCuCxe0TIVBon5wubau6HLOxorcXSvxO5PO2kzhy3+GY
+xcCMJ+Wo0dTFXjQD3oxRKuDrPRK7AX/grYn7qJo6W7SM9xYEq3HspJJFGkcRsvem
+MALt8bvG5NkGmLJD+pTOKVaTZRjW3BM6GcMzBgsLynQcLllRtNI8Hcw9AoGBAMOa
+CMsWQfoOUjUffrXN0UnXLEPEeazPobnCHVtE244FdX/BFu5WMA7qqaPRyvnfK0Vl
+vF5sGNiBCOnq1zjYee6FD2eyAzVmWJXM1DB4Ewp4ZaABS0ZCZgNfyd1badY4IZpw
+pjYEQprguw+J8yZItNJRo+WBmnSgZy6o1bpDaflhAoGAYf61GS9VkFPlQbFAg1FY
++NXW1f1Bt2VgV48nKAByx3/8PRAt70ndo+PUaAlXIJDI+I3xHzFo6bDNWBKy0IVT
+8TSf3UbB0gvP1k7h1NDnfAQ/txrZeg1Uuwr5nE0Pxc0zLyyffzh6EkXgqsYmT5MM
+MKYiz2WvlTCAFTE3jGEHZy0CgYBti/cgxnZs9VhVKC5u47YzBK9lxMPgZOjOgEiw
+tP/Bqo0D38BX+y0vLX2UogprpvE1DKVSvHetyZaUa1HeJF8llp/qE2h4n7k9LFoq
+SxVe588CrbbawpUfjqYfsvKzZvxq4mw0FG65DuO08C2dY1rh75c7EjrO1obzOtt4
+VgkkAQKBgDnRyLnzlMfvjCyW9+cHbURQNe2iupfnlrXWEntg56USBVrFtfRQxDRp
+fBtlq+0BNfDVdoVNasTCBW16UKoRBH1/k5idz5QPEbKY2055sNxHMVg0uzdb4HXr
+73uaYzNrT8P7wyHFF3UL5bd0aO5DT1VYvGlHHgOhCyqcM+RBgPBS
+-----END RSA PRIVATE KEY-----
+

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

tuf/utils/x509.go

@@ -421,7 +421,7 @@ func KeyToPEM(privKey data.PrivateKey, role string) ([]byte, error) {
 
 // EncryptPrivateKey returns an encrypted PEM key given a Privatekey
 // and a passphrase
-func EncryptPrivateKey(key data.PrivateKey, role, passphrase string) ([]byte, error) {
+func EncryptPrivateKey(key data.PrivateKey, role, gun, passphrase string) ([]byte, error) {
 	bt, err := blockType(key)
 	if err != nil {
 		return nil, err
@@ -443,6 +443,9 @@ func EncryptPrivateKey(key data.PrivateKey, role, passphrase string) ([]byte, er
 		return nil, fmt.Errorf("unable to encrypt key - invalid PEM file produced")
 	}
 	encryptedPEMBlock.Headers["role"] = role
+	if gun != "" {
+		encryptedPEMBlock.Headers["gun"] = gun
+	}
 
 	return pem.EncodeToMemory(encryptedPEMBlock), nil
 }

tuf/utils/x509_test.go

@@ -124,15 +124,15 @@ func TestKeyOperations(t *testing.T) {
 	require.Equal(t, rsaKey.Private(), decodedRSAKey.Private())
 
 	// Encrypt our ED Key
-	encryptedEDKey, err := EncryptPrivateKey(edKey, "root", "ponies")
+	encryptedEDKey, err := EncryptPrivateKey(edKey, "root", "", "ponies")
 	require.NoError(t, err)
 
 	// Encrypt our EC Key
-	encryptedECKey, err := EncryptPrivateKey(ecKey, "root", "ponies")
+	encryptedECKey, err := EncryptPrivateKey(ecKey, "root", "", "ponies")
 	require.NoError(t, err)
 
 	// Encrypt our RSA Key
-	encryptedRSAKey, err := EncryptPrivateKey(rsaKey, "root", "ponies")
+	encryptedRSAKey, err := EncryptPrivateKey(rsaKey, "root", "", "ponies")
 	require.NoError(t, err)
 
 	// Check to see if ED key it is encrypted

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
@@ -94,7 +95,7 @@ func ExportKeys(to io.Writer, s Exporter, from string) error {
 // Each block is written to the subpath indicated in the "path" PEM
 // header. If the file already exists, the file is truncated. Multiple
 // adjacent PEMs with the same "path" header are appended together.
-func ImportKeys(from io.Reader, to []Importer, role string, gun string, passRet notary.PassRetriever) error {
+func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun string, passRet notary.PassRetriever) error {
 	data, err := ioutil.ReadAll(from)
 	if err != nil {
 		return err
@@ -106,11 +107,18 @@ func ImportKeys(from io.Reader, to []Importer, role string, gun string, passRet
 	for block, rest := pem.Decode(data); block != nil; block, rest = pem.Decode(rest) {
 		if block.Headers["role"] == "" {
 			// no worries about if check as for GUN here because empty roles will get a role:notary.DefaultImportRole
-			block.Headers["role"] = role
+			block.Headers["role"] = fallbackRole
+		}
+		if rawPath := block.Headers["path"]; rawPath != "" {
+			pathWOFileName := strings.TrimSuffix(rawPath, filepath.Base(rawPath))
+			if strings.HasPrefix(pathWOFileName, notary.NonRootKeysSubdir) {
+				gunName := strings.TrimPrefix(pathWOFileName, notary.NonRootKeysSubdir)
+				block.Headers["gun"] = gunName[1:(len(gunName) - 1)] //removes the slashes
+			}
 		}
 		if block.Headers["gun"] == "" {
-			if gun != "" {
-				block.Headers["gun"] = gun
+			if fallbackGun != "" {
+				block.Headers["gun"] = fallbackGun
 			}
 		}
 		loc, ok := block.Headers["path"]
@@ -125,12 +133,12 @@ func ImportKeys(from io.Reader, to []Importer, role string, gun string, passRet
 
 			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 {
@@ -174,7 +182,7 @@ func ImportKeys(from io.Reader, to []Importer, role string, gun string, passRet
 				}
 				break
 			}
-			toSave, _ = utils.EncryptPrivateKey(privKey, block.Headers["role"], chosenPassphrase)
+			toSave, _ = utils.EncryptPrivateKey(privKey, block.Headers["role"], block.Headers["gun"], chosenPassphrase)
 		}
 
 		toWrite = append(toWrite, toSave...)

utils/keys_test.go

@@ -224,7 +224,7 @@ func TestExport2InOneFile(t *testing.T) {
 func TestImportKeys(t *testing.T) {
 	s := NewTestImportStore()
 
-	from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms)
+	from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms)
 	b := &pem.Block{
 		Headers: make(map[string]string),
 	}
@@ -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)
 }
 
@@ -267,7 +269,7 @@ func TestImportNoPath(t *testing.T) {
 	b := &pem.Block{
 		Headers: make(map[string]string),
 	}
-	from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms)
+	from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms)
 	defer from.Close()
 	b.Bytes, _ = ioutil.ReadAll(from)
 
@@ -278,7 +280,7 @@ func TestImportNoPath(t *testing.T) {
 
 	for key := range s.data {
 		// no path but role included should work
-		require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "7baafcc9e5100ab062d886f06468f6c76e70b54b90e5d38537dadc6299c976d9"))
+		require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497"))
 	}
 
 	err = ImportKeys(in, []Importer{s}, "", "", passphraseRetriever)
@@ -293,7 +295,7 @@ func TestNonRootPathInference(t *testing.T) {
 	b := &pem.Block{
 		Headers: make(map[string]string),
 	}
-	from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms)
+	from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms)
 	defer from.Close()
 	b.Bytes, _ = ioutil.ReadAll(from)
 
@@ -304,7 +306,7 @@ func TestNonRootPathInference(t *testing.T) {
 
 	for key := range s.data {
 		// no path but role included should work
-		require.Equal(t, key, filepath.Join(notary.NonRootKeysSubdir, "somegun", "7baafcc9e5100ab062d886f06468f6c76e70b54b90e5d38537dadc6299c976d9"))
+		require.Equal(t, key, filepath.Join(notary.NonRootKeysSubdir, "somegun", "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497"))
 	}
 }
 
@@ -314,7 +316,7 @@ func TestBlockHeaderPrecedence(t *testing.T) {
 	b := &pem.Block{
 		Headers: make(map[string]string),
 	}
-	from, _ := os.OpenFile("testprecedence.key", os.O_RDONLY, notary.PrivKeyPerms)
+	from, _ := os.OpenFile("../fixtures/precedence.example.com.key", os.O_RDONLY, notary.PrivKeyPerms)
 	defer from.Close()
 	b.Bytes, _ = ioutil.ReadAll(from)
 
@@ -325,7 +327,7 @@ func TestBlockHeaderPrecedence(t *testing.T) {
 
 	for key := range s.data {
 		// block header role should take precedence over command line flag
-		require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "7baafcc9e5100ab062d886f06468f6c76e70b54b90e5d38537dadc6299c976d9"))
+		require.Equal(t, key, filepath.Join(notary.RootKeysSubdir, "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497"))
 	}
 }
 
@@ -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)
 }
 
@@ -386,7 +391,7 @@ func TestImportKeys2InOneFileNoPath(t *testing.T) {
 	b := &pem.Block{
 		Headers: make(map[string]string),
 	}
-	from, _ := os.OpenFile("test.key", os.O_RDONLY, notary.PrivKeyPerms)
+	from, _ := os.OpenFile("../fixtures/secure.example.com.key", os.O_RDONLY, notary.PrivKeyPerms)
 	b.Bytes, _ = ioutil.ReadAll(from)
 	rand.Read(b.Bytes)
 	b.Headers["path"] = "ankh"
@@ -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)