From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kostja@tarantool.org>
Date: Tue, 9 Jul 2019 21:13:30 +0300
From: Konstantin Osipov <kostja@tarantool.org>
Subject: Re: [PATCH] auth: fix empty password authentication
Message-ID: <20190709181330.GB12956@atlas>
References: <d477dafd5d73c82bba3df0444aba58827aecd2bd.1562680546.git.vdavydov.dev@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d477dafd5d73c82bba3df0444aba58827aecd2bd.1562680546.git.vdavydov.dev@gmail.com>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

* Vladimir Davydov <vdavydov.dev@gmail.com> [19/07/09 17:01]:

> -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="

Please use a string constant not a define.

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

Please allow both empty password and no password and treat them
the same on server side.

> +	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;

I don't get this change and it's not in the changeset comment. We
should not allow anyone except guest to have an empty password. If
the password is empty, not set, like in unix, it should be
impossible toauthenticate to this user, only sudo to it.

>  	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

this looks very wrong, net.box should work with all versions of
the server.


-- 
Konstantin Osipov, Moscow, Russia