From e93e3b60da181fff60f74b3b755608dd256b93f2 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 10 Sep 2019 11:51:38 -0700 Subject: [PATCH 1/5] DTLS Maintenance Allow the DTLS server to retransmit a stored flight of messages in an additional acccept state. (ZD5644) --- src/internal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index e96755cf9..ef7681360 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6645,7 +6645,8 @@ int DtlsMsgPoolSend(WOLFSSL* ssl, int sendOnlyFirstPacket) if (pool != NULL) { if ((ssl->options.side == WOLFSSL_SERVER_END && !(ssl->options.acceptState == SERVER_HELLO_DONE || - ssl->options.acceptState == ACCEPT_FINISHED_DONE)) || + ssl->options.acceptState == ACCEPT_FINISHED_DONE || + ssl->options.acceptState == ACCEPT_THIRD_REPLY_DONE)) || (ssl->options.side == WOLFSSL_CLIENT_END && !(ssl->options.connectState == CLIENT_HELLO_SENT || ssl->options.connectState == HELLO_AGAIN_REPLY || From 22c398494e974783705e3f100bc9af2dc7206a99 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 10 Sep 2019 15:59:22 -0700 Subject: [PATCH 2/5] DTLS Maintenance The options to switch on and off the code to serialize/deserialize items in the struct need to match the options for the struct. (ZD5130, ZD5590) --- src/ssl.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index a511ac933..1ee210a29 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -11409,7 +11409,8 @@ static WC_INLINE void RestoreSession(WOLFSSL* ssl, WOLFSSL_SESSION* session, #endif } #endif /* SESSION_CERTS */ -#ifndef NO_RESUME_SUITE_CHECK +#if !defined(NO_RESUME_SUITE_CHECK) || \ + (defined(WOLFSSL_TLS13) && defined(HAVE_SESSION_TICKET)) ssl->session.cipherSuite0 = session->cipherSuite0; ssl->session.cipherSuite = session->cipherSuite; #endif @@ -18352,7 +18353,8 @@ const char* wolfSSL_SESSION_CIPHER_get_name(WOLFSSL_SESSION* session) return NULL; } -#ifdef SESSION_CERTS +#if defined(SESSION_CERTS) || !defined(NO_RESUME_SUITE_CHECK) || \ + (defined(WOLFSSL_TLS13) && defined(HAVE_SESSION_TICKET)) #if !defined(WOLFSSL_CIPHER_INTERNALNAME) && !defined(NO_ERROR_STRINGS) return GetCipherNameIana(session->cipherSuite0, session->cipherSuite); #else @@ -24775,12 +24777,16 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p) size += OPAQUE8_LEN; for (i = 0; i < sess->chain.count; i++) size += OPAQUE16_LEN + sess->chain.certs[i].length; +#endif +#if defined(SESSION_CERTS) || (defined(WOLFSSL_TLS13) && \ + defined(HAVE_SESSION_TICKET)) /* Protocol version */ size += OPAQUE16_LEN; #endif -#if defined(SESSION_CERTS) || !defined(NO_RESUME_SUITE_CHECK) +#if defined(SESSION_CERTS) || !defined(NO_RESUME_SUITE_CHECK) || \ + (defined(WOLFSSL_TLS13) && defined(HAVE_SESSION_TICKET)) /* cipher suite */ - size += OPAQUE16_LEN + OPAQUE16_LEN; + size += OPAQUE16_LEN; #endif #ifndef NO_CLIENT_CACHE /* ServerID len | ServerID */ @@ -24818,10 +24824,14 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p) sess->chain.certs[i].length); idx += sess->chain.certs[i].length; } +#endif +#if defined(SESSION_CERTS) || (defined(WOLFSSL_TLS13) && \ + defined(HAVE_SESSION_TICKET)) data[idx++] = sess->version.major; data[idx++] = sess->version.minor; #endif -#if defined(SESSION_CERTS) || !defined(NO_RESUME_SUITE_CHECK) +#if defined(SESSION_CERTS) || !defined(NO_RESUME_SUITE_CHECK) || \ + (defined(WOLFSSL_TLS13) && defined(HAVE_SESSION_TICKET)) data[idx++] = sess->cipherSuite0; data[idx++] = sess->cipherSuite; #endif @@ -24940,16 +24950,24 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess, XMEMCPY(s->chain.certs[j].buffer, data + idx, length); idx += length; } - - /* Protocol Version | Cipher suite */ - if (i - idx < OPAQUE16_LEN + OPAQUE16_LEN) { +#endif +#if defined(SESSION_CERTS) || (defined(WOLFSSL_TLS13) && \ + defined(HAVE_SESSION_TICKET)) + /* Protocol Version */ + if (i - idx < OPAQUE16_LEN) { ret = BUFFER_ERROR; goto end; } s->version.major = data[idx++]; s->version.minor = data[idx++]; #endif -#if defined(SESSION_CERTS) || !defined(NO_RESUME_SUITE_CHECK) +#if defined(SESSION_CERTS) || !defined(NO_RESUME_SUITE_CHECK) || \ + (defined(WOLFSSL_TLS13) && defined(HAVE_SESSION_TICKET)) + /* Cipher suite */ + if (i - idx < OPAQUE16_LEN) { + ret = BUFFER_ERROR; + goto end; + } s->cipherSuite0 = data[idx++]; s->cipherSuite = data[idx++]; #endif From 852d50adcf54db410e236ad892bcf8065bd0871c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 11 Sep 2019 15:28:30 -0700 Subject: [PATCH 3/5] DTLS Maintenance To go with the fix for the functions wolfSSL_(i2d|d2i)_SSL_SESSION, modify the example client to use a serialized session record for resumption instead of the direct reference into the session cache. This change only happens when OPENSSL_EXTRA and HAVE_EXT_CACHE are defined. --- examples/client/client.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/examples/client/client.c b/examples/client/client.c index 56f551b95..ef0b16977 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -1326,6 +1326,8 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) WOLFSSL* sslResume = 0; WOLFSSL_SESSION* session = 0; + byte* flatSession = NULL; + int flatSessionSz = 0; #ifndef WOLFSSL_ALT_TEST_STRINGS char msg[32] = "hello wolfssl!"; /* GET may make bigger */ @@ -1485,6 +1487,8 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) #endif (void)resumeSz; (void)session; + (void)flatSession; + (void)flatSessionSz; (void)sslResume; (void)atomicUser; (void)scr; @@ -2986,6 +2990,19 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) } #endif +#if defined(OPENSSL_EXTRA) && defined(HAVE_EXT_CACHE) + if (session != NULL && resumeSession) { + flatSessionSz = wolfSSL_i2d_SSL_SESSION(session, NULL); + if (flatSessionSz != 0) { + int checkSz = wolfSSL_i2d_SSL_SESSION(session, &flatSession); + if (flatSession == NULL) + err_sys("error creating flattened session buffer"); + if (checkSz != flatSessionSz) + err_sys("flat session size check failure"); + } + } +#endif + if (dtlsUDP == 0) { /* don't send alert after "break" command */ ret = wolfSSL_shutdown(ssl); if (wc_shutdown && ret == WOLFSSL_SHUTDOWN_NOT_DONE) @@ -3059,7 +3076,23 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) } } #endif + +#if defined(OPENSSL_EXTRA) && defined(HAVE_EXT_CACHE) + if (flatSession) { + const byte* constFlatSession = flatSession; + session = wolfSSL_d2i_SSL_SESSION(NULL, + &constFlatSession, flatSessionSz); + } +#endif + wolfSSL_set_session(sslResume, session); + +#if defined(OPENSSL_EXTRA) && defined(HAVE_EXT_CACHE) + if (flatSession) { + XFREE(flatSession, heap, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(session, heap, DYNAMIC_TYPE_OPENSSL); + } +#endif #ifdef HAVE_SESSION_TICKET wolfSSL_set_SessionTicket_cb(sslResume, sessionTicketCB, (void*)"resumed session"); From c27a4b38656342d286e649cc04b01a6b1e37b7b4 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 11 Sep 2019 16:44:54 -0700 Subject: [PATCH 4/5] TLS Maintenance When serializing the WOLFSSL_SESSION, serialize everything. --- src/ssl.c | 128 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 104 insertions(+), 24 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 1ee210a29..24e9c071e 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -24792,14 +24792,31 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p) /* ServerID len | ServerID */ size += OPAQUE16_LEN + sess->idLen; #endif -#ifdef HAVE_SESSION_TICKET - /* ticket len | ticket */ - size += OPAQUE16_LEN + sess->ticketLen; -#endif #ifdef OPENSSL_EXTRA /* session context ID len | session context ID */ size += OPAQUE8_LEN + sess->sessionCtxSz; #endif +#ifdef WOLFSSL_TLS13 + /* namedGroup */ + size += OPAQUE16_LEN; +#endif +#if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) +#ifdef WOLFSSL_TLS13 + /* ticketSeen | ticketAdd */ + size += OPAQUE32_LEN + OPAQUE32_LEN; +#ifndef WOLFSSL_TLS13_DRAFT_18 + /* ticketNonce */ + size += OPAQUE8_LEN + sess->ticketNonce.len; +#endif +#endif +#ifdef WOLFSSL_EARLY_DATA + size += OPAQUE32_LEN; +#endif +#endif +#ifdef HAVE_SESSION_TICKET + /* ticket len | ticket */ + size += OPAQUE16_LEN + sess->ticketLen; +#endif if (p != NULL) { if (*p == NULL) @@ -24840,15 +24857,36 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p) XMEMCPY(data + idx, sess->serverID, sess->idLen); idx += sess->idLen; #endif -#ifdef HAVE_SESSION_TICKET - c16toa(sess->ticketLen, data + idx); idx += OPAQUE16_LEN; - XMEMCPY(data + idx, sess->ticket, sess->ticketLen); - idx += sess->ticketLen; -#endif #ifdef OPENSSL_EXTRA data[idx++] = sess->sessionCtxSz; XMEMCPY(data + idx, sess->sessionCtx, sess->sessionCtxSz); idx += sess->sessionCtxSz; +#endif +#ifdef WOLFSSL_TLS13 + c16toa(sess->namedGroup, data + idx); + idx += OPAQUE16_LEN; +#endif +#if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) +#ifdef WOLFSSL_TLS13 + c32toa(sess->ticketSeen, data + idx); + idx += OPAQUE32_LEN; + c32toa(sess->ticketAdd, data + idx); + idx += OPAQUE32_LEN; +#ifndef WOLFSSL_TLS13_DRAFT_18 + data[idx++] = sess->ticketNonce.len; + XMEMCPY(data + idx, sess->ticketNonce.data, sess->ticketNonce.len); + idx += sess->ticketNonce.len; +#endif +#endif +#ifdef WOLFSSL_EARLY_DATA + c32toa(sess->maxEarlyDataSz); + idx += OPAQUE32_LEN; +#endif +#endif +#ifdef HAVE_SESSION_TICKET + c16toa(sess->ticketLen, data + idx); idx += OPAQUE16_LEN; + XMEMCPY(data + idx, sess->ticket, sess->ticketLen); + idx += sess->ticketLen; #endif } #endif @@ -24986,6 +25024,63 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess, } XMEMCPY(s->serverID, data + idx, s->idLen); idx += s->idLen; #endif +#ifdef OPENSSL_EXTRA + /* byte for length of session context ID */ + if (i - idx < OPAQUE8_LEN) { + ret = BUFFER_ERROR; + goto end; + } + s->sessionCtxSz = data[idx++]; + + /* app session context ID */ + if (i - idx < s->sessionCtxSz) { + ret = BUFFER_ERROR; + goto end; + } + XMEMCPY(s->sessionCtx, data + idx, s->sessionCtxSz); idx += s->sessionCtxSz; +#endif +#ifdef WOLFSSL_TLS13 + if (i - idx < OPAQUE16_LEN) { + ret = BUFFER_ERROR; + goto end; + } + ato16(data + idx, &s->namedGroup); + idx += OPAQUE16_LEN; +#endif +#if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) +#ifdef WOLFSSL_TLS13 + if (i - idx < (OPAQUE32_LEN * 2)) { + ret = BUFFER_ERROR; + goto end; + } + ato32(data + idx, &s->ticketSeen); + idx += OPAQUE32_LEN; + ato32(data + idx, &s->ticketAdd); + idx += OPAQUE32_LEN; +#ifndef WOLFSSL_TLS13_DRAFT_18 + if (i - idx < OPAQUE8_LEN) { + ret = BUFFER_ERROR; + goto end; + } + s->ticketNonce.len = data[idx++]; + + if (i - idx < s->ticketNonce.len) { + ret = BUFFER_ERROR; + goto end; + } + XMEMCPY(s->ticketNonce.data, data + idx, s->ticketNonce.len); + idx += s->ticketNonce.len; +#endif +#endif +#ifdef WOLFSSL_EARLY_DATA + if (i - idx < OPAQUE32_LEN) { + ret = BUFFER_ERROR; + goto end; + } + ato32(data + idx, &s->maxEarlyDataSz); + idx += OPAQUE32_LEN; +#endif +#endif #ifdef HAVE_SESSION_TICKET /* ticket len */ if (i - idx < OPAQUE16_LEN) { @@ -25015,21 +25110,6 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess, goto end; } XMEMCPY(s->ticket, data + idx, s->ticketLen); idx += s->ticketLen; -#endif -#ifdef OPENSSL_EXTRA - /* byte for length of session context ID */ - if (i - idx < OPAQUE8_LEN) { - ret = BUFFER_ERROR; - goto end; - } - s->sessionCtxSz = data[idx++]; - - /* app session context ID */ - if (i - idx < s->sessionCtxSz) { - ret = BUFFER_ERROR; - goto end; - } - XMEMCPY(s->sessionCtx, data + idx, s->sessionCtxSz); idx += s->sessionCtxSz; #endif (void)idx; From b70f22e21a13fd49f2c2f5344c69a5dfdfbfab05 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 12 Sep 2019 16:02:36 -0700 Subject: [PATCH 5/5] 1. Use the session deallocator on the deserialized session in the client. 2. Free the flatten session if the size check fails. --- examples/client/client.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index ef0b16977..33f28f061 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -2997,8 +2997,10 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) int checkSz = wolfSSL_i2d_SSL_SESSION(session, &flatSession); if (flatSession == NULL) err_sys("error creating flattened session buffer"); - if (checkSz != flatSessionSz) + if (checkSz != flatSessionSz) { + XFREE(flatSession, NULL, DYNAMIC_TYPE_TMP_BUFFER); err_sys("flat session size check failure"); + } } } #endif @@ -3090,7 +3092,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) #if defined(OPENSSL_EXTRA) && defined(HAVE_EXT_CACHE) if (flatSession) { XFREE(flatSession, heap, DYNAMIC_TYPE_TMP_BUFFER); - XFREE(session, heap, DYNAMIC_TYPE_OPENSSL); + wolfSSL_SESSION_free(session); } #endif #ifdef HAVE_SESSION_TICKET