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.

Sign up for GitHub

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

Move auth check to the front #1475

Open
wants to merge 5 commits into
base: unstable
Choose a base branch
from
+35 −17
Open

Move auth check to the front #1475

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa66479
Move the CMD_PROTECTED check to after the auth check
enjoy-binbin Dec 23, 2024
14e5761
auth check before command existence check and command arity check
enjoy-binbin Dec 24, 2024
513e02d
update the name in securityWarningCommand message
enjoy-binbin Dec 24, 2024
1f6fccc
cleanup client close in tests
enjoy-binbin Dec 24, 2024
bbdba23
Fix the test, consume the return value
enjoy-binbin Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions src/server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4006,21 +4006,6 @@ int processCommand(client *c) {
rejectCommandSds(c, err);
enjoy-binbin marked this conversation as resolved.
Show resolved Hide resolved
return C_OK;
}


/* Check if the command is marked as protected and the relevant configuration allows it */
if (c->cmd->flags & CMD_PROTECTED) {
if ((c->cmd->proc == debugCommand && !allowProtectedAction(server.enable_debug_cmd, c)) ||
(c->cmd->proc == moduleCommand && !allowProtectedAction(server.enable_module_cmd, c))) {
rejectCommandFormat(c,
"%s command not allowed. If the %s option is set to \"local\", "
"you can run it from a local connection, otherwise you need to set this option "
"in the configuration file, and then restart the server.",
c->cmd->proc == debugCommand ? "DEBUG" : "MODULE",
c->cmd->proc == debugCommand ? "enable-debug-command" : "enable-module-command");
return C_OK;
}
}
}

uint64_t cmd_flags = getCommandFlags(c);
Expand Down Expand Up @@ -4051,6 +4036,20 @@ int processCommand(client *c) {
}
}

/* Check if the command is marked as protected and the relevant configuration allows it */
if (c->cmd->flags & CMD_PROTECTED) {
if ((c->cmd->proc == debugCommand && !allowProtectedAction(server.enable_debug_cmd, c)) ||
(c->cmd->proc == moduleCommand && !allowProtectedAction(server.enable_module_cmd, c))) {
rejectCommandFormat(c,
"%s command not allowed. If the %s option is set to \"local\", "
"you can run it from a local connection, otherwise you need to set this option "
"in the configuration file, and then restart the server.",
c->cmd->proc == debugCommand ? "DEBUG" : "MODULE",
c->cmd->proc == debugCommand ? "enable-debug-command" : "enable-module-command");
return C_OK;
}
}

if (c->flag.multi && c->cmd->flags & CMD_NO_MULTI) {
rejectCommandFormat(c, "Command not allowed inside a transaction");
return C_OK;
Expand Down
6 changes: 5 additions & 1 deletion tests/unit/introspection.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -991,8 +991,12 @@ start_server {tags {"introspection"}} {
# known keywords. Might be a good idea to avoid adding tests here.
}

start_server {tags {"introspection external:skip"} overrides {enable-protected-configs {no} enable-debug-command {no}}} {
start_server {tags {"introspection external:skip"} overrides {requirepass mypass enable-protected-configs {no} enable-debug-command {no}}} {
test {cannot modify protected configuration - no} {
assert_error "NOAUTH *" {r config set dir somedir}
assert_error "NOAUTH *" {r DEBUG HELP}

r auth mypass
assert_error "ERR *protected*" {r config set dir somedir}
assert_error "ERR *DEBUG command not allowed*" {r DEBUG HELP}
} {} {needs:debug}
Expand Down
7 changes: 5 additions & 2 deletions tests/unit/moduleapi/basics.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ start_server {tags {"modules"}} {
}
}

start_server {tags {"modules external:skip"} overrides {enable-module-command no}} {
start_server {tags {"modules external:skip"} overrides {requirepass mypass enable-module-command no}} {
test {module command disabled} {
assert_error "ERR *MODULE command not allowed*" {r module load $testmodule}
assert_error "NOAUTH *" {r module load $testmodule}

r auth mypass
assert_error "ERR *MODULE command not allowed*" {r module load $testmodule}
}
}
Loading