From 2fa43ece12e25ffe4ac19e6259686c146068c580 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 14 Jun 2018 18:57:58 +0200
Subject: [PATCH] Sentinel: add an option to deny online script
 reconfiguration.

The ability of "SENTINEL SET" to change the reconfiguration script at
runtime is a problem even in the security model of Redis: any client
inside the network may set any executable to be ran once a failover is
triggered.

This option adds protection for this problem: by default the two
SENTINEL SET subcommands modifying scripts paths are denied. However the
user is still able to rever that using the Sentinel configuration file
in order to allow such a feature.
---
 sentinel.conf  |  9 +++++++++
 src/sentinel.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/sentinel.conf b/sentinel.conf
index 0e1b266ed..38b097254 100644
--- a/sentinel.conf
+++ b/sentinel.conf
@@ -194,3 +194,12 @@ sentinel failover-timeout mymaster 180000
 #
 # sentinel client-reconfig-script mymaster /var/redis/reconfig.sh
 
+# SECURITY
+#
+# By default SENTINEL SET will not be able to change the notification-script
+# and client-reconfig-script at runtime. This avoids a trivial security issue
+# where clients can set the script to anything and trigger a failover in order
+# to get the program executed.
+
+sentinel deny-scripts-reconfig yes
+
diff --git a/src/sentinel.c b/src/sentinel.c
index ef1be7291..d3818707b 100644
--- a/src/sentinel.c
+++ b/src/sentinel.c
@@ -84,6 +84,7 @@ typedef struct sentinelAddr {
 #define SENTINEL_MAX_PENDING_COMMANDS 100
 #define SENTINEL_ELECTION_TIMEOUT 10000
 #define SENTINEL_MAX_DESYNC 1000
+#define SENTINEL_DEFAULT_DENY_SCRIPTS_RECONFIG 1
 
 /* Failover machine different states. */
 #define SENTINEL_FAILOVER_STATE_NONE 0  /* No failover in progress. */
@@ -241,6 +242,8 @@ struct sentinelState {
     int announce_port;  /* Port that is gossiped to other sentinels if
                            non zero. */
     unsigned long simfailure_flags; /* Failures simulation. */
+    int deny_scripts_reconfig; /* Allow SENTINEL SET ... to change script
+                                  paths at runtime? */
 } sentinel;
 
 /* A script execution job. */
@@ -468,6 +471,7 @@ void initSentinel(void) {
     sentinel.announce_ip = NULL;
     sentinel.announce_port = 0;
     sentinel.simfailure_flags = SENTINEL_SIMFAILURE_NONE;
+    sentinel.deny_scripts_reconfig = SENTINEL_DEFAULT_DENY_SCRIPTS_RECONFIG;
     memset(sentinel.myid,0,sizeof(sentinel.myid));
 }
 
@@ -1684,6 +1688,12 @@ char *sentinelHandleConfiguration(char **argv, int argc) {
     } else if (!strcasecmp(argv[0],"announce-port") && argc == 2) {
         /* announce-port <port> */
         sentinel.announce_port = atoi(argv[1]);
+    } else if (!strcasecmp(argv[0],"deny-scripts-reconfig") && argc == 2) {
+        /* deny-scripts-reconfig <yes|no> */
+        if ((sentinel.deny_scripts_reconfig = yesnotoi(argv[1])) == -1) {
+            return "Please specify yes or no for the "
+                   "deny-scripts-reconfig options.";
+        }
     } else {
         return "Unrecognized sentinel configuration statement.";
     }
@@ -1704,6 +1714,12 @@ void rewriteConfigSentinelOption(struct rewriteConfigState *state) {
     line = sdscatprintf(sdsempty(), "sentinel myid %s", sentinel.myid);
     rewriteConfigRewriteLine(state,"sentinel",line,1);
 
+    /* sentinel deny-scripts-reconfig. */
+    line = sdscatprintf(sdsempty(), "sentinel deny-scripts-reconfig %s",
+        sentinel.deny_scripts_reconfig ? "yes" : "no");
+    rewriteConfigRewriteLine(state,"sentinel",line,
+        sentinel.deny_scripts_reconfig != SENTINEL_DEFAULT_DENY_SCRIPTS_RECONFIG);
+
     /* For every master emit a "sentinel monitor" config entry. */
     di = dictGetIterator(sentinel.masters);
     while((de = dictNext(di)) != NULL) {
@@ -3331,6 +3347,14 @@ void sentinelSetCommand(client *c) {
             changes++;
        } else if (!strcasecmp(option,"notification-script")) {
             /* notification-script <path> */
+            if (sentinel.deny_scripts_reconfig) {
+                addReplyError(c,
+                    "Reconfiguration of scripts path is denied for "
+                    "security reasons. Check the deny-scripts-reconfig "
+                    "configuration directive in your Sentinel configuration");
+                return;
+            }
+
             if (strlen(value) && access(value,X_OK) == -1) {
                 addReplyError(c,
                     "Notification script seems non existing or non executable");
@@ -3342,6 +3366,14 @@ void sentinelSetCommand(client *c) {
             changes++;
        } else if (!strcasecmp(option,"client-reconfig-script")) {
             /* client-reconfig-script <path> */
+            if (sentinel.deny_scripts_reconfig) {
+                addReplyError(c,
+                    "Reconfiguration of scripts path is denied for "
+                    "security reasons. Check the deny-scripts-reconfig "
+                    "configuration directive in your Sentinel configuration");
+                return;
+            }
+
             if (strlen(value) && access(value,X_OK) == -1) {
                 addReplyError(c,
                     "Client reconfiguration script seems non existing or "
-- 
GitLab