From d04048fb044c63d8032f0f950d41c93276ec9bdf Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 10 Apr 2023 20:12:31 +0200 Subject: [PATCH 1/6] ext/openssl: Prevent exposed error when using a PEM public key Adapt tests to changed behavior Remove line --- ext/openssl/openssl.c | 43 +++++++++++++++++++ ext/openssl/php_openssl.h | 1 + ext/openssl/tests/bug11054.pem | 9 ++++ ext/openssl/tests/bug11054.phpt | 15 +++++++ .../openssl_error_string_basic_openssl3.phpt | 6 +-- 5 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 ext/openssl/tests/bug11054.pem create mode 100644 ext/openssl/tests/bug11054.phpt diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 10af453c89519..de25a1de5c2dc 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -474,6 +474,47 @@ void php_openssl_store_errors() } /* }}} */ +/* {{{ php_openssl_errors_set_mark */ +void php_openssl_errors_set_mark(void) { + struct php_openssl_errors *errors; + struct php_openssl_errors *errors_mark; + + if (!OPENSSL_G(errors)) { + return; + } + + if (!OPENSSL_G(errors_mark)) { + OPENSSL_G(errors_mark) = pecalloc(1, sizeof(struct php_openssl_errors), 1); + } + + errors = OPENSSL_G(errors); + errors_mark = OPENSSL_G(errors_mark); + + memcpy(errors_mark, errors, sizeof(struct php_openssl_errors)); +} +/* }}} */ + +/* {{{ php_openssl_errors_restore_mark */ +void php_openssl_errors_restore_mark(void) { + struct php_openssl_errors *errors; + struct php_openssl_errors *errors_mark; + + if (!OPENSSL_G(errors)) { + return; + } + + errors = OPENSSL_G(errors); + + if (!OPENSSL_G(errors_mark)) { + errors->top = 0; + errors->bottom = 0; + } else { + errors_mark = OPENSSL_G(errors_mark); + memcpy(errors, errors_mark, sizeof(struct php_openssl_errors)); + } +} +/* }}} */ + /* openssl file path check error function */ static void php_openssl_check_path_error(uint32_t arg_num, int type, const char *format, ...) { @@ -3690,12 +3731,14 @@ static EVP_PKEY *php_openssl_pkey_from_zval( } /* it's an X509 file/cert of some kind, and we need to extract the data from that */ if (public_key) { + php_openssl_errors_set_mark(); cert = php_openssl_x509_from_str(Z_STR_P(val), arg_num, false, NULL); if (cert) { free_cert = 1; } else { /* not a X509 certificate, try to retrieve public key */ + php_openssl_errors_restore_mark(); BIO* in; if (is_file) { in = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY)); diff --git a/ext/openssl/php_openssl.h b/ext/openssl/php_openssl.h index 3f0c436190fa3..66f76f6ace546 100644 --- a/ext/openssl/php_openssl.h +++ b/ext/openssl/php_openssl.h @@ -80,6 +80,7 @@ struct php_openssl_errors { ZEND_BEGIN_MODULE_GLOBALS(openssl) struct php_openssl_errors *errors; + struct php_openssl_errors *errors_mark; ZEND_END_MODULE_GLOBALS(openssl) #define OPENSSL_G(v) ZEND_MODULE_GLOBALS_ACCESSOR(openssl, v) diff --git a/ext/openssl/tests/bug11054.pem b/ext/openssl/tests/bug11054.pem new file mode 100644 index 0000000000000..60d7afa827f2c --- /dev/null +++ b/ext/openssl/tests/bug11054.pem @@ -0,0 +1,9 @@ +-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvYH14fT4DPgyffkDOrHt +x0q+rxclB48h2ykgbR3QyDG2d7hMSXjtqEseO/iR1FdAv7UevIKyHFbHpJilOIwo +mEqQNxUQCWdZsWhv7ZVfG8UUgki7LKMGPruJM97vteBS101hSCaCQz+zTyVyP8Uy +nqx5zlPmcBUA92gAFfSCa+tm/lR2BY5g/20mZX/lMY0xXV1iLhfdK6RgJYXX2SdH +YR/01IgmjgTfIp7gX+xixDgGZuZY++jo8C52udFkCf5vxyG4Ed57vRfCLFOPfeY4 +r3i0Jiply65zSo8y/6KxudRtmGOfV2qb2EsMTW9PaLs3+rnhhiYBM/nR4V5ux6u6 +DwIDAQAB +-----END PUBLIC KEY----- diff --git a/ext/openssl/tests/bug11054.phpt b/ext/openssl/tests/bug11054.phpt new file mode 100644 index 0000000000000..25f23d5b1de3e --- /dev/null +++ b/ext/openssl/tests/bug11054.phpt @@ -0,0 +1,15 @@ +--TEST-- +Bug #11054: Calling with a PEM public key results in error +--EXTENSIONS-- +openssl +--FILE-- + +--EXPECT-- +bool(false) diff --git a/ext/openssl/tests/openssl_error_string_basic_openssl3.phpt b/ext/openssl/tests/openssl_error_string_basic_openssl3.phpt index d435a53e3047f..041a0a0b5648a 100644 --- a/ext/openssl/tests/openssl_error_string_basic_openssl3.phpt +++ b/ext/openssl/tests/openssl_error_string_basic_openssl3.phpt @@ -114,9 +114,6 @@ expect_openssl_errors('openssl_pkey_export_to_file write', ['10080002']); // successful export @openssl_pkey_export($private_key_file_with_pass, $out, 'wrong pwd', $options); expect_openssl_errors('openssl_pkey_export', ['1C800064', '04800065']); -// invalid x509 for getting public key -@openssl_pkey_get_public($private_key_file); -expect_openssl_errors('openssl_pkey_get_public', [$err_pem_no_start_line]); // private encrypt with unknown padding @openssl_private_encrypt("data", $crypted, $private_key_file, 1000); expect_openssl_errors('openssl_private_encrypt', ['1C8000A5']); @@ -126,7 +123,7 @@ expect_openssl_errors('openssl_private_decrypt', ['0200009F', '02000072']); // public encrypt and decrypt with failed padding check and padding @openssl_public_encrypt("data", $crypted, $public_key_file, 1000); @openssl_public_decrypt("data", $crypted, $public_key_file); -expect_openssl_errors('openssl_private_(en|de)crypt padding', [$err_pem_no_start_line, '02000076', '0200008A', '02000072', '1C880004']); +expect_openssl_errors('openssl_private_(en|de)crypt padding', ['02000076', '0200008A', '02000072', '1C880004']); // X509 echo "X509 errors\n"; @@ -170,7 +167,6 @@ openssl_pkey_export_to_file opening: ok openssl_pkey_export_to_file pem: ok openssl_pkey_export_to_file write: ok openssl_pkey_export: ok -openssl_pkey_get_public: ok openssl_private_encrypt: ok openssl_private_decrypt: ok openssl_private_(en|de)crypt padding: ok From a3e19bfd3f8bc6444be4c7bcaca8959c72dc6f17 Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 10 Apr 2023 22:28:38 +0200 Subject: [PATCH 2/6] Fix openSSL < 3.0 --- ext/openssl/tests/openssl_error_string_basic.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/openssl/tests/openssl_error_string_basic.phpt b/ext/openssl/tests/openssl_error_string_basic.phpt index e4ea264b3bf1f..02e8b3fbc49d1 100644 --- a/ext/openssl/tests/openssl_error_string_basic.phpt +++ b/ext/openssl/tests/openssl_error_string_basic.phpt @@ -123,7 +123,7 @@ expect_openssl_errors('openssl_private_decrypt', ['04065072']); // public encrypt and decrypt with failed padding check and padding @openssl_public_encrypt("data", $crypted, $public_key_file, 1000); @openssl_public_decrypt("data", $crypted, $public_key_file); -expect_openssl_errors('openssl_private_(en|de)crypt padding', [$err_pem_no_start_line, '0408F090', '04067072']); +expect_openssl_errors('openssl_private_(en|de)crypt padding', ['0408F090', '04067072']); // X509 echo "X509 errors\n"; From 2928dd0fb41777b8d10c8a94c9865d9a94b128ad Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 17 Apr 2023 17:38:41 +0200 Subject: [PATCH 3/6] Directly use variable Co-authored-by: Jakub Zelenka --- ext/openssl/openssl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index de25a1de5c2dc..6df16c407d15c 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -509,8 +509,7 @@ void php_openssl_errors_restore_mark(void) { errors->top = 0; errors->bottom = 0; } else { - errors_mark = OPENSSL_G(errors_mark); - memcpy(errors, errors_mark, sizeof(struct php_openssl_errors)); + memcpy(errors, OPENSSL_G(errors_mark), sizeof(struct php_openssl_errors)); } } /* }}} */ From 74ebf075d861b891f75f78c5edd0eb2d1efd12b3 Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 17 Apr 2023 17:38:57 +0200 Subject: [PATCH 4/6] Avoid using extra variable Co-authored-by: Jakub Zelenka --- ext/openssl/openssl.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 6df16c407d15c..d10a8a145f5eb 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -487,10 +487,7 @@ void php_openssl_errors_set_mark(void) { OPENSSL_G(errors_mark) = pecalloc(1, sizeof(struct php_openssl_errors), 1); } - errors = OPENSSL_G(errors); - errors_mark = OPENSSL_G(errors_mark); - - memcpy(errors_mark, errors, sizeof(struct php_openssl_errors)); + memcpy(OPENSSL_G(errors_mark), OPENSSL_G(errors), sizeof(struct php_openssl_errors)); } /* }}} */ From 841e157241d72b76c686e1f7dec2d60018885c1f Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 17 Apr 2023 17:45:25 +0200 Subject: [PATCH 5/6] Add errors_mark to GINIT and GSHUTDOWN --- ext/openssl/openssl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index d10a8a145f5eb..cc9af7d66502e 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -1452,6 +1452,7 @@ PHP_GINIT_FUNCTION(openssl) ZEND_TSRMLS_CACHE_UPDATE(); #endif openssl_globals->errors = NULL; + openssl_globals->errors_mark = NULL; } /* }}} */ @@ -1461,6 +1462,9 @@ PHP_GSHUTDOWN_FUNCTION(openssl) if (openssl_globals->errors) { pefree(openssl_globals->errors, 1); } + if (openssl_globals->errors_mark) { + pefree(openssl_globals->errors_mark, 1); + } } /* }}} */ From f77b90566490f731505e8c4190c98be135f0f295 Mon Sep 17 00:00:00 2001 From: Florian Moser Date: Mon, 17 Apr 2023 17:45:46 +0200 Subject: [PATCH 6/6] Shorten & simplify syntax --- ext/openssl/openssl.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index cc9af7d66502e..83501f5e907f6 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -476,9 +476,6 @@ void php_openssl_store_errors() /* {{{ php_openssl_errors_set_mark */ void php_openssl_errors_set_mark(void) { - struct php_openssl_errors *errors; - struct php_openssl_errors *errors_mark; - if (!OPENSSL_G(errors)) { return; } @@ -493,14 +490,11 @@ void php_openssl_errors_set_mark(void) { /* {{{ php_openssl_errors_restore_mark */ void php_openssl_errors_restore_mark(void) { - struct php_openssl_errors *errors; - struct php_openssl_errors *errors_mark; - if (!OPENSSL_G(errors)) { return; } - errors = OPENSSL_G(errors); + struct php_openssl_errors *errors = OPENSSL_G(errors); if (!OPENSSL_G(errors_mark)) { errors->top = 0;