From 52df9d6c69ee627a1e5d0829ff0d654c1be1eba2 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 27 Aug 2020 20:57:53 +0200 Subject: [PATCH] TLS and DTLS both need to support APP DATA during SCR Also some misc fixes --- examples/client/client.c | 4 +-- examples/server/server.c | 28 +++++++++++++-- src/internal.c | 73 +++++++++++++++++++--------------------- src/ssl.c | 16 --------- 4 files changed, 63 insertions(+), 58 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 0288325e3..b8c37b110 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -3096,8 +3096,8 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) if (err == WOLFSSL_ERROR_WANT_READ || err == WOLFSSL_ERROR_WANT_WRITE) { (void)ClientWrite(ssl, - "This is a fun message sent during renegotiation", - sizeof("This is a fun message sent during renegotiation"), + "msg sent during renegotiation", + sizeof("msg sent during renegotiation"), "", 1); do { if (err == APP_DATA_READY) { diff --git a/examples/server/server.c b/examples/server/server.c index d5acdff89..8ccac29b4 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -474,6 +474,17 @@ static void ServerRead(WOLFSSL* ssl, char* input, int inputLen) if (ret < 0) { err = SSL_get_error(ssl, 0); + #ifdef HAVE_SECURE_RENEGOTIATION + if (err == APP_DATA_READY) { + ret = SSL_read(ssl, input, inputLen); + if (ret >= 0) { + /* null terminate message */ + input[ret] = '\0'; + printf("Client message: %s\n", input); + return; + } + } + #endif #ifdef WOLFSSL_ASYNC_CRYPT if (err == WC_PENDING_E) { ret = wolfSSL_AsyncPoll(ssl, WOLF_POLL_FLAG_CHECK_HW); @@ -487,7 +498,11 @@ static void ServerRead(WOLFSSL* ssl, char* input, int inputLen) } else #endif - if (err != WOLFSSL_ERROR_WANT_READ) { + if (err != WOLFSSL_ERROR_WANT_READ + #ifdef HAVE_SECURE_RENEGOTIATION + && err != APP_DATA_READY + #endif + ) { printf("SSL_read input error %d, %s\n", err, ERR_error_string(err, buffer)); err_sys_ex(runWithErrors, "SSL_read failed"); @@ -499,7 +514,8 @@ static void ServerRead(WOLFSSL* ssl, char* input, int inputLen) } } while (err == WC_PENDING_E || err == WOLFSSL_ERROR_WANT_READ); if (ret > 0) { - input[ret] = 0; /* null terminate message */ + /* null terminate message */ + input[ret] = '\0'; printf("Client message: %s\n", input); } } @@ -2428,6 +2444,14 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) if (echoData == 0 && throughput == 0) { ServerRead(ssl, input, sizeof(input)-1); err = SSL_get_error(ssl, 0); +#ifdef HAVE_SECURE_RENEGOTIATION + if (err == APP_DATA_READY) { + /* Data was sent during SCR so let's get the message + * after the SCR as well */ + ServerRead(ssl, input, sizeof(input)-1); + err = SSL_get_error(ssl, 0); + } +#endif } #if defined(HAVE_SECURE_RENEGOTIATION) && \ diff --git a/src/internal.c b/src/internal.c index ab370fdf4..b81c0feb2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14589,23 +14589,12 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx) *inOutIdx = idx; #ifdef HAVE_SECURE_RENEGOTIATION if (IsSCR(ssl)) { + /* Reset the processReply state since + * we finished processing this message. */ + ssl->options.processReply = doProcessInit; /* If we are in a secure renegotiation then APP DATA is treated * differently */ - if (ssl->options.dtls) { - /* Reset the processReply state since - * we finished processing this message. */ - ssl->options.processReply = doProcessInit; - return APP_DATA_READY; - } - else { - /* TODO should fail for TLS? */ - ssl->buffers.clearOutputBuffer.buffer = NULL; - ssl->buffers.clearOutputBuffer.length = 0; -#ifdef WOLFSSL_EXTRA_ALERTS - SendAlert(ssl, alert_fatal, unexpected_message); -#endif - return OUT_OF_ORDER_E; - } + return APP_DATA_READY; } #endif return 0; @@ -17676,8 +17665,7 @@ int SendData(WOLFSSL* ssl, const void* data, int sz) } else #endif - if (ssl->options.handShakeState != HANDSHAKE_DONE && - !ssl->options.dtls /* Allow data during renegotiation */ ) { + if (ssl->options.handShakeState != HANDSHAKE_DONE && !IsSCR(ssl)) { int err; WOLFSSL_MSG("handshake not complete, trying to finish"); if ( (err = wolfSSL_negotiate(ssl)) != WOLFSSL_SUCCESS) { @@ -17836,6 +17824,9 @@ int ReceiveData(WOLFSSL* ssl, byte* output, int sz, int peek) if (ssl->error != 0 && ssl->error != WANT_WRITE #ifdef WOLFSSL_ASYNC_CRYPT && ssl->error != WC_PENDING_E +#endif +#ifdef HAVE_SECURE_RENEGOTIATION + && ssl->error != APP_DATA_READY #endif ) { WOLFSSL_MSG("User calling wolfSSL_read in error state, not allowed"); @@ -17847,17 +17838,31 @@ int ReceiveData(WOLFSSL* ssl, byte* output, int sz, int peek) } else #endif - if (ssl->options.handShakeState != HANDSHAKE_DONE) { - int err; - WOLFSSL_MSG("Handshake not complete, trying to finish"); - if ( (err = wolfSSL_negotiate(ssl)) != WOLFSSL_SUCCESS) { - #ifdef WOLFSSL_ASYNC_CRYPT - /* if async would block return WANT_WRITE */ - if (ssl->error == WC_PENDING_E) { - return WOLFSSL_CBIO_ERR_WANT_READ; + { + int negotiate = 0; +#ifdef HAVE_SECURE_RENEGOTIATION + if (ssl->secure_renegotiation && ssl->secure_renegotiation->enabled) { + if (ssl->options.handShakeState != HANDSHAKE_DONE + && ssl->buffers.clearOutputBuffer.length == 0) + negotiate = 1; + } + else +#endif + if (ssl->options.handShakeState != HANDSHAKE_DONE) + negotiate = 1; + + if (negotiate) { + int err; + WOLFSSL_MSG("Handshake not complete, trying to finish"); + if ( (err = wolfSSL_negotiate(ssl)) != WOLFSSL_SUCCESS) { + #ifdef WOLFSSL_ASYNC_CRYPT + /* if async would block return WANT_WRITE */ + if (ssl->error == WC_PENDING_E) { + return WOLFSSL_CBIO_ERR_WANT_READ; + } + #endif + return err; } - #endif - return err; } } @@ -17865,18 +17870,10 @@ int ReceiveData(WOLFSSL* ssl, byte* output, int sz, int peek) startScr: if (ssl->secure_renegotiation && ssl->secure_renegotiation->startScr) { int ret; - int err; - WOLFSSL_MSG("Need to start scr, server requested"); - if ( (ret = wolfSSL_Rehandshake(ssl)) != WOLFSSL_SUCCESS) { - err = wolfSSL_get_error(ssl, 0); - if (err == WOLFSSL_ERROR_WANT_READ || - err == WOLFSSL_ERROR_WANT_WRITE || - err == APP_DATA_READY) - ssl->secure_renegotiation->startScr = 0; /* only start once - * on non-blocking */ - return ret; - } + ret = wolfSSL_Rehandshake(ssl); ssl->secure_renegotiation->startScr = 0; /* only start once */ + if (ret != WOLFSSL_SUCCESS) + return ret; } #endif diff --git a/src/ssl.c b/src/ssl.c index e51f068fb..11bc08a3c 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -2020,22 +2020,6 @@ static int wolfSSL_read_internal(WOLFSSL* ssl, void* data, int sz, int peek) errno = 0; #endif -#ifdef HAVE_SECURE_RENEGOTIATION - if (ssl->buffers.clearOutputBuffer.length > 0) { - int size = min(sz, ssl->buffers.clearOutputBuffer.length); - XMEMCPY(data, ssl->buffers.clearOutputBuffer.buffer, size); - if (peek == 0) { - ssl->buffers.clearOutputBuffer.length -= size; - ssl->buffers.clearOutputBuffer.buffer += size; - } - if (ssl->buffers.clearOutputBuffer.length == 0 && - ssl->buffers.inputBuffer.dynamicFlag) - ShrinkInputBuffer(ssl, NO_FORCED_FREE); - WOLFSSL_LEAVE("wolfSSL_read_internal()", size); - return size; - } -#endif - #ifdef WOLFSSL_DTLS if (ssl->options.dtls) { ssl->dtls_expected_rx = max(sz + 100, MAX_MTU);