From 6bc92f7592b93b98a0726e443e83613d3b5bf65d Mon Sep 17 00:00:00 2001 From: Andy Green Date: Wed, 8 Nov 2017 20:27:11 +0800 Subject: [PATCH] wrapper: untrash partial sends before WANT 1) The original wrapper logic for accounting for partial sends on mbedtls is broken... adapt it 2) mbedtls has an additional restriction you MUST come back to retry with EXACTLY the unsent part that you originally asked for, or it loses coherency in the TLS tunnel. 3) the wrapper is inconsistent between read WANT state dynamically reported from mbedtls apis and the SSL_want() api results. Check both on read. --- lib/tls/mbedtls/ssl.c | 4 +- .../wrapper/include/internal/ssl_types.h | 1 + lib/tls/mbedtls/wrapper/library/ssl_lib.c | 31 +++++++++----- lib/tls/mbedtls/wrapper/platform/ssl_pm.c | 41 +++++++++++++++++-- 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/lib/tls/mbedtls/ssl.c b/lib/tls/mbedtls/ssl.c index a9d019ce..7746b53a 100644 --- a/lib/tls/mbedtls/ssl.c +++ b/lib/tls/mbedtls/ssl.c @@ -100,12 +100,12 @@ lws_ssl_capable_read(struct lws *wsi, unsigned char *buf, int len) m == SSL_ERROR_SYSCALL) return LWS_SSL_CAPABLE_ERROR; - if (SSL_want_read(wsi->ssl)) { + if (m == SSL_ERROR_WANT_READ || SSL_want_read(wsi->ssl)) { lwsl_debug("%s: WANT_READ\n", __func__); lwsl_debug("%p: LWS_SSL_CAPABLE_MORE_SERVICE\n", wsi); return LWS_SSL_CAPABLE_MORE_SERVICE; } - if (SSL_want_write(wsi->ssl)) { + if (m == SSL_ERROR_WANT_WRITE || SSL_want_write(wsi->ssl)) { lwsl_debug("%s: WANT_WRITE\n", __func__); lwsl_debug("%p: LWS_SSL_CAPABLE_MORE_SERVICE\n", wsi); return LWS_SSL_CAPABLE_MORE_SERVICE; diff --git a/lib/tls/mbedtls/wrapper/include/internal/ssl_types.h b/lib/tls/mbedtls/wrapper/include/internal/ssl_types.h index 45198bc9..2ca438c4 100644 --- a/lib/tls/mbedtls/wrapper/include/internal/ssl_types.h +++ b/lib/tls/mbedtls/wrapper/include/internal/ssl_types.h @@ -215,6 +215,7 @@ struct ssl_st int (*verify_callback) (int ok, X509_STORE_CTX *ctx); int rwstate; + int interrupted_remaining_write; long verify_result; diff --git a/lib/tls/mbedtls/wrapper/library/ssl_lib.c b/lib/tls/mbedtls/wrapper/library/ssl_lib.c index 187fc9f0..d8fdd06f 100644 --- a/lib/tls/mbedtls/wrapper/library/ssl_lib.c +++ b/lib/tls/mbedtls/wrapper/library/ssl_lib.c @@ -142,9 +142,9 @@ int SSL_get_error(const SSL *ssl, int ret_code) ret = SSL_ERROR_NONE; else if (ret_code < 0) { - if (SSL_want_read(ssl)) + if (ssl->err == SSL_ERROR_WANT_READ || SSL_want_read(ssl)) ret = SSL_ERROR_WANT_READ; - else if (SSL_want_write(ssl)) + else if (ssl->err == SSL_ERROR_WANT_WRITE || SSL_want_write(ssl)) ret = SSL_ERROR_WANT_WRITE; else ret = SSL_ERROR_SYSCALL; //unknown @@ -457,7 +457,7 @@ int SSL_read(SSL *ssl, void *buffer, int len) int SSL_write(SSL *ssl, const void *buffer, int len) { int ret; - int send_bytes; + int send_bytes, bytes; const unsigned char *pbuf; SSL_ASSERT1(ssl); @@ -470,25 +470,36 @@ int SSL_write(SSL *ssl, const void *buffer, int len) pbuf = (const unsigned char *)buffer; do { - int bytes; - if (send_bytes > SSL_SEND_DATA_MAX_LENGTH) bytes = SSL_SEND_DATA_MAX_LENGTH; else bytes = send_bytes; + if (ssl->interrupted_remaining_write) { + bytes = ssl->interrupted_remaining_write; + ssl->interrupted_remaining_write = 0; + } + ret = SSL_METHOD_CALL(send, ssl, pbuf, bytes); + //printf("%s: ssl_pm said %d for %d requested (cum %d)\n", __func__, ret, bytes, len -send_bytes); + /* the return is a NEGATIVE OpenSSL error code, or the length sent */ if (ret > 0) { pbuf += ret; send_bytes -= ret; - } - } while (ret > 0 && send_bytes); + } else + ssl->interrupted_remaining_write = bytes; + } while (ret > 0 && send_bytes && ret == bytes); if (ret >= 0) { ret = len - send_bytes; - ssl->rwstate = SSL_NOTHING; - } else - ret = -1; + if (!ret) + ssl->rwstate = SSL_NOTHING; + } else { + if (send_bytes == len) + ret = -1; + else + ret = len - send_bytes; + } return ret; } diff --git a/lib/tls/mbedtls/wrapper/platform/ssl_pm.c b/lib/tls/mbedtls/wrapper/platform/ssl_pm.c index 536733fb..63504919 100755 --- a/lib/tls/mbedtls/wrapper/platform/ssl_pm.c +++ b/lib/tls/mbedtls/wrapper/platform/ssl_pm.c @@ -360,17 +360,52 @@ int ssl_pm_read(SSL *ssl, void *buffer, int len) return ret; } +/* + * This returns -1, or the length sent. + * If -1, then you need to find out if the error was + * fatal or recoverable using SSL_get_error() + */ int ssl_pm_send(SSL *ssl, const void *buffer, int len) { int ret; struct ssl_pm *ssl_pm = (struct ssl_pm *)ssl->ssl_pm; ret = mbedtls_ssl_write(&ssl_pm->ssl, buffer, len); + /* + * We can get a positive number, which may be less than len... that + * much was sent successfully and you can call again to send more. + * + * We can get a negative mbedtls error code... if WANT_WRITE or WANT_READ, + * it's nonfatal and means it should be retried as-is. If something else, + * it's fatal actually. + * + * If this function returns something other than a positive value or + * MBEDTLS_ERR_SSL_WANT_READ/WRITE, the ssl context becomes unusable, and + * you should either free it or call mbedtls_ssl_session_reset() on it + * before re-using it for a new connection; the current connection must + * be closed. + * + * When this function returns MBEDTLS_ERR_SSL_WANT_WRITE/READ, it must be + * called later with the same arguments, until it returns a positive value. + */ + if (ret < 0) { - if (ret == MBEDTLS_ERR_NET_CONN_RESET) + SSL_DEBUG(SSL_PLATFORM_ERROR_LEVEL, "mbedtls_ssl_write() return -0x%x", -ret); + switch (ret) { + case MBEDTLS_ERR_NET_CONN_RESET: ssl->err = SSL_ERROR_SYSCALL; - SSL_DEBUG(SSL_PLATFORM_ERROR_LEVEL, "mbedtls_ssl_write() return -0x%x", -ret); - ret = -1; + break; + case MBEDTLS_ERR_SSL_WANT_WRITE: + ssl->err = SSL_ERROR_WANT_WRITE; + break; + case MBEDTLS_ERR_SSL_WANT_READ: + ssl->err = SSL_ERROR_WANT_READ; + break; + default: + break; + } + + ret = -1; } return ret;