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

Connection discards user name #252

Closed
ghost opened this issue Apr 27, 2021 · 2 comments
Closed

Connection discards user name #252

ghost opened this issue Apr 27, 2021 · 2 comments

Comments

@ghost
Copy link

ghost commented Apr 27, 2021

Connection discards user name

Reproduce the issue

The steps below assume bash, gpg, Docker and docker-compose are installed.

Starting from carmine root directory

git clone https://github.com/ptaoussanis/carmine.git
cd carmine

Set up a docker-compose file

cat << EOF > docker-compose.yml
---
services:
  redis:
    image: redis:6.2.2-buster
    container_name: carmine-test
    network_mode: "host"
    volumes:
      - type: bind
        source: ./redis.conf
        target: /usr/local/etc/redis/redis.conf
    command: [ "redis-server", "/usr/local/etc/redis/redis.conf" ]
EOF

Download Redis default configuration

curl -o redis.conf 'https://raw.githubusercontent.com/redis/redis/6.0/redis.conf'

Remove Redis protected mode

sed -i 's/^protected-mode yes$/protected-mode no' redis.conf

Add a password on user default. This user is created by Redis.

echo "user default on +@all ~* >$(gpg --gen-random --armor 2 32 | tr -d '=/:@' )" >> redis.conf

Add another user srv_usr with the same password

tail -n 1 redis.conf | sed 's/ default / srv_usr /' >> redis.conf

Put the password in redis-cli.env

echo "REDISCLI_AUTH=$(grep 'user srv_usr' redis.conf | grep -Eo '>.+' | sed 's/^>//')" > redis-cli.env

Start Redis Docker container

docker-compose up

Test the connection

  • Test connection with Redis CLI on user default without password
docker run --rm -it --net=host redis redis-cli -h localhost -p 6379 ping 
(error) NOAUTH Authentication required.
  • Test connection with Redis CLI on user default with password
docker run --rm -it --net=host --env-file redis-cli.env redis redis-cli -h localhost -p 6379 ping
PONG
  • Test connection with Redis CLI on user srv_usr without password
docker run --rm -it --net=host redis redis-cli -h localhost -p 6379 --user srv_usr ping 
(error) NOAUTH Authentication required.
  • Test connection with Redis CLI on user default with password
docker run --rm -it --net=host --env-file redis-cli.env redis redis-cli -h localhost -p 6379 --user srv_usr ping
PONG

Good !

Test carmine

Fire up REPL

Replace the password [REDACTED] with yours

$ lein repl
user=> (require '[taoensso.carmine :as car :refer (wcar)])
nil

user=> (def conn {:pool {} :spec {:uri "redis://localhost:6379/"}})
#'user/conn
user=> (car/wcar conn (car/ping))
Execution error (ExceptionInfo) at taoensso.carmine.protocol/get-unparsed-reply (protocol.clj:143).
NOAUTH Authentication required.

user=> (def conn {:pool {} :spec {:uri "redis://default:[REDACTED]@localhost:6379/"}})
#'user/conn
user=> (car/wcar conn (car/ping))
"PONG"

user=> (def conn {:pool {} :spec {:uri "redis://srv_usr:[REDACTED]@localhost:6379/"}})
#'user/conn
user=> (car/wcar conn (car/ping))
"PONG"

Good !

The problem

When I disable the user default, user srv_usr also stop working. However, redis-cli correctly handles srv_usr.

sed -i '/user default on/d' redis.conf
echo 'user default off' >> redis.conf

Restart carmine-test Docker container.

docker-compose down
docker-compose up

Test well-behave redis-CLI

docker run --rm -it --net=host --env-file redis-cli.env redis redis-cli -h localhost -p 6379 --user default ping
Warning: AUTH failed
(error) NOAUTH Authentication required.
docker run --rm -it --net=host --env-file redis-cli.env redis redis-cli -h localhost -p 6379 --user srv_usr ping
[sudo] password for root: 
PONG

VS carmine

lein repl
user=> (require '[taoensso.carmine :as car :refer (wcar)])
nil
user=> (def conn {:pool {} :spec {:uri "redis://default:[REDACTED]@localhost:6379/"}})
#'user/conn
user=> (car/wcar conn (car/ping))
Execution error (ExceptionInfo) at taoensso.carmine.protocol/get-unparsed-reply (protocol.clj:143).
WRONGPASS invalid username-password pair or user is disabled.

user=> (def conn {:pool {} :spec {:uri "redis://srv_usr:[REDACTED]@localhost:6379/"}})
#'user/conn
user=> (car/wcar conn (car/ping))
Execution error (ExceptionInfo) at taoensso.carmine.protocol/get-unparsed-reply (protocol.clj:143).
WRONGPASS invalid username-password pair or user is disabled.

Solution

The namespace taoensso.carmine.connections at commit 1ee8781 destructures the user name, but never uses it afterwards. Hence, Carmine always uses the user default made by Redis instead of the one provided.

At a quick glance, it seems it requires changes to 3 places to get it fixed:

src/taoensso/carmine/connections.clj:218:(def conn-spec
src/taoensso/carmine/connections.clj:77(defn make-new-connection
src/taoensso/carmine/commands.edn:99

I don't want to engage myself into this issue, but I will take a stab at it.

@ptaoussanis
Copy link
Member

Hi there!

Redis support for auth [<username>] <password> was added to Redis in v6, and this isn't yet supported by Carmine.
Would be happy to see a PR to add support. Otherwise will do this myself next time I'm working on Carmine.

At a quick glance, it seems it requires changes to 3 places to get it fixed

Sounds correct to me! (Though please don't modify the commands.edn, that'll be covered by #251)

@ptaoussanis
Copy link
Member

Closing for now since #257 will be merged in forthcoming release 👍

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

Successfully merging a pull request may close this issue.

1 participant