transport: fix default behavior for SSL authorization

Previously, enabling SSL authentication/encryption but not authorization
required explicitly setting ssl-allow=*.  Now that same behavior is the
default (i.e. when ssl-allow is not set).

Also, there's no reason that a name used for *login* auth (typically a
UUID for internal purposes or a human name when using SSL) should
validate as an RFC-compliant host name or IP address.  Therefore the
validation only occurs when the auth type is "addr" (not "login" or
anything else).

Change-Id: I01485ff4f0ab37de4b182858235a5fb0cf4c3c7d
BUG: 1179208
Signed-off-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-on: http://review.gluster.org/9397
Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

4 files changed, 51 insertions, 2 deletions

diff --git a/tests/features/ssl-authz.t b/tests/features/ssl-authz.t
index efaa47c6d40..67d72e8f136 100755
--- a/tests/features/ssl-authz.t
+++ b/tests/features/ssl-authz.t
@@ -49,10 +49,31 @@ TEST $CLI volume create $V0 $H0:$B0/1
 TEST $CLI volume set $V0 server.ssl on
 TEST $CLI volume set $V0 client.ssl on
 #EST $CLI volume set $V0 ssl.cipher-list $(valid_ciphers)
+TEST $CLI volume start $V0
+
+# This mount should SUCCEED because ssl-allow=* by default.  This effectively
+# disables SSL authorization, though authentication and encryption might still
+# be enabled.
+TEST glusterfs --volfile-server=$H0 --volfile-id=$V0 $M0
+TEST ping_file $M0/before
+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
+
+# Set ssl-allow to a wildcard that includes our identity.
+TEST $CLI volume stop $V0
+TEST $CLI volume set $V0 auth.ssl-allow Any*
+TEST $CLI volume start $V0
+
+# This mount should SUCCEED because we match the wildcard.
+TEST glusterfs --volfile-server=$H0 --volfile-id=$V0 $M0
+TEST ping_file $M0/before
+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
+
+# Set ssl-allow to include the identity we've created.
+TEST $CLI volume stop $V0
 TEST $CLI volume set $V0 auth.ssl-allow Anyone
 TEST $CLI volume start $V0
 
-# This mount should WORK.
+# This mount should SUCCEED because this specific identity is allowed.
 TEST glusterfs --volfile-server=$H0 --volfile-id=$V0 $M0
 TEST ping_file $M0/before
 EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-set.c b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
index f4c6cff1220..a92bfffdb4f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-set.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
@@ -963,6 +963,7 @@ struct volopt_map_entry glusterd_volopt_map[] = {
         { .key         = "auth.ssl-allow",
           .voltype     = "protocol/server",
           .option      = "!ssl-allow",
+          .value       = "*",
           .type        = NO_DOC,
           .op_version  = GD_OP_VERSION_3_6_0,
         },
diff --git a/xlators/protocol/auth/login/src/login.c b/xlators/protocol/auth/login/src/login.c
index 56b93a9f9e9..b53c5ccba21 100644
--- a/xlators/protocol/auth/login/src/login.c
+++ b/xlators/protocol/auth/login/src/login.c
@@ -38,7 +38,6 @@ auth_result_t gf_auth (dict_t *input_params, dict_t *config_params)
                 gf_log ("auth/login", GF_LOG_INFO,
                         "connecting user name: %s", username_data->data);
                 using_ssl = _gf_true;
-                result = AUTH_REJECT;
         }
         else {
                 username_data = dict_get (input_params, "username");
@@ -80,6 +79,28 @@ auth_result_t gf_auth (dict_t *input_params, dict_t *config_params)
         if (allow_user) {
                 gf_log ("auth/login", GF_LOG_INFO,
                         "allowed user names: %s", allow_user->data);
+                /*
+                 * There's a subtle difference between SSL and non-SSL behavior
+                 * if we can't match anything in the "while" loop below.
+                 * Intuitively, we should AUTH_REJECT if there's no match.
+                 * However, existing code depends on allowing untrusted users
+                 * to connect with *no credentials at all* by falling through
+                 * the loop.  They're still distinguished from trusted users
+                 * 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 sane thing
+                 * only for SSL here.
+                 *
+                 * For SSL, if there's a list *you must be on it*.  Note that
+                 * if there's no list we don't care.  In that case (and the
+                 * ssl-allow=* case as well) authorization is effectively
+                 * disabled, though authentication and encryption are still
+                 * active.
+                 */
+                if (using_ssl) {
+                        result = AUTH_REJECT;
+                }
                 username_cpy = gf_strdup (allow_user->data);
                 if (!username_cpy)
                         goto out;
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index 6f6be52ab15..0dfe19a16b4 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -380,6 +380,12 @@ _check_for_auth_option (dict_t *d, char *k, data_t *v,
         if (!tail)
                 goto out;
 
+        if (strncmp(tail, "addr.", 5) != 0) {
+                gf_log (xl->name, GF_LOG_INFO,
+                        "skip format check for non-addr auth option %s", k);
+                goto out;
+        }
+
         /* fast fwd thru module type */
         tail = strchr (tail, '.');
         if (!tail)