socket/glusterd/client: enable SSL for management

The feature is controlled by presence of the following file:

	/var/lib/glusterd/secure-access

See the comment near the definition of SECURE_ACCESS_FILE in glusterfs.h
for the rationale.  With this enabled, the following rules apply to
connections:

	UNIX-domain sockets never have SSL.

	Management-port sockets (both connecting and accepting, in
	daemons and CLI) have SSL based on presence of the file.

	Other IP sockets have SSL based on the existing client.ssl and
	server.ssl volume options.

Transport multi-threading is explicitly turned off in glusterd (it would
otherwise be turned on when SSL is) due to multi-threading issues.
Tests have been elided to avoid risk of leaving a file which will cause
all subsequent tests to run with management SSL still enabled.

IMPLEMENTATION NOTE
The implementation is a bit messy, and consists of two stages.  First we
decide whether to set the relevant fields in our context structure, based
on presence of the sentinel file OR a command-line override.  Later we
decide whether a particular connection should actually use SSL, based on the
context flags plus what kind of connection we're making[1] and what kind of
daemon we're in[2].

[1] inbound, outbound to glusterd port, other outbound
[2] glusterd, glusterfsd, other

TESTING NOTE
Instead of just running one special test for this feature, the ideal
would be to run all tests with management SSL enabled.  However, it
would be inappropriate or premature to set up an optional feature in the
patch itself.  Therefore, the method of choice is to submit a separate
patch on top, which modifies "cleanup" in include.rc to recreate the
secure-access file and associated SSL certificate/key files before each
test.

Change-Id: I0e04d6d08163893e24ec8c031748c5c447d7f780
BUG: 1114604
Signed-off-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-on: http://review.gluster.org/8094
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

9 files changed, 197 insertions, 33 deletions

diff --git a/cli/src/cli.c b/cli/src/cli.c
index 745b0b45bf5..fa3c747d154 100644
--- a/cli/src/cli.c
+++ b/cli/src/cli.c
@@ -297,7 +297,8 @@ cli_rpc_notify (struct rpc_clnt *rpc, void *mydata, rpc_clnt_event_t event,
 int
 cli_opt_parse (char *opt, struct cli_state *state)
 {
-        char *oarg;
+        char            *oarg           = NULL;
+        gf_boolean_t    secure_mgmt_tmp = 0;
 
         if (strcmp (opt, "") == 0)
                 return 1;
@@ -370,6 +371,20 @@ cli_opt_parse (char *opt, struct cli_state *state)
                 return 0;
         }
 
+        oarg = strtail (opt, "secure-mgmt=");
+        if (oarg) {
+                if (gf_string2boolean(oarg,&secure_mgmt_tmp) == 0) {
+                        if (secure_mgmt_tmp) {
+                                /* See declaration for why this is an int. */
+                                state->ctx->secure_mgmt = 1;
+                        }
+                }
+                else {
+                        cli_err ("invalide secure-mgmt value (ignored)");
+                }
+                return 0;
+        }
+
         return -1;
 }
 
@@ -384,6 +399,11 @@ parse_cmdline (int argc, char *argv[], struct cli_state *state)
         state->argc=argc-1;
         state->argv=&argv[1];
 
+        /* Do this first so that an option can override. */
+        if (access(SECURE_ACCESS_FILE,F_OK) == 0) {
+                state->ctx->secure_mgmt = 1;
+        }
+
         for (i = 0; i < state->argc; i++) {
                 opt = strtail (state->argv[i], "--");
                 if (opt) {
@@ -546,7 +566,6 @@ cli_rpc_init (struct cli_state *state)
         int                     port = CLI_GLUSTERD_PORT;
         xlator_t                *this = NULL;
 
-
         this = THIS;
         cli_rpc_prog = &cli_prog;
         options = dict_new ();
@@ -565,7 +584,8 @@ cli_rpc_init (struct cli_state *state)
                                                         0);
                 if (ret)
                         goto out;
-        } else if (state->remote_host) {
+        }
+        else if (state->remote_host) {
                 gf_log ("cli", GF_LOG_INFO, "Connecting to remote glusterd at "
                         "%s", state->remote_host);
                 ret = dict_set_str (options, "remote-host", state->remote_host);
@@ -583,7 +603,8 @@ cli_rpc_init (struct cli_state *state)
                                     "inet");
                 if (ret)
                         goto out;
