Blob Blame History Raw
From 8f44b5a73f2c838ed0c498370d525ab702be6da2 Mon Sep 17 00:00:00 2001
From: Mohammed Rafi KC <rkavunga@redhat.com>
Date: Mon, 23 Apr 2018 09:02:09 +0530
Subject: [PATCH 656/656] server/auth: fix regression in honouring auth.allow

The patch 16d6904d54e7409f95e9e893e131b6bf11f0e4c7 to fix
problem with shared storage introduced a regression. auth.allow
is now not honouring. This changes will fix the issue with
auth.allow

The option is invalid when SSL/TLS is in use, at which point
the SSL/TLS certificate user name is used to validate and
hence authorize the right user. This expects TLS allow rules
to be setup correctly rather than the default *.

credit : Shyam

Change-Id: Ifcb202b16a451e5ef2ea04a6486276ba163c07e4
BUG: 1570906
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/136453
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
 xlators/mgmt/glusterd/src/glusterd-volgen.c    |   3 +-
 xlators/protocol/auth/login/src/login.c        | 142 ++++++++++++-------------
 xlators/protocol/server/src/authenticate.c     |  32 ++----
 xlators/protocol/server/src/authenticate.h     |   4 +-
 xlators/protocol/server/src/server-handshake.c |   2 +-
 xlators/protocol/server/src/server.c           |   4 +-
 xlators/protocol/server/src/server.h           |   2 -
 7 files changed, 85 insertions(+), 104 deletions(-)

diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 02e8393..4198be8 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -2226,7 +2226,6 @@ brick_graph_add_server (volgen_graph_t *graph, glusterd_volinfo_t *volinfo,
 
 
         if (volname && !strcmp (volname, GLUSTER_SHARED_STORAGE)) {
-
                 memset (key, 0, sizeof (key));
                 snprintf (key, sizeof (key), "strict-auth-accept");
 
@@ -5522,7 +5521,7 @@ generate_client_volfiles (glusterd_volinfo_t *volinfo,
 
 
         if (volname && !strcmp (volname, GLUSTER_SHARED_STORAGE) &&
-             client_type != GF_CLIENT_TRUSTED) {
+            client_type != GF_CLIENT_TRUSTED) {
                 /*
                  * shared storage volume cannot be mounted from non trusted
                  * nodes. So we are not creating volfiles for non-trusted
diff --git a/xlators/protocol/auth/login/src/login.c b/xlators/protocol/auth/login/src/login.c
index e918d38..4310027 100644
--- a/xlators/protocol/auth/login/src/login.c
+++ b/xlators/protocol/auth/login/src/login.c
@@ -11,18 +11,78 @@
 #include <fnmatch.h>
 #include "authenticate.h"
 
-auth_result_t gf_authenticate_user (dict_t *input_params, dict_t *config_params,
-                                    char *username, char *password,
-                                    gf_boolean_t using_ssl) {
+/* Note on strict_auth
+ * - Strict auth kicks in when authentication is using the username, password
+ *   in the volfile to login
+ * - If enabled, auth is rejected if the username and password is not matched
+ *   or is not present
+ * - When using SSL names, this is automatically strict, and allows only those
+ *   names that are present in the allow list, IOW strict auth checking has no
+ *   implication when using SSL names
+*/
+
+auth_result_t gf_auth (dict_t *input_params, dict_t *config_params)
+{
         auth_result_t  result  = AUTH_DONT_CARE;
         int             ret             = 0;
         data_t          *allow_user     = NULL;
+        data_t          *username_data  = NULL;
         data_t          *passwd_data    = NULL;
+        data_t          *password_data  = NULL;
+        char            *username       = NULL;
+        char            *password       = NULL;
         char            *brick_name     = NULL;
         char            *searchstr      = NULL;
         char            *username_str   = NULL;
         char            *tmp            = NULL;
         char            *username_cpy   = NULL;
+        gf_boolean_t    using_ssl       = _gf_false;
+        gf_boolean_t    strict_auth     = _gf_false;
+
+        username_data = dict_get (input_params, "ssl-name");
+        if (username_data) {
+                gf_log ("auth/login", GF_LOG_INFO,
+                        "connecting user name: %s", username_data->data);
+                using_ssl = _gf_true;
+        } else {
+                ret = dict_get_str_boolean (config_params, "strict-auth-accept",
+                                            _gf_false);
+                if (ret == -1)
+                        strict_auth = _gf_false;
+                else
+                        strict_auth = ret;
+
+                username_data = dict_get (input_params, "username");
+                if (!username_data) {
+                        if (strict_auth) {
+                                gf_log ("auth/login", GF_LOG_DEBUG,
+                                        "username not found, strict auth"
+                                        " configured returning REJECT");
+                                result = AUTH_REJECT;
+                        } else {
+                                gf_log ("auth/login", GF_LOG_DEBUG,
+                                        "username not found, returning"
+                                        " DONT-CARE");
+                        }
+                        goto out;
+                }
+                password_data = dict_get (input_params, "password");
+                if (!password_data) {
+                        if (strict_auth) {
+                                gf_log ("auth/login", GF_LOG_DEBUG,
+                                        "password not found, strict auth"
+                                        " configured returning REJECT");
+                                result = AUTH_REJECT;
+                        } else {
+                                gf_log ("auth/login", GF_LOG_WARNING,
+                                        "password not found, returning"
+                                        " DONT-CARE");
+                        }
+                        goto out;
+                }
+                password = data_to_str (password_data);
+        }
+        username = data_to_str (username_data);
 
         brick_name = data_to_str (dict_get (input_params, "remote-subvolume"));
         if (!brick_name) {
@@ -35,10 +95,10 @@ auth_result_t gf_authenticate_user (dict_t *input_params, dict_t *config_params,
         ret = gf_asprintf (&searchstr, "auth.login.%s.%s", brick_name,
                            using_ssl ? "ssl-allow" : "allow");
         if (-1 == ret) {
-                gf_log ("auth/login", GF_LOG_WARNING,
+                gf_log ("auth/login", GF_LOG_ERROR,
                         "asprintf failed while setting search string, "
-                        "returning AUTH_STRICT_ACCEPT");
-                result = AUTH_STRICT_ACCEPT;
+                        "returning REJECT");
+                result = AUTH_REJECT;
                 goto out;
         }
 
@@ -58,7 +118,7 @@ auth_result_t gf_authenticate_user (dict_t *input_params, dict_t *config_params,
                  * who do provide a valid username and password (in fact that's
                  * pretty much the only thing we use non-SSL login auth for),
                  * but they are allowed to connect.  It's wrong, but it's not
-                 * worth changing elsewhere.  Therefore, we do the same thing
+                 * worth changing elsewhere.  Therefore, we do the sane thing
                  * only for SSL here.
                  *
                  * For SSL, if there's a list *you must be on it*.  Note that
@@ -67,19 +127,14 @@ auth_result_t gf_authenticate_user (dict_t *input_params, dict_t *config_params,
                  * disabled, though authentication and encryption are still
                  * active.
                  *
-                 * If option strict-auth-accept is enabled, connection
-                 * will be rejected if you don't have any matching
-                 * username and password (password is only for non-ssl users).
+                 * Read NOTE on strict_auth above.
                  */
-                if (using_ssl) {
+                if (using_ssl || strict_auth) {
                         result = AUTH_REJECT;
                 }
                 username_cpy = gf_strdup (allow_user->data);
-                if (!username_cpy) {
-                        if (!using_ssl)
-                                result = AUTH_STRICT_ACCEPT;
+                if (!username_cpy)
                         goto out;
-                }
 
                 username_str = strtok_r (username_cpy, " ,", &tmp);
 
@@ -101,7 +156,6 @@ auth_result_t gf_authenticate_user (dict_t *input_params, dict_t *config_params,
                                 if (-1 == ret) {
                                         gf_log ("auth/login", GF_LOG_WARNING,
                                                 "asprintf failed while setting search string");
-                                        result = AUTH_STRICT_ACCEPT;
                                         goto out;
                                 }
                                 passwd_data = dict_get (config_params, searchstr);
@@ -127,65 +181,11 @@ auth_result_t gf_authenticate_user (dict_t *input_params, dict_t *config_params,
                         }
                         username_str = strtok_r (NULL, " ,", &tmp);
                 }
-        } else {
-                result = AUTH_STRICT_ACCEPT;
         }
+
 out:
         GF_FREE (username_cpy);
-        return result;
-}
-
-auth_result_t gf_auth (dict_t *input_params, dict_t *config_params)
-{
-        auth_result_t   result           = AUTH_DONT_CARE;
-        auth_result_t   ssl_result       = AUTH_DONT_CARE;
-        data_t          *username_data  = NULL;
-        data_t          *password_data  = NULL;
-        char            *username       = NULL;
-        char            *password       = NULL;
-        gf_boolean_t    using_ssl       = _gf_false;
 
-        username_data = dict_get (input_params, "username");
-        if (!username_data) {
-                gf_log ("auth/login", GF_LOG_DEBUG,
-                        "username not found, returning AUTH_STRICT_ACCEPT");
-                result = AUTH_STRICT_ACCEPT;
-                goto out;
-        }
-        password_data = dict_get (input_params, "password");
-        if (!password_data) {
-                gf_log ("auth/login", GF_LOG_WARNING,
-                        "password not found, returning AUTH_STRICT_ACCEPT");
-                result = AUTH_STRICT_ACCEPT;
-                goto out;
-        }
-        password = data_to_str (password_data);
-        username = data_to_str (username_data);
-
-        result = gf_authenticate_user (input_params, config_params,
-                                                username, password, using_ssl);
-
-        username_data = dict_get (input_params, "ssl-name");
-        if (username_data) {
-                gf_log ("auth/login", GF_LOG_INFO,
-                        "connecting user name: %s", username_data->data);
-                username = data_to_str (username_data);
-                using_ssl = _gf_true;
-                ssl_result = gf_authenticate_user (input_params, config_params,
-                                                   username, NULL, using_ssl);
-                if (ssl_result == AUTH_ACCEPT && result != AUTH_ACCEPT) {
-                        /*
-                         * Here, ssl authentication returns true, but non-ssl
-                         * authentication returns differnt result. We are
-                         * calling for a strict auth check in this case.
-                         */
-                        result = AUTH_STRICT_ACCEPT;
-                } else {
-                        result = ssl_result;
-                }
-        }
-
-out:
         return result;
 }
 
diff --git a/xlators/protocol/server/src/authenticate.c b/xlators/protocol/server/src/authenticate.c
index 977ed75..c000776 100644
--- a/xlators/protocol/server/src/authenticate.c
+++ b/xlators/protocol/server/src/authenticate.c
@@ -18,9 +18,7 @@
 #include <dlfcn.h>
 #include <errno.h>
 #include "authenticate.h"
-#include "authenticate.h"
 #include "server-messages.h"
-#include "server.h"
 
 static int
 init (dict_t *this, char *key, data_t *value, void *data)
@@ -175,7 +173,6 @@ gf_auth_one_method (dict_t *this, char *key, data_t *value, void *data)
 {
         gf_auth_args_t  *args   = data;
         auth_handle_t   *handle = NULL;
-        int64_t          result;
 
         if (!value) {
                 return 0;
@@ -186,13 +183,10 @@ gf_auth_one_method (dict_t *this, char *key, data_t *value, void *data)
                 return 0;
         }
 
-        result = handle->authenticate (args->iparams, args->cparams);
-        switch (result) {
+        switch (handle->authenticate (args->iparams, args->cparams)) {
         case AUTH_ACCEPT:
-        case AUTH_STRICT_ACCEPT:
-                if (args->result != AUTH_REJECT &&
-                    args->result != AUTH_STRICT_ACCEPT) {
-                        args->result = result;
+                if (args->result != AUTH_REJECT) {
+                        args->result = AUTH_ACCEPT;
                 }
                 /* FALLTHROUGH */
         default:
@@ -204,8 +198,9 @@ gf_auth_one_method (dict_t *this, char *key, data_t *value, void *data)
 }
 
 auth_result_t
-gf_authenticate (server_conf_t *conf, dict_t *input_params,
-                 dict_t *config_params, dict_t *auth_modules)
+gf_authenticate (dict_t *input_params,
+                 dict_t *config_params,
+                 dict_t *auth_modules)
 {
         char *name = NULL;
         data_t *peerinfo_data = NULL;
@@ -215,19 +210,9 @@ gf_authenticate (server_conf_t *conf, dict_t *input_params,
         args.cparams = config_params;
         args.result = AUTH_DONT_CARE;
 
-        GF_VALIDATE_OR_GOTO ("authentication", conf, out);
         dict_foreach (auth_modules, gf_auth_one_method, &args);
 
-        switch (args.result) {
-        case AUTH_STRICT_ACCEPT:
-                if (!conf->strict_auth_enabled) {
-                        args.result = AUTH_ACCEPT;
-                        break;
-                }
-                gf_msg ("auth", GF_LOG_ERROR, 0, PS_MSG_REMOTE_CLIENT_REFUSED,
-                        "Authentication is failed due to the strict options.");
-                /* Fallthrough */
-        case AUTH_DONT_CARE:
+        if (AUTH_DONT_CARE == args.result) {
                 peerinfo_data = dict_get (input_params, "peer-info-name");
 
                 if (peerinfo_data) {
@@ -238,9 +223,8 @@ gf_authenticate (server_conf_t *conf, dict_t *input_params,
                         "no authentication module is interested in "
                         "accepting remote-client %s", name);
                 args.result = AUTH_REJECT;
-                /* Fallthrough */
         }
-out:
+
         return args.result;
 }
 
diff --git a/xlators/protocol/server/src/authenticate.h b/xlators/protocol/server/src/authenticate.h
index d5971d3..5f92183 100644
--- a/xlators/protocol/server/src/authenticate.h
+++ b/xlators/protocol/server/src/authenticate.h
@@ -25,8 +25,7 @@
 typedef enum {
         AUTH_ACCEPT,
         AUTH_REJECT,
-        AUTH_DONT_CARE,
-        AUTH_STRICT_ACCEPT
+        AUTH_DONT_CARE
 } auth_result_t;
 
 typedef auth_result_t (*auth_fn_t) (dict_t *input_params,
@@ -40,5 +39,6 @@ typedef struct {
 
 int32_t gf_auth_init (xlator_t *xl, dict_t *auth_modules);
 void gf_auth_fini (dict_t *auth_modules);
+auth_result_t gf_authenticate (dict_t *, dict_t *, dict_t *);
 
 #endif /* _AUTHENTICATE_H */
diff --git a/xlators/protocol/server/src/server-handshake.c b/xlators/protocol/server/src/server-handshake.c
index b67ed47..f6d5a0b 100644
--- a/xlators/protocol/server/src/server-handshake.c
+++ b/xlators/protocol/server/src/server-handshake.c
@@ -765,7 +765,7 @@ server_setvolume (rpcsvc_request_t *req)
                         PS_MSG_CLIENT_VERSION_NOT_SET,
                         "client-version not set, may be of older version");
 
-        ret = gf_authenticate (conf, params, config_params,
+        ret = gf_authenticate (params, config_params,
                                conf->auth_modules);
 
         if (ret == AUTH_ACCEPT) {
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index f89e743..7dd2a5a 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -910,7 +910,7 @@ do_rpc:
                                 if (strcmp (xprt_path, auth_path) != 0) {
                                         continue;
                                 }
-                                ret = gf_authenticate (conf, xprt->clnt_options,
+                                ret = gf_authenticate (xprt->clnt_options,
                                                        options,
                                                        conf->auth_modules);
                                 if (ret == AUTH_ACCEPT) {
@@ -1104,7 +1104,7 @@ init (xlator_t *this)
         }
 
         ret = dict_get_str_boolean (this->options, "strict-auth-accept",
-                        _gf_false);
+                                    _gf_false);
         if (ret == -1)
                 conf->strict_auth_enabled = _gf_false;
         else
diff --git a/xlators/protocol/server/src/server.h b/xlators/protocol/server/src/server.h
index 4c35aaa..6af4657 100644
--- a/xlators/protocol/server/src/server.h
+++ b/xlators/protocol/server/src/server.h
@@ -245,8 +245,6 @@ serialize_rsp_dirent (gf_dirent_t *entries, gfs3_readdir_rsp *rsp);
 
 int
 serialize_rsp_direntp (gf_dirent_t *entries, gfs3_readdirp_rsp *rsp);
-auth_result_t gf_authenticate (server_conf_t *conf, dict_t *input_params,
-                               dict_t *config_params, dict_t *auth_modules);
 
 server_ctx_t*
 server_ctx_get (client_t *client, xlator_t *xlator);
-- 
1.8.3.1