Please support redis 6 AUTH with user / password

[cilex-ft]

I installed redis 6.0.11 on my ubuntu server via http://ppa.launchpad.net/redislabs/redis/ubuntu

I enabled /etc/redis/users.acl, added a password to user "default", and defined 2 more users with password:

user default allcommands allkeys on >secret
user alice allcommands allkeys on >ALICE2
user bob  on >bob3 ~bob:* +get

This works perfectly - I can swap users with redis-cli, GET and SET data:

127.0.0.1:6379> AUTH secret
OK
127.0.0.1:6379> SET bob:password xyz
OK
127.0.0.1:6379> AUTH alice ALICE2
OK
127.0.0.1:6379> SET bob:password xyz123
OK
127.0.0.1:6379> AUTH bob bob3
OK
127.0.0.1:6379> GET bob:password
"xyz123"
127.0.0.1:6379> SET bob:password supersecret
(error) NOPERM this user has no permissions to run the 'set' command or its subcommand

I got and compiled Webdis 0.1.13-dev and tried to define redis_auth in webdis.json:

• "secret" is OK
• "alice ALICE2" receives {"GET":[false,"NOAUTH Authentication required."]}

I tried to hack pool.c to have it using the full "user password" string from redis_auth, but didn't succeed. I also tested using redisAsyncCommand(ac, NULL, NULL, "AUTH alice ALICE2"); instead of the current redisAsyncCommand(ac, NULL, NULL, "AUTH %s", p->cfg->redis_auth); and it works well.

Can you please implement Redis 6 AUTH command as defined on https://redis.io/commands/auth?

[nicolasff]

Hi @cilex-ft, thanks for the detailed bug report. I hadn't followed these new developments, this certainly seems like an important feature to add.

I was a bit puzzled at first by your mention that using "AUTH alice ALICE2" worked but "AUTH %s" didn't, since using any printf-style command with %s and the parameter "alice ALICE2" should lead to the same result. But after thinking about it, it does make sense: I think what is happening is that this is just not the same as using a simple sprintf to construct a buffer: think about how this would work with credentials that contain spaces for example, using sprintf would make Redis understand the AUTH command as having multiple parameters instead of just one. So what redisAsyncCommand likely does is build some sort of command structure with a command name and an array of arguments, and just walks over the format string splitting it every time it finds a space and copying the full argument value when it finds a %s.

Actually after writing this I went to check: in hiredis.c on line 232 you can see it checking the input string for a % character, and if there is one followed by s it goes to line 260 where a single new argument newarg is built using the vararg parameter (in your case alice ALICE2), before appending the argument to newargv either when the next space is found on line 235 or at the end of the function on line 384 if we've reached the end of the input.

Meaning that using "alice ALICE2" as the auth value is a bit like constructing the command AUTH "alice ALICE2", not AUTH alice ALICE2. In order to support this feature Webdis would need to call redisAsyncCommand with AUTH %s %s if v6 auth is used.

@cilex-ft what do you think of allowing redis_auth to be either a string or an array of two strings? In the latter case the new auth would be used. An alternative would be to have something like redis_auth_username and redis_auth_password but that seems more complex with no actual benefits.

@jessie-murray do you want to look into this and open a PR with this change? (or propose another alternative I haven't thought of).

Thanks again! This is definitely worth adding soon.

[cilex-ft]

Wow that was a fast answer, thank you!

I was quite puzzled, too, and I found this explanation about hiredis splitting the given string. Makes perfect sense... (and I should have included this link in my issue)

It seems that allowing redis_auth to be either a string or an array of two strings would provide with better backward compatibility with current syntax? It would fit with Redis rationale about ACL:

When it is used according to the old form, that is: AUTH <password> What happens is that the username used to authenticate is "default", so just specifying the password implies that we want to authenticate against the default user. This provides perfect backward compatibility with the past.

[nicolasff]

@cilex-ft would you mind trying the latest master? If this works well for you and once a separate issue (relatively minor) is also addressed I'll tag a new release and will publish new Docker images.

[fredt34]

I'll test it today.

[cilex-ft]

Per Elon : "Time is the ultimate currency"

Sorry I've been awfully long to test & report, but I finally found some hours today, and it works perfectly!

[nicolasff]

Thanks @cilex-ft! Glad it works well for you.

[cilex-ft]

Update: now with #209 fixed, it works with websockets, which was our primary goal.