]> granicus.if.org Git - php/commitdiff
Require $method parameter in openssl_seal/openssl_open
authorNikita Popov <nikita.ppv@gmail.com>
Tue, 8 Sep 2020 09:06:49 +0000 (11:06 +0200)
committerNikita Popov <nikita.ppv@gmail.com>
Tue, 8 Sep 2020 12:21:01 +0000 (14:21 +0200)
RC4 is considered insecure, and it's not possible to change the
default of these functions. As such, require the method to be
passed explicitly.

Closes GH-6093.

UPGRADING
ext/openssl/openssl.c
ext/openssl/openssl.stub.php
ext/openssl/openssl_arginfo.h
ext/openssl/tests/bug70395.phpt [deleted file]
ext/openssl/tests/bug71475.phpt
ext/openssl/tests/bug75307.phpt [deleted file]
ext/openssl/tests/openssl_open_basic.phpt
ext/openssl/tests/openssl_seal_basic.phpt

index 0621d8eadee4115837ff4d3cb6e8c7895fbc31ec..e38f0307c0e8bc75e508fa2c56f02ef0da8bc36f 100644 (file)
--- 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
index 0bcf034f0242e372f8df7a49c17bf3e542a126ae..cd4eeaa2de7f2fb0868307ace9fd2a50eb47f7c8 100644 (file)
@@ -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);
index c4a9d2b2b19ed6543a427c580ed355a390c28726..36fdcf42af24c203d8f09cd2ecf1c8b3252bc28b 100644 (file)
@@ -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 {}
 
index 1800a8f1284ceb33b7931f306df5137c5af4969e..dd877268228cd723867c1582e2d289ce73325d06 100644 (file)
@@ -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 (file)
index bfa881a..0000000
+++ /dev/null
@@ -1,19 +0,0 @@
---TEST--
-Bug #70395 (Missing ARG_INFO for openssl_seal())
---SKIPIF--
-<?php
-if (!extension_loaded("openssl")) die("skip openssl not loaded");
-?>
---FILE--
-<?php
-$func = new ReflectionFunction('openssl_seal');
-$param = $func->getParameters()[4];
-var_dump($param);
-var_dump($param->isOptional());
-?>
---EXPECTF--
-object(ReflectionParameter)#%d (1) {
-  ["name"]=>
-  string(6) "method"
-}
-bool(true)
index 6d4ca665806ee0f3f49e80573f18ec2c0aace967..84f2343ba0dce9228bdbb22eb6b5a7fa2b2eebf7 100644 (file)
@@ -7,9 +7,13 @@ if (!extension_loaded("openssl")) die("skip openssl not loaded");
 --FILE--
 <?php
 $_ = str_repeat("A", 512);
-openssl_seal($_, $_, $_, array_fill(0,64,0));
+try {
+    openssl_seal($_, $_, $_, array_fill(0,64,0));
+} catch (TypeError $e) {
+    echo $e->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 (file)
index 0b1f4ac..0000000
+++ /dev/null
@@ -1,15 +0,0 @@
---TEST--
-Bug #75307 Wrong reflection for openssl_open function
---SKIPIF--
-<?php
-if (!extension_loaded("openssl")) die("skip openssl not available");
-?>
---FILE--
-<?php
-$rf = new ReflectionFunction('openssl_open');
-var_dump($rf->getNumberOfParameters());
-var_dump($rf->getNumberOfRequiredParameters());
-?>
---EXPECT--
-int(6)
-int(4)
index fc410228183909e1d31a1c4761f85e0566cf9912..5e551c507f1479360fc59354452388939800a4b8 100644 (file)
@@ -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--
index 0a49cea566acfaed25efbd56c20a192fd3caff8f..bdbbd01208aa6e36b07df69a988c15e9b474decb 100644 (file)
@@ -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--