From 47e482291d9c2e8f4fc9f4ea845cb2e0d34ad34f Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Nov 2022 10:46:23 -0700 Subject: [PATCH 1/3] Commit 3 Differential Revision: D40942193 fbshipit-source-id: 3b58a588ad34cda7831feed4d62321ce2411e66b --- hphp/runtime/ext/openssl/ext_openssl.cpp | 70 ++++++++++++++++-------- 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/hphp/runtime/ext/openssl/ext_openssl.cpp b/hphp/runtime/ext/openssl/ext_openssl.cpp index c90ea3ace3ebc7..f5c96be42342c0 100644 --- a/hphp/runtime/ext/openssl/ext_openssl.cpp +++ b/hphp/runtime/ext/openssl/ext_openssl.cpp @@ -1907,7 +1907,7 @@ Array HHVM_FUNCTION(openssl_pkey_get_details, const Resource& key) { case EVP_PKEY_RSA2: { ktype = OPENSSL_KEYTYPE_RSA; - RSA *rsa = EVP_PKEY_get0_RSA(pkey); + auto rsa = EVP_PKEY_get0_RSA(pkey); assertx(rsa); const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp; RSA_get0_key(rsa, &n, &e, &d); @@ -1930,7 +1930,7 @@ Array HHVM_FUNCTION(openssl_pkey_get_details, const Resource& key) { case EVP_PKEY_DSA4: { ktype = OPENSSL_KEYTYPE_DSA; - DSA *dsa = EVP_PKEY_get0_DSA(pkey); + auto dsa = EVP_PKEY_get0_DSA(pkey); assertx(dsa); const BIGNUM *p, *q, *g, *pub_key, *priv_key; DSA_get0_pqg(dsa, &p, &q, &g); @@ -1946,7 +1946,7 @@ Array HHVM_FUNCTION(openssl_pkey_get_details, const Resource& key) { case EVP_PKEY_DH: { ktype = OPENSSL_KEYTYPE_DH; - DH *dh = EVP_PKEY_get0_DH(pkey); + auto dh = EVP_PKEY_get0_DH(pkey); assertx(dh); const BIGNUM *p, *q, *g, *pub_key, *priv_key; DH_get0_pqg(dh, &p, &q, &g); @@ -2060,11 +2060,17 @@ bool HHVM_FUNCTION(openssl_private_decrypt, const String& data, switch (EVP_PKEY_id(pkey)) { case EVP_PKEY_RSA: case EVP_PKEY_RSA2: - cryptedlen = RSA_private_decrypt(data.size(), - (unsigned char *)data.data(), - cryptedbuf, - EVP_PKEY_get0_RSA(pkey), - padding); + { + auto rsa = EVP_PKEY_get1_RSA(pkey); + SCOPE_EXIT { + RSA_free(rsa); + }; + cryptedlen = RSA_private_decrypt(data.size(), + (unsigned char *)data.data(), + cryptedbuf, + rsa, + padding); + } if (cryptedlen != -1) { successful = 1; } @@ -2100,11 +2106,17 @@ bool HHVM_FUNCTION(openssl_private_encrypt, const String& data, switch (EVP_PKEY_id(pkey)) { case EVP_PKEY_RSA: case EVP_PKEY_RSA2: - successful = (RSA_private_encrypt(data.size(), - (unsigned char *)data.data(), - cryptedbuf, - EVP_PKEY_get0_RSA(pkey), - padding) == cryptedlen); + { + auto rsa = EVP_PKEY_get1_RSA(pkey); + SCOPE_EXIT { + RSA_free(rsa); + }; + successful = (RSA_private_encrypt(data.size(), + (unsigned char *)data.data(), + cryptedbuf, + rsa, + padding) == cryptedlen); + } break; default: raise_warning("key type not supported"); @@ -2136,11 +2148,17 @@ bool HHVM_FUNCTION(openssl_public_decrypt, const String& data, switch (EVP_PKEY_id(pkey)) { case EVP_PKEY_RSA: case EVP_PKEY_RSA2: - cryptedlen = RSA_public_decrypt(data.size(), - (unsigned char *)data.data(), - cryptedbuf, - EVP_PKEY_get0_RSA(pkey), - padding); + { + auto rsa = EVP_PKEY_get1_RSA(pkey); + SCOPE_EXIT { + RSA_free(rsa); + }; + cryptedlen = RSA_public_decrypt(data.size(), + (unsigned char *)data.data(), + cryptedbuf, + rsa, + padding); + } if (cryptedlen != -1) { successful = 1; } @@ -2176,11 +2194,17 @@ bool HHVM_FUNCTION(openssl_public_encrypt, const String& data, switch (EVP_PKEY_id(pkey)) { case EVP_PKEY_RSA: case EVP_PKEY_RSA2: - successful = (RSA_public_encrypt(data.size(), - (unsigned char *)data.data(), - cryptedbuf, - EVP_PKEY_get0_RSA(pkey), - padding) == cryptedlen); + { + auto rsa = EVP_PKEY_get1_RSA(pkey); + SCOPE_EXIT { + RSA_free(rsa); + }; + successful = (RSA_public_encrypt(data.size(), + (unsigned char *)data.data(), + cryptedbuf, + rsa, + padding) == cryptedlen); + } break; default: raise_warning("key type not supported"); From e993d4b3435235a26d28c0ad228ca381efbe2cf2 Mon Sep 17 00:00:00 2001 From: "Yang, Bo" Date: Wed, 2 Nov 2022 10:46:23 -0700 Subject: [PATCH 2/3] Extract public key portion via PEM roundtrip (#9282) Summary: This diff applies the approach similar to https://github.com/php/php-src/commit/26a51e8d7a6026f6bd69813d044785d154a296a3 in order to fix behavior changes in OpenSSL 3 Pull Request resolved: https://github.com/facebook/hhvm/pull/9282 Test Plan: The existing tests should still pass with OpenSSL 1.1 ``` "$HHVM_BIN" hphp/test/run.php hphp/test/slow/ext_openssl/ext_openssl.php ``` Differential Revision: D40876120 Pulled By: Atry fbshipit-source-id: a0d7d9932da78f06b986096f45a03d169331f38c --- hphp/runtime/ext/openssl/ext_openssl.cpp | 26 +++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/hphp/runtime/ext/openssl/ext_openssl.cpp b/hphp/runtime/ext/openssl/ext_openssl.cpp index f5c96be42342c0..7dc120ac791370 100644 --- a/hphp/runtime/ext/openssl/ext_openssl.cpp +++ b/hphp/runtime/ext/openssl/ext_openssl.cpp @@ -998,25 +998,27 @@ bool HHVM_FUNCTION(openssl_csr_export, const Variant& csr, Variant& out, return false; } +static EVP_PKEY *duplicate_public_key(EVP_PKEY *priv_key) { + /* Extract public key portion by round-tripping through PEM. */ + BIO *bio = BIO_new(BIO_s_mem()); + SCOPE_EXIT { BIO_free(bio); }; + if (!bio || !PEM_write_bio_PUBKEY(bio, priv_key)) { + return nullptr; + } + + EVP_PKEY *pub_key = PEM_read_bio_PUBKEY(bio, nullptr, nullptr, nullptr); + return pub_key; +} + Variant HHVM_FUNCTION(openssl_csr_get_public_key, const Variant& csr) { auto pcsr = CSRequest::Get(csr); if (!pcsr) return false; auto input_csr = pcsr->csr(); -#if OPENSSL_VERSION_NUMBER >= 0x10100000 - /* Due to changes in OpenSSL 1.1 related to locking when decoding CSR, - * the pub key is not changed after assigning. It means if we pass - * a private key, it will be returned including the private part. - * If we duplicate it, then we get just the public part which is - * the same behavior as for OpenSSL 1.0 */ - input_csr = X509_REQ_dup(input_csr); - /* We need to free the CSR as it was duplicated */ - SCOPE_EXIT { X509_REQ_free(input_csr); }; -#endif - auto pubkey = X509_REQ_get_pubkey(input_csr); + auto pubkey = X509_REQ_get0_pubkey(input_csr); if (!pubkey) return false; - return Variant(req::make(pubkey)); + return Variant(req::make(duplicate_public_key(pubkey))); } Variant HHVM_FUNCTION(openssl_csr_get_subject, const Variant& csr, From e7b15c6c86ee514ee4b1b97cd8c5bbbb0d95cab9 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Nov 2022 10:46:48 -0700 Subject: [PATCH 3/3] Load the legacy providers from OpenSSL 3 Summary: `openssl_seal` is by default using RC4. However RC4 is only available from the legacy providers in OpenSSL 3, which is not loaded by default. This diff loads the legacy providers so that `openssl_seal` will not fail with default parameters. See https://www.openssl.org/docs/man3.0/man7/crypto.html#:~:text=md_whirlpool)%3B%0AEVP_MD_free(md_sha256)%3B-,OPENSSL%20PROVIDERS,-OpenSSL%20comes%20with for providers in OpenSSL 3 ## Internal: We should also consider removing the default value `'RC4'` from `openssl_seal` like [what PHP 8.0 did](https://www.php.net/manual/en/function.openssl-seal.php), because RC4 is vulnerable. Differential Revision: D40942189 fbshipit-source-id: 5669e7ba2caf09b6e2c12b7de709141276308513 --- hphp/runtime/ext/openssl/ext_openssl.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hphp/runtime/ext/openssl/ext_openssl.cpp b/hphp/runtime/ext/openssl/ext_openssl.cpp index 7dc120ac791370..3a63bdb4e21180 100644 --- a/hphp/runtime/ext/openssl/ext_openssl.cpp +++ b/hphp/runtime/ext/openssl/ext_openssl.cpp @@ -28,6 +28,9 @@ #include #include #include +#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3) +#include +#endif #include #include @@ -66,6 +69,12 @@ struct OpenSSLInitializer { ERR_load_crypto_strings(); ERR_load_EVP_strings(); +// RC4 is only available in legacy providers +#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3) + OSSL_PROVIDER_load(nullptr, "default"); + OSSL_PROVIDER_load(nullptr, "legacy"); +#endif + /* Determine default SSL configuration file */ char *config_filename = getenv("OPENSSL_CONF"); if (config_filename == nullptr) {