Skip to content

Commit 7b538f4

Browse files
committed
tls: actually look at TLS errors
Adds TLS specific errors to `watch connevents` log stream. Ensures read/write retry errors have the correct errno set before returning to caller.
1 parent bbd3739 commit 7b538f4

File tree

4 files changed

+109
-30
lines changed

4 files changed

+109
-30
lines changed

logger.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,9 @@ static const entry_details default_entries[] = {
498498
},
499499
[LOGGER_CONNECTION_NEW] = {512, LOG_CONNEVENTS, _logger_log_conn_event, _logger_parse_cne, NULL},
500500
[LOGGER_CONNECTION_CLOSE] = {512, LOG_CONNEVENTS, _logger_log_conn_event, _logger_parse_cce, NULL},
501+
[LOGGER_CONNECTION_ERROR] = {512, LOG_CONNEVENTS, _logger_log_text, _logger_parse_text,
502+
"type=connerr fd=%d msg=%s"
503+
},
501504
[LOGGER_DELETIONS] = {512, LOG_DELETIONS, _logger_log_item_deleted, _logger_parse_ide, NULL},
502505
#ifdef EXTSTORE
503506
[LOGGER_EXTSTORE_WRITE] = {512, LOG_EVICTIONS, _logger_log_ext_write, _logger_parse_extw, NULL},

