Skip to content

Improve parameter handling in ext/openssl #6025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Improve parameter handling in ext/openssl #6025

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b60a16c
Improve parameter handling in ext/openssl
kocsismate Aug 20, 2020
69366ac
Fix code review
kocsismate Aug 25, 2020
f96620d
Minor fixes
kocsismate Sep 9, 2020
125d39d
Last review fixes
kocsismate Sep 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 54 additions & 58 deletions ext/openssl/openssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ PHP_MINIT_FUNCTION(openssl)
REGISTER_LONG_CONSTANT("X509_PURPOSE_ANY", X509_PURPOSE_ANY, CONST_CS|CONST_PERSISTENT);
#endif

/* signature algorithm constants */
/* digest algorithm constants */
REGISTER_LONG_CONSTANT("OPENSSL_ALGO_SHA1", OPENSSL_ALGO_SHA1, CONST_CS|CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("OPENSSL_ALGO_MD5", OPENSSL_ALGO_MD5, CONST_CS|CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("OPENSSL_ALGO_MD4", OPENSSL_ALGO_MD4, CONST_CS|CONST_PERSISTENT);
Expand Down Expand Up @@ -1563,7 +1563,7 @@ PHP_FUNCTION(openssl_spki_new)
mdtype = php_openssl_get_evp_md_from_algo(algo);

if (!mdtype) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown digest algorithm");
goto cleanup;
}

Expand All @@ -1589,7 +1589,7 @@ PHP_FUNCTION(openssl_spki_new)

if (!NETSCAPE_SPKI_sign(spki, pkey, mdtype)) {
php_openssl_store_errors();
php_error_docref(NULL, E_WARNING, "Unable to sign with specified algorithm");
php_error_docref(NULL, E_WARNING, "Unable to sign with specified digest algorithm");
goto cleanup;
}

Expand Down Expand Up @@ -1845,7 +1845,7 @@ zend_string* php_openssl_x509_fingerprint(X509 *peer, const char *method, zend_b
zend_string *ret;

if (!(mdtype = EVP_get_digestbyname(method))) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown digest algorithm");
return NULL;
} else if (!X509_digest(peer, mdtype, md, &n)) {
php_openssl_store_errors();
Expand Down Expand Up @@ -3753,7 +3753,7 @@ static EVP_PKEY * php_openssl_generate_private_key(struct php_x509_request * req
{
EC_KEY *eckey;
if (req->curve_name == NID_undef) {
php_error_docref(NULL, E_WARNING, "Missing configuration value: 'curve_name' not set");
php_error_docref(NULL, E_WARNING, "Missing configuration value: \"curve_name\" not set");
return NULL;
}
eckey = EC_KEY_new_by_curve_name(req->curve_name);
Expand Down Expand Up @@ -4465,11 +4465,13 @@ PHP_FUNCTION(openssl_pkey_get_private)
size_t passphrase_len = sizeof("")-1;
php_openssl_pkey_object *key_object;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "z|s", &cert, &passphrase, &passphrase_len) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "z|s!", &cert, &passphrase, &passphrase_len) == FAILURE) {
RETURN_THROWS();
}

PHP_OPENSSL_CHECK_SIZE_T_TO_INT(passphrase_len, passphrase, 2);
if (passphrase) {
PHP_OPENSSL_CHECK_SIZE_T_TO_INT(passphrase_len, passphrase, 2);
}

pkey = php_openssl_pkey_from_zval(cert, 0, passphrase, passphrase_len);
if (pkey == NULL) {
Expand Down Expand Up @@ -4805,7 +4807,7 @@ PHP_FUNCTION(openssl_pbkdf2)
}

if (!digest) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown digest algorithm");
RETURN_FALSE;
}

Expand Down Expand Up @@ -4847,7 +4849,7 @@ PHP_FUNCTION(openssl_pkcs7_verify)

RETVAL_LONG(-1);