-        } else {
+        }
+        else {
                 gf_log ("cli", GF_LOG_DEBUG, "Connecting to glusterd using "
                         "default socket");
                 ret = rpc_transport_unix_options_build
diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
index 79e6e593a53..3c92783cb3d 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -218,6 +218,8 @@ static struct argp_option gf_options[] = {
         {"use-readdirp", ARGP_FUSE_USE_READDIRP_KEY, "BOOL", OPTION_ARG_OPTIONAL,
          "Use readdirp mode in fuse kernel module"
          " [default: \"off\"]"},
+        {"secure-mgmt", ARGP_SECURE_MGMT_KEY, "BOOL", OPTION_ARG_OPTIONAL,
+         "Override default for secure (SSL) management connections"},
         {0, 0, 0, 0, "Miscellaneous Options:"},
         {0, }
 };
@@ -1145,6 +1147,19 @@ parse_opts (int key, char *arg, struct argp_state *state)
                 }
 
                 break;
+
+        case ARGP_SECURE_MGMT_KEY:
+                if (!arg)
+                        arg = "yes";
+
+                if (gf_string2boolean (arg, &b) == 0) {
+                        cmd_args->secure_mgmt = b ? 1 : 0;
+                        break;
+                }
+
+                argp_failure (state, -1, 0,
+                              "unknown secure-mgmt setting \"%s\"", arg);
+                break;
 	}
 
         return 0;
@@ -1493,8 +1508,15 @@ parse_cmdline (int argc, char *argv[], glusterfs_ctx_t *ctx)
 
         cmd_args = &ctx->cmd_args;
 
+        /* Do this before argp_parse so it can be overridden. */
+        if (access(SECURE_ACCESS_FILE,F_OK) == 0) {
+                cmd_args->secure_mgmt = 1;
+        }
+
         argp_parse (&argp, argc, argv, ARGP_IN_ORDER, NULL, cmd_args);
 
