rpms/gnutls
5
0
Fork 0
Code Issues Pull Requests Releases Activity

Make allowlisting configuration robuster

Browse Source 
- Increase GNUTLS_MAX_ALGORITHM_NUM for allowlisting
- Ensure allowlisting API is called before priority string is constructed

Related: #2033220
Related: #2042532
Signed-off-by: Daiki Ueno <dueno@redhat.com>
This commit is contained in:
Daiki Ueno 2022-02-23 18:56:18 +01:00
parent 7784eaae22
commit 89eb1823f0
4 changed files with 560 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

484
gnutls-3.7.3-allowlist-api.patch Normal file
View File

@ -0,0 +1,484 @@
From 495ad3d82aff125f237f370a70882b97843edb6f Mon Sep 17 00:00:00 2001
From: Alexander Sosedkin <asosedkin@redhat.com>
Date: Mon, 14 Feb 2022 12:44:57 +0100
Subject: [PATCH 1/3] lib/priority: split up update_system_wide_priority_string
This is done in preparation for deferring priority string evaluation.
Signed-off-by: Alexander Sosedkin <asosedkin@redhat.com>
---
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 <asosedkin@redhat.com>
Date: Mon, 14 Feb 2022 13:48:37 +0100
Subject: [PATCH 2/3] lib/priority: defer setting system-wide priority string
Signed-off-by: Alexander Sosedkin <asosedkin@redhat.com>
---
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 <asosedkin@redhat.com>
Date: Mon, 21 Feb 2022 18:19:25 +0100
Subject: [PATCH 3/3] lib/algorithms: add UB warnings on late allowlisting API
invocations
Signed-off-by: Alexander Sosedkin <asosedkin@redhat.com>
---
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

25
gnutls-3.7.3-disable-config-reload.patch
View File

@ -1,19 +1,26 @@
diff --color -ru a/lib/priority.c b/lib/priority.c
--- a/lib/priority.c 2022-01-14 07:53:21.000000000 +0100
+++ b/lib/priority.c 2022-02-15 09:31:36.388485784 +0100
@@ -2030,15 +2030,6 @@
diff --git a/lib/priority.c b/lib/priority.c
index 9feec47fe2..40511710fd 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -2001,13 +2001,14 @@ char *_gnutls_resolve_priorities(const char* priorities)
additional++;
}
- /* 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));
- }
-
+ /* If priority string is not constructed yet, construct and finalize */
+ if (!system_wide_config.priority_string) {
+ 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));
+ }
}
do {
ss_next = strchr(ss, ',');
if (ss_next) {

44
gnutls-3.7.3-max-algos.patch Normal file
View File

@ -0,0 +1,44 @@
From b5a2cbce49d94a04a68acbbc31caaa0c5d7b3321 Mon Sep 17 00:00:00 2001
From: Alexander Sosedkin <asosedkin@redhat.com>
Date: Fri, 18 Feb 2022 11:05:15 +0100
Subject: [PATCH] bump GNUTLS_MAX_ALGORITHM_NUM / MAX_ALGOS
Fedora 36 LEGACY crypto-policy uses allowlisting format
and is long enough to blow past the 64 priority string
elements mark, causing, effectively, priority string truncation.
Signed-off-by: Alexander Sosedkin <asosedkin@redhat.com>
---
lib/includes/gnutls/gnutls.h.in | 2 +-
lib/priority.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/includes/gnutls/gnutls.h.in b/lib/includes/gnutls/gnutls.h.in
index 6359a0edb6..16140c8787 100644
--- a/lib/includes/gnutls/gnutls.h.in
+++ b/lib/includes/gnutls/gnutls.h.in
@@ -408,7 +408,7 @@ typedef enum {
/* exported for other gnutls headers. This is the maximum number of
* algorithms (ciphers, kx or macs).
*/
-#define GNUTLS_MAX_ALGORITHM_NUM 64
+#define GNUTLS_MAX_ALGORITHM_NUM 128
#define GNUTLS_MAX_SESSION_ID_SIZE 32
diff --git a/lib/priority.c b/lib/priority.c
index 54d7b1bb45..e7698ba7eb 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -43,7 +43,7 @@
#include "profiles.h"
#include "name_val_array.h"
-#define MAX_ELEMENTS 64
+#define MAX_ELEMENTS GNUTLS_MAX_ALGORITHM_NUM
#define ENABLE_PROFILE(c, profile) do { \
c->additional_verify_flags &= 0x00ffffff; \
--
2.34.1

23
gnutls.spec
View File

@ -13,17 +13,22 @@ print(string.sub(hash, 0, 16))
}
Version: 3.7.3
Release: 6%{?dist}
Release: 7%{?dist}
Patch1: gnutls-3.6.7-no-now-guile.patch
Patch2: gnutls-3.2.7-rpath.patch
Patch3: gnutls-3.7.2-enable-intel-cet.patch
Patch4: gnutls-3.7.2-no-explicit-init.patch
Patch5: gnutls-3.7.3-disable-config-reload.patch
Patch6: gnutls-3.7.3-fips-rsa-keygen.patch
Patch7: gnutls-3.7.3-ktls-stub.patch
Patch8: gnutls-3.7.3-fips-pkcs12.patch
Patch9: gnutls-3.7.3-fix-tests-in-fips.patch
Patch10: gnutls-3.7.3-gost-ifdef.patch
Patch5: gnutls-3.7.3-fips-rsa-keygen.patch
Patch6: gnutls-3.7.3-ktls-stub.patch
Patch7: gnutls-3.7.3-fips-pkcs12.patch
Patch8: gnutls-3.7.3-fix-tests-in-fips.patch
Patch9: gnutls-3.7.3-gost-ifdef.patch
Patch10: gnutls-3.7.3-max-algos.patch
Patch11: gnutls-3.7.3-allowlist-api.patch
# not upstreamed
Patch100: gnutls-3.7.3-disable-config-reload.patch
%bcond_with bootstrap
%bcond_without dane
%if 0%{?rhel}
@ -337,6 +342,10 @@ make check %{?_smp_mflags} GNUTLS_SYSTEM_PRIORITY_FILE=/dev/null
%endif
%changelog
* Wed Feb 23 2022 Daiki Ueno <dueno@redhat.com> - 3.7.3-7
- Increase GNUTLS_MAX_ALGORITHM_NUM for allowlisting (#2033220)
- Ensure allowlisting API is called before priority string is constructed (#2042532)
* Tue Feb 22 2022 Daiki Ueno <dueno@redhat.com> - 3.7.3-6
- Compile out GOST algorithm IDs (#1945292)