logger.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ enum log_entry_type {
2222
LOGGER_SLAB_MOVE,
2323
LOGGER_CONNECTION_NEW,
2424
LOGGER_CONNECTION_CLOSE,
25+
LOGGER_CONNECTION_ERROR,
2526
LOGGER_DELETIONS,
2627
#ifdef EXTSTORE
2728
LOGGER_EXTSTORE_WRITE,

tls.c

Lines changed: 105 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,26 +9,31 @@
99
#include <openssl/ssl.h>
1010
#include <openssl/err.h>
1111

12+
/* constant session ID context for application-level SSL session scoping.
13+
* used in server-side SSL session caching, when enabled. */
14+
#define SESSION_ID_CONTEXT "memcached"
15+
1216
#ifndef MAXPATHLEN
1317
#define MAXPATHLEN 4096
1418
#endif
1519

16-
ssize_t ssl_read(conn *c, void *buf, size_t count);
17-
ssize_t ssl_sendmsg(conn *c, struct msghdr *msg, int flags);
18-
ssize_t ssl_write(conn *c, void *buf, size_t count);
19-
void ssl_callback(const SSL *s, int where, int ret);
20-
int ssl_new_session_callback(SSL *s, SSL_SESSION *sess);
20+
static ssize_t ssl_read(conn *c, void *buf, size_t count);
21+
static ssize_t ssl_sendmsg(conn *c, struct msghdr *msg, int flags);
22+
static ssize_t ssl_write(conn *c, void *buf, size_t count);
23+
static void print_ssl_error(char *buff, size_t len);
24+
static void ssl_callback(const SSL *s, int where, int ret);
25+
static int ssl_new_session_callback(SSL *s, SSL_SESSION *sess);
2126

2227
static pthread_mutex_t ssl_ctx_lock = PTHREAD_MUTEX_INITIALIZER;
2328

2429
const unsigned ERROR_MSG_SIZE = 64;
2530
const size_t SSL_ERROR_MSG_SIZE = 256;
2631

27-
void SSL_LOCK(void) {
32+
static void SSL_LOCK(void) {
2833
pthread_mutex_lock(&(ssl_ctx_lock));
2934
}
3035

31-
void SSL_UNLOCK(void) {
36+
static void SSL_UNLOCK(void) {
3237
pthread_mutex_unlock(&(ssl_ctx_lock));
3338
}
3439

@@ -52,16 +57,33 @@ void *ssl_accept(conn *c, int sfd, bool *fail) {
5257
fprintf(stderr, "Failed to created the SSL object\n");
5358
}
5459
*fail = true;
60+
ERR_clear_error();
5561
return NULL;
5662
}
5763
SSL_set_fd(ssl, sfd);
64+
ERR_clear_error();
5865
int ret = SSL_accept(ssl);
5966
if (ret <= 0) {
6067
int err = SSL_get_error(ssl, ret);
61-
if (err == SSL_ERROR_SYSCALL || err == SSL_ERROR_SSL) {
62-
if (settings.verbose) {
63-
fprintf(stderr, "SSL connection failed with error code : %d : %s\n", err, strerror(errno));
68+
if (err == SSL_ERROR_WANT_WRITE || err == SSL_ERROR_WANT_READ) {
69+
// we're actually fine, let the worker thread continue.
70+
ERR_clear_error();
71+
} else {
72+
// TODO: ship full error to log stream? conn events?
73+
// SYSCALL specifically means we need to check errno/strerror.
74+
// Else we need to look at the main error stack.
75+
if (err == SSL_ERROR_SYSCALL) {
76+
LOGGER_LOG(c->thread->l, LOG_CONNEVENTS, LOGGER_CONNECTION_ERROR,
77+
NULL, c->sfd, strerror(errno));
78+
} else {
79+
char ssl_err[SSL_ERROR_MSG_SIZE];
80+
// OpenSSL internal error. One or more, but lets only care about
81+
// the top error for now.
82+
print_ssl_error(ssl_err, SSL_ERROR_MSG_SIZE);
83+
LOGGER_LOG(c->thread->l, LOG_CONNEVENTS, LOGGER_CONNECTION_ERROR,
84+
NULL, c->sfd, ssl_err);
6485
}
86+
ERR_clear_error();
6587
SSL_free(ssl);
6688
STATS_LOCK();
6789
stats.ssl_handshake_errors++;
@@ -75,22 +97,91 @@ void *ssl_accept(conn *c, int sfd, bool *fail) {
7597
return ssl;
7698
}
7799

100+
/*
101+
* Note on setting errno in the follow functions:
102+
* We either have to refactor callers of read/write/sendmsg to take an error
103+
* flag to find out if we're in an EAGAIN state, or we ensure the errno is set
104+
* properly before returning from our TLS call. We do this because it's
105+
* _possible_ for OpenSSL to do something weird and land with an errno that
106+
* doesn't match the WANT_READ|WRITE state.
107+
*
108+
* Also: we _might_ have to communicate from these calls if we need to wait on
109+
* reads or write. Since I haven't yet proved that's even possible I'll save
110+
* that for a future refactor.
111+
*/
112+
78113
/*
79114
* Reads decrypted data from the underlying BIO read buffers,
80115
* which reads from the socket.
81116
*/
82-
ssize_t ssl_read(conn *c, void *buf, size_t count) {
117+
static ssize_t ssl_read(conn *c, void *buf, size_t count) {
83118
assert (c != NULL);
84119
/* TODO : document the state machine interactions for SSL_read with
85120
non-blocking sockets/ SSL re-negotiations
86121
*/
87-
return SSL_read(c->ssl, buf, count);
122+
123+
ssize_t ret = SSL_read(c->ssl, buf, count);
124+
if (ret <= 0) {
125+
int err = SSL_get_error(c->ssl, ret);
126+
if (err == SSL_ERROR_WANT_WRITE || err == SSL_ERROR_WANT_READ) {
127+
errno = EAGAIN;
128+
} else if (err == SSL_ERROR_ZERO_RETURN) {
129+
// TLS session is closed... let the caller move this along.
130+
return 0;
131+
} else if (err == SSL_ERROR_SYSCALL) {
132+
// need to rely on errno to find out what happened
133+
LOGGER_LOG(c->thread->l, LOG_CONNEVENTS, LOGGER_CONNECTION_ERROR,
134+
NULL, c->sfd, strerror(errno));
135+
} else if (ret != 0) {
136+
char ssl_err[SSL_ERROR_MSG_SIZE];
137+
// OpenSSL internal error. One or more, but lets only care about
138+
// the top error for now.
139+
print_ssl_error(ssl_err, SSL_ERROR_MSG_SIZE);
140+
LOGGER_LOG(c->thread->l, LOG_CONNEVENTS, LOGGER_CONNECTION_ERROR,
141+
NULL, c->sfd, ssl_err);
142+
}
143+
ERR_clear_error();
144+
}
145+
146+
return ret;
147+
}
148+
149+
/*
150+
* Writes data to the underlying BIO write buffers,
151+
* which encrypt and write them to the socket.
152+
*/
153+
static ssize_t ssl_write(conn *c, void *buf, size_t count) {
154+
assert (c != NULL);
155+
156+
ssize_t ret = SSL_write(c->ssl, buf, count);
157+
if (ret <= 0) {
158+
int err = SSL_get_error(c->ssl, ret);
159+
if (err == SSL_ERROR_WANT_WRITE || err == SSL_ERROR_WANT_READ) {
160+
errno = EAGAIN;
161+
} else if (err == SSL_ERROR_ZERO_RETURN) {
162+
// TLS session is closed... let the caller move this along.
163+
return 0;
164+
} else if (err == SSL_ERROR_SYSCALL) {
165+
// need to rely on errno to find out what happened
166+
LOGGER_LOG(c->thread->l, LOG_CONNEVENTS, LOGGER_CONNECTION_ERROR,
167+
NULL, c->sfd, strerror(errno));
168+
} else if (ret != 0) {
169+
char ssl_err[SSL_ERROR_MSG_SIZE];
170+
// OpenSSL internal error. One or more, but lets only care about
171+
// the top error for now.
172+
print_ssl_error(ssl_err, SSL_ERROR_MSG_SIZE);
173+
LOGGER_LOG(c->thread->l, LOG_CONNEVENTS, LOGGER_CONNECTION_ERROR,
174+
NULL, c->sfd, ssl_err);
175+
}
176+
ERR_clear_error();
177+
}
178+
return ret;
88179
}
89180

90181
/*
91182
* SSL sendmsg implementation. Perform a SSL_write.
92183
*/
93-
ssize_t ssl_sendmsg(conn *c, struct msghdr *msg, int flags) {
184+
static ssize_t ssl_sendmsg(conn *c, struct msghdr *msg, int flags) {
94185
assert (c != NULL);
95186
size_t buf_remain = settings.ssl_wbuf_size;
96187
size_t bytes = 0;
@@ -121,16 +212,7 @@ ssize_t ssl_sendmsg(conn *c, struct msghdr *msg, int flags) {
121212
/* TODO : document the state machine interactions for SSL_write with
122213
non-blocking sockets/ SSL re-negotiations
123214
*/
124-
return SSL_write(c->ssl, c->ssl_wbuf, bytes);
125-
}
126-
127-
/*
128-
* Writes data to the underlying BIO write buffers,
129-
* which encrypt and write them to the socket.
130-
*/
131-
ssize_t ssl_write(conn *c, void *buf, size_t count) {
132-
assert (c != NULL);
133-
return SSL_write(c->ssl, buf, count);
215+
return ssl_write(c, c->ssl_wbuf, bytes);
134216
}
135217

136218
/*

tls.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,6 @@
22
#define TLS_H
33

44
#ifdef TLS
5-
/* constant session ID context for application-level SSL session scoping.
6-
* used in server-side SSL session caching, when enabled. */
7-
#define SESSION_ID_CONTEXT "memcached"
8-
9-
void SSL_LOCK(void);
10-
void SSL_UNLOCK(void);
11-
125
void *ssl_accept(conn *c, int sfd, bool *fail);
136
int ssl_init(void);
147
void ssl_init_settings(void);

0 commit comments

Comments
 (0)