From 4c9ab80f6b33add2db69b3a563bba4fd2dbe5ee3 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Tue, 30 Aug 2016 22:57:25 +0000 Subject: [PATCH 01/17] MbedTLS: Major restructuring I have removed all non-MbedTLS-specific code from this backend and copied the non-OpenSSL-specific portions of the OpenSSL backend code over; as it seems to be more reliable. --- libratbox/src/mbedtls.c | 708 +++++++++++++++++++++------------------- 1 file changed, 380 insertions(+), 328 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index a7a71cdb..097b31ba 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -70,6 +70,17 @@ static mbedtls_entropy_context entropy_ctx; static mbedtls_x509_crt dummy_ca_ctx; static rb_mbedtls_cfg_context *rb_mbedtls_cfg = NULL; + + +static int rb_sock_net_recv(void *, unsigned char *, size_t); +static int rb_sock_net_xmit(void *, const unsigned char *, size_t); + + + +/* + * Internal MbedTLS-specific code + */ + static const char * rb_get_ssl_strerror_internal(int err) { @@ -86,20 +97,16 @@ rb_get_ssl_strerror_internal(int err) return errbuf; } -const char * -rb_get_ssl_strerror(rb_fde_t *F) -{ - return rb_get_ssl_strerror_internal(F->ssl_errno); -} - -static void rb_mbedtls_cfg_incref(rb_mbedtls_cfg_context *cfg) +static void +rb_mbedtls_cfg_incref(rb_mbedtls_cfg_context *cfg) { lrb_assert(cfg->refcount > 0); cfg->refcount++; } -static void rb_mbedtls_cfg_decref(rb_mbedtls_cfg_context *cfg) +static void +rb_mbedtls_cfg_decref(rb_mbedtls_cfg_context *cfg) { if(cfg == NULL) return; @@ -118,7 +125,8 @@ static void rb_mbedtls_cfg_decref(rb_mbedtls_cfg_context *cfg) rb_free(cfg); } -static rb_mbedtls_cfg_context *rb_mbedtls_cfg_new(void) +static rb_mbedtls_cfg_context * +rb_mbedtls_cfg_new(void) { rb_mbedtls_cfg_context *cfg; int ret; @@ -166,146 +174,21 @@ static rb_mbedtls_cfg_context *rb_mbedtls_cfg_new(void) return cfg; } -void -rb_ssl_shutdown(rb_fde_t *F) -{ - if(F == NULL || F->ssl == NULL) - return; - - if(SSL_P(F) != NULL) - { - for(int i = 0; i < 4; i++) - { - int r = mbedtls_ssl_close_notify(SSL_P(F)); - if(r != MBEDTLS_ERR_SSL_WANT_READ && r != MBEDTLS_ERR_SSL_WANT_WRITE) - break; - } - mbedtls_ssl_free(SSL_P(F)); - } - - if(SSL_C(F) != NULL) - rb_mbedtls_cfg_decref(SSL_C(F)); - - rb_free(F->ssl); -} - -unsigned int -rb_ssl_handshake_count(rb_fde_t *F) -{ - return F->handshake_count; -} - -void -rb_ssl_clear_handshake_count(rb_fde_t *F) -{ - F->handshake_count = 0; -} - static void -rb_ssl_timeout(rb_fde_t *F, void *notused) +rb_ssl_setup_mbed_context(rb_fde_t *F, mbedtls_ssl_config *const mbed_config) { - lrb_assert(F->accept != NULL); - F->accept->callback(F, RB_ERR_TIMEOUT, NULL, 0, F->accept->data); -} - -static int -do_ssl_handshake(rb_fde_t *F, PF * callback, void *data) -{ - int ret = mbedtls_ssl_handshake(SSL_P(F)); - - if(ret == 0) - { - F->handshake_count++; - return 1; - } - - if(ret == -1 && rb_ignore_errno(errno)) - ret = MBEDTLS_ERR_SSL_WANT_READ; - - switch(ret) - { - case MBEDTLS_ERR_SSL_WANT_READ: - rb_setselect(F, RB_SELECT_READ, callback, data); - return 0; - case MBEDTLS_ERR_SSL_WANT_WRITE: - rb_setselect(F, RB_SELECT_WRITE, callback, data); - return 0; - default: - F->ssl_errno = ret; - return -1; - } -} - -static void -rb_ssl_tryaccept(rb_fde_t *F, void *data) -{ - lrb_assert(F->accept != NULL); - - int ret = do_ssl_handshake(F, rb_ssl_tryaccept, NULL); - - /* do_ssl_handshake does the rb_setselect */ - if(ret == 0) - return; - - struct acceptdata *ad = F->accept; - F->accept = NULL; - rb_settimeout(F, 0, NULL, NULL); - rb_setselect(F, RB_SELECT_READ | RB_SELECT_WRITE, NULL, NULL); - - if(ret > 0) - ad->callback(F, RB_OK, (struct sockaddr *)&ad->S, ad->addrlen, ad->data); - else - ad->callback(F, RB_ERROR_SSL, NULL, 0, ad->data); - - rb_free(ad); -} - -static int -rb_ssl_read_cb(void *opaque, unsigned char *buf, size_t size) -{ - rb_fde_t *F = opaque; - - int ret = (int) read(F->fd, buf, size); - if(ret < 0 && rb_ignore_errno(errno)) - return MBEDTLS_ERR_SSL_WANT_READ; - - return ret; -} - -static int -rb_ssl_write_cb(void *opaque, const unsigned char *buf, size_t size) -{ - rb_fde_t *F = opaque; - - int ret = (int) write(F->fd, buf, size); - if(ret < 0 && rb_ignore_errno(errno)) - return MBEDTLS_ERR_SSL_WANT_WRITE; - - return ret; -} - -static void -rb_ssl_setup_mbed_context(rb_fde_t *F, int is_server) -{ - rb_mbedtls_ssl_context *mbed_ssl_ctx; - mbedtls_ssl_config *mbed_config; - int ret; - - if((mbed_ssl_ctx = rb_malloc(sizeof(rb_mbedtls_ssl_context))) == NULL) + rb_mbedtls_ssl_context *const mbed_ssl_ctx = rb_malloc(sizeof *mbed_ssl_ctx); + if(mbed_ssl_ctx == NULL) { rb_lib_log("rb_ssl_setup_mbed_context: rb_malloc: allocation failure"); rb_close(F); return; } - if(is_server) - mbed_config = &rb_mbedtls_cfg->server_cfg; - else - mbed_config = &rb_mbedtls_cfg->client_cfg; - mbedtls_ssl_init(&mbed_ssl_ctx->ssl); - mbedtls_ssl_set_bio(&mbed_ssl_ctx->ssl, F, rb_ssl_write_cb, rb_ssl_read_cb, NULL); + mbedtls_ssl_set_bio(&mbed_ssl_ctx->ssl, F, rb_sock_net_xmit, rb_sock_net_recv, NULL); + int ret; if((ret = mbedtls_ssl_setup(&mbed_ssl_ctx->ssl, mbed_config)) != 0) { rb_lib_log("rb_ssl_setup_mbed_context: ssl_setup: %s", @@ -316,94 +199,38 @@ rb_ssl_setup_mbed_context(rb_fde_t *F, int is_server) return; } + rb_mbedtls_cfg_incref(rb_mbedtls_cfg); mbed_ssl_ctx->cfg = rb_mbedtls_cfg; - rb_mbedtls_cfg_incref(mbed_ssl_ctx->cfg); F->ssl = mbed_ssl_ctx; } -void -rb_ssl_start_accepted(rb_fde_t *F, ACCB * cb, void *data, int timeout) -{ - F->type |= RB_FD_SSL; - F->accept = rb_malloc(sizeof(struct acceptdata)); - F->accept->callback = cb; - F->accept->data = data; - rb_settimeout(F, timeout, rb_ssl_timeout, NULL); - F->accept->addrlen = 0; - - rb_ssl_setup_mbed_context(F, 1); - if(do_ssl_handshake(F, rb_ssl_tryaccept, NULL)) - { - struct acceptdata *ad = F->accept; - F->accept = NULL; - ad->callback(F, RB_OK, (struct sockaddr *)&ad->S, ad->addrlen, ad->data); - rb_free(ad); - } -} +/* + * External MbedTLS-specific code + */ void -rb_ssl_accept_setup(rb_fde_t *F, rb_fde_t *new_F, struct sockaddr *st, int addrlen) +rb_ssl_shutdown(rb_fde_t *const F) { - new_F->type |= RB_FD_SSL; - new_F->accept = rb_malloc(sizeof(struct acceptdata)); + if(F == NULL || F->ssl == NULL) + return; - new_F->accept->callback = F->accept->callback; - new_F->accept->data = F->accept->data; - rb_settimeout(new_F, 10, rb_ssl_timeout, NULL); - - memcpy(&new_F->accept->S, st, addrlen); - new_F->accept->addrlen = addrlen; - - rb_ssl_setup_mbed_context(new_F, 1); - if(do_ssl_handshake(F, rb_ssl_tryaccept, NULL)) + if(SSL_P(F) != NULL) { - struct acceptdata *ad = F->accept; - F->accept = NULL; - ad->callback(F, RB_OK, (struct sockaddr *)&ad->S, ad->addrlen, ad->data); - rb_free(ad); - } -} - -static ssize_t -rb_ssl_read_or_write(int r_or_w, rb_fde_t *F, void *rbuf, const void *wbuf, size_t count) -{ - ssize_t ret; - - if(r_or_w == 0) - ret = mbedtls_ssl_read(SSL_P(F), rbuf, count); - else - ret = mbedtls_ssl_write(SSL_P(F), wbuf, count); - - if(ret < 0) - { - switch(ret) + for(int i = 0; i < 4; i++) { - case MBEDTLS_ERR_SSL_WANT_READ: - return RB_RW_SSL_NEED_READ; - case MBEDTLS_ERR_SSL_WANT_WRITE: - return RB_RW_SSL_NEED_WRITE; - default: - F->ssl_errno = ret; - errno = EIO; - return RB_RW_SSL_ERROR; + int ret = mbedtls_ssl_close_notify(SSL_P(F)); + if(ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE) + break; } + mbedtls_ssl_free(SSL_P(F)); } - return ret; -} + if(SSL_C(F) != NULL) + rb_mbedtls_cfg_decref(SSL_C(F)); -ssize_t -rb_ssl_read(rb_fde_t *F, void *buf, size_t count) -{ - return rb_ssl_read_or_write(0, F, buf, NULL, count); -} - -ssize_t -rb_ssl_write(rb_fde_t *F, const void *buf, size_t count) -{ - return rb_ssl_read_or_write(1, F, NULL, buf, count); + rb_free(F->ssl); } int @@ -430,11 +257,13 @@ rb_init_ssl(void) return 0; } + rb_lib_log("rb_init_ssl: MbedTLS backend initialised"); return 1; } int -rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfile, const char *cipher_list) +rb_setup_ssl_server(const char *const certfile, const char *keyfile, + const char *const dhfile, const char *const cipherlist) { rb_mbedtls_cfg_context *newcfg; int ret; @@ -510,128 +339,41 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil rb_mbedtls_cfg_decref(rb_mbedtls_cfg); rb_mbedtls_cfg = newcfg; + rb_lib_log("rb_setup_ssl_server: TLS configuration successful"); return 1; } int -rb_ssl_listen(rb_fde_t *F, int backlog, int defer_accept) -{ - int result = rb_listen(F, backlog, defer_accept); - F->type = RB_FD_SOCKET | RB_FD_LISTEN | RB_FD_SSL; - - return result; -} - -struct ssl_connect -{ - CNCB *callback; - void *data; - int timeout; -}; - -static void -rb_ssl_connect_realcb(rb_fde_t *F, int status, struct ssl_connect *sconn) -{ - F->connect->callback = sconn->callback; - F->connect->data = sconn->data; - rb_free(sconn); - rb_connect_callback(F, status); -} - -static void -rb_ssl_tryconn_timeout_cb(rb_fde_t *F, void *data) -{ - rb_ssl_connect_realcb(F, RB_ERR_TIMEOUT, data); -} - -static void -rb_ssl_tryconn_cb(rb_fde_t *F, void *data) -{ - int ret = do_ssl_handshake(F, rb_ssl_tryconn_cb, data); - - switch(ret) - { - case -1: - rb_ssl_connect_realcb(F, RB_ERROR_SSL, data); - break; - case 0: - /* do_ssl_handshake does the rb_setselect stuff */ - return; - default: - break; - } - rb_ssl_connect_realcb(F, RB_OK, data); -} - -static void -rb_ssl_tryconn(rb_fde_t *F, int status, void *data) -{ - if(status != RB_OK) - { - rb_ssl_connect_realcb(F, status, data); - return; - } - - F->type |= RB_FD_SSL; - - rb_ssl_setup_mbed_context(F, 0); - rb_settimeout(F, ((struct ssl_connect *)data)->timeout, rb_ssl_tryconn_timeout_cb, data); - do_ssl_handshake(F, rb_ssl_tryconn_cb, data); -} - -void -rb_connect_tcp_ssl(rb_fde_t *F, struct sockaddr *dest, - struct sockaddr *clocal, int socklen, CNCB * callback, void *data, int timeout) -{ - if(F == NULL) - return; - - struct ssl_connect *sconn = rb_malloc(sizeof(struct ssl_connect)); - sconn->data = data; - sconn->callback = callback; - sconn->timeout = timeout; - - rb_connect_tcp(F, dest, clocal, socklen, rb_ssl_tryconn, sconn, timeout); -} - -void -rb_ssl_start_connected(rb_fde_t *F, CNCB * callback, void *data, int timeout) -{ - if(F == NULL) - return; - - struct ssl_connect *sconn = rb_malloc(sizeof(struct ssl_connect)); - sconn->data = data; - sconn->callback = callback; - sconn->timeout = timeout; - - F->connect = rb_malloc(sizeof(struct conndata)); - F->connect->callback = callback; - F->connect->data = data; - F->type |= RB_FD_SSL; - - rb_ssl_setup_mbed_context(F, 0); - rb_settimeout(F, sconn->timeout, rb_ssl_tryconn_timeout_cb, sconn); - do_ssl_handshake(F, rb_ssl_tryconn_cb, sconn); -} - -int -rb_init_prng(const char *path, prng_seed_t seed_type) +rb_init_prng(const char *const path, prng_seed_t seed_type) { + rb_lib_log("rb_init_prng: Skipping PRNG initialisation; not required by MbedTLS backend"); return 1; } int -rb_get_random(void *buf, size_t length) +rb_get_random(void *const buf, size_t length) { - if(mbedtls_ctr_drbg_random(&ctr_drbg_ctx, buf, length)) + int ret; + + if((ret = mbedtls_ctr_drbg_random(&ctr_drbg_ctx, buf, length)) != 0) + { + rb_lib_log("rb_get_random: ctr_drbg_random: %s", + rb_get_ssl_strerror_internal(ret)); + return 0; + } return 1; } +const char * +rb_get_ssl_strerror(rb_fde_t *const F) +{ + return rb_get_ssl_strerror_internal(F->ssl_errno); +} + int -rb_get_ssl_certfp(rb_fde_t *F, uint8_t certfp[RB_SSL_CERTFP_LEN], int method) +rb_get_ssl_certfp(rb_fde_t *const F, uint8_t certfp[RB_SSL_CERTFP_LEN], int method) { const mbedtls_x509_crt *peer_cert; const mbedtls_md_info_t *md_info; @@ -673,14 +415,8 @@ rb_get_ssl_certfp(rb_fde_t *F, uint8_t certfp[RB_SSL_CERTFP_LEN], int method) return hashlen; } -int -rb_supports_ssl(void) -{ - return 1; -} - void -rb_get_ssl_info(char *buf, size_t len) +rb_get_ssl_info(char *const buf, size_t len) { char version_str[512]; @@ -691,19 +427,335 @@ rb_get_ssl_info(char *buf, size_t len) } const char * -rb_ssl_get_cipher(rb_fde_t *F) +rb_ssl_get_cipher(rb_fde_t *const F) { if(F == NULL || F->ssl == NULL || SSL_P(F) == NULL) return NULL; static char buf[512]; - const char *version = mbedtls_ssl_get_version(SSL_P(F)); - const char *cipher = mbedtls_ssl_get_ciphersuite(SSL_P(F)); + const char *const version = mbedtls_ssl_get_version(SSL_P(F)); + const char *const cipher = mbedtls_ssl_get_ciphersuite(SSL_P(F)); (void) rb_snprintf(buf, sizeof buf, "%s, %s", version, cipher); return buf; } +ssize_t +rb_ssl_read(rb_fde_t *const F, void *const buf, size_t count) +{ + lrb_assert(F != NULL); + lrb_assert(F->ssl != NULL); + + ssize_t ret = (ssize_t) mbedtls_ssl_read(SSL_P(F), buf, count); + + if(ret > 0) + return ret; + + switch(ret) + { + case MBEDTLS_ERR_SSL_WANT_READ: + errno = EAGAIN; + return RB_RW_SSL_NEED_READ; + case MBEDTLS_ERR_SSL_WANT_WRITE: + errno = EAGAIN; + return RB_RW_SSL_NEED_WRITE; + default: + errno = EIO; + F->ssl_errno = ret; + return RB_RW_SSL_ERROR; + } +} + +ssize_t +rb_ssl_write(rb_fde_t *const F, const void *const buf, size_t count) +{ + lrb_assert(F != NULL); + lrb_assert(F->ssl != NULL); + + ssize_t ret = (ssize_t) mbedtls_ssl_write(SSL_P(F), buf, count); + + if(ret > 0) + return ret; + + switch(ret) + { + case MBEDTLS_ERR_SSL_WANT_READ: + errno = EAGAIN; + return RB_RW_SSL_NEED_READ; + case MBEDTLS_ERR_SSL_WANT_WRITE: + errno = EAGAIN; + return RB_RW_SSL_NEED_WRITE; + default: + errno = EIO; + F->ssl_errno = ret; + return RB_RW_SSL_ERROR; + } +} + + + +/* + * Internal library-agnostic code + * Mostly copied from the OpenSSL backend, with some optimisations and complete const-correctness + */ + +struct ssl_connect +{ + CNCB *callback; + void *data; + int timeout; +}; + +static void +rb_ssl_connect_realcb(rb_fde_t *const F, int status, struct ssl_connect *const sconn) +{ + lrb_assert(F->connect != NULL); + + F->connect->callback = sconn->callback; + F->connect->data = sconn->data; + rb_connect_callback(F, status); + rb_free(sconn); +} + +static void +rb_ssl_timeout_cb(rb_fde_t *const F, void *const data) +{ + lrb_assert(F->accept != NULL); + lrb_assert(F->accept->callback != NULL); + + F->accept->callback(F, RB_ERR_TIMEOUT, NULL, 0, F->accept->data); +} + +static void +rb_ssl_accept_common(rb_fde_t *const F, void *const data) +{ + lrb_assert(F != NULL); + lrb_assert(F->accept != NULL); + lrb_assert(F->accept->callback != NULL); + lrb_assert(F->ssl != NULL); + + mbedtls_ssl_context *const ssl_ctx = (mbedtls_ssl_context *) SSL_P(F); + + if(ssl_ctx->state != MBEDTLS_SSL_HANDSHAKE_OVER) + { + int ret = mbedtls_ssl_handshake(ssl_ctx); + + switch(ret) + { + case 0: + F->handshake_count++; + break; + case MBEDTLS_ERR_SSL_WANT_READ: + rb_setselect(F, RB_SELECT_READ, rb_ssl_accept_common, NULL); + return; + case MBEDTLS_ERR_SSL_WANT_WRITE: + rb_setselect(F, RB_SELECT_WRITE, rb_ssl_accept_common, NULL); + return; + default: + F->ssl_errno = ret; + F->accept->callback(F, RB_ERROR_SSL, NULL, 0, F->accept->data); + return; + } + } + + rb_settimeout(F, 0, NULL, NULL); + rb_setselect(F, RB_SELECT_READ | RB_SELECT_WRITE, NULL, NULL); + + struct acceptdata *const ad = F->accept; + F->accept = NULL; + ad->callback(F, RB_OK, (struct sockaddr *)&ad->S, ad->addrlen, ad->data); + rb_free(ad); +} + +static void +rb_ssl_tryconn_cb(rb_fde_t *const F, void *const data) +{ + lrb_assert(F != NULL); + lrb_assert(F->ssl != NULL); + + mbedtls_ssl_context *const ssl_ctx = SSL_P(F); + + if(ssl_ctx->state != MBEDTLS_SSL_HANDSHAKE_OVER) + { + int ret = mbedtls_ssl_handshake(ssl_ctx); + + switch(ret) + { + case 0: + F->handshake_count++; + break; + case MBEDTLS_ERR_SSL_WANT_READ: + rb_setselect(F, RB_SELECT_READ, rb_ssl_tryconn_cb, data); + return; + case MBEDTLS_ERR_SSL_WANT_WRITE: + rb_setselect(F, RB_SELECT_WRITE, rb_ssl_tryconn_cb, data); + return; + default: + errno = EIO; + F->ssl_errno = ret; + rb_ssl_connect_realcb(F, RB_ERROR_SSL, data); + return; + } + } + + rb_ssl_connect_realcb(F, RB_OK, data); +} + +static void +rb_ssl_tryconn_timeout_cb(rb_fde_t *const F, void *const data) +{ + rb_ssl_connect_realcb(F, RB_ERR_TIMEOUT, data); +} + +static void +rb_ssl_tryconn(rb_fde_t *const F, int status, void *const data) +{ + lrb_assert(F != NULL); + + if(status != RB_OK) + { + rb_ssl_connect_realcb(F, status, data); + return; + } + + F->type |= RB_FD_SSL; + + struct ssl_connect *const sconn = data; + rb_settimeout(F, sconn->timeout, rb_ssl_tryconn_timeout_cb, sconn); + + rb_ssl_setup_mbed_context(F, &rb_mbedtls_cfg->client_cfg); + rb_ssl_tryconn_cb(F, sconn); +} + +static int +rb_sock_net_recv(void *const context_ptr, unsigned char *const buf, size_t count) +{ + rb_fde_t *const F = (rb_fde_t *)context_ptr; + + int ret = (int) read(F->fd, buf, count); + if(ret < 0 && rb_ignore_errno(errno)) + return MBEDTLS_ERR_SSL_WANT_READ; + + return ret; +} + +static int +rb_sock_net_xmit(void *const context_ptr, const unsigned char *const buf, size_t count) +{ + rb_fde_t *const F = (rb_fde_t *)context_ptr; + + int ret = (int) write(F->fd, buf, count); + if(ret < 0 && rb_ignore_errno(errno)) + return MBEDTLS_ERR_SSL_WANT_READ; + + return ret; +} + + + +/* + * External library-agnostic code + * Mostly copied from the OpenSSL backend, with some optimisations and const-correctness + */ + +int +rb_supports_ssl(void) +{ + return 1; +} + +unsigned int +rb_ssl_handshake_count(rb_fde_t *const F) +{ + return F->handshake_count; +} + +void +rb_ssl_clear_handshake_count(rb_fde_t *const F) +{ + F->handshake_count = 0; +} + +void +rb_ssl_start_accepted(rb_fde_t *const F, ACCB *const cb, void *const data, int timeout) +{ + F->type |= RB_FD_SSL; + F->accept = rb_malloc(sizeof(struct acceptdata)); + + F->accept->callback = cb; + F->accept->data = data; + rb_settimeout(F, timeout, rb_ssl_timeout_cb, NULL); + + F->accept->addrlen = 0; + (void) memset(&F->accept->S, 0x00, sizeof F->accept->S); + + rb_ssl_setup_mbed_context(F, &rb_mbedtls_cfg->server_cfg); + rb_ssl_accept_common(F, NULL); +} + +void +rb_ssl_accept_setup(rb_fde_t *const srv_F, rb_fde_t *const cli_F, struct sockaddr *const st, int addrlen) +{ + cli_F->type |= RB_FD_SSL; + cli_F->accept = rb_malloc(sizeof(struct acceptdata)); + + cli_F->accept->callback = srv_F->accept->callback; + cli_F->accept->data = srv_F->accept->data; + rb_settimeout(cli_F, 10, rb_ssl_timeout_cb, NULL); + + cli_F->accept->addrlen = addrlen; + (void) memset(&cli_F->accept->S, 0x00, sizeof cli_F->accept->S); + (void) memcpy(&cli_F->accept->S, st, addrlen); + + rb_ssl_setup_mbed_context(cli_F, &rb_mbedtls_cfg->server_cfg); + rb_ssl_accept_common(cli_F, NULL); +} + +int +rb_ssl_listen(rb_fde_t *const F, int backlog, int defer_accept) +{ + int result = rb_listen(F, backlog, defer_accept); + F->type = RB_FD_SOCKET | RB_FD_LISTEN | RB_FD_SSL; + + return result; +} + +void +rb_connect_tcp_ssl(rb_fde_t *const F, struct sockaddr *const dest, struct sockaddr *const clocal, int socklen, + CNCB *const callback, void *const data, int timeout) +{ + if(F == NULL) + return; + + struct ssl_connect *const sconn = rb_malloc(sizeof(struct ssl_connect)); + sconn->data = data; + sconn->callback = callback; + sconn->timeout = timeout; + rb_connect_tcp(F, dest, clocal, socklen, rb_ssl_tryconn, sconn, timeout); +} + +void +rb_ssl_start_connected(rb_fde_t *const F, CNCB *const callback, void *const data, int timeout) +{ + if(F == NULL) + return; + + F->connect = rb_malloc(sizeof(struct conndata)); + F->connect->callback = callback; + F->connect->data = data; + F->type |= RB_FD_SSL; + + struct ssl_connect *const sconn = rb_malloc(sizeof(struct ssl_connect)); + sconn->data = data; + sconn->callback = callback; + sconn->timeout = timeout; + + rb_settimeout(F, sconn->timeout, rb_ssl_tryconn_timeout_cb, sconn); + + rb_ssl_setup_mbed_context(F, &rb_mbedtls_cfg->client_cfg); + rb_ssl_tryconn_cb(F, sconn); +} + #endif /* HAVE_MBEDTLS */ From 3ba0923c0ee4110ad71a991c9e41a9777a1a1bb7 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Tue, 30 Aug 2016 23:13:53 +0000 Subject: [PATCH 02/17] MbedTLS: Move some MbedTLS-specific code to the appropriate section --- libratbox/src/mbedtls.c | 152 ++++++++++++++++++++-------------------- 1 file changed, 77 insertions(+), 75 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 097b31ba..382c9e2d 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -72,6 +72,8 @@ static rb_mbedtls_cfg_context *rb_mbedtls_cfg = NULL; +static void rb_ssl_connect_realcb(rb_fde_t *, int, struct ssl_connect *); + static int rb_sock_net_recv(void *, unsigned char *, size_t); static int rb_sock_net_xmit(void *, const unsigned char *, size_t); @@ -204,6 +206,81 @@ rb_ssl_setup_mbed_context(rb_fde_t *F, mbedtls_ssl_config *const mbed_config) F->ssl = mbed_ssl_ctx; } +static void +rb_ssl_accept_common(rb_fde_t *const F, void *const data) +{ + lrb_assert(F != NULL); + lrb_assert(F->accept != NULL); + lrb_assert(F->accept->callback != NULL); + lrb_assert(F->ssl != NULL); + + mbedtls_ssl_context *const ssl_ctx = (mbedtls_ssl_context *) SSL_P(F); + + if(ssl_ctx->state != MBEDTLS_SSL_HANDSHAKE_OVER) + { + int ret = mbedtls_ssl_handshake(ssl_ctx); + + switch(ret) + { + case 0: + F->handshake_count++; + break; + case MBEDTLS_ERR_SSL_WANT_READ: + rb_setselect(F, RB_SELECT_READ, rb_ssl_accept_common, NULL); + return; + case MBEDTLS_ERR_SSL_WANT_WRITE: + rb_setselect(F, RB_SELECT_WRITE, rb_ssl_accept_common, NULL); + return; + default: + F->ssl_errno = ret; + F->accept->callback(F, RB_ERROR_SSL, NULL, 0, F->accept->data); + return; + } + } + + rb_settimeout(F, 0, NULL, NULL); + rb_setselect(F, RB_SELECT_READ | RB_SELECT_WRITE, NULL, NULL); + + struct acceptdata *const ad = F->accept; + F->accept = NULL; + ad->callback(F, RB_OK, (struct sockaddr *)&ad->S, ad->addrlen, ad->data); + rb_free(ad); +} + +static void +rb_ssl_tryconn_cb(rb_fde_t *const F, void *const data) +{ + lrb_assert(F != NULL); + lrb_assert(F->ssl != NULL); + + mbedtls_ssl_context *const ssl_ctx = SSL_P(F); + + if(ssl_ctx->state != MBEDTLS_SSL_HANDSHAKE_OVER) + { + int ret = mbedtls_ssl_handshake(ssl_ctx); + + switch(ret) + { + case 0: + F->handshake_count++; + break; + case MBEDTLS_ERR_SSL_WANT_READ: + rb_setselect(F, RB_SELECT_READ, rb_ssl_tryconn_cb, data); + return; + case MBEDTLS_ERR_SSL_WANT_WRITE: + rb_setselect(F, RB_SELECT_WRITE, rb_ssl_tryconn_cb, data); + return; + default: + errno = EIO; + F->ssl_errno = ret; + rb_ssl_connect_realcb(F, RB_ERROR_SSL, data); + return; + } + } + + rb_ssl_connect_realcb(F, RB_OK, data); +} + /* @@ -528,81 +605,6 @@ rb_ssl_timeout_cb(rb_fde_t *const F, void *const data) F->accept->callback(F, RB_ERR_TIMEOUT, NULL, 0, F->accept->data); } -static void -rb_ssl_accept_common(rb_fde_t *const F, void *const data) -{ - lrb_assert(F != NULL); - lrb_assert(F->accept != NULL); - lrb_assert(F->accept->callback != NULL); - lrb_assert(F->ssl != NULL); - - mbedtls_ssl_context *const ssl_ctx = (mbedtls_ssl_context *) SSL_P(F); - - if(ssl_ctx->state != MBEDTLS_SSL_HANDSHAKE_OVER) - { - int ret = mbedtls_ssl_handshake(ssl_ctx); - - switch(ret) - { - case 0: - F->handshake_count++; - break; - case MBEDTLS_ERR_SSL_WANT_READ: - rb_setselect(F, RB_SELECT_READ, rb_ssl_accept_common, NULL); - return; - case MBEDTLS_ERR_SSL_WANT_WRITE: - rb_setselect(F, RB_SELECT_WRITE, rb_ssl_accept_common, NULL); - return; - default: - F->ssl_errno = ret; - F->accept->callback(F, RB_ERROR_SSL, NULL, 0, F->accept->data); - return; - } - } - - rb_settimeout(F, 0, NULL, NULL); - rb_setselect(F, RB_SELECT_READ | RB_SELECT_WRITE, NULL, NULL); - - struct acceptdata *const ad = F->accept; - F->accept = NULL; - ad->callback(F, RB_OK, (struct sockaddr *)&ad->S, ad->addrlen, ad->data); - rb_free(ad); -} - -static void -rb_ssl_tryconn_cb(rb_fde_t *const F, void *const data) -{ - lrb_assert(F != NULL); - lrb_assert(F->ssl != NULL); - - mbedtls_ssl_context *const ssl_ctx = SSL_P(F); - - if(ssl_ctx->state != MBEDTLS_SSL_HANDSHAKE_OVER) - { - int ret = mbedtls_ssl_handshake(ssl_ctx); - - switch(ret) - { - case 0: - F->handshake_count++; - break; - case MBEDTLS_ERR_SSL_WANT_READ: - rb_setselect(F, RB_SELECT_READ, rb_ssl_tryconn_cb, data); - return; - case MBEDTLS_ERR_SSL_WANT_WRITE: - rb_setselect(F, RB_SELECT_WRITE, rb_ssl_tryconn_cb, data); - return; - default: - errno = EIO; - F->ssl_errno = ret; - rb_ssl_connect_realcb(F, RB_ERROR_SSL, data); - return; - } - } - - rb_ssl_connect_realcb(F, RB_OK, data); -} - static void rb_ssl_tryconn_timeout_cb(rb_fde_t *const F, void *const data) { From c1007a93d534b606d48282e70c65132f4d17ff0a Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Tue, 30 Aug 2016 23:16:33 +0000 Subject: [PATCH 03/17] MbedTLS: Move more code to appropriate section --- libratbox/src/mbedtls.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 382c9e2d..c6a55210 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -72,6 +72,13 @@ static rb_mbedtls_cfg_context *rb_mbedtls_cfg = NULL; +struct ssl_connect +{ + CNCB *callback; + void *data; + int timeout; +}; + static void rb_ssl_connect_realcb(rb_fde_t *, int, struct ssl_connect *); static int rb_sock_net_recv(void *, unsigned char *, size_t); @@ -578,13 +585,6 @@ rb_ssl_write(rb_fde_t *const F, const void *const buf, size_t count) * Mostly copied from the OpenSSL backend, with some optimisations and complete const-correctness */ -struct ssl_connect -{ - CNCB *callback; - void *data; - int timeout; -}; - static void rb_ssl_connect_realcb(rb_fde_t *const F, int status, struct ssl_connect *const sconn) { From f89406ac7240a69ceeb9902a4edae7b7a8fdc7cb Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Tue, 30 Aug 2016 23:22:41 +0000 Subject: [PATCH 04/17] MbedTLS: Misc sizeof prettiness --- libratbox/src/mbedtls.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index c6a55210..875b6864 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -140,7 +140,7 @@ rb_mbedtls_cfg_new(void) rb_mbedtls_cfg_context *cfg; int ret; - if((cfg = rb_malloc(sizeof(rb_mbedtls_cfg_context))) == NULL) + if((cfg = rb_malloc(sizeof *cfg)) == NULL) return NULL; mbedtls_x509_crt_init(&cfg->crt); @@ -731,7 +731,7 @@ rb_connect_tcp_ssl(rb_fde_t *const F, struct sockaddr *const dest, struct sockad if(F == NULL) return; - struct ssl_connect *const sconn = rb_malloc(sizeof(struct ssl_connect)); + struct ssl_connect *const sconn = rb_malloc(sizeof *sconn); sconn->data = data; sconn->callback = callback; sconn->timeout = timeout; @@ -749,7 +749,7 @@ rb_ssl_start_connected(rb_fde_t *const F, CNCB *const callback, void *const data F->connect->data = data; F->type |= RB_FD_SSL; - struct ssl_connect *const sconn = rb_malloc(sizeof(struct ssl_connect)); + struct ssl_connect *const sconn = rb_malloc(sizeof *sconn); sconn->data = data; sconn->callback = callback; sconn->timeout = timeout; From f2fbec4510d5eac5d13416655845119b2a069fe7 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Tue, 30 Aug 2016 23:31:47 +0000 Subject: [PATCH 05/17] MbedTLS: More const-correctness --- libratbox/src/mbedtls.c | 50 ++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 875b6864..40a83fa5 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -107,7 +107,7 @@ rb_get_ssl_strerror_internal(int err) } static void -rb_mbedtls_cfg_incref(rb_mbedtls_cfg_context *cfg) +rb_mbedtls_cfg_incref(rb_mbedtls_cfg_context *const cfg) { lrb_assert(cfg->refcount > 0); @@ -115,7 +115,7 @@ rb_mbedtls_cfg_incref(rb_mbedtls_cfg_context *cfg) } static void -rb_mbedtls_cfg_decref(rb_mbedtls_cfg_context *cfg) +rb_mbedtls_cfg_decref(rb_mbedtls_cfg_context *const cfg) { if(cfg == NULL) return; @@ -137,10 +137,9 @@ rb_mbedtls_cfg_decref(rb_mbedtls_cfg_context *cfg) static rb_mbedtls_cfg_context * rb_mbedtls_cfg_new(void) { - rb_mbedtls_cfg_context *cfg; - int ret; + rb_mbedtls_cfg_context *const cfg = rb_malloc(sizeof *cfg); - if((cfg = rb_malloc(sizeof *cfg)) == NULL) + if(cfg == NULL) return NULL; mbedtls_x509_crt_init(&cfg->crt); @@ -151,6 +150,8 @@ rb_mbedtls_cfg_new(void) cfg->refcount = 1; + int ret; + if((ret = mbedtls_ssl_config_defaults(&cfg->server_cfg, MBEDTLS_SSL_IS_SERVER, MBEDTLS_SSL_TRANSPORT_STREAM, MBEDTLS_SSL_PRESET_DEFAULT)) != 0) @@ -184,7 +185,7 @@ rb_mbedtls_cfg_new(void) } static void -rb_ssl_setup_mbed_context(rb_fde_t *F, mbedtls_ssl_config *const mbed_config) +rb_ssl_setup_mbed_context(rb_fde_t *const F, mbedtls_ssl_config *const mbed_config) { rb_mbedtls_ssl_context *const mbed_ssl_ctx = rb_malloc(sizeof *mbed_ssl_ctx); if(mbed_ssl_ctx == NULL) @@ -198,6 +199,7 @@ rb_ssl_setup_mbed_context(rb_fde_t *F, mbedtls_ssl_config *const mbed_config) mbedtls_ssl_set_bio(&mbed_ssl_ctx->ssl, F, rb_sock_net_xmit, rb_sock_net_recv, NULL); int ret; + if((ret = mbedtls_ssl_setup(&mbed_ssl_ctx->ssl, mbed_config)) != 0) { rb_lib_log("rb_ssl_setup_mbed_context: ssl_setup: %s", @@ -305,6 +307,7 @@ rb_ssl_shutdown(rb_fde_t *const F) for(int i = 0; i < 4; i++) { int ret = mbedtls_ssl_close_notify(SSL_P(F)); + if(ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE) break; } @@ -315,16 +318,17 @@ rb_ssl_shutdown(rb_fde_t *const F) rb_mbedtls_cfg_decref(SSL_C(F)); rb_free(F->ssl); + F->ssl = NULL; } int rb_init_ssl(void) { - int ret; - mbedtls_ctr_drbg_init(&ctr_drbg_ctx); mbedtls_entropy_init(&entropy_ctx); + int ret; + if((ret = mbedtls_ctr_drbg_seed(&ctr_drbg_ctx, mbedtls_entropy_func, &entropy_ctx, (const unsigned char *)rb_mbedtls_personal_str, sizeof(rb_mbedtls_personal_str))) != 0) { @@ -349,9 +353,6 @@ int rb_setup_ssl_server(const char *const certfile, const char *keyfile, const char *const dhfile, const char *const cipherlist) { - rb_mbedtls_cfg_context *newcfg; - int ret; - if(certfile == NULL) { rb_lib_log("rb_setup_ssl_server: no certificate file specified"); @@ -361,16 +362,21 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, if(keyfile == NULL) keyfile = certfile; - if((newcfg = rb_mbedtls_cfg_new()) == NULL) + rb_mbedtls_cfg_context *const newcfg = rb_mbedtls_cfg_new(); + + if(newcfg == NULL) { rb_lib_log("rb_setup_ssl_server: rb_mbedtls_cfg_new: allocation failed"); return 0; } + int ret; + if((ret = mbedtls_x509_crt_parse_file(&newcfg->crt, certfile)) != 0) { rb_lib_log("rb_setup_ssl_server: x509_crt_parse_file ('%s'): %s", certfile, rb_get_ssl_strerror_internal(ret)); + rb_mbedtls_cfg_decref(newcfg); return 0; } @@ -378,6 +384,7 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, { rb_lib_log("rb_setup_ssl_server: pk_parse_keyfile ('%s'): %s", keyfile, rb_get_ssl_strerror_internal(ret)); + rb_mbedtls_cfg_decref(newcfg); return 0; } @@ -407,6 +414,7 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, { rb_lib_log("rb_setup_ssl_server: ssl_conf_own_cert (server): %s", rb_get_ssl_strerror_internal(ret)); + rb_mbedtls_cfg_decref(newcfg); return 0; } @@ -414,6 +422,7 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, { rb_lib_log("rb_setup_ssl_server: ssl_conf_own_cert (client): %s", rb_get_ssl_strerror_internal(ret)); + rb_mbedtls_cfg_decref(newcfg); return 0; } @@ -457,13 +466,10 @@ rb_get_ssl_strerror(rb_fde_t *const F) } int -rb_get_ssl_certfp(rb_fde_t *const F, uint8_t certfp[RB_SSL_CERTFP_LEN], int method) +rb_get_ssl_certfp(rb_fde_t *const F, uint8_t certfp[const RB_SSL_CERTFP_LEN], int method) { - const mbedtls_x509_crt *peer_cert; - const mbedtls_md_info_t *md_info; mbedtls_md_type_t md_type; int hashlen; - int ret; switch(method) { @@ -483,12 +489,18 @@ rb_get_ssl_certfp(rb_fde_t *const F, uint8_t certfp[RB_SSL_CERTFP_LEN], int meth return 0; } - if((peer_cert = mbedtls_ssl_get_peer_cert(SSL_P(F))) == NULL) + const mbedtls_x509_crt *const peer_cert = mbedtls_ssl_get_peer_cert(SSL_P(F)); + + if(peer_cert == NULL) return 0; - if((md_info = mbedtls_md_info_from_type(md_type)) == NULL) + const mbedtls_md_info_t *const md_info = mbedtls_md_info_from_type(md_type); + + if(md_info == NULL) return 0; + int ret; + if((ret = mbedtls_md(md_info, peer_cert->raw.p, peer_cert->raw.len, certfp)) != 0) { rb_lib_log("rb_get_ssl_certfp: mbedtls_md: %s", @@ -637,6 +649,7 @@ rb_sock_net_recv(void *const context_ptr, unsigned char *const buf, size_t count rb_fde_t *const F = (rb_fde_t *)context_ptr; int ret = (int) read(F->fd, buf, count); + if(ret < 0 && rb_ignore_errno(errno)) return MBEDTLS_ERR_SSL_WANT_READ; @@ -649,6 +662,7 @@ rb_sock_net_xmit(void *const context_ptr, const unsigned char *const buf, size_t rb_fde_t *const F = (rb_fde_t *)context_ptr; int ret = (int) write(F->fd, buf, count); + if(ret < 0 && rb_ignore_errno(errno)) return MBEDTLS_ERR_SSL_WANT_READ; From 19d9c417af1b6442aa26c3b087e5effdc2a22df0 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Tue, 30 Aug 2016 23:38:25 +0000 Subject: [PATCH 06/17] MbedTLS: Fix casing on opening comment block --- libratbox/src/mbedtls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 40a83fa5..c5195c02 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -1,6 +1,6 @@ /* * libratbox: a library used by ircd-ratbox and other things - * mbedtls.c: ARM mbedTLS backend + * mbedtls.c: ARM MbedTLS backend * * Copyright (C) 2007-2008 ircd-ratbox development team * Copyright (C) 2007-2008 Aaron Sethman From cfcd4615ed5d93c4f3d9c0469a51fb349811cb42 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Tue, 30 Aug 2016 23:39:22 +0000 Subject: [PATCH 07/17] README: Fix more MbedTLS casing --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2cfa2910..fe8fb412 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ used with an IRCv3-capable services implementation such as [Atheme][atheme] or [ * OpenSSL 1.0.0 or newer (--enable-openssl) * LibreSSL (--enable-openssl) - * mbedTLS (--enable-mbedtls) + * MbedTLS (--enable-mbedtls) * GnuTLS (--enable-gnutls) * For certificate-based oper CHALLENGE, OpenSSL 1.0.0 or newer. From 036419c3440c2c9141eae47245139da130390fb4 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Wed, 31 Aug 2016 00:13:56 +0000 Subject: [PATCH 08/17] MbedTLS: Misc security improvements As a client, require all peers (i.e. other IRC servers) to support secure renegotiation. Break handshakes with servers that don't. We do not renegotiate our sessions, but this is the most secure option regardless. As a client, disable TLS Session Tickets. The server side MbedTLS code does not have any ticket callbacks configured, so an MbedTLS IRC Server will not issue tickets -- however, others could. Server connections are not expected to be short-lived enough to benefit from the usage of tickets, and their issuance harms forward secrecy. --- libratbox/src/mbedtls.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index c5195c02..1b3640c6 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -158,6 +158,7 @@ rb_mbedtls_cfg_new(void) { rb_lib_log("rb_mbedtls_cfg_new: ssl_config_defaults (server): %s", rb_get_ssl_strerror_internal(ret)); + rb_mbedtls_cfg_decref(cfg); return NULL; } @@ -168,6 +169,7 @@ rb_mbedtls_cfg_new(void) { rb_lib_log("rb_mbedtls_cfg_new: ssl_config_defaults (client): %s", rb_get_ssl_strerror_internal(ret)); + rb_mbedtls_cfg_decref(cfg); return NULL; } @@ -181,6 +183,14 @@ rb_mbedtls_cfg_new(void) mbedtls_ssl_conf_authmode(&cfg->server_cfg, MBEDTLS_SSL_VERIFY_OPTIONAL); mbedtls_ssl_conf_authmode(&cfg->client_cfg, MBEDTLS_SSL_VERIFY_NONE); + #ifdef MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE + mbedtls_ssl_conf_legacy_renegotiation(&cfg->client_cfg, MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE); + #endif + + #ifdef MBEDTLS_SSL_SESSION_TICKETS_DISABLED + mbedtls_ssl_conf_session_tickets(&cfg->client_cfg, MBEDTLS_SSL_SESSION_TICKETS_DISABLED); + #endif + return cfg; } From 531e6323d886d76771d8f389cb25a31a14071363 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Wed, 31 Aug 2016 01:01:42 +0000 Subject: [PATCH 09/17] MbedTLS: Explicitly ignore rb_snprintf() return value --- libratbox/src/mbedtls.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 1b3640c6..3849bb59 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -98,9 +98,9 @@ rb_get_ssl_strerror_internal(int err) #ifdef MBEDTLS_ERROR_C char mbed_errbuf[512]; mbedtls_strerror(err, mbed_errbuf, sizeof mbed_errbuf); - rb_snprintf(errbuf, sizeof errbuf, "(-0x%x) %s", -err, mbed_errbuf); + (void) rb_snprintf(errbuf, sizeof errbuf, "(-0x%x) %s", -err, mbed_errbuf); #else - rb_snprintf(errbuf, sizeof errbuf, "-0x%x", -err); + (void) rb_snprintf(errbuf, sizeof errbuf, "-0x%x", -err); #endif return errbuf; From 42b029d0d613ab07cb6659464b90842b0b145032 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Wed, 31 Aug 2016 17:03:02 +0000 Subject: [PATCH 10/17] MbedTLS: Preliminary attempt at ciphersuite configuration --- libratbox/src/mbedtls.c | 61 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 3849bb59..c9735710 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -45,6 +45,8 @@ #include "mbedtls_embedded_data.h" +#define RB_MAX_CIPHERSUITES 512 + typedef struct { mbedtls_x509_crt crt; @@ -52,6 +54,7 @@ typedef struct mbedtls_dhm_context dhp; mbedtls_ssl_config server_cfg; mbedtls_ssl_config client_cfg; + int suites[RB_MAX_CIPHERSUITES + 1]; size_t refcount; } rb_mbedtls_cfg_context; @@ -148,6 +151,8 @@ rb_mbedtls_cfg_new(void) mbedtls_ssl_config_init(&cfg->server_cfg); mbedtls_ssl_config_init(&cfg->client_cfg); + (void) memset(cfg->suites, 0x00, sizeof cfg->suites); + cfg->refcount = 1; int ret; @@ -340,7 +345,7 @@ rb_init_ssl(void) int ret; if((ret = mbedtls_ctr_drbg_seed(&ctr_drbg_ctx, mbedtls_entropy_func, &entropy_ctx, - (const unsigned char *)rb_mbedtls_personal_str, sizeof(rb_mbedtls_personal_str))) != 0) + (const unsigned char *)rb_mbedtls_personal_str, sizeof(rb_mbedtls_personal_str))) != 0) { rb_lib_log("rb_init_ssl: ctr_drbg_seed: %s", rb_get_ssl_strerror_internal(ret)); @@ -437,7 +442,59 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, return 0; } - /* XXX support cipher lists when added to mbedtls */ + if(cipher_list != NULL) + { + // The cipher_list is (const char *) -- we should not modify it + char *const cipher_list_dup = strdup(cipher_list); + + if(cipher_list_dup == NULL) + { + rb_lib_log("rb_setup_ssl_server: strdup: %s", strerror(errno)); + rb_lib_log("rb_setup_ssl_server: will not configure ciphersuites!"); + } + else + { + size_t suites_count = 0; + char *cipher_str = cipher_list_dup; + + while(*cipher_str != '\0' && suites_count < RB_MAX_CIPHERSUITES) + { + // Arbitrary, but the same separator as OpenSSL uses + char *const cipher_idx = strchr(cipher_str, ':'); + + // This could legitimately be NULL (last ciphersuite in the list) + if(cipher_idx != NULL) + *cipher_idx = '\0'; + + size_t cipher_len = strlen(cipher_str); + int cipher_idn = 0; + + // All MbedTLS ciphersuite names begin with these 4 characters + if(cipher_len > 4 && strncmp(cipher_str, "TLS-", 4) == 0) + cipher_idn = mbedtls_ssl_get_ciphersuite_id(cipher_str); + + // Prevent the same ciphersuite being added multiple times + for(size_t x = 0; cipher_idn != 0 && newcfg->suites[x] != 0; x++) + if(newcfg->suites[x] == cipher_idn) + cipher_idn = 0; + + // Add the suite to the list + if(cipher_idn != 0) + newcfg->suites[suites_count++] = cipher_idn; + + // Advance the string to the next entry -- this could end the loop + cipher_str += (cipher_len + 1); + } + + if(suites_count > 0) + { + mbedtls_ssl_conf_ciphersuites(&newcfg->server_cfg, newcfg->suites); + mbedtls_ssl_conf_ciphersuites(&newcfg->client_cfg, newcfg->suites); + } + + free(cipher_list_dup); + } + } rb_mbedtls_cfg_decref(rb_mbedtls_cfg); rb_mbedtls_cfg = newcfg; From b21ed5c0aa797bb11354b806acc39df6aa94ff02 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Wed, 31 Aug 2016 17:06:51 +0000 Subject: [PATCH 11/17] MbedTLS: Ciphersuite configuration fixes --- libratbox/src/mbedtls.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index c9735710..cab71098 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -442,12 +442,12 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, return 0; } - if(cipher_list != NULL) + if(cipherlist != NULL) { - // The cipher_list is (const char *) -- we should not modify it - char *const cipher_list_dup = strdup(cipher_list); + // The cipherlist is (const char *) -- we should not modify it + char *const cipherlist_dup = strdup(cipherlist); - if(cipher_list_dup == NULL) + if(cipherlist_dup == NULL) { rb_lib_log("rb_setup_ssl_server: strdup: %s", strerror(errno)); rb_lib_log("rb_setup_ssl_server: will not configure ciphersuites!"); @@ -455,7 +455,7 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, else { size_t suites_count = 0; - char *cipher_str = cipher_list_dup; + char *cipher_str = cipherlist_dup; while(*cipher_str != '\0' && suites_count < RB_MAX_CIPHERSUITES) { @@ -492,7 +492,7 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, mbedtls_ssl_conf_ciphersuites(&newcfg->client_cfg, newcfg->suites); } - free(cipher_list_dup); + free(cipherlist_dup); } } From 6f3651f8ecca16f16ed3b82d28e60655061e4454 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Wed, 31 Aug 2016 18:34:21 +0000 Subject: [PATCH 12/17] MbedTLS: Remove pointless no-op cast --- libratbox/src/mbedtls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index cab71098..bfe39f09 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -238,7 +238,7 @@ rb_ssl_accept_common(rb_fde_t *const F, void *const data) lrb_assert(F->accept->callback != NULL); lrb_assert(F->ssl != NULL); - mbedtls_ssl_context *const ssl_ctx = (mbedtls_ssl_context *) SSL_P(F); + mbedtls_ssl_context *const ssl_ctx = SSL_P(F); if(ssl_ctx->state != MBEDTLS_SSL_HANDSHAKE_OVER) { From ede25e0a8a7240629328e103f4c5fb4f92da0cbb Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Wed, 31 Aug 2016 22:03:42 +0000 Subject: [PATCH 13/17] MbedTLS: Log success or failure to parse ciphersuite list --- libratbox/src/mbedtls.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index bfe39f09..6745a4c3 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -490,7 +490,10 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, { mbedtls_ssl_conf_ciphersuites(&newcfg->server_cfg, newcfg->suites); mbedtls_ssl_conf_ciphersuites(&newcfg->client_cfg, newcfg->suites); + rb_lib_log("rb_setup_ssl_server: Configured %zu ciphersuites", suites_count); } + else + rb_lib_log("rb_setup_ssl_server: Passed a list of ciphersuites, but could not configure any"); free(cipherlist_dup); } From 6df12e81691ec2cfbeb3f02fff3d76dd8ef2a2a0 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Thu, 1 Sep 2016 18:18:09 +0000 Subject: [PATCH 14/17] MbedTLS: Cleaner iteration of ciphersuite list --- libratbox/src/mbedtls.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 6745a4c3..8435942e 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -456,11 +456,12 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, { size_t suites_count = 0; char *cipher_str = cipherlist_dup; + char *cipher_idx; - while(*cipher_str != '\0' && suites_count < RB_MAX_CIPHERSUITES) + do { // Arbitrary, but the same separator as OpenSSL uses - char *const cipher_idx = strchr(cipher_str, ':'); + cipher_idx = strchr(cipher_str, ':'); // This could legitimately be NULL (last ciphersuite in the list) if(cipher_idx != NULL) @@ -482,9 +483,11 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, if(cipher_idn != 0) newcfg->suites[suites_count++] = cipher_idn; - // Advance the string to the next entry -- this could end the loop - cipher_str += (cipher_len + 1); - } + // Advance the string to the next entry + if (cipher_idx) + cipher_str = cipher_idx + 1; + + } while(cipher_idx && suites_count < RB_MAX_CIPHERSUITES); if(suites_count > 0) { From df51e80717e3367a99ff7dc34be733cc4646a9d5 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Thu, 1 Sep 2016 20:47:34 +0000 Subject: [PATCH 15/17] MbedTLS: Provide default list of configured ciphersuites --- libratbox/src/mbedtls.c | 45 ++++++++++----- libratbox/src/mbedtls_embedded_data.h | 79 +++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 15 deletions(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index 8435942e..b3c9678d 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -442,19 +442,18 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, return 0; } + + + const int *rb_ciphersuites = newcfg->suites; + size_t suites_count = 0; + if(cipherlist != NULL) { // The cipherlist is (const char *) -- we should not modify it char *const cipherlist_dup = strdup(cipherlist); - if(cipherlist_dup == NULL) + if(cipherlist_dup != NULL) { - rb_lib_log("rb_setup_ssl_server: strdup: %s", strerror(errno)); - rb_lib_log("rb_setup_ssl_server: will not configure ciphersuites!"); - } - else - { - size_t suites_count = 0; char *cipher_str = cipherlist_dup; char *cipher_idx; @@ -489,18 +488,34 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, } while(cipher_idx && suites_count < RB_MAX_CIPHERSUITES); - if(suites_count > 0) - { - mbedtls_ssl_conf_ciphersuites(&newcfg->server_cfg, newcfg->suites); - mbedtls_ssl_conf_ciphersuites(&newcfg->client_cfg, newcfg->suites); - rb_lib_log("rb_setup_ssl_server: Configured %zu ciphersuites", suites_count); - } - else - rb_lib_log("rb_setup_ssl_server: Passed a list of ciphersuites, but could not configure any"); + if(suites_count == 0) + rb_lib_log("rb_setup_ssl_server: Ciphersuites provided, but could not parse any"); free(cipherlist_dup); } + else + { + rb_lib_log("rb_setup_ssl_server: strdup: %s", strerror(errno)); + } } + else + { + rb_lib_log("rb_setup_ssl_server: No ciphersuite list provided"); + } + + if(suites_count == 0) + { + rb_lib_log("rb_setup_ssl_server: Using default ciphersuites"); + + rb_ciphersuites = rb_mbedtls_ciphersuites; + suites_count = sizeof(rb_mbedtls_ciphersuites) / sizeof(rb_mbedtls_ciphersuites[0]); + } + + mbedtls_ssl_conf_ciphersuites(&newcfg->server_cfg, rb_ciphersuites); + mbedtls_ssl_conf_ciphersuites(&newcfg->client_cfg, rb_ciphersuites); + rb_lib_log("rb_setup_ssl_server: Configured %zu ciphersuites", suites_count); + + rb_mbedtls_cfg_decref(rb_mbedtls_cfg); rb_mbedtls_cfg = newcfg; diff --git a/libratbox/src/mbedtls_embedded_data.h b/libratbox/src/mbedtls_embedded_data.h index 83782089..4eb7ca81 100644 --- a/libratbox/src/mbedtls_embedded_data.h +++ b/libratbox/src/mbedtls_embedded_data.h @@ -25,11 +25,90 @@ #ifndef RB_MBEDTLS_EMBEDDED_DATA_H #define RB_MBEDTLS_EMBEDDED_DATA_H +#include "mbedtls/ssl_ciphersuites.h" + /* * Personalization string for CTR-DRBG initialization */ static const char rb_mbedtls_personal_str[] = "charybdis/librb personalization string"; +/* + * Default list of supported ciphersuites + * User can override with ssl_cipher_list option in ircd.conf + * + * Charybdis cannot have more than one certificate configured, which means that with + * the MbedTLS backend, it will ALWAYS be serving EITHER an RSA OR ECDSA certificate. + * + * This means we can order ciphersuites to place all ECDSA ones ahead of RSA ones, + * without weird interactions of cipher order, such as inadvertantly preferring an + * ECDSA ciphersuite with AES128-CBC-SHA over an RSA ciphersuite with + * AES256-GCM-SHA384. + * + * We also prefer all AEAD ciphersuites first, even if it results in using a 128-bit + * AEAD ciphersuite instead of a 256-bit CBC ciphersuite. This is due to the fact that + * ONLY the AEAD ciphersuites in TLS are cryptographically secure in practice; the ETM + * extension for CBC ciphersuites has not seen wide adoption. This choice can be + * revisited in future; please consult me first. -- amdj + */ +static const int rb_mbedtls_ciphersuites[] = { + + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_CAMELLIA_256_GCM_SHA384, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_256_CCM, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_CAMELLIA_128_GCM_SHA256, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_CCM, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_CAMELLIA_256_CBC_SHA384, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_CAMELLIA_128_CBC_SHA256, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + + MBEDTLS_TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + MBEDTLS_TLS_ECDHE_RSA_WITH_CAMELLIA_256_GCM_SHA384, + MBEDTLS_TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + MBEDTLS_TLS_ECDHE_RSA_WITH_CAMELLIA_128_GCM_SHA256, + MBEDTLS_TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, + MBEDTLS_TLS_ECDHE_RSA_WITH_CAMELLIA_256_CBC_SHA384, + MBEDTLS_TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + MBEDTLS_TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, + MBEDTLS_TLS_ECDHE_RSA_WITH_CAMELLIA_128_CBC_SHA256, + MBEDTLS_TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + + MBEDTLS_TLS_DHE_RSA_WITH_AES_256_GCM_SHA384, + MBEDTLS_TLS_DHE_RSA_WITH_CAMELLIA_256_GCM_SHA384, + MBEDTLS_TLS_DHE_RSA_WITH_AES_256_CCM, + MBEDTLS_TLS_DHE_RSA_WITH_AES_256_CBC_SHA256, + MBEDTLS_TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA256, + MBEDTLS_TLS_DHE_RSA_WITH_AES_256_CBC_SHA, + MBEDTLS_TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA, + MBEDTLS_TLS_DHE_RSA_WITH_AES_128_GCM_SHA256, + MBEDTLS_TLS_DHE_RSA_WITH_CAMELLIA_128_GCM_SHA256, + MBEDTLS_TLS_DHE_RSA_WITH_AES_128_CCM, + MBEDTLS_TLS_DHE_RSA_WITH_AES_128_CBC_SHA256, + MBEDTLS_TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA256, + MBEDTLS_TLS_DHE_RSA_WITH_AES_128_CBC_SHA, + MBEDTLS_TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA, + + MBEDTLS_TLS_RSA_WITH_AES_256_GCM_SHA384, + MBEDTLS_TLS_RSA_WITH_CAMELLIA_256_GCM_SHA384, + MBEDTLS_TLS_RSA_WITH_AES_256_CCM, + MBEDTLS_TLS_RSA_WITH_AES_128_GCM_SHA256, + MBEDTLS_TLS_RSA_WITH_CAMELLIA_128_GCM_SHA256, + MBEDTLS_TLS_RSA_WITH_AES_128_CCM, + MBEDTLS_TLS_RSA_WITH_AES_256_CBC_SHA256, + MBEDTLS_TLS_RSA_WITH_CAMELLIA_256_CBC_SHA256, + MBEDTLS_TLS_RSA_WITH_AES_256_CBC_SHA, + MBEDTLS_TLS_RSA_WITH_CAMELLIA_256_CBC_SHA, + MBEDTLS_TLS_RSA_WITH_AES_128_CBC_SHA256, + MBEDTLS_TLS_RSA_WITH_CAMELLIA_128_CBC_SHA256, + MBEDTLS_TLS_RSA_WITH_AES_128_CBC_SHA, + MBEDTLS_TLS_RSA_WITH_CAMELLIA_128_CBC_SHA, + + 0 // End of list +}; + /* * YES, this is a hardcoded CA certificate. * From 0db0805ed596ae850eb1369d38d17671592739d8 Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Thu, 1 Sep 2016 20:57:07 +0000 Subject: [PATCH 16/17] MbedTLS: Don't include the sentinel in suites count calculation --- libratbox/src/mbedtls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index b3c9678d..aa898d77 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -508,7 +508,7 @@ rb_setup_ssl_server(const char *const certfile, const char *keyfile, rb_lib_log("rb_setup_ssl_server: Using default ciphersuites"); rb_ciphersuites = rb_mbedtls_ciphersuites; - suites_count = sizeof(rb_mbedtls_ciphersuites) / sizeof(rb_mbedtls_ciphersuites[0]); + suites_count = (sizeof(rb_mbedtls_ciphersuites) / sizeof(rb_mbedtls_ciphersuites[0])) - 1; } mbedtls_ssl_conf_ciphersuites(&newcfg->server_cfg, rb_ciphersuites); From be31ac33d534b7be46b4ee62762b6724256191ed Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Fri, 2 Sep 2016 00:28:17 +0000 Subject: [PATCH 17/17] MbedTLS: Use correct error code for failed socket writes This should make writing more efficient. --- libratbox/src/mbedtls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libratbox/src/mbedtls.c b/libratbox/src/mbedtls.c index aa898d77..e059672f 100644 --- a/libratbox/src/mbedtls.c +++ b/libratbox/src/mbedtls.c @@ -752,7 +752,7 @@ rb_sock_net_xmit(void *const context_ptr, const unsigned char *const buf, size_t int ret = (int) write(F->fd, buf, count); if(ret < 0 && rb_ignore_errno(errno)) - return MBEDTLS_ERR_SSL_WANT_READ; + return MBEDTLS_ERR_SSL_WANT_WRITE; return ret; }