Re: [PATCH v2] auth: fix empty password authentication

[Konstantin Osipov <kostja@tarantool.org>]

* 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