From 495ad3d82aff125f237f370a70882b97843edb6f Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Mon, 14 Feb 2022 12:44:57 +0100 Subject: [PATCH 1/8] lib/priority: split up update_system_wide_priority_string This is done in preparation for deferring priority string evaluation. Signed-off-by: Alexander Sosedkin --- lib/priority.c | 77 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/lib/priority.c b/lib/priority.c index e7698ba7eb..755729da18 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -1735,110 +1735,127 @@ static int cfg_ini_handler(void *_ctx, const char *section, const char *name, co return 1; } -static int -update_system_wide_priority_string(void) +static int /* not locking system_wide_config */ +construct_system_wide_priority_string(gnutls_buffer_st* buf) { - gnutls_buffer_st buf; int ret; size_t i; - _gnutls_buffer_init(&buf); + _gnutls_buffer_init(buf); - ret = _gnutls_buffer_append_str(&buf, "NONE"); + ret = _gnutls_buffer_append_str(buf, "NONE"); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } for (i = 0; system_wide_config.kxs[i] != 0; i++) { - ret = _gnutls_buffer_append_str(&buf, ":+"); + ret = _gnutls_buffer_append_str(buf, ":+"); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } - ret = _gnutls_buffer_append_str(&buf, + ret = _gnutls_buffer_append_str(buf, gnutls_kx_get_name(system_wide_config.kxs[i])); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } } for (i = 0; system_wide_config.groups[i] != 0; i++) { - ret = _gnutls_buffer_append_str(&buf, ":+GROUP-"); + ret = _gnutls_buffer_append_str(buf, ":+GROUP-"); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } - ret = _gnutls_buffer_append_str(&buf, + ret = _gnutls_buffer_append_str(buf, gnutls_group_get_name(system_wide_config.groups[i])); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } } for (i = 0; system_wide_config.ciphers[i] != 0; i++) { - ret = _gnutls_buffer_append_str(&buf, ":+"); + ret = _gnutls_buffer_append_str(buf, ":+"); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } - ret = _gnutls_buffer_append_str(&buf, + ret = _gnutls_buffer_append_str(buf, gnutls_cipher_get_name(system_wide_config.ciphers[i])); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } } for (i = 0; system_wide_config.macs[i] != 0; i++) { - ret = _gnutls_buffer_append_str(&buf, ":+"); + ret = _gnutls_buffer_append_str(buf, ":+"); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } - ret = _gnutls_buffer_append_str(&buf, + ret = _gnutls_buffer_append_str(buf, gnutls_mac_get_name(system_wide_config.macs[i])); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } } for (i = 0; system_wide_config.sigs[i] != 0; i++) { - ret = _gnutls_buffer_append_str(&buf, ":+SIGN-"); + ret = _gnutls_buffer_append_str(buf, ":+SIGN-"); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } - ret = _gnutls_buffer_append_str(&buf, + ret = _gnutls_buffer_append_str(buf, gnutls_sign_get_name(system_wide_config.sigs[i])); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } } for (i = 0; system_wide_config.versions[i] != 0; i++) { - ret = _gnutls_buffer_append_str(&buf, ":+VERS-"); + ret = _gnutls_buffer_append_str(buf, ":+VERS-"); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } - ret = _gnutls_buffer_append_str(&buf, + ret = _gnutls_buffer_append_str(buf, gnutls_protocol_get_name(system_wide_config.versions[i])); if (ret < 0) { - _gnutls_buffer_clear(&buf); + _gnutls_buffer_clear(buf); return ret; } } + return 0; +} + +static int /* not locking system_wide_config */ +update_system_wide_priority_string(void) +{ + /* doesn't do locking, _gnutls_update_system_priorities does */ + gnutls_buffer_st buf; + int ret; + + ret = construct_system_wide_priority_string(&buf); + if (ret < 0) { + _gnutls_debug_log("cfg: unable to construct " + "system-wide priority string: %s", + gnutls_strerror(ret)); + _gnutls_buffer_clear(&buf); + return ret; + } gnutls_free(system_wide_config.priority_string); system_wide_config.priority_string = gnutls_strdup((char *)buf.data); -- 2.34.1 From a74633975e97e491e1e1cdf12416ce160699b703 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Mon, 14 Feb 2022 13:48:37 +0100 Subject: [PATCH 2/8] lib/priority: defer setting system-wide priority string Signed-off-by: Alexander Sosedkin --- lib/global.c | 2 +- lib/global.h | 2 +- lib/priority.c | 112 ++++++++++++++++++++++++++++--------------------- 3 files changed, 66 insertions(+), 50 deletions(-) diff --git a/lib/global.c b/lib/global.c index 65c0b81709..faa7f0afb2 100644 --- a/lib/global.c +++ b/lib/global.c @@ -365,7 +365,7 @@ static int _gnutls_global_init(unsigned constructor) _gnutls_fips_mode_reset_zombie(); } #endif - _gnutls_load_system_priorities(); + _gnutls_prepare_to_load_system_priorities(); _gnutls_switch_lib_state(LIB_STATE_OPERATIONAL); ret = 0; diff --git a/lib/global.h b/lib/global.h index e30187e7ad..16fde08b5c 100644 --- a/lib/global.h +++ b/lib/global.h @@ -46,7 +46,7 @@ extern void gnutls_crypto_deinit(void); extern void _gnutls_tpm_global_deinit(void); extern void _gnutls_nss_keylog_deinit(void); -extern void _gnutls_load_system_priorities(void); +extern void _gnutls_prepare_to_load_system_priorities(void); extern void _gnutls_unload_system_priorities(void); #endif /* GNUTLS_LIB_GLOBAL_H */ diff --git a/lib/priority.c b/lib/priority.c index 755729da18..4faf96fabf 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -1864,11 +1864,12 @@ update_system_wide_priority_string(void) return 0; } -static int _gnutls_update_system_priorities(void) +static int _gnutls_update_system_priorities(bool defer_system_wide) { int ret, err = 0; struct stat sb; FILE *fp; + gnutls_buffer_st buf; struct ini_ctx ctx; ret = gnutls_rwlock_rdlock(&system_wide_config_rwlock); @@ -1883,10 +1884,12 @@ static int _gnutls_update_system_priorities(void) } if (system_priority_file_loaded && - sb.st_mtime == system_priority_last_mod) { + system_priority_last_mod == sb.st_mtime) { _gnutls_debug_log("cfg: system priority %s has not changed\n", system_priority_file); - goto out; + if (system_wide_config.priority_string) { + goto out; /* nothing to do */ + } } (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); @@ -1896,54 +1899,71 @@ static int _gnutls_update_system_priorities(void) return gnutls_assert_val(ret); } - /* Another thread has successfully updated the system wide config (with - * the same modification time as checked above), while upgrading to - * write lock; no need to reload. + /* 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 && - system_priority_last_mod == sb.st_mtime) { - goto out; + if (system_priority_file_loaded) { + system_priority_file_loaded = + (system_priority_last_mod == sb.st_mtime); } - system_priority_file_loaded = 0; - _name_val_array_clear(&system_wide_config.priority_strings); + 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; + 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); - 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); + fp = fopen(system_priority_file, "re"); + if (fp == NULL) { + _gnutls_debug_log("cfg: unable to open: %s: %d\n", + system_priority_file, errno); + goto out; } - err = ini_parse_file(fp, cfg_ini_handler, &ctx); - } - fclose(fp); - if (err) { + /* 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: unable to parse: %s: %d\n", - system_priority_file, err); - goto out; + _gnutls_debug_log("cfg: loaded system config %s mtime %lld\n", + system_priority_file, + (unsigned long long)sb.st_mtime); + } - cfg_apply(&system_wide_config, &ctx); - ini_ctx_deinit(&ctx); if (system_wide_config.allowlisting) { - ret = update_system_wide_priority_string(); + if (defer_system_wide) { + /* try constructing a priority string, + * but don't apply it yet, at this point + * we're only interested in whether we can */ + ret = construct_system_wide_priority_string(&buf); + _gnutls_buffer_clear(&buf); + _gnutls_debug_log("cfg: deferred setting " + "system-wide priority string\n"); + } else { + ret = update_system_wide_priority_string(); + _gnutls_debug_log("cfg: finalized " + "system-wide priority string\n"); + } if (ret < 0) { _gnutls_debug_log("cfg: unable to build priority string: %s\n", gnutls_strerror(ret)); @@ -1953,10 +1973,6 @@ static int _gnutls_update_system_priorities(void) } } - _gnutls_debug_log("cfg: loaded system priority %s mtime %lld\n", - system_priority_file, - (unsigned long long)sb.st_mtime); - system_priority_file_loaded = 1; system_priority_last_mod = sb.st_mtime; @@ -1970,7 +1986,7 @@ static int _gnutls_update_system_priorities(void) return ret; } -void _gnutls_load_system_priorities(void) +void _gnutls_prepare_to_load_system_priorities(void) { const char *p; int ret; @@ -1983,7 +1999,7 @@ void _gnutls_load_system_priorities(void) if (p != NULL && p[0] == '1' && p[1] == 0) fail_on_invalid_config = 1; - ret = _gnutls_update_system_priorities(); + ret = _gnutls_update_system_priorities(true /* defer_system_wide */); if (ret < 0) { _gnutls_debug_log("failed to update system priorities: %s\n", gnutls_strerror(ret)); @@ -2050,7 +2066,7 @@ char *_gnutls_resolve_priorities(const char* priorities) /* Always try to refresh the cached data, to allow it to be * updated without restarting all applications. */ - ret = _gnutls_update_system_priorities(); + ret = _gnutls_update_system_priorities(false /* defer_system_wide */); if (ret < 0) { _gnutls_debug_log("failed to update system priorities: %s\n", gnutls_strerror(ret)); -- 2.34.1 From 52d5c8627165ff9bc0e99564b772f178ad1f80dd Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Mon, 21 Feb 2022 18:19:25 +0100 Subject: [PATCH 3/8] lib/algorithms: add UB warnings on late allowlisting API invocations Signed-off-by: Alexander Sosedkin --- lib/algorithms/ecc.c | 3 +++ lib/algorithms/mac.c | 3 +++ lib/algorithms/protocols.c | 3 +++ lib/algorithms/sign.c | 6 ++++++ 4 files changed, 15 insertions(+) diff --git a/lib/algorithms/ecc.c b/lib/algorithms/ecc.c index 736e5dd07f..52ae1db0e4 100644 --- a/lib/algorithms/ecc.c +++ b/lib/algorithms/ecc.c @@ -389,6 +389,9 @@ void _gnutls_ecc_curve_mark_disabled_all(void) * enabled through the allowlisting mode in the configuration file, or * when the setting is modified with a prior call to this function. * + * This function must be called prior to any session priority setting functions; + * otherwise the behavior is undefined. + * * Returns: 0 on success or negative error code otherwise. * * Since: 3.7.3 diff --git a/lib/algorithms/mac.c b/lib/algorithms/mac.c index a2c66e76bb..166d51d552 100644 --- a/lib/algorithms/mac.c +++ b/lib/algorithms/mac.c @@ -332,6 +332,9 @@ void _gnutls_digest_mark_insecure_all(void) * through the allowlisting mode in the configuration file, or when * the setting is modified with a prior call to this function. * + * This function must be called prior to any session priority setting functions; + * otherwise the behavior is undefined. + * * Since: 3.7.3 */ int diff --git a/lib/algorithms/protocols.c b/lib/algorithms/protocols.c index b0f3e0bc30..64c86eba3c 100644 --- a/lib/algorithms/protocols.c +++ b/lib/algorithms/protocols.c @@ -237,6 +237,9 @@ void _gnutls_version_mark_revertible_all(void) * enabled through the allowlisting mode in the configuration file, or * when the setting is modified with a prior call to this function. * + * This function must be called prior to any session priority setting functions; + * otherwise the behavior is undefined. + * * Returns: 0 on success or negative error code otherwise. * * Since: 3.7.3 diff --git a/lib/algorithms/sign.c b/lib/algorithms/sign.c index 543bd19bb5..06abdb4cf8 100644 --- a/lib/algorithms/sign.c +++ b/lib/algorithms/sign.c @@ -516,6 +516,9 @@ void _gnutls_sign_mark_insecure_all(hash_security_level_t level) * use in certificates. Use gnutls_sign_set_secure_for_certs() to * mark it secure as well for certificates. * + * This function must be called prior to any session priority setting functions; + * otherwise the behavior is undefined. + * * Since: 3.7.3 */ int @@ -560,6 +563,9 @@ gnutls_sign_set_secure(gnutls_sign_algorithm_t sign, * for the use in certificates. Use gnutls_sign_set_secure() to mark * it insecure for any uses. * + * This function must be called prior to any session priority setting functions; + * otherwise the behavior is undefined. + * * Since: 3.7.3 */ int -- 2.34.1 From 52067e1e5b16c30b2f3ce6488a4f72d19d906721 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Mon, 14 Feb 2022 18:00:25 +0100 Subject: [PATCH 4/8] lib/priority: move sigalgs filtering to set_ciphersuite_list Signed-off-by: Alexander Sosedkin --- lib/priority.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/lib/priority.c b/lib/priority.c index 4faf96fabf..9775040410 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -1140,9 +1140,6 @@ cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) } if (cfg->allowlisting) { - unsigned tls_sig_sem = 0; - size_t j; - _gnutls_digest_mark_insecure_all(); for (i = 0; i < ctx->hashes_size; i++) { int ret = gnutls_digest_set_secure(ctx->hashes[i], 1); @@ -1156,6 +1153,7 @@ cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) if (unlikely(ret < 0)) { return ret; } + cfg->sigs[i] = ctx->sigs[i]; } for (i = 0; i < ctx->sigs_for_cert_size; i++) { int ret = gnutls_sign_set_secure_for_certs(ctx->sigs_for_cert[i], @@ -1165,13 +1163,13 @@ cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) } } _gnutls_version_mark_revertible_all(); - for (i = 0, j = 0; i < ctx->versions_size; i++) { - const version_entry_st *vers; - vers = version_to_entry(ctx->versions[i]); - if (vers && vers->supported) { - tls_sig_sem |= vers->tls_sig_sem; - cfg->versions[j++] = vers->id; + for (i = 0; i < ctx->versions_size; i++) { + int ret; + ret = gnutls_protocol_set_enabled(ctx->versions[i], 1); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); } + cfg->versions[i] = ctx->versions[i]; } _gnutls_ecc_curve_mark_disabled_all(); for (i = 0; i < ctx->curves_size; i++) { @@ -1180,15 +1178,6 @@ cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) return ret; } } - for (i = 0, j = 0; i < ctx->sigs_size; i++) { - const gnutls_sign_entry_st *se; - - se = _gnutls_sign_to_entry(ctx->sigs[i]); - if (se != NULL && se->aid.tls_sem & tls_sig_sem && - _gnutls_sign_is_secure2(se, 0)) { - cfg->sigs[j++] = se->id; - } - } } else { for (i = 0; i < ctx->hashes_size; i++) { int ret = _gnutls_digest_mark_insecure(ctx->hashes[i]); -- 2.34.1 From fa056709aaa974360d5e5dd8ceb52089b4da7858 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Tue, 15 Feb 2022 14:41:53 +0100 Subject: [PATCH 5/8] lib/config_int.h: split struct cfg off lib/priority.c Signed-off-by: Alexander Sosedkin --- lib/Makefile.am | 3 +- lib/config_int.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ lib/priority.c | 48 +-------------------------- 3 files changed, 87 insertions(+), 48 deletions(-) create mode 100644 lib/config_int.h diff --git a/lib/Makefile.am b/lib/Makefile.am index 35df35ee8d..5b540db142 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -134,7 +134,8 @@ HFILES = abstract_int.h debug.h cipher.h \ srp.h auth/srp_kx.h auth/srp_passwd.h \ file.h supplemental.h crypto.h random.h system.h\ locks.h mbuffers.h ecc.h pin.h fips.h \ - priority_options.h secrets.h stek.h cert-cred.h + priority_options.h secrets.h stek.h cert-cred.h \ + config_int.h if ENABLE_PKCS11 HFILES += pkcs11_int.h pkcs11x.h diff --git a/lib/config_int.h b/lib/config_int.h new file mode 100644 index 0000000000..733536bb98 --- /dev/null +++ b/lib/config_int.h @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2004-2015 Free Software Foundation, Inc. + * Copyright (C) 2015-2022 Red Hat, Inc. + * + * Author: Nikos Mavrogiannopoulos + * + * This file is part of GnuTLS. + * + * The GnuTLS is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public License + * as published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see + * + */ + +/* Code split off from priority.c, `struct cfg` and some operations on it */ + +#ifndef GNUTLS_LIB_CONFIG_INT_H +#define GNUTLS_LIB_CONFIG_INT_H + +/* + * struct cfg + */ + +struct cfg { + bool allowlisting; + + name_val_array_t priority_strings; + char *priority_string; + char *default_priority_string; + gnutls_certificate_verification_profiles_t verification_profile; + + gnutls_cipher_algorithm_t ciphers[MAX_ALGOS+1]; + gnutls_mac_algorithm_t macs[MAX_ALGOS+1]; + gnutls_group_t groups[MAX_ALGOS+1]; + gnutls_kx_algorithm_t kxs[MAX_ALGOS+1]; + gnutls_sign_algorithm_t sigs[MAX_ALGOS+1]; + gnutls_protocol_t versions[MAX_ALGOS+1]; +}; + +/* + * deinit / partial duplication. no initialization, must be zero-initialized + */ + +static inline void +cfg_deinit(struct cfg *cfg) +{ + if (cfg->priority_strings) { + _name_val_array_clear(&cfg->priority_strings); + } + gnutls_free(cfg->priority_string); + gnutls_free(cfg->default_priority_string); +} + +static inline void +cfg_steal(struct cfg *dst, struct cfg *src) +{ + dst->verification_profile = src->verification_profile; + + dst->priority_strings = src->priority_strings; + src->priority_strings = NULL; + + dst->priority_string = src->priority_string; + src->priority_string = NULL; + + dst->default_priority_string = src->default_priority_string; + src->default_priority_string = NULL; + + dst->allowlisting = src->allowlisting; + memcpy(dst->ciphers, src->ciphers, sizeof(src->ciphers)); + memcpy(dst->macs, src->macs, sizeof(src->macs)); + memcpy(dst->groups, src->groups, sizeof(src->groups)); + memcpy(dst->kxs, src->kxs, sizeof(src->kxs)); +} + +#endif /* GNUTLS_LIB_CONFIG_INT_H */ diff --git a/lib/priority.c b/lib/priority.c index 9775040410..80b737b938 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -42,6 +42,7 @@ #include "locks.h" #include "profiles.h" #include "name_val_array.h" +#include "config_int.h" #define MAX_ELEMENTS GNUTLS_MAX_ALGORITHM_NUM @@ -1008,32 +1009,6 @@ static void dummy_func(gnutls_priority_t c) #include -struct cfg { - bool allowlisting; - - name_val_array_t priority_strings; - char *priority_string; - char *default_priority_string; - gnutls_certificate_verification_profiles_t verification_profile; - - gnutls_cipher_algorithm_t ciphers[MAX_ALGOS+1]; - gnutls_mac_algorithm_t macs[MAX_ALGOS+1]; - gnutls_group_t groups[MAX_ALGOS+1]; - gnutls_kx_algorithm_t kxs[MAX_ALGOS+1]; - gnutls_sign_algorithm_t sigs[MAX_ALGOS+1]; - gnutls_protocol_t versions[MAX_ALGOS+1]; -}; - -static inline void -cfg_deinit(struct cfg *cfg) -{ - if (cfg->priority_strings) { - _name_val_array_clear(&cfg->priority_strings); - } - gnutls_free(cfg->priority_string); - gnutls_free(cfg->default_priority_string); -} - /* Lock for reading and writing system_wide_config */ GNUTLS_RWLOCK(system_wide_config_rwlock); static struct cfg system_wide_config; @@ -1107,27 +1082,6 @@ ini_ctx_deinit(struct ini_ctx *ctx) gnutls_free(ctx->curves); } -static inline void -cfg_steal(struct cfg *dst, struct cfg *src) -{ - dst->verification_profile = src->verification_profile; - - dst->priority_strings = src->priority_strings; - src->priority_strings = NULL; - - dst->priority_string = src->priority_string; - src->priority_string = NULL; - - dst->default_priority_string = src->default_priority_string; - src->default_priority_string = NULL; - - dst->allowlisting = src->allowlisting; - memcpy(dst->ciphers, src->ciphers, sizeof(src->ciphers)); - memcpy(dst->macs, src->macs, sizeof(src->macs)); - memcpy(dst->groups, src->groups, sizeof(src->groups)); - memcpy(dst->kxs, src->kxs, sizeof(src->kxs)); -} - static inline int cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) { -- 2.34.1 From 9600788ef81bd424de8c9fcf053bcb717dd50c92 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Tue, 15 Feb 2022 16:26:52 +0100 Subject: [PATCH 6/8] lib/priority: extract parts of cfg_apply into cfg_*_set_array* Signed-off-by: Alexander Sosedkin --- lib/config_int.h | 151 ++++++++++++++++++++++++++++++++++++++++++++++- lib/priority.c | 68 +++++++++------------ 2 files changed, 179 insertions(+), 40 deletions(-) diff --git a/lib/config_int.h b/lib/config_int.h index 733536bb98..be8c71e414 100644 --- a/lib/config_int.h +++ b/lib/config_int.h @@ -44,6 +44,10 @@ struct cfg { gnutls_kx_algorithm_t kxs[MAX_ALGOS+1]; gnutls_sign_algorithm_t sigs[MAX_ALGOS+1]; gnutls_protocol_t versions[MAX_ALGOS+1]; + + gnutls_digest_algorithm_t hashes[MAX_ALGOS+1]; + gnutls_ecc_curve_t ecc_curves[MAX_ALGOS+1]; + gnutls_sign_algorithm_t sigs_for_cert[MAX_ALGOS+1]; }; /* @@ -79,6 +83,151 @@ cfg_steal(struct cfg *dst, struct cfg *src) memcpy(dst->macs, src->macs, sizeof(src->macs)); memcpy(dst->groups, src->groups, sizeof(src->groups)); memcpy(dst->kxs, src->kxs, sizeof(src->kxs)); + memcpy(dst->hashes, src->hashes, sizeof(src->hashes)); + memcpy(dst->ecc_curves, src->ecc_curves, sizeof(src->ecc_curves)); + memcpy(dst->sigs, src->sigs, sizeof(src->sigs)); + memcpy(dst->sigs_for_cert, src->sigs_for_cert, + sizeof(src->sigs_for_cert)); +} + +/* + * synchronizing changes from struct cfg to global `lib/algorithms` arrays + */ + +/* global side-effect! modifies `flags` in `hash_algorithms[]` */ +static inline int /* allowlisting-only */ +_cfg_hashes_remark(struct cfg* cfg) +{ + size_t i; + _gnutls_digest_mark_insecure_all(); + for (i = 0; cfg->hashes[i] != 0; i++) { + int ret = gnutls_digest_set_secure(cfg->hashes[i], 1); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); + } + } + return 0; +} + +/* global side-effect! modifies `flags` in `sign_algorithms[]` */ +static inline int /* allowlisting-only */ +_cfg_sigs_remark(struct cfg* cfg) +{ + size_t i; + _gnutls_sign_mark_insecure_all(_INSECURE); + for (i = 0; cfg->sigs[i] != 0; i++) { + int ret = gnutls_sign_set_secure(cfg->sigs[i], 1); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); + } + } + for (i = 0; cfg->sigs_for_cert[i] != 0; i++) { + int ret = gnutls_sign_set_secure_for_certs( + cfg->sigs_for_cert[i], 1 + ); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); + } + } + return 0; +} + +/* global side-effect! modifies `supported` in `sup_versions[]` */ +static inline int /* allowlisting-only */ +_cfg_versions_remark(struct cfg* cfg) +{ + size_t i; + _gnutls_version_mark_revertible_all(); + for (i = 0; cfg->versions[i] != 0; i++) { + int ret = gnutls_protocol_set_enabled(cfg->versions[i], 1); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); + } + } + return 0; +} + +/* global side-effect! modifies `supported` in `ecc_curves[]` */ +static inline int /* allowlisting-only */ +_cfg_ecc_curves_remark(struct cfg* cfg) +{ + size_t i; + _gnutls_ecc_curve_mark_disabled_all(); + for (i = 0; cfg->ecc_curves[i] != 0; i++) { + int ret = gnutls_ecc_curve_set_enabled(cfg->ecc_curves[i], 1); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); + } + } + return 0; +} + +/* + * setting arrays of struct cfg: from other arrays + */ + +static inline int /* allowlisting-only */ +cfg_hashes_set_array(struct cfg* cfg, + gnutls_digest_algorithm_t* src, size_t len) +{ + if (unlikely(len >= MAX_ALGOS)) { + return gnutls_assert_val(GNUTLS_A_INTERNAL_ERROR); + } + if (len) { + memcpy(cfg->hashes, + src, sizeof(gnutls_digest_algorithm_t) * len); + } + cfg->hashes[len] = 0; + return _cfg_hashes_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_sigs_set_arrays(struct cfg* cfg, + gnutls_sign_algorithm_t* src, size_t len, + gnutls_sign_algorithm_t* src_for_cert, size_t len_for_cert) +{ + if (unlikely(len >= MAX_ALGOS)) { + return gnutls_assert_val(GNUTLS_A_INTERNAL_ERROR); + } + if (unlikely(len_for_cert >= MAX_ALGOS)) { + return gnutls_assert_val(GNUTLS_A_INTERNAL_ERROR); + } + if (len) { + memcpy(cfg->sigs, src, sizeof(gnutls_sign_algorithm_t) * len); + } + if (len_for_cert) { + memcpy(cfg->sigs_for_cert, src_for_cert, + sizeof(gnutls_sign_algorithm_t) * len_for_cert); + } + cfg->sigs[len] = 0; + cfg->sigs_for_cert[len_for_cert] = 0; + return _cfg_sigs_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_versions_set_array(struct cfg* cfg, gnutls_protocol_t* src, size_t len) +{ + if (unlikely(len >= MAX_ALGOS)) { + return gnutls_assert_val(GNUTLS_A_INTERNAL_ERROR); + } + if (len) { + memcpy(cfg->versions, src, sizeof(gnutls_protocol_t) * len); + } + cfg->versions[len] = 0; + return _cfg_versions_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_ecc_curves_set_array(struct cfg* cfg, gnutls_ecc_curve_t* src, size_t len) +{ + if (unlikely(len >= MAX_ALGOS)) { + return gnutls_assert_val(GNUTLS_A_INTERNAL_ERROR); + } + if (len) { + memcpy(cfg->ecc_curves, src, sizeof(gnutls_ecc_curve_t) * len); + } + cfg->ecc_curves[len] = 0; + return _cfg_ecc_curves_remark(cfg); } -#endif /* GNUTLS_LIB_CONFIG_INT_H */ +#endif /* GNUTLS_LIB_CONFIG_INT_H */ diff --git a/lib/priority.c b/lib/priority.c index 80b737b938..8d8428e1da 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -1086,6 +1086,7 @@ static inline int cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) { size_t i; + int ret; cfg_steal(cfg, &ctx->cfg); @@ -1094,72 +1095,61 @@ cfg_apply(struct cfg *cfg, struct ini_ctx *ctx) } if (cfg->allowlisting) { - _gnutls_digest_mark_insecure_all(); - for (i = 0; i < ctx->hashes_size; i++) { - int ret = gnutls_digest_set_secure(ctx->hashes[i], 1); - if (unlikely(ret < 0)) { - return ret; - } - } - _gnutls_sign_mark_insecure_all(_INSECURE); - for (i = 0; i < ctx->sigs_size; i++) { - int ret = gnutls_sign_set_secure(ctx->sigs[i], 1); - if (unlikely(ret < 0)) { - return ret; - } - cfg->sigs[i] = ctx->sigs[i]; + /* also updates `flags` of global `hash_algorithms[]` */ + ret = cfg_hashes_set_array(cfg, ctx->hashes, ctx->hashes_size); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); } - for (i = 0; i < ctx->sigs_for_cert_size; i++) { - int ret = gnutls_sign_set_secure_for_certs(ctx->sigs_for_cert[i], - 1); - if (unlikely(ret < 0)) { - return ret; - } + /* also updates `flags` of global `sign_algorithms[]` */ + ret = cfg_sigs_set_arrays(cfg, ctx->sigs, ctx->sigs_size, + ctx->sigs_for_cert, + ctx->sigs_for_cert_size); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); } - _gnutls_version_mark_revertible_all(); - for (i = 0; i < ctx->versions_size; i++) { - int ret; - ret = gnutls_protocol_set_enabled(ctx->versions[i], 1); - if (unlikely(ret < 0)) { - return gnutls_assert_val(ret); - } - cfg->versions[i] = ctx->versions[i]; + /* also updates `supported` field of global `sup_versions[]` */ + ret = cfg_versions_set_array(cfg, + ctx->versions, ctx->versions_size); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); } - _gnutls_ecc_curve_mark_disabled_all(); - for (i = 0; i < ctx->curves_size; i++) { - int ret = gnutls_ecc_curve_set_enabled(ctx->curves[i], 1); - if (unlikely(ret < 0)) { - return ret; - } + /* also updates `supported` field of global `ecc_curves[]` */ + ret = cfg_ecc_curves_set_array(cfg, + ctx->curves, ctx->curves_size); + if (unlikely(ret < 0)) { + return gnutls_assert_val(ret); } } else { + /* updates same global arrays as above, but doesn't store + * the algorithms into the `struct cfg` as allowlisting does. + * blocklisting doesn't allow relaxing the restrictions */ for (i = 0; i < ctx->hashes_size; i++) { - int ret = _gnutls_digest_mark_insecure(ctx->hashes[i]); + ret = _gnutls_digest_mark_insecure(ctx->hashes[i]); if (unlikely(ret < 0)) { return ret; } } for (i = 0; i < ctx->sigs_size; i++) { - int ret = _gnutls_sign_mark_insecure(ctx->sigs[i], + ret = _gnutls_sign_mark_insecure(ctx->sigs[i], _INSECURE); if (unlikely(ret < 0)) { return ret; } } for (i = 0; i < ctx->sigs_for_cert_size; i++) { - int ret = _gnutls_sign_mark_insecure(ctx->sigs_for_cert[i], _INSECURE_FOR_CERTS); + ret = _gnutls_sign_mark_insecure(ctx->sigs_for_cert[i], _INSECURE_FOR_CERTS); if (unlikely(ret < 0)) { return ret; } } for (i = 0; i < ctx->versions_size; i++) { - int ret = _gnutls_version_mark_disabled(ctx->versions[i]); + ret = _gnutls_version_mark_disabled(ctx->versions[i]); if (unlikely(ret < 0)) { return ret; } } for (i = 0; i < ctx->curves_size; i++) { - int ret = _gnutls_ecc_curve_mark_disabled(ctx->curves[i]); + ret = _gnutls_ecc_curve_mark_disabled(ctx->curves[i]); if (unlikely(ret < 0)) { return ret; } -- 2.34.1 From 1767ced4c0abf9d372d334a749b87fd00cd8ab5d Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Wed, 16 Feb 2022 14:28:18 +0100 Subject: [PATCH 7/8] plumb allowlisting API through the config, restrict usage to early times Signed-off-by: Alexander Sosedkin --- lib/algorithms.h | 7 +- lib/algorithms/ecc.c | 20 +-- lib/algorithms/mac.c | 18 +- lib/algorithms/protocols.c | 47 +++-- lib/algorithms/sign.c | 78 +-------- lib/config_int.h | 156 ++++++++++++++++- lib/global.h | 1 + lib/priority.c | 259 +++++++++++++++++++++++++++- tests/protocol-set-allowlist.c | 47 +++-- tests/protocol-set-allowlist.sh | 296 +++++++++++++++++++++----------- 10 files changed, 663 insertions(+), 266 deletions(-) diff --git a/lib/algorithms.h b/lib/algorithms.h index da72403fba..2c33a7210f 100644 --- a/lib/algorithms.h +++ b/lib/algorithms.h @@ -354,13 +354,18 @@ const gnutls_protocol_t *_gnutls_protocol_list(void); int _gnutls_version_mark_disabled(gnutls_protocol_t version); gnutls_protocol_t _gnutls_protocol_get_id_if_supported(const char *name); +int _gnutls_digest_set_secure(gnutls_digest_algorithm_t dig, unsigned int secure); +int _gnutls_sign_set_secure(gnutls_sign_algorithm_t sign, hash_security_level_t slevel); +int _gnutls_protocol_set_enabled(gnutls_protocol_t version, unsigned int enabled); +int _gnutls_ecc_curve_set_enabled(gnutls_ecc_curve_t curve, unsigned int enabled); + /* these functions are for revertible settings, meaning that algorithms marked * as disabled/insecure with mark_*_all functions can be re-enabled with * mark_{enabled,secure} functions */ void _gnutls_ecc_curve_mark_disabled_all(void); +void _gnutls_version_mark_disabled_all(void); void _gnutls_sign_mark_insecure_all(hash_security_level_t level); void _gnutls_digest_mark_insecure_all(void); -void _gnutls_version_mark_revertible_all(void); #define GNUTLS_SIGN_FLAG_TLS13_OK 1 /* if it is ok to use under TLS1.3 */ #define GNUTLS_SIGN_FLAG_CRT_VRFY_REVERSE (1 << 1) /* reverse order of bytes in CrtVrfy signature */ diff --git a/lib/algorithms/ecc.c b/lib/algorithms/ecc.c index 52ae1db0e4..303a42612f 100644 --- a/lib/algorithms/ecc.c +++ b/lib/algorithms/ecc.c @@ -379,26 +379,8 @@ void _gnutls_ecc_curve_mark_disabled_all(void) } } -/** - * gnutls_ecc_curve_set_enabled: - * @curve: is an ECC curve - * @enabled: whether to enable the curve - * - * Modify the previous system wide setting that marked @curve as - * enabled or disabled. This only has effect when the curve is - * enabled through the allowlisting mode in the configuration file, or - * when the setting is modified with a prior call to this function. - * - * This function must be called prior to any session priority setting functions; - * otherwise the behavior is undefined. - * - * Returns: 0 on success or negative error code otherwise. - * - * Since: 3.7.3 - */ int -gnutls_ecc_curve_set_enabled(gnutls_ecc_curve_t curve, - unsigned int enabled) +_gnutls_ecc_curve_set_enabled(gnutls_ecc_curve_t curve, unsigned int enabled) { gnutls_ecc_curve_entry_st *p; diff --git a/lib/algorithms/mac.c b/lib/algorithms/mac.c index 166d51d552..47fbc226bd 100644 --- a/lib/algorithms/mac.c +++ b/lib/algorithms/mac.c @@ -322,24 +322,8 @@ void _gnutls_digest_mark_insecure_all(void) #endif } -/** - * gnutls_digest_set_secure: - * @dig: is a digest algorithm - * @secure: whether to mark the digest algorithm secure - * - * Modify the previous system wide setting that marked @dig as secure - * or insecure. This only has effect when the algorithm is enabled - * through the allowlisting mode in the configuration file, or when - * the setting is modified with a prior call to this function. - * - * This function must be called prior to any session priority setting functions; - * otherwise the behavior is undefined. - * - * Since: 3.7.3 - */ int -gnutls_digest_set_secure(gnutls_digest_algorithm_t dig, - unsigned int secure) +_gnutls_digest_set_secure(gnutls_digest_algorithm_t dig, unsigned int secure) { #ifndef DISABLE_SYSTEM_CONFIG mac_entry_st *p; diff --git a/lib/algorithms/protocols.c b/lib/algorithms/protocols.c index 64c86eba3c..5a88123470 100644 --- a/lib/algorithms/protocols.c +++ b/lib/algorithms/protocols.c @@ -192,10 +192,11 @@ static int version_is_valid_for_session(gnutls_session_t session, const version_entry_st *v) { - if (v->supported && v->transport == session->internals.transport) { - return 1; - } - return 0; + if (!v->supported && !(v->supported_revertible && _gnutls_allowlisting_mode())) + return 0; + if (v->transport != session->internals.transport) + return 0; + return 1; } /* This is only called by cfg_apply in priority.c, in blocklisting mode. */ @@ -215,37 +216,20 @@ int _gnutls_version_mark_disabled(gnutls_protocol_t version) } /* This is only called by cfg_apply in priority.c, in allowlisting mode. */ -void _gnutls_version_mark_revertible_all(void) +void _gnutls_version_mark_disabled_all(void) { #ifndef DISABLE_SYSTEM_CONFIG version_entry_st *p; for (p = sup_versions; p->name != NULL; p++) { + p->supported = false; p->supported_revertible = true; } - #endif } -/** - * gnutls_protocol_set_enabled: - * @version: is a (gnutls) version number - * @enabled: whether to enable the protocol - * - * Mark the previous system wide setting that marked @version as - * enabled or disabled. This only has effect when the version is - * enabled through the allowlisting mode in the configuration file, or - * when the setting is modified with a prior call to this function. - * - * This function must be called prior to any session priority setting functions; - * otherwise the behavior is undefined. - * - * Returns: 0 on success or negative error code otherwise. - * - * Since: 3.7.3 - */ int -gnutls_protocol_set_enabled(gnutls_protocol_t version, +_gnutls_protocol_set_enabled(gnutls_protocol_t version, unsigned int enabled) { #ifndef DISABLE_SYSTEM_CONFIG @@ -331,7 +315,10 @@ const version_entry_st *_gnutls_version_max(gnutls_session_t session) if (p->obsolete != 0) break; #endif - if (!p->supported || p->transport != session->internals.transport) + if (!p->supported && !(p->supported_revertible && _gnutls_allowlisting_mode())) + break; + + if (p->transport != session->internals.transport) break; if (p->tls13_sem && (session->internals.flags & INT_FLAG_NO_TLS13)) @@ -386,7 +373,10 @@ int _gnutls_write_supported_versions(gnutls_session_t session, uint8_t *buffer, if (p->obsolete != 0) break; - if (!p->supported || p->transport != session->internals.transport) + if (!p->supported && !(p->supported_revertible && _gnutls_allowlisting_mode())) + break; + + if (p->transport != session->internals.transport) break; if (p->only_extension) @@ -569,7 +559,10 @@ _gnutls_nversion_is_supported(gnutls_session_t session, if (p->tls13_sem && (session->internals.flags & INT_FLAG_NO_TLS13)) return 0; - if (!p->supported || p->transport != session->internals.transport) + if (!p->supported && !(p->supported_revertible && _gnutls_allowlisting_mode())) + return 0; + + if (p->transport != session->internals.transport) return 0; version = p->id; diff --git a/lib/algorithms/sign.c b/lib/algorithms/sign.c index 06abdb4cf8..26816feef5 100644 --- a/lib/algorithms/sign.c +++ b/lib/algorithms/sign.c @@ -502,28 +502,9 @@ void _gnutls_sign_mark_insecure_all(hash_security_level_t level) #endif } -/** - * gnutls_sign_set_secure: - * @sign: the sign algorithm - * @secure: whether to mark the sign algorithm secure - * - * Modify the previous system wide setting that marked @sign as secure - * or insecure. This only has effect when the algorithm is marked as - * secure through the allowlisting mode in the configuration file, or - * when the setting is modified with a prior call to this function. - * - * Even when @secure is true, @sign is not marked as secure for the - * use in certificates. Use gnutls_sign_set_secure_for_certs() to - * mark it secure as well for certificates. - * - * This function must be called prior to any session priority setting functions; - * otherwise the behavior is undefined. - * - * Since: 3.7.3 - */ int -gnutls_sign_set_secure(gnutls_sign_algorithm_t sign, - unsigned int secure) +_gnutls_sign_set_secure(gnutls_sign_algorithm_t sign, + hash_security_level_t slevel) { #ifndef DISABLE_SYSTEM_CONFIG gnutls_sign_entry_st *p; @@ -533,60 +514,7 @@ gnutls_sign_set_secure(gnutls_sign_algorithm_t sign, if (!(p->flags & GNUTLS_SIGN_FLAG_INSECURE_REVERTIBLE)) { return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); } - if (secure) { - if (p->slevel > _INSECURE_FOR_CERTS) { - p->slevel = _INSECURE_FOR_CERTS; - } - } else { - p->slevel = _INSECURE; - } - return 0; - } - } -#endif - return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); -} - -/** - * gnutls_sign_set_secure_for_certs: - * @sign: the sign algorithm - * @secure: whether to mark the sign algorithm secure for certificates - * - * Modify the previous system wide setting that marked @sign as secure - * or insecure for the use in certificates. This only has effect when - * the algorithm is marked as secure through the allowlisting mode in - * the configuration file, or when the setting is modified with a - * prior call to this function. - * - * When @secure is true, @sign is marked as secure for any use unlike - * gnutls_sign_set_secure(). Otherwise, it is marked as insecure only - * for the use in certificates. Use gnutls_sign_set_secure() to mark - * it insecure for any uses. - * - * This function must be called prior to any session priority setting functions; - * otherwise the behavior is undefined. - * - * Since: 3.7.3 - */ -int -gnutls_sign_set_secure_for_certs(gnutls_sign_algorithm_t sign, - unsigned int secure) -{ -#ifndef DISABLE_SYSTEM_CONFIG - gnutls_sign_entry_st *p; - - for(p = sign_algorithms; p->name != NULL; p++) { - if (p->id && p->id == sign) { - if (!(p->flags & GNUTLS_SIGN_FLAG_INSECURE_REVERTIBLE)) { - return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); - } - if (secure) { - p->slevel = _SECURE; - } else { - if (p->slevel < _INSECURE_FOR_CERTS) { - p->slevel = _INSECURE_FOR_CERTS; - } - } + p->slevel = slevel; return 0; } } diff --git a/lib/config_int.h b/lib/config_int.h index be8c71e414..df39f2bf83 100644 --- a/lib/config_int.h +++ b/lib/config_int.h @@ -101,7 +101,7 @@ _cfg_hashes_remark(struct cfg* cfg) size_t i; _gnutls_digest_mark_insecure_all(); for (i = 0; cfg->hashes[i] != 0; i++) { - int ret = gnutls_digest_set_secure(cfg->hashes[i], 1); + int ret = _gnutls_digest_set_secure(cfg->hashes[i], 1); if (unlikely(ret < 0)) { return gnutls_assert_val(ret); } @@ -116,15 +116,15 @@ _cfg_sigs_remark(struct cfg* cfg) size_t i; _gnutls_sign_mark_insecure_all(_INSECURE); for (i = 0; cfg->sigs[i] != 0; i++) { - int ret = gnutls_sign_set_secure(cfg->sigs[i], 1); + int ret = _gnutls_sign_set_secure(cfg->sigs[i], + _INSECURE_FOR_CERTS); if (unlikely(ret < 0)) { return gnutls_assert_val(ret); } } for (i = 0; cfg->sigs_for_cert[i] != 0; i++) { - int ret = gnutls_sign_set_secure_for_certs( - cfg->sigs_for_cert[i], 1 - ); + int ret = _gnutls_sign_set_secure(cfg->sigs_for_cert[i], + _SECURE); if (unlikely(ret < 0)) { return gnutls_assert_val(ret); } @@ -137,9 +137,9 @@ static inline int /* allowlisting-only */ _cfg_versions_remark(struct cfg* cfg) { size_t i; - _gnutls_version_mark_revertible_all(); + _gnutls_version_mark_disabled_all(); for (i = 0; cfg->versions[i] != 0; i++) { - int ret = gnutls_protocol_set_enabled(cfg->versions[i], 1); + int ret = _gnutls_protocol_set_enabled(cfg->versions[i], 1); if (unlikely(ret < 0)) { return gnutls_assert_val(ret); } @@ -154,7 +154,7 @@ _cfg_ecc_curves_remark(struct cfg* cfg) size_t i; _gnutls_ecc_curve_mark_disabled_all(); for (i = 0; cfg->ecc_curves[i] != 0; i++) { - int ret = gnutls_ecc_curve_set_enabled(cfg->ecc_curves[i], 1); + int ret = _gnutls_ecc_curve_set_enabled(cfg->ecc_curves[i], 1); if (unlikely(ret < 0)) { return gnutls_assert_val(ret); } @@ -168,7 +168,7 @@ _cfg_ecc_curves_remark(struct cfg* cfg) static inline int /* allowlisting-only */ cfg_hashes_set_array(struct cfg* cfg, - gnutls_digest_algorithm_t* src, size_t len) + gnutls_digest_algorithm_t* src, size_t len) { if (unlikely(len >= MAX_ALGOS)) { return gnutls_assert_val(GNUTLS_A_INTERNAL_ERROR); @@ -230,4 +230,142 @@ cfg_ecc_curves_set_array(struct cfg* cfg, gnutls_ecc_curve_t* src, size_t len) return _cfg_ecc_curves_remark(cfg); } +/* + * appending to arrays of struct cfg + */ + +/* polymorphic way to DRY this operation. other possible approaches: + * 1. just unmacro (long) + * 2. cast to ints and write a function operating on ints + * (hacky, every call is +4 lines, needs a portable static assert) + * 3. macro whole functions, not just this operation (harder to find/read) + */ +#define APPEND_TO_NULL_TERMINATED_ARRAY(dst, element) \ + do { \ + size_t i; \ + for (i = 0; dst[i] != 0; i++) { \ + if (dst[i] == element) { \ + return 0; \ + } \ + } \ + if (unlikely(i >= MAX_ALGOS)) { \ + return gnutls_assert_val(GNUTLS_A_INTERNAL_ERROR); \ + } \ + dst[i] = element; \ + dst[i + 1] = 0; \ + } while (0) + +static inline int /* allowlisting-only */ +cfg_hashes_add(struct cfg *cfg, gnutls_digest_algorithm_t dig) +{ + _gnutls_debug_log("cfg: enabling digest algorithm %s\n", + gnutls_digest_get_name(dig)); + APPEND_TO_NULL_TERMINATED_ARRAY(cfg->hashes, dig); + return _cfg_hashes_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_sigs_add(struct cfg *cfg, gnutls_sign_algorithm_t sig) +{ + _gnutls_debug_log("cfg: enabling signature algorithm " + "(for non-certificate usage) " + "%s\n", gnutls_sign_get_name(sig)); + APPEND_TO_NULL_TERMINATED_ARRAY(cfg->sigs, sig); + return _cfg_sigs_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_sigs_for_cert_add(struct cfg *cfg, gnutls_sign_algorithm_t sig) +{ + _gnutls_debug_log("cfg: enabling signature algorithm" + "(for certificate usage) " + "%s\n", gnutls_sign_get_name(sig)); + APPEND_TO_NULL_TERMINATED_ARRAY(cfg->sigs_for_cert, sig); + return _cfg_sigs_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_versions_add(struct cfg *cfg, gnutls_protocol_t prot) +{ + _gnutls_debug_log("cfg: enabling version %s\n", + gnutls_protocol_get_name(prot)); + APPEND_TO_NULL_TERMINATED_ARRAY(cfg->versions, prot); + return _cfg_versions_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_ecc_curves_add(struct cfg *cfg, gnutls_ecc_curve_t curve) +{ + _gnutls_debug_log("cfg: enabling curve %s\n", + gnutls_ecc_curve_get_name(curve)); + APPEND_TO_NULL_TERMINATED_ARRAY(cfg->ecc_curves, curve); + return _cfg_ecc_curves_remark(cfg); +} + +#undef APPEND_TO_NULL_TERMINATED_ARRAY + +/* + * removing from arrays of struct cfg + */ + +/* polymorphic way to DRY this removal, see APPEND_TO_NULL_TERMINATED_ARRAY */ +#define REMOVE_FROM_NULL_TERMINATED_ARRAY(dst, element) \ + do { \ + size_t i, j; \ + for (i = 0; dst[i] != 0; i++) { \ + if (dst[i] == element) { \ + for (j = i; dst[j] != 0; j++) { \ + dst[j] = dst[j + 1]; \ + } \ + } \ + } \ + } while (0) + +static inline int /* allowlisting-only */ +cfg_hashes_remove(struct cfg *cfg, gnutls_digest_algorithm_t dig) +{ + _gnutls_debug_log("cfg: disabling digest algorithm %s\n", + gnutls_digest_get_name(dig)); + REMOVE_FROM_NULL_TERMINATED_ARRAY(cfg->hashes, dig); + return _cfg_hashes_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_sigs_remove(struct cfg *cfg, gnutls_sign_algorithm_t sig) +{ + _gnutls_debug_log("cfg: disabling signature algorithm " + "(for non-certificate usage) " + "%s\n", gnutls_sign_get_name(sig)); + REMOVE_FROM_NULL_TERMINATED_ARRAY(cfg->sigs, sig); + return _cfg_sigs_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_sigs_for_cert_remove(struct cfg *cfg, gnutls_sign_algorithm_t sig) +{ + _gnutls_debug_log("cfg: disabling signature algorithm" + "(for certificate usage) " + "%s\n", gnutls_sign_get_name(sig)); + REMOVE_FROM_NULL_TERMINATED_ARRAY(cfg->sigs_for_cert, sig); + return _cfg_sigs_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_versions_remove(struct cfg *cfg, gnutls_protocol_t prot) +{ + _gnutls_debug_log("cfg: disabling version %s\n", + gnutls_protocol_get_name(prot)); + REMOVE_FROM_NULL_TERMINATED_ARRAY(cfg->versions, prot); + return _cfg_versions_remark(cfg); +} + +static inline int /* allowlisting-only */ +cfg_ecc_curves_remove(struct cfg *cfg, gnutls_ecc_curve_t curve) +{ + _gnutls_debug_log("cfg: disabling curve %s\n", + gnutls_ecc_curve_get_name(curve)); + REMOVE_FROM_NULL_TERMINATED_ARRAY(cfg->ecc_curves, curve); + return _cfg_ecc_curves_remark(cfg); +} + #endif /* GNUTLS_LIB_CONFIG_INT_H */ diff --git a/lib/global.h b/lib/global.h index 16fde08b5c..6bd70df8e3 100644 --- a/lib/global.h +++ b/lib/global.h @@ -48,5 +48,6 @@ extern void _gnutls_nss_keylog_deinit(void); extern void _gnutls_prepare_to_load_system_priorities(void); extern void _gnutls_unload_system_priorities(void); +extern bool _gnutls_allowlisting_mode(void); #endif /* GNUTLS_LIB_GLOBAL_H */ diff --git a/lib/priority.c b/lib/priority.c index 8d8428e1da..c187284024 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -1023,6 +1023,11 @@ static unsigned system_priority_file_loaded = 0; #define OVERRIDES_SECTION "overrides" #define MAX_ALGO_NAME 2048 +bool _gnutls_allowlisting_mode(void) +{ + return system_wide_config.allowlisting; +} + static void _clear_default_system_priority(void) { gnutls_free(system_wide_config.default_priority_string); @@ -2215,7 +2220,9 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache) /* disable TLS versions which are added but are unsupported */ for (i = j = 0; i < priority_cache->protocol.num_priorities; i++) { vers = version_to_entry(priority_cache->protocol.priorities[i]); - if (!vers || vers->supported) + if (!vers || vers->supported || + (system_wide_config.allowlisting && \ + vers->supported_revertible)) priority_cache->protocol.priorities[j++] = priority_cache->protocol.priorities[i]; } priority_cache->protocol.num_priorities = j; @@ -3393,3 +3400,253 @@ gnutls_priority_string_list(unsigned iter, unsigned int flags) } return NULL; } + +/* + * high-level interface for overriding configuration files + */ + +static inline int /* not locking system_wide_config */ +system_wide_config_is_malleable(void) { + if (!system_wide_config.allowlisting) { + _gnutls_debug_log("allowlisting is not enabled!\n"); + return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); + } + if (system_wide_config.priority_string) { + _gnutls_debug_log("priority strings have already been " + "initialized!\n"); + return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); + } + return 1; +} + +/** + * gnutls_digest_set_secure: + * @dig: is a digest algorithm + * @secure: whether to mark the digest algorithm secure + * + * Modify the previous system wide setting that marked @dig as secure + * or insecure. This only has effect when the algorithm is enabled + * through the allowlisting mode in the configuration file, or when + * the setting is modified with a prior call to this function. + * + * Since: 3.7.3 + */ +int +gnutls_digest_set_secure(gnutls_digest_algorithm_t dig, unsigned int secure) +{ +#ifndef DISABLE_SYSTEM_CONFIG + int ret; + ret = gnutls_rwlock_wrlock(&system_wide_config_rwlock); + if (ret < 0) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + ret = system_wide_config_is_malleable(); + if (ret != 1) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + + if (secure) { + ret = cfg_hashes_add(&system_wide_config, dig); + } else { + ret = cfg_hashes_remove(&system_wide_config, dig); + } + + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return ret; +#else + return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); +#endif +} + +/** + * gnutls_sign_set_secure: + * @sign: the sign algorithm + * @secure: whether to mark the sign algorithm secure + * + * Modify the previous system wide setting that marked @sign as secure + * or insecure. This only has effect when the algorithm is marked as + * secure through the allowlisting mode in the configuration file, or + * when the setting is modified with a prior call to this function. + * + * Even when @secure is true, @sign is not marked as secure for the + * use in certificates. Use gnutls_sign_set_secure_for_certs() to + * mark it secure as well for certificates. + * + * Since: 3.7.3 + */ +int +gnutls_sign_set_secure(gnutls_sign_algorithm_t sign, unsigned int secure) +{ +#ifndef DISABLE_SYSTEM_CONFIG + int ret; + ret = gnutls_rwlock_wrlock(&system_wide_config_rwlock); + if (ret < 0) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + ret = system_wide_config_is_malleable(); + if (ret != 1) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + + if (secure) { + ret = cfg_sigs_add(&system_wide_config, sign); + } else { + ret = cfg_sigs_remove(&system_wide_config, sign); + if (ret < 0) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return ret; + } + /* irregularity, distrusting also means distrusting for certs */ + ret = cfg_sigs_for_cert_remove(&system_wide_config, sign); + } + + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return ret; +#else + return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); +#endif +} + +/** + * gnutls_sign_set_secure_for_certs: + * @sign: the sign algorithm + * @secure: whether to mark the sign algorithm secure for certificates + * + * Modify the previous system wide setting that marked @sign as secure + * or insecure for the use in certificates. This only has effect when + * the algorithm is marked as secure through the allowlisting mode in + * the configuration file, or when the setting is modified with a + * prior call to this function. + * + * When @secure is true, @sign is marked as secure for any use unlike + * gnutls_sign_set_secure(). Otherwise, it is marked as insecure only + * for the use in certificates. Use gnutls_sign_set_secure() to mark + * it insecure for any uses. + * + * Since: 3.7.3 + */ +int +gnutls_sign_set_secure_for_certs(gnutls_sign_algorithm_t sign, + unsigned int secure) +{ +#ifndef DISABLE_SYSTEM_CONFIG + int ret; + ret = gnutls_rwlock_wrlock(&system_wide_config_rwlock); + if (ret < 0) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + ret = system_wide_config_is_malleable(); + if (ret != 1) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + + if (secure) { + /* irregularity, trusting for certs means trusting in general */ + ret = cfg_sigs_add(&system_wide_config, sign); + if (ret < 0) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return ret; + } + ret = cfg_sigs_for_cert_add(&system_wide_config, sign); + } else { + ret = cfg_sigs_for_cert_remove(&system_wide_config, sign); + } + + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return ret; +#else + return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); +#endif +} + +/** + * gnutls_protocol_set_enabled: + * @version: is a (gnutls) version number + * @enabled: whether to enable the protocol + * + * Mark the previous system wide setting that marked @version as + * enabled or disabled. This only has effect when the version is + * enabled through the allowlisting mode in the configuration file, or + * when the setting is modified with a prior call to this function. + * + * Returns: 0 on success or negative error code otherwise. + * + * Since: 3.7.3 + */ +int /* allowlisting-only */ /* not thread-safe */ +gnutls_protocol_set_enabled(gnutls_protocol_t version, unsigned int enabled) +{ +#ifndef DISABLE_SYSTEM_CONFIG + int ret; + ret = gnutls_rwlock_wrlock(&system_wide_config_rwlock); + if (ret < 0) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + ret = system_wide_config_is_malleable(); + if (ret != 1) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + + if (enabled) { + ret = cfg_versions_add(&system_wide_config, version); + } else { + ret = cfg_versions_remove(&system_wide_config, version); + } + + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return ret; +#else + return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); +#endif +} + +/** + * gnutls_ecc_curve_set_enabled: + * @curve: is an ECC curve + * @enabled: whether to enable the curve + * + * Modify the previous system wide setting that marked @curve as + * enabled or disabled. This only has effect when the curve is + * enabled through the allowlisting mode in the configuration file, or + * when the setting is modified with a prior call to this function. + * + * Returns: 0 on success or negative error code otherwise. + * + * Since: 3.7.3 + */ +int +gnutls_ecc_curve_set_enabled(gnutls_ecc_curve_t curve, unsigned int enabled) +{ +#ifndef DISABLE_SYSTEM_CONFIG + int ret; + ret = gnutls_rwlock_wrlock(&system_wide_config_rwlock); + if (ret < 0) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + ret = system_wide_config_is_malleable(); + if (ret != 1) { + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return gnutls_assert_val(ret); + } + + if (enabled) { + ret = cfg_ecc_curves_add(&system_wide_config, curve); + } else { + ret = cfg_ecc_curves_remove(&system_wide_config, curve); + } + + (void)gnutls_rwlock_unlock(&system_wide_config_rwlock); + return ret; +#else + return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST); +#endif +} diff --git a/tests/protocol-set-allowlist.c b/tests/protocol-set-allowlist.c index 754e4d12d1..744d70b315 100644 --- a/tests/protocol-set-allowlist.c +++ b/tests/protocol-set-allowlist.c @@ -37,15 +37,21 @@ * This is not a test by itself. * This is a helper for the real test in protocol-set-allowlist.sh. * It executes sequences of commands like: - * > connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) - * > protocol_set_disabled TLS1.2 -> OK - * > connect -> bad priority: (actually, any arrow-less text can go here) + * > protocol_set_disabled TLS1.2 + * > protocol_set_enabled TLS1.1 + * > connect + * > protocol_set_enabled TLS1.2 + * > protocol_set_disabled TLS1.1 + * > connect -> connection established * where `connect` connects to $TEST_SERVER_PORT using $TEST_SERVER_CA, * and gnutls_protocol_set_enabled simply call the underlying API. * leaving the outer test to check return code and output: - * connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) * protocol_set_disabled TLS1.2 -> OK - * connect -> bad priority: No or insufficient priorities were set. + * protocol_set_enabled TLS1.1 -> OK + * connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) + * protocol_set_enabled TLS1.2 -> INVALID_REQUEST + * protocol_set_disabled TLS1.1 -> INVALID_REQUEST + * connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) */ #define _assert(cond, format, ...) if (!(cond)) \ @@ -58,6 +64,7 @@ void test_echo_server(gnutls_session_t session); void cmd_connect(const char* ca_file, unsigned port); void cmd_protocol_set_disabled(const char* name); void cmd_protocol_set_enabled(const char* name); +void cmd_reinit(void); const char* unprefix(const char* s, const char* prefix); @@ -167,15 +174,32 @@ void cmd_connect(const char* ca_file, unsigned port) void cmd_protocol_set_disabled(const char* name) { - _check(gnutls_protocol_set_enabled(parse_protocol(name), 0) >= 0); - printf("protocol_set_disabled %s -> OK\n", name); + int ret; + ret = gnutls_protocol_set_enabled(parse_protocol(name), 0); + printf("protocol_set_disabled %s -> %s\n", name, + ret == 0 ? "OK" : + ret == GNUTLS_E_INVALID_REQUEST ? "INVALID_REQUEST" : + gnutls_strerror(ret)); } void cmd_protocol_set_enabled(const char* name) { - _check(gnutls_protocol_set_enabled(parse_protocol(name), 1) >= 0); - printf("protocol_set_enabled %s -> OK\n", name); + int ret; + ret = gnutls_protocol_set_enabled(parse_protocol(name), 1); + printf("protocol_set_enabled %s -> %s\n", name, + ret == 0 ? "OK" : + ret == GNUTLS_E_INVALID_REQUEST ? "INVALID_REQUEST" : + gnutls_strerror(ret)); +} + + +void cmd_reinit(void) +{ + int ret; + gnutls_global_deinit(); + ret = gnutls_global_init(); + printf("reinit -> %s\n", ret == 0 ? "OK" : gnutls_strerror(ret)); } @@ -204,8 +228,6 @@ void doit(void) _assert(port_str, "TEST_SERVER_PORT is not set"); port = parse_port(port_str); - _check(gnutls_global_init() >= 0); - while (!feof(stdin)) { memset(cmd_buf, '\0', MAX_CMD_LEN + 1); fgets(cmd_buf, MAX_CMD_LEN, stdin); @@ -220,6 +242,8 @@ void doit(void) cmd_protocol_set_disabled(p); else if ((p = unprefix(cmd_buf, "> protocol_set_enabled "))) cmd_protocol_set_enabled(p); + else if (!strcmp(cmd_buf, "> reinit")) + cmd_reinit(); else if ((p = unprefix(cmd_buf, "> "))) _fail("Unknown command `%s`\n", p); else @@ -227,6 +251,5 @@ void doit(void) cmd_buf); } - gnutls_global_deinit(); exit(0); } diff --git a/tests/protocol-set-allowlist.sh b/tests/protocol-set-allowlist.sh index 907f37562f..ee2fe649bd 100755 --- a/tests/protocol-set-allowlist.sh +++ b/tests/protocol-set-allowlist.sh @@ -25,6 +25,7 @@ # from within the shell wrapper protocol-set-allowlist.sh # The shell part of it feeds commands into a C helper # and compares its output to the reference output. +# Commands are derived from the reference output. : ${srcdir=.} : ${builddir=.} @@ -161,6 +162,9 @@ fi ### Harness for the actual tests test_with_helper() { + echo '#' + echo "# $1" + echo '#' ${CAT} > "$TMPFILE_EXPECTED_LOG" ${SED} 's/\(.*\) -> .*/> \1/' "${TMPFILE_EXPECTED_LOG}" \ > "${TMPFILE_INPUT_SCRIPT}" @@ -197,152 +201,234 @@ launch_server --echo --priority "NORMAL:-VERS-TLS-ALL:+VERS-TLS1.2" \ SERVER_PID=$! wait_server ${SERVER_PID} -# ["gnutls_protocol_set_enabled can disable, TLS"] -# With a configuration file allowlisting a specific TLS protocol version (1.2), -# gnutls_protocol_set_enabled can disable it. -test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -protocol_set_disabled TLS1.2 -> OK -connect -> bad priority: No or insufficient priorities were set. EOF -# ["gnutls_protocol_set_enabled disables revertibly, TLS"] -# consecutive gnutls_protocol_set_enabled can make connection possible -# (with a different session handle). -test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -protocol_set_disabled TLS1.2 -> OK -connect -> bad priority: No or insufficient priorities were set. -protocol_set_enabled TLS1.2 -> OK +protocol_set_disabled TLS1.2 -> INVALID_REQUEST connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) EOF -# Just a random long-ish scenario -test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +test_with_helper 'disabling TLS 1.2 leaves us with no versions' < OK connect -> bad priority: No or insufficient priorities were set. -protocol_set_enabled TLS1.3 -> OK +protocol_set_enabled TLS1.2 -> INVALID_REQUEST connect -> bad priority: No or insufficient priorities were set. -protocol_set_disabled TLS1.3 -> OK +EOF + +test_with_helper \ + 'disabling is revertible if done before the first gnutls_init' << EOF +protocol_set_disabled TLS1.2 -> OK protocol_set_enabled TLS1.2 -> OK connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +protocol_set_disabled TLS1.2 -> INVALID_REQUEST +protocol_set_enabled TLS1.2 -> INVALID_REQUEST +connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) EOF -# !!! CURRENTLY NOT WORKING AS EXPECTED !!! -# Insufficient priority vs handshake failed -#test_with_helper < OK +#connect -> bad priority: No or insufficient priorities were set. +#reinit -> OK +#connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +#EOF + +# Reinit after restricting algorithms has problems with FIPS self-tests +#test_with_helper \ +# 'library reinitialization allows new API again, but resets changes' \ +# < OK #connect -> bad priority: No or insufficient priorities were set. -#protocol_set_enabled TLS1.3 -> OK -#connect -> handshake failed: A packet with illegal or unsupported version was received. +#protocol_set_enabled TLS1.2 -> INVALID_REQUEST +#connect -> bad priority: No or insufficient priorities were set. +#reinit -> OK +#connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +#protocol_set_disabled TLS1.2 -> INVALID_REQUEST +#connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +#reinit -> OK +#protocol_set_disabled TLS1.2 -> OK +#protocol_set_enabled TLS1.2 -> OK +#connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +#protocol_set_disabled TLS1.2 -> INVALID_REQUEST #EOF +test_with_helper 'Insufficient priority vs handshake failed: 1/2' < OK +connect -> bad priority: No or insufficient priorities were set. +EOF + +test_with_helper 'Insufficient priority vs handshake failed: 2/2' < OK +protocol_set_enabled TLS1.3 -> OK +connect -> handshake failed: A TLS fatal alert has been received. +EOF +# TLS 1.3 does some masquerading as TLS 1.2, I guess, so it's not +# handshake failed: A packet with illegal or unsupported version was received. + terminate_proc ${SERVER_PID} ### Tests against a NORMAL server (all three TLS versions enabled) eval "${GETPORT}" # server is launched without allowlisting config file in effect -launch_server -d9 --echo --priority NORMAL \ +launch_server --echo --priority NORMAL \ --x509keyfile "${TMPFILE_KEY}" --x509certfile "${TMPFILE_CERT}" SERVER_PID=$! wait_server ${SERVER_PID} -# !!! CURRENTLY NOT WORKING AS EXPECTED !!! -# smoke-test enabling with protocol_set -#test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#protocol_set_enabled TLS1.3 -> OK -#connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) -#EOF +# sanity-test +test_with_helper 'sanity test against liberal server' < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +EOF -# !!! CURRENTLY NOT WORKING AS EXPECTED !!! -# ["gnutls_protocol_set_enabled enables, TLS"] -# with a configuration file not allowlisting a specific TLS protocol version, -# enabling that version with gnutls_protocol_set_enabled -# allows connecting to a server accepting this TLS protocol version alone -#test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#protocol_set_enabled TLS1.3 -> OK -#connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) -#EOF +test_with_helper 'smoke-test enabling' < OK +connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) +EOF -# !!! CURRENTLY NOT WORKING AS EXPECTED !!! -# ["gnutls_protocol_set_enabled enables revertibly, TLS"] -# consecutive gnutls_protocol_set -# can prevent the client from connecting (with a different session handle) -#test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#protocol_set_enabled TLS1.1 -> OK -#connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#protocol_set_disabled TLS1.2 -> OK -#connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) -#protocol_set_disabled TLS1.1 -> OK -#connect -> bad priority: No or insufficient priorities were set. -#EOF -# Alternative one -#test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#protocol_set_enabled TLS1.3 -> OK -#connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) -#protocol_set_disabled TLS1.3 -> OK -#connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#EOF +test_with_helper 'going down to TLS1.1' < OK +protocol_set_disabled TLS1.2 -> OK +connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) +EOF -# !!! CURRENTLY NOT WORKING AS EXPECTED !!! -# ["gnutls_protocol_set_disabled disables selectively, TLS"] -# gnutls_protocol_set_disabled with a specific version -# doesn't disable other previously enabled version. -# ["gnutls_protocol_set_enabled enables selectively, TLS"] -# gnutls_protocol_set_enabled enabling a specific version -# doesn't enable other previously disabled version. -#test_with_helper < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#protocol_set_enabled TLS1.3 -> OK -#protocol_set_enabled TLS1.2 -> OK -#protocol_set_enabled TLS1.1 -> OK -#connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) -#protocol_set_disabled TLS1.3 -> OK -#connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) -#protocol_set_disabled TLS1.2 -> OK -#connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) -#protocol_set_disabled TLS1.1 -> OK -#connect -> bad priority: No or insufficient priorities were set. -#protocol_set_enabled TLS1.1 -> OK -#connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) -#protocol_set_enabled TLS1.2 -> OK -#connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) -#protocol_set_enabled TLS1.3 -> OK -#connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) -#EOF +test_with_helper 'going up to TLS 1.3' < OK +connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) +EOF + +test_with_helper 'useless toggles' < OK +protocol_set_disabled TLS1.2 -> OK +protocol_set_enabled TLS1.2 -> OK +protocol_set_enabled TLS1.1 -> OK +protocol_set_enabled TLS1.1 -> OK +protocol_set_enabled TLS1.3 -> OK +protocol_set_disabled TLS1.1 -> OK +protocol_set_disabled TLS1.3 -> OK +connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +EOF + +test_with_helper 'disable does not overdisable: 1/2' < OK +protocol_set_enabled TLS1.2 -> OK +protocol_set_enabled TLS1.1 -> OK +protocol_set_disabled TLS1.3 -> OK +protocol_set_disabled TLS1.1 -> OK +connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +EOF + +test_with_helper 'disable does not overdisable: 2/2' < OK +protocol_set_enabled TLS1.2 -> OK +protocol_set_enabled TLS1.1 -> OK +protocol_set_disabled TLS1.3 -> OK +protocol_set_disabled TLS1.2 -> OK +connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) +EOF terminate_proc ${SERVER_PID} -### Tests against a TLS 1.1 & 1.3 server (1.2 disabled) +#### Tests against a TLS 1.3 server +# +eval "${GETPORT}" +# server is launched without allowlisting config file in effect +launch_server --echo \ + --priority "NORMAL:-VERS-TLS-ALL:+VERS-TLS1.3" \ + --x509keyfile "${TMPFILE_KEY}" --x509certfile "${TMPFILE_CERT}" +SERVER_PID=$! +wait_server ${SERVER_PID} + +test_with_helper 'sanity negative' < handshake failed: A TLS fatal alert has been received. +protocol_set_enabled TLS1.3 -> INVALID_REQUEST +protocol_set_enabled TLS1.1 -> INVALID_REQUEST +protocol_set_disabled TLS1.2 -> INVALID_REQUEST +connect -> handshake failed: A TLS fatal alert has been received. +EOF + +test_with_helper 'enable 1.3' < OK +connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) +EOF + +test_with_helper 'enable 1.3 only' < OK +protocol_set_enabled TLS1.3 -> OK +connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) +EOF + +test_with_helper 'enable 1.1' < OK +connect -> handshake failed: A TLS fatal alert has been received. +EOF +# A special case according to a comment in set_ciphersuite_list: +# > we require TLS1.2 to be enabled if TLS1.3 is asked for, and +# > a pre-TLS1.2 protocol is there; that is because servers which +# > do not support TLS1.3 will negotiate TLS1.2 if seen a TLS1.3 handshake +test_with_helper 'enable 1.1 and 1.3 only - does not work as you expect' < OK +protocol_set_disabled TLS1.2 -> OK +protocol_set_enabled TLS1.1 -> OK +connect -> handshake failed: A packet with illegal or unsupported version was received. +EOF + +test_with_helper 'enable 1.1 and 1.3' < OK +protocol_set_enabled TLS1.1 -> OK +connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) +EOF + +test_with_helper 'enable 1.1 and 1.3, different order' < OK +protocol_set_enabled TLS1.3 -> OK +connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) +EOF + +terminate_proc ${SERVER_PID} + +#### Tests against a TLS 1.1 + TLS 1.2 server +# eval "${GETPORT}" # server is launched without allowlisting config file in effect -launch_server -d9 --echo \ - --priority "NORMAL:-VERS-TLS-ALL:+VERS-TLS1.1:+VERS-TLS1.3" \ +launch_server --echo \ + --priority "NORMAL:-VERS-TLS-ALL:+VERS-TLS1.1:+VERS-TLS1.2" \ --x509keyfile "${TMPFILE_KEY}" --x509certfile "${TMPFILE_CERT}" SERVER_PID=$! wait_server ${SERVER_PID} -# !!! CURRENTLY NOT WORKING AS EXPECTED !!! -#test_with_helper < handshake failed: A packet with illegal or unsupported version was received. -#protocol_set_enabled TLS1.1 -> OK -#connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) -#protocol_set_enabled TLS1.3 -> OK -#connect -> connection established: (TLS1.3)-(DHE-FFDHE3072)-(RSA-PSS-RSAE-SHA256)-(AES-128-GCM) -#protocol_set_disabled TLS1.3 -> OK -#connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) -#protocol_set_disabled TLS1.1 -> OK -#connect -> handshake failed: A packet with illegal or unsupported version was received. -#protocol_set_disabled TLS1.2 -> OK -#connect -> bad priority: No or insufficient priorities were set. -#EOF +test_with_helper 'sanity 1.2' < connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +EOF + +test_with_helper 'enable 1.1' < OK +connect -> connection established: (TLS1.2)-(RSA)-(AES-128-GCM) +EOF + +test_with_helper 'enable 1.1 only' < OK +protocol_set_disabled TLS1.2 -> OK +connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) +EOF + +test_with_helper 'enable 1.1 and 1.3 only' < OK +protocol_set_disabled TLS1.2 -> OK +protocol_set_enabled TLS1.1 -> OK +connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) +EOF + +test_with_helper 'enable 1.1 and 1.3 only, different order' < OK +protocol_set_disabled TLS1.2 -> OK +protocol_set_enabled TLS1.3 -> OK +connect -> connection established: (TLS1.1)-(RSA)-(AES-128-CBC)-(SHA1) +EOF terminate_proc ${SERVER_PID} -- 2.34.1 From ebcf38f6ac4dce0fe62bdd43114cb7415eee8d16 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Wed, 16 Feb 2022 14:36:48 +0100 Subject: [PATCH 8/8] update documentation on allowlisting API (in a separate commit so that it's easier to compare) Signed-off-by: Alexander Sosedkin --- lib/priority.c | 69 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 15 deletions(-) diff --git a/lib/priority.c b/lib/priority.c index c187284024..4e9d2d9cc4 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -3466,14 +3466,25 @@ gnutls_digest_set_secure(gnutls_digest_algorithm_t dig, unsigned int secure) * @secure: whether to mark the sign algorithm secure * * Modify the previous system wide setting that marked @sign as secure - * or insecure. This only has effect when the algorithm is marked as - * secure through the allowlisting mode in the configuration file, or - * when the setting is modified with a prior call to this function. + * or insecure. Calling this function is allowed + * only if allowlisting mode is set in the configuration file, + * and only if the system-wide TLS priority string + * has not been initialized yet. + * The intended usage is to provide applications with a way + * to expressly deviate from the distribution or site defaults + * inherited from the configuration file. + * The modification is composable with further modifications + * performed through the priority string mechanism. + * + * This function is not thread-safe and is intended to be called + * in the main thread at the beginning of the process execution. * * Even when @secure is true, @sign is not marked as secure for the * use in certificates. Use gnutls_sign_set_secure_for_certs() to * mark it secure as well for certificates. * + * Returns: 0 on success or negative error code otherwise. + * * Since: 3.7.3 */ int @@ -3517,16 +3528,26 @@ gnutls_sign_set_secure(gnutls_sign_algorithm_t sign, unsigned int secure) * @secure: whether to mark the sign algorithm secure for certificates * * Modify the previous system wide setting that marked @sign as secure - * or insecure for the use in certificates. This only has effect when - * the algorithm is marked as secure through the allowlisting mode in - * the configuration file, or when the setting is modified with a - * prior call to this function. - * + * or insecure for the use in certificates. Calling this fuction is allowed + * only if allowlisting mode is set in the configuration file, + * and only if the system-wide TLS priority string + * has not been initialized yet. + * The intended usage is to provide applications with a way + * to expressly deviate from the distribution or site defaults + * inherited from the configuration file. + * The modification is composable with further modifications + * performed through the priority string mechanism. + * + * This function is not thread-safe and is intended to be called + * in the main thread at the beginning of the process execution. + * When @secure is true, @sign is marked as secure for any use unlike * gnutls_sign_set_secure(). Otherwise, it is marked as insecure only * for the use in certificates. Use gnutls_sign_set_secure() to mark * it insecure for any uses. * + * Returns: 0 on success or negative error code otherwise. + * * Since: 3.7.3 */ int @@ -3570,10 +3591,19 @@ gnutls_sign_set_secure_for_certs(gnutls_sign_algorithm_t sign, * @version: is a (gnutls) version number * @enabled: whether to enable the protocol * - * Mark the previous system wide setting that marked @version as - * enabled or disabled. This only has effect when the version is - * enabled through the allowlisting mode in the configuration file, or - * when the setting is modified with a prior call to this function. + * Control the previous system-wide setting that marked @version as + * enabled or disabled. Calling this fuction is allowed + * only if allowlisting mode is set in the configuration file, + * and only if the system-wide TLS priority string + * has not been initialized yet. + * The intended usage is to provide applications with a way + * to expressly deviate from the distribution or site defaults + * inherited from the configuration file. + * The modification is composable with further modifications + * performed through the priority string mechanism. + * + * This function is not thread-safe and is intended to be called + * in the main thread at the beginning of the process execution. * * Returns: 0 on success or negative error code otherwise. * @@ -3614,9 +3644,18 @@ gnutls_protocol_set_enabled(gnutls_protocol_t version, unsigned int enabled) * @enabled: whether to enable the curve * * Modify the previous system wide setting that marked @curve as - * enabled or disabled. This only has effect when the curve is - * enabled through the allowlisting mode in the configuration file, or - * when the setting is modified with a prior call to this function. + * enabled or disabled. Calling this fuction is allowed + * only if allowlisting mode is set in the configuration file, + * and only if the system-wide TLS priority string + * has not been initialized yet. + * The intended usage is to provide applications with a way + * to expressly deviate from the distribution or site defaults + * inherited from the configuration file. + * The modification is composable with further modifications + * performed through the priority string mechanism. + * + * This function is not thread-safe and is intended to be called + * in the main thread at the beginning of the process execution. * * Returns: 0 on success or negative error code otherwise. * -- 2.34.1