Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.5.0 breaks uri's with username/password #1038

Closed
kirbycool opened this issue Oct 15, 2021 · 20 comments · Fixed by #1041
Closed

4.5.0 breaks uri's with username/password #1038

kirbycool opened this issue Oct 15, 2021 · 20 comments · Fixed by #1041

Comments

@kirbycool
Copy link

kirbycool commented Oct 15, 2021

Hi,

I haven't had time to fully debug this, but noticed that uri's with username/password seem broken in the 4.5.0 version.

Steps I ran to repro

> Redis.new(url: 'redis://someuser:somepassword@a.redis.server.example.com:1234').info
Traceback (most recent call last):
        1: from (irb):20
Redis::CommandError (WRONGPASS invalid username-password pair)

I can connect with the same url using redis-cli

> redis-cli -u redis://someuser:somepassword@a.redis.server.example.com:1234
Warning: Using a password with '-a' or '-u' option on the command line interface may not be safe.
a.redis.server.example.com:1234>

I can try to provide more info if needed. Thanks!

@tadejm
Copy link

tadejm commented Oct 15, 2021

Seeing the same after upgrading to 4.5.0.

@byroot
Copy link
Collaborator

byroot commented Oct 15, 2021

What version of the redis-server are you running?

@olleolleolle
Copy link
Contributor

olleolleolle commented Oct 15, 2021

Is this related? #1025

(I'm on Redis 6.0.3, and seeing this.)

https://my.diffend.io/gems/redis/4.4.0/4.5.0 I was on 4.4.0, and locked my gem to that, to have the functionality back.

(We are using url, via the MessageBus gem from discourse, incidentally.)

This works in 4.4.0 and not in 4.5.0:

Redis.new(url: "redis://rediscloud:mypass@pub-redis-12345.eu-west-1-1.2.ec2.garantiadata.com:12345").info

@fatkodima
Copy link
Contributor

Can you try passing :url (not :uri)? Works for me on redis 6.2 and correct option. With incorrect option - seeing the same error as you.

@kirbycool
Copy link
Author

kirbycool commented Oct 15, 2021

@fatkodima Sorry, :uri was a typo. I was using the correct :url option. I've updated the ticket description.

The redis server is running 6.0.9.

@casperisfine
Copy link

I'm sorry but I can't repro:

$ redis-cli INFO | grep version
redis_version:6.0.16
gcc_version:4.2.1
$ redis-cli ACL SETUSER foo '>bar' on allkeys allcommands
OK
$ redis-cli -u redis://foo:bar@localhost SET key value
Warning: Using a password with '-a' or '-u' option on the command line interface may not be safe.
OK
$ bundle exec ruby -Ilib -r redis -e 'p Redis.new(url: "redis://foo:bar@localhost").get("key")'
"value"

If you are able to repo, could you modify the gem code so that you see what the AUTH command look like?

Also is there any non-alphanumeric character in your password? Could be an URL escaping issue.

@allisonphillips
Copy link

I'm wondering if this change could have caused it: 17ba094

Prior to that change, a WRONGPASS CommandError would have been caught and auth would be attempted again as AUTH password (and prior to adding ACL support for versions < 6, auth was attempted as AUTH password). If the server is using the default user but an incorrect username is being passed to auth, it seems like that commit could prevent auth from succeeding and result in this error.

@casperisfine
Copy link

Yes, it's likely as it's the only auth related change. It's just weird I can't repro. But it seems that 6.0.16 accepts two arguments to the AUTH command, maybe 6.0.3 doesn't? I'll try to see if I can compile that exact version.

@kirbycool
Copy link
Author

Yeah, I'm pretty sure that's it. I can connect if I leave off the username.

@casperisfine
Copy link

Erf, yes that's it....

Ok, I'll work on a fix. But this sucks, we test multiple redis server versions for this exact reason, I didn't except such difference in a tiny release like this.

@casperisfine
Copy link

Ah nevermind, I forgot to recreate the user. Still can't repro even with 6.0.3

@olleolleolle
Copy link
Contributor

In My password, I had numbers and alphanumeric

@casperisfine
Copy link

Yeah, I'm pretty sure that's it. I can connect if I leave off the username.

Ah, so the password is valid, but the username doesn't correspond to anything? That's what you mean? I suppose you get this URL form some kind of redis as a service?

@casperisfine
Copy link

I suppose I can re-introduce the old behavior of falling back to connecting without password, but that's quite dirty, in some way I feel like I'm re-introducing a bug.

@kirbycool
Copy link
Author

kirbycool commented Oct 15, 2021

Ah, so the password is valid, but the username doesn't correspond to anything? That's what you mean? I suppose you get this URL form some kind of redis as a service?

I'm looking at an existing server which was configured with this url, so I'm not entirely sure how it was constructed and didn't think to look at it too closely since it worked with redis-cli. But clearly the url is incorrect if the username is invalid and it just happened to work due to the old fallback behavior.

It could be fine to consider this a bugfix and leave it in, but other people might also be bitten by this so maybe it's safer to consider this a breaking change and put it in a major version. That's up to the project maintainers.

I appreciate you taking a look at this so quickly and let me know if you need any more info or help. 👍

@casperisfine
Copy link

other people might also be bitten by this

Yeah, I'll see if I can cut a release which restore the behavior but emit some kind of warning.

@allisonphillips
Copy link

Yeah, I'm pretty sure that's it. I can connect if I leave off the username.

Ah, so the password is valid, but the username doesn't correspond to anything? That's what you mean? I suppose you get this URL form some kind of redis as a service?

Correct. I think our rediscloud URL was probably initially generated back when we were on heroku, which generates a URL with rediscloud as the username: https://devcenter.heroku.com/articles/rediscloud#getting-started
But the actual instance is configured to use the default user. Because username was not passed to auth prior to the ACL changes, the URL worked fine until the change to restrict catching only one specific CommandError was released.

@casperisfine
Copy link

PR is here: #1041

I have to go away from my keyboard for a bit, but I'll cut a 4.5.1 as soon as I can assuming CI is green (couple hours max).

@byroot
Copy link
Collaborator

byroot commented Oct 15, 2021

4.5.1 is out https://rubygems.org/gems/redis/versions/4.5.1

@andreas-venturini
Copy link

For anyone using rediscloud there is a section in their docs explaning how you can create role based access (and disable the default user) https://docs.redis.com/latest/rs/security/passwords-users-roles/#configuring-roles-and-users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants