Improve core.PublicKeysEqual (#6996)

Rather than marshalling and comparing the bytes of each key, simply use
the .Equal() method provided by all go stdlib types that implement the
crypto.PublicKey interface.

Author: aarongable

ca/crl_test.go

@@ -127,7 +127,7 @@ func TestGenerateCRL(t *testing.T) {
 	close(ins)
 	err = <-errs
 	test.AssertError(t, err, "can't generate CRL with bad serials")
-	test.AssertContains(t, err.Error(), "Invalid serial number")
+	test.AssertContains(t, err.Error(), "invalid serial number")
 
 	// Test that we get an error when an entry has a bad revocation time.
 	ins = make(chan *capb.GenerateCRLRequest)

core/util.go

@@ -1,9 +1,10 @@
 package core
 
 import (
-	"bytes"
 	"crypto"
+	"crypto/ecdsa"
 	"crypto/rand"
+	"crypto/rsa"
 	"crypto/sha256"
 	"crypto/x509"
 	"encoding/base64"
@@ -97,7 +98,7 @@ func KeyDigest(key crypto.PublicKey) (Sha256Digest, error) {
 	switch t := key.(type) {
 	case *jose.JSONWebKey:
 		if t == nil {
-			return Sha256Digest{}, fmt.Errorf("Cannot compute digest of nil key")
+			return Sha256Digest{}, errors.New("cannot compute digest of nil key")
 		}
 		return KeyDigest(t.Key)
 	case jose.JSONWebKey:
@@ -133,21 +134,16 @@ func KeyDigestEquals(j, k crypto.PublicKey) bool {
 	return digestJ == digestK
 }
 
-// PublicKeysEqual determines whether two public keys have the same marshalled
-// bytes as one another
-func PublicKeysEqual(a, b interface{}) (bool, error) {
-	if a == nil || b == nil {
-		return false, errors.New("One or more nil arguments to PublicKeysEqual")
-	}
-	aBytes, err := x509.MarshalPKIXPublicKey(a)
-	if err != nil {
-		return false, err
-	}
-	bBytes, err := x509.MarshalPKIXPublicKey(b)
-	if err != nil {
-		return false, err
+// PublicKeysEqual determines whether two public keys are identical.
+func PublicKeysEqual(a, b crypto.PublicKey) (bool, error) {
+	switch ak := a.(type) {
+	case *rsa.PublicKey:
+		return ak.Equal(b), nil
+	case *ecdsa.PublicKey:
+		return ak.Equal(b), nil
+	default:
+		return false, fmt.Errorf("unsupported public key type %T", ak)
 	}
-	return bytes.Equal(aBytes, bBytes), nil
 }
 
 // SerialToString converts a certificate serial number (big.Int) to a String
@@ -161,7 +157,7 @@ func SerialToString(serial *big.Int) string {
 func StringToSerial(serial string) (*big.Int, error) {
 	var serialNum big.Int
 	if !ValidSerial(serial) {
-		return &serialNum, errors.New("Invalid serial number")
+		return &serialNum, fmt.Errorf("invalid serial number %q", serial)
 	}
 	_, err := fmt.Sscanf(serial, "%036x", &serialNum)
 	return &serialNum, err
@@ -254,7 +250,7 @@ func LoadCert(filename string) (*x509.Certificate, error) {
 	}
 	block, _ := pem.Decode(certPEM)
 	if block == nil {
-		return nil, fmt.Errorf("No data in cert PEM file %s", filename)
+		return nil, fmt.Errorf("no data in cert PEM file %q", filename)
 	}
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {

core/util_test.go

@@ -52,7 +52,7 @@ func TestSerialUtils(t *testing.T) {
 	}
 
 	badSerial, err := StringToSerial("doop!!!!000")
-	test.AssertEquals(t, fmt.Sprintf("%v", err), "Invalid serial number")
+	test.AssertContains(t, err.Error(), "invalid serial number")
 	fmt.Println(badSerial)
 }
 
@@ -166,7 +166,7 @@ func TestLoadCert(t *testing.T) {
 
 	_, err = LoadCert("../test/test-ca.der")
 	test.AssertError(t, err, "Loading non-PEM file did not error")
-	test.AssertEquals(t, err.Error(), "No data in cert PEM file ../test/test-ca.der")
+	test.AssertEquals(t, err.Error(), "no data in cert PEM file \"../test/test-ca.der\"")
 
 	_, err = LoadCert("../test/test-ca.key")
 	test.AssertError(t, err, "Loading non-cert file did not error")