openssl_sign() stub probably should not contain array type in private_key

[piotrekkr]

Description

The following stub code:

php-src/ext/openssl/openssl.stub.php

Lines 602 to 606 in 2501cad

	/**
	* @param string $signature
	* @param OpenSSLAsymmetricKey|OpenSSLCertificate|array|string $private_key
	*/
	function openssl_sign(string $data, &$signature, #[\SensitiveParameter] $private_key, string|int $algorithm = OPENSSL_ALGO_SHA1): bool {}

suggest that $private_key can be array somhow but it is not documented anywhere beside stub.
Also, I'm not fluent in C but there seems to be no array handling in function

php-src/ext/openssl/openssl.c

Lines 7107 to 7174 in 2501cad

	PHP_FUNCTION(openssl_sign)
	{
	zval *key, *signature;
	EVP_PKEY *pkey;
	zend_string *sigbuf = NULL;
	char * data;
	size_t data_len;
	EVP_MD_CTX *md_ctx;
	zend_string *method_str = NULL;
	zend_long method_long = OPENSSL_ALGO_SHA1;
	const EVP_MD *mdtype;
	bool can_default_digest = ZEND_THREEWAY_COMPARE(PHP_OPENSSL_API_VERSION, 0x30000) >= 0;
	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, 3);
	if (pkey == NULL) {
	if (!EG(exception)) {
	php_error_docref(NULL, E_WARNING, "Supplied key param cannot be coerced into a private key");
	}
	RETURN_FALSE;
	}
	if (method_str) {
	mdtype = EVP_get_digestbyname(ZSTR_VAL(method_str));
	} else {
	mdtype = php_openssl_get_evp_md_from_algo(method_long);
	}
	if (!mdtype && (!can_default_digest || method_long != 0)) {
	php_error_docref(NULL, E_WARNING, "Unknown digest algorithm");
	RETURN_FALSE;
	}
	md_ctx = EVP_MD_CTX_create();
	size_t siglen;
	#if PHP_OPENSSL_API_VERSION >= 0x10100
	if (md_ctx != NULL &&
	EVP_DigestSignInit(md_ctx, NULL, mdtype, NULL, pkey) &&
	EVP_DigestSign(md_ctx, NULL, &siglen, (unsigned char*)data, data_len) &&
	(sigbuf = zend_string_alloc(siglen, 0)) != NULL &&
	EVP_DigestSign(md_ctx, (unsigned char*)ZSTR_VAL(sigbuf), &siglen, (unsigned char*)data, data_len)) {
	#else
	if (md_ctx != NULL &&
	EVP_SignInit(md_ctx, mdtype) &&
	EVP_SignUpdate(md_ctx, data, data_len) &&
	(siglen = EVP_PKEY_size(pkey)) &&
	(sigbuf = zend_string_alloc(siglen, 0)) != NULL &&
	EVP_SignFinal(md_ctx, (unsigned char*)ZSTR_VAL(sigbuf), (unsigned int*)&siglen, pkey)) {
	#endif
	ZSTR_VAL(sigbuf)[siglen] = '\0';
	ZSTR_LEN(sigbuf) = siglen;
	ZEND_TRY_ASSIGN_REF_NEW_STR(signature, sigbuf);
	RETVAL_TRUE;
	} else {
	php_openssl_store_errors();
	efree(sigbuf);
	RETVAL_FALSE;
	}
	EVP_MD_CTX_destroy(md_ctx);
	EVP_PKEY_free(pkey);
	}
	/* }}} */

There is a (high) possibility that I'm wrong and stub is correct and just docs are to be updated and array type documented.

Can you check what is correct?

I also added an issue in for the docs. I've got reply that docs is based on stub files from php-src, so better to check first here.

Thank you.

PHP Version

PHP 8.3.11

Operating System

No response

[DanielEScherzer]

Also, I'm not fluent in C but there seems to be no array handling in function

The key parameter is parsed with Z_PARAM_ZVAL(key) which accepts any value. The key is then passed to php_openssl_pkey_from_zval() which does have explicit array handling:

php-src/ext/openssl/openssl.c

Lines 3603 to 3631 in 2501cad

	if (Z_TYPE_P(val) == IS_ARRAY) {
	zval * zphrase;
	/* get passphrase */
	if ((zphrase = zend_hash_index_find(Z_ARRVAL_P(val), 1)) == NULL) {
	zend_value_error("Key array must be of the form array(0 => key, 1 => phrase)");
	return NULL;
	}
	if (Z_TYPE_P(zphrase) == IS_STRING) {
	passphrase = Z_STRVAL_P(zphrase);
	passphrase_len = Z_STRLEN_P(zphrase);
	} else {
	ZVAL_COPY(&tmp, zphrase);
	if (!try_convert_to_string(&tmp)) {
	return NULL;
	}
	passphrase = Z_STRVAL(tmp);
	passphrase_len = Z_STRLEN(tmp);
	}
	/* now set val to be the key param and continue */
	if ((val = zend_hash_index_find(Z_ARRVAL_P(val), 0)) == NULL) {
	zend_value_error("Key array must be of the form array(0 => key, 1 => phrase)");
	TMP_CLEAN;
	}
	}

So I guess the documentation should make it clear that the array is expected to be of the form array(0 => key, 1 => phrase)"

[piotrekkr]

@DanielEScherzer Okay thank you very much for checking this. Will ping them to update docs.

[piotrekkr]

@DanielEScherzer Just one more question. Should key be a string or path to key file or can be both?

[piotrekkr]

I can see some checks for val here

php-src/ext/openssl/openssl.c

Lines 3633 to 3721 in 2501cad

	if (Z_TYPE_P(val) == IS_OBJECT && Z_OBJCE_P(val) == php_openssl_pkey_ce) {
	php_openssl_pkey_object *obj = php_openssl_pkey_from_obj(Z_OBJ_P(val));
	key = obj->pkey;
	bool is_priv = obj->is_private;
	/* check whether it is actually a private key if requested */
	if (!public_key && !is_priv) {
	php_error_docref(NULL, E_WARNING, "Supplied key param is a public key");
	TMP_CLEAN;
	}
	if (public_key && is_priv) {
	php_error_docref(NULL, E_WARNING, "Don't know how to get public key from this private key");
	TMP_CLEAN;
	} else {
	if (Z_TYPE(tmp) == IS_STRING) {
	zval_ptr_dtor_str(&tmp);
	}
	EVP_PKEY_up_ref(key);
	return key;
	}
	} else if (Z_TYPE_P(val) == IS_OBJECT && Z_OBJCE_P(val) == php_openssl_certificate_ce) {
	cert = php_openssl_certificate_from_obj(Z_OBJ_P(val))->x509;
	} else {
	/* force it to be a string and check if it refers to a file */
	/* passing non string values leaks, object uses toString, it returns NULL
	* See bug38255.phpt
	*/
	if (!(Z_TYPE_P(val) == IS_STRING || Z_TYPE_P(val) == IS_OBJECT)) {
	TMP_CLEAN;
	}
	if (!try_convert_to_string(val)) {
	TMP_CLEAN;
	}
	if (Z_STRLEN_P(val) > 7 && memcmp(Z_STRVAL_P(val), "file://", sizeof("file://") - 1) == 0) {
	if (!php_openssl_check_path_str(Z_STR_P(val), file_path, arg_num)) {
	TMP_CLEAN;
	}
	is_file = true;
	}
	/* 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));
	} else {
	in = BIO_new_mem_buf(Z_STRVAL_P(val), (int)Z_STRLEN_P(val));
	}
	if (in == NULL) {
	php_openssl_store_errors();
	TMP_CLEAN;
	}
	key = PEM_read_bio_PUBKEY(in, NULL,NULL, NULL);
	BIO_free(in);
	}
	} else {
	/* we want the private key */
	BIO *in;
	if (is_file) {
	in = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY));
	} else {
	in = BIO_new_mem_buf(Z_STRVAL_P(val), (int)Z_STRLEN_P(val));
	}
	if (in == NULL) {
	TMP_CLEAN;
	}
	if (passphrase == NULL) {
	key = PEM_read_bio_PrivateKey(in, NULL, NULL, NULL);
	} else {
	struct php_openssl_pem_password password;
	password.key = passphrase;
	password.len = passphrase_len;
	key = PEM_read_bio_PrivateKey(in, NULL, php_openssl_pem_password_cb, &password);
	}
	BIO_free(in);
	}
	}

