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.
This commit is contained in:
Willy Tarreau 2018-10-29 18:02:54 +01:00
parent 744de5b52a
commit 34d4b525a1
2 changed files with 9 additions and 0 deletions

View File

@ -386,6 +386,7 @@ enum lock_label {
PIPES_LOCK,
START_LOCK,
TLSKEYS_REF_LOCK,
AUTH_LOCK,
LOCK_LABELS
};
struct lock_stat {
@ -501,6 +502,7 @@ static inline const char *lock_label(enum lock_label label)
case PIPES_LOCK: return "PIPES";
case START_LOCK: return "START";
case TLSKEYS_REF_LOCK: return "TLSKEYS_REF";
case AUTH_LOCK: return "AUTH";
case LOCK_LABELS: break; /* keep compiler happy */
};
/* only way to come here is consecutive to an internal bug */

View File

@ -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