From 1ee83d848a54f6905653ba06a75e4a47a6e29aef Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 15 Feb 2021 15:54:49 +0100 Subject: [PATCH 1/3] Suppress OpenSSL error on optional conf --- ext/openssl/openssl.c | 50 ++++++++++++++++----------------- ext/openssl/tests/bug80747.phpt | 6 +++- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 66f18516a4625..d5c75984eb971 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -1064,6 +1064,17 @@ static inline int php_openssl_config_check_syntax(const char * section_label, co } /* }}} */ +static char *conf_get_string_quiet( + LHASH_OF(CONF_VALUE) *conf, const char *group, const char *name) { + char *str = CONF_get_string(conf, group, name); + if (!str) { + /* OpenSSL reports an error if a configuration value is not found. + * However, we don't want to generate errors for optional configuration. */ + ERR_clear_error(); + } + return str; +} + static int php_openssl_add_oid_section(struct php_x509_request * req) /* {{{ */ { char * str; @@ -1071,9 +1082,8 @@ static int php_openssl_add_oid_section(struct php_x509_request * req) /* {{{ */ CONF_VALUE * cnf; int i; - str = CONF_get_string(req->req_config, NULL, "oid_section"); + str = conf_get_string_quiet(req->req_config, NULL, "oid_section"); if (str == NULL) { - php_openssl_store_errors(); return SUCCESS; } sktmp = CONF_get_section(req->req_config, str); @@ -1158,10 +1168,8 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option } /* read in the oids */ - str = CONF_get_string(req->req_config, NULL, "oid_file"); - if (str == NULL) { - php_openssl_store_errors(); - } else if (!php_openssl_open_base_dir_chk(str)) { + str = conf_get_string_quiet(req->req_config, NULL, "oid_file"); + if (str && !php_openssl_open_base_dir_chk(str)) { BIO *oid_bio = BIO_new_file(str, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY)); if (oid_bio) { OBJ_create_objects(oid_bio); @@ -1173,11 +1181,11 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option return FAILURE; } SET_OPTIONAL_STRING_ARG("digest_alg", req->digest_name, - CONF_get_string(req->req_config, req->section_name, "default_md")); + conf_get_string_quiet(req->req_config, req->section_name, "default_md")); SET_OPTIONAL_STRING_ARG("x509_extensions", req->extensions_section, - CONF_get_string(req->req_config, req->section_name, "x509_extensions")); + conf_get_string_quiet(req->req_config, req->section_name, "x509_extensions")); SET_OPTIONAL_STRING_ARG("req_extensions", req->request_extensions_section, - CONF_get_string(req->req_config, req->section_name, "req_extensions")); + conf_get_string_quiet(req->req_config, req->section_name, "req_extensions")); SET_OPTIONAL_LONG_ARG("private_key_bits", req->priv_key_bits, CONF_get_number(req->req_config, req->section_name, "default_bits")); @@ -1186,11 +1194,9 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option if (optional_args && (item = zend_hash_str_find(Z_ARRVAL_P(optional_args), "encrypt_key", sizeof("encrypt_key")-1)) != NULL) { req->priv_key_encrypt = Z_TYPE_P(item) == IS_TRUE ? 1 : 0; } else { - str = CONF_get_string(req->req_config, req->section_name, "encrypt_rsa_key"); + str = conf_get_string_quiet(req->req_config, req->section_name, "encrypt_rsa_key"); if (str == NULL) { - str = CONF_get_string(req->req_config, req->section_name, "encrypt_key"); - /* it is sure that there are some errrors as str was NULL for encrypt_rsa_key */ - php_openssl_store_errors(); + str = conf_get_string_quiet(req->req_config, req->section_name, "encrypt_key"); } if (str != NULL && strcmp(str, "no") == 0) { req->priv_key_encrypt = 0; @@ -1218,12 +1224,10 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option /* digest alg */ if (req->digest_name == NULL) { - req->digest_name = CONF_get_string(req->req_config, req->section_name, "default_md"); + req->digest_name = conf_get_string_quiet(req->req_config, req->section_name, "default_md"); } if (req->digest_name != NULL) { req->digest = req->md_alg = EVP_get_digestbyname(req->digest_name); - } else { - php_openssl_store_errors(); } if (req->md_alg == NULL) { req->md_alg = req->digest = EVP_sha1(); @@ -1245,10 +1249,8 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option #endif /* set the string mask */ - str = CONF_get_string(req->req_config, req->section_name, "string_mask"); - if (str == NULL) { - php_openssl_store_errors(); - } else if (!ASN1_STRING_set_default_mask_asc(str)) { + str = conf_get_string_quiet(req->req_config, req->section_name, "string_mask"); + if (str && !ASN1_STRING_set_default_mask_asc(str)) { php_error_docref(NULL, E_WARNING, "Invalid global string mask setting %s", str); return FAILURE; } @@ -3138,9 +3140,8 @@ static int php_openssl_make_REQ(struct php_x509_request * req, X509_REQ * csr, z php_openssl_store_errors(); return FAILURE; } - attr_sect = CONF_get_string(req->req_config, req->section_name, "attributes"); + attr_sect = conf_get_string_quiet(req->req_config, req->section_name, "attributes"); if (attr_sect == NULL) { - php_openssl_store_errors(); attr_sk = NULL; } else { attr_sk = CONF_get_section(req->req_config, attr_sect); @@ -3994,10 +3995,7 @@ static EVP_PKEY * php_openssl_generate_private_key(struct php_x509_request * req return NULL; } - randfile = CONF_get_string(req->req_config, req->section_name, "RANDFILE"); - if (randfile == NULL) { - php_openssl_store_errors(); - } + randfile = conf_get_string_quiet(req->req_config, req->section_name, "RANDFILE"); php_openssl_load_rand_file(randfile, &egdsocket, &seeded); if ((req->priv_key = EVP_PKEY_new()) != NULL) { diff --git a/ext/openssl/tests/bug80747.phpt b/ext/openssl/tests/bug80747.phpt index db83d0266f2e6..b9bba3687008c 100644 --- a/ext/openssl/tests/bug80747.phpt +++ b/ext/openssl/tests/bug80747.phpt @@ -13,7 +13,11 @@ $conf = array( 'private_key_bits' => 511, ); var_dump(openssl_pkey_new($conf)); +while ($e = openssl_error_string()) { + echo $e, "\n"; +} ?> ---EXPECT-- +--EXPECTF-- bool(false) +error:%s:key size too small From e054eb299c322022a17e8f53043ddca47c95cf4c Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 15 Feb 2021 16:26:23 +0100 Subject: [PATCH 2/3] Rename function, add explicit NULL check --- ext/openssl/openssl.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index d5c75984eb971..dc81fcf04e8bd 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -1064,10 +1064,10 @@ static inline int php_openssl_config_check_syntax(const char * section_label, co } /* }}} */ -static char *conf_get_string_quiet( +static char *php_openssl_conf_get_string( LHASH_OF(CONF_VALUE) *conf, const char *group, const char *name) { char *str = CONF_get_string(conf, group, name); - if (!str) { + if (str == NULL) { /* OpenSSL reports an error if a configuration value is not found. * However, we don't want to generate errors for optional configuration. */ ERR_clear_error(); @@ -1082,7 +1082,7 @@ static int php_openssl_add_oid_section(struct php_x509_request * req) /* {{{ */ CONF_VALUE * cnf; int i; - str = conf_get_string_quiet(req->req_config, NULL, "oid_section"); + str = php_openssl_conf_get_string(req->req_config, NULL, "oid_section"); if (str == NULL) { return SUCCESS; } @@ -1168,7 +1168,7 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option } /* read in the oids */ - str = conf_get_string_quiet(req->req_config, NULL, "oid_file"); + str = php_openssl_conf_get_string(req->req_config, NULL, "oid_file"); if (str && !php_openssl_open_base_dir_chk(str)) { BIO *oid_bio = BIO_new_file(str, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY)); if (oid_bio) { @@ -1181,11 +1181,11 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option return FAILURE; } SET_OPTIONAL_STRING_ARG("digest_alg", req->digest_name, - conf_get_string_quiet(req->req_config, req->section_name, "default_md")); + php_openssl_conf_get_string(req->req_config, req->section_name, "default_md")); SET_OPTIONAL_STRING_ARG("x509_extensions", req->extensions_section, - conf_get_string_quiet(req->req_config, req->section_name, "x509_extensions")); + php_openssl_conf_get_string(req->req_config, req->section_name, "x509_extensions")); SET_OPTIONAL_STRING_ARG("req_extensions", req->request_extensions_section, - conf_get_string_quiet(req->req_config, req->section_name, "req_extensions")); + php_openssl_conf_get_string(req->req_config, req->section_name, "req_extensions")); SET_OPTIONAL_LONG_ARG("private_key_bits", req->priv_key_bits, CONF_get_number(req->req_config, req->section_name, "default_bits")); @@ -1194,9 +1194,9 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option if (optional_args && (item = zend_hash_str_find(Z_ARRVAL_P(optional_args), "encrypt_key", sizeof("encrypt_key")-1)) != NULL) { req->priv_key_encrypt = Z_TYPE_P(item) == IS_TRUE ? 1 : 0; } else { - str = conf_get_string_quiet(req->req_config, req->section_name, "encrypt_rsa_key"); + str = php_openssl_conf_get_string(req->req_config, req->section_name, "encrypt_rsa_key"); if (str == NULL) { - str = conf_get_string_quiet(req->req_config, req->section_name, "encrypt_key"); + str = php_openssl_conf_get_string(req->req_config, req->section_name, "encrypt_key"); } if (str != NULL && strcmp(str, "no") == 0) { req->priv_key_encrypt = 0; @@ -1224,7 +1224,7 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option /* digest alg */ if (req->digest_name == NULL) { - req->digest_name = conf_get_string_quiet(req->req_config, req->section_name, "default_md"); + req->digest_name = php_openssl_conf_get_string(req->req_config, req->section_name, "default_md"); } if (req->digest_name != NULL) { req->digest = req->md_alg = EVP_get_digestbyname(req->digest_name); @@ -1249,7 +1249,7 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option #endif /* set the string mask */ - str = conf_get_string_quiet(req->req_config, req->section_name, "string_mask"); + str = php_openssl_conf_get_string(req->req_config, req->section_name, "string_mask"); if (str && !ASN1_STRING_set_default_mask_asc(str)) { php_error_docref(NULL, E_WARNING, "Invalid global string mask setting %s", str); return FAILURE; @@ -3140,7 +3140,7 @@ static int php_openssl_make_REQ(struct php_x509_request * req, X509_REQ * csr, z php_openssl_store_errors(); return FAILURE; } - attr_sect = conf_get_string_quiet(req->req_config, req->section_name, "attributes"); + attr_sect = php_openssl_conf_get_string(req->req_config, req->section_name, "attributes"); if (attr_sect == NULL) { attr_sk = NULL; } else { @@ -3995,7 +3995,7 @@ static EVP_PKEY * php_openssl_generate_private_key(struct php_x509_request * req return NULL; } - randfile = conf_get_string_quiet(req->req_config, req->section_name, "RANDFILE"); + randfile = php_openssl_conf_get_string(req->req_config, req->section_name, "RANDFILE"); php_openssl_load_rand_file(randfile, &egdsocket, &seeded); if ((req->priv_key = EVP_PKEY_new()) != NULL) { From cb5f9e92f2f8eb1db832281443b6d34bb8101d19 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 15 Feb 2021 16:36:45 +0100 Subject: [PATCH 3/3] Add moar NULL checks --- ext/openssl/openssl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index dc81fcf04e8bd..b3ca042306eee 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -1169,7 +1169,7 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option /* read in the oids */ str = php_openssl_conf_get_string(req->req_config, NULL, "oid_file"); - if (str && !php_openssl_open_base_dir_chk(str)) { + if (str != NULL && !php_openssl_open_base_dir_chk(str)) { BIO *oid_bio = BIO_new_file(str, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY)); if (oid_bio) { OBJ_create_objects(oid_bio); @@ -1250,7 +1250,7 @@ static int php_openssl_parse_config(struct php_x509_request * req, zval * option /* set the string mask */ str = php_openssl_conf_get_string(req->req_config, req->section_name, "string_mask"); - if (str && !ASN1_STRING_set_default_mask_asc(str)) { + if (str != NULL && !ASN1_STRING_set_default_mask_asc(str)) { php_error_docref(NULL, E_WARNING, "Invalid global string mask setting %s", str); return FAILURE; }