fixups as per Riyaz's review

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

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

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

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