From e823029bca6232e12c9bd17c06da641c99c9eb30 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Mon, 10 Apr 2023 11:24:09 +0900 Subject: [PATCH 1/3] Stop using ECDSA_* function for signatures For creating and verifying ECDSA signatures in a pre-hashed manner, we used the legacy ECDSA_sign and ECDSA_verify functions, which bypass the system-wide restriction on ECDSA. This switches to using our _goboringcrypto_EVP_{sign,verify}_raw, which internally use EVP_PKEY_ functions. Signed-off-by: Daiki Ueno --- openssl/ecdsa.go | 16 ++++--- openssl/goopenssl.h | 21 +++------ openssl/notboring.go | 9 ++-- openssl/openssl_ecdsa_signature.c | 75 +++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 24 deletions(-) diff --git a/openssl/ecdsa.go b/openssl/ecdsa.go index 5e1e789d..d9b918ef 100644 --- a/openssl/ecdsa.go +++ b/openssl/ecdsa.go @@ -131,12 +131,13 @@ func NewPrivateKeyECDSA(curve string, X, Y BigInt, D BigInt) (*PrivateKeyECDSA, func HashSignECDSA(priv *PrivateKeyECDSA, hash []byte, h crypto.Hash) (*big.Int, *big.Int, error) { size := C._goboringcrypto_ECDSA_size(priv.key) sig := make([]byte, size) - var sigLen C.size_t + var sigLen C.size_t = C.size_t(size) md := cryptoHashToMD(h) if md == nil { panic("boring: invalid hash") } - if C._goboringcrypto_ECDSA_sign(md, base(hash), C.size_t(len(hash)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), &sigLen, priv.key) == 0 { + if C._goboringcrypto_ECDSA_sign(md, base(hash), C.size_t(len(hash)), + (*C.uint8_t)(unsafe.Pointer(&sig[0])), &sigLen, priv.key) == 0 { return nil, nil, NewOpenSSLError("ECDSA_sign failed") } runtime.KeepAlive(priv) @@ -151,10 +152,10 @@ func HashSignECDSA(priv *PrivateKeyECDSA, hash []byte, h crypto.Hash) (*big.Int, func SignMarshalECDSA(priv *PrivateKeyECDSA, hash []byte) ([]byte, error) { size := C._goboringcrypto_ECDSA_size(priv.key) sig := make([]byte, size) - var sigLen C.uint - ok := C._goboringcrypto_internal_ECDSA_sign(0, base(hash), C.size_t(len(hash)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), &sigLen, priv.key) > 0 - if !ok { - return nil, NewOpenSSLError(("ECDSA_sign failed")) + var sigLen C.size_t = C.size_t(size) + if C._goboringcrypto_ECDSA_sign_raw(nil, base(hash), C.size_t(len(hash)), + (*C.uint8_t)(unsafe.Pointer(&sig[0])), &sigLen, priv.key) == 0 { + return nil, NewOpenSSLError("ECDSA_sign failed") } runtime.KeepAlive(priv) @@ -162,7 +163,8 @@ func SignMarshalECDSA(priv *PrivateKeyECDSA, hash []byte) ([]byte, error) { } func VerifyECDSA(pub *PublicKeyECDSA, hash []byte, sig []byte) bool { - ok := C._goboringcrypto_internal_ECDSA_verify(0, base(hash), C.size_t(len(hash)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), C.uint(len(sig)), pub.key) > 0 + ok := C._goboringcrypto_ECDSA_verify_raw(nil, base(hash), C.size_t(len(hash)), + (*C.uint8_t)(unsafe.Pointer(&sig[0])), C.uint(len(sig)), pub.key) > 0 runtime.KeepAlive(pub) return ok } diff --git a/openssl/goopenssl.h b/openssl/goopenssl.h index a900b3f9..0785fe3d 100644 --- a/openssl/goopenssl.h +++ b/openssl/goopenssl.h @@ -499,22 +499,8 @@ _goboringcrypto_EC_KEY_oct2key(GO_EC_KEY *eckey, const unsigned char *buf, size_ #include -typedef ECDSA_SIG GO_ECDSA_SIG; - -DEFINEFUNC(GO_ECDSA_SIG *, ECDSA_SIG_new, (void), ()) -DEFINEFUNC(void, ECDSA_SIG_free, (GO_ECDSA_SIG * arg0), (arg0)) -DEFINEFUNC(GO_ECDSA_SIG *, ECDSA_do_sign, (const uint8_t *arg0, size_t arg1, const GO_EC_KEY *arg2), (arg0, arg1, arg2)) -DEFINEFUNC(int, ECDSA_do_verify, (const uint8_t *arg0, size_t arg1, const GO_ECDSA_SIG *arg2, GO_EC_KEY *arg3), (arg0, arg1, arg2, arg3)) DEFINEFUNC(size_t, ECDSA_size, (const GO_EC_KEY *arg0), (arg0)) -DEFINEFUNCINTERNAL(int, ECDSA_sign, - (int type, const unsigned char *dgst, size_t dgstlen, unsigned char *sig, unsigned int *siglen, EC_KEY *eckey), - (type, dgst, dgstlen, sig, siglen, eckey)) - -DEFINEFUNCINTERNAL(int, ECDSA_verify, - (int type, const unsigned char *dgst, size_t dgstlen, const unsigned char *sig, unsigned int siglen, EC_KEY *eckey), - (type, dgst, dgstlen, sig, siglen, eckey)) - #if OPENSSL_VERSION_NUMBER < 0x10100000L DEFINEFUNC(EVP_MD_CTX*, EVP_MD_CTX_create, (void), ()) #else @@ -578,6 +564,13 @@ DEFINEFUNC(void, EVP_MD_CTX_free, (EVP_MD_CTX *ctx), (ctx)) int _goboringcrypto_ECDSA_sign(EVP_MD *md, const uint8_t *arg1, size_t arg2, uint8_t *arg3, size_t *arg4, GO_EC_KEY *arg5); int _goboringcrypto_ECDSA_verify(EVP_MD *md, const uint8_t *arg1, size_t arg2, const uint8_t *arg3, unsigned int arg4, GO_EC_KEY *arg5); +int _goboringcrypto_ECDSA_sign_raw(EVP_MD *md, const uint8_t *msg, + size_t msgLen, uint8_t *sig, size_t *slen, + GO_EC_KEY *ec_key); +int _goboringcrypto_ECDSA_verify_raw(EVP_MD *md, + const uint8_t *msg, size_t msgLen, + const uint8_t *sig, unsigned int slen, + GO_EC_KEY *ec_key); #include diff --git a/openssl/notboring.go b/openssl/notboring.go index 2fa4a38e..bf3ab571 100644 --- a/openssl/notboring.go +++ b/openssl/notboring.go @@ -58,13 +58,16 @@ func NewPrivateKeyECDSA(curve string, X, Y, D BigInt) (*PrivateKeyECDSA, error) func NewPublicKeyECDSA(curve string, X, Y BigInt) (*PublicKeyECDSA, error) { panic("boringcrypto: not available") } -func SignECDSA(priv *PrivateKeyECDSA, hash []byte, h crypto.Hash) (r, s BigInt, err error) { +func HashSignECDSA(priv *PrivateKeyECDSA, hash []byte, h crypto.Hash) (r, s BigInt, err error) { panic("boringcrypto: not available") } -func SignMarshalECDSA(priv *PrivateKeyECDSA, hash []byte, h crypto.Hash) ([]byte, error) { +func HashVerifyECDSA(pub *PublicKeyECDSA, msg []byte, r, s *big.Int, h crypto.Hash) bool { panic("boringcrypto: not available") } -func VerifyECDSA(pub *PublicKeyECDSA, hash []byte, r, s BigInt, h crypto.Hash) bool { +func SignMarshalECDSA(priv *PrivateKeyECDSA, hash []byte) ([]byte, error) { + panic("boringcrypto: not available") +} +func VerifyECDSA(pub *PublicKeyECDSA, hash []byte, r, s BigInt) bool { panic("boringcrypto: not available") } diff --git a/openssl/openssl_ecdsa_signature.c b/openssl/openssl_ecdsa_signature.c index 714d18f1..3cba7ba6 100644 --- a/openssl/openssl_ecdsa_signature.c +++ b/openssl/openssl_ecdsa_signature.c @@ -44,3 +44,78 @@ int _goboringcrypto_ECDSA_verify(EVP_MD *md, const uint8_t *msg, size_t msgLen, _goboringcrypto_EVP_PKEY_free(key); return result; } + +int _goboringcrypto_ECDSA_sign_raw(EVP_MD *md, const uint8_t *msg, + size_t msgLen, uint8_t *sig, size_t *slen, + GO_EC_KEY *ec_key) { + int ret = 0; + GO_EVP_PKEY_CTX *ctx = NULL; + GO_EVP_PKEY *pkey = NULL; + + pkey = _goboringcrypto_EVP_PKEY_new(); + if (!pkey) + goto err; + + if (_goboringcrypto_EVP_PKEY_set1_EC_KEY(pkey, ec_key) != 1) + goto err; + + ctx = _goboringcrypto_EVP_PKEY_CTX_new(pkey, NULL); + if (!ctx) + goto err; + + if (_goboringcrypto_EVP_PKEY_sign_init(ctx) != 1) + goto err; + + if (md && _goboringcrypto_EVP_PKEY_CTX_set_signature_md(ctx, md) != 1) + goto err; + + if (_goboringcrypto_EVP_PKEY_sign(ctx, sig, slen, msg, msgLen) != 1) + goto err; + + /* Success */ + ret = 1; + +err: + _goboringcrypto_EVP_PKEY_CTX_free(ctx); + _goboringcrypto_EVP_PKEY_free(pkey); + + return ret; +} + +int _goboringcrypto_ECDSA_verify_raw(EVP_MD *md, + const uint8_t *msg, size_t msgLen, + const uint8_t *sig, unsigned int slen, + GO_EC_KEY *ec_key) { + int ret = 0; + GO_EVP_PKEY_CTX *ctx = NULL; + GO_EVP_PKEY *pkey = NULL; + + pkey = _goboringcrypto_EVP_PKEY_new(); + if (!pkey) + goto err; + + if (_goboringcrypto_EVP_PKEY_set1_EC_KEY(pkey, ec_key) != 1) + goto err; + + ctx = _goboringcrypto_EVP_PKEY_CTX_new(pkey, NULL); + if (!ctx) + goto err; + + if (_goboringcrypto_EVP_PKEY_verify_init(ctx) != 1) + goto err; + + if (md && _goboringcrypto_EVP_PKEY_CTX_set_signature_md(ctx, md) != 1) + goto err; + + if (_goboringcrypto_EVP_PKEY_verify(ctx, sig, slen, msg, msgLen) != 1) + goto err; + + /* Success */ + ret = 1; + +err: + _goboringcrypto_EVP_PKEY_CTX_free(ctx); + _goboringcrypto_EVP_PKEY_free(pkey); + + return ret; +} From b4cb8d8f151ccf89478f9d261486eb439c1a808f Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Mon, 10 Apr 2023 13:52:44 +0900 Subject: [PATCH 2/3] Stop using EC_KEY_generate_key for key generation Signed-off-by: Daiki Ueno --- openssl/ecdsa.go | 7 ++----- openssl/goopenssl.h | 5 +++-- openssl/openssl_ecdsa_signature.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/openssl/ecdsa.go b/openssl/ecdsa.go index d9b918ef..9f463888 100644 --- a/openssl/ecdsa.go +++ b/openssl/ecdsa.go @@ -188,14 +188,11 @@ func GenerateKeyECDSA(curve string) (X, Y, D BigInt, err error) { if err != nil { return nil, nil, nil, err } - key := C._goboringcrypto_EC_KEY_new_by_curve_name(nid) + key := C._goboringcrypto_EC_KEY_generate_key_fips(nid) if key == nil { - return nil, nil, nil, NewOpenSSLError("EC_KEY_new_by_curve_name failed") + return nil, nil, nil, NewOpenSSLError("EC_KEY_generate_key_fips failed") } defer C._goboringcrypto_EC_KEY_free(key) - if C._goboringcrypto_EC_KEY_generate_key(key) == 0 { - return nil, nil, nil, NewOpenSSLError("EC_KEY_generate_key failed") - } group := C._goboringcrypto_EC_KEY_get0_group(key) pt := C._goboringcrypto_EC_KEY_get0_public_key(key) bd := C._goboringcrypto_EC_KEY_get0_private_key(key) diff --git a/openssl/goopenssl.h b/openssl/goopenssl.h index 0785fe3d..35a98ef3 100644 --- a/openssl/goopenssl.h +++ b/openssl/goopenssl.h @@ -459,12 +459,12 @@ DEFINEFUNC(int, EC_POINT_set_affine_coordinates_GFp, typedef EC_KEY GO_EC_KEY; +GO_EC_KEY *_goboringcrypto_EC_KEY_generate_key_fips(int nid); + DEFINEFUNC(GO_EC_KEY *, EC_KEY_new, (void), ()) DEFINEFUNC(GO_EC_KEY *, EC_KEY_new_by_curve_name, (int arg0), (arg0)) DEFINEFUNC(void, EC_KEY_free, (GO_EC_KEY * arg0), (arg0)) DEFINEFUNC(const GO_EC_GROUP *, EC_KEY_get0_group, (const GO_EC_KEY *arg0), (arg0)) -DEFINEFUNC(int, EC_KEY_set_group, (GO_EC_KEY *arg0, const EC_GROUP *arg1), (arg0, arg1)) -DEFINEFUNC(int, EC_KEY_generate_key, (GO_EC_KEY * arg0), (arg0)) DEFINEFUNC(int, EC_KEY_set_private_key, (GO_EC_KEY * arg0, const GO_BIGNUM *arg1), (arg0, arg1)) DEFINEFUNC(int, EC_KEY_set_public_key, (GO_EC_KEY * arg0, const GO_EC_POINT *arg1), (arg0, arg1)) DEFINEFUNC(const GO_BIGNUM *, EC_KEY_get0_private_key, (const GO_EC_KEY *arg0), (arg0)) @@ -819,6 +819,7 @@ typedef EVP_PKEY GO_EVP_PKEY; DEFINEFUNC(GO_EVP_PKEY *, EVP_PKEY_new, (void), ()) DEFINEFUNC(void, EVP_PKEY_free, (GO_EVP_PKEY * arg0), (arg0)) DEFINEFUNC(int, EVP_PKEY_set1_RSA, (GO_EVP_PKEY * arg0, GO_RSA *arg1), (arg0, arg1)) +DEFINEFUNC(GO_EC_KEY *, EVP_PKEY_get1_EC_KEY, (GO_EVP_PKEY * arg0), (arg0)) DEFINEFUNC(int, EVP_PKEY_set1_EC_KEY, (GO_EVP_PKEY * arg0, GO_EC_KEY *arg1), (arg0, arg1)) DEFINEFUNC(int, EVP_PKEY_verify, (EVP_PKEY_CTX *ctx, const unsigned char *sig, unsigned int siglen, const unsigned char *tbs, size_t tbslen), diff --git a/openssl/openssl_ecdsa_signature.c b/openssl/openssl_ecdsa_signature.c index 3cba7ba6..7ce98333 100644 --- a/openssl/openssl_ecdsa_signature.c +++ b/openssl/openssl_ecdsa_signature.c @@ -6,6 +6,34 @@ #include "goopenssl.h" +// Only in BoringSSL. +GO_EC_KEY *_goboringcrypto_EC_KEY_generate_key_fips(int nid) { + GO_EVP_PKEY_CTX *ctx = NULL; + GO_EVP_PKEY *pkey = NULL; + GO_BIGNUM *e = NULL; + GO_EC_KEY *ret = NULL; + + ctx = _goboringcrypto_EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL); + if (!ctx) + return NULL; + + if (_goboringcrypto_EVP_PKEY_keygen_init(ctx) <= 0) + goto err; + + if (_goboringcrypto_EVP_PKEY_CTX_set_ec_paramgen_curve_nid(ctx, nid) <= 0) + goto err; + + if (_goboringcrypto_EVP_PKEY_keygen(ctx, &pkey) <= 0) + goto err; + + ret = _goboringcrypto_EVP_PKEY_get1_EC_KEY(pkey); + +err: + _goboringcrypto_EVP_PKEY_free(pkey); + _goboringcrypto_EVP_PKEY_CTX_free(ctx); + return ret; +} + int _goboringcrypto_ECDSA_sign(EVP_MD *md, const uint8_t *msg, size_t msgLen, uint8_t *sig, size_t *slen, GO_EC_KEY *eckey) { From d0efb0e7e19ec2d1309540ad6866e2a52ef516f6 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Mon, 10 Apr 2023 13:59:48 +0900 Subject: [PATCH 3/3] Backport ECDSA tests from wip/combined-codebase-staging Signed-off-by: Daiki Ueno --- openssl/ecdsa_test.go | 100 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 openssl/ecdsa_test.go diff --git a/openssl/ecdsa_test.go b/openssl/ecdsa_test.go new file mode 100644 index 00000000..a16c08ef --- /dev/null +++ b/openssl/ecdsa_test.go @@ -0,0 +1,100 @@ +//go:build linux && !android +// +build linux,!android + +package openssl_test + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "testing" + + "github.com/golang-fips/openssl-fips/openssl" + "github.com/golang-fips/openssl-fips/openssl/bbig" +) + +func testAllCurves(t *testing.T, f func(*testing.T, elliptic.Curve)) { + tests := []struct { + name string + curve elliptic.Curve + }{ + {"P256", elliptic.P256()}, + {"P384", elliptic.P384()}, + {"P521", elliptic.P521()}, + } + for _, test := range tests { + curve := test.curve + t.Run(test.name, func(t *testing.T) { + t.Parallel() + f(t, curve) + }) + } +} + +func TestECDSAKeyGeneration(t *testing.T) { + testAllCurves(t, testECDSAKeyGeneration) +} + +func testECDSAKeyGeneration(t *testing.T, c elliptic.Curve) { + priv, err := generateKeyForCurve(c) + if err != nil { + t.Fatal(err) + } + if !c.IsOnCurve(priv.PublicKey.X, priv.PublicKey.Y) { + t.Errorf("public key invalid: %s", err) + } +} + +func TestECDSASignAndVerify(t *testing.T) { + testAllCurves(t, testECDSASignAndVerify) +} + +func testECDSASignAndVerify(t *testing.T, c elliptic.Curve) { + key, err := generateKeyForCurve(c) + if err != nil { + t.Fatal(err) + } + msg := []byte("hi!") + hashed := openssl.SHA256(msg) + + priv, err := openssl.NewPrivateKeyECDSA(key.Params().Name, bbig.Enc(key.X), bbig.Enc(key.Y), bbig.Enc(key.D)) + if err != nil { + t.Fatal(err) + } + pub, err := openssl.NewPublicKeyECDSA(key.Params().Name, bbig.Enc(key.X), bbig.Enc(key.Y)) + if err != nil { + t.Fatal(err) + } + signed, err := openssl.SignMarshalECDSA(priv, hashed[:]) + if err != nil { + t.Fatal(err) + } + if !openssl.VerifyECDSA(pub, hashed[:], signed) { + t.Errorf("Verify failed") + } + signed[0] ^= 0xff + if openssl.VerifyECDSA(pub, hashed[:], signed) { + t.Errorf("Verify succeeded despite intentionally invalid hash!") + } + r, s, err := openssl.HashSignECDSA(priv, msg, crypto.SHA256) + if err != nil { + t.Fatal(err) + } + if !openssl.HashVerifyECDSA(pub, msg, r, s, crypto.SHA256) { + t.Errorf("Verify failed") + } + rb := r.Bytes() + rb[0] ^= 0xff + r.SetBytes(rb) + if openssl.HashVerifyECDSA(pub, msg, r, s, crypto.SHA256) { + t.Errorf("Verify succeeded on modified signature!") + } +} + +func generateKeyForCurve(c elliptic.Curve) (*ecdsa.PrivateKey, error) { + x, y, d, err := openssl.GenerateKeyECDSA(c.Params().Name) + if err != nil { + return nil, err + } + return &ecdsa.PrivateKey{PublicKey: ecdsa.PublicKey{Curve: c, X: bbig.Dec(x), Y: bbig.Dec(y)}, D: bbig.Dec(d)}, nil +}