added skip import if a key is imported to a non root canonical role w…

…ithout a gun

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

cmd/notary/integration_test.go

@@ -2242,6 +2242,35 @@ func TestClientKeyImport(t *testing.T) {
 	_, _ = assertNumKeys(t, tempDir, 2, 4, !rootOnHardware())
 	_, err = os.Open(filepath.Join(tempDir, "private", notary.NonRootKeysSubdir, newKeyID+".key"))
 	require.NoError(t, err)
+
+	// test 8, non root canonical key with no gun
+
+	tempFile8, err := ioutil.TempFile("", "pemfile")
+	require.NoError(t, err)
+	// close later, because we might need to write to it
+	defer os.Remove(tempFile8.Name())
+
+	privKey, err = utils.GenerateECDSAKey(rand.Reader)
+	require.NoError(t, err)
+
+	pemBytes, err = utils.EncryptPrivateKey(privKey, data.CanonicalSnapshotRole, "", "")
+	require.NoError(t, err)
+
+	nBytes, err = tempFile8.Write(pemBytes)
+	require.NoError(t, err)
+	tempFile8.Close()
+	require.Equal(t, len(pemBytes), nBytes)
+	newKeyID = privKey.ID()
+
+	// import the key
+	_, err = runCommand(t, tempDir, "key", "import", tempFile8.Name())
+	require.NoError(t, err)
+
+	// if there is hardware available, root will only be on hardware, and not
+	// on disk
+	_, _ = assertNumKeys(t, tempDir, 2, 4, !rootOnHardware())
+	_, err = os.Open(filepath.Join(tempDir, "private", notary.NonRootKeysSubdir, newKeyID+".key"))
+	require.Error(t, err)
 }
 
 func TestAddDelImportKeyPublishFlow(t *testing.T) {
@@ -2487,7 +2516,6 @@ func TestExportImportFlow(t *testing.T) {
 	snapString := string(snapBytes)
 	require.Contains(t, snapString, "gun: gun")
 	require.True(t, strings.Contains(snapString, "role: snapshot") || strings.Contains(snapString, "role: target"))
-	require.Contains(t, snapString, "role: snapshot")
 
 	// validate targets is imported correctly
 	targKey, err := os.OpenFile(filepath.Join(newTempDir, "private", notary.NonRootKeysSubdir, "gun", signing[1]+".key"), os.O_RDONLY, notary.PrivKeyPerms)

utils/keys.go

@@ -140,14 +140,6 @@ func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun
 				continue
 			}
 			keyID := decodedKey.ID()
-			if block.Headers["role"] == tufdata.CanonicalRootRole {
-				// this is a root key so import it to trustDir/root_keys/
-				loc = filepath.Join(notary.RootKeysSubdir, keyID)
-			} else if block.Headers["role"] == tufdata.CanonicalSnapshotRole || block.Headers["role"] == tufdata.CanonicalTargetsRole || block.Headers["role"] == tufdata.CanonicalTimestampRole {
-				loc = filepath.Join(notary.NonRootKeysSubdir, block.Headers["gun"], keyID)
-			} else {
-				loc = filepath.Join(notary.NonRootKeysSubdir, keyID)
-			}
 			switch block.Headers["role"] {
 			case tufdata.CanonicalRootRole:
 				// this is a root key so import it to trustDir/root_keys/
@@ -165,6 +157,12 @@ func ImportKeys(from io.Reader, to []Importer, fallbackRole string, fallbackGun
 		// Note that a key that is not any of the canonical roles (except root) is a delegations key and should not have a gun
 		if block.Headers["role"] != tufdata.CanonicalSnapshotRole && block.Headers["role"] != tufdata.CanonicalTargetsRole && block.Headers["role"] != tufdata.CanonicalTimestampRole {
 			delete(block.Headers, "gun")
+		} else {
+			// check if the key is missing a gun header or has an empty gun and error out since we don't know where to import this key to
+			if block.Headers["gun"] == "" {
+				logrus.Info("failed to import key to store: Cannot have canonical role key without a gun, don't know where to import it")
+				continue
+			}
 		}
 
 		// the path header is not of any use once we've imported the key so strip it away

utils/keys_test.go

@@ -431,10 +431,12 @@ func TestImportKeys2InOneFileNoPath(t *testing.T) {
 	defer from.Close()
 	fromBytes, _ := ioutil.ReadAll(from)
 	b, _ := pem.Decode(fromBytes)
+	b.Headers["gun"] = "testgun"
 	b.Headers["role"] = data.CanonicalSnapshotRole
 	bBytes := pem.EncodeToMemory(b)
 
 	b2, _ := pem.Decode(fromBytes)
+	b2.Headers["gun"] = "testgun"
 	b2.Headers["role"] = data.CanonicalSnapshotRole
 	b2Bytes := pem.EncodeToMemory(b2)
 
@@ -455,10 +457,12 @@ func TestImportKeys2InOneFileNoPath(t *testing.T) {
 	err := ImportKeys(in, []Importer{s}, "", "", passphraseRetriever)
 	require.NoError(t, err)
 
-	bFinal, bRest := pem.Decode(s.data[filepath.Join(notary.NonRootKeysSubdir, "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497")])
+	bFinal, bRest := pem.Decode(s.data[filepath.Join(notary.NonRootKeysSubdir, "testgun", "12ba0e0a8e05e177bc2c3489bdb6d28836879469f078e68a4812fc8a2d521497")])
+	require.Equal(t, b.Headers["gun"], bFinal.Headers["gun"])
 	require.Equal(t, b.Headers["role"], bFinal.Headers["role"])
 
 	b2Final, b2Rest := pem.Decode(bRest)
+	require.Equal(t, b2.Headers["gun"], b2Final.Headers["gun"])
 	require.Equal(t, b2.Headers["role"], b2Final.Headers["role"])
 	require.Len(t, b2Rest, 0)