Support for importing matching keys for a given cert from keystore. u…

…pdated tests, minor refactoring

Signed-off-by: Yuechuan Chen <[email protected]>

client/client.go

@@ -207,7 +207,7 @@ func (r *NotaryRepository) repoInitialize(rootKeyIDs []string, rootCerts []data.
 	}
 
 	// gets valid public keys corresponding to the rootKeyIDs from r.crypto
-	publicKeys, err := getCertsOfKeyIDs(r, rootKeyIDs, rootCerts)
+	publicKeys, err := r.certsOfKeyIDs(rootKeyIDs, rootCerts)
 	if err != nil {
 		return err
 	}
@@ -246,12 +246,15 @@ func (r *NotaryRepository) repoInitialize(rootKeyIDs []string, rootCerts []data.
 	return r.saveMetadata(serverManagesSnapshot)
 }
 
-func getCertsOfKeyIDs(r *NotaryRepository, rootKeyIDs []string, rootCerts []data.PublicKey) ([]data.PublicKey, error) {
-	publicKeys := make([]data.PublicKey, 0, len(rootKeyIDs))
+// certsOfKeyIDs either confirms that the certs and keys (represented by Key IDs) forms valid key pairs.
+// Or throw error when they missmatch.
+// Or generate certificates from the keys if no certificate is provided
+func (r *NotaryRepository) certsOfKeyIDs(keyIDs []string, certs []data.PublicKey) ([]data.PublicKey, error) {
 
-	//generate certificates if no certificate is provided
-	if rootCerts == nil || len(rootCerts) == 0 {
-		privKeys, err := getAllPrivKeys(rootKeyIDs, r.CryptoService)
+	publicKeys := make([]data.PublicKey, 0, len(keyIDs))
+	// generate certificates if none are provided
+	if certs == nil || len(certs) == 0 {
+		privKeys, err := getAllPrivKeys(keyIDs, r.CryptoService)
 		if err != nil {
 			return nil, err
 		}
@@ -263,24 +266,27 @@ func getCertsOfKeyIDs(r *NotaryRepository, rootKeyIDs []string, rootCerts []data
 			}
 			publicKeys = append(publicKeys, rootKey)
 		}
-	} else if len(rootKeyIDs) != len(rootCerts) { //return error if length does not match
-		errMsg := fmt.Sprintf("require matching number of keys and certificates but got %d rootkeys and %d certificates", len(rootKeyIDs), len(rootCerts))
+
+	} else if len(keyIDs) != len(certs) { //return error if length does not match
+		errMsg := fmt.Sprintf("require matching number of keys and certificates but got %d rootkeys and %d certificates", len(keyIDs), len(certs))
 		logrus.Debug(errMsg)
 		return nil, fmt.Errorf(errMsg)
+
 	} else { // match each cert with corresponding Key ID
-		match, err := matchKeyIdsWithCerts(*r, rootKeyIDs, rootCerts)
+		match, err := matchKeyIdsWithCerts(r, keyIDs, certs)
 		if err != nil {
 			return nil, err
 		} else if match {
-			publicKeys = rootCerts
+			publicKeys = certs
 		}
 	}
 
 	return publicKeys, nil
 }
 
-func matchKeyIdsWithCerts(r NotaryRepository, ids []string, certs []data.PublicKey) (bool, error) {
-
+// matchKeyIdsWithCerts validates that the private keys (represented by their IDs) and the x509 PublicKeys
+// forms matching key pairs
+func matchKeyIdsWithCerts(r *NotaryRepository, ids []string, certs []data.PublicKey) (bool, error) {
 	for i := 0; i < len(ids); i++ {
 		privKey, _, err := r.CryptoService.GetPrivateKey(ids[i])
 		if err != nil {
@@ -296,7 +302,6 @@ func matchKeyIdsWithCerts(r NotaryRepository, ids []string, certs []data.PublicK
 	}
 
 	return true, nil
-
 }
 
 // Initialize creates a new repository by using rootKey as the root Key for the
@@ -308,8 +313,46 @@ func (r *NotaryRepository) Initialize(rootKeyIDs []string, serverManagedRoles ..
 	return r.repoInitialize(rootKeyIDs, nil, serverManagedRoles...)
 }
 
+// keyExistsInList returns the id of the private key in idList that matches the public key
+// otherwise return empty string
+func keyExistsInList(cert data.PublicKey, idList []string) (string, error) {
+	pubKeyID, err := utils.CanonicalKeyID(cert)
+	if err != nil {
+		return "", err
+	}
+
+	for _, id := range idList {
+		if id == pubKeyID {
+			return id, nil
+		}
+	}
+	return "", nil
+}
+
 // InitializeWithCertificate initializes the repository with root key and their corresponding certificates
-func (r *NotaryRepository) InitializeWithCertificate(rootKeyIDs []string, rootCerts []data.PublicKey, serverManagedRoles ...data.RoleName) error {
+func (r *NotaryRepository) InitializeWithCertificate(rootKeyIDs []string, rootCerts []data.PublicKey,
+	nRepo *NotaryRepository, serverManagedRoles ...data.RoleName) error {
+
+	// If we explicitly pass in certificate(s) but not key, then look keys up using certificate
+	if rootKeyIDs == nil || len(rootKeyIDs) == 0 && rootCerts != nil && len(rootCerts) != 0 {
+
+		rootKeyIDs = []string{}
+		availableRootKeyIDs := nRepo.CryptoService.ListKeys(data.CanonicalRootRole)
+
+		for _, cert := range rootCerts {
+			id, err := keyExistsInList(cert, availableRootKeyIDs)
+			if err != nil {
+				return fmt.Errorf("error when initializing with certificate: %v", err)
+			}
+			if id != "" {
+				rootKeyIDs = append(rootKeyIDs, id)
+			} else {
+				return fmt.Errorf("cannot find matching private key")
+			}
+		}
+
+	}
+
 	return r.repoInitialize(rootKeyIDs, rootCerts, serverManagedRoles...)
 }
 

client/client_test.go

@@ -434,7 +434,7 @@ func TestMatchInvalidKeyIdsWithCerts(t *testing.T) {
 	repo, _, rootPubKeyID := createRepoAndKey(
 		t, data.ECDSAKey, tempBaseDir, "docker.com/notary", ts.URL)
 	cert1 := repo.CryptoService.GetKey(rootPubKeyID)
-	_, err = matchKeyIdsWithCerts(*repo, []string{"fake id"}, []data.PublicKey{cert1})
+	_, err = matchKeyIdsWithCerts(repo, []string{"fake id"}, []data.PublicKey{cert1})
 	require.Error(t, err)
 }
 

cmd/notary/integration_test.go

@@ -183,7 +183,7 @@ func TestInitWithRootKey(t *testing.T) {
 	require.Error(t, err, "Init with wrong role should error")
 }
 
-func TestWithRootCert(t *testing.T) {
+func TestInitWithRootCert(t *testing.T) {
 	setUp(t)
 
 	// key pairs
@@ -236,7 +236,7 @@ i85wnaTwOgWv8n6q3tavmnIA/v2QqsTpmI+bhwrPNKQ=
 	tempFile.Close()
 	defer os.Remove(tempFile.Name())
 
-	gun := "docker/repoName1"
+	gun := "docker/repoName"
 	privKeyFilename := filepath.Join(tempDir, "priv.key")
 	certFilename := filepath.Join(tempDir, "cert.pem")
 	nonMatchingKeyFilename := filepath.Join(tempDir, "nmkey.key")
@@ -249,53 +249,61 @@ i85wnaTwOgWv8n6q3tavmnIA/v2QqsTpmI+bhwrPNKQ=
 	err = ioutil.WriteFile(nonMatchingKeyFilename, []byte(nonMatchingKeyStr), 0644)
 	require.NoError(t, err)
 
-	// === test init - add - publish workflow ====
-
 	//test init repo without --rootkey and --rootcert
-	_, err = runCommand(t, tempDir,
+	output, err := runCommand(t, tempDir,
 		"-s", server.URL,
-		"init", gun,
+		"init", gun+"1",
 		"--rootkey", privKeyFilename,
 		"--rootcert", certFilename)
 	require.NoError(t, err)
+	require.Contains(t, output, "Root key found")
+	// === no rootkey specified: look up in keystore ===
+	// this requires the previous test to inject private key in keystore
+	output, err = runCommand(t, tempDir,
+		"-s", server.URL,
+		"init", gun+"2",
+		"--rootcert", certFilename)
+	require.NoError(t, err)
+	require.Contains(t, output, "Root key found")
 
 	//add a file to repo
-	_, err = runCommand(t, tempDir,
+	output, err = runCommand(t, tempDir,
 		"-s", server.URL,
-		"add", gun,
+		"add", gun+"2",
 		"v1",
 		certFilename)
 	require.NoError(t, err)
+	require.Contains(t, output, "staged for next publish")
 
 	//publish repo
-	_, err = runCommand(t, tempDir,
+	output, err = runCommand(t, tempDir,
 		"-s", server.URL,
-		"publish", gun)
+		"publish", gun+"2")
 	require.NoError(t, err)
+	require.Contains(t, output, "Successfully published changes")
 
 	// === test init with no argument to --rootcert ===
 	_, err = runCommand(t, tempDir,
 		"-s", server.URL,
-		"init", gun,
+		"init", gun+"3",
 		"--rootkey", privKeyFilename,
 		"--rootcert")
 	require.Error(t, err)
 
 	// === test non matching key pairs ===
 	_, err = runCommand(t, tempDir,
 		"-s", server.URL,
-		"init", gun,
+		"init", gun+"4",
 		"--rootkey", nonMatchingKeyFilename,
 		"--rootcert", certFilename)
 	require.Error(t, err)
 
 	// === test non existing path ===
 	_, err = runCommand(t, tempDir,
 		"-s", server.URL,
-		"init", gun,
+		"init", gun+"5",
 		"--rootkey", nonMatchingKeyFilename,
-		"--rootcert", certFilename)
-
+		"--rootcert", "fake/path/to/cert")
 	require.Error(t, err)
 
 }

cmd/notary/tuf.go

@@ -467,14 +467,14 @@ func importRootCert(certFilePath string) ([]data.PublicKey, error) {
 	// read certificate from file
 	certFile, err := os.Open(certFilePath)
 	if err != nil {
-		return nil, fmt.Errorf("Opening file to import as root cert: %v", err)
+		return nil, fmt.Errorf("error when opening certificate file: %v", err)
 	}
 	defer certFile.Close()
 
 	certPEM, err := ioutil.ReadAll(certFile)
 	block, _ := pem.Decode([]byte(certPEM))
 	if block == nil {
-		return nil, fmt.Errorf("Cannot read certificate, not a valid certificate PEM file: %v", err)
+		return nil, fmt.Errorf("the provided file does not contain a valid PEM certificate %v", err)
 	}
 
 	// convert the file to data.PublicKey
@@ -525,7 +525,12 @@ func (t *tufCommander) tufInit(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	if err = nRepo.InitializeWithCertificate(rootKeyIDs, rootCerts); err != nil {
+	// if key is not defined but cert is, then clear the key to to allow key to be searched in keystore
+	if t.rootKey == "" && t.rootCert != "" {
+		rootKeyIDs = []string{}
+	}
+
+	if err = nRepo.InitializeWithCertificate(rootKeyIDs, rootCerts, nRepo); err != nil {
 		return err
 	}
 

cmd/notary/tuf_test.go

@@ -24,6 +24,7 @@ import (
 // 3. confirm the content of the certificate to match expected content
 // 4. test reading non-existing file
 // 5. test reading non-certificate file
+// 6. test import non-valid certificate
 func TestImportRootCert(t *testing.T) {
 
 	certStr := `-----BEGIN CERTIFICATE-----
@@ -60,7 +61,7 @@ adLwkjqoeEKMaAXf
 	fakeFile := filepath.Join(dir, "fake.crt")
 	_, err = importRootCert(fakeFile)
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "Opening file to import as root cert:")
+	require.Contains(t, err.Error(), "error when opening")
 
 	//5. test import non-certificate file
 	nonCert := filepath.Join(dir, "noncert.crt")
@@ -69,9 +70,9 @@ adLwkjqoeEKMaAXf
 
 	_, err = importRootCert(nonCert)
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "Cannot read certificate, not a valid certificate PEM file")
+	require.Contains(t, err.Error(), "does not contain a valid PEM certificate")
 
-	//6. test import certificate containing errors
+	// 6. test import non-valid certificate
 	errCert := `-----BEGIN CERTIFICATE-----
 MIIBWDCB/6ADAgECAhBKKoVsRNJdGsGh6tPWnE4rMAoGCCqGSM49BAMCMBMxETAP
 BgNVBAMTCGRvY2tlci8qMB4XDTE31111111wMTczMFoXDTI3MDQyNjIwMTczMFow

tuf/signed/verify.go

@@ -109,7 +109,8 @@ func VerifySignature(msg []byte, sig *data.Signature, pk data.PublicKey) error {
 	return nil
 }
 
-// VerifyPublicKeyMatchesPrivateKey checks if the private key and the public keys forms valid key pairs
+// VerifyPublicKeyMatchesPrivateKey checks if the private key and the public keys forms valid key pairs.
+// This should work with both ecdsa-x509 certificate PublicKey as well as ecdsa PublicKey
 func VerifyPublicKeyMatchesPrivateKey(privKey data.PrivateKey, pubKey data.PublicKey) error {
 
 	msgLen := 64