if (zend_parse_parameters(ZEND_NUM_ARGS(), "pl|pappp", &filename, &filename_len,
if (zend_parse_parameters(ZEND_NUM_ARGS(), "pl|p!a!p!p!p!", &filename, &filename_len,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see $cainfo = [] should work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though looking at other signatures, I see that this parameter is declared as ?array in other cases, so this makes sense for consistency. Though the default is sometimes null and sometimes []...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, so I'll change the default value of all of them to [].

&flags, &signersfilename, &signersfilename_len, &cainfo,
&extracerts, &extracerts_len, &datafilename, &datafilename_len, &p7bfilename, &p7bfilename_len) == FAILURE) {
RETURN_THROWS();
Expand Down Expand Up @@ -6082,7 +6084,7 @@ PHP_FUNCTION(openssl_cms_decrypt)
Z_PARAM_PATH(outfilename, outfilename_len)
Z_PARAM_ZVAL(recipcert)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(recipkey)
Z_PARAM_ZVAL_OR_NULL(recipkey)
Z_PARAM_LONG(encoding)
ZEND_PARSE_PARAMETERS_END();

Expand Down Expand Up @@ -6128,8 +6130,7 @@ PHP_FUNCTION(openssl_cms_decrypt)
cms = SMIME_read_CMS(in, &datain);
break;
default:
php_error_docref(NULL, E_WARNING,
"Unknown OPENSSL encoding");
zend_argument_value_error(5, "must be an OPENSSL_ENCODING_* constant");
goto clean_exit;
}

Expand Down Expand Up @@ -6456,13 +6457,18 @@ PHP_FUNCTION(openssl_sign)
char * data;
size_t data_len;
EVP_MD_CTX *md_ctx;
zval *method = NULL;
zend_long signature_algo = OPENSSL_ALGO_SHA1;
zend_string *method_str = NULL;
zend_long method_long = OPENSSL_ALGO_SHA1;
const EVP_MD *mdtype;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "szz|z", &data, &data_len, &signature, &key, &method) == FAILURE) {
RETURN_THROWS();
}
ZEND_PARSE_PARAMETERS_START(3, 4)
Z_PARAM_STRING(data, data_len)
Z_PARAM_ZVAL(signature)
Z_PARAM_ZVAL(key)
Z_PARAM_OPTIONAL
Z_PARAM_STR_OR_LONG(method_str, method_long)
ZEND_PARSE_PARAMETERS_END();

pkey = php_openssl_pkey_from_zval(key, 0, "", 0);
if (pkey == NULL) {
if (!EG(exception)) {
Expand All @@ -6471,20 +6477,13 @@ PHP_FUNCTION(openssl_sign)
RETURN_FALSE;
}

if (method == NULL || Z_TYPE_P(method) == IS_LONG) {
if (method != NULL) {
signature_algo = Z_LVAL_P(method);
}
mdtype = php_openssl_get_evp_md_from_algo(signature_algo);
} else if (Z_TYPE_P(method) == IS_STRING) {
mdtype = EVP_get_digestbyname(Z_STRVAL_P(method));
if (method_str) {
mdtype = EVP_get_digestbyname(ZSTR_VAL(method_str));
} else {
// TODO Use proper ZPP check.
zend_argument_type_error(4, "must be of type string|int|null, %s given" , zend_zval_type_name(method));
RETURN_THROWS();
mdtype = php_openssl_get_evp_md_from_algo(method_long);
}
if (!mdtype) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown digest algorithm");
RETURN_FALSE;
}

Expand Down Expand Up @@ -6522,29 +6521,26 @@ PHP_FUNCTION(openssl_verify)
size_t data_len;
char * signature;
size_t signature_len;
zval *method = NULL;
zend_long signature_algo = OPENSSL_ALGO_SHA1;
zend_string *method_str = NULL;
zend_long method_long = OPENSSL_ALGO_SHA1;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "ssz|z", &data, &data_len, &signature, &signature_len, &key, &method) == FAILURE) {
RETURN_THROWS();
}
ZEND_PARSE_PARAMETERS_START(3, 4)
Z_PARAM_STRING(data, data_len)
Z_PARAM_STRING(signature, signature_len)
Z_PARAM_ZVAL(key)
Z_PARAM_OPTIONAL
Z_PARAM_STR_OR_LONG(method_str, method_long)
ZEND_PARSE_PARAMETERS_END();

PHP_OPENSSL_CHECK_SIZE_T_TO_UINT(signature_len, signature, 2);

if (method == NULL || Z_TYPE_P(method) == IS_LONG) {
if (method != NULL) {
signature_algo = Z_LVAL_P(method);
}
mdtype = php_openssl_get_evp_md_from_algo(signature_algo);
} else if (Z_TYPE_P(method) == IS_STRING) {
mdtype = EVP_get_digestbyname(Z_STRVAL_P(method));
if (method_str) {
mdtype = EVP_get_digestbyname(ZSTR_VAL(method_str));
} else {
// TODO Use proper ZPP check.
zend_argument_type_error(4, "must be of type string|int|null, %s given" , zend_zval_type_name(method));
RETURN_THROWS();
mdtype = php_openssl_get_evp_md_from_algo(method_long);
}
if (!mdtype) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown digest algorithm");
RETURN_FALSE;
}

