From 97c0d2cfd802893058c0709509ee15c80ce3bbf4 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 13 Dec 2024 16:00:02 +0100 Subject: [PATCH 01/12] implement SupportsSignatureRSAPKCS1 --- evp.go | 107 +++++++++++++++++++++++++++------------------------- hash.go | 21 ++++++++++- rsa.go | 46 ++++++++++++++++++++++ rsa_test.go | 71 +++++++++++++++++++++++++--------- shims.h | 3 ++ 5 files changed, 178 insertions(+), 70 deletions(-) diff --git a/evp.go b/evp.go index ef68bbfb..29822de4 100644 --- a/evp.go +++ b/evp.go @@ -89,78 +89,77 @@ func hashFuncToMD(fn func() hash.Hash) (C.GO_EVP_MD_PTR, error) { return md, nil } -// cryptoHashToMD converts a crypto.Hash to a GO_EVP_MD_PTR. -func cryptoHashToMD(ch crypto.Hash) (md C.GO_EVP_MD_PTR) { +// cryptoHashToMD converts a crypto.Hash to a EVP_MD. +func cryptoHashToMD(ch crypto.Hash) C.GO_EVP_MD_PTR { if v, ok := cacheMD.Load(ch); ok { return v.(C.GO_EVP_MD_PTR) } - defer func() { - if md != nil { - switch vMajor { - case 1: - // On OpenSSL 1 EVP_MD objects can be not-nil even - // when they are not supported. We need to pass the md - // to a EVP_MD_CTX to really know if they can be used. - ctx := C.go_openssl_EVP_MD_CTX_new() - if C.go_openssl_EVP_DigestInit_ex(ctx, md, nil) != 1 { - md = nil - } - C.go_openssl_EVP_MD_CTX_free(ctx) - case 3: - // On OpenSSL 3, directly operating on a EVP_MD object - // not created by EVP_MD_fetch has negative performance - // implications, as digest operations will have - // to fetch it on every call. Better to just fetch it once here. - md = C.go_openssl_EVP_MD_fetch(nil, C.go_openssl_EVP_MD_get0_name(md), nil) - default: - panic(errUnsupportedVersion()) - } - } - cacheMD.Store(ch, md) - }() - // SupportsHash returns false for MD5SHA1 because we don't - // provide a hash.Hash implementation for it. Yet, it can - // still be used when signing/verifying with an RSA key. - if ch == crypto.MD5SHA1 { - if vMajor == 1 && vMinor == 0 { - return C.go_openssl_EVP_md5_sha1_backport() - } else { - return C.go_openssl_EVP_md5_sha1() - } - } + var md C.GO_EVP_MD_PTR switch ch { + case crypto.RIPEMD160: + md = C.go_openssl_EVP_ripemd160() case crypto.MD4: - return C.go_openssl_EVP_md4() + md = C.go_openssl_EVP_md4() case crypto.MD5: - return C.go_openssl_EVP_md5() + md = C.go_openssl_EVP_md5() + case crypto.MD5SHA1: + if vMajor == 1 && vMinor == 0 { + md = C.go_openssl_EVP_md5_sha1_backport() + } else { + md = C.go_openssl_EVP_md5_sha1() + } case crypto.SHA1: - return C.go_openssl_EVP_sha1() + md = C.go_openssl_EVP_sha1() case crypto.SHA224: - return C.go_openssl_EVP_sha224() + md = C.go_openssl_EVP_sha224() case crypto.SHA256: - return C.go_openssl_EVP_sha256() + md = C.go_openssl_EVP_sha256() case crypto.SHA384: - return C.go_openssl_EVP_sha384() + md = C.go_openssl_EVP_sha384() case crypto.SHA512: - return C.go_openssl_EVP_sha512() + md = C.go_openssl_EVP_sha512() + case crypto.SHA512_224: + if versionAtOrAbove(1, 1, 1) { + md = C.go_openssl_EVP_sha512_224() + } + case crypto.SHA512_256: + if versionAtOrAbove(1, 1, 1) { + md = C.go_openssl_EVP_sha512_256() + } case crypto.SHA3_224: if versionAtOrAbove(1, 1, 1) { - return C.go_openssl_EVP_sha3_224() + md = C.go_openssl_EVP_sha3_224() } case crypto.SHA3_256: if versionAtOrAbove(1, 1, 1) { - return C.go_openssl_EVP_sha3_256() + md = C.go_openssl_EVP_sha3_256() } case crypto.SHA3_384: if versionAtOrAbove(1, 1, 1) { - return C.go_openssl_EVP_sha3_384() + md = C.go_openssl_EVP_sha3_384() } case crypto.SHA3_512: if versionAtOrAbove(1, 1, 1) { - return C.go_openssl_EVP_sha3_512() + md = C.go_openssl_EVP_sha3_512() } } - return nil + if md == nil { + cacheMD.Store(ch, nil) + } + if vMajor == 3 { + // On OpenSSL 3, directly operating on a EVP_MD object + // not created by EVP_MD_fetch has negative performance + // implications, as digest operations will have + // to fetch it on every call. Better to just fetch it once here. + md1 := C.go_openssl_EVP_MD_fetch(nil, C.go_openssl_EVP_MD_get0_name(md), nil) + // Don't overwrite md in case it can't be fetched, as the md may still be used + // outside of EVP_MD_CTX, for example to sign and verify RSA signatures. + if md1 != nil { + md = md1 + } + } + cacheMD.Store(ch, md) + return md } // generateEVPPKey generates a new EVP_PKEY with the given id and properties. @@ -218,6 +217,13 @@ type initFunc func(C.GO_EVP_PKEY_CTX_PTR) error type cryptFunc func(C.GO_EVP_PKEY_CTX_PTR, *C.uchar, *C.size_t, *C.uchar, C.size_t) error type verifyFunc func(C.GO_EVP_PKEY_CTX_PTR, *C.uchar, C.size_t, *C.uchar, C.size_t) error +func setRSAPAdding(ctx C.GO_EVP_PKEY_CTX_PTR, padding C.int) error { + if C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, C.GO_EVP_PKEY_RSA, -1, C.GO_EVP_PKEY_CTRL_RSA_PADDING, padding, nil) != 1 { + return newOpenSSLError("EVP_PKEY_CTX_ctrl failed") + } + return nil +} + func setupEVP(withKey withKeyFunc, padding C.int, h, mgfHash hash.Hash, label []byte, saltLen C.int, ch crypto.Hash, init initFunc) (_ C.GO_EVP_PKEY_CTX_PTR, err error) { @@ -246,10 +252,7 @@ func setupEVP(withKey withKeyFunc, padding C.int, // Each padding type has its own requirements in terms of when to apply the padding, // so it can't be just set at this point. setPadding := func() error { - if C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, C.GO_EVP_PKEY_RSA, -1, C.GO_EVP_PKEY_CTRL_RSA_PADDING, padding, nil) != 1 { - return newOpenSSLError("EVP_PKEY_CTX_ctrl failed") - } - return nil + return setRSAPAdding(ctx, padding) } switch padding { case C.GO_RSA_PKCS1_OAEP_PADDING: diff --git a/hash.go b/hash.go index 6fd3a518..2a9bbeb1 100644 --- a/hash.go +++ b/hash.go @@ -78,9 +78,28 @@ func SHA512(p []byte) (sum [64]byte) { return } +// cacheHashSupported is a cache of crypto.Hash support. +var cacheHashSupported sync.Map + // SupportsHash returns true if a hash.Hash implementation is supported for h. func SupportsHash(h crypto.Hash) bool { - return cryptoHashToMD(h) != nil + if v, ok := cacheHashSupported.Load(h); ok { + return v.(bool) + } + md := cryptoHashToMD(h) + if md == nil { + return false + } + // EVP_MD objects can be not-nil even when they can't be used + // in a EVP_MD_CTX, e.g. MD5 in FIPS mode. We need to prove + // if they can be used by passing them to a EVP_MD_CTX. + var supported bool + if ctx := C.go_openssl_EVP_MD_CTX_new(); ctx != nil { + supported = C.go_openssl_EVP_DigestInit_ex(ctx, md, nil) == 1 + C.go_openssl_EVP_MD_CTX_free(ctx) + } + cacheHashSupported.Store(h, supported) + return supported } func SHA3_224(p []byte) (sum [28]byte) { diff --git a/rsa.go b/rsa.go index da5c7636..8e2fa5f9 100644 --- a/rsa.go +++ b/rsa.go @@ -10,9 +10,55 @@ import ( "errors" "hash" "runtime" + "sync" "unsafe" ) +var testRSAKey = sync.OnceValue(func() C.GO_EVP_PKEY_PTR { + pkey, err := generateEVPPKey(C.GO_EVP_PKEY_RSA, 512, "") + if err != nil { + // Try with a larger key. + pkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 1024, "") + } + return pkey +}) + +var cachePKCS1Supported sync.Map + +// SupportsSignatureRSAPKCS1v15 returns true if PKCS1 signatures are supported +// for the given hash. +func SupportsSignatureRSAPKCS1v15(ch crypto.Hash) (supported bool) { + if val, ok := cachePKCS1Supported.Load(ch); ok { + return val.(bool) + } + defer func() { + cachePKCS1Supported.Store(ch, supported) + }() + md := cryptoHashToMD(ch) + if ch != 0 && md == nil { + return false + } + pkey := testRSAKey() + if pkey == nil { + return false + } + ctx := C.go_openssl_EVP_PKEY_CTX_new(pkey, nil) + if ctx == nil { + return false + } + defer C.go_openssl_EVP_PKEY_CTX_free(ctx) + if C.go_openssl_EVP_PKEY_sign_init(ctx) != 1 { + return false + } + if C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, -1, -1, C.GO_EVP_PKEY_CTRL_MD, 0, unsafe.Pointer(md)) != 1 { + return false + } + if setRSAPAdding(ctx, C.GO_RSA_PKCS1_PADDING) != nil { + return false + } + return true +} + func GenerateKeyRSA(bits int) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) { bad := func(e error) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) { return nil, nil, nil, nil, nil, nil, nil, nil, e diff --git a/rsa_test.go b/rsa_test.go index 5b92025e..1b7d0b60 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -194,6 +194,60 @@ func TestRSAEncryptDecryptOAEP_WrongLabel(t *testing.T) { } func TestRSASignVerifyPKCS1v15(t *testing.T) { + priv, pub := newRSAKey(t, 2048) + // These are all the hashes supported by Go's crypto/rsa package + // as off Go 1.24. + hashes := []crypto.Hash{ + 0, + crypto.MD5SHA1, + crypto.MD5, + crypto.SHA1, + crypto.SHA224, + crypto.SHA256, + crypto.SHA512, + crypto.SHA512_224, + crypto.SHA512_256, + crypto.SHA3_224, + crypto.SHA3_256, + crypto.SHA3_512, + crypto.RIPEMD160, + } + for _, hash := range hashes { + var name string + if hash == 0 { + name = "unhashed" + } else { + name = hash.String() + } + t.Run(name, func(t *testing.T) { + if !openssl.SupportsSignatureRSAPKCS1v15(hash) { + t.Skip("skipping test because hash is not supported") + } + // Construct a fake hashed data. + size := 1 + if hash != 0 { + size = hash.Size() + } + hashed := make([]byte, size) + hashed[0] = 0x30 + signed, err := openssl.SignRSAPKCS1v15(priv, hash, hashed) + if err != nil { + if openssl.FIPS() && (hash == 0 || hash == crypto.MD5SHA1 || hash == crypto.MD5 || hash == crypto.RIPEMD160) { + // This test is not supported in FIPS mode, but at least we + // can check that we don't panic. + t.Skip("skipping test in FIPS mode") + } + t.Fatal(err) + } + err = openssl.VerifyRSAPKCS1v15(pub, hash, hashed, signed) + if err != nil { + t.Fatal(err) + } + }) + } +} + +func TestRSAHashSignVerifyPKCS1v15(t *testing.T) { sha256 := openssl.NewSHA256() priv, pub := newRSAKey(t, 2048) msg := []byte("hi!") @@ -220,23 +274,6 @@ func TestRSASignVerifyPKCS1v15(t *testing.T) { } } -func TestRSASignVerifyPKCS1v15_Unhashed(t *testing.T) { - if openssl.SymCryptProviderAvailable() { - t.Skip("SymCrypt provider does not support unhashed PKCS1v15") - } - - msg := []byte("hi!") - priv, pub := newRSAKey(t, 2048) - signed, err := openssl.SignRSAPKCS1v15(priv, 0, msg) - if err != nil { - t.Fatal(err) - } - err = openssl.VerifyRSAPKCS1v15(pub, 0, msg, signed) - if err != nil { - t.Fatal(err) - } -} - func TestRSASignVerifyPKCS1v15_Invalid(t *testing.T) { sha256 := openssl.NewSHA256() msg := []byte("hi!") diff --git a/shims.h b/shims.h index c8f599f7..cfb65ff5 100644 --- a/shims.h +++ b/shims.h @@ -237,6 +237,7 @@ DEFINEFUNC_LEGACY_1_0(int, SHA1_Init, (GO_SHA_CTX_PTR c), (c)) \ DEFINEFUNC_LEGACY_1_0(int, SHA1_Update, (GO_SHA_CTX_PTR c, const void *data, size_t len), (c, data, len)) \ DEFINEFUNC_LEGACY_1_0(int, SHA1_Final, (unsigned char *md, GO_SHA_CTX_PTR c), (md, c)) \ DEFINEFUNC_1_1(const GO_EVP_MD_PTR, EVP_md5_sha1, (void), ()) \ +DEFINEFUNC(const GO_EVP_MD_PTR, EVP_ripemd160, (void), ()) \ DEFINEFUNC(const GO_EVP_MD_PTR, EVP_md4, (void), ()) \ DEFINEFUNC(const GO_EVP_MD_PTR, EVP_md5, (void), ()) \ DEFINEFUNC(const GO_EVP_MD_PTR, EVP_sha1, (void), ()) \ @@ -244,6 +245,8 @@ DEFINEFUNC(const GO_EVP_MD_PTR, EVP_sha224, (void), ()) \ DEFINEFUNC(const GO_EVP_MD_PTR, EVP_sha256, (void), ()) \ DEFINEFUNC(const GO_EVP_MD_PTR, EVP_sha384, (void), ()) \ DEFINEFUNC(const GO_EVP_MD_PTR, EVP_sha512, (void), ()) \ +DEFINEFUNC_1_1_1(const GO_EVP_MD_PTR, EVP_sha512_224, (void), ()) \ +DEFINEFUNC_1_1_1(const GO_EVP_MD_PTR, EVP_sha512_256, (void), ()) \ DEFINEFUNC_1_1_1(const GO_EVP_MD_PTR, EVP_sha3_224, (void), ()) \ DEFINEFUNC_1_1_1(const GO_EVP_MD_PTR, EVP_sha3_256, (void), ()) \ DEFINEFUNC_1_1_1(const GO_EVP_MD_PTR, EVP_sha3_384, (void), ()) \ From 3284129ea2bc70407f98c23dbe25bce6de6081e8 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 13 Dec 2024 16:11:03 +0100 Subject: [PATCH 02/12] fix memory leak --- rsa.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/rsa.go b/rsa.go index 8e2fa5f9..31fce768 100644 --- a/rsa.go +++ b/rsa.go @@ -14,13 +14,14 @@ import ( "unsafe" ) -var testRSAKey = sync.OnceValue(func() C.GO_EVP_PKEY_PTR { - pkey, err := generateEVPPKey(C.GO_EVP_PKEY_RSA, 512, "") - if err != nil { +var testRSAPkey C.GO_EVP_PKEY_PTR + +var initTestRSAKey = sync.OnceFunc(func() { + testRSAPkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 512, "") + if testRSAPkey == nil { // Try with a larger key. - pkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 1024, "") + testRSAPkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 1024, "") } - return pkey }) var cachePKCS1Supported sync.Map @@ -38,11 +39,11 @@ func SupportsSignatureRSAPKCS1v15(ch crypto.Hash) (supported bool) { if ch != 0 && md == nil { return false } - pkey := testRSAKey() - if pkey == nil { + initTestRSAKey() + if testRSAPkey == nil { return false } - ctx := C.go_openssl_EVP_PKEY_CTX_new(pkey, nil) + ctx := C.go_openssl_EVP_PKEY_CTX_new(testRSAPkey, nil) if ctx == nil { return false } From 41bb6ef005e69022cdcc2e16d4dec1bc6eb96b6e Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 13 Dec 2024 16:23:19 +0100 Subject: [PATCH 03/12] fallback to SupportsHash --- rsa.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rsa.go b/rsa.go index 31fce768..8ac910af 100644 --- a/rsa.go +++ b/rsa.go @@ -16,12 +16,10 @@ import ( var testRSAPkey C.GO_EVP_PKEY_PTR +// Use sync.OnceFunc instead of sync.OnceValue to avoid the +// memory sanitizer from incorrectly reporting the result as a leak. var initTestRSAKey = sync.OnceFunc(func() { - testRSAPkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 512, "") - if testRSAPkey == nil { - // Try with a larger key. - testRSAPkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 1024, "") - } + testRSAPkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 1024, "") }) var cachePKCS1Supported sync.Map @@ -41,7 +39,9 @@ func SupportsSignatureRSAPKCS1v15(ch crypto.Hash) (supported bool) { } initTestRSAKey() if testRSAPkey == nil { - return false + // The test key is not available, so we cannot determine if PKCS1 signatures are supported. + // Use SupportsHash instead as a fallback. + return SupportsHash(ch) } ctx := C.go_openssl_EVP_PKEY_CTX_new(testRSAPkey, nil) if ctx == nil { From 028b466a51af690ee0ddd84065104177c9d1dbb8 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 13 Dec 2024 16:27:39 +0100 Subject: [PATCH 04/12] add TestSupportsSignatureRSAPKCS1v15 --- rsa_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rsa_test.go b/rsa_test.go index 1b7d0b60..9e8c6673 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -193,6 +193,14 @@ func TestRSAEncryptDecryptOAEP_WrongLabel(t *testing.T) { } } +func TestSupportsSignatureRSAPKCS1v15(t *testing.T) { + // crypto.SHA256 should always be supported. + // Use it to test that the function works. + if !openssl.SupportsSignatureRSAPKCS1v15(crypto.SHA256) { + t.Error("crypto.SHA256 not supported") + } +} + func TestRSASignVerifyPKCS1v15(t *testing.T) { priv, pub := newRSAKey(t, 2048) // These are all the hashes supported by Go's crypto/rsa package From 8d39402d33659efabe223e5ebdc8b2c86bb135e5 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 13 Dec 2024 16:34:45 +0100 Subject: [PATCH 05/12] simplify func --- rsa.go | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/rsa.go b/rsa.go index 8ac910af..a027ced9 100644 --- a/rsa.go +++ b/rsa.go @@ -33,31 +33,26 @@ func SupportsSignatureRSAPKCS1v15(ch crypto.Hash) (supported bool) { defer func() { cachePKCS1Supported.Store(ch, supported) }() - md := cryptoHashToMD(ch) - if ch != 0 && md == nil { - return false + var md C.GO_EVP_MD_PTR + if ch != 0 { + md = cryptoHashToMD(ch) + if md == nil { + return false + } } initTestRSAKey() if testRSAPkey == nil { - // The test key is not available, so we cannot determine if PKCS1 signatures are supported. - // Use SupportsHash instead as a fallback. - return SupportsHash(ch) + return false } ctx := C.go_openssl_EVP_PKEY_CTX_new(testRSAPkey, nil) if ctx == nil { return false } defer C.go_openssl_EVP_PKEY_CTX_free(ctx) - if C.go_openssl_EVP_PKEY_sign_init(ctx) != 1 { - return false - } - if C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, -1, -1, C.GO_EVP_PKEY_CTRL_MD, 0, unsafe.Pointer(md)) != 1 { - return false - } - if setRSAPAdding(ctx, C.GO_RSA_PKCS1_PADDING) != nil { - return false - } - return true + + return C.go_openssl_EVP_PKEY_sign_init(ctx) == 1 && + C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, -1, -1, C.GO_EVP_PKEY_CTRL_MD, 0, unsafe.Pointer(md)) == 1 && + setRSAPAdding(ctx, C.GO_RSA_PKCS1_PADDING) == nil } func GenerateKeyRSA(bits int) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) { From 34344f7b1df7ae16d05e2c54d0d3b987b690c30f Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 13 Dec 2024 16:43:14 +0100 Subject: [PATCH 06/12] fix FIPS mode --- rsa.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rsa.go b/rsa.go index a027ced9..36e719ae 100644 --- a/rsa.go +++ b/rsa.go @@ -50,9 +50,12 @@ func SupportsSignatureRSAPKCS1v15(ch crypto.Hash) (supported bool) { } defer C.go_openssl_EVP_PKEY_CTX_free(ctx) + msg := make([]byte, 1) + var outLen C.size_t return C.go_openssl_EVP_PKEY_sign_init(ctx) == 1 && C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, -1, -1, C.GO_EVP_PKEY_CTRL_MD, 0, unsafe.Pointer(md)) == 1 && - setRSAPAdding(ctx, C.GO_RSA_PKCS1_PADDING) == nil + setRSAPAdding(ctx, C.GO_RSA_PKCS1_PADDING) == nil && + C.go_openssl_EVP_PKEY_sign(ctx, nil, &outLen, base(msg), C.size_t(len(msg))) == 1 } func GenerateKeyRSA(bits int) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) { From 988485d84a08c304d24779aa4efe3f96318fde49 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 17 Dec 2024 13:58:06 +0100 Subject: [PATCH 07/12] small fixes --- hash.go | 5 +++-- rsa.go | 45 --------------------------------------------- rsa_test.go | 17 ++++------------- 3 files changed, 7 insertions(+), 60 deletions(-) diff --git a/hash.go b/hash.go index 2a9bbeb1..379dd1b8 100644 --- a/hash.go +++ b/hash.go @@ -81,16 +81,17 @@ func SHA512(p []byte) (sum [64]byte) { // cacheHashSupported is a cache of crypto.Hash support. var cacheHashSupported sync.Map -// SupportsHash returns true if a hash.Hash implementation is supported for h. +// SupportsHash reports whether the current OpenSSL version supports the given hash. func SupportsHash(h crypto.Hash) bool { if v, ok := cacheHashSupported.Load(h); ok { return v.(bool) } md := cryptoHashToMD(h) if md == nil { + cacheHashSupported.Store(h, false) return false } - // EVP_MD objects can be not-nil even when they can't be used + // EVP_MD objects can be non-nil even when they can't be used // in a EVP_MD_CTX, e.g. MD5 in FIPS mode. We need to prove // if they can be used by passing them to a EVP_MD_CTX. var supported bool diff --git a/rsa.go b/rsa.go index 36e719ae..da5c7636 100644 --- a/rsa.go +++ b/rsa.go @@ -10,54 +10,9 @@ import ( "errors" "hash" "runtime" - "sync" "unsafe" ) -var testRSAPkey C.GO_EVP_PKEY_PTR - -// Use sync.OnceFunc instead of sync.OnceValue to avoid the -// memory sanitizer from incorrectly reporting the result as a leak. -var initTestRSAKey = sync.OnceFunc(func() { - testRSAPkey, _ = generateEVPPKey(C.GO_EVP_PKEY_RSA, 1024, "") -}) - -var cachePKCS1Supported sync.Map - -// SupportsSignatureRSAPKCS1v15 returns true if PKCS1 signatures are supported -// for the given hash. -func SupportsSignatureRSAPKCS1v15(ch crypto.Hash) (supported bool) { - if val, ok := cachePKCS1Supported.Load(ch); ok { - return val.(bool) - } - defer func() { - cachePKCS1Supported.Store(ch, supported) - }() - var md C.GO_EVP_MD_PTR - if ch != 0 { - md = cryptoHashToMD(ch) - if md == nil { - return false - } - } - initTestRSAKey() - if testRSAPkey == nil { - return false - } - ctx := C.go_openssl_EVP_PKEY_CTX_new(testRSAPkey, nil) - if ctx == nil { - return false - } - defer C.go_openssl_EVP_PKEY_CTX_free(ctx) - - msg := make([]byte, 1) - var outLen C.size_t - return C.go_openssl_EVP_PKEY_sign_init(ctx) == 1 && - C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, -1, -1, C.GO_EVP_PKEY_CTRL_MD, 0, unsafe.Pointer(md)) == 1 && - setRSAPAdding(ctx, C.GO_RSA_PKCS1_PADDING) == nil && - C.go_openssl_EVP_PKEY_sign(ctx, nil, &outLen, base(msg), C.size_t(len(msg))) == 1 -} - func GenerateKeyRSA(bits int) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) { bad := func(e error) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) { return nil, nil, nil, nil, nil, nil, nil, nil, e diff --git a/rsa_test.go b/rsa_test.go index 9e8c6673..7e9710e4 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -7,6 +7,7 @@ import ( "fmt" "math/big" "strconv" + "strings" "testing" "github.com/golang-fips/openssl/v2" @@ -193,14 +194,6 @@ func TestRSAEncryptDecryptOAEP_WrongLabel(t *testing.T) { } } -func TestSupportsSignatureRSAPKCS1v15(t *testing.T) { - // crypto.SHA256 should always be supported. - // Use it to test that the function works. - if !openssl.SupportsSignatureRSAPKCS1v15(crypto.SHA256) { - t.Error("crypto.SHA256 not supported") - } -} - func TestRSASignVerifyPKCS1v15(t *testing.T) { priv, pub := newRSAKey(t, 2048) // These are all the hashes supported by Go's crypto/rsa package @@ -228,7 +221,7 @@ func TestRSASignVerifyPKCS1v15(t *testing.T) { name = hash.String() } t.Run(name, func(t *testing.T) { - if !openssl.SupportsSignatureRSAPKCS1v15(hash) { + if hash != 0 && !openssl.SupportsHash(hash) { t.Skip("skipping test because hash is not supported") } // Construct a fake hashed data. @@ -240,10 +233,8 @@ func TestRSASignVerifyPKCS1v15(t *testing.T) { hashed[0] = 0x30 signed, err := openssl.SignRSAPKCS1v15(priv, hash, hashed) if err != nil { - if openssl.FIPS() && (hash == 0 || hash == crypto.MD5SHA1 || hash == crypto.MD5 || hash == crypto.RIPEMD160) { - // This test is not supported in FIPS mode, but at least we - // can check that we don't panic. - t.Skip("skipping test in FIPS mode") + if strings.Contains(err.Error(), "check_padding_md:invalid digest") { + t.Skip("skipping test because hash is not supported by PKCS1v15") } t.Fatal(err) } From 6af264f042992a6c9689b301d1fcc179ea29bb93 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 17 Dec 2024 14:07:35 +0100 Subject: [PATCH 08/12] test RSA PSS --- rsa_test.go | 65 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/rsa_test.go b/rsa_test.go index 7e9710e4..4141d0fa 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -194,26 +194,26 @@ func TestRSAEncryptDecryptOAEP_WrongLabel(t *testing.T) { } } +// These are all the hashes supported by Go's crypto/rsa package +// as off Go 1.24. +var stdHashes = [...]crypto.Hash{ + crypto.MD5SHA1, + crypto.MD5, + crypto.SHA1, + crypto.SHA224, + crypto.SHA256, + crypto.SHA512, + crypto.SHA512_224, + crypto.SHA512_256, + crypto.SHA3_224, + crypto.SHA3_256, + crypto.SHA3_512, + crypto.RIPEMD160, +} + func TestRSASignVerifyPKCS1v15(t *testing.T) { priv, pub := newRSAKey(t, 2048) - // These are all the hashes supported by Go's crypto/rsa package - // as off Go 1.24. - hashes := []crypto.Hash{ - 0, - crypto.MD5SHA1, - crypto.MD5, - crypto.SHA1, - crypto.SHA224, - crypto.SHA256, - crypto.SHA512, - crypto.SHA512_224, - crypto.SHA512_256, - crypto.SHA3_224, - crypto.SHA3_256, - crypto.SHA3_512, - crypto.RIPEMD160, - } - for _, hash := range hashes { + for _, hash := range append([]crypto.Hash{0}, stdHashes[:]...) { var name string if hash == 0 { name = "unhashed" @@ -290,6 +290,35 @@ func TestRSASignVerifyPKCS1v15_Invalid(t *testing.T) { } func TestRSASignVerifyRSAPSS(t *testing.T) { + priv, pub := newRSAKey(t, 2048) + for _, hash := range stdHashes { + t.Run(hash.String(), func(t *testing.T) { + if !openssl.SupportsHash(hash) { + t.Skip("skipping test because hash is not supported") + } + // Construct a fake hashed data. + size := 1 + if hash != 0 { + size = hash.Size() + } + hashed := make([]byte, size) + hashed[0] = 0x30 + signed, err := openssl.SignRSAPSS(priv, hash, hashed, rsa.PSSSaltLengthEqualsHash) + if err != nil { + if strings.Contains(err.Error(), "check_padding_md:invalid digest") { + t.Skip("skipping test because hash is not supported by PKCS1v15") + } + t.Fatal(err) + } + err = openssl.VerifyRSAPSS(pub, hash, hashed, signed, rsa.PSSSaltLengthEqualsHash) + if err != nil { + t.Fatal(err) + } + }) + } +} + +func TestRSASignVerifyRSAPSS_SaltLength(t *testing.T) { // Test cases taken from // https://github.com/golang/go/blob/54182ff54a687272dd7632c3a963e036ce03cb7c/src/crypto/rsa/pss_test.go#L200. const keyBits = 2048 From 007e0cc64fc4291581a14c1349d69e6084e5e9fd Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 17 Dec 2024 14:25:30 +0100 Subject: [PATCH 09/12] improve error handling --- rsa_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rsa_test.go b/rsa_test.go index 4141d0fa..042e201e 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -233,8 +233,10 @@ func TestRSASignVerifyPKCS1v15(t *testing.T) { hashed[0] = 0x30 signed, err := openssl.SignRSAPKCS1v15(priv, hash, hashed) if err != nil { - if strings.Contains(err.Error(), "check_padding_md:invalid digest") { - t.Skip("skipping test because hash is not supported by PKCS1v15") + if strings.Contains(err.Error(), "invalid digest") || strings.Contains(err.Error(), "digest not allowed") { + // Can happen if the hash is supported by EVP_MD_CTX but not by EVP_PKEY_CTX. + // There is nothing we can do about it. + t.Skip("skipping test because hash is not supported") } t.Fatal(err) } @@ -305,8 +307,10 @@ func TestRSASignVerifyRSAPSS(t *testing.T) { hashed[0] = 0x30 signed, err := openssl.SignRSAPSS(priv, hash, hashed, rsa.PSSSaltLengthEqualsHash) if err != nil { - if strings.Contains(err.Error(), "check_padding_md:invalid digest") { - t.Skip("skipping test because hash is not supported by PKCS1v15") + if strings.Contains(err.Error(), "invalid digest") || strings.Contains(err.Error(), "digest not allowed") { + // Can happen if the hash is supported by EVP_MD_CTX but not by EVP_PKEY_CTX. + // There is nothing we can do about it. + t.Skip("skipping test because hash is not supported") } t.Fatal(err) } From 84870057d737a8857f2a4840a34668574a5b57b8 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 17 Dec 2024 14:27:23 +0100 Subject: [PATCH 10/12] revert setPadding changes --- evp.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/evp.go b/evp.go index 29822de4..bd51c3f3 100644 --- a/evp.go +++ b/evp.go @@ -217,13 +217,6 @@ type initFunc func(C.GO_EVP_PKEY_CTX_PTR) error type cryptFunc func(C.GO_EVP_PKEY_CTX_PTR, *C.uchar, *C.size_t, *C.uchar, C.size_t) error type verifyFunc func(C.GO_EVP_PKEY_CTX_PTR, *C.uchar, C.size_t, *C.uchar, C.size_t) error -func setRSAPAdding(ctx C.GO_EVP_PKEY_CTX_PTR, padding C.int) error { - if C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, C.GO_EVP_PKEY_RSA, -1, C.GO_EVP_PKEY_CTRL_RSA_PADDING, padding, nil) != 1 { - return newOpenSSLError("EVP_PKEY_CTX_ctrl failed") - } - return nil -} - func setupEVP(withKey withKeyFunc, padding C.int, h, mgfHash hash.Hash, label []byte, saltLen C.int, ch crypto.Hash, init initFunc) (_ C.GO_EVP_PKEY_CTX_PTR, err error) { @@ -252,7 +245,10 @@ func setupEVP(withKey withKeyFunc, padding C.int, // Each padding type has its own requirements in terms of when to apply the padding, // so it can't be just set at this point. setPadding := func() error { - return setRSAPAdding(ctx, padding) + if C.go_openssl_EVP_PKEY_CTX_ctrl(ctx, C.GO_EVP_PKEY_RSA, -1, C.GO_EVP_PKEY_CTRL_RSA_PADDING, padding, nil) != 1 { + return newOpenSSLError("EVP_PKEY_CTX_ctrl failed") + } + return nil } switch padding { case C.GO_RSA_PKCS1_OAEP_PADDING: From 1eba53d528f0300c32f117ff5726eed1f3c12313 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 17 Dec 2024 14:29:43 +0100 Subject: [PATCH 11/12] fix cryptoHashToMD --- evp.go | 1 + 1 file changed, 1 insertion(+) diff --git a/evp.go b/evp.go index bd51c3f3..f6a2d3ed 100644 --- a/evp.go +++ b/evp.go @@ -145,6 +145,7 @@ func cryptoHashToMD(ch crypto.Hash) C.GO_EVP_MD_PTR { } if md == nil { cacheMD.Store(ch, nil) + return nil } if vMajor == 3 { // On OpenSSL 3, directly operating on a EVP_MD object From ae7ba9c547bdd9ec42fa0fa5372ef01f3ab8e6a8 Mon Sep 17 00:00:00 2001 From: Quim Muntal Date: Tue, 17 Dec 2024 19:13:55 +0100 Subject: [PATCH 12/12] Update rsa_test.go Co-authored-by: Davis Goodin --- rsa_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rsa_test.go b/rsa_test.go index 042e201e..4b383384 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -195,7 +195,7 @@ func TestRSAEncryptDecryptOAEP_WrongLabel(t *testing.T) { } // These are all the hashes supported by Go's crypto/rsa package -// as off Go 1.24. +// as of Go 1.24. var stdHashes = [...]crypto.Hash{ crypto.MD5SHA1, crypto.MD5,