[PATCH] auth: fix empty password authentication

Vladimir Davydov vdavydov.dev at gmail.com
Tue Jul 9 16:56:38 MSK 2019


If a user has an empty password, we should allow to authenticate it
without specifying a password in IPROTO_AUTH request. In particular,
this is required to authenticate as guest from connectors.

However, we only allow to authenticate without a password if the user
has empty hash2 and is guest, which is never the case, because since
commit 076a842011e0 guest has non-empty hash2 calculated from an empty
password.

The above-mentioned commit circumvents the issue in net.box by passing
an empty password to the server if no password is specified. However,
connectors don't usually do that - they simply omit the password field
altogether, which results in authentication failure.

So instead of replacing no-password with empty-password at the client
side (i.e. in net.box) let's allow authenticating without a password
if the user has an empty password.

Fixes commit 076a842011e0 ("Permit empty passwords in net.box").

While we are at it, make sure on_auth triggers are invoked when password
check is skipped - currently they are not, which would otherwise result
in box-tap/auth test hang.

Fixes commit ecb0e698dfb8 ("gh-844: Authentication trigger feature in
iproto").

Note, IPROTO_AUTH must have IPROTO_TUPLE set so netbox_encode_auth is
incorrect - it skips the field if the password isn't specified. Fix it
to encode an empty array instead.

Closes #4327
---
https://github.com/tarantool/tarantool/issues/4327
https://github.com/tarantool/tarantool/commits/dv/gh-4327-fix-empty-password-auth

 src/box/authentication.cc | 28 ++++++++++++++++++++--------
 src/box/lua/net_box.c     |  6 ++++--
 src/box/lua/net_box.lua   |  3 +--
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/box/authentication.cc b/src/box/authentication.cc
index 811974cb..ebc67e96 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)
+ */
+#define CHAP_SHA1_EMPTY_PASSWORD "vhvewKp0tNyweZQ+cFKAlsyphfg="
 
 void
 authenticate(const char *user_name, uint32_t len, const char *salt,
@@ -47,15 +52,21 @@ authenticate(const char *user_name, uint32_t len, const char *salt,
 	const char *scramble;
 	struct on_auth_trigger_ctx auth_res = { user->def->name, true };
 	/*
-	 * Allow authenticating back to GUEST user without
-	 * checking a password. This is useful for connection
+	 * Allow authenticating without a password if the user
+	 * has an empty password. This is useful for connection
 	 * 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) {
+		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;
+		}
+		/* Otherwise treat as password mismatch. */
+		goto err;
 	}
 
 	access_check_session_xc(user);
@@ -85,16 +96,17 @@ authenticate(const char *user_name, uint32_t len, const char *salt,
 	}
 
 	if (scramble_check(scramble, salt, user->def->hash2)) {
+err:
 		auth_res.is_authenticated = false;
 		if (session_run_on_auth_triggers(&auth_res) != 0)
 			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/lua/net_box.c b/src/box/lua/net_box.c
index 001af95d..f1c71c2f 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -141,16 +141,18 @@ netbox_encode_auth(lua_State *L)
 		return luaL_error(L, "Invalid salt");
 
 	/* Adapted from xrow_encode_auth() */
-	mpstream_encode_map(&stream, password != NULL ? 2 : 1);
+	mpstream_encode_map(&stream, 2);
 	mpstream_encode_uint(&stream, IPROTO_USER_NAME);
 	mpstream_encode_strn(&stream, user, user_len);
+	mpstream_encode_uint(&stream, IPROTO_TUPLE);
 	if (password != NULL) { /* password can be omitted */
 		char scramble[SCRAMBLE_SIZE];
 		scramble_prepare(scramble, salt, password, password_len);
-		mpstream_encode_uint(&stream, IPROTO_TUPLE);
 		mpstream_encode_array(&stream, 2);
 		mpstream_encode_str(&stream, "chap-sha1");
 		mpstream_encode_strn(&stream, scramble, SCRAMBLE_SIZE);
+	} else {
+		mpstream_encode_array(&stream, 0);
 	}
 
 	netbox_encode_request(&stream, svp);
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 2892bb3d..0d486fa6 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -222,7 +222,6 @@ local function create_transport(host, port, user, password, callback,
     if user == nil and password ~= nil then
         box.error(E_PROC_LUA, 'net.box: user is not defined')
     end
-    if user ~= nil and password == nil then password = '' end
 
     -- Current state machine's state.
     local state            = 'initial'
@@ -712,7 +711,7 @@ local function create_transport(host, port, user, password, callback,
 
     iproto_auth_sm = function(salt)
         set_state('auth')
-        if not user or not password then
+        if not user then
             set_state('fetch_schema')
             return iproto_schema_sm()
         end
-- 
2.11.0




More information about the Tarantool-patches mailing list