commit 87c1cf0f20be20608d3becf854e9cf0910f4ad32
Author: 伊藤洋也 <hiroyan@gmail.com>
Date: Fri Dec 20 18:49:54 2013 +0000
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>
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");
}