From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 15 Jul 2019 22:52:05 +0300 From: Konstantin Osipov Subject: Re: [PATCH v2] auth: fix empty password authentication Message-ID: <20190715195205.GC4099@atlas> References: <704d9c5686cb5bacfa53a7459a2eea411812bcc5.1563207875.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <704d9c5686cb5bacfa53a7459a2eea411812bcc5.1563207875.git.vdavydov.dev@gmail.com> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * Vladimir Davydov [19/07/15 19:27]: > 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 > --- > https://github.com/tarantool/tarantool/issues/4327 > https://github.com/tarantool/tarantool/tree/dv/gh-4327-fix-empty-password-auth > > Changes in v2: > - Don't change the way net.box treats absense of a password. > Just fix the issue in question. > > v1: https://www.freelists.org/post/tarantool-patches/PATCH-auth-fix-empty-password-authentication > > src/box/authentication.cc | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/src/box/authentication.cc b/src/box/authentication.cc > index 811974cb..b0459a5b 100644 > --- a/src/box/authentication.cc > +++ b/src/box/authentication.cc > @@ -33,8 +33,13 @@ > #include "session.h" > #include "msgpuck.h" > #include "error.h" > +#include "third_party/base64.h" > > -static char zero_hash[SCRAMBLE_SIZE]; > +/** > + * 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? > > 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. -- Konstantin Osipov, Moscow, Russia