https://bugs.gentoo.org/930752 https://bugs.gentoo.org/930529 https://gitlab.com/gnutls/gnutls/-/issues/1540 https://gitlab.com/gnutls/gnutls/-/merge_requests/1830 https://gitlab.com/gnutls/gnutls/-/commit/2d73d945c4b1dfcf8d2328c4d23187d62ffaab2d From 2d73d945c4b1dfcf8d2328c4d23187d62ffaab2d Mon Sep 17 00:00:00 2001 From: Zoltan Fridrich Date: Wed, 10 Apr 2024 12:51:33 +0200 Subject: [PATCH] Fix RSAES-PKCS1-v1_5 system-wide configuration Signed-off-by: Zoltan Fridrich --- a/lib/priority.c +++ b/lib/priority.c @@ -1018,6 +1018,12 @@ struct cfg { bool force_ext_master_secret_set; }; +static inline void cfg_init(struct cfg *cfg) +{ + memset(cfg, 0, sizeof(*cfg)); + cfg->allow_rsa_pkcs1_encrypt = true; +} + static inline void cfg_deinit(struct cfg *cfg) { if (cfg->priority_strings) { @@ -1095,6 +1101,12 @@ struct ini_ctx { size_t curves_size; }; +static inline void ini_ctx_init(struct ini_ctx *ctx) +{ + memset(ctx, 0, sizeof(*ctx)); + cfg_init(&ctx->cfg); +} + static inline void ini_ctx_deinit(struct ini_ctx *ctx) { cfg_deinit(&ctx->cfg); @@ -1423,9 +1435,6 @@ static inline int cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) _gnutls_default_priority_string = cfg->default_priority_string; } - /* enable RSA-PKCS1-V1_5 by default */ - cfg->allow_rsa_pkcs1_encrypt = true; - if (cfg->allowlisting) { /* also updates `flags` of global `hash_algorithms[]` */ ret = cfg_hashes_set_array(cfg, ctx->hashes, ctx->hashes_size); @@ -2217,22 +2226,73 @@ update_system_wide_priority_string(void) return 0; } +/* Returns false on parse error, otherwise true. + * The system_wide_config must be locked for writing. + */ +static inline bool load_system_priority_file(void) +{ + int err; + FILE *fp; + struct ini_ctx ctx; + + cfg_init(&system_wide_config); + + fp = fopen(system_priority_file, "re"); + if (fp == NULL) { + _gnutls_debug_log("cfg: unable to open: %s: %d\n", + system_priority_file, errno); + return true; + } + + /* Parsing the configuration file needs to be done in 2 phases: + * first parsing the [global] section + * and then the other sections, + * because the [global] section modifies the parsing behavior. + */ + ini_ctx_init(&ctx); + err = ini_parse_file(fp, global_ini_handler, &ctx); + if (!err) { + if (fseek(fp, 0L, SEEK_SET) < 0) { + _gnutls_debug_log("cfg: unable to rewind: %s\n", + system_priority_file); + if (fail_on_invalid_config) + exit(1); + } + err = ini_parse_file(fp, cfg_ini_handler, &ctx); + } + fclose(fp); + if (err) { + ini_ctx_deinit(&ctx); + _gnutls_debug_log("cfg: unable to parse: %s: %d\n", + system_priority_file, err); + return false; + } + cfg_apply(&system_wide_config, &ctx); + ini_ctx_deinit(&ctx); + return true; +} + static int _gnutls_update_system_priorities(bool defer_system_wide) { - int ret, err = 0; + int ret; + bool config_parse_error = false; struct stat sb; - FILE *fp; gnutls_buffer_st buf; - struct ini_ctx ctx; ret = gnutls_rwlock_rdlock(&system_wide_config_rwlock); - if (ret < 0) { + if (ret < 0) return gnutls_assert_val(ret); - } if (stat(system_priority_file, &sb) < 0) { _gnutls_debug_log("cfg: unable to access: %s: %d\n", system_priority_file, errno); + + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + ret = gnutls_rwlock_wrlock(&system_wide_config_rwlock); + if (ret < 0) + goto out; + /* If system-wide config is unavailable, apply the defaults */ + cfg_init(&system_wide_config); goto out; } @@ -2240,63 +2300,27 @@ static int _gnutls_update_system_priorities(bool defer_system_wide) system_priority_last_mod == sb.st_mtime) { _gnutls_debug_log("cfg: system priority %s has not changed\n", system_priority_file); - if (system_wide_config.priority_string) { + if (system_wide_config.priority_string) goto out; /* nothing to do */ - } } (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); ret = gnutls_rwlock_wrlock(&system_wide_config_rwlock); - if (ret < 0) { + if (ret < 0) return gnutls_assert_val(ret); - } /* Another thread could have successfully re-read system-wide config, * skip re-reading if the mtime it has used is exactly the same. */ - if (system_priority_file_loaded) { + if (system_priority_file_loaded) system_priority_file_loaded = (system_priority_last_mod == sb.st_mtime); - } if (!system_priority_file_loaded) { - _name_val_array_clear(&system_wide_config.priority_strings); - - gnutls_free(system_wide_config.priority_string); - system_wide_config.priority_string = NULL; - - fp = fopen(system_priority_file, "re"); - if (fp == NULL) { - _gnutls_debug_log("cfg: unable to open: %s: %d\n", - system_priority_file, errno); + config_parse_error = !load_system_priority_file(); + if (config_parse_error) goto out; - } - /* Parsing the configuration file needs to be done in 2 phases: - * first parsing the [global] section - * and then the other sections, - * because the [global] section modifies the parsing behavior. - */ - memset(&ctx, 0, sizeof(ctx)); - err = ini_parse_file(fp, global_ini_handler, &ctx); - if (!err) { - if (fseek(fp, 0L, SEEK_SET) < 0) { - _gnutls_debug_log("cfg: unable to rewind: %s\n", - system_priority_file); - if (fail_on_invalid_config) - exit(1); - } - err = ini_parse_file(fp, cfg_ini_handler, &ctx); - } - fclose(fp); - if (err) { - ini_ctx_deinit(&ctx); - _gnutls_debug_log("cfg: unable to parse: %s: %d\n", - system_priority_file, err); - goto out; - } - cfg_apply(&system_wide_config, &ctx); - ini_ctx_deinit(&ctx); _gnutls_debug_log("cfg: loaded system config %s mtime %lld\n", system_priority_file, (unsigned long long)sb.st_mtime); @@ -2332,9 +2356,8 @@ static int _gnutls_update_system_priorities(bool defer_system_wide) out: (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); - if (err && fail_on_invalid_config) { + if (config_parse_error && fail_on_invalid_config) exit(1); - } return ret; } --- a/tests/system-override-allow-rsa-pkcs1-encrypt.sh +++ b/tests/system-override-allow-rsa-pkcs1-encrypt.sh @@ -19,9 +19,8 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see -: ${srcdir=.} -TEST=${srcdir}/rsaes-pkcs1-v1_5 -CONF=${srcdir}/config.$$.tmp +TEST=${builddir}/rsaes-pkcs1-v1_5 +CONF=config.$$.tmp export GNUTLS_SYSTEM_PRIORITY_FILE=${CONF} export GNUTLS_SYSTEM_PRIORITY_FAIL_ON_INVALID=1 @@ -38,15 +37,33 @@ cat <<_EOF_ > ${CONF} allow-rsa-pkcs1-encrypt = true _EOF_ -${TEST} && fail "RSAES-PKCS1-v1_5 expected to succeed" +${TEST} +if [ $? != 0 ]; then + echo "${TEST} expected to succeed" + exit 1 +fi +echo "RSAES-PKCS1-v1_5 successfully enabled" cat <<_EOF_ > ${CONF} [overrides] allow-rsa-pkcs1-encrypt = false _EOF_ -${TEST} || fail "RSAES-PKCS1-v1_5 expected to fail" +${TEST} +if [ $? = 0 ]; then + echo "${TEST} expected to fail" + exit 1 +fi +echo "RSAES-PKCS1-v1_5 successfully disabled" unset GNUTLS_SYSTEM_PRIORITY_FILE unset GNUTLS_SYSTEM_PRIORITY_FAIL_ON_INVALID + +${TEST} +if [ $? != 0 ]; then + echo "${TEST} expected to succeed by default" + exit 1 +fi +echo "RSAES-PKCS1-v1_5 successfully enabled by default" + exit 0 -- GitLab