So it means that key in the array can be one of

• OpenSSLAsymmetricKey
• OpenSSLCertificate
• key contents as string
• file:///path/to/key.file
• object that can be cast to string

I it correct? Have I missed something?

[DanielEScherzer]

@DanielEScherzer Just one more question. Should key be a string or path to key file or can be both?

So I think that key can be, based on the implementation at

php-src/ext/openssl/openssl.c

Line 3586 in 2501cad

static EVP_PKEY *php_openssl_pkey_from_zval(

• an OpenSSLAsymmetricKey object (handling on lines 3633-3654)
• an OpenSSLCertificate object (handling on lines 3655-6)
• some string (or other object that can be cast to string) (handling on lines 3662-7) that will
• • be treated as a file path if it starts with file:// (lines 3669-74)
• • if it doesn't, then be treated as a normal string

but to be clear, I don't think I have ever used this extension and even knew about it before seeing this issue, so no promises - maybe test it locally?

Edit: you seem to have posted a similar analysis a few seconds before me, so I guess you came to the same conclusions

[piotrekkr]

Okay thanks will point docs people to this issue maybe they will be able to fully test it. I'll try myself with some simple php code when I have time. Thanks again for help

[damianwadley]

If it helps, I too believe your breakdowns of the different supported types are correct.

Here are all the places using that helper function, and so will support arrays, and thus whose docs need to be checked:

• openssl_x509_check_private_key
• openssl_x509_verify
• openssl_pkcs12_export_to_file
• openssl_pkcs12_export
• openssl_csr_sign
• openssl_csr_new
• openssl_pkey_export_to_file
• openssl_pkey_export
• openssl_pkey_get_public
• openssl_pkey_get_private
• openssl_pkey_derive (both $public_key and $private_key)
• openssl_pkcs7_sign
• openssl_pkcs7_decrypt
• openssl_cms_sign
• openssl_cms_decrypt
• openssl_private_encrypt
• openssl_private_decrypt
• openssl_public_encrypt
• openssl_public_decrypt
• openssl_sign
• openssl_verify
• openssl_seal (where $public_key is an array of keys)
• openssl_open

Related, there's a minor matter with openssl_spki_new: it does use the helper, but the argument handling currently only allows for OpenSSLAsymmetricKey objects - no strings or arrays. I don't see a reason that should have to be the case so I'd think it should be fixed to support the entire zval spectrum...

CC @cmb69

[Girgias]

Related, there's a minor matter with openssl_spki_new: it does use the helper, but the argument handling currently only allows for OpenSSLAsymmetricKey objects - no strings or arrays. I don't see a reason that should have to be the case so I'd think it should be fixed to support the entire zval spectrum...

@bukka usually does OpenSSL stuff. So pinging him to check the above.

[bukka]

openssl_spki_new accepts only OpenSSLAsymmetricKey from a quick look. We could probably change it but why bother with SPKI which no one probably uses anyway... :)

[bukka]

I mean there might be some legacy apps that use that but I highly doubt that anyone would need it for a new code. So it's very low priority as there are just more important things to do in openssl ext.