Protected configs and sensitive commands (#9920)

Block sensitive configs and commands by default.

* `enable-protected-configs` - block modification of configs with the new `PROTECTED_CONFIG` flag.
   Currently we add this flag to `dbfilename`, and `dir` configs,
   all of which are non-mutable configs that can set a file redis will write to.
* `enable-debug-command` - block the `DEBUG` command
* `enable-module-command` - block the `MODULE` command

These have a default value set to `no`, so that these features are not
exposed by default to client connections, and can only be set by modifying the config file.

Users can change each of these to either `yes` (allow all access), or `local` (allow access from
local TCP connections and unix domain connections)

Note that this is a **breaking change** (specifically the part about MODULE command being disabled by default).
I.e. we don't consider DEBUG command being blocked as an issue (people shouldn't have been using it),
and the few configs we protected are unlikely to have been set at runtime anyway.
On the other hand, it's likely to assume some users who use modules, load them from the config file anyway.
Note that's the whole point of this PR, for redis to be more secure by default and reduce the attack surface on
innocent users, so secure defaults will necessarily mean a breaking change.

15 files changed, 148 insertions, 31 deletions

diff --git a/.github/workflows/external.yml b/.github/workflows/external.yml
index 2dbd4704b..35b480106 100644
--- a/.github/workflows/external.yml
+++ b/.github/workflows/external.yml
@@ -15,7 +15,7 @@ jobs:
     - name: Build
       run: make REDIS_CFLAGS=-Werror
     - name: Start redis-server
-      run: ./src/redis-server --daemonize yes --logfile external-redis.log
+      run: ./src/redis-server --daemonize yes --logfile external-redis.log --enable-protected-configs yes --enable-debug-command yes --enable-module-command yes
     - name: Run external test
       run: |
           ./runtest \
@@ -36,7 +36,7 @@ jobs:
     - name: Build
       run: make REDIS_CFLAGS=-Werror
     - name: Start redis-server
-      run: ./src/redis-server --cluster-enabled yes --daemonize yes --logfile external-redis.log
+      run: ./src/redis-server --cluster-enabled yes --daemonize yes --logfile external-redis.log --enable-protected-configs yes --enable-debug-command yes --enable-module-command yes
     - name: Create a single node cluster
       run: ./src/redis-cli cluster addslots $(for slot in {0..16383}; do echo $slot; done); sleep 5
     - name: Run external test
@@ -51,4 +51,3 @@ jobs:
       with:
         name: test-external-cluster-log
         path: external-redis.log
-
diff --git a/redis.conf b/redis.conf
index 1350ac96d..8bac2afb5 100644
--- a/redis.conf
+++ b/redis.conf
@@ -110,6 +110,29 @@ bind 127.0.0.1 -::1
 # even if no authentication is configured.
 protected-mode yes
 
+# Redis uses default hardened security configuration directives to reduce the
+# attack surface on innocent users. Therefore, several sensitive configuration
+# directives are immutable, and some potentially-dangerous commands are blocked.
+#
+# Configuration directives that control files that Redis writes to (e.g., 'dir'
+# and 'dbfilename') and that aren't usually modified during runtime
+# are protected by making them immutable.
+#
+# Commands that can increase the attack surface of Redis and that aren't usually
+# called by users are blocked by default.
+#
+# These can be exposed to either all connections or just local ones by setting
+# each of the configs listed below to either of these values:
+#
+# no    - Block for any connection (remain immutable)
+# yes   - Allow for any connection (no protection)
+# local - Allow only for local local connections. Ones originating from the
+#         IPv4 address (127.0.0.1), IPv6 address (::1) or Unix domain sockets.
+#
+# enable-protected-configs no
+# enable-debug-command no
+# enable-module-command no
+
 # Accept connections on the specified port, default is 6379 (IANA #815344).
 # If port 0 is specified Redis will not listen on a TCP socket.
 port 6379
diff --git a/src/commands.c b/src/commands.c
index 4232ba8b5..978c03f89 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -4148,8 +4148,8 @@ struct redisCommandArg MODULE_UNLOAD_Args[] = {
 struct redisCommand MODULE_Subcommands[] = {
 {"help","Show helpful text about the different subcommands","O(1)","5.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,MODULE_HELP_History,MODULE_HELP_Hints,moduleCommand,2,CMD_LOADING|CMD_STALE,0},
 {"list","List all modules loaded by the server","O(N) where N is the number of loaded modules.","4.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,MODULE_LIST_History,MODULE_LIST_Hints,moduleCommand,2,CMD_ADMIN|CMD_NOSCRIPT,0},
-{"load","Load a module","O(1)","4.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,MODULE_LOAD_History,MODULE_LOAD_Hints,moduleCommand,-3,CMD_ADMIN|CMD_NOSCRIPT,0,.args=MODULE_LOAD_Args},
-{"unload","Unload a module","O(1)","4.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,MODULE_UNLOAD_History,MODULE_UNLOAD_Hints,moduleCommand,3,CMD_ADMIN|CMD_NOSCRIPT,0,.args=MODULE_UNLOAD_Args},
+{"load","Load a module","O(1)","4.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,MODULE_LOAD_History,MODULE_LOAD_Hints,moduleCommand,-3,CMD_ADMIN|CMD_NOSCRIPT|CMD_PROTECTED,0,.args=MODULE_LOAD_Args},
+{"unload","Unload a module","O(1)","4.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,MODULE_UNLOAD_History,MODULE_UNLOAD_Hints,moduleCommand,3,CMD_ADMIN|CMD_NOSCRIPT|CMD_PROTECTED,0,.args=MODULE_UNLOAD_Args},
 {0}
 };
 
@@ -6431,7 +6431,7 @@ struct redisCommand redisCommandTable[] = {
 {"command","Get array of Redis command details","O(N) where N is the total number of Redis commands","2.8.13",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,COMMAND_History,COMMAND_Hints,commandCommand,-1,CMD_RANDOM|CMD_LOADING|CMD_STALE|CMD_SENTINEL,ACL_CATEGORY_CONNECTION,.subcommands=COMMAND_Subcommands},
 {"config","A container for server configuration commands","Depends on subcommand.","2.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,CONFIG_History,CONFIG_Hints,NULL,-2,0,0,.subcommands=CONFIG_Subcommands},
 {"dbsize","Return the number of keys in the selected database","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,DBSIZE_History,DBSIZE_Hints,dbsizeCommand,1,CMD_READONLY|CMD_FAST,ACL_CATEGORY_KEYSPACE},
-{"debug","A container for debugging commands","Depends on subcommand.","1.0.0",CMD_DOC_SYSCMD,NULL,NULL,COMMAND_GROUP_SERVER,DEBUG_History,DEBUG_Hints,debugCommand,-2,CMD_ADMIN|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE,0},
+{"debug","A container for debugging commands","Depends on subcommand.","1.0.0",CMD_DOC_SYSCMD,NULL,NULL,COMMAND_GROUP_SERVER,DEBUG_History,DEBUG_Hints,debugCommand,-2,CMD_ADMIN|CMD_NOSCRIPT|CMD_LOADING|CMD_STALE|CMD_PROTECTED,0},
 {"failover","Start a coordinated failover between this server and one of its replicas.","O(1)","6.2.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,FAILOVER_History,FAILOVER_Hints,failoverCommand,-1,CMD_ADMIN|CMD_NOSCRIPT|CMD_STALE,0,.args=FAILOVER_Args},
 {"flushall","Remove all keys from all databases","O(N) where N is the total number of keys in all databases","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,FLUSHALL_History,FLUSHALL_Hints,flushallCommand,-1,CMD_WRITE,ACL_CATEGORY_KEYSPACE|ACL_CATEGORY_DANGEROUS,.args=FLUSHALL_Args},
 {"flushdb","Remove all keys from the current database","O(N) where N is the number of keys in the selected database","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_SERVER,FLUSHDB_History,FLUSHDB_Hints,flushdbCommand,-1,CMD_WRITE,ACL_CATEGORY_KEYSPACE|ACL_CATEGORY_DANGEROUS,.args=FLUSHDB_Args},
diff --git a/src/commands/debug.json b/src/commands/debug.json
index f2000a440..092d2e9ab 100644
--- a/src/commands/debug.json
+++ b/src/commands/debug.json
@@ -13,7 +13,8 @@
             "ADMIN",
             "NOSCRIPT",
             "LOADING",
-            "STALE"
+            "STALE",
+            "PROTECTED"
         ]
     }
 }
diff --git a/src/commands/module-load.json b/src/commands/module-load.json
index 6f3159ae2..836f5261b 100644
--- a/src/commands/module-load.json
+++ b/src/commands/module-load.json
@@ -9,7 +9,8 @@
         "function": "moduleCommand",
         "command_flags": [
             "ADMIN",
-            "NOSCRIPT"
+            "NOSCRIPT",
+            "PROTECTED"
         ],
         "arguments": [
             {
diff --git a/src/commands/module-unload.json b/src/commands/module-unload.json
index 1e8623e81..38cba4e43 100644
--- a/src/commands/module-unload.json
+++ b/src/commands/module-unload.json
@@ -9,7 +9,8 @@
         "function": "moduleCommand",
         "command_flags": [
             "ADMIN",
-            "NOSCRIPT"
+            "NOSCRIPT",
+            "PROTECTED"
         ],
         "arguments": [
             {
diff --git a/src/config.c b/src/config.c
index f171b0041..b22036e03 100644
--- a/src/config.c
+++ b/src/config.c
@@ -128,6 +128,13 @@ configEnum sanitize_dump_payload_enum[] = {
     {NULL, 0}
 };
 
+configEnum protected_action_enum[] = {
+    {"no", PROTECTED_ACTION_ALLOWED_NO},
+    {"yes", PROTECTED_ACTION_ALLOWED_YES},
+    {"local", PROTECTED_ACTION_ALLOWED_LOCAL},
+    {NULL, 0}
+};
+
 /* Output buffer limits presets. */
 clientBufferLimitsConfig clientBufferLimitsDefaults[CLIENT_TYPE_OBUF_COUNT] = {
     {0, 0, 0}, /* normal */
@@ -255,6 +262,7 @@ typedef struct standardConfig {
 #define DEBUG_CONFIG (1ULL<<2) /* Values that are useful for debugging. */
 #define MULTI_ARG_CONFIG (1ULL<<3) /* This config receives multiple arguments. */
 #define HIDDEN_CONFIG (1ULL<<4) /* This config is hidden in `config get <pattern>` (used for tests/debugging) */
+#define PROTECTED_CONFIG (1ULL<<5) /* Becomes immutable if enable-protected-configs is enabled. */
 
 standardConfig configs[];
 
@@ -711,9 +719,11 @@ void configSetCommand(client *c) {
                 }
 
                 if (!invalid_args) {
-                    if (config->flags & IMMUTABLE_CONFIG) {
+                    if (config->flags & IMMUTABLE_CONFIG ||
+                        (config->flags & PROTECTED_CONFIG && !allowProtectedAction(server.enable_protected_configs, c)))
+                    {
                         /* Note: we don't abort the loop since we still want to handle redacting sensitive configs (above) */
-                        errstr = "can't set immutable config";
+                        errstr = (config->flags & IMMUTABLE_CONFIG) ? "can't set immutable config" : "can't set protected config";
                         err_arg_name = c->argv[2+i*2]->ptr;
                         invalid_args = 1;
                     }
@@ -2552,6 +2562,11 @@ static sds getConfigReplicaOfOption(typeData data) {
     return sdsnew(buf);
 }
 
+int allowProtectedAction(int config, client *c) {
+    return (config == PROTECTED_ACTION_ALLOWED_YES) ||
+           (config == PROTECTED_ACTION_ALLOWED_LOCAL && islocalClient(c));
+}
+
 standardConfig configs[] = {
     /* Bool configs */
     createBoolConfig("rdbchecksum", NULL, IMMUTABLE_CONFIG, server.rdb_checksum, 1, NULL, NULL),
@@ -2607,7 +2622,7 @@ standardConfig configs[] = {
     createStringConfig("cluster-announce-ip", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_ip, NULL, NULL, updateClusterIp),
     createStringConfig("cluster-config-file", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.cluster_configfile, "nodes.conf", NULL, NULL),
     createStringConfig("syslog-ident", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.syslog_ident, "redis", NULL, NULL),
-    createStringConfig("dbfilename", NULL, MODIFIABLE_CONFIG, ALLOW_EMPTY_STRING, server.rdb_filename, "dump.rdb", isValidDBfilename, NULL),
+    createStringConfig("dbfilename", NULL, MODIFIABLE_CONFIG | PROTECTED_CONFIG, ALLOW_EMPTY_STRING, server.rdb_filename, "dump.rdb", isValidDBfilename, NULL),
     createStringConfig("appendfilename", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.aof_filename, "appendonly.aof", isValidAOFfilename, NULL),
     createStringConfig("server_cpulist", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.server_cpulist, NULL, NULL, NULL),
     createStringConfig("bio_cpulist", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.bio_cpulist, NULL, NULL, NULL),
@@ -2632,6 +2647,9 @@ standardConfig configs[] = {
     createEnumConfig("oom-score-adj", NULL, MODIFIABLE_CONFIG, oom_score_adj_enum, server.oom_score_adj, OOM_SCORE_ADJ_NO, NULL, updateOOMScoreAdj),
     createEnumConfig("acl-pubsub-default", NULL, MODIFIABLE_CONFIG, acl_pubsub_default_enum, server.acl_pubsub_default, USER_FLAG_ALLCHANNELS, NULL, NULL),
     createEnumConfig("sanitize-dump-payload", NULL, DEBUG_CONFIG | MODIFIABLE_CONFIG, sanitize_dump_payload_enum, server.sanitize_dump_payload, SANITIZE_DUMP_NO, NULL, NULL),
+    createEnumConfig("enable-protected-configs", NULL, IMMUTABLE_CONFIG, protected_action_enum, server.enable_protected_configs, PROTECTED_ACTION_ALLOWED_NO, NULL, NULL),
+    createEnumConfig("enable-debug-command", NULL, IMMUTABLE_CONFIG, protected_action_enum, server.enable_debug_cmd, PROTECTED_ACTION_ALLOWED_NO, NULL, NULL),
+    createEnumConfig("enable-module-command", NULL, IMMUTABLE_CONFIG, protected_action_enum, server.enable_module_cmd, PROTECTED_ACTION_ALLOWED_NO, NULL, NULL),
 
     /* Integer configs */
     createIntConfig("databases", NULL, IMMUTABLE_CONFIG, 1, INT_MAX, server.dbnum, 16, INTEGER_CONFIG, NULL, NULL),
@@ -2734,7 +2752,7 @@ standardConfig configs[] = {
 #endif
 
     /* Special configs */
-    createSpecialConfig("dir", NULL, MODIFIABLE_CONFIG, setConfigDirOption, getConfigDirOption, rewriteConfigDirOption, NULL),
+    createSpecialConfig("dir", NULL, MODIFIABLE_CONFIG | PROTECTED_CONFIG, setConfigDirOption, getConfigDirOption, rewriteConfigDirOption, NULL),
     createSpecialConfig("save", NULL, MODIFIABLE_CONFIG | MULTI_ARG_CONFIG, setConfigSaveOption, getConfigSaveOption, rewriteConfigSaveOption, NULL),
     createSpecialConfig("client-output-buffer-limit", NULL, MODIFIABLE_CONFIG | MULTI_ARG_CONFIG, setConfigClientOutputBufferLimitOption, getConfigClientOutputBufferLimitOption, rewriteConfigClientOutputBufferLimitOption, NULL),
     createSpecialConfig("oom-score-adj-values", NULL, MODIFIABLE_CONFIG | MULTI_ARG_CONFIG, setConfigOOMScoreAdjValuesOption, getConfigOOMScoreAdjValuesOption, rewriteConfigOOMScoreAdjValuesOption, updateOOMScoreAdj),
diff --git a/src/networking.c b/src/networking.c
index 70abf0588..2c12b39f9 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -1026,6 +1026,18 @@ int clientHasPendingReplies(client *c) {
     }
 }
 
+/* Return true if client connected from loopback interface */
+int islocalClient(client *c) {
+    /* unix-socket */
+    if (c->flags & CLIENT_UNIX_SOCKET) return 1;
+
+    /* tcp */
+    char cip[NET_IP_STR_LEN+1] = { 0 };
+    connPeerToString(c->conn, cip, sizeof(cip)-1, NULL);
+
+    return !strcmp(cip,"127.0.0.1") || !strcmp(cip,"::1");
+}
+
 void clientAcceptHandler(connection *conn) {
     client *c = connGetPrivateData(conn);
 
@@ -1042,13 +1054,9 @@ void clientAcceptHandler(connection *conn) {
      * requests from non loopback interfaces. Instead we try to explain the
      * user what to do to fix it if needed. */
     if (server.protected_mode &&
-        DefaultUser->flags & USER_FLAG_NOPASS &&
-        !(c->flags & CLIENT_UNIX_SOCKET))
+        DefaultUser->flags & USER_FLAG_NOPASS)
     {
-        char cip[NET_IP_STR_LEN+1] = { 0 };
-        connPeerToString(conn, cip, sizeof(cip)-1, NULL);
-
-        if (strcmp(cip,"127.0.0.1") && strcmp(cip,"::1")) {
+        if (!islocalClient(c)) {
             char *err =
                 "-DENIED Redis is running in protected mode because protected "
                 "mode is enabled and no password is set for the default user. "
diff --git a/src/server.c b/src/server.c
index 8be093cc2..529e5e6b6 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3210,6 +3210,21 @@ 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)) ||
+            (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;
+
+        }
+    }
+
     int is_read_command = (c->cmd->flags & CMD_READONLY) ||
                            (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_READONLY));
     int is_write_command = (c->cmd->flags & CMD_WRITE) ||
diff --git a/src/server.h b/src/server.h
index 6ecad5456..7c0663983 100644
--- a/src/server.h
+++ b/src/server.h
@@ -203,9 +203,10 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT];
 #define CMD_SENTINEL (1ULL<<17)
 #define CMD_ONLY_SENTINEL (1ULL<<18)
 #define CMD_NO_MANDATORY_KEYS (1ULL<<19)
+#define CMD_PROTECTED (1ULL<<20)
 /* Command flags used by the module system. */
-#define CMD_MODULE_GETKEYS (1ULL<<20)  /* Use the modules getkeys interface. */
-#define CMD_MODULE_NO_CLUSTER (1ULL<<21) /* Deny on Redis Cluster. */
+#define CMD_MODULE_GETKEYS (1ULL<<21)  /* Use the modules getkeys interface. */
+#define CMD_MODULE_NO_CLUSTER (1ULL<<22) /* Deny on Redis Cluster. */
 
 /* Command flags that describe ACLs categories. */
 #define ACL_CATEGORY_KEYSPACE (1ULL<<0)
@@ -437,6 +438,11 @@ typedef enum {
 #define SANITIZE_DUMP_YES 1
 #define SANITIZE_DUMP_CLIENTS 2
 
+/* Enable protected config/command */
+#define PROTECTED_ACTION_ALLOWED_NO 0
+#define PROTECTED_ACTION_ALLOWED_YES 1
+#define PROTECTED_ACTION_ALLOWED_LOCAL 2
+
 /* Sets operations codes */
 #define SET_OP_UNION 0
 #define SET_OP_DIFF 1
@@ -1408,6 +1414,9 @@ struct redisServer {
     int io_threads_do_reads;    /* Read and parse from IO threads? */
     int io_threads_active;      /* Is IO threads currently active? */
     long long events_processed_while_blocked; /* processEventsWhileBlocked() */
+    int enable_protected_configs;    /* Enable the modification of protected configs, see PROTECTED_ACTION_ALLOWED_* */
+    int enable_debug_cmd;            /* Enable DEBUG commands, see PROTECTED_ACTION_ALLOWED_* */
+    int enable_module_cmd;           /* Enable MODULE commands, see PROTECTED_ACTION_ALLOWED_* */
 
     /* RDB / AOF loading information */
     volatile sig_atomic_t loading; /* We are loading data from disk if true */
@@ -2339,6 +2348,7 @@ int handleClientsWithPendingWritesUsingThreads(void);
 int handleClientsWithPendingReadsUsingThreads(void);
 int stopThreadedIOIfNeeded(void);
 int clientHasPendingReplies(client *c);
+int islocalClient(client *c);
 int updateClientMemUsage(client *c);
 void updateClientMemUsageBucket(client *c);
 void unlinkClient(client *c);
@@ -2799,6 +2809,7 @@ void rewriteConfigMarkAsProcessed(struct rewriteConfigState *state, const char *
 int rewriteConfig(char *path, int force_all);
 void initConfigValues();
 sds getConfigDebugInfo();
+int allowProtectedAction(int config, client *c);
 
 /* db.c -- Keyspace access API */
 int removeExpire(redisDb *db, robj *key);
diff --git a/tests/assets/default.conf b/tests/assets/default.conf
index d7b8a75c6..ef15b1041 100644
--- a/tests/assets/default.conf
+++ b/tests/assets/default.conf
@@ -25,3 +25,7 @@ appendonly no
 appendfsync everysec
 no-appendfsync-on-rewrite no
 activerehashing yes
+
+enable-protected-configs yes
+enable-debug-command yes
+enable-module-command yes
diff --git a/tests/support/util.tcl b/tests/support/util.tcl
index 08fea1faa..e4e420db7 100644
--- a/tests/support/util.tcl
+++ b/tests/support/util.tcl
@@ -908,6 +908,16 @@ proc delete_lines_with_pattern {filename tmpfilename pattern} {
     file rename -force $tmpfilename $filename
 }
 
+proc get_nonloopback_addr {} {
+    set addrlist [list {}]
+    catch { set addrlist [exec hostname -I] }
+    return [lindex $addrlist 0]
+}
+
+proc get_nonloopback_client {} {
+    return [redis [get_nonloopback_addr] [srv 0 "port"] 0 $::tls]
+}
+
 # The following functions and variables are used only when running large-memory
 # tests. We avoid defining them when not running large-memory tests because the 
 # global variables takes up lots of memory.
diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl
index 4c9652016..eeab7ee06 100644
--- a/tests/unit/introspection.tcl
+++ b/tests/unit/introspection.tcl
@@ -201,6 +201,12 @@ start_server {tags {"introspection"}} {
             cluster-port
             oom-score-adj
             oom-score-adj-values
+            enable-protected-configs
+            enable-debug-command
+            enable-module-command
+            dbfilename
+            logfile
+            dir
         }
 
         if {!$::tls} {
@@ -431,3 +437,27 @@ start_server {tags {"introspection"}} {
     # Config file at this point is at a weird state, and includes all
     # 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}}} {
+    test {cannot modify protected configuration - no} {
+        assert_error "ERR*protected*" {r config set dir somedir}
+        assert_error "ERR*DEBUG command not allowed*" {r DEBUG HELP}
+    } {} {needs:debug}
+}
+
+start_server {config "minimal.conf" tags {"introspection external:skip"} overrides {protected-mode {no} enable-protected-configs {local} enable-debug-command {local}}} {
+    test {cannot modify protected configuration - local} {
+        # verify that for local connection it doesn't error
+        r config set dbfilename somename
+        r DEBUG HELP
+
+        # Get a non-loopback address of this instance for this test.
+        set myaddr [get_nonloopback_addr]
+        if {$myaddr != "" && ![string match {127.*} $myaddr]} {
+            # Non-loopback client should fail
+            set r2 [get_nonloopback_client]
+            assert_error "ERR*protected*" {$r2 config set dir somedir}
+            assert_error "ERR*DEBUG command not allowed*" {$r2 DEBUG HELP}
+        }
+    } {} {needs:debug}
+}
\ No newline at end of file
diff --git a/tests/unit/moduleapi/basics.tcl b/tests/unit/moduleapi/basics.tcl
index 45e02d2cc..be8b697f0 100644
--- a/tests/unit/moduleapi/basics.tcl
+++ b/tests/unit/moduleapi/basics.tcl
@@ -32,3 +32,9 @@ start_server {tags {"modules"}} {
 
     r module unload test
 }
+
+start_server {tags {"modules external:skip"} overrides {enable-module-command no}} {
+    test {module command disabled} {
+       assert_error "ERR*MODULE command not allowed*" {r module load $testmodule}
+    }
+}
\ No newline at end of file
diff --git a/tests/unit/networking.tcl b/tests/unit/networking.tcl
index 49faa0548..559a88e74 100644
--- a/tests/unit/networking.tcl
+++ b/tests/unit/networking.tcl
@@ -130,16 +130,6 @@ start_server {config "minimal.conf" tags {"external:skip"}} {
         r ping
     } {PONG}
 
-    proc get_nonloopback_addr {} {
-        set addrlist [list {}]
-        catch { set addrlist [exec hostname -I] }
-        return [lindex $addrlist 0]
-    }
-
-    proc get_nonloopback_client {} {
-        return [redis [get_nonloopback_addr] [srv 0 "port"] 0 $::tls]
-    }
-
     test {Protected mode works as expected} {
         # Get a non-loopback address of this instance for this test.
         set myaddr [get_nonloopback_addr]