+        ctx->secure_mgmt = cmd_args->secure_mgmt;
+
         if (ENABLE_DEBUG_MODE == cmd_args->debug_mode) {
                 cmd_args->log_level = GF_LOG_DEBUG;
                 cmd_args->log_file = gf_strdup ("/dev/stderr");
@@ -1985,6 +2007,7 @@ main (int argc, char *argv[])
         if (ret)
                 goto out;
 
+
         /* log the version of glusterfs running here along with the actual
            command line options. */
         {
diff --git a/glusterfsd/src/glusterfsd.h b/glusterfsd/src/glusterfsd.h
index a75369a24f5..41f7b1d9a38 100644
--- a/glusterfsd/src/glusterfsd.h
+++ b/glusterfsd/src/glusterfsd.h
@@ -91,6 +91,7 @@ enum argp_option_keys {
         ARGP_LOG_FORMAT                   = 169,
         ARGP_LOG_BUF_SIZE                 = 170,
         ARGP_LOG_FLUSH_TIMEOUT            = 171,
+        ARGP_SECURE_MGMT_KEY              = 172,
 };
 
 struct _gfd_vol_top_priv_t {
diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index 4867da42aff..3b0cc4b4eaa 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -414,6 +414,9 @@ struct _cmd_args {
         int             brick_port;
         char           *brick_name;
         int             brick_port2;
+
+        /* Should management connections use SSL? */
+        int             secure_mgmt;
 };
 typedef struct _cmd_args cmd_args_t;
 
@@ -435,6 +438,13 @@ typedef struct _glusterfs_graph glusterfs_graph_t;
 
 typedef int32_t (*glusterfsd_mgmt_event_notify_fn_t) (int32_t event, void *data,
                                                       ...);
+
+typedef enum {
+        MGMT_SSL_NEVER = 0,
+        MGMT_SSL_COPY_IO,
+        MGMT_SSL_ALWAYS
+} mgmt_ssl_t;
+
 struct _glusterfs_ctx {
         cmd_args_t          cmd_args;
         char               *process_uuid;
@@ -483,6 +493,26 @@ struct _glusterfs_ctx {
         int                 daemon_pipe[2];
 
         struct clienttable *clienttable;
+
+        /*
+         * Should management connections use SSL?  This is the only place we
+         * can put it where both daemon-startup and socket code will see it.
+         *
+         * Why is it an int?  Because we're included before common-utils.h,
+         * which defines gf_boolean_t (what we really want).  It doesn't make
+         * any sense, but it's not worth turning the codebase upside-down to
+         * fix it.  Thus, an int.
+         */
+        int                 secure_mgmt;
+
+        /*
+         * Should *our* server/inbound connections use SSL?  This is only true
+         * if we're glusterd and secure_mgmt is set, or if we're glusterfsd
+         * and SSL is set on the I/O path.  It should never be set e.g. for
+         * NFS.
+         */
+        mgmt_ssl_t          secure_srvr;
+
 };
 typedef struct _glusterfs_ctx glusterfs_ctx_t;
 
@@ -528,6 +558,26 @@ struct gf_flock {
  */
 #define GF_UNUSED __attribute__((unused))
 
+/*
+ * If present, this has the following effects:
+ *
+ *      glusterd enables privileged commands over TCP
+ *
+ *      all code enables SSL for outbound connections to management port
+ *
+ *      glusterd enables SSL for inbound connections
+ *
+ * Servers and clients enable/disable SSL among themselves by other means.
+ * Making secure management connections conditional on a file is a bit of a
+ * hack, but we don't have any other place for such global settings across
+ * all of the affected components.  Making it a compile-time option would
+ * reduce functionality, both for users and for testing (which can now be
+ * done using secure connections for all tests without change elsewhere).
+ *
+ * Nonetheless, TBD: define in terms of build-time PREFIX
+ */
+#define SECURE_ACCESS_FILE      "/var/lib/glusterd/secure-access"
+
 int glusterfs_graph_prepare (glusterfs_graph_t *graph, glusterfs_ctx_t *ctx);
 int glusterfs_graph_destroy (glusterfs_graph_t *graph);
 int glusterfs_graph_activate (glusterfs_graph_t *graph, glusterfs_ctx_t *ctx);
diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c
index ccef2f605cc..e969a5cf7fd 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -318,6 +318,7 @@ ssl_teardown_connection (socket_private_t *priv)
         SSL_clear(priv->ssl_ssl);
         SSL_free(priv->ssl_ssl);
         priv->ssl_ssl = NULL;
+        priv->use_ssl = _gf_false;
 }
 
 
@@ -2563,12 +2564,29 @@ socket_server_event_handler (int fd, int idx, void *data,
                         new_trans->listener = this;
                         new_priv = new_trans->private;
 
-			new_priv->use_ssl = priv->use_ssl;
+                        if (new_sockaddr.ss_family == AF_UNIX) {
+                                new_priv->use_ssl = _gf_false;
+                        }
+                        else {
+                                switch (priv->srvr_ssl) {
+                                case MGMT_SSL_ALWAYS:
+                                        /* Glusterd with secure_mgmt. */
+                                        new_priv->use_ssl = _gf_true;
+                                        break;
+                                case MGMT_SSL_COPY_IO:
+                                        /* Glusterfsd. */
+                                        new_priv->use_ssl = priv->ssl_enabled;
+                                        break;
+                                default:
+                                        new_priv->use_ssl = _gf_false;
+                                }
+                        }
+
 			new_priv->sock = new_sock;
 			new_priv->own_thread = priv->own_thread;
 
                         new_priv->ssl_ctx = priv->ssl_ctx;
-			if (priv->use_ssl && !priv->own_thread) {
+			if (new_priv->use_ssl && !new_priv->own_thread) {
 				cname = ssl_setup_connection(new_trans,1);
                                 if (!cname) {
 					gf_log(this->name,GF_LOG_ERROR,
@@ -2692,6 +2710,23 @@ socket_connect_error_cbk (void *opaque)
         return NULL;
 }
 
+static void
+socket_fix_ssl_opts (rpc_transport_t *this, socket_private_t *priv,
+                     uint16_t port)
+{
+        if (port == GF_DEFAULT_SOCKET_LISTEN_PORT) {
+                gf_log (this->name, GF_LOG_DEBUG,
+                        "%s SSL for portmapper connection",
+                        priv->mgmt_ssl ? "enabling" : "disabling");
+                priv->use_ssl = priv->mgmt_ssl;
+        }
+        else if (priv->ssl_enabled && !priv->use_ssl) {
+                gf_log(this->name,GF_LOG_DEBUG,
+                       "re-enabling SSL for I/O connection");
+                priv->use_ssl = _gf_true;
+        }
+}
+
 static int
 socket_connect (rpc_transport_t *this, int port)
 {
@@ -2744,23 +2779,16 @@ socket_connect (rpc_transport_t *this, int port)
                         goto unlock;
                 }
 
-                if (port > 0) {
-                        sock_union.sin.sin_port = htons (port);
-                }
-                if (ntohs(sock_union.sin.sin_port) ==
-                    GF_DEFAULT_SOCKET_LISTEN_PORT) {
-                        if (priv->use_ssl) {
-                                gf_log(this->name,GF_LOG_DEBUG,
-                                "disabling SSL for portmapper connection");
-                                priv->use_ssl = _gf_false;
-                        }
+                if (sa_family == AF_UNIX) {
+                        priv->ssl_enabled = _gf_false;
+                        priv->mgmt_ssl = _gf_false;
                 }
                 else {
-                        if (priv->ssl_enabled && !priv->use_ssl) {
-                                gf_log(this->name,GF_LOG_DEBUG,
-                                       "re-enabling SSL for I/O connection");
-                                priv->use_ssl = _gf_true;
+                        if (port > 0) {
+                                sock_union.sin.sin_port = htons (port);
                         }
+                        socket_fix_ssl_opts (this, priv,
+                                             ntohs(sock_union.sin.sin_port));
                 }
 
                 memcpy (&this->peerinfo.sockaddr, &sock_union.storage,
@@ -3621,6 +3649,8 @@ socket_init (rpc_transport_t *this)
 				"invalid value given for ssl-enabled boolean");
 		}
 	}
+        priv->mgmt_ssl = this->ctx->secure_mgmt;
+        priv->srvr_ssl = this->ctx->secure_srvr;
 
         priv->ssl_own_cert = DEFAULT_CERT_PATH;
 	if (dict_get_str(this->options,SSL_OWN_CERT_OPT,&optstr) == 0) {
@@ -3656,8 +3686,11 @@ socket_init (rpc_transport_t *this)
         priv->ssl_ca_list = gf_strdup(priv->ssl_ca_list);
 
         gf_log(this->name, priv->ssl_enabled ? GF_LOG_INFO: GF_LOG_DEBUG,
-               "SSL support is %s",
+               "SSL support on the I/O path is %s",
                priv->ssl_enabled ? "ENABLED" : "NOT enabled");
+        gf_log(this->name, priv->mgmt_ssl ? GF_LOG_INFO: GF_LOG_DEBUG,
+               "SSL support for glusterd is %s",
+               priv->mgmt_ssl ? "ENABLED" : "NOT enabled");
         /*
          * This might get overridden temporarily in socket_connect (q.v.)
          * if we're using the glusterd portmapper.
@@ -3666,8 +3699,9 @@ socket_init (rpc_transport_t *this)
 
 	priv->own_thread = priv->use_ssl;
 	if (dict_get_str(this->options,OWN_THREAD_OPT,&optstr) == 0) {
+                gf_log (this->name, GF_LOG_INFO, "OWN_THREAD_OPT found");
                 if (gf_string2boolean (optstr, &priv->own_thread) != 0) {
-                        gf_log (this->name, GF_LOG_ERROR,
+                        gf_log (this->name, GF_LOG_WARNING,
 				"invalid value given for own-thread boolean");
 		}
 	}
@@ -3684,7 +3718,7 @@ socket_init (rpc_transport_t *this)
                         "using cipher list %s", cipher_list);
         }
 
-	if (priv->use_ssl) {
+	if (priv->ssl_enabled || priv->mgmt_ssl) {
 		SSL_library_init();
 		SSL_load_error_strings();
 		priv->ssl_meth = (SSL_METHOD *)TLSv1_method();
diff --git a/rpc/rpc-transport/socket/src/socket.h b/rpc/rpc-transport/socket/src/socket.h
index e0b412fcce1..33c936938eb 100644
--- a/rpc/rpc-transport/socket/src/socket.h
+++ b/rpc/rpc-transport/socket/src/socket.h
@@ -217,7 +217,9 @@ typedef struct {
         int                    keepaliveintvl;
         uint32_t               backlog;
         gf_boolean_t           read_fail_log;
-        gf_boolean_t           ssl_enabled;
+        gf_boolean_t           ssl_enabled;     /* outbound I/O */
+        gf_boolean_t           mgmt_ssl;        /* outbound mgmt */
+        mgmt_ssl_t             srvr_ssl;
 	gf_boolean_t           use_ssl;
 	SSL_METHOD            *ssl_meth;
 	SSL_CTX               *ssl_ctx;
diff --git a/tests/bugs/bug-873367.t b/tests/bugs/bug-873367.t
index 4849c2fea31..771c8628219 100755
--- a/tests/bugs/bug-873367.t
+++ b/tests/bugs/bug-873367.t
@@ -13,14 +13,14 @@ rm -f $SSL_BASE/glusterfs.*
 mkdir -p $B0/1
 mkdir -p $M0
 
-TEST glusterd
-TEST pidof glusterd
-TEST $CLI volume info;
-
 TEST openssl genrsa -out $SSL_KEY 1024
 TEST openssl req -new -x509 -key $SSL_KEY -subj /CN=Anyone -out $SSL_CERT
 ln $SSL_CERT $SSL_CA
 
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume info;
+
 TEST $CLI volume create $V0 $H0:$B0/1
 TEST $CLI volume set $V0 server.ssl on
 TEST $CLI volume set $V0 client.ssl on
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index 3e8a3e2e17a..adf58cc7dde 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -68,7 +68,7 @@ rpcsvc_cbk_program_t glusterd_cbk_prog = {
 
 struct rpcsvc_program *gd_inet_programs[] = {
         &gd_svc_peer_prog,
-        &gd_svc_cli_trusted_progs,
+        &gd_svc_cli_trusted_progs, /* Must be index 1 for secure_mgmt! */
         &gd_svc_mgmt_prog,
         &gd_svc_mgmt_v3_prog,
         &gluster_pmap_prog,
@@ -1327,8 +1327,34 @@ init (xlator_t *this)
                 goto out;
         }
 
+        if (this->ctx->secure_mgmt) {
+                /*
+                 * The socket code will turn on SSL based on the same check,
+                 * but that will by default turn on own-thread as well and
+                 * we're not multi-threaded enough to handle that.  Thus, we
+                 * override the value here.
+                 */
+                ret = dict_set_str (this->options,
+                                    "transport.socket.own-thread", "off");
+                if (ret != 0) {
+                        gf_log (this->name, GF_LOG_ERROR,
+                                "failed to clear own-thread");
+                        goto out;
+                }
+                /*
+                 * With strong authentication, we can afford to allow
+                 * privileged operations over TCP.
+                 */
+                gd_inet_programs[1] = &gd_svc_cli_prog;
+                /*
+                 * This is the only place where we want secure_srvr to reflect
+                 * the management-plane setting.
+                 */
+                this->ctx->secure_srvr = MGMT_SSL_ALWAYS;
+        }
+
         /*
-         * only one (atmost a pair - rdma and socket) listener for
+         * only one (at most a pair - rdma and socket) listener for
          * glusterd1_mop_prog, gluster_pmap_prog and gluster_handshake_prog.
          */
         ret = rpcsvc_create_listeners (rpc, this->options, this->name);
@@ -1352,9 +1378,10 @@ init (xlator_t *this)
                 }
         }
 
-        /* Start a unix domain socket listener just for cli commands
-         * This should prevent ports from being wasted by being in TIMED_WAIT
-         * when cli commands are done continuously
+        /*
+         * Start a unix domain socket listener just for cli commands This
+         * should prevent ports from being wasted by being in TIMED_WAIT when
+         * cli commands are done continuously
          */
         uds_rpc = glusterd_init_uds_listener (this);
         if (uds_rpc == NULL) {
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index 3de856e8e09..6c4f81067cb 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -913,6 +913,12 @@ init (xlator_t *this)
                 goto out;
         }
 
+        /*
+         * This is the only place where we want secure_srvr to reflect
+         * the data-plane setting.
+         */
+        this->ctx->secure_srvr = MGMT_SSL_COPY_IO;
+
         ret = rpcsvc_create_listeners (conf->rpc, this->options,
                                        this->name);
         if (ret < 1) {