-
Notifications
You must be signed in to change notification settings - Fork 24.4k
Add internal connection and command mechanism #13740
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
Conversation
MeirShpilraien
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@raz-mon looks good.
Added some comments but please notice that I did not yet finished the review all the changes.
The PR intoruduces a new shared secret that is shared over all the nodes on the Redis cluster. The main idea is to leverage the cluster bus to share a secret between all the nodes such that later the nodes will be able to authenticate using this secret and send internal commands to each other (see redis#13740 for more inforamtion about internal commands). The way the shared secret is chosen is the following: 1. Each node, when start, randomly generate its own internal secret. 2. Each node share its internal secret over the cluster ping messages. 3. In addition, each node also shared the secrets of the other nodes it knows about as part of the gossip protocol. 4. Node choses the secret with the minimal value (lexicographical). The converges of the secret is as good as the topology converges. To extend the ping messages to contain the secret, we leverage the extention machanism. Nodes that runs an older Redis version will just ignore those extentions. Secrets are also persisted to the `node.conf` file so that on start we will have the secret right away. Specific tests were added to verify that eventually all nodes see the secrets. In addition, a verification was added to the test infra to verify the secret on `cluster_config_consistent` and to `assert_cluster_state`
| test {Internal commands are reported in the latency report} { | ||
| # The latency report should contain the internal command | ||
| set report [r latency histogram internalauth.internalcommand] | ||
| assert_match {*internalauth.internalcommand*} $report | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably do it now via an internal connection, we are still internal client from the test above this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow your meaning of doing it now via an internal connection.
I can switch to a non-internal connection so that we make sure we get this there as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can switch to a non-internal connection so that we make sure we get this there as well
Yes this is what I meant.
| } | ||
| } | ||
|
|
||
| start_cluster 1 0 [list config_lines $modules] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you do not need cluster deployment for those tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mainly for the regular internal connection authentication flow, otherwise can be reached via the non-standard debug command.
Any reason not to use a cluster and the regular flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be faster to deploy, I believe we better use this and just promote the connection with debug command. Also simplify the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I can change to use a standalone server
MeirShpilraien
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Added a few comments on the tests.
MeirShpilraien
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there, 2 small comments.
MeirShpilraien
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
The PR makes changes to use the internal command capabilities introduced on: redis/redis#13740. The main idea is to use the new redismodule API that provides internal secret for internal shard communication. This means that the following is no longer needed: 1. The new user configuration. 2. The internal ACL category All the commands are marked with the `internal` flag (if we see that the internal commands feature is supported on the current Redis version we run on). In addition, the connection between the shards uses `INTERNALAUTH` with the given internal secret so it can run all the internal commands. Regular user can not run any internal command. The PR require the following PR's to be merge: 1. redis/redis#13740 2. RedisLabsModules/redismodule-rs#407 The tests was modified to avoid giving password to the test module in case it detects that the internal commands feature exists. In such case, avoiding giving the password to the module, make sure that LibMR actually using the `INTERNALAUTH` command with the internal secret to authenticate.
The PR introduces a new shared secret that is shared over all the nodes on the Redis cluster. The main idea is to leverage the cluster bus to share a secret between all the nodes such that later the nodes will be able to authenticate using this secret and send internal commands to each other (see redis#13740 for more information about internal commands). The way the shared secret is chosen is the following: 1. Each node, when start, randomly generate its own internal secret. 2. Each node share its internal secret over the cluster ping messages. 3. If a node gets a ping message with secret smaller then his current secret, it embrace it. 4. Eventually all nodes should embrace the minimal secret The converges of the secret is as good as the topology converges. To extend the ping messages to contain the secret, we leverage the extension mechanism. Nodes that runs an older Redis version will just ignore those extensions. Specific tests were added to verify that eventually all nodes see the secrets. In addition, a verification was added to the test infra to verify the secret on `cluster_config_consistent` and to `assert_cluster_state`.
# PR: Add Mechanism for Internal Commands and Connections in Redis This PR introduces a mechanism to handle **internal commands and connections** in Redis. It includes enhancements for command registration, internal authentication, and observability. ## Key Features 1. **Internal Command Flag**: - Introduced a new **module command registration flag**: `internal`. - Commands marked with `internal` can only be executed by **internal connections**, AOF loading flows, and master-replica connections. - For any other connection, these commands will appear as non-existent. 2. **Support for internal authentication added to `AUTH`**: - Used by depicting the special username `internal connection` with the right internal password, i.e.,: `AUTH "internal connection" <internal_secret>`. - No user-defined ACL username can have this name, since spaces are not aloud in the ACL parser. - Allows connections to authenticate as **internal connections**. - Authenticated internal connections can execute internal commands successfully. 4. **Module API for Internal Secret**: - Added the `RedisModule_GetInternalSecret()` API, that exposes the internal secret that should be used as the password for the new `AUTH "internal connection" <password>` command. - This API enables the modules to authenticate against other shards as local connections. ## Notes on Behavior - **ACL validation**: - Commands dispatched by internal connections bypass ACL validation, to give the caller full access regardless of the user with which it is connected. - **Command Visibility**: - Internal commands **do not appear** in `COMMAND <subcommand>` and `MONITOR` for non-internal connections. - Internal commands **are logged** in the slow log, latency report and commands' statistics to maintain observability. - **`RM_Call()` Updates**: - **Non-internal connections**: - Cannot execute internal commands when the command is sent with the `C` flag (otherwise can). - Internal connections bypass ACL validations (i.e., run as the unrestricted user). - **Internal commands' success**: - Internal commands succeed upon being sent from either an internal connection (i.e., authenticated via the new `AUTH "internal connection" <internal_secret>` API), an AOF loading process, or from a master via the replication link. Any other connections that attempt to execute an internal command fail with the `unknown command` error message raised. - **`CLIENT LIST` flags**: - Added the `I` flag, to indicate that the connection is internal. - **Lua Scripts**: - Prevented internal commands from being executed via Lua scripts. --------- Co-authored-by: Meir Shpilraien <meir@redis.com>
PR: Add Mechanism for Internal Commands and Connections in Redis
This PR introduces a mechanism to handle internal commands and connections in Redis. It includes enhancements for command registration, internal authentication, and observability.
Key Features
Internal Command Flag:
internal.internalcan only be executed by internal connections, AOF loading flows, and master-replica connections.Support for internal authentication added to
AUTH:internal connectionwith the right internal password, i.e.,:AUTH "internal connection" <internal_secret>.Module API for Internal Secret:
RedisModule_GetInternalSecret()API, that exposes the internal secret that should be used as the password for the newAUTH "internal connection" <password>command.Notes on Behavior
ACL validation:
Command Visibility:
COMMAND <subcommand>andMONITORfor non-internal connections.RM_Call()Updates:Cflag (otherwise can).Internal commands' success:
AUTH "internal connection" <internal_secret>API), an AOF loading process, or from a master via the replication link.Any other connections that attempt to execute an internal command fail with the
unknown commanderror message raised.CLIENT LISTflags:Iflag, to indicate that the connection is internal.Lua Scripts: