test for legacy keys and some bugfixes for same

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

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

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.

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. <hexID>_<role>.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)