ext/openssl: allow openssl_sign()/openssl_verify() to sign/verify a precomputed digest - #23506
Open
2akouwu wants to merge 1 commit into
Open
ext/openssl: allow openssl_sign()/openssl_verify() to sign/verify a precomputed digest#235062akouwu wants to merge 1 commit into
2akouwu wants to merge 1 commit into
Conversation
openssl_sign() and openssl_verify() always drive OpenSSL's one-shot EVP_DigestSign()/EVP_DigestVerify() APIs, which hash the supplied data internally before signing/verifying it. Callers who already hold a precomputed digest (e.g. produced by a HSM, or hashed incrementally) had no way to sign or verify it directly: passing the digest as $data causes it to be hashed a second time. Add an optional trailing bool $data_is_digest parameter (default false) to both functions. When true, $data is signed/verified directly via the low-level EVP_PKEY_sign()/EVP_PKEY_verify() APIs instead of being re-hashed, after registering the digest algorithm on the EVP_PKEY_CTX via EVP_PKEY_CTX_set_signature_md() so RSA PKCS#1 and PSS padding still use the correct DigestInfo/MGF1 digest. The existing RSA padding and PSS salt-length helpers are reused unchanged. Fixes phpGH-23422. Signed-off-by: ulofiai <309826581+ulofiai@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #23422.
Root cause
openssl_sign()andopenssl_verify()are implemented purely on top of OpenSSL's one-shotEVP_DigestSign()/EVP_DigestVerify()APIs. Those APIs always hash the buffer that is passed to them before performing the RSA/EC/etc. signature operation. That is correct when$datais the original message, but it means there is currently no way to sign or verify a digest that was already computed elsewhere (e.g. by a HSM, or incrementally viahash_update()/hash_final()): passing the digest as$datajust hashes it a second time and producessign(SHA256(digest))instead ofsign(digest).Why this fix
OpenSSL exposes exactly the primitive needed for this:
EVP_PKEY_sign_init()/EVP_PKEY_sign()andEVP_PKEY_verify_init()/EVP_PKEY_verify()operate on a buffer without hashing it, as long as the expected digest algorithm is registered on theEVP_PKEY_CTXviaEVP_PKEY_CTX_set_signature_md()(RSA needs this to build the correct PKCS#1DigestInfoprefix / PSS MGF1 digest; for EC/DSA it is used to validate the digest length).I added a new optional trailing parameter,
bool $data_is_digest = false, to bothopenssl_sign()andopenssl_verify(), defaulting tofalseso existing call sites are unaffected. Whentrue, the implementation takes a separate branch that builds its ownEVP_PKEY_CTXand drives the raw sign/verify primitives instead ofEVP_DigestSign/EVP_DigestVerify. It reuses the existingphp_openssl_setup_rsa_padding()andphp_openssl_setup_rsa_pss_salt_length()static helpers unchanged, since they only operate on anEVP_PKEY_CTXand don't care how it was created, so padding/PSS-salt-length handling stays identical between both code paths.I considered instead exposing this only through a brand-new function (e.g.
openssl_pkey_sign_digest()), but extending the existing functions with a defaulted parameter is smaller, keeps all the existing algorithm/padding/salt-length validation and error handling in one place, and mirrors howopenssl_private_encrypt()/openssl_public_decrypt()etc. already expose several signing-adjacent primitives as parameterized variants of one function rather than a family of near-duplicate functions.For keys where OpenSSL doesn't support raw sign/verify at all (e.g. Ed25519, which only implements the one-shot digest-sign flow),
EVP_PKEY_sign_init()/EVP_PKEY_verify_init()fail naturally and the existingphp_openssl_store_errors()path surfaces that as a warning +false/-1, same as any other OpenSSL-level failure already handled by these functions.ext/openssl/openssl.stub.phpand the generatedext/openssl/openssl_arginfo.hwere updated together (the stub hash comment was recomputed by hand sincegen_stub.phprequires a PHP CLI that isn't available in this sandbox);UPGRADINGdocuments the new parameter.Testing
Added
ext/openssl/tests/gh23422.phpt, which:openssl_pkey_new()(no fixture files needed).openssl_sign($data, ..., OPENSSL_ALGO_SHA256)) and once as a precomputed digest (openssl_sign(hash('sha256', $data, true), ..., OPENSSL_ALGO_SHA256, 0, ..., true)), and asserts the two PKCS#1 v1.5 signatures are byte-identical — this is the core proof that signing a precomputed digest now produces the mathematically correct signature rather than double-hashing it.openssl_verify(..., data_is_digest: true/false).int(0)) instead of silently succeeding.OPENSSL_PKCS1_PSS_PADDINGto confirm padding/salt-length handling still applies on the new code path.I could not execute the test suite in this sandbox (no PHP/build toolchain available, and the bundle doesn't include the pre-generated RSA key fixtures used by some of the pre-existing openssl_sign*/openssl_verify* tests), so the new test intentionally avoids depending on any fixture files and generates its own key. I traced the OpenSSL/PHP control flow by hand to confirm the expected output, which is documented as
--EXPECT--in the phpt file.