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

fix: acl forward compatible #2459

Merged
merged 22 commits into from
Mar 8, 2024

Conversation

dingxiaoshuai123
Copy link
Collaborator

@dingxiaoshuai123 dingxiaoshuai123 commented Mar 6, 2024

向前兼容 userpass 和 userblacklist ,具体实现方法请看评论。

chejinge and others added 8 commits February 7, 2024 14:45
* fix: codis-dashboard uses 100% cpu(OpenAtomFoundation#2332) (OpenAtomFoundation#2393)

Co-authored-by: liuchengyu <liuchengyu@360.cn>

* fix: The role displayed on the first Server in the Group area of the codis-fe is incorrect (OpenAtomFoundation#2350) (OpenAtomFoundation#2387)

Co-authored-by: liuchengyu <liuchengyu@360.cn>

---------

Co-authored-by: Chengyu Liu <chengyu_l@126.com>
Co-authored-by: liuchengyu <liuchengyu@360.cn>
* fix: codis-dashboard uses 100% cpu(OpenAtomFoundation#2332) (OpenAtomFoundation#2393)

Co-authored-by: liuchengyu <liuchengyu@360.cn>

* fix: The role displayed on the first Server in the Group area of the codis-fe is incorrect (OpenAtomFoundation#2350) (OpenAtomFoundation#2387)

Co-authored-by: liuchengyu <liuchengyu@360.cn>

* fix: automatic fix master-slave replication relationship after master or slave service restarted (OpenAtomFoundation#2373, OpenAtomFoundation#2038, OpenAtomFoundation#1950, OpenAtomFoundation#1967, OpenAtomFoundation#2351)) (OpenAtomFoundation#2386)

Co-authored-by: liuchengyu <liuchengyu@360.cn>

* feat:add 3.5.3 changelog (OpenAtomFoundation#2395)

* add 3.5.3 changelog
---------

Co-authored-by: chejinge <chejinge@360.cn>

---------

Co-authored-by: Chengyu Liu <chengyu_l@126.com>
Co-authored-by: liuchengyu <liuchengyu@360.cn>
Co-authored-by: chejinge <chejinge@360.cn>
@github-actions github-actions bot added the ☢️ Bug Something isn't working label Mar 6, 2024
@lqxhub
Copy link
Collaborator

lqxhub commented Mar 6, 2024

ACL 兼容以前的方案

基本和这个pr的内容一致, 在 pika启动的时候, 除了 要初始化原来 default用户外, 还要判断一下, 配置中的 blacklist 是否为空, 如果不为空, 那么需要再初始化一个 命令受限的用户, 比如 limit. 在启动后, 根据 配置中的 requirepass 来设置 default 用户的 密码, 使用 配置中的 userpass 来设置 limit用户的密码, 后续使用 CONFIG 命令来修改 密码时, 不能修改, 只能使用 ACL 的命令来修改. 使用 ACL命令修改 用户密码时, 也不要 修改 配置中 requirepassuserpass

下面需要讨论待定:

  1. 如果在配置 或者 ACL的配置文件中有多行 limit 相关的配置, 是否和 加载default用户一样, 提示启动失败
  2. 如果同时配置了 limit 用户的 ACL规则 和 blacklist, 以哪个为准, 还是说取两个规则的并集

这地方修改一下:

  1. 默认使用 default 用户代替原来的 admin 用户 来作为 拥有最高权限的用户
  2. 在启动的时候, 判断一下, 如果 blacklist 是空的, 那么久不用初始化 命令受限的用户了

@luky116
Copy link
Collaborator

luky116 commented Mar 7, 2024

ACL 兼容以前的方案

基本和这个pr的内容一致, 在 pika启动的时候, 除了 要初始化原来 default用户外, 还要判断一下, 配置中的 blacklist 是否为空, 如果不为空, 那么需要再初始化一个 命令受限的用户, 比如 limit. 在启动后, 根据 配置中的 requirepass 来设置 default 用户的 密码, 使用 配置中的 userpass 来设置 limit用户的密码, 后续使用 CONFIG 命令来修改 密码时, 不能修改, 只能使用 ACL 的命令来修改. 使用 ACL命令修改 用户密码时, 也不要 修改 配置中 requirepassuserpass

下面需要讨论待定:

  1. 如果在配置 或者 ACL的配置文件中有多行 limit 相关的配置, 是否和 加载default用户一样, 提示启动失败
  2. 如果同时配置了 limit 用户的 ACL规则 和 blacklist, 以哪个为准, 还是说取两个规则的并集

这地方修改一下:

  1. 默认使用 default 用户代替原来的 admin 用户 来作为 拥有最高权限的用户
  2. 在启动的时候, 判断一下, 如果 blacklist 是空的, 那么久不用初始化 命令受限的用户了

1、limit 用户我们现在默认叫“admin”,即pika会内置 “default”和“admin” 这俩用户(命名可以再讨论下)
2、按照之前 pika 的逻辑,requirepass 应该是 admin 用户的,userpass 是 default 用户的。
3、black-list 只针对默认用的用户,即 default 和 admin 用户;black-list 会被翻译成 acl 的语法,就当是用户多配置了几条 acl 规则,不冲突

@lqxhub
Copy link
Collaborator

lqxhub commented Mar 7, 2024

ACL 兼容以前的方案
基本和这个pr的内容一致, 在 pika启动的时候, 除了 要初始化原来 default用户外, 还要判断一下, 配置中的 blacklist 是否为空, 如果不为空, 那么需要再初始化一个 命令受限的用户, 比如 limit. 在启动后, 根据 配置中的 requirepass 来设置 default 用户的 密码, 使用 配置中的 userpass 来设置 limit用户的密码, 后续使用 CONFIG 命令来修改 密码时, 不能修改, 只能使用 ACL 的命令来修改. 使用 ACL命令修改 用户密码时, 也不要 修改 配置中 requirepassuserpass
下面需要讨论待定:

  1. 如果在配置 或者 ACL的配置文件中有多行 limit 相关的配置, 是否和 加载default用户一样, 提示启动失败
  2. 如果同时配置了 limit 用户的 ACL规则 和 blacklist, 以哪个为准, 还是说取两个规则的并集

这地方修改一下:

  1. 默认使用 default 用户代替原来的 admin 用户 来作为 拥有最高权限的用户
  2. 在启动的时候, 判断一下, 如果 blacklist 是空的, 那么久不用初始化 命令受限的用户了

1、limit 用户我们现在默认叫“admin”,即pika会内置 “default”和“admin” 这俩用户 2、按照之前 pika 的逻辑,requirepass 应该是 admin 用户的,userpass 是 default 用户的。 3、black-list 只针对默认用的用户,即 default 和 admin 用户;acl 规则理论上不动这俩默认用户,black-list 会被翻译成 acl 的语法,就当是用户多配置了几条 acl 规则

  1. 我的意思是 用 default 代替 admin, 以后pika中不会有 admin这个用户了, default 是最高权限的用户, 也是默认用户, 这样可以和redis保持一致, 否则, 一些测试用例可能要修改
  2. 这个是没问题的, requirepass是最高权限用户的密码
  3. blacklist 应该是只针对, pika创建的 那个功能受限的用户, 假设叫 limit, ACL规则 理论上不动这俩用户 没懂什么意思, 后续使用 ACL setuser 命令 不能修改 这两个用户吗? ack-list 会被翻译成 acl 的语法,就当是用户多配置了几条ad规则 这个是没问题的, 假设创建的那个功能受限的用户命令是 limit, 在 pika.conf文件里有一条 user : limit on >123 ~* +@all -flush 这个规则, 然后 blacklist中配置了 flushdb,shutdown, 最后再给 limit 设置规则时, 用这两个的并集? 再极端一点, 如果pika.conf 里 user : limit的规则和blacklist里配置的冲突了, 怎么解决, 如果配置了多条怎么解决, 执行了 ACL SAVE 是否把这两个用户的规则 保存, 以及保存后重新加载, 这些都需要考虑

@dingxiaoshuai123
Copy link
Collaborator Author

ACL 兼容以前的方案
基本和这个pr的内容一致, 在 pika启动的时候, 除了 要初始化原来 default用户外, 还要判断一下, 配置中的 blacklist 是否为空, 如果不为空, 那么需要再初始化一个 命令受限的用户, 比如 limit. 在启动后, 根据 配置中的 requirepass 来设置 default 用户的 密码, 使用 配置中的 userpass 来设置 limit用户的密码, 后续使用 CONFIG 命令来修改 密码时, 不能修改, 只能使用 ACL 的命令来修改. 使用 ACL命令修改 用户密码时, 也不要 修改 配置中 requirepassuserpass
下面需要讨论待定:

  1. 如果在配置 或者 ACL的配置文件中有多行 limit 相关的配置, 是否和 加载default用户一样, 提示启动失败
  2. 如果同时配置了 limit 用户的 ACL规则 和 blacklist, 以哪个为准, 还是说取两个规则的并集

这地方修改一下:

  1. 默认使用 default 用户代替原来的 admin 用户 来作为 拥有最高权限的用户
  2. 在启动的时候, 判断一下, 如果 blacklist 是空的, 那么久不用初始化 命令受限的用户了

1、limit 用户我们现在默认叫“admin”,即pika会内置 “default”和“admin” 这俩用户(命名可以再讨论下) 2、按照之前 pika 的逻辑,requirepass 应该是 admin 用户的,userpass 是 default 用户的。 3、black-list 只针对默认用的用户,即 default 和 admin 用户;black-list 会被翻译成 acl 的语法,就当是用户多配置了几条 acl 规则,不冲突

修改完后:
1、根据配置文件产生两条 acl 规则:
a、default >requirepass &* ~* +@ALL
b、limit >userpass &* ~* +@ALL -userblacklist
2、在配置文件中不能再添加有关 limit 的用户规则,否则启动失败,可以添加default用户的规则。
3、在进行验证的时候,如果传入了两个参数(只传入了密码),那么首先跟 default 用户进行密码匹配,如果匹配失败了(任何错误原因),那么和 limit 用户进行密码匹配。如果 传入了用户名和密码,那么就正常匹配。
4、userblacklist 为空的情况下,同样会初始化 Limit用户,否则就会遇到之前的问题,都用只传入密码的方式去验证,但是只有一个 default 用户。

@dingxiaoshuai123
Copy link
Collaborator Author

dingxiaoshuai123 commented Mar 7, 2024

ACL 兼容以前的方案
基本和这个pr的内容一致, 在 pika启动的时候, 除了 要初始化原来 default用户外, 还要判断一下, 配置中的 blacklist 是否为空, 如果不为空, 那么需要再初始化一个 命令受限的用户, 比如 limit. 在启动后, 根据 配置中的 requirepass 来设置 default 用户的 密码, 使用 配置中的 userpass 来设置 limit用户的密码, 后续使用 CONFIG 命令来修改 密码时, 不能修改, 只能使用 ACL 的命令来修改. 使用 ACL命令修改 用户密码时, 也不要 修改 配置中 requirepassuserpass
下面需要讨论待定:

  1. 如果在配置 或者 ACL的配置文件中有多行 limit 相关的配置, 是否和 加载default用户一样, 提示启动失败
  2. 如果同时配置了 limit 用户的 ACL规则 和 blacklist, 以哪个为准, 还是说取两个规则的并集

这地方修改一下:

  1. 默认使用 default 用户代替原来的 admin 用户 来作为 拥有最高权限的用户
  2. 在启动的时候, 判断一下, 如果 blacklist 是空的, 那么久不用初始化 命令受限的用户了

1、limit 用户我们现在默认叫“admin”,即pika会内置 “default”和“admin” 这俩用户 2、按照之前 pika 的逻辑,requirepass 应该是 admin 用户的,userpass 是 default 用户的。 3、black-list 只针对默认用的用户,即 default 和 admin 用户;acl 规则理论上不动这俩默认用户,black-list 会被翻译成 acl 的语法,就当是用户多配置了几条 acl 规则

  1. 我的意思是 用 default 代替 admin, 以后pika中不会有 admin这个用户了, default 是最高权限的用户, 也是默认用户, 这样可以和redis保持一致, 否则, 一些测试用例可能要修改
  2. 这个是没问题的, requirepass是最高权限用户的密码
  3. blacklist 应该是只针对, pika创建的 那个功能受限的用户, 假设叫 limit, ACL规则 理论上不动这俩用户 没懂什么意思, 后续使用 ACL setuser 命令 不能修改 这两个用户吗? ack-list 会被翻译成 acl 的语法,就当是用户多配置了几条ad规则 这个是没问题的, 假设创建的那个功能受限的用户命令是 limit, 在 pika.conf文件里有一条 user : limit on >123 ~* +@all -flush 这个规则, 然后 blacklist中配置了 flushdb,shutdown, 最后再给 limit 设置规则时, 用这两个的并集? 再极端一点, 如果pika.conf 里 user : limit的规则和blacklist里配置的冲突了, 怎么解决, 如果配置了多条怎么解决, 执行了 ACL SAVE 是否把这两个用户的规则 保存, 以及保存后重新加载, 这些都需要考虑

这个 limit 用户可以是为了兼容以前的 userpass 和 blacklist 默认产生的一个用户,如果想用使用 acl ,那么user的用户名不可以是 limit 。这样可不可以?
我没有考虑到 save 后的重新加载,如果在 save 的时候跳过 limit 用户,这样重新加载的时候就可以重新初始化一个 limit 用户,而不从文件中加载,是否可行。

@lqxhub
Copy link
Collaborator

lqxhub commented Mar 7, 2024

ACL 兼容以前的方案
基本和这个pr的内容一致, 在 pika启动的时候, 除了 要初始化原来 default用户外, 还要判断一下, 配置中的 blacklist 是否为空, 如果不为空, 那么需要再初始化一个 命令受限的用户, 比如 limit. 在启动后, 根据 配置中的 requirepass 来设置 default 用户的 密码, 使用 配置中的 userpass 来设置 limit用户的密码, 后续使用 CONFIG 命令来修改 密码时, 不能修改, 只能使用 ACL 的命令来修改. 使用 ACL命令修改 用户密码时, 也不要 修改 配置中 requirepassuserpass
下面需要讨论待定:

  1. 如果在配置 或者 ACL的配置文件中有多行 limit 相关的配置, 是否和 加载default用户一样, 提示启动失败
  2. 如果同时配置了 limit 用户的 ACL规则 和 blacklist, 以哪个为准, 还是说取两个规则的并集

这地方修改一下:

  1. 默认使用 default 用户代替原来的 admin 用户 来作为 拥有最高权限的用户
  2. 在启动的时候, 判断一下, 如果 blacklist 是空的, 那么久不用初始化 命令受限的用户了

1、limit 用户我们现在默认叫“admin”,即pika会内置 “default”和“admin” 这俩用户 2、按照之前 pika 的逻辑,requirepass 应该是 admin 用户的,userpass 是 default 用户的。 3、black-list 只针对默认用的用户,即 default 和 admin 用户;acl 规则理论上不动这俩默认用户,black-list 会被翻译成 acl 的语法,就当是用户多配置了几条 acl 规则

  1. 我的意思是 用 default 代替 admin, 以后pika中不会有 admin这个用户了, default 是最高权限的用户, 也是默认用户, 这样可以和redis保持一致, 否则, 一些测试用例可能要修改
  2. 这个是没问题的, requirepass是最高权限用户的密码
  3. blacklist 应该是只针对, pika创建的 那个功能受限的用户, 假设叫 limit, ACL规则 理论上不动这俩用户 没懂什么意思, 后续使用 ACL setuser 命令 不能修改 这两个用户吗? ack-list 会被翻译成 acl 的语法,就当是用户多配置了几条ad规则 这个是没问题的, 假设创建的那个功能受限的用户命令是 limit, 在 pika.conf文件里有一条 user : limit on >123 ~* +@all -flush 这个规则, 然后 blacklist中配置了 flushdb,shutdown, 最后再给 limit 设置规则时, 用这两个的并集? 再极端一点, 如果pika.conf 里 user : limit的规则和blacklist里配置的冲突了, 怎么解决, 如果配置了多条怎么解决, 执行了 ACL SAVE 是否把这两个用户的规则 保存, 以及保存后重新加载, 这些都需要考虑

这个 limit 用户可以是为了兼容以前的 userpass 和 blacklist 默认产生的一个用户,如果想用使用 acl ,那么user的用户名不可以是 limit 。这样可不可以? 我没有考虑到 save 后的重新加载,如果在 save 的时候跳过 limit 用户,这样重新加载的时候就可以重新初始化一个 limit 用户,而不从文件中加载,是否可行。

这个不能用 ACL命令操作 limit 用户, save的时候跳过 limit, 这样会需要很多额外的判断, 而且会和redis不兼容

我觉的最简单的方式就是 在启动的时候, 用 blacklist 的规则初始化一个 limit用户, 后续就和 default 用户一样了, 哪怕是执行 ACL SETUSER limit +@ALL等等操作 也一样执行就行了, 但是为了安全, limit需要和 default一样, 不能被删除. 至于加载的优先级, 可以强制用 blacklist的规则覆盖 limit 的规则

src/pika_conf.cc Outdated
// SetConfStr("userpass", userpass_);
// SetConfStr("userblacklist", userblacklist);
SetConfStr("userpass", userpass_);
// SetConfStr("userblacklist", userblacklist_);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

检查一下 config rewrite 、config set get 相关的兼容性。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

检查一下 config rewrite 、config set get 相关的兼容性。

1、config get 已支持 userblacklist
2、config rewrite:已支持 userblacklist
3、config set:不支持 userblacklist,通过 acl 的命令去操作即可

@dingxiaoshuai123
Copy link
Collaborator Author

image

@dingxiaoshuai123 dingxiaoshuai123 changed the base branch from 3.5 to unstable March 7, 2024 11:42
@dingxiaoshuai123 dingxiaoshuai123 changed the title fix: fix acl fix: acl forward compatible Mar 7, 2024
Copy link
Collaborator

@lqxhub lqxhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultLimitUser 应该和 default 用户一样,也是不能被删除的, ACL DELUSER 命令那里 应该判断一下,

src/acl.cc Outdated Show resolved Hide resolved
src/acl.cc Show resolved Hide resolved
conf/pika.conf Outdated Show resolved Hide resolved
@AlexStocks AlexStocks merged commit 59e6273 into OpenAtomFoundation:unstable Mar 8, 2024
12 checks passed
luky116 pushed a commit to luky116/pika that referenced this pull request Mar 14, 2024
* fix: ACL user authentication errors

* blacklist instead of acl user

* add rename command (OpenAtomFoundation#2462)

* support config get userblacklist

---------
luky116 pushed a commit to luky116/pika that referenced this pull request Mar 14, 2024
* fix: ACL user authentication errors

* blacklist instead of acl user

* add rename command (OpenAtomFoundation#2462)

* support config get userblacklist

---------
luky116 pushed a commit that referenced this pull request Mar 18, 2024
* fix: ACL user authentication errors

* blacklist instead of acl user

* add rename command (#2462)

* support config get userblacklist

---------
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
* fix: ACL user authentication errors

* blacklist instead of acl user

* add rename command (OpenAtomFoundation#2462)

* support config get userblacklist

---------
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
* fix: ACL user authentication errors

* blacklist instead of acl user

* add rename command (OpenAtomFoundation#2462)

* support config get userblacklist

---------
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
* fix: ACL user authentication errors

* blacklist instead of acl user

* add rename command (OpenAtomFoundation#2462)

* support config get userblacklist

---------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.5.3 ☢️ Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants