From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 17 Jul 2019 14:51:07 +0300 From: Vladimir Davydov Subject: Re: [PATCH v2] auth: fix empty password authentication Message-ID: <20190717115107.nbvubbkhz3d7hmat@esperanza> References: <704d9c5686cb5bacfa53a7459a2eea411812bcc5.1563207875.git.vdavydov.dev@gmail.com> <20190715195205.GC4099@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190715195205.GC4099@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Mon, Jul 15, 2019 at 10:52:05PM +0300, Konstantin Osipov wrote: > > +/** > > + * chap-sha1 of empty string, i.e. > > + * base64_encode(sha1(sha1(""), 0) > > + */ > > +static const char *CHAP_SHA1_EMPTY_PASSWORD = "vhvewKp0tNyweZQ+cFKAlsyphfg="; > > Do you insist on copy-pasting it from alter.cc? Thought you wouldn't notice :-) > > void > > authenticate(const char *user_name, uint32_t len, const char *salt, > > @@ -52,10 +57,14 @@ authenticate(const char *user_name, uint32_t len, const char *salt, > > * pooling. > > */ > > part_count = mp_decode_array(&tuple); > > - if (part_count == 0 && user->def->uid == GUEST && > > - memcmp(user->def->hash2, zero_hash, SCRAMBLE_SIZE) == 0) { > > - /* No password is set for GUEST, OK. */ > > - goto ok; > > + if (part_count == 0 && user->def->uid == GUEST) { > > + char hash2[SCRAMBLE_SIZE]; > > + base64_decode(CHAP_SHA1_EMPTY_PASSWORD, SCRAMBLE_BASE64_SIZE, > > + hash2, SCRAMBLE_SIZE); > > + if (memcmp(user->def->hash2, hash2, SCRAMBLE_SIZE) == 0) { > > + /* Empty password is set, OK. */ > > + goto ok; > > + } > > I think we're still misunderstanding each other. Both zero hash > and empty string should work. We should become more permissive, > not less. Sorry for misunderstanding. Fixed. See the new patch below and on the branch. >From c185a3872dc2cd86a383e72d9544ec5b97a9dc8d Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 17 Jul 2019 14:41:26 +0300 Subject: [PATCH] auth: fix empty password authentication We are supposed to authenticate guest user without a password. This used to work before commit 076a842011e0 ("Permit empty passwords in net.box"), when guest didn't have any password. Now it has an empty password and the check in authenticate turns out to be broken, which breaks assumptions made by certain connectors. This patch fixes the check. Closes #4327 diff --git a/src/box/alter.cc b/src/box/alter.cc index 1dbfe6b2..f98a77a5 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -58,12 +58,6 @@ #include "sequence.h" #include "sql.h" -/** - * chap-sha1 of empty string, i.e. - * base64_encode(sha1(sha1(""), 0) - */ -#define CHAP_SHA1_EMPTY_PASSWORD "vhvewKp0tNyweZQ+cFKAlsyphfg=" - /* {{{ Auxiliary functions and methods. */ static void diff --git a/src/box/authentication.cc b/src/box/authentication.cc index 811974cb..fdad7395 100644 --- a/src/box/authentication.cc +++ b/src/box/authentication.cc @@ -33,6 +33,7 @@ #include "session.h" #include "msgpuck.h" #include "error.h" +#include "third_party/base64.h" static char zero_hash[SCRAMBLE_SIZE]; @@ -52,10 +53,14 @@ authenticate(const char *user_name, uint32_t len, const char *salt, * pooling. */ part_count = mp_decode_array(&tuple); - if (part_count == 0 && user->def->uid == GUEST && - memcmp(user->def->hash2, zero_hash, SCRAMBLE_SIZE) == 0) { - /* No password is set for GUEST, OK. */ - goto ok; + if (part_count == 0 && user->def->uid == GUEST) { + if (memcmp(user->def->hash2, zero_hash, SCRAMBLE_SIZE) == 0) + goto ok; /* no password is set, OK */ + char hash2[SCRAMBLE_SIZE]; + base64_decode(CHAP_SHA1_EMPTY_PASSWORD, SCRAMBLE_BASE64_SIZE, + hash2, SCRAMBLE_SIZE); + if (memcmp(user->def->hash2, hash2, SCRAMBLE_SIZE) == 0) + goto ok; /* empty password is set, OK */ } access_check_session_xc(user); @@ -90,11 +95,11 @@ authenticate(const char *user_name, uint32_t len, const char *salt, diag_raise(); tnt_raise(ClientError, ER_PASSWORD_MISMATCH, user->def->name); } +ok: /* check and run auth triggers on success */ if (! rlist_empty(&session_on_auth) && session_run_on_auth_triggers(&auth_res) != 0) diag_raise(); -ok: credentials_init(&session->credentials, user->auth_token, user->def->uid); } diff --git a/src/box/user_def.c b/src/box/user_def.c index 7d783771..4d9821a2 100644 --- a/src/box/user_def.c +++ b/src/box/user_def.c @@ -29,6 +29,9 @@ * SUCH DAMAGE. */ #include "user_def.h" + +const char *CHAP_SHA1_EMPTY_PASSWORD = "vhvewKp0tNyweZQ+cFKAlsyphfg="; + const char * priv_name(user_access_t access) { diff --git a/src/box/user_def.h b/src/box/user_def.h index 8bf31c2f..98097c39 100644 --- a/src/box/user_def.h +++ b/src/box/user_def.h @@ -39,6 +39,11 @@ extern "C" { #endif /* defined(__cplusplus) */ +/** + * chap-sha1 of empty string, i.e. base64_encode(sha1(sha1(""), 0) + */ +extern const char *CHAP_SHA1_EMPTY_PASSWORD; + typedef uint16_t user_access_t; /** * Effective session user. A cache of user data