diff --git a/proxy_network.c b/proxy_network.c index 2adc3380b1..e8fbcabfa6 100644 --- a/proxy_network.c +++ b/proxy_network.c @@ -1181,6 +1181,7 @@ static void proxy_beconn_tls_handler(const int fd, const short which, void *arg) if (_proxy_beconn_checkconnect(be) == -1) { return; } + // TODO: check return code. mcp_tls_connect(be); // fall through to handshake attempt. } @@ -1193,9 +1194,11 @@ static void proxy_beconn_tls_handler(const int fd, const short which, void *arg) _set_main_event(be, be->event_thread->base, EV_READ, &tmp_time, proxy_beconn_tls_handler); return; } else if (ret == 1) { - // complete. - // TODO: handle errors. - mcp_tls_send_validate(be); + // handshake complete. + if (mcp_tls_send_validate(be) != 1) { + _reset_bad_backend(be, P_BE_FAIL_BADVALIDATE); + return; + } // switch to another handler for the final stage. _set_main_event(be, be->event_thread->base, EV_READ, &tmp_time, proxy_bevalidate_tls_handler); diff --git a/proxy_tls.c b/proxy_tls.c index c575fb1864..fe545cd52d 100644 --- a/proxy_tls.c +++ b/proxy_tls.c @@ -3,6 +3,20 @@ #include "proxy.h" #include "proxy_tls.h" #include +#include + +/* Notes on ERR_clear_error() and friends: + * - Errors from SSL calls leave errors on a thread-local "error stack" + * - If an error is received from an SSL call, the stack needs to be inspected + * and cleared. + * - The error stack _must_ be clear before any SSL_get_error() calls, as it + * may return garbage. + * - ERR_clear_error() is not "free", so we would prefer to avoid calling it + * before hotpath calls. Thus, we should ensure it's called _after_ any + * hotpath call that receives any kind of error. + * - We should also call it _before_ any non-hotpath SSL calls (such as + * SSL_connect()) for defense against bugs in our code or OpenSSL. + */ int mcp_tls_init(proxy_ctx_t *ctx) { if (ctx->tls_ctx) { @@ -47,8 +61,23 @@ int mcp_tls_connect(struct mcp_backendconn_s *be) { // TODO: check return code. can fail if BIO fails to alloc. SSL_set_fd(be->ssl, mcmc_fd(be->client)); - // TODO: care about the error code. - int ret = SSL_connect(be->ssl); + ERR_clear_error(); + int n = SSL_connect(be->ssl); + int ret = 1; + // TODO: complete error handling. + if (n == 1) { + // Successfully established and handshake complete. + } else if (n == 0) { + // Not successsful, but shut down normally. + ERR_clear_error(); + ret = -1; + } else if (n < 0) { + // Not successful. Check for temporary error. + + // clear all errors in case of other junk. + ERR_clear_error(); + ret = -1; + } return ret; } @@ -58,6 +87,8 @@ int mcp_tls_handshake(struct mcp_backendconn_s *be) { return 1; } + // Non hot path, so clear errors before running. + ERR_clear_error(); int n = SSL_do_handshake(be->ssl); if (n == 1) { return 1; @@ -69,20 +100,32 @@ int mcp_tls_handshake(struct mcp_backendconn_s *be) { // leaving this note just in case. if (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE) { + // So far as I can tell there would be an error on the queue here. + ERR_clear_error(); return 0; } else { // TODO: can get the full error message and give to the caller to log // to proxyevents? + ERR_clear_error(); return -1; } } -// TODO: error processing. -void mcp_tls_send_validate(struct mcp_backendconn_s *be) { +int mcp_tls_send_validate(struct mcp_backendconn_s *be) { const char *str = "version\r\n"; const size_t len = strlen(str); - SSL_write(be->ssl, str, len); + // Non hot path, clear errors. + ERR_clear_error(); + int n = SSL_write(be->ssl, str, len); + + // TODO: more detailed error checking. + if (n < 0 || n != len) { + ERR_clear_error(); + return -1; + } + + return 1; } int mcp_tls_read(struct mcp_backendconn_s *be) { @@ -92,8 +135,11 @@ int mcp_tls_read(struct mcp_backendconn_s *be) { int err = SSL_get_error(be->ssl, n); if (err == SSL_ERROR_WANT_WRITE || err == SSL_ERROR_WANT_READ) { + ERR_clear_error(); return -1; } else { + // TODO: log detailed error. + ERR_clear_error(); return -2; } } else { @@ -142,8 +188,10 @@ int mcp_tls_writev(struct mcp_backendconn_s *be, int iovcnt) { int err = SSL_get_error(be->ssl, n); if (err == SSL_ERROR_WANT_WRITE || err == SSL_ERROR_WANT_READ) { + ERR_clear_error(); return -1; } + ERR_clear_error(); return -2; } diff --git a/proxy_tls.h b/proxy_tls.h index 1f9d3b6651..b1621338e5 100644 --- a/proxy_tls.h +++ b/proxy_tls.h @@ -5,7 +5,7 @@ int mcp_tls_init(proxy_ctx_t *ctx); int mcp_tls_backend_init(proxy_ctx_t *ctx, struct mcp_backendconn_s *be); int mcp_tls_connect(struct mcp_backendconn_s *be); int mcp_tls_handshake(struct mcp_backendconn_s *be); -void mcp_tls_send_validate(struct mcp_backendconn_s *be); +int mcp_tls_send_validate(struct mcp_backendconn_s *be); int mcp_tls_read(struct mcp_backendconn_s *be); int mcp_tls_writev(struct mcp_backendconn_s *be, int iovcnt);