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

[ISSUE #7560] [RIP-68] Support RocketMQ ACL 2.0 #7725

Merged
merged 80 commits into from
Mar 18, 2024

Conversation

dingshuangxi888
Copy link
Contributor

@dingshuangxi888 dingshuangxi888 commented Jan 5, 2024

Which Issue(s) This PR Fixes

Fixes #7560

Brief Description

  1. Standardized IP whitelist control: The enhanced ACL design provides a more standardized IP whitelist control mechanism. It effectively restricts user requests to specific IP sources and blocks access from untrusted IP addresses.
  2. Scalable ACL configuration and authentication mechanism: The improved design allows for easy extension and implementation of ACL-related logic. Users can conveniently customize and expand ACL configurations to meet their specific requirements. Additionally, the ACL design includes access control for control-related interfaces, enhancing the overall security of the system.
  3. Effective separation of user and permission management: The optimized design successfully achieves a clear separation between user authentication and permission management, establishing explicit responsibilities and boundaries for each. This enhancement significantly improves the security of the system. Additionally, User passwords should be stored in a non-plaintext format., effectively reducing the risk of password leaks.

How Did You Test This Change?

broker config

authenticationEnabled = true
authenticationProvider = org.apache.rocketmq.auth.authentication.provider.DefaultAuthenticationProvider
initAuthenticationUser = {"username":"rocketmq","password":"12345678"}
innerClientAuthenticationCredentials = {"accessKey":"rocketmq","secretKey":"12345678"}
authenticationMetadataProvider = org.apache.rocketmq.auth.authentication.provider.LocalAuthenticationMetadataProvider
authorizationEnabled = true
authorizationProvider = org.apache.rocketmq.auth.authorization.provider.DefaultAuthorizationProvider
authorizationMetadataProvider = org.apache.rocketmq.auth.authorization.provider.LocalAuthorizationMetadataProvider

proxy config

{
  "authenticationEnabled": true,
  "authenticationProvider": "org.apache.rocketmq.auth.authentication.provider.DefaultAuthenticationProvider",
  "authenticationMetadataProvider": "org.apache.rocketmq.proxy.auth.ProxyAuthenticationMetadataProvider",
  "innerClientAuthenticationCredentials": "{\"accessKey\":\"rocketmq\", \"secretKey\":\"12345678\"}",
  "authorizationEnabled": true,
  "authorizationProvider": "org.apache.rocketmq.auth.authorization.provider.DefaultAuthorizationProvider",
  "authorizationMetadataProvider": "org.apache.rocketmq.proxy.auth.ProxyAuthorizationMetadataProvider"
}

migration from acl v1

migrateAuthFromV1Enabled = true

…acl_2.0

# Conflicts:
#	remoting/src/main/java/org/apache/rocketmq/remoting/protocol/RequestCode.java
#	remoting/src/main/java/org/apache/rocketmq/remoting/protocol/ResponseCode.java
#	remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/controller/ElectMasterRequestHeader.java
Copy link
Member

@lizhimins lizhimins left a comment

Choose a reason for hiding this comment

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

  1. 缓存部分,打开 cache 自带的 stats 统计能力并输出,性能影响很小,默认缓存大小是否合理?
  2. 能否快速找到问题用户或者资源?
  3. this.kvStore.flushWal(true); rocksdb 部分应该保证数据同步落盘。
  4. 鉴权降级和部分降级能力是否验证过。
  5. RequestHeaderRegistry.getInstance().initialize(); 这个地方能不能去单例,broker container 模式是否可以?
  6. if (this.authorizationMetadataManager != null) 这里 bug 了,shutdown 了两次。。。
  7. 请求失败时,错误日志过多,会打爆磁盘。
  8. RemotingCode 需要加前缀和别的请求区分。
  9. AbstractTransactionalMessageCheckListener.java 这里加个 topic 干啥用的?
  10. GrpcServerBuilder.java 鉴权 interceptors 和 global interceptors 的顺序是否正确?

@dingshuangxi888
Copy link
Contributor Author

  1. 缓存部分,打开 cache 自带的 stats 统计能力并输出,性能影响很小,默认缓存大小是否合理?
    先试运行一段时间,后期考虑可以加一下
  2. 能否快速找到问题用户或者资源?
    目前通过审计日志进行支持
  3. this.kvStore.flushWal(true); rocksdb 部分应该保证数据同步落盘。
    底层方法使用的就是这个能力
  4. 鉴权降级和部分降级能力是否验证过。
    可通过修改配置进行降级
  5. RequestHeaderRegistry.getInstance().initialize(); 这个地方能不能去单例,broker container 模式是否可以?
    这个功能是扫描所有的RequestHeader然后构建一个映射关系,这个系统级别的数据,用单例还是比较合适的
  6. if (this.authorizationMetadataManager != null) 这里 bug 了,shutdown 了两次。。。
    已修复,感谢指出
  7. 请求失败时,错误日志过多,会打爆磁盘。
    目前通过日志级别来进行日志区分,后面可以有更加高级的策略进行日志打印,支持问题有效排查的同时,避免日志过多
  8. RemotingCode 需要加前缀和别的请求区分。
    已修复,感谢指出
  9. AbstractTransactionalMessageCheckListener.java 这里加个 topic 干啥用的?
    事务相关的API原来没有透传topic,导致无法在请求面解析用户资源。最新客户端SDK已经加上这个字段
  10. GrpcServerBuilder.java 鉴权 interceptors 和 global interceptors 的顺序是否正确?
    interceptor就是倒叙添加的,这个已经检查并确认

@drpmma
Copy link
Contributor

drpmma commented Mar 18, 2024

It‘s better to have a clear roadmap like how to treat the old version acl.

Copy link
Contributor

@drpmma drpmma left a comment

Choose a reason for hiding this comment

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

LGTM

@dingshuangxi888
Copy link
Contributor Author

It‘s better to have a clear roadmap like how to treat the old version acl.

The old version acl will be removed soon, It is good idea to deprecated at enableAcl config.

@lizhimins lizhimins merged commit e1339ac into apache:develop Mar 18, 2024
9 of 10 checks passed
@frinda
Copy link
Contributor

frinda commented Jul 27, 2024

Are there any plans to reverse this ability into RocketMQ 4.x?

@dingshuangxi888
Copy link
Contributor Author

Are there any plans to reverse this ability into RocketMQ 4.x?

There is no such plan at the moment, but you can implement it yourself.

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.

[RIP-68] RocketMQ ACL 2.0
5 participants