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