Skip to content

Commit

Permalink
Require $method parameter in openssl_seal/openssl_open
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nikic committed Sep 8, 2020
1 parent 259af93 commit 3e14942
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 72 deletions.
2 changes: 2 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 10 additions & 18 deletions ext/openssl/openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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);
Expand Down Expand Up @@ -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();
}
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions ext/openssl/openssl.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand Down
6 changes: 3 additions & 3 deletions ext/openssl/openssl_arginfo.h
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
19 changes: 0 additions & 19 deletions ext/openssl/tests/bug70395.phpt

This file was deleted.

10 changes: 7 additions & 3 deletions ext/openssl/tests/bug71475.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 0 additions & 15 deletions ext/openssl/tests/bug75307.phpt

This file was deleted.

11 changes: 6 additions & 5 deletions ext/openssl/tests/openssl_open_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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--
Expand Down
15 changes: 8 additions & 7 deletions ext/openssl/tests/openssl_seal_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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--
Expand Down

0 comments on commit 3e14942

Please sign in to comment.