Skip to content

Commit 56cd641

Browse files
sebastian-carpentersebastian-carpenter
committed
testing + bug fixes for TLS ECH
1 parent 1a5cfa0 commit 56cd641

File tree

7 files changed

+790
-108
lines changed

7 files changed

+790
-108
lines changed

‎src/internal.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8634,10 +8634,9 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)
86348634
ForceZero(&ssl->serverSecret, sizeof(ssl->serverSecret));
86358635

86368636
#if defined(HAVE_ECH)
8637-
if (ssl->options.useEch == 1) {
8637+
if (ssl->echConfigs != NULL) {
86388638
FreeEchConfigs(ssl->echConfigs, ssl->heap);
86398639
ssl->echConfigs = NULL;
8640-
ssl->options.useEch = 0;
86418640
}
86428641
#endif /* HAVE_ECH */
86438642
#endif /* WOLFSSL_TLS13 */

‎src/ssl_ech.c‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ int wolfSSL_CTX_GenerateEchConfig(WOLFSSL_CTX* ctx, const char* publicName,
3636
int ret = 0;
3737
word16 encLen = DHKEM_X25519_ENC_LEN;
3838
WOLFSSL_EchConfig* newConfig;
39-
WOLFSSL_EchConfig* parentConfig;
4039
#ifdef WOLFSSL_SMALL_STACK
4140
Hpke* hpke = NULL;
4241
WC_RNG* rng;
@@ -63,7 +62,9 @@ int wolfSSL_CTX_GenerateEchConfig(WOLFSSL_CTX* ctx, const char* publicName,
6362
else
6463
XMEMSET(newConfig, 0, sizeof(WOLFSSL_EchConfig));
6564

66-
/* set random config id */
65+
/* set random configId */
66+
/* TODO: if an equal configId is found should the old config be removed from
67+
* the LL? Prevents growth beyond 255+ items */
6768
if (ret == 0)
6869
ret = wc_RNG_GenerateByte(rng, &newConfig->configId);
6970

@@ -139,17 +140,14 @@ int wolfSSL_CTX_GenerateEchConfig(WOLFSSL_CTX* ctx, const char* publicName,
139140
}
140141
}
141142
else {
142-
parentConfig = ctx->echConfigs;
143-
144-
if (parentConfig == NULL) {
143+
/* insert new configs at beginning of LL as preference should be given
144+
* to the most recently generated configs */
145+
if (ctx->echConfigs == NULL) {
145146
ctx->echConfigs = newConfig;
146147
}
147148
else {
148-
while (parentConfig->next != NULL) {
149-
parentConfig = parentConfig->next;
150-
}
151-
152-
parentConfig->next = newConfig;
149+
newConfig->next = ctx->echConfigs;
150+
ctx->echConfigs = newConfig;
153151
}
154152
}
155153

@@ -242,7 +240,7 @@ void wolfSSL_CTX_SetEchEnable(WOLFSSL_CTX* ctx, byte enable)
242240

243241
/* set the ech config from base64 for our client ssl object, base64 is the
244242
* format ech configs are sent using dns records */
245-
int wolfSSL_SetEchConfigsBase64(WOLFSSL* ssl, char* echConfigs64,
243+
int wolfSSL_SetEchConfigsBase64(WOLFSSL* ssl, const char* echConfigs64,
246244
word32 echConfigs64Len)
247245
{
248246
int ret = 0;
@@ -253,7 +251,7 @@ int wolfSSL_SetEchConfigsBase64(WOLFSSL* ssl, char* echConfigs64,
253251
return BAD_FUNC_ARG;
254252

255253
/* already have ech configs */
256-
if (ssl->options.useEch == 1) {
254+
if (ssl->echConfigs != NULL) {
257255
return WOLFSSL_FATAL_ERROR;
258256
}
259257

@@ -266,7 +264,7 @@ int wolfSSL_SetEchConfigsBase64(WOLFSSL* ssl, char* echConfigs64,
266264
decodedConfigs[decodedLen - 1] = 0;
267265

268266
/* decode the echConfigs */
269-
ret = Base64_Decode((byte*)echConfigs64, echConfigs64Len,
267+
ret = Base64_Decode((const byte*)echConfigs64, echConfigs64Len,
270268
decodedConfigs, &decodedLen);
271269

272270
if (ret != 0) {
@@ -292,7 +290,7 @@ int wolfSSL_SetEchConfigs(WOLFSSL* ssl, const byte* echConfigs,
292290
return BAD_FUNC_ARG;
293291

294292
/* already have ech configs */
295-
if (ssl->options.useEch == 1) {
293+
if (ssl->echConfigs != NULL) {
296294
return WOLFSSL_FATAL_ERROR;
297295
}
298296

@@ -301,7 +299,6 @@ int wolfSSL_SetEchConfigs(WOLFSSL* ssl, const byte* echConfigs,
301299

302300
/* if we found valid configs */
303301
if (ret == 0) {
304-
ssl->options.useEch = 1;
305302
return WOLFSSL_SUCCESS;
306303
}
307304

@@ -459,7 +456,7 @@ int wolfSSL_GetEchConfigs(WOLFSSL* ssl, byte* output, word32* outputLen)
459456
return BAD_FUNC_ARG;
460457

461458
/* if we don't have ech configs */
462-
if (ssl->options.useEch != 1) {
459+
if (ssl->echConfigs == NULL) {
463460
return WOLFSSL_FATAL_ERROR;
464461
}
465462

‎src/tls.c‎

Lines changed: 71 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2236,6 +2236,7 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
22362236
WOLFSSL_ECH* ech = NULL;
22372237
WOLFSSL_EchConfig* workingConfig;
22382238
TLSX* echX;
2239+
word16 privateNameLen;
22392240
#endif
22402241
#endif /* !NO_WOLFSSL_SERVER */
22412242
TLSX *extension = TLSX_Find(ssl->extensions, TLSX_SERVER_NAME);
@@ -2315,24 +2316,56 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
23152316
if (!cacheOnly && !(sni = TLSX_SNI_Find((SNI*)extension->data, type)))
23162317
return 0; /* not using this type of SNI. */
23172318

2318-
#ifdef WOLFSSL_TLS13
2319+
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
2320+
echX = TLSX_Find(ssl->extensions, TLSX_ECH);
2321+
if (echX != NULL)
2322+
ech = (WOLFSSL_ECH*)(echX->data);
2323+
2324+
/* SNI status is carried over from processing the outer hello so it is
2325+
* necessary to clear it before processing the inner hello */
2326+
if (ech != NULL && ech->processingInner == 1) {
2327+
ech->processingInner = 2;
2328+
sni->status = 0;
2329+
2330+
if (ssl->ctx->sniRecvCb) {
2331+
cacheOnly = 1;
2332+
}
2333+
2334+
if (cacheOnly) {
2335+
WOLFSSL_MSG("Forcing SSL object to store SNI parameter");
2336+
}
2337+
}
2338+
#endif
2339+
2340+
#if defined(WOLFSSL_TLS13)
23192341
/* Don't process the second ClientHello SNI extension if there
23202342
* was problems with the first.
23212343
*/
23222344
if (!cacheOnly && sni->status != 0)
23232345
return 0;
23242346
#endif
2325-
matched = cacheOnly || (XSTRLEN(sni->data.host_name) == size &&
2326-
XSTRNCMP(sni->data.host_name, (const char*)input + offset, size) == 0);
23272347

2328-
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
2329-
echX = TLSX_Find(ssl->extensions, TLSX_ECH);
2330-
if (echX != NULL)
2331-
ech = (WOLFSSL_ECH*)(echX->data);
2348+
#if defined(HAVE_ECH)
2349+
if (ech != NULL && ech->processingInner == 2) {
2350+
if (ech->privateName != NULL) {
2351+
matched = cacheOnly || (XSTRLEN(ech->privateName) == size &&
2352+
XSTRNCMP(ech->privateName, (const char*)input + offset,
2353+
size) == 0);
2354+
}
2355+
else {
2356+
matched = 0;
2357+
}
2358+
}
2359+
else
2360+
#endif
2361+
{
2362+
matched = cacheOnly || (XSTRLEN(sni->data.host_name) == size &&
2363+
XSTRNCMP(sni->data.host_name, (const char*)input + offset, size) == 0);
2364+
}
23322365

2333-
if (!matched && ech != NULL) {
2366+
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
2367+
if (!matched && ech != NULL && ech->processingInner == 0) {
23342368
workingConfig = ech->echConfig;
2335-
23362369
while (workingConfig != NULL) {
23372370
matched = XSTRLEN(workingConfig->publicName) == size &&
23382371
XSTRNCMP(workingConfig->publicName,
@@ -2348,8 +2381,25 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
23482381

23492382
if (matched || sni->options & WOLFSSL_SNI_ANSWER_ON_MISMATCH) {
23502383
int matchStat;
2351-
int r = TLSX_UseSNI(&ssl->extensions, type, input + offset, size,
2384+
int r;
2385+
2386+
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
2387+
/* save the private SNI before it is overwritten by the public SNI */
2388+
if (ech != NULL && ech->processingInner == 0 && sni != NULL &&
2389+
ech->privateName == NULL) {
2390+
privateNameLen = (word16)XSTRLEN(sni->data.host_name) + 1;
2391+
ech->privateName = (char*)XMALLOC(privateNameLen, ssl->heap,
2392+
DYNAMIC_TYPE_TMP_BUFFER);
2393+
if (ech->privateName == NULL)
2394+
return MEMORY_E;
2395+
XMEMCPY((char*)ech->privateName, sni->data.host_name,
2396+
privateNameLen);
2397+
}
2398+
#endif
2399+
2400+
r = TLSX_UseSNI(&ssl->extensions, type, input + offset, size,
23522401
ssl->heap);
2402+
23532403
if (r != WOLFSSL_SUCCESS)
23542404
return r; /* throws error. */
23552405

@@ -13886,12 +13936,12 @@ static int TLSX_ECH_Parse(WOLFSSL* ssl, const byte* readBuf, word16 size,
1388613936
byte* aadCopy;
1388713937
byte* readBuf_p = (byte*)readBuf;
1388813938
WOLFSSL_MSG("TLSX_ECH_Parse");
13889-
if (size == 0)
13890-
return BAD_FUNC_ARG;
1389113939
if (ssl->options.disableECH) {
1389213940
WOLFSSL_MSG("TLSX_ECH_Parse: ECH disabled. Ignoring.");
1389313941
return 0;
1389413942
}
13943+
if (size == 0)
13944+
return BAD_FUNC_ARG;
1389513945
/* retry configs */
1389613946
if (msgType == encrypted_extensions) {
1389713947
ret = wolfSSL_SetEchConfigs(ssl, readBuf, size);
@@ -13900,7 +13950,8 @@ static int TLSX_ECH_Parse(WOLFSSL* ssl, const byte* readBuf, word16 size,
1390013950
ret = 0;
1390113951
}
1390213952
/* HRR with special confirmation */
13903-
else if (msgType == hello_retry_request && ssl->options.useEch) {
13953+
else if (msgType == hello_retry_request && ssl->echConfigs != NULL &&
13954+
!ssl->options.disableECH) {
1390413955
/* length must be 8 */
1390513956
if (size != ECH_ACCEPT_CONFIRMATION_SZ)
1390613957
return BAD_FUNC_ARG;
@@ -14000,6 +14051,7 @@ static int TLSX_ECH_Parse(WOLFSSL* ssl, const byte* readBuf, word16 size,
1400014051
if (ret == 0) {
1400114052
i = 0;
1400214053
/* decrement until before the padding */
14054+
/* TODO: verify padding is 0, abort with illegal_parameter */
1400314055
while (ech->innerClientHello[ech->innerClientHelloLen +
1400414056
HANDSHAKE_HEADER_SZ - i - 1] != ECH_TYPE_INNER) {
1400514057
i++;
@@ -14035,6 +14087,8 @@ static void TLSX_ECH_Free(WOLFSSL_ECH* ech, void* heap)
1403514087
XFREE(ech->hpke, heap, DYNAMIC_TYPE_TMP_BUFFER);
1403614088
if (ech->hpkeContext != NULL)
1403714089
XFREE(ech->hpkeContext, heap, DYNAMIC_TYPE_TMP_BUFFER);
14090+
if (ech->privateName != NULL)
14091+
XFREE((char*)ech->privateName, heap, DYNAMIC_TYPE_TMP_BUFFER);
1403814092

1403914093
XFREE(ech, heap, DYNAMIC_TYPE_TMP_BUFFER);
1404014094
(void)heap;
@@ -15814,7 +15868,7 @@ int TLSX_GetRequestSize(WOLFSSL* ssl, byte msgType, word32* pLength)
1581415868
}
1581515869
#endif
1581615870
#if defined(HAVE_ECH)
15817-
if (ssl->options.useEch == 1 && !ssl->options.disableECH
15871+
if (ssl->echConfigs != NULL && !ssl->options.disableECH
1581815872
&& msgType == client_hello) {
1581915873
ret = TLSX_GetSizeWithEch(ssl, semaphore, msgType, &length);
1582015874
if (ret != 0)
@@ -16000,7 +16054,7 @@ int TLSX_WriteRequest(WOLFSSL* ssl, byte* output, byte msgType, word32* pOffset)
1600016054
#endif
1600116055
#endif
1600216056
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
16003-
if (ssl->options.useEch == 1 && !ssl->options.disableECH
16057+
if (ssl->echConfigs != NULL && !ssl->options.disableECH
1600416058
&& msgType == client_hello) {
1600516059
ret = TLSX_WriteWithEch(ssl, output, semaphore,
1600616060
msgType, &offset);
@@ -17280,7 +17334,8 @@ int TLSX_Parse(WOLFSSL* ssl, const byte* input, word16 length, byte msgType,
1728017334

1728117335
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
1728217336
/* If client used ECH, server HRR must include ECH confirmation */
17283-
if (ret == 0 && msgType == hello_retry_request && ssl->options.useEch == 1) {
17337+
if (ret == 0 && msgType == hello_retry_request && ssl->echConfigs != NULL &&
17338+
!ssl->options.disableECH) {
1728417339
TLSX* echX = TLSX_Find(ssl->extensions, TLSX_ECH);
1728517340
if (echX == NULL || ((WOLFSSL_ECH*)echX->data)->confBuf == NULL) {
1728617341
WOLFSSL_MSG("ECH used but HRR missing ECH confirmation");

‎src/tls13.c‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4681,7 +4681,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
46814681

46824682
/* find length of outer and inner */
46834683
#if defined(HAVE_ECH)
4684-
if (ssl->options.useEch == 1 && !ssl->options.disableECH) {
4684+
if (ssl->echConfigs != NULL && !ssl->options.disableECH) {
46854685
TLSX* echX = TLSX_Find(ssl->extensions, TLSX_ECH);
46864686
if (echX == NULL)
46874687
return WOLFSSL_FATAL_ERROR;
@@ -4835,7 +4835,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
48354835

48364836
#if defined(HAVE_ECH)
48374837
/* write inner then outer */
4838-
if (ssl->options.useEch == 1 && !ssl->options.disableECH &&
4838+
if (ssl->echConfigs != NULL && !ssl->options.disableECH &&
48394839
(ssl->options.echAccepted || args->ech->innerCount == 0)) {
48404840
/* set the type to inner */
48414841
args->ech->type = ECH_TYPE_INNER;
@@ -4900,7 +4900,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
49004900

49014901
#if defined(HAVE_ECH)
49024902
/* encrypt and pack the ech innerClientHello */
4903-
if (ssl->options.useEch == 1 && !ssl->options.disableECH &&
4903+
if (ssl->echConfigs != NULL && !ssl->options.disableECH &&
49044904
(ssl->options.echAccepted || args->ech->innerCount == 0)) {
49054905
ret = TLSX_FinalizeEch(args->ech,
49064906
args->output + RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ,
@@ -4931,7 +4931,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
49314931
{
49324932
#if defined(HAVE_ECH)
49334933
/* compute the inner hash */
4934-
if (ssl->options.useEch == 1 && !ssl->options.disableECH &&
4934+
if (ssl->echConfigs != NULL && !ssl->options.disableECH &&
49354935
(ssl->options.echAccepted || args->ech->innerCount == 0)) {
49364936
ret = EchHashHelloInner(ssl, args->ech);
49374937
}
@@ -5633,7 +5633,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
56335633

56345634
#if defined(HAVE_ECH)
56355635
/* check for acceptConfirmation, must be done after hashes restart */
5636-
if (ssl->options.useEch == 1) {
5636+
if (ssl->echConfigs != NULL && !ssl->options.disableECH) {
56375637
args->echX = TLSX_Find(ssl->extensions, TLSX_ECH);
56385638
/* account for hrr extension instead of server random */
56395639
if (args->extMsgType == hello_retry_request) {
@@ -7129,9 +7129,19 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
71297129
}
71307130

71317131
#if defined(HAVE_ECH)
7132-
/* jump to the end to clean things up */
7133-
if (echX != NULL && ((WOLFSSL_ECH*)echX->data)->state == ECH_WRITE_NONE)
7134-
goto exit_dch;
7132+
if (echX != NULL && ((WOLFSSL_ECH*)echX->data)->state == ECH_WRITE_NONE) {
7133+
if (((WOLFSSL_ECH*)echX->data)->innerClientHello != NULL) {
7134+
/* Client sent real ECH and inner hello was decrypted, jump to
7135+
* exit so the caller can re-invoke with the inner hello */
7136+
goto exit_dch;
7137+
}
7138+
else {
7139+
/* Server has ECH but client did not send ECH. Clear the
7140+
* response flag so the empty ECH extension is not written
7141+
* in EncryptedExtensions. */
7142+
echX->resp = 0;
7143+
}
7144+
}
71357145
#endif
71367146

71377147
#ifdef HAVE_SNI
@@ -7185,7 +7195,8 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
71857195
#if defined(HAVE_ECH)
71867196
/* hash clientHelloInner to hsHashesEch */
71877197
if (echX != NULL && ssl->ctx->echConfigs != NULL &&
7188-
!ssl->options.disableECH) {
7198+
!ssl->options.disableECH &&
7199+
((WOLFSSL_ECH*)echX->data)->innerClientHello != NULL) {
71897200
ret = EchHashHelloInner(ssl, (WOLFSSL_ECH*)echX->data);
71907201
if (ret != 0)
71917202
goto exit_dch;
@@ -7444,7 +7455,8 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
74447455

74457456
#if defined(HAVE_ECH)
74467457
if (ret == 0 && echX != NULL &&
7447-
((WOLFSSL_ECH*)echX->data)->state == ECH_WRITE_NONE) {
7458+
((WOLFSSL_ECH*)echX->data)->state == ECH_WRITE_NONE &&
7459+
((WOLFSSL_ECH*)echX->data)->innerClientHello != NULL) {
74487460

74497461
/* add the header to the inner hello */
74507462
AddTls13HandShakeHeader(((WOLFSSL_ECH*)echX->data)->innerClientHello,
@@ -12997,16 +13009,21 @@ int DoTls13HandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
1299713009
echX = TLSX_Find(ssl->extensions, TLSX_ECH);
1299813010

1299913011
if (echX != NULL &&
13000-
((WOLFSSL_ECH*)echX->data)->state == ECH_WRITE_NONE) {
13012+
((WOLFSSL_ECH*)echX->data)->state == ECH_WRITE_NONE &&
13013+
((WOLFSSL_ECH*)echX->data)->innerClientHello != NULL) {
1300113014
byte copyRandom = ((WOLFSSL_ECH*)echX->data)->innerCount == 0;
1300213015
/* reset the inOutIdx to the outer start */
1300313016
*inOutIdx = echInOutIdx;
1300413017
/* call again with the inner hello */
1300513018
if (ret == 0) {
13019+
((WOLFSSL_ECH*)echX->data)->processingInner = 1;
13020+
1300613021
ret = DoTls13ClientHello(ssl,
1300713022
((WOLFSSL_ECH*)echX->data)->innerClientHello,
1300813023
&echInOutIdx,
1300913024
((WOLFSSL_ECH*)echX->data)->innerClientHelloLen);
13025+
13026+
((WOLFSSL_ECH*)echX->data)->processingInner = 0;
1301013027
}
1301113028
/* if the inner ech parsed successfully we have successfully
1301213029
* handled the hello and can skip the whole message */

0 commit comments

Comments
 (0)