Expand Down Expand Up @@ -6579,8 +6575,8 @@ PHP_FUNCTION(openssl_seal)
unsigned char iv_buf[EVP_MAX_IV_LENGTH + 1], *buf = NULL, **eks;
char * data;
size_t data_len;
char *method =NULL;
size_t method_len = 0;
char *method;
size_t method_len;
const EVP_CIPHER *cipher;
EVP_CIPHER_CTX *ctx;

Expand All @@ -6600,13 +6596,13 @@ PHP_FUNCTION(openssl_seal)

cipher = EVP_get_cipherbyname(method);
if (!cipher) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown cipher algorithm");
RETURN_FALSE;
}

iv_len = EVP_CIPHER_iv_length(cipher);
if (!iv && iv_len > 0) {
zend_argument_value_error(6, "must provide an IV for chosen cipher algorithm");
zend_argument_value_error(6, "cannot be null for the chosen cipher algorithm");
RETURN_THROWS();
}

Expand Down Expand Up @@ -6707,11 +6703,11 @@ PHP_FUNCTION(openssl_open)
size_t data_len;
char * ekey;
size_t ekey_len;
char *method = NULL, *iv = NULL;
size_t method_len = 0, iv_len = 0;
char *method, *iv = NULL;
size_t method_len, iv_len = 0;
const EVP_CIPHER *cipher;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "szszs|s", &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 @@ -6729,14 +6725,14 @@ PHP_FUNCTION(openssl_open)

cipher = EVP_get_cipherbyname(method);
if (!cipher) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown cipher algorithm");
RETURN_FALSE;
}

cipher_iv_len = EVP_CIPHER_iv_length(cipher);
if (cipher_iv_len > 0) {
if (!iv) {
zend_argument_value_error(6, "must provide an IV for chosen cipher algorithm");
zend_argument_value_error(6, "cannot be null for the chosen cipher algorithm");
RETURN_THROWS();
}
if ((size_t)cipher_iv_len != iv_len) {
Expand Down Expand Up @@ -6782,7 +6778,7 @@ static void php_openssl_add_method(const OBJ_NAME *name, void *arg) /* {{{ */
}
/* }}} */

/* {{{ Return array of available digest methods */
/* {{{ Return array of available digest algorithms */
PHP_FUNCTION(openssl_get_md_methods)
{
zend_bool aliases = 0;
Expand All @@ -6797,7 +6793,7 @@ PHP_FUNCTION(openssl_get_md_methods)
}
/* }}} */

/* {{{ Return array of available cipher methods */
/* {{{ Return array of available cipher algorithms */
PHP_FUNCTION(openssl_get_cipher_methods)
{
zend_bool aliases = 0;
Expand Down Expand Up @@ -6858,7 +6854,7 @@ PHP_FUNCTION(openssl_digest)
}
mdtype = EVP_get_digestbyname(method);
if (!mdtype) {
php_error_docref(NULL, E_WARNING, "Unknown signature algorithm");
php_error_docref(NULL, E_WARNING, "Unknown digest algorithm");
RETURN_FALSE;
}

Expand Down Expand Up @@ -7012,7 +7008,7 @@ static int php_openssl_cipher_init(const EVP_CIPHER *cipher_type,
}
} else if (!enc && tag && tag_len > 0) {
if (!mode->is_aead) {
php_error_docref(NULL, E_WARNING, "The tag cannot be used because the cipher method does not support AEAD");
php_error_docref(NULL, E_WARNING, "The tag cannot be used because the cipher algorithm does not support AEAD");
} else if (!EVP_CIPHER_CTX_ctrl(cipher_ctx, mode->aead_set_tag_flag, tag_len, (unsigned char *) tag)) {
php_error_docref(NULL, E_WARNING, "Setting tag for AEAD cipher decryption failed");
return FAILURE;
Expand All @@ -7024,7 +7020,7 @@ static int php_openssl_cipher_init(const EVP_CIPHER *cipher_type,
if (key_len > password_len) {
if ((OPENSSL_DONT_ZERO_PAD_KEY & options) && !EVP_CIPHER_CTX_set_key_length(cipher_ctx, password_len)) {
php_openssl_store_errors();
php_error_docref(NULL, E_WARNING, "Key length cannot be set for the cipher method");
php_error_docref(NULL, E_WARNING, "Key length cannot be set for the cipher algorithm");
return FAILURE;
}
key = emalloc(key_len);
Expand Down
Loading