crypto/rsa,crypto/ecdsa,crypto/ed25519: implement PublicKey.Equal

This makes all modern public keys in the standard library implement a
common interface (below) that can be used by applications for better
type safety and allows for checking that public (and private keys via
Public()) are equivalent.

interface {
    Equal(crypto.PublicKey) bool
}

Equality for ECDSA keys is complicated, we take a strict interpretation
that works for all secure applications (the ones not using the
unfortunate non-constant time CurveParams implementation) and fails
closed otherwise.

Tests in separate files to make them x_tests and avoid an import loop
with crypto/x509.

Fixes #21704

Change-Id: Id5379c96384a11c5afde0614955360e7470bb1c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>

diff --git a/src/crypto/ecdsa/ecdsa.go b/src/crypto/ecdsa/ecdsa.go
index 744182aac2822aa4a6c4aa7a37a799c02fe3de41..189399d12646d350517de6d293a9148021842259 100644 (file)
--- a/src/crypto/ecdsa/ecdsa.go
+++ b/src/crypto/ecdsa/ecdsa.go
@@ -62,6 +62,24 @@ type PublicKey struct {
        X, Y *big.Int
 }
 
+// Equal reports whether pub and x have the same value.
+//
+// Two keys are only considered to have the same value if they have the same Curve value.
+// Note that for example elliptic.P256() and elliptic.P256().Params() are different
+// values, as the latter is a generic not constant time implementation.
+func (pub *PublicKey) Equal(x crypto.PublicKey) bool {
+       xx, ok := x.(*PublicKey)
+       if !ok {
+               return false
+       }
+       return pub.X.Cmp(xx.X) == 0 && pub.Y.Cmp(xx.Y) == 0 &&
+               // Standard library Curve implementations are singletons, so this check
+               // will work for those. Other Curves might be equivalent even if not
+               // singletons, but there is no definitive way to check for that, and
+               // better to err on the side of safety.
+               pub.Curve == xx.Curve
+}
+
 // PrivateKey represents an ECDSA private key.
 type PrivateKey struct {
        PublicKey

diff --git a/src/crypto/ecdsa/equal_test.go b/src/crypto/ecdsa/equal_test.go
new file mode 100644 (file)
index 0000000..099a273
--- /dev/null
+++ b/src/crypto/ecdsa/equal_test.go
@@ -0,0 +1,77 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ecdsa_test
+
+import (
+       "crypto"
+       "crypto/ecdsa"
+       "crypto/elliptic"
+       "crypto/rand"
+       "crypto/x509"
+       "testing"
+)
+
+func testEqual(t *testing.T, c elliptic.Curve) {
+       private, _ := ecdsa.GenerateKey(c, rand.Reader)
+       public := &private.PublicKey
+
+       if !public.Equal(public) {
+               t.Errorf("public key is not equal to itself: %v", public)
+       }
+       if !public.Equal(crypto.Signer(private).Public().(*ecdsa.PublicKey)) {
+               t.Errorf("private.Public() is not Equal to public: %q", public)
+       }
+
+       enc, err := x509.MarshalPKIXPublicKey(public)
+       if err != nil {
+               t.Fatal(err)
+       }
+       decoded, err := x509.ParsePKIXPublicKey(enc)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if !public.Equal(decoded) {
+               t.Errorf("public key is not equal to itself after decoding: %v", public)
+       }
+
+       other, _ := ecdsa.GenerateKey(c, rand.Reader)
+       if public.Equal(other) {
+               t.Errorf("different public keys are Equal")
+       }
+
+       // Ensure that keys with the same coordinates but on different curves
+       // aren't considered Equal.
+       differentCurve := &ecdsa.PublicKey{}
+       *differentCurve = *public // make a copy of the public key
+       if differentCurve.Curve == elliptic.P256() {
+               differentCurve.Curve = elliptic.P224()
+       } else {
+               differentCurve.Curve = elliptic.P256()
+       }
+       if public.Equal(differentCurve) {
+               t.Errorf("public keys with different curves are Equal")
+       }
+
+       // This is not necessarily desirable, but if the Curve implementations are
+       // different, the PublicKeys are not considered Equal.
+       differentImpl := &ecdsa.PublicKey{}
+       *differentImpl = *public
+       // CurveParams also implements the Curve interface, although with a generic
+       // non-constant time implementation. See golang.org/issue/34648.
+       differentImpl.Curve = differentImpl.Curve.Params()
+       if public.Equal(differentImpl) {
+               t.Errorf("public keys with different curve implementations are Equal")
+       }
+}
+
+func TestEqual(t *testing.T) {
+       t.Run("P224", func(t *testing.T) { testEqual(t, elliptic.P224()) })
+       if testing.Short() {
+               return
+       }
+       t.Run("P256", func(t *testing.T) { testEqual(t, elliptic.P256()) })
+       t.Run("P384", func(t *testing.T) { testEqual(t, elliptic.P384()) })
+       t.Run("P521", func(t *testing.T) { testEqual(t, elliptic.P521()) })
+}

diff --git a/src/crypto/ed25519/ed25519.go b/src/crypto/ed25519/ed25519.go
index dcb4f9544f918320e110b8cc690862c9dba79728..b4f69564203f369e6dee23bd52aaf5b5eb77d21d 100644 (file)
--- a/src/crypto/ed25519/ed25519.go
+++ b/src/crypto/ed25519/ed25519.go
@@ -40,6 +40,15 @@ const (
 // PublicKey is the type of Ed25519 public keys.
 type PublicKey []byte
 
+// Equal reports whether pub and x have the same value.
+func (pub PublicKey) Equal(x crypto.PublicKey) bool {
+       xx, ok := x.(PublicKey)
+       if !ok {
+               return false
+       }
+       return bytes.Equal(pub, xx)
+}
+
 // PrivateKey is the type of Ed25519 private keys. It implements crypto.Signer.
 type PrivateKey []byte
 

diff --git a/src/crypto/ed25519/ed25519_test.go b/src/crypto/ed25519/ed25519_test.go
index cacd281f1cc62b9568ed3cd7f508274b8126833b..98e22a719ea04d33e6f96f08f50751ddfdcd2d00 100644 (file)
--- a/src/crypto/ed25519/ed25519_test.go
+++ b/src/crypto/ed25519/ed25519_test.go
@@ -88,6 +88,22 @@ func TestCryptoSigner(t *testing.T) {
        }
 }
 
+func TestEqual(t *testing.T) {
+       public, private, _ := GenerateKey(rand.Reader)
+
+       if !public.Equal(public) {
+               t.Errorf("public key is not equal to itself: %q", public)
+       }
+       if !public.Equal(crypto.Signer(private).Public().(PublicKey)) {
+               t.Errorf("private.Public() is not Equal to public: %q", public)
+       }
+
+       other, _, _ := GenerateKey(rand.Reader)
+       if public.Equal(other) {
+               t.Errorf("different public keys are Equal")
+       }
+}
+
 func TestGolden(t *testing.T) {
        // sign.input.gz is a selection of test cases from
        // https://ed25519.cr.yp.to/python/sign.input

diff --git a/src/crypto/rsa/equal_test.go b/src/crypto/rsa/equal_test.go
new file mode 100644 (file)
index 0000000..b00d0ea
--- /dev/null
+++ b/src/crypto/rsa/equal_test.go
@@ -0,0 +1,42 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package rsa_test
+
+import (
+       "crypto"
+       "crypto/rand"
+       "crypto/rsa"
+       "crypto/x509"
+       "testing"
+)
+
+func TestEqual(t *testing.T) {
+       private, _ := rsa.GenerateKey(rand.Reader, 512)
+       public := &private.PublicKey
+
+       if !public.Equal(public) {
+               t.Errorf("public key is not equal to itself: %v", public)
+       }
+       if !public.Equal(crypto.Signer(private).Public().(*rsa.PublicKey)) {
+               t.Errorf("private.Public() is not Equal to public: %q", public)
+       }
+
+       enc, err := x509.MarshalPKIXPublicKey(public)
+       if err != nil {
+               t.Fatal(err)
+       }
+       decoded, err := x509.ParsePKIXPublicKey(enc)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if !public.Equal(decoded) {
+               t.Errorf("public key is not equal to itself after decoding: %v", public)
+       }
+
+       other, _ := rsa.GenerateKey(rand.Reader, 512)
+       if public.Equal(other) {
+               t.Errorf("different public keys are Equal")
+       }
+}

diff --git a/src/crypto/rsa/rsa.go b/src/crypto/rsa/rsa.go
index d058949242d034fb89db0ed833ec479eb8c5c1a8..5a4299064016420edb2c21cec4dad3fca22cbc5d 100644 (file)
--- a/src/crypto/rsa/rsa.go
+++ b/src/crypto/rsa/rsa.go
@@ -50,6 +50,15 @@ func (pub *PublicKey) Size() int {
        return (pub.N.BitLen() + 7) / 8
 }
 
+// Equal reports whether pub and x have the same value.
+func (pub *PublicKey) Equal(x crypto.PublicKey) bool {
+       xx, ok := x.(*PublicKey)
+       if !ok {
+               return false
+       }
+       return pub.N.Cmp(xx.N) == 0 && pub.E == xx.E
+}
+
 // OAEPOptions is an interface for passing options to OAEP decryption using the
 // crypto.Decrypter interface.
 type OAEPOptions struct {