From 85bd696d7d10763bd7d87fa467fc585c351560eb Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 8 Sep 2020 11:06:49 +0200 Subject: [PATCH] Require $method parameter in openssl_seal/openssl_open --- UPGRADING | 2 ++ ext/openssl/openssl.c | 28 ++++++++--------------- ext/openssl/openssl.stub.php | 4 ++-- ext/openssl/openssl_arginfo.h | 6 ++--- ext/openssl/tests/bug70395.phpt | 19 --------------- ext/openssl/tests/bug71475.phpt | 10 +++++--- ext/openssl/tests/bug75307.phpt | 15 ------------ ext/openssl/tests/openssl_open_basic.phpt | 11 +++++---- ext/openssl/tests/openssl_seal_basic.phpt | 15 ++++++------ 9 files changed, 38 insertions(+), 72 deletions(-) delete mode 100644 ext/openssl/tests/bug70395.phpt delete mode 100644 ext/openssl/tests/bug75307.phpt diff --git a/UPGRADING b/UPGRADING index 0621d8eadee4..e38f0307c0e8 100644 --- a/UPGRADING +++ b/UPGRADING @@ -403,6 +403,8 @@ PHP 8.0 UPGRADE NOTES . The openssl_pkey_free() function is deprecated and no longer has an effect, instead the OpenSSLAsymmetricKey instance is automatically destroyed if it is no longer referenced. + . openssl_seal() and openssl_open() now require $method to be passed, as the + previous default of "RC4" is considered insecure. - PCRE: . When passing invalid escape sequences they are no longer interpreted as diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 0bcf034f0242..cd4eeaa2de7f 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -6584,7 +6584,7 @@ PHP_FUNCTION(openssl_seal) const EVP_CIPHER *cipher; EVP_CIPHER_CTX *ctx; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "szza|sz", &data, &data_len, + if (zend_parse_parameters(ZEND_NUM_ARGS(), "szzas|z", &data, &data_len, &sealdata, &ekeys, &pubkeys, &method, &method_len, &iv) == FAILURE) { RETURN_THROWS(); } @@ -6598,14 +6598,10 @@ PHP_FUNCTION(openssl_seal) RETURN_THROWS(); } - if (method) { - cipher = EVP_get_cipherbyname(method); - if (!cipher) { - php_error_docref(NULL, E_WARNING, "Unknown signature algorithm"); - RETURN_FALSE; - } - } else { - cipher = EVP_rc4(); + cipher = EVP_get_cipherbyname(method); + if (!cipher) { + php_error_docref(NULL, E_WARNING, "Unknown signature algorithm"); + RETURN_FALSE; } iv_len = EVP_CIPHER_iv_length(cipher); @@ -6715,7 +6711,7 @@ PHP_FUNCTION(openssl_open) size_t method_len = 0, iv_len = 0; const EVP_CIPHER *cipher; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "szsz|ss", &data, &data_len, &opendata, + if (zend_parse_parameters(ZEND_NUM_ARGS(), "szszs|s", &data, &data_len, &opendata, &ekey, &ekey_len, &privkey, &method, &method_len, &iv, &iv_len) == FAILURE) { RETURN_THROWS(); } @@ -6731,14 +6727,10 @@ PHP_FUNCTION(openssl_open) RETURN_FALSE; } - if (method) { - cipher = EVP_get_cipherbyname(method); - if (!cipher) { - php_error_docref(NULL, E_WARNING, "Unknown signature algorithm"); - RETURN_FALSE; - } - } else { - cipher = EVP_rc4(); + cipher = EVP_get_cipherbyname(method); + if (!cipher) { + php_error_docref(NULL, E_WARNING, "Unknown signature algorithm"); + RETURN_FALSE; } cipher_iv_len = EVP_CIPHER_iv_length(cipher); diff --git a/ext/openssl/openssl.stub.php b/ext/openssl/openssl.stub.php index c4a9d2b2b19e..36fdcf42af24 100644 --- a/ext/openssl/openssl.stub.php +++ b/ext/openssl/openssl.stub.php @@ -187,13 +187,13 @@ function openssl_verify(string $data, string $signature, $key, $method = OPENSSL * @param OpenSSLAsymmetricKey|OpenSSLCertificate|array|string $pubkeys * @param string $iv */ -function openssl_seal(string $data, &$sealdata, &$ekeys, array $pubkeys, string $method = UNKNOWN, &$iv = UNKNOWN): int|false {} +function openssl_seal(string $data, &$sealdata, &$ekeys, array $pubkeys, string $method, &$iv = UNKNOWN): int|false {} /** * @param string $opendata * @param OpenSSLAsymmetricKey|OpenSSLCertificate|array|string $privkey */ -function openssl_open(string $data, &$opendata, string $ekey, $privkey, string $method = UNKNOWN, string $iv = UNKNOWN): bool {} +function openssl_open(string $data, &$opendata, string $ekey, $privkey, string $method, string $iv = UNKNOWN): bool {} function openssl_get_md_methods(bool $aliases = false): array {} diff --git a/ext/openssl/openssl_arginfo.h b/ext/openssl/openssl_arginfo.h index 1800a8f1284c..dd877268228c 100644 --- a/ext/openssl/openssl_arginfo.h +++ b/ext/openssl/openssl_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 10a514c9947313694296c6ec9ec6f2fa8e6c850b */ + * Stub hash: 7f1066b832ce307914f641de5ed2c40ec10290ba */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_openssl_x509_export_to_file, 0, 2, _IS_BOOL, 0) ZEND_ARG_OBJ_TYPE_MASK(0, x509, OpenSSLCertificate, MAY_BE_STRING, NULL) @@ -272,7 +272,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_openssl_verify, 0, 3, MAY_BE_LON ZEND_ARG_INFO_WITH_DEFAULT_VALUE(0, method, "OPENSSL_ALGO_SHA1") ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_openssl_seal, 0, 4, MAY_BE_LONG|MAY_BE_FALSE) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_openssl_seal, 0, 5, MAY_BE_LONG|MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0) ZEND_ARG_INFO(1, sealdata) ZEND_ARG_INFO(1, ekeys) @@ -281,7 +281,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_openssl_seal, 0, 4, MAY_BE_LONG| ZEND_ARG_INFO(1, iv) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_openssl_open, 0, 4, _IS_BOOL, 0) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_openssl_open, 0, 5, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0) ZEND_ARG_INFO(1, opendata) ZEND_ARG_TYPE_INFO(0, ekey, IS_STRING, 0) diff --git a/ext/openssl/tests/bug70395.phpt b/ext/openssl/tests/bug70395.phpt deleted file mode 100644 index bfa881a0cd66..000000000000 --- a/ext/openssl/tests/bug70395.phpt +++ /dev/null @@ -1,19 +0,0 @@ ---TEST-- -Bug #70395 (Missing ARG_INFO for openssl_seal()) ---SKIPIF-- - ---FILE-- -getParameters()[4]; -var_dump($param); -var_dump($param->isOptional()); -?> ---EXPECTF-- -object(ReflectionParameter)#%d (1) { - ["name"]=> - string(6) "method" -} -bool(true) diff --git a/ext/openssl/tests/bug71475.phpt b/ext/openssl/tests/bug71475.phpt index 6d4ca665806e..84f2343ba0dc 100644 --- a/ext/openssl/tests/bug71475.phpt +++ b/ext/openssl/tests/bug71475.phpt @@ -7,9 +7,13 @@ if (!extension_loaded("openssl")) die("skip openssl not loaded"); --FILE-- getMessage(), "\n"; +} ?> DONE ---EXPECTF-- -Warning: openssl_seal(): Not a public key (1th member of pubkeys) in %s%ebug71475.php on line %d +--EXPECT-- +openssl_seal() expects at least 5 parameters, 4 given DONE diff --git a/ext/openssl/tests/bug75307.phpt b/ext/openssl/tests/bug75307.phpt deleted file mode 100644 index 0b1f4ac21074..000000000000 --- a/ext/openssl/tests/bug75307.phpt +++ /dev/null @@ -1,15 +0,0 @@ ---TEST-- -Bug #75307 Wrong reflection for openssl_open function ---SKIPIF-- - ---FILE-- -getNumberOfParameters()); -var_dump($rf->getNumberOfRequiredParameters()); -?> ---EXPECT-- -int(6) -int(4) diff --git a/ext/openssl/tests/openssl_open_basic.phpt b/ext/openssl/tests/openssl_open_basic.phpt index fc4102281839..5e551c507f14 100644 --- a/ext/openssl/tests/openssl_open_basic.phpt +++ b/ext/openssl/tests/openssl_open_basic.phpt @@ -8,15 +8,16 @@ $data = "openssl_open() test"; $pub_key = "file://" . __DIR__ . "/public.key"; $priv_key = "file://" . __DIR__ . "/private_rsa_1024.key"; $wrong = "wrong"; +$method = "RC4"; -openssl_seal($data, $sealed, $ekeys, array($pub_key, $pub_key, $pub_key)); -openssl_open($sealed, $output, $ekeys[0], $priv_key); +openssl_seal($data, $sealed, $ekeys, array($pub_key, $pub_key, $pub_key), $method); +openssl_open($sealed, $output, $ekeys[0], $priv_key, $method); var_dump($output); -openssl_open($sealed, $output2, $ekeys[1], $wrong); +openssl_open($sealed, $output2, $ekeys[1], $wrong, $method); var_dump($output2); -openssl_open($sealed, $output3, $ekeys[2], $priv_key); +openssl_open($sealed, $output3, $ekeys[2], $priv_key, $method); var_dump($output3); -openssl_open($sealed, $output4, $wrong, $priv_key); +openssl_open($sealed, $output4, $wrong, $priv_key, $method); var_dump($output4); ?> --EXPECTF-- diff --git a/ext/openssl/tests/openssl_seal_basic.phpt b/ext/openssl/tests/openssl_seal_basic.phpt index 0a49cea566ac..bdbbd01208aa 100644 --- a/ext/openssl/tests/openssl_seal_basic.phpt +++ b/ext/openssl/tests/openssl_seal_basic.phpt @@ -9,11 +9,12 @@ $a = 1; $b = array(1); $c = array(1); $d = array(1); +$method = "RC4"; -var_dump(openssl_seal($a, $b, $c, $d)); +var_dump(openssl_seal($a, $b, $c, $d, $method)); try { - var_dump(openssl_seal($a, $a, $a, array())); + var_dump(openssl_seal($a, $a, $a, array(), $method)); } catch (\ValueError $e) { echo $e->getMessage() . \PHP_EOL; } @@ -23,17 +24,17 @@ $data = "openssl_open() test"; $pub_key = "file://" . __DIR__ . "/public.key"; $wrong = "wrong"; -var_dump(openssl_seal($data, $sealed, $ekeys, array($pub_key))); // no output -var_dump(openssl_seal($data, $sealed, $ekeys, array($pub_key, $pub_key))); // no output -var_dump(openssl_seal($data, $sealed, $ekeys, array($pub_key, $wrong))); +var_dump(openssl_seal($data, $sealed, $ekeys, array($pub_key), $method)); // no output +var_dump(openssl_seal($data, $sealed, $ekeys, array($pub_key, $pub_key), $method)); // no output +var_dump(openssl_seal($data, $sealed, $ekeys, array($pub_key, $wrong), $method)); try { - var_dump(openssl_seal($data, $sealed, $ekeys, array())); + var_dump(openssl_seal($data, $sealed, $ekeys, array(), $method)); } catch (\ValueError $e) { echo $e->getMessage() . \PHP_EOL; } -var_dump(openssl_seal($data, $sealed, $ekeys, array($wrong))); +var_dump(openssl_seal($data, $sealed, $ekeys, array($wrong), $method)); ?> --EXPECTF--