Various fixes

client/client.go

@@ -15,7 +15,6 @@ import (
 	"github.com/Sirupsen/logrus"
 	canonicaljson "github.com/docker/go/canonical/json"
 	"github.com/docker/notary"
-
 	"github.com/docker/notary/client/changelist"
 	"github.com/docker/notary/cryptoservice"
 	store "github.com/docker/notary/storage"
@@ -170,13 +169,13 @@ func rootCertKey(gun data.GUN, privKey data.PrivateKey) (data.PublicKey, error)
 
 	x509PublicKey := utils.CertToKey(cert)
 	if x509PublicKey == nil {
-		return nil, fmt.Errorf("cannot use regenerated certificate: format %d", cert.PublicKeyAlgorithm)
+		return nil, fmt.Errorf("cannot generate public key from private key with id: %v. %v is not a supported type", privKey.ID(), privKey.Algorithm())
 	}
 
 	return x509PublicKey, nil
 }
 
-// repoInitialize initializes the notary repository wit a set of rootkeys, root certificates and roles.
+// repoInitialize initializes the notary repository with a set of rootkeys, root certificates and roles.
 func (r *NotaryRepository) repoInitialize(rootKeyIDs []string, rootCerts []data.PublicKey, serverManagedRoles ...data.RoleName) error {
 
 	// currently we only support server managing timestamps and snapshots, and
@@ -246,8 +245,9 @@ func (r *NotaryRepository) repoInitialize(rootKeyIDs []string, rootCerts []data.
 	return r.saveMetadata(serverManagesSnapshot)
 }
 
-// certsOfKeyIDs either confirms that the certs and keys (represented by Key IDs) forms valid key pairs.
-// Or throw error when they missmatch.
+// certsOfKeyIDs either confirms that the certs and keys (represented by Key IDs) forms valid, strictly ordered key pairs
+// (eg. keyIDs[0] must match certs[0] and keyIDs[1] must match certs[1] and so on).
+// Or throw error when they mismatch.
 // Or generate certificates from the keys if no certificate is provided
 func (r *NotaryRepository) certsOfKeyIDs(keyIDs []string, certs []data.PublicKey) ([]data.PublicKey, error) {
 
@@ -313,20 +313,26 @@ func (r *NotaryRepository) Initialize(rootKeyIDs []string, serverManagedRoles ..
 	return r.repoInitialize(rootKeyIDs, nil, serverManagedRoles...)
 }
 
+type errKeyNotFound struct{}
+
+func (errKeyNotFound) Error() string {
+	return fmt.Sprintf("cannot find matching private key id")
+}
+
 // 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) {
+func keyExistsInList(cert data.PublicKey, idList []string) error {
 	pubKeyID, err := utils.CanonicalKeyID(cert)
 	if err != nil {
-		return "", err
+		return fmt.Errorf("failed to obtain the public key id from the given certificate: %v", err)
 	}
 
 	for _, id := range idList {
 		if id == pubKeyID {
-			return id, nil
+			return nil
 		}
 	}
-	return "", nil
+	return errKeyNotFound{}
 }
 
 // InitializeWithCertificate initializes the repository with root key and their corresponding certificates
@@ -340,19 +346,13 @@ func (r *NotaryRepository) InitializeWithCertificate(rootKeyIDs []string, rootCe
 		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")
+			if err := keyExistsInList(cert, availableRootKeyIDs); err != nil {
+				return fmt.Errorf("error initializing repository with certificate: %v", err)
 			}
+			keyID, _ := utils.CanonicalKeyID(cert)
+			rootKeyIDs = append(rootKeyIDs, keyID)
 		}
-
 	}
-
 	return r.repoInitialize(rootKeyIDs, rootCerts, serverManagedRoles...)
 }
 

cmd/notary/tuf_test.go

@@ -7,9 +7,8 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"testing"
-
 	"strings"
+	"testing"
 
 	"github.com/docker/distribution/registry/client/auth"
 	"github.com/docker/notary/tuf/data"

tuf/signed/verify.go

@@ -1,7 +1,6 @@
 package signed
 
 import (
-	"crypto/rand"
 	"errors"
 	"fmt"
 	"strings"
@@ -10,6 +9,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/go/canonical/json"
 	"github.com/docker/notary/tuf/data"
+	"github.com/docker/notary/tuf/utils"
 )
 
 // Various basic signing errors
@@ -112,26 +112,18 @@ func VerifySignature(msg []byte, sig *data.Signature, pk data.PublicKey) error {
 // 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
-	msg := make([]byte, msgLen)
-	rand.Read(msg)
-
-	//sign msg with private key
-	sigBytes, err := privKey.Sign(rand.Reader, msg, nil)
-	if err != nil {
-		return fmt.Errorf("failed to sign test message: %s", err)
+	if pubKey.Algorithm() != privKey.Algorithm() {
+		return fmt.Errorf("public key and private key has different algorithms")
 	}
 
-	verifier, ok := Verifiers[privKey.SignatureAlgorithm()]
-	if !ok {
-		return fmt.Errorf("signing method is not supported: %s", privKey.SignatureAlgorithm())
-	}
+	privKeyID := privKey.ID()
+	pubKeyID, err := utils.CanonicalKeyID(pubKey)
 
-	err = verifier.Verify(pubKey, sigBytes, msg)
 	if err != nil {
-		return fmt.Errorf("private key did not match public key: %s", err)
+		return fmt.Errorf("could not verify key pair: %v", err)
+	}
+	if pubKeyID != privKeyID {
+		return fmt.Errorf("private key did not match public key")
 	}
-
 	return nil
 }