From bc77f9f46667be8890af806b02fe7e00c6a2107b Mon Sep 17 00:00:00 2001 From: John Bland Date: Tue, 26 Sep 2023 13:24:27 -0400 Subject: [PATCH] fix writing empty string when sending enc in response to an hrr, fix bad getSize for hrr ech, fix using the wrong transcript hash for hrr ech, add new hrr test for ech to api.c --- src/tls.c | 19 +++++++++++++++---- src/tls13.c | 19 +++++++++++++------ tests/api.c | 18 +++++++++++++++++- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/tls.c b/src/tls.c index 586713386..2d4f1b93a 100644 --- a/src/tls.c +++ b/src/tls.c @@ -11551,8 +11551,8 @@ static int TLSX_ECH_Write(WOLFSSL_ECH* ech, byte msgType, byte* writeBuf, #endif } else { - /* set to emptry string on hrr */ - if (msgType == hello_retry_request) { + /* write empty string for enc if this isn't our first ech */ + if (ech->hpkeContext != NULL) { XMEMSET(writeBuf_p, 0, ech->encLen); } else { @@ -11577,7 +11577,7 @@ static int TLSX_ECH_Write(WOLFSSL_ECH* ech, byte msgType, byte* writeBuf, } /* return the size needed for the ech extension */ -static int TLSX_ECH_GetSize(WOLFSSL_ECH* ech) +static int TLSX_ECH_GetSize(WOLFSSL_ECH* ech, byte msgType) { int ret; word32 size; @@ -11589,6 +11589,9 @@ static int TLSX_ECH_GetSize(WOLFSSL_ECH* ech) size += GREASE_ECH_SIZE + (size % 32); } + else if (msgType == hello_retry_request) { + size = ECH_ACCEPT_CONFIRMATION_SZ; + } else if (ech->state == ECH_WRITE_NONE || ech->state == ECH_PARSED_INTERNAL) { size = 0; @@ -12308,7 +12311,7 @@ static int TLSX_GetSize(TLSX* list, byte* semaphore, byte msgType, #endif /* WOLFSSL_DTLS_CID */ #if defined(WOLFSSL_TLS13) && defined(HAVE_ECH) case TLSX_ECH: - length += ECH_GET_SIZE((WOLFSSL_ECH*)extension->data); + length += ECH_GET_SIZE((WOLFSSL_ECH*)extension->data, msgType); break; #endif default: @@ -13767,6 +13770,10 @@ int TLSX_GetResponseSize(WOLFSSL* ssl, byte msgType, word16* pLength) #ifdef WOLFSSL_SEND_HRR_COOKIE TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_COOKIE)); #endif +#ifdef HAVE_ECH + /* send the special confirmation */ + TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_ECH)); +#endif break; #endif @@ -13908,6 +13915,10 @@ int TLSX_WriteResponse(WOLFSSL *ssl, byte* output, byte msgType, word16* pOffset TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_KEY_SHARE)); } #endif +#ifdef HAVE_ECH + /* send the special confirmation */ + TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_ECH)); +#endif /* Cookie is written below as last extension. */ break; #endif diff --git a/src/tls13.c b/src/tls13.c index 866b58d3f..fee31b88b 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -4740,12 +4740,13 @@ static int EchCheckAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz, XMEMSET(acceptConfirmation, 0, sizeof(acceptConfirmation)); /* copy ech hashes to accept */ ret = InitHandshakeHashesAndCopy(ssl, ssl->hsHashesEch, &acceptHashes); - /* swap hsHashes to acceptHashes */ - tmpHashes = ssl->hsHashes; - ssl->hsHashes = acceptHashes; - /* hash up to the last 8 bytes */ - if (ret == 0) + if (ret == 0) { + /* swap hsHashes to acceptHashes */ + tmpHashes = ssl->hsHashes; + ssl->hsHashes = acceptHashes; + /* hash up to the last 8 bytes */ ret = HashRaw(ssl, input, acceptOffset); + } /* hash 8 zeros */ if (ret == 0) ret = HashRaw(ssl, zeros, ECH_ACCEPT_CONFIRMATION_SZ); @@ -4866,7 +4867,7 @@ static int EchWriteAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz, XMEMSET(transcriptEchConf, 0, sizeof(transcriptEchConf)); XMEMSET(expandLabelPrk, 0, sizeof(expandLabelPrk)); /* copy ech hashes to accept */ - ret = InitHandshakeHashesAndCopy(ssl, ssl->hsHashes, &acceptHashes); + ret = InitHandshakeHashesAndCopy(ssl, ssl->hsHashesEch, &acceptHashes); if (ret == 0) { /* swap hsHashes to acceptHashes */ tmpHashes = ssl->hsHashes; @@ -7253,6 +7254,12 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) WOLFSSL_START(WC_FUNC_SERVER_HELLO_SEND); WOLFSSL_ENTER("SendTls13ServerHello"); +#ifdef HAVE_ECH + /* copy the hsHashes to hsHashesEch since they will get blown away by hrr */ + if (ssl->hsHashesEch == NULL) + InitHandshakeHashesAndCopy(ssl, ssl->hsHashes, &ssl->hsHashesEch); +#endif + /* When ssl->options.dtlsStateful is not set then cookie is calculated in * dtls.c */ if (extMsgType == hello_retry_request diff --git a/tests/api.c b/tests/api.c index 4ded25ef3..a12009b91 100644 --- a/tests/api.c +++ b/tests/api.c @@ -36759,7 +36759,7 @@ static int test_wolfSSL_Tls13_ECH_params(void) return EXPECT_RESULT(); } -static int test_wolfSSL_Tls13_ECH(void) +static int test_wolfSSL_Tls13_ECH_ex(int hrr) { EXPECT_DECLS; tcp_ready ready; @@ -36830,6 +36830,11 @@ static int test_wolfSSL_Tls13_ECH(void) ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_UseSNI(ssl, WOLFSSL_SNI_HOST_NAME, privateName, privateNameLen)); + /* force hello retry request */ + if (hrr) + ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_NoKeyShares(ssl)); + + /* connect like normal */ ExpectIntEQ(wolfSSL_set_fd(ssl, sockfd), WOLFSSL_SUCCESS); ExpectIntEQ(wolfSSL_connect(ssl), WOLFSSL_SUCCESS); ExpectIntEQ(wolfSSL_write(ssl, privateName, privateNameLen), @@ -36850,6 +36855,16 @@ static int test_wolfSSL_Tls13_ECH(void) return EXPECT_RESULT(); } + +static int test_wolfSSL_Tls13_ECH(void) +{ + return test_wolfSSL_Tls13_ECH_ex(0); +} + +static int test_wolfSSL_Tls13_ECH_HRR(void) +{ + return test_wolfSSL_Tls13_ECH_ex(1); +} #endif /* HAVE_ECH && WOLFSSL_TLS13 */ #if defined(HAVE_IO_TESTS_DEPENDENCIES) && \ @@ -70072,6 +70087,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_Tls13_ECH_params), /* Uses Assert in handshake callback. */ TEST_DECL(test_wolfSSL_Tls13_ECH), + TEST_DECL(test_wolfSSL_Tls13_ECH_HRR), #endif TEST_DECL(test_wolfSSL_X509_TLS_version_test_1),