Move auth check to the front

src/networking.c

@@ -4242,13 +4242,13 @@ void securityWarningCommand(client *c) {
         int port;
         if (connAddrPeerName(c->conn, ip, sizeof(ip), &port) == -1) {
             serverLog(LL_WARNING, "Possible SECURITY ATTACK detected. It looks like somebody is sending POST or Host: "
-                                  "commands to Redis. This is likely due to an attacker attempting to use Cross "
-                                  "Protocol Scripting to compromise your Redis instance. Connection aborted.");
+                                  "commands to Valkey. This is likely due to an attacker attempting to use Cross "
+                                  "Protocol Scripting to compromise your Valkey instance. Connection aborted.");
         } else {
             serverLog(LL_WARNING,
                       "Possible SECURITY ATTACK detected. It looks like somebody is sending POST or Host: commands to "
-                      "Redis. This is likely due to an attacker attempting to use Cross Protocol Scripting to "
-                      "compromise your Redis instance. Connection from %s:%d aborted.",
+                      "Valkey. This is likely due to an attacker attempting to use Cross Protocol Scripting to "
+                      "compromise your Valkey instance. Connection from %s:%d aborted.",
                       ip, port);
         }
         logged_time = now;

src/server.c

@@ -3997,6 +3997,16 @@ int processCommand(client *c) {
             }
         }
         c->cmd = c->lastcmd = c->realcmd = cmd;
+
+        if (authRequired(c)) {
+            /* AUTH and HELLO and no auth commands are valid even in
+             * non-authenticated state. */
+            if (!c->cmd || !(c->cmd->flags & CMD_NO_AUTH)) {
+                rejectCommand(c, shared.noautherr);
+                return C_OK;
+            }
+        }
+
         sds err;
         if (!commandCheckExistence(c, &err)) {
             rejectCommandSds(c, err);
@@ -4007,7 +4017,6 @@ int processCommand(client *c) {
             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)) ||
@@ -4042,15 +4051,6 @@ int processCommand(client *c) {
                                         (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_NO_ASYNC_LOADING));
     const int obey_client = mustObeyClient(c);
 
-    if (authRequired(c)) {
-        /* AUTH and HELLO and no auth commands are valid even in
-         * non-authenticated state. */
-        if (!(c->cmd->flags & CMD_NO_AUTH)) {
-            rejectCommand(c, shared.noautherr);
-            return C_OK;
-        }
-    }
-
     if (c->flag.multi && c->cmd->flags & CMD_NO_MULTI) {
         rejectCommandFormat(c, "Command not allowed inside a transaction");
         return C_OK;

tests/unit/introspection.tcl

@@ -991,8 +991,17 @@ 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 {auth check before command existence check and command arity check} {
+        assert_error "NOAUTH *" {r non-existing-command}
+        assert_error "NOAUTH *" {r set key value wrong_arg}
+    }
+
     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}
@@ -1011,6 +1020,7 @@ start_server {config "minimal.conf" tags {"introspection external:skip"} overrid
             set r2 [get_nonloopback_client]
             assert_error "ERR *protected*" {$r2 config set dir somedir}
             assert_error "ERR *DEBUG command not allowed*" {$r2 DEBUG HELP}
+            $r2 close
         }
     } {} {needs:debug}
 }

tests/unit/moduleapi/basics.tcl

@@ -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}
     }
 }

tests/unit/networking.tcl

@@ -145,28 +145,33 @@ start_server {config "minimal.conf" tags {"external:skip"}} {
             set r2 [get_nonloopback_client]
             catch {$r2 ping} err
             assert_match {*DENIED*} $err
+            $r2 close
 
             # Bind configuration should not matter
             assert_equal {OK} [r config set bind "*"]
             set r2 [get_nonloopback_client]
             catch {$r2 ping} err
             assert_match {*DENIED*} $err
+            $r2 close
 
             # Setting a password should disable protected mode
             assert_equal {OK} [r config set requirepass "secret"]
             set r2 [valkey $myaddr [srv 0 "port"] 0 $::tls]
             assert_equal {OK} [$r2 auth secret]
             assert_equal {PONG} [$r2 ping]
+            $r2 close
 
             # Clearing the password re-enables protected mode
             assert_equal {OK} [r config set requirepass ""]
             set r2 [valkey $myaddr [srv 0 "port"] 0 $::tls]
             assert_match {*DENIED*} $err
+            $r2 close
 
             # Explicitly disabling protected-mode works
             assert_equal {OK} [r config set protected-mode no]
             set r2 [valkey $myaddr [srv 0 "port"] 0 $::tls]
             assert_equal {PONG} [$r2 ping]
+            $r2 close
         }
     }
 }