|
 |
73dccb2 |
From a873c161d251abd025008034c0ddef8cd7f39511 Mon Sep 17 00:00:00 2001
|
|
|
73dccb2 |
From: Willy Tarreau <w@1wt.eu>
|
|
|
73dccb2 |
Date: Mon, 29 Oct 2018 18:02:54 +0100
|
|
|
73dccb2 |
Subject: BUG/MEDIUM: auth/threads: use of crypt() is not thread-safe
|
|
|
73dccb2 |
|
|
|
73dccb2 |
It was reported here that authentication may fail when threads are
|
|
|
73dccb2 |
enabled :
|
|
|
73dccb2 |
|
|
|
73dccb2 |
https://bugzilla.redhat.com/show_bug.cgi?id=1643941
|
|
|
73dccb2 |
|
|
|
73dccb2 |
While I couldn't reproduce the issue, it's obvious that there is a
|
|
|
73dccb2 |
problem with the use of the non-reentrant crypt() function there.
|
|
|
73dccb2 |
On Linux systems there's crypt_r() but not on the vast majority of
|
|
|
73dccb2 |
other ones. Thus a first approach consists in placing a lock around
|
|
|
73dccb2 |
this crypt() call. Another patch may relax it when crypt_r() is
|
|
|
73dccb2 |
available.
|
|
|
73dccb2 |
|
|
|
73dccb2 |
This fix must be backported to 1.8. Thanks to Ryan O'Hara for the
|
|
|
73dccb2 |
quick notification.
|
|
|
73dccb2 |
|
|
|
73dccb2 |
(cherry picked from commit 34d4b525a129baa6f52a930ae629ddb1ba4255c2)
|
|
|
73dccb2 |
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
|
|
73dccb2 |
---
|
|
|
73dccb2 |
include/common/hathreads.h | 2 ++
|
|
|
73dccb2 |
src/auth.c | 7 +++++++
|
|
|
73dccb2 |
2 files changed, 9 insertions(+)
|
|
|
73dccb2 |
|
|
|
73dccb2 |
diff --git a/include/common/hathreads.h b/include/common/hathreads.h
|
|
|
73dccb2 |
index 44bd66d1d..24fb1d1a7 100644
|
|
|
73dccb2 |
--- a/include/common/hathreads.h
|
|
|
73dccb2 |
+++ b/include/common/hathreads.h
|
|
|
73dccb2 |
@@ -373,6 +373,7 @@ enum lock_label {
|
|
|
73dccb2 |
START_LOCK,
|
|
|
73dccb2 |
TLSKEYS_REF_LOCK,
|
|
|
73dccb2 |
PENDCONN_LOCK,
|
|
|
73dccb2 |
+ AUTH_LOCK,
|
|
|
73dccb2 |
LOCK_LABELS
|
|
|
73dccb2 |
};
|
|
|
73dccb2 |
struct lock_stat {
|
|
|
73dccb2 |
@@ -495,6 +496,7 @@ static inline const char *lock_label(enum lock_label label)
|
|
|
73dccb2 |
case START_LOCK: return "START";
|
|
|
73dccb2 |
case TLSKEYS_REF_LOCK: return "TLSKEYS_REF";
|
|
|
73dccb2 |
case PENDCONN_LOCK: return "PENDCONN";
|
|
|
73dccb2 |
+ case AUTH_LOCK: return "AUTH";
|
|
|
73dccb2 |
case LOCK_LABELS: break; /* keep compiler happy */
|
|
|
73dccb2 |
};
|
|
|
73dccb2 |
/* only way to come here is consecutive to an internal bug */
|
|
|
73dccb2 |
diff --git a/src/auth.c b/src/auth.c
|
|
|
73dccb2 |
index a2c689f76..e0fb13522 100644
|
|
|
73dccb2 |
--- a/src/auth.c
|
|
|
73dccb2 |
+++ b/src/auth.c
|
|
|
73dccb2 |
@@ -28,6 +28,7 @@
|
|
|
73dccb2 |
#include <types/global.h>
|
|
|
73dccb2 |
#include <common/config.h>
|
|
|
73dccb2 |
#include <common/errors.h>
|
|
|
73dccb2 |
+#include <common/hathreads.h>
|
|
|
73dccb2 |
|
|
|
73dccb2 |
#include <proto/acl.h>
|
|
|
73dccb2 |
#include <proto/log.h>
|
|
|
73dccb2 |
@@ -37,6 +38,10 @@
|
|
|
73dccb2 |
|
|
|
73dccb2 |
struct userlist *userlist = NULL; /* list of all existing userlists */
|
|
|
73dccb2 |
|
|
|
73dccb2 |
+#ifdef CONFIG_HAP_CRYPT
|
|
|
73dccb2 |
+__decl_hathreads(static HA_SPINLOCK_T auth_lock);
|
|
|
73dccb2 |
+#endif
|
|
|
73dccb2 |
+
|
|
|
73dccb2 |
/* find targets for selected gropus. The function returns pointer to
|
|
|
73dccb2 |
* the userlist struct ot NULL if name is NULL/empty or unresolvable.
|
|
|
73dccb2 |
*/
|
|
|
73dccb2 |
@@ -245,7 +250,9 @@ check_user(struct userlist *ul, const char *user, const char *pass)
|
|
|
73dccb2 |
|
|
|
73dccb2 |
if (!(u->flags & AU_O_INSECURE)) {
|
|
|
73dccb2 |
#ifdef CONFIG_HAP_CRYPT
|
|
|
73dccb2 |
+ HA_SPIN_LOCK(AUTH_LOCK, &auth_lock);
|
|
|
73dccb2 |
ep = crypt(pass, u->pass);
|
|
|
73dccb2 |
+ HA_SPIN_UNLOCK(AUTH_LOCK, &auth_lock);
|
|
|
73dccb2 |
#else
|
|
|
73dccb2 |
return 0;
|
|
|
73dccb2 |
#endif
|
|
|
73dccb2 |
--
|
|
|
73dccb2 |
2.14.4
|
|
|
73dccb2 |
|