validate ECDH keys

ec.go

@@ -19,6 +19,22 @@ func curveNID(curve string) (C.int, error) {
 	return 0, errUnknownCurve
 }
 
+// curveSize returns the size of the curve in bytes.
+func curveSize(curve string) int {
+	switch curve {
+	default:
+		panic("openssl: unknown curve " + curve)
+	case "P-224":
+		return 224 / 8
+	case "P-256":
+		return 256 / 8
+	case "P-384":
+		return 384 / 8
+	case "P-521":
+		return (521 + 7) / 8
+	}
+}
+
 // encodeEcPoint encodes pt.
 func encodeEcPoint(group C.GO_EC_GROUP_PTR, pt C.GO_EC_POINT_PTR) ([]byte, error) {
 	// Get encoded point size.

ecdh.go

@@ -6,6 +6,7 @@ package openssl
 import "C"
 import (
 	"errors"
+	"math/bits"
 	"runtime"
 	"unsafe"
 )
@@ -30,8 +31,8 @@ func (k *PrivateKeyECDH) finalize() {
 }
 
 func NewPublicKeyECDH(curve string, bytes []byte) (*PublicKeyECDH, error) {
-	if len(bytes) < 1 {
-		return nil, errors.New("NewPublicKeyECDH: missing key")
+	if len(bytes) != 1+2*curveSize(curve) {
+		return nil, errors.New("NewPublicKeyECDH: wrong key length")
 	}
 	pkey, err := newECDHPkey(curve, bytes, false)
 	if err != nil {
@@ -45,6 +46,9 @@ func NewPublicKeyECDH(curve string, bytes []byte) (*PublicKeyECDH, error) {
 func (k *PublicKeyECDH) Bytes() []byte { return k.bytes }
 
 func NewPrivateKeyECDH(curve string, bytes []byte) (*PrivateKeyECDH, error) {
+	if len(bytes) != curveSize(curve) {
+		return nil, errors.New("NewPrivateKeyECDH: wrong key length")
+	}
 	pkey, err := newECDHPkey(curve, bytes, true)
 	if err != nil {
 		return nil, err
@@ -161,6 +165,9 @@ func newECDHPkey1(nid C.int, bytes []byte, isPrivate bool) (pkey C.GO_EVP_PKEY_P
 			return nil, newOpenSSLError("EC_KEY_set_public_key")
 		}
 	}
+	if C.go_openssl_EC_KEY_check_key(key) != 1 {
+		return nil, newOpenSSLError("EC_KEY_check_key")
+	}
 	return newEVPPKEY(key)
 }
 
@@ -311,3 +318,48 @@ func GenerateKeyECDH(curve string) (*PrivateKeyECDH, []byte, error) {
 	runtime.SetFinalizer(k, (*PrivateKeyECDH).finalize)
 	return k, bytes, nil
 }
+
+// isZero reports whether x is all zeroes in constant time.
+func isZero(x []byte) bool {
+	var acc byte
+	for _, b := range x {
+		acc |= b
+	}
+	return acc == 0
+}
+
+// isECDHLess reports whether a < b, where a and b are big-endian buffers of the
+// same length and shorter than 72 bytes.
+func isECDHLess(a, b []byte) bool {
+	if len(a) != len(b) {
+		panic("crypto/ecdh: internal error: mismatched isLess inputs")
+	}
+
+	// Copy the values into a fixed-size preallocated little-endian buffer.
+	// 72 bytes is enough for every scalar in this package, and having a fixed
+	// size lets us avoid heap allocations.
+	if len(a) > 72 {
+		panic("crypto/ecdh: internal error: isLess input too large")
+	}
+	bufA, bufB := make([]byte, 72), make([]byte, 72)
+	for i := range a {
+		bufA[i], bufB[i] = a[len(a)-i-1], b[len(b)-i-1]
+	}
+
+	// Perform a subtraction with borrow.
+	var borrow uint64
+	for i := 0; i < len(bufA); i += 8 {
+		limbA, limbB := leUint64(bufA[i:]), leUint64(bufB[i:])
+		_, borrow = bits.Sub64(limbA, limbB, borrow)
+	}
+
+	// If there is a borrow at the end of the operation, then a < b.
+	return borrow == 1
+}
+
+// leUint64 returns the little-endian uint64 value in b.
+func leUint64(b []byte) uint64 {
+	_ = b[7] // bounds check hint to compiler; see golang.org/issue/14808
+	return uint64(b[0]) | uint64(b[1])<<8 | uint64(b[2])<<16 | uint64(b[3])<<24 |
+		uint64(b[4])<<32 | uint64(b[5])<<40 | uint64(b[6])<<48 | uint64(b[7])<<56
+}

ecdh_test.go

@@ -3,6 +3,7 @@ package openssl_test
 import (
 	"bytes"
 	"encoding/hex"
+	"strings"
 	"testing"
 
 	"github.com/golang-fips/openssl/v2"
@@ -171,3 +172,128 @@ func BenchmarkECDH(b *testing.B) {
 		}
 	}
 }
+
+var invalidECDHPrivateKeys = map[string][]string{
+	"P-256": {
+		// Bad lengths.
+		"",
+		"01",
+		"01010101010101010101010101010101010101010101010101010101010101",
+		"000101010101010101010101010101010101010101010101010101010101010101",
+		strings.Repeat("01", 200),
+		// Zero.
+		"0000000000000000000000000000000000000000000000000000000000000000",
+		// Order of the curve and above.
+		"ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551",
+		"ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632552",
+		"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+	},
+	"P-384": {
+		// Bad lengths.
+		"",
+		"01",
+		"0101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101",
+		"00010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101",
+		strings.Repeat("01", 200),
+		// Zero.
+		"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+		// Order of the curve and above.
+		"ffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52973",
+		"ffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52974",
+		"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+	},
+	"P-521": {
+		// Bad lengths.
+		"",
+		"01",
+		"0101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101",
+		"00010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101",
+		strings.Repeat("01", 200),
+		// Zero.
+		"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+		// Order of the curve and above.
+		"01fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb71e91386409",
+		"01fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb71e9138640a",
+		"11fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa51868783bf2f966b7fcc0148f709a5d03bb5c9b8899c47aebb6fb71e91386409",
+		"03fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff4a30d0f077e5f2cd6ff980291ee134ba0776b937113388f5d76df6e3d2270c812",
+	},
+}
+
+var invalidECDHPublicKeys = map[string][]string{
+	"P-256": {
+		// Bad lengths.
+		"",
+		"04",
+		strings.Repeat("04", 200),
+		// Infinity.
+		"00",
+		// Compressed encodings.
+		"036b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296",
+		"02e2534a3532d08fbba02dde659ee62bd0031fe2db785596ef509302446b030852",
+		// Points not on the curve.
+		"046b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f6",
+		"0400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+	},
+	"P-384": {
+		// Bad lengths.
+		"",
+		"04",
+		strings.Repeat("04", 200),
+		// Infinity.
+		"00",
+		// Compressed encodings.
+		"03aa87ca22be8b05378eb1c71ef320ad746e1d3b628ba79b9859f741e082542a385502f25dbf55296c3a545e3872760ab7",
+		"0208d999057ba3d2d969260045c55b97f089025959a6f434d651d207d19fb96e9e4fe0e86ebe0e64f85b96a9c75295df61",
+		// Points not on the curve.
+		"04aa87ca22be8b05378eb1c71ef320ad746e1d3b628ba79b9859f741e082542a385502f25dbf55296c3a545e3872760ab73617de4a96262c6f5d9e98bf9292dc29f8f41dbd289a147ce9da3113b5f0b8c00a60b1ce1d7e819d7a431d7c90ea0e60",
+		"04000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+	},
+	"P-521": {
+		// Bad lengths.
+		"",
+		"04",
+		strings.Repeat("04", 200),
+		// Infinity.
+		"00",
+		// Compressed encodings.
+		"030035b5df64ae2ac204c354b483487c9070cdc61c891c5ff39afc06c5d55541d3ceac8659e24afe3d0750e8b88e9f078af066a1d5025b08e5a5e2fbc87412871902f3",
+		"0200c6858e06b70404e9cd9e3ecb662395b4429c648139053fb521f828af606b4d3dbaa14b5e77efe75928fe1dc127a2ffa8de3348b3c1856a429bf97e7e31c2e5bd66",
+		// Points not on the curve.
+		"0400c6858e06b70404e9cd9e3ecb662395b4429c648139053fb521f828af606b4d3dbaa14b5e77efe75928fe1dc127a2ffa8de3348b3c1856a429bf97e7e31c2e5bd66011839296a789a3bc0045c8a5fb42c7d1bd998f54449579b446817afbd17273e662c97ee72995ef42640c550b9013fad0761353c7086a272c24088be94769fd16651",
+		"04000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+	},
+}
+
+func TestECDHNewPrivateKeyECDH_Invalid(t *testing.T) {
+	for _, curve := range []string{"P-256", "P-384", "P-521"} {
+		t.Run(curve, func(t *testing.T) {
+			for _, input := range invalidECDHPrivateKeys[curve] {
+				k, err := openssl.NewPrivateKeyECDH(curve, hexDecode(t, input))
+				if err == nil {
+					t.Errorf("unexpectedly accepted %q", input)
+				} else if k != nil {
+					t.Error("PrivateKey was not nil on error")
+				} else if strings.Contains(err.Error(), "boringcrypto") {
+					t.Errorf("boringcrypto error leaked out: %v", err)
+				}
+			}
+		})
+	}
+}
+
+func TestECDHNewPublicKeyECDH_Invalid(t *testing.T) {
+	for _, curve := range []string{"P-256", "P-384", "P-521"} {
+		t.Run(curve, func(t *testing.T) {
+			for _, input := range invalidECDHPublicKeys[curve] {
+				k, err := openssl.NewPublicKeyECDH(curve, hexDecode(t, input))
+				if err == nil {
+					t.Errorf("unexpectedly accepted %q", input)
+				} else if k != nil {
+					t.Error("PublicKey was not nil on error")
+				} else if strings.Contains(err.Error(), "boringcrypto") {
+					t.Errorf("boringcrypto error leaked out: %v", err)
+				}
+			}
+		})
+	}
+}

shims.h

@@ -345,6 +345,7 @@ DEFINEFUNC_LEGACY_1(const GO_BIGNUM_PTR, EC_KEY_get0_private_key, (const GO_EC_K
 DEFINEFUNC_LEGACY_1(const GO_EC_POINT_PTR, EC_KEY_get0_public_key, (const GO_EC_KEY_PTR arg0), (arg0)) \
 DEFINEFUNC_LEGACY_1(GO_EC_KEY_PTR, EC_KEY_new_by_curve_name, (int arg0), (arg0)) \
 DEFINEFUNC_LEGACY_1(int, EC_KEY_set_private_key, (GO_EC_KEY_PTR arg0, const GO_BIGNUM_PTR arg1), (arg0, arg1)) \
+DEFINEFUNC_LEGACY_1(int, EC_KEY_check_key, (const GO_EC_KEY_PTR key), (key)) \
 DEFINEFUNC(GO_EC_POINT_PTR, EC_POINT_new, (const GO_EC_GROUP_PTR arg0), (arg0)) \
 DEFINEFUNC(void, EC_POINT_free, (GO_EC_POINT_PTR arg0), (arg0)) \
 DEFINEFUNC(int, EC_POINT_mul, (const GO_EC_GROUP_PTR group, GO_EC_POINT_PTR r, const GO_BIGNUM_PTR n, const GO_EC_POINT_PTR q, const GO_BIGNUM_PTR m, GO_BN_CTX_PTR ctx), (group, r, n, q, m, ctx)) \