dragonflydb / dragonflydb/dragonfly

Support sub commands in ACLs.

Open
#5,507 1 comment 2 reactions 0 assignees View on GitHub
API-7 bug
Dominant language
C++
Stars
31.5k
Forks
1.3k
Avg merge
1d 10h
Merged PRs (30d)
137

Description

We are interested in switching from Redis 7/8 to Dragonfly. During our POC I noticed Dragonfly does not support sub commands in ACLs.

For example, we use the recommended ACLs to secure access for redis-exporter and sentinel:

```
user monitor on >{{ monitor_password }} -@all +@connection +memory -readonly +strlen +config|get +xinfo +pfcount -quit +zcard +type +xlen -readwrite -command +client -wait +scard +llen +hlen +get +eval +slowlog +cluster|info +cluster|slots +cluster|nodes -hello -echo +info +latency +scan -reset -auth -asking`
user sentinel on >{{ sentinel_password }} &* -@all +multi +slaveof +ping +exec +subscribe +config|rewrite +role +publish +info +client|setname +client|kill +script|kill
```

Executing ACL load generates the following error:

```
127.0.0.1:6379> acl load
(error) ERR Error loading: /etc/dragonfly/users.acl Error materializing acl file
```

It would also be helpful for the error to include the line number and error from trying to execute the specific line. Currently, I have to convert the ACL file to individual commands for ACL SETUSER to find which one is causing the error.

In the case of sub commands, using the monitor user as an example, it generates the following error:

`(error) ERR Unrecognized parameter +CONFIG|GET`

The error is avoidable by changing +config|get to just +config, but this permits a read-only monitor user the ability to "config set" as well.

For now I have modified the above ACLs to the following working config:

```
user monitor on >{{ monitor_password }} -@all +@connection +memory -readonly +strlen +config +xinfo +pfcount -quit +zcard +type +xlen -readwrite -command +client +scard +llen +hlen +get +eval +slowlog +cluster -hello -echo +info +latency +scan -auth
user sentinel on >{{ sentinel_password }} &* -@all +multi +slaveof +ping +exec +subscribe +config +role +publish +info +client +script
```

**Note**: Some commands had to be removed since they are not supported, but because they were -command, for example -reset and -asking, it was non-issue.. just a heads up though.

Contributor guide

Open the contributing guide

Research direction

Start by reproducing ACL LOAD with the monitor and sentinel entries shown in the issue, then trace the ACL parser and materialization path for the failing +CONFIG|GET parameter. Done means subcommand permissions load successfully without broadening access, and ACL syntax errors report the relevant line and underlying command error.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, redis
Domain
authorization, databases, security
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.