* [PATCH v2] auth: fix empty password authentication
@ 2019-07-15 16:26 Vladimir Davydov
2019-07-15 19:52 ` Konstantin Osipov
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Davydov @ 2019-07-15 16:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
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=";
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;
+ }
}
access_check_session_xc(user);
@@ -90,11 +99,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);
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] auth: fix empty password authentication 2019-07-15 16:26 [PATCH v2] auth: fix empty password authentication Vladimir Davydov @ 2019-07-15 19:52 ` Konstantin Osipov 2019-07-17 11:51 ` Vladimir Davydov 0 siblings, 1 reply; 5+ messages in thread From: Konstantin Osipov @ 2019-07-15 19:52 UTC (permalink / raw) To: Vladimir Davydov; +Cc: tarantool-patches * Vladimir Davydov <vdavydov.dev@gmail.com> [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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] auth: fix empty password authentication 2019-07-15 19:52 ` Konstantin Osipov @ 2019-07-17 11:51 ` Vladimir Davydov 2019-07-17 14:28 ` Konstantin Osipov 0 siblings, 1 reply; 5+ messages in thread From: Vladimir Davydov @ 2019-07-17 11:51 UTC (permalink / raw) To: Konstantin Osipov; +Cc: tarantool-patches 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 <vdavydov.dev@gmail.com> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] auth: fix empty password authentication 2019-07-17 11:51 ` Vladimir Davydov @ 2019-07-17 14:28 ` Konstantin Osipov 2019-07-17 14:49 ` Vladimir Davydov 0 siblings, 1 reply; 5+ messages in thread From: Konstantin Osipov @ 2019-07-17 14:28 UTC (permalink / raw) To: Vladimir Davydov; +Cc: tarantool-patches * Vladimir Davydov <vdavydov.dev@gmail.com> [19/07/17 14:54]: lgtm -- Konstantin Osipov, Moscow, Russia ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] auth: fix empty password authentication 2019-07-17 14:28 ` Konstantin Osipov @ 2019-07-17 14:49 ` Vladimir Davydov 0 siblings, 0 replies; 5+ messages in thread From: Vladimir Davydov @ 2019-07-17 14:49 UTC (permalink / raw) To: Konstantin Osipov; +Cc: tarantool-patches On Wed, Jul 17, 2019 at 05:28:37PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov <vdavydov.dev@gmail.com> [19/07/17 14:54]: > > lgtm Pushed to master, 2.1, and 1.10. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-17 14:49 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-15 16:26 [PATCH v2] auth: fix empty password authentication Vladimir Davydov 2019-07-15 19:52 ` Konstantin Osipov 2019-07-17 11:51 ` Vladimir Davydov 2019-07-17 14:28 ` Konstantin Osipov 2019-07-17 14:49 ` Vladimir Davydov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox