Use of crpyt() is not thread safe (#1643941)

This commit is contained in:
Ryan O'Hara 2018-12-01 13:59:26 -06:00
parent 978a1ad105
commit 73dccb20ca
2 changed files with 90 additions and 1 deletions

View File

@ -0,0 +1,83 @@
From a873c161d251abd025008034c0ddef8cd7f39511 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <w@1wt.eu>
---
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 <types/global.h>
#include <common/config.h>
#include <common/errors.h>
+#include <common/hathreads.h>
#include <proto/acl.h>
#include <proto/log.h>
@@ -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

View File

@ -8,7 +8,7 @@
Name: haproxy Name: haproxy
Version: 1.8.14 Version: 1.8.14
Release: 1%{?dist} Release: 2%{?dist}
Summary: HAProxy reverse proxy for high availability environments Summary: HAProxy reverse proxy for high availability environments
Group: System Environment/Daemons Group: System Environment/Daemons
@ -22,6 +22,8 @@ Source3: %{name}.logrotate
Source4: %{name}.sysconfig Source4: %{name}.sysconfig
Source5: halog.1 Source5: halog.1
Patch0: 0001-BUG-MEDIUM-auth-threads-use-of-crypt-not-thread-safe.patch
BuildRequires: gcc BuildRequires: gcc
BuildRequires: lua-devel BuildRequires: lua-devel
BuildRequires: pcre-devel BuildRequires: pcre-devel
@ -51,6 +53,7 @@ availability environments. Indeed, it can:
%prep %prep
%setup -q %setup -q
%patch0 -p1
%build %build
regparm_opts= regparm_opts=
@ -135,6 +138,9 @@ exit 0
%{_mandir}/man1/* %{_mandir}/man1/*
%changelog %changelog
* Sat Dec 01 2018 Ryan O'Hara <rohara@redhat.com> - 1.8.14-2
- Use of crpyt() is not thread safe (#1643941)
* Thu Sep 20 2018 Ryan O'Hara <rohara@redhat.com> - 1.8.14-1 * Thu Sep 20 2018 Ryan O'Hara <rohara@redhat.com> - 1.8.14-1
- Update to 1.8.14 (#1610066) - Update to 1.8.14 (#1610066)