From 73dccb20ca40e9614a0070c117f9eaffdfae672b Mon Sep 17 00:00:00 2001 From: Ryan O'Hara Date: Sat, 1 Dec 2018 13:59:26 -0600 Subject: [PATCH] Use of crpyt() is not thread safe (#1643941) --- ...threads-use-of-crypt-not-thread-safe.patch | 83 +++++++++++++++++++ haproxy.spec | 8 +- 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 0001-BUG-MEDIUM-auth-threads-use-of-crypt-not-thread-safe.patch diff --git a/0001-BUG-MEDIUM-auth-threads-use-of-crypt-not-thread-safe.patch b/0001-BUG-MEDIUM-auth-threads-use-of-crypt-not-thread-safe.patch new file mode 100644 index 0000000..a0149e5 --- /dev/null +++ b/0001-BUG-MEDIUM-auth-threads-use-of-crypt-not-thread-safe.patch @@ -0,0 +1,83 @@ +From a873c161d251abd025008034c0ddef8cd7f39511 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Mon, 29 Oct 2018 18:02:54 +0100 +Subject: BUG/MEDIUM: auth/threads: use of crypt() is not thread-safe + +It was reported here that authentication may fail when threads are +enabled : + + https://bugzilla.redhat.com/show_bug.cgi?id=1643941 + +While I couldn't reproduce the issue, it's obvious that there is a +problem with the use of the non-reentrant crypt() function there. +On Linux systems there's crypt_r() but not on the vast majority of +other ones. Thus a first approach consists in placing a lock around +this crypt() call. Another patch may relax it when crypt_r() is +available. + +This fix must be backported to 1.8. Thanks to Ryan O'Hara for the +quick notification. + +(cherry picked from commit 34d4b525a129baa6f52a930ae629ddb1ba4255c2) +Signed-off-by: Willy Tarreau +--- + include/common/hathreads.h | 2 ++ + src/auth.c | 7 +++++++ + 2 files changed, 9 insertions(+) + +diff --git a/include/common/hathreads.h b/include/common/hathreads.h +index 44bd66d1d..24fb1d1a7 100644 +--- a/include/common/hathreads.h ++++ b/include/common/hathreads.h +@@ -373,6 +373,7 @@ enum lock_label { + START_LOCK, + TLSKEYS_REF_LOCK, + PENDCONN_LOCK, ++ AUTH_LOCK, + LOCK_LABELS + }; + struct lock_stat { +@@ -495,6 +496,7 @@ static inline const char *lock_label(enum lock_label label) + case START_LOCK: return "START"; + case TLSKEYS_REF_LOCK: return "TLSKEYS_REF"; + case PENDCONN_LOCK: return "PENDCONN"; ++ case AUTH_LOCK: return "AUTH"; + case LOCK_LABELS: break; /* keep compiler happy */ + }; + /* only way to come here is consecutive to an internal bug */ +diff --git a/src/auth.c b/src/auth.c +index a2c689f76..e0fb13522 100644 +--- a/src/auth.c ++++ b/src/auth.c +@@ -28,6 +28,7 @@ + #include + #include + #include ++#include + + #include + #include +@@ -37,6 +38,10 @@ + + struct userlist *userlist = NULL; /* list of all existing userlists */ + ++#ifdef CONFIG_HAP_CRYPT ++__decl_hathreads(static HA_SPINLOCK_T auth_lock); ++#endif ++ + /* find targets for selected gropus. The function returns pointer to + * the userlist struct ot NULL if name is NULL/empty or unresolvable. + */ +@@ -245,7 +250,9 @@ check_user(struct userlist *ul, const char *user, const char *pass) + + if (!(u->flags & AU_O_INSECURE)) { + #ifdef CONFIG_HAP_CRYPT ++ HA_SPIN_LOCK(AUTH_LOCK, &auth_lock); + ep = crypt(pass, u->pass); ++ HA_SPIN_UNLOCK(AUTH_LOCK, &auth_lock); + #else + return 0; + #endif +-- +2.14.4 + diff --git a/haproxy.spec b/haproxy.spec index 7d078c0..cb7e154 100644 --- a/haproxy.spec +++ b/haproxy.spec @@ -8,7 +8,7 @@ Name: haproxy Version: 1.8.14 -Release: 1%{?dist} +Release: 2%{?dist} Summary: HAProxy reverse proxy for high availability environments Group: System Environment/Daemons @@ -22,6 +22,8 @@ Source3: %{name}.logrotate Source4: %{name}.sysconfig Source5: halog.1 +Patch0: 0001-BUG-MEDIUM-auth-threads-use-of-crypt-not-thread-safe.patch + BuildRequires: gcc BuildRequires: lua-devel BuildRequires: pcre-devel @@ -51,6 +53,7 @@ availability environments. Indeed, it can: %prep %setup -q +%patch0 -p1 %build regparm_opts= @@ -135,6 +138,9 @@ exit 0 %{_mandir}/man1/* %changelog +* Sat Dec 01 2018 Ryan O'Hara - 1.8.14-2 +- Use of crpyt() is not thread safe (#1643941) + * Thu Sep 20 2018 Ryan O'Hara - 1.8.14-1 - Update to 1.8.14 (#1610066)