Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH] auth: fix empty password authentication
Date: Tue, 9 Jul 2019 21:13:30 +0300	[thread overview]
Message-ID: <20190709181330.GB12956@atlas> (raw)
In-Reply-To: <d477dafd5d73c82bba3df0444aba58827aecd2bd.1562680546.git.vdavydov.dev@gmail.com>

* 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

  reply	other threads:[~2019-07-09 18:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-09 13:56 Vladimir Davydov
2019-07-09 18:13 ` Konstantin Osipov [this message]
2019-07-10  8:13   ` Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190709181330.GB12956@atlas \
    --to=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH] auth: fix empty password authentication' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox