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

Redis-Cluster: Add support to auth in redis-trib.rb #4288

Closed
wants to merge 7 commits into from

Conversation

otherpirate
Copy link

Use --password='secret'
Added in: info, check, create, add-node, del-node, set-timeout, reshard,
rebalance and fix

Issues #2866 and #3389

Google groups: https://groups.google.com/forum/#!topic/redis-db/Z8lMxTfDct8

Use --password='secret'
Added in: info, check, create, add-node, del-node, set-timeout, reshard,
rebalance and fix
@paltryeffort
Copy link

Hi,
is there a reason your fix doesn't work for me?

redis-trib.rb info 10.10.3.4:7001
[OK] 28055 keys in 3 masters.
redis-cli -h 10.10.3.4 -p 7001 config set requirepass "test123"
OK
redis-trib.rb info 10.10.3.4:7001 --password='test123'
[ERR] Sorry, can't connect to node 10.10.3.4:7001

@otherpirate
Copy link
Author

Hi @paltryeffort

Could you please try "redis-trib.rb info --password test123 10.10.3.4:7001"

@paltryeffort
Copy link

That's works. Thank you!

@k19421
Copy link

k19421 commented Oct 31, 2017

Hi @otherpirate
why reshard doesn't work for me?

./redis-trib.rb reshard --password test123 host:8080
........................
Do you want to proceed with the proposed reshard plan (yes/no)? yes
Moving slot 5461 from host:8080 to host:8080:
[ERR] Calling MIGRATE: ERR Target instance replied with error: NOAUTH Authentication required.```

@otherpirate
Copy link
Author

otherpirate commented Oct 31, 2017

Hi @k19421

The problem is in redis migrate command, do not have password parameter by default.

Migrate command is called here

Issue: #2500
Solution: #2501

@otherpirate otherpirate reopened this Oct 31, 2017
@vernak2539
Copy link

vernak2539 commented Nov 23, 2017

So setting up a cluster now and would like to use auth. Is this PR the only way to do it at the moment without manual setup?

@kd-devops
Copy link

kd-devops commented Dec 8, 2017

Just wanted to cross check, weather authentication issue with Migrate has been fixed or its still have same issue.

I am using redis-4.0.6 with redis-trib-pass.rb and still facing issue while doing redis-trib-pass.rb reshard

[ERR] Calling MIGRATE: ERR Target instance replied with error: NOAUTH Authentication required.

I am passing password using auth flag:

Command Executed :

redis-trib-pass.rb reshard --auth host:ip

Weird this is , this error only comes when cluster nodes has keys ...for empty masters slots resharding works fine.

Any suggestion please ?

@otherpirate
Copy link
Author

Hi @dineshk8666

The problem is in redis migrate command, do not have support for password/auth parameter.

Migrate command is called here

Issue: #2500
Solution: #2501

You can also remove auth before do reshard, and reset after reshard

I know it's not the best solution, but unfortunately is the only way I figured out.

@kd-devops
Copy link

Hi @otherpirate

Thanks for the update.

Is there any future solution expected, where auth can be used to reshard slots with auth?

My purpose is to scale-up functional redis cluster and it is already configured with auth.

Thanks

@otherpirate
Copy link
Author

Hi @dineshk8666 as you saw in #4240 and #2507, it's was solved by community.

Probably @antirez will review, merge and publish a new version as soon he can.

@artix75
Copy link
Contributor

artix75 commented Dec 19, 2017

@otherpirate I've reviewed the #2507 and I'll talk to @antirez about this issue. Both PRs are obviously related.
As for the #4288: the implementation it's ok to me, but since the PR is strongly related to the migrate issue, I'd prefer to face the whole issue. Just a note: this implementation assumes that all the cluster nodes have the same password, which can make sense, but maybe it should be clear in the help message.

@otherpirate
Copy link
Author

Thanks for the feedback @artix75
I added the help message ;)

Please let me know if I can help with this issue.

@BrotherGao
Copy link

Hi, @otherpirate
The call doesn't work in your fix!

./redis-trib.rb call --password 123 **.***.**.49:13234 dbsize
Unknown option 'password' for command 'call'

Doesn't your fix support call? I think the call is important in my work. I often use "redis-trib.rb call" to batch modifying each node configuration in cluster.

@otherpirate
Copy link
Author

Hi @BrotherGao

Fixed, thanks!

@damora
Copy link

damora commented Mar 19, 2018

what is the trick for calling this with create?
${REDISHOME}/bin/redis-trib-pw.rb create --password "foobared" --replicas <host:port string....>

10.10.1.12:1500 10.10.11.3:1500 10.10.14.17:1500 10.10.14.18:1500 10.10.15.2:1500 10.10.15.3:1500

Creating cluster
command:[:auth, "foobared"]
[ERR] Sorry, can't connect to node 10.10.1.12:1500

@otherpirate
Copy link
Author

otherpirate commented Mar 20, 2018

Hi @damora,

Are you sure 10.10.1.12:1500 is up? with auth?

The following command works?
redis-cli -h 10.10.1.12 -p 1500 -a foobared

Obs : You should use --password foobared without "quotes"

@chihuo91
Copy link

For add-node command, does it support two password? One for the source node , one for the target node?

@damora
Copy link

damora commented Apr 11, 2018

@otherpirate I got it to work. It was user error..:-(

@otherpirate
Copy link
Author

Hi @chihuo91

No, it do not support more than one password.

I never saw a redis cluster with two (or more) password, just for curiosity, could you please explain to me:

  1. What is the necessity of have two different password?
  2. How your application connect in both nodes?

Regards

@chihuo91
Copy link

@otherpirate I just verified that our application only uses one password.

@chihuo91
Copy link

I have one question. Since you pass the password as command, user can actually use ps to see the password. Is there any way to hide password from ps ?

@chihuo91
Copy link

So what I wanna ask is that is the command accept the secret argument through stdin?

@otherpirate
Copy link
Author

Hi @chihuo91

No, do not support.
This command runs of a few seconds and exit, so the password gonna be "visible" for a small time.
Should your user access PS from redis-cluster host?

@animesks
Copy link

animesks commented Apr 18, 2018

Even I had a similar concern about passwords being visible to any user on the redis host. Ideally, we should read password from standard input if password is not specified as a command line parameter.

We shouldn't say 'Won't Fix' for this issue for the reason that it runs for a few seconds (cause its a security issue after all).

Though one could still write a wrapper script to pass password via stdin, it defeats the purpose of having the original script when a developer needs to write wrapper scripts for provisioning systems in production.

@otherpirate
Copy link
Author

Hi @animesks and @chihuo91

I'm not sure it is the best implementation, but works for me.

Any problem/doubt, please let me know.

@chihuo91
Copy link

chihuo91 commented Apr 19, 2018

Hi @otherpirate

Thanks for fixing. But I got Invalid option: --use-empty-masters error in line 1718 when I try to run rebalance --use-empty-masters. Same as the option --weight. Do you know what might cause this error?

Command ran:
$ echo pwd | /usr/local/rvm/rubies/ruby-2.4.1/bin/ruby redis-trib.rb rebalance --password --use-empty-masters a05230a6=0 192.168.50.51:27104
Error:
redis-trib.rb:1718:in '<main>': invalid option: --use-empty-masters (OptionParser::InvalidOption)

@chihuo91
Copy link

@otherpirate Just curious. Any update for the error we got?

@otherpirate
Copy link
Author

Hi @chihuo91

Sorry for bug and delay :/

It's fixed, please, try again :)

@chihuo91
Copy link

chihuo91 commented Apr 24, 2018

Hi @otherpirate
Thanks for fixing.
Here are come commands I tried and the errors I got. I am not familiar with ruby so if I run the command in a wrong way, please let me know.

  1. First try
ruby redis-trib.rb rebalance --password --use-empty-masters ip:port
Enter with password: Addr is  and password is --use-empty-masters (this is the error message I print out)
redis-trib.rb:62:in `initialize': undefined method `split' for nil:NilClass (NoMethodError)
	from redis-trib.rb:834:in `new'
	from redis-trib.rb:834:in `load_cluster_info_from_node'
	from redis-trib.rb:998:in `rebalance_cluster_cmd'
	from redis-trib.rb:1747:in `<main>'
  1. Second try
ruby redis-trib.rb rebalance --use-empty-masters ip:port
redis-trib.rb:998:in `rebalance_cluster_cmd': undefined method `[]' for nil:NilClass (NoMethodError)
	from redis-trib.rb:1747:in `<main>'

Working on it...

Revert "Add help message for password argument"

This reverts commit 88e1599.
@BrotherGao
Copy link

Hi,@chihuo91
I think you should check your redis version at first. As we all know, redis version(<4.0.7) doesn't support migrate command in the cluster with auth mode. It's useless to execute this script(redis-trib.rb) with the options( ( i. e. , rebalance)) which use migrate command.

@artix75
Copy link
Contributor

artix75 commented Jul 3, 2018

Redis-trib will be deprecated in the next release and replaced by redis-cli, that also supports auth.

@stillerrr
Copy link

Hi @otherpirate
It would be something wrong when adding nodes or delete nodes with password.
If the node has data, the "migrate" command will report error "no auth".
I think the redis-trib.rb need to be changed at the function “move_slot” that input "opt" parameter and add the password when calling "migrate":

source.r.client.call(["migrate",target.info[:host],port,"",0,@timeout,:auth,opt['password'],:keys,*keys])

@antirez
Copy link
Contributor

antirez commented Jul 13, 2018

Support added to redis-cli as @artix75 said. Closing. Thanks.

@antirez antirez closed this Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet