explicitly record sasl auth states

It was previously possible to bypass authentication due to implicit
state management.  Now we explicitly consider ourselves
unauthenticated on any new connections and authentication attempts.

bug316

Signed-off-by: Dustin Sallings <dustin@spy.net>

3 files changed, 41 insertions, 8 deletions

diff --git a/memcached.c b/memcached.c
index f129865..3a79fba 100644
--- a/memcached.c
+++ b/memcached.c
@@ -457,6 +457,7 @@ conn *conn_new(const int sfd, enum conn_states init_state,
     c->iovused = 0;
     c->msgcurr = 0;
     c->msgused = 0;
+    c->authenticated = false;
 
     c->write_and_go = init_state;
     c->write_and_free = 0;
@@ -1637,6 +1638,8 @@ static void init_sasl_conn(conn *c) {
     if (!settings.sasl)
         return;
 
+    c->authenticated = false;
+
     if (!c->sasl_conn) {
         int result=sasl_server_new("memcached",
                                    NULL,
@@ -1771,6 +1774,7 @@ static void process_bin_complete_sasl_auth(conn *c) {
 
     switch(result) {
     case SASL_OK:
+        c->authenticated = true;
         write_bin_response(c, "Authenticated", 0, 0, strlen("Authenticated"));
         pthread_mutex_lock(&c->thread->stats.mutex);
         c->thread->stats.auth_cmds++;
@@ -1807,11 +1811,7 @@ static bool authenticated(conn *c) {
         rv = true;
         break;
     default:
-        if (c->sasl_conn) {
-            const void *uname = NULL;
-            sasl_getprop(c->sasl_conn, SASL_USERNAME, &uname);
-            rv = uname != NULL;
-        }
+        rv = c->authenticated;
     }
 
     if (settings.verbose > 1) {
diff --git a/memcached.h b/memcached.h
index 45b3213..7c212d5 100644
--- a/memcached.h
+++ b/memcached.h
@@ -376,6 +376,7 @@ typedef struct conn conn;
 struct conn {
     int    sfd;
     sasl_conn_t *sasl_conn;
+    bool authenticated;
     enum conn_states  state;
     enum bin_substates substate;
     struct event event;
diff --git a/t/binary-sasl.t b/t/binary-sasl.t
index 69a05c2..85ef069 100755
--- a/t/binary-sasl.t
+++ b/t/binary-sasl.t
@@ -13,7 +13,7 @@ use Test::More;
 
 if (supports_sasl()) {
     if ($ENV{'RUN_SASL_TESTS'}) {
-        plan tests => 25;
+        plan tests => 33;
     } else {
         plan skip_all => 'Skipping SASL tests';
         exit 0;
@@ -229,6 +229,38 @@ $check->('x','somevalue');
 }
 $empty->('x');
 
+{
+    my $mc = MC::Client->new;
+
+    # Attempt bad authentication.
+    is ($mc->authenticate('testuser', 'wrongpassword'), 0x20, "bad auth");
+
+    # This should fail because $mc is not authenticated
+    my ($status, $val)= $mc->set('x', "somevalue");
+    ok($status, "this fails to authenticate");
+    cmp_ok($status,'==',ERR_AUTH_ERROR, "error code matches");
+}
+$empty->('x', 'somevalue');
+
+{
+    my $mc = MC::Client->new;
+
+    # Attempt bad authentication.
+    is ($mc->authenticate('testuser', 'wrongpassword'), 0x20, "bad auth");
+
+    # Mix an authenticated connection and an unauthenticated connection to
+    # confirm c->authenticated is not shared among connections
+    my $mc2 = MC::Client->new;
+    is ($mc2->authenticate('testuser', 'testpass'), 0, "authenticated");
+    my ($status, $val)= $mc2->set('x', "somevalue");
+    ok(! $status);
+
+    # This should fail because $mc is not authenticated
+    ($status, $val)= $mc->set('x', "somevalue");
+    ok($status, "this fails to authenticate");
+    cmp_ok($status,'==',ERR_AUTH_ERROR, "error code matches");
+}
+
 # check the SASL stats, make sure they track things correctly
 # note: the enabled or not is presence checked in stats.t
 
@@ -241,8 +273,8 @@ $empty->('x');
 
 {
     my %stats = $mc->stats('');
-    is ($stats{'auth_cmds'}, 2, "auth commands counted");
-    is ($stats{'auth_errors'}, 1, "auth errors correct");
+    is ($stats{'auth_cmds'}, 5, "auth commands counted");
+    is ($stats{'auth_errors'}, 3, "auth errors correct");
 }