Fix race condition when resolving SYSTEM priority in allowlisting

Resolves: #2012249
Signed-off-by: Daiki Ueno <dueno@redhat.com>
This commit is contained in:
Daiki Ueno 2021-10-26 13:02:28 +02:00
parent 8daf88e559
commit f86e12e4cb
2 changed files with 257 additions and 1 deletions

View File

@ -0,0 +1,254 @@
From dbdcc29ee9e31acaa8286f633a4f0c23abd09d03 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Tue, 26 Oct 2021 12:56:52 +0200
Subject: [PATCH] priority: fix race condition when resolving SYSTEM in
allowlisting
Signed-off-by: Daiki Ueno <ueno@gnu.org>
---
lib/priority.c | 65 +++++++++++++++++++++++++++++++-------------------
1 file changed, 41 insertions(+), 24 deletions(-)
diff --git a/lib/priority.c b/lib/priority.c
index 20230e46d1..606443f1f9 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -39,6 +39,7 @@
#include "profiles.h"
#include "c-strcase.h"
#include "inih/ini.h"
+#include "locks.h"
#include "profiles.h"
#include "name_val_array.h"
@@ -1001,6 +1002,7 @@ static void dummy_func(gnutls_priority_t c)
#include <priority_options.h>
static gnutls_certificate_verification_profiles_t system_wide_verification_profile = GNUTLS_PROFILE_UNKNOWN;
+GNUTLS_STATIC_MUTEX(system_wide_priority_strings_mutex);
static name_val_array_t system_wide_priority_strings = NULL;
static char *system_wide_priority_string = NULL;
static unsigned system_wide_priority_strings_init = 0;
@@ -1727,6 +1729,9 @@ static int cfg_ini_handler(void *ctx, const char *section, const char *name, con
return 1;
}
+static int
+resolve_priorities_from_system_wide_allowlisting(void);
+
static void _gnutls_update_system_priorities(void)
{
int ret;
@@ -1734,17 +1739,19 @@ static void _gnutls_update_system_priorities(void)
FILE *fp;
struct cfg cfg;
+ GNUTLS_STATIC_MUTEX_LOCK(system_wide_priority_strings_mutex);
+
if (stat(system_priority_file, &sb) < 0) {
_gnutls_debug_log("cfg: unable to access: %s: %d\n",
system_priority_file, errno);
- return;
+ goto out;
}
if (system_wide_priority_strings_init != 0 &&
sb.st_mtime == system_priority_last_mod) {
_gnutls_debug_log("cfg: system priority %s has not changed\n",
system_priority_file);
- return;
+ goto out;
}
if (system_wide_priority_strings_init != 0)
@@ -1757,7 +1764,7 @@ static void _gnutls_update_system_priorities(void)
if (fp == NULL) {
_gnutls_debug_log("cfg: unable to open: %s: %d\n",
system_priority_file, errno);
- return;
+ 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
@@ -1781,16 +1788,30 @@ static void _gnutls_update_system_priorities(void)
system_priority_file, ret);
if (fail_on_invalid_config)
exit(1);
- return;
+ goto out;
}
cfg_apply(&cfg);
cfg_deinit(&cfg);
+ if (system_wide_allowlisting) {
+ ret = resolve_priorities_from_system_wide_allowlisting();
+ if (ret < 0) {
+ _gnutls_debug_log("cfg: unable to resolve system priority string: %s\n",
+ gnutls_strerror(ret));
+ if (fail_on_invalid_config)
+ exit(1);
+ goto out;
+ }
+ }
+
_gnutls_debug_log("cfg: loaded system priority %s mtime %lld\n",
system_priority_file,
(unsigned long long)sb.st_mtime);
system_priority_last_mod = sb.st_mtime;
+
+ out:
+ GNUTLS_STATIC_MUTEX_UNLOCK(system_wide_priority_strings_mutex);
}
void _gnutls_load_system_priorities(void)
@@ -1835,17 +1856,13 @@ const char *gnutls_get_system_config_file(void)
return NULL;
}
-static const char *
+static int
resolve_priorities_from_system_wide_allowlisting(void)
{
gnutls_buffer_st buf;
int ret;
size_t i;
- if (system_wide_priority_string) {
- return system_wide_priority_string;
- }
-
assert(system_wide_allowlisting);
_gnutls_buffer_init(&buf);
@@ -1853,21 +1870,21 @@ resolve_priorities_from_system_wide_allowlisting(void)
ret = _gnutls_buffer_append_str(&buf, "NONE");
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
for (i = 0; system_wide_tls_kxs[i] != 0; i++) {
ret = _gnutls_buffer_append_str(&buf, ":+");
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
ret = _gnutls_buffer_append_str(&buf,
gnutls_kx_get_name(system_wide_tls_kxs[i]));
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
}
@@ -1875,14 +1892,14 @@ resolve_priorities_from_system_wide_allowlisting(void)
ret = _gnutls_buffer_append_str(&buf, ":+GROUP-");
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
ret = _gnutls_buffer_append_str(&buf,
gnutls_group_get_name(system_wide_tls_groups[i]));
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
}
@@ -1890,14 +1907,14 @@ resolve_priorities_from_system_wide_allowlisting(void)
ret = _gnutls_buffer_append_str(&buf, ":+");
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
ret = _gnutls_buffer_append_str(&buf,
gnutls_cipher_get_name(system_wide_tls_ciphers[i]));
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
}
@@ -1905,14 +1922,14 @@ resolve_priorities_from_system_wide_allowlisting(void)
ret = _gnutls_buffer_append_str(&buf, ":+");
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
ret = _gnutls_buffer_append_str(&buf,
gnutls_mac_get_name(system_wide_tls_macs[i]));
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
}
@@ -1920,14 +1937,14 @@ resolve_priorities_from_system_wide_allowlisting(void)
ret = _gnutls_buffer_append_str(&buf, ":+SIGN-");
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
ret = _gnutls_buffer_append_str(&buf,
gnutls_sign_get_name(system_wide_tls_sigs[i]));
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
}
@@ -1935,14 +1952,14 @@ resolve_priorities_from_system_wide_allowlisting(void)
ret = _gnutls_buffer_append_str(&buf, ":+VERS-");
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
ret = _gnutls_buffer_append_str(&buf,
gnutls_protocol_get_name(system_wide_tls_vers[i]));
if (ret < 0) {
_gnutls_buffer_clear(&buf);
- return NULL;
+ return ret;
}
}
@@ -1950,7 +1967,7 @@ resolve_priorities_from_system_wide_allowlisting(void)
system_wide_priority_string = gnutls_strdup((char *)buf.data);
_gnutls_buffer_clear(&buf);
- return system_wide_priority_string;
+ return ret;
}
#define S(str) ((str!=NULL)?str:"")
@@ -2010,7 +2027,7 @@ char *_gnutls_resolve_priorities(const char* priorities)
if (system_wide_allowlisting &&
ss_len == sizeof(LEVEL_SYSTEM) - 1 &&
strncmp(LEVEL_SYSTEM, ss, ss_len) == 0) {
- p = resolve_priorities_from_system_wide_allowlisting();
+ p = system_wide_priority_string;
} else {
p = _name_val_array_value(system_wide_priority_strings, ss, ss_len);
}
--
2.31.1

View File

@ -7,6 +7,7 @@ Patch3: gnutls-3.7.2-config-allowlisting.patch
Patch4: gnutls-3.7.2-key-share-ecdhx.patch
Patch5: gnutls-3.7.2-enable-intel-cet.patch
Patch6: gnutls-3.7.2-libopts-covscan.patch
Patch7: gnutls-3.7.2-config-allowlisting-race.patch
%bcond_with bootstrap
%bcond_without dane
%if 0%{?rhel}
@ -322,8 +323,9 @@ make check %{?_smp_mflags} GNUTLS_SYSTEM_PRIORITY_FILE=/dev/null
%endif
%changelog
* Thu Oct 21 2021 Daiki Ueno <dueno@redhat.com> - 3.7.2-8
* Tue Oct 26 2021 Daiki Ueno <dueno@redhat.com> - 3.7.2-8
- Fix issues in bundled libopts, spotted by covscan (#1938730)
- Fix race condition when resolving SYSTEM priority in allowlisting mode (#2012249)
* Tue Oct 12 2021 Daiki Ueno <dueno@redhat.com> - 3.7.2-7
- Enable Intel CET