Skip to content

Commit

Permalink
Some SSL related fixes
Browse files Browse the repository at this point in the history 
The SSL connections would sometimes just hang caused by
incorrect buffer refill logic

Change-Id: Ieb74bf6b8284cb4e83b56680415dfd0f7ed82d9d
Reviewed-on: http://review.couchbase.org/33883
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Tested-by: Trond Norbye <trond.norbye@gmail.com>
  • Loading branch information
@trondn
trondn committed Feb 25, 2014
1 parent 760394f commit 5e2eff6
Show file tree
Hide file tree
Showing 5 changed files with 308 additions and 36 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,6 @@
/mchello
/memcached_sizes
/memcached_testapp
/ssltest
/timedrun
/config.h
5 changes: 5 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ ADD_EXECUTABLE(memcached
daemon/stats.c
daemon/thread.c)

ADD_EXECUTABLE(ssltest programs/ssltest.c
programs/utilities.c
programs/utilities.h)

ADD_EXECUTABLE(memcached_testapp programs/testapp.c daemon/cache.c)
ADD_EXECUTABLE(gencode programs/gencode.cc)
SET_TARGET_PROPERTIES(gencode PROPERTIES COMPILE_FLAGS -I${CMAKE_CURRENT_SOURCE_DIR}/../libvbucket/include)
Expand All @@ -142,6 +146,7 @@ TARGET_LINK_LIBRARIES(bucket_engine_testapp mcd_util platform ${COUCHBASE_NETWOR
TARGET_LINK_LIBRARIES(cbsasladm platform ${OPENSSL_LIBRARIES} ${COUCHBASE_NETWORK_LIBS})
TARGET_LINK_LIBRARIES(mcstat platform ${OPENSSL_LIBRARIES} ${COUCHBASE_NETWORK_LIBS})
TARGET_LINK_LIBRARIES(mchello platform ${OPENSSL_LIBRARIES} ${COUCHBASE_NETWORK_LIBS})
TARGET_LINK_LIBRARIES(ssltest platform ${OPENSSL_LIBRARIES} ${COUCHBASE_NETWORK_LIBS})
TARGET_LINK_LIBRARIES(tap_mock_engine platform ${COUCHBASE_NETWORK_LIBS})

TARGET_LINK_LIBRARIES(mcd_util platform)
Expand Down
174 changes: 138 additions & 36 deletions daemon/memcached.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,7 @@ conn *conn_new(const SOCKET sfd, in_port_t parent_port,
}

c->ssl.enabled = true;
c->ssl.error = false;
c->ssl.client = NULL;

c->ssl.in.buffer = malloc(settings.bio_drain_buffer_sz);
Expand Down Expand Up @@ -998,6 +999,7 @@ static void conn_cleanup(conn *c) {
BIO_free_all(c->ssl.network);
SSL_free(c->ssl.client);
c->ssl.enabled = false;
c->ssl.error = false;
free(c->ssl.in.buffer);
free(c->ssl.out.buffer);
memset(&c->ssl, 0, sizeof(c->ssl));
Expand Down Expand Up @@ -5455,14 +5457,17 @@ static int try_read_command(conn *c) {
return 1;
}


static void drain_bio_send_pipe(conn *c) {
/* First check if we have data in our send bio buffer */
int n;
bool stop = false;

do {
if (c->ssl.out.current < c->ssl.out.total) {
#ifdef WIN32
DWORD error;
#else
int error;
#endif
n = send(c->sfd, c->ssl.out.buffer + c->ssl.out.current,
c->ssl.out.total - c->ssl.out.current, 0);
if (n > 0) {
Expand All @@ -5471,7 +5476,17 @@ static void drain_bio_send_pipe(conn *c) {
c->ssl.out.current = c->ssl.out.total = 0;
}
} else {
stop = true;
if (n == -1) {
#ifdef WIN32
error = WSAGetLastError();
#else
error = errno;
#endif
if (!is_blocking(error)) {
c->ssl.error = true;
}
}
return ;
}
}

Expand All @@ -5486,8 +5501,6 @@ static void drain_bio_send_pipe(conn *c) {
} while (!stop);
}



static void drain_bio_recv_pipe(conn *c) {
int n;
bool stop = false;
Expand All @@ -5503,16 +5516,36 @@ static void drain_bio_recv_pipe(conn *c) {
c->ssl.in.current = c->ssl.in.total = 0;
}
} else {
stop = true;
/* Our input BIO is full, no need to grab more data from
* the network at this time..
*/
return ;
}
}

if (c->ssl.in.total == 0) {
#ifdef WIN32
DWORD error;
#else
int error;
#endif
n = recv(c->sfd, c->ssl.in.buffer, c->ssl.in.buffsz, 0);
if (n > 0) {
c->ssl.in.total = n;
} else {
stop = true;
if (n == 0) {
c->ssl.error = true; /* read end shutdown */
} else {
#ifdef WIN32
error = WSAGetLastError();
#else
error = errno;
#endif
if (!is_blocking(error)) {
c->ssl.error = true;
}
}
}
}
} while (!stop);
Expand Down Expand Up @@ -5551,6 +5584,57 @@ static int do_ssl_pre_connection(conn *c) {
return 0;
}

static int do_ssl_read(conn *c, char *dest, size_t nbytes) {
int ret = 0;

while (ret < nbytes) {
int n;
drain_bio_recv_pipe(c);
if (c->ssl.error) {
set_econnreset();
return -1;
}
n = SSL_read(c->ssl.client, dest + ret, nbytes - ret);
if (n > 0) {
ret += n;
} else {
if (ret > 0) {
/* I've gotten some data, let the user have that */
return ret;
}

if (n < 0) {
int error = SSL_get_error(c->ssl.client, n);
switch (error) {
case SSL_ERROR_WANT_READ:
/*
* Drain the buffers and retry if we've got data in
* our input buffers
*/
if (c->ssl.in.current >= c->ssl.in.total) {
set_ewouldblock();
return -1;
}
break;

default:
/*
* @todo I don't know how to gracefully recover from this
* let's just shut down the connection
*/
settings.extensions.logger->log(EXTENSION_LOG_WARNING, c,
"%d: ERROR: SSL_read returned -1 with error %d",
c->sfd, error);
set_econnreset();
return -1;
}
}
}
}

return ret;
}

static int do_data_recv(conn *c, void *dest, size_t nbytes) {
int res;
if (c->ssl.enabled) {
Expand All @@ -5565,11 +5649,48 @@ static int do_data_recv(conn *c, void *dest, size_t nbytes) {

/* The SSL negotiation might be complete at this time */
if (c->ssl.connected) {
res = SSL_read(c->ssl.client, dest, nbytes);
if (res < 0) {
int error = SSL_get_error(c->ssl.client, res);
res = do_ssl_read(c, dest, nbytes);
}
} else {
res = recv(c->sfd, dest, nbytes, 0);
}

return res;
}

static int do_ssl_write(conn *c, char *dest, size_t nbytes) {
int ret = 0;

int chunksize = settings.bio_drain_buffer_sz;

while (ret < nbytes) {
int n;
int chunk;

drain_bio_send_pipe(c);
if (c->ssl.error) {
set_econnreset();
return -1;
}

chunk = nbytes - ret;
if (chunk > chunksize) {
chunk = chunksize;
}

n = SSL_write(c->ssl.client, dest + ret, chunk);
if (n > 0) {
ret += n;
} else {
if (ret > 0) {
/* We've sent some data.. let the caller have them */
return ret;
}

if (n < 0) {
int error = SSL_get_error(c->ssl.client, n);
switch (error) {
case SSL_ERROR_WANT_READ:
case SSL_ERROR_WANT_WRITE:
set_ewouldblock();
return -1;

Expand All @@ -5579,51 +5700,32 @@ static int do_data_recv(conn *c, void *dest, size_t nbytes) {
* let's just shut down the connection
*/
settings.extensions.logger->log(EXTENSION_LOG_WARNING, c,
"%d: ERROR: SSL_read returned -1 with error %d",
"%d: ERROR: SSL_write returned -1 with error %d",
c->sfd, error);
set_econnreset();
return -1;
}
}
}
} else {
res = recv(c->sfd, dest, nbytes, 0);
}

return res;
return ret;
}


static int do_data_sendmsg(conn *c, struct msghdr *m) {
int res;
if (c->ssl.enabled) {
int ii;
res = 0;
for (ii = 0; ii < m->msg_iovlen; ++ii) {
int n = SSL_write(c->ssl.client,
m->msg_iov[ii].iov_base,
m->msg_iov[ii].iov_len);
int n = do_ssl_write(c,
m->msg_iov[ii].iov_base,
m->msg_iov[ii].iov_len);
if (n > 0) {
res += n;
} else {
int error = SSL_get_error(c->ssl.client, res);
switch (error) {
case SSL_ERROR_WANT_WRITE:
drain_bio_send_pipe(c);
set_ewouldblock();
return res > 0 ? res : -1;

default:
/*
* @todo I don't know how to gracefully recover from this
* let's just shut down the connection
*/
settings.extensions.logger->log(EXTENSION_LOG_WARNING, c,
"%d: ERROR: SSL_write returned -1 with error %d",
c->sfd, error);
set_econnreset();
return -1;
}
break;
return res > 0 ? res : -1;
}
}

Expand Down
1 change: 1 addition & 0 deletions daemon/memcached.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ struct conn {
} in, out;

bool enabled;
bool error;
SSL_CTX *ctx;
SSL *client;

Expand Down
Loading

0 comments on commit 5e2eff6

Please sign in to comment.