Skip to content

Commit 1fd952d

Browse files
jpbland1jpbland1
committed
fix bad ech transaction hash calculations
1 parent 8ff0874 commit 1fd952d

2 files changed

Lines changed: 95 additions & 79 deletions

File tree

‎src/tls.c‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14983,7 +14983,9 @@ static int TLSX_GetSizeWithEch(WOLFSSL* ssl, byte* semaphore, byte msgType,
1498314983
echX = TLSX_Find(ssl->ctx->extensions, TLSX_ECH);
1498414984

1498514985
/* if type is outer change sni to public name */
14986-
if (echX != NULL && ((WOLFSSL_ECH*)echX->data)->type == ECH_TYPE_OUTER) {
14986+
if (echX != NULL && ((WOLFSSL_ECH*)echX->data)->type == ECH_TYPE_OUTER &&
14987+
(ssl->options.echAccepted ||
14988+
((WOLFSSL_ECH*)echX->data)->innerCount == 0)) {
1498714989
if (ssl->extensions) {
1498814990
serverNameX = TLSX_Find(ssl->extensions, TLSX_SERVER_NAME);
1498914991

@@ -15190,7 +15192,9 @@ static int TLSX_WriteWithEch(WOLFSSL* ssl, byte* output, byte* semaphore,
1519015192
}
1519115193

1519215194
/* if type is outer change sni to public name */
15193-
if (echX != NULL && ((WOLFSSL_ECH*)echX->data)->type == ECH_TYPE_OUTER) {
15195+
if (echX != NULL && ((WOLFSSL_ECH*)echX->data)->type == ECH_TYPE_OUTER &&
15196+
(ssl->options.echAccepted ||
15197+
((WOLFSSL_ECH*)echX->data)->innerCount == 0)) {
1519415198
if (ssl->extensions) {
1519515199
serverNameX = TLSX_Find(ssl->extensions, TLSX_SERVER_NAME);
1519615200

@@ -15250,31 +15254,34 @@ static int TLSX_WriteWithEch(WOLFSSL* ssl, byte* output, byte* semaphore,
1525015254
msgType, pOffset);
1525115255
}
1525215256

15253-
if (echX != NULL) {
15254-
/* turn off and write it last */
15255-
TURN_OFF(semaphore, TLSX_ToSemaphore(echX->type));
15256-
}
15257+
/* only write if have a shot at acceptance */
15258+
if (ssl->options.echAccepted || ((WOLFSSL_ECH*)echX->data)->innerCount == 0) {
15259+
if (echX != NULL) {
15260+
/* turn off and write it last */
15261+
TURN_OFF(semaphore, TLSX_ToSemaphore(echX->type));
15262+
}
1525715263

15258-
if (ret == 0 && ssl->extensions) {
15259-
ret = TLSX_Write(ssl->extensions, output + *pOffset, semaphore,
15260-
msgType, pOffset);
15261-
}
15264+
if (ret == 0 && ssl->extensions) {
15265+
ret = TLSX_Write(ssl->extensions, output + *pOffset, semaphore,
15266+
msgType, pOffset);
15267+
}
1526215268

15263-
if (ret == 0 && ssl->ctx && ssl->ctx->extensions) {
15264-
ret = TLSX_Write(ssl->ctx->extensions, output + *pOffset, semaphore,
15265-
msgType, pOffset);
15266-
}
15269+
if (ret == 0 && ssl->ctx && ssl->ctx->extensions) {
15270+
ret = TLSX_Write(ssl->ctx->extensions, output + *pOffset, semaphore,
15271+
msgType, pOffset);
15272+
}
1526715273

15268-
if (serverNameX != NULL) {
15269-
/* remove the public name SNI */
15270-
TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap);
15274+
if (serverNameX != NULL) {
15275+
/* remove the public name SNI */
15276+
TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap);
1527115277

15272-
ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME, tmpServerName,
15273-
XSTRLEN(tmpServerName), ssl->heap);
15278+
ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME, tmpServerName,
15279+
XSTRLEN(tmpServerName), ssl->heap);
1527415280

15275-
/* restore the inner server name */
15276-
if (ret == WOLFSSL_SUCCESS)
15277-
ret = 0;
15281+
/* restore the inner server name */
15282+
if (ret == WOLFSSL_SUCCESS)
15283+
ret = 0;
15284+
}
1527815285
}
1527915286

1528015287
#ifdef WOLFSSL_SMALL_STACK

‎src/tls13.c‎

Lines changed: 66 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4182,10 +4182,10 @@ static int EchHashHelloInner(WOLFSSL* ssl, WOLFSSL_ECH* ech)
41824182
return BAD_FUNC_ARG;
41834183
realSz = ech->innerClientHelloLen - ech->paddingLen - ech->hpke->Nt;
41844184
tmpHashes = ssl->hsHashes;
4185+
ssl->hsHashes = NULL;
41854186
/* init the ech hashes */
4186-
InitHandshakeHashesAndCopy(ssl, ssl->hsHashes, &ssl->hsHashesEch);
4187-
/* swap hsHashes so the regular hash functions work */
4188-
ssl->hsHashes = ssl->hsHashesEch;
4187+
InitHandshakeHashes(ssl);
4188+
ssl->hsHashesEch = ssl->hsHashes;
41894189
if (ret == 0) {
41904190
/* do the handshake header then the body */
41914191
AddTls13HandShakeHeader(falseHeader, realSz, 0, 0, client_hello, ssl);
@@ -4200,7 +4200,7 @@ static int EchHashHelloInner(WOLFSSL* ssl, WOLFSSL_ECH* ech)
42004200
ech->innerCount = 1;
42014201
}
42024202
else {
4203-
/* switch back to primary so we can copy it to inner */
4203+
/* switch back to hsHashes so we have hrr -> echInner2 */
42044204
ssl->hsHashes = tmpHashes;
42054205
InitHandshakeHashesAndCopy(ssl, ssl->hsHashes,
42064206
&ssl->hsHashesEchInner);
@@ -4464,23 +4464,26 @@ int SendTls13ClientHello(WOLFSSL* ssl)
44644464
if (args->ech == NULL)
44654465
return WOLFSSL_FATAL_ERROR;
44664466

4467-
/* set the type to inner */
4468-
args->ech->type = ECH_TYPE_INNER;
4469-
args->preXLength = (int)args->length;
4467+
/* only prepare if we have a chance at acceptance */
4468+
if (ssl->options.echAccepted || args->ech->innerCount == 0) {
4469+
/* set the type to inner */
4470+
args->ech->type = ECH_TYPE_INNER;
4471+
args->preXLength = (int)args->length;
44704472

4471-
/* get size for inner */
4472-
ret = TLSX_GetRequestSize(ssl, client_hello, &args->length);
4473-
if (ret != 0)
4474-
return ret;
4473+
/* get size for inner */
4474+
ret = TLSX_GetRequestSize(ssl, client_hello, &args->length);
4475+
if (ret != 0)
4476+
return ret;
44754477

4476-
/* set the type to outer */
4477-
args->ech->type = 0;
4478-
/* set innerClientHelloLen to ClientHelloInner + padding + tag */
4479-
args->ech->paddingLen = 31 - ((args->length - 1) % 32);
4480-
args->ech->innerClientHelloLen = (word16)(args->length +
4481-
args->ech->paddingLen + args->ech->hpke->Nt);
4482-
/* set the length back to before we computed ClientHelloInner size */
4483-
args->length = (word32)args->preXLength;
4478+
/* set the type to outer */
4479+
args->ech->type = 0;
4480+
/* set innerClientHelloLen to ClientHelloInner + padding + tag */
4481+
args->ech->paddingLen = 31 - ((args->length - 1) % 32);
4482+
args->ech->innerClientHelloLen = (word16)(args->length +
4483+
args->ech->paddingLen + args->ech->hpke->Nt);
4484+
/* set the length back to before we computed ClientHelloInner size */
4485+
args->length = (word32)args->preXLength;
4486+
}
44844487
}
44854488
#endif
44864489

@@ -4606,7 +4609,8 @@ int SendTls13ClientHello(WOLFSSL* ssl)
46064609

46074610
#if defined(HAVE_ECH)
46084611
/* write inner then outer */
4609-
if (ssl->options.useEch == 1 && !ssl->options.disableECH) {
4612+
if (ssl->options.useEch == 1 && !ssl->options.disableECH &&
4613+
(ssl->options.echAccepted || args->ech->innerCount == 0)) {
46104614
/* set the type to inner */
46114615
args->ech->type = ECH_TYPE_INNER;
46124616
/* innerClientHello may already exist from hrr, free if it does */
@@ -4663,7 +4667,8 @@ int SendTls13ClientHello(WOLFSSL* ssl)
46634667

46644668
#if defined(HAVE_ECH)
46654669
/* encrypt and pack the ech innerClientHello */
4666-
if (ssl->options.useEch == 1 && !ssl->options.disableECH) {
4670+
if (ssl->options.useEch == 1 && !ssl->options.disableECH &&
4671+
(ssl->options.echAccepted || args->ech->innerCount == 0)) {
46674672
ret = TLSX_FinalizeEch(args->ech,
46684673
args->output + RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ,
46694674
(word32)(args->sendSz - (RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ)));
@@ -4693,7 +4698,8 @@ int SendTls13ClientHello(WOLFSSL* ssl)
46934698
{
46944699
#if defined(HAVE_ECH)
46954700
/* compute the inner hash */
4696-
if (ssl->options.useEch == 1 && !ssl->options.disableECH)
4701+
if (ssl->options.useEch == 1 && !ssl->options.disableECH &&
4702+
(ssl->options.echAccepted || args->ech->innerCount == 0))
46974703
ret = EchHashHelloInner(ssl, args->ech);
46984704
#endif
46994705
/* compute the outer hash */
@@ -4789,13 +4795,12 @@ static int Dtls13ClientDoDowngrade(WOLFSSL* ssl)
47894795
/* check if the server accepted ech or not, must be run after an hsHashes
47904796
* restart */
47914797
static int EchCheckAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
4792-
const byte* input, int acceptOffset, int helloSz, byte msgType)
4798+
const byte* input, int acceptOffset, int helloSz)
47934799
{
47944800
int ret = 0;
47954801
int digestType = 0;
47964802
int digestSize = 0;
47974803
HS_Hashes* tmpHashes;
4798-
HS_Hashes* acceptHashes = NULL;
47994804
byte zeros[WC_MAX_DIGEST_SIZE];
48004805
byte transcriptEchConf[WC_MAX_DIGEST_SIZE];
48014806
byte expandLabelPrk[WC_MAX_DIGEST_SIZE];
@@ -4806,14 +4811,10 @@ static int EchCheckAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
48064811
XMEMSET(acceptConfirmation, 0, sizeof(acceptConfirmation));
48074812
/* store so we can restore regardless of the outcome */
48084813
tmpHashes = ssl->hsHashes;
4809-
/* copy ech hashes to accept */
4810-
ret = InitHandshakeHashesAndCopy(ssl, ssl->hsHashesEch, &acceptHashes);
4811-
if (ret == 0) {
4812-
/* swap hsHashes to acceptHashes */
4813-
ssl->hsHashes = acceptHashes;
4814-
/* hash up to the last 8 bytes */
4815-
ret = HashRaw(ssl, input, acceptOffset);
4816-
}
4814+
/* swap hsHashes to hsHashesEch */
4815+
ssl->hsHashes = ssl->hsHashesEch;
4816+
/* hash up to the last 8 bytes */
4817+
ret = HashRaw(ssl, input, acceptOffset);
48174818
/* hash 8 zeros */
48184819
if (ret == 0)
48194820
ret = HashRaw(ssl, zeros, ECH_ACCEPT_CONFIRMATION_SZ);
@@ -4903,17 +4904,17 @@ static int EchCheckAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
49034904
else {
49044905
/* set echAccepted to 0, needed in case HRR */
49054906
ssl->options.echAccepted = 0;
4907+
/* free inner since we're continuing with outer */
4908+
ssl->hsHashes = ssl->hsHashesEchInner;
4909+
FreeHandshakeHashes(ssl);
4910+
ssl->hsHashesEchInner = NULL;
49064911
}
49074912
/* continue with outer if we failed to verify ech was accepted */
49084913
ret = 0;
49094914
}
4910-
/* free hsHashesEch */
4911-
if (ssl->options.echAccepted == 0 || msgType != hello_retry_request) {
4912-
/* free hsHashesEch */
4913-
FreeHandshakeHashes(ssl);
4914-
/* set hsHashesEch to NULL to avoid double free */
4915-
ssl->hsHashesEch = NULL;
4916-
}
4915+
FreeHandshakeHashes(ssl);
4916+
/* set hsHashesEch to NULL to avoid double free */
4917+
ssl->hsHashesEch = NULL;
49174918
/* swap to tmp, will be inner if accepted, hsHashes if rejected */
49184919
ssl->hsHashes = tmpHashes;
49194920
return ret;
@@ -4928,7 +4929,6 @@ static int EchWriteAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
49284929
int digestType = 0;
49294930
int digestSize = 0;
49304931
HS_Hashes* tmpHashes = NULL;
4931-
HS_Hashes* acceptHashes = NULL;
49324932
byte zeros[WC_MAX_DIGEST_SIZE];
49334933
byte transcriptEchConf[WC_MAX_DIGEST_SIZE];
49344934
byte expandLabelPrk[WC_MAX_DIGEST_SIZE];
@@ -4937,14 +4937,9 @@ static int EchWriteAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
49374937
XMEMSET(expandLabelPrk, 0, sizeof(expandLabelPrk));
49384938
/* store so we can restore regardless of the outcome */
49394939
tmpHashes = ssl->hsHashes;
4940-
/* copy ech hashes to accept */
4941-
ret = InitHandshakeHashesAndCopy(ssl, ssl->hsHashesEch, &acceptHashes);
4942-
if (ret == 0) {
4943-
/* swap hsHashes to acceptHashes */
4944-
ssl->hsHashes = acceptHashes;
4945-
/* hash up to the acceptOffset */
4946-
ret = HashRaw(ssl, output, acceptOffset);
4947-
}
4940+
ssl->hsHashes = ssl->hsHashesEch;
4941+
/* hash up to the acceptOffset */
4942+
ret = HashRaw(ssl, output, acceptOffset);
49484943
/* hash 8 zeros */
49494944
if (ret == 0)
49504945
ret = HashRaw(ssl, zeros, ECH_ACCEPT_CONFIRMATION_SZ);
@@ -5015,12 +5010,12 @@ static int EchWriteAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
50155010
PRIVATE_KEY_LOCK();
50165011
}
50175012
if (ret == 0) {
5018-
/* free hsHashesEch if this is the last ech involved message */
5019-
if (msgType != hello_retry_request) {
5020-
FreeHandshakeHashes(ssl);
5021-
ssl->hsHashesEch = NULL;
5013+
/* free hsHashesEch, if this is an HRR we will start at client hello 2*/
5014+
FreeHandshakeHashes(ssl);
5015+
ssl->hsHashesEch = NULL;
5016+
/* mark that ech was accepted */
5017+
if (msgType != hello_retry_request)
50225018
ssl->options.echAccepted = 1;
5023-
}
50245019
}
50255020
ssl->hsHashes = tmpHashes;
50265021
return ret;
@@ -5560,8 +5555,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
55605555
/* check acceptance */
55615556
if (ret == 0) {
55625557
ret = EchCheckAcceptance(ssl, args->acceptLabel,
5563-
args->acceptLabelSz, input, args->acceptOffset, helloSz,
5564-
args->extMsgType);
5558+
args->acceptLabelSz, input, args->acceptOffset, helloSz);
55655559
}
55665560
if (ret != 0)
55675561
return ret;
@@ -6741,6 +6735,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
67416735
#endif
67426736
#if defined(HAVE_ECH)
67436737
TLSX* echX = NULL;
6738+
HS_Hashes* tmpHashes;
67446739
#endif
67456740

67466741
WOLFSSL_START(WC_FUNC_CLIENT_HELLO_DO);
@@ -7064,6 +7059,22 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
70647059
}
70657060
#endif
70667061

7062+
#if defined(HAVE_ECH)
7063+
/* hash clientHelloInner to hsHashesEch independently since it can't include
7064+
* the HRR */
7065+
if (!ssl->options.disableECH) {
7066+
tmpHashes = ssl->hsHashes;
7067+
ssl->hsHashes = NULL;
7068+
ret = InitHandshakeHashes(ssl);
7069+
if (ret != 0)
7070+
goto exit_dch;
7071+
if ((ret = HashInput(ssl, input + args->begin, (int)helloSz)) != 0)
7072+
goto exit_dch;
7073+
ssl->hsHashesEch = ssl->hsHashes;
7074+
ssl->hsHashes = tmpHashes;
7075+
}
7076+
#endif
7077+
70677078
#if (defined(HAVE_SESSION_TICKET) || !defined(NO_PSK)) && \
70687079
defined(HAVE_TLS_EXTENSIONS)
70697080
ret = CheckPreSharedKeys(ssl, input + args->begin, helloSz, ssl->clSuites,
@@ -7481,8 +7492,6 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType)
74817492
}
74827493
/* replace the last 8 bytes of server random with the accept */
74837494
if (((WOLFSSL_ECH*)echX->data)->state == ECH_PARSED_INTERNAL) {
7484-
ret = InitHandshakeHashesAndCopy(ssl, ssl->hsHashes,
7485-
&ssl->hsHashesEch);
74867495
if (ret == 0) {
74877496
ret = EchWriteAcceptance(ssl, acceptLabel,
74887497
acceptLabelSz, output + RECORD_HEADER_SZ,

0 commit comments

Comments
 (0)