Skip to content

Commit 4a86143

Browse files
sebastian-carpentersebastian-carpenter
committed
minor fixes
1 parent 9675b89 commit 4a86143

3 files changed

Lines changed: 80 additions & 45 deletions

File tree

‎src/tls.c‎

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2362,6 +2362,7 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
23622362
word16 offset = 0;
23632363
int cacheOnly = 0;
23642364
SNI *sni = NULL;
2365+
const char *hostName = NULL;
23652366
byte type;
23662367
byte matched;
23672368
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
@@ -2464,14 +2465,14 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
24642465
return 0;
24652466
#endif
24662467

2467-
{
2468-
const char* hostName = (sni != NULL) ? sni->data.host_name : NULL;
2469-
matched = cacheOnly || (hostName != NULL &&
2470-
XSTRLEN(hostName) == size &&
2471-
XSTRNCMP(hostName, (const char*)input + offset, size) == 0);
2472-
}
2468+
hostName = (sni != NULL) ? sni->data.host_name : NULL;
2469+
matched = (hostName != NULL &&
2470+
XSTRLEN(hostName) == size &&
2471+
XSTRNCMP(hostName, (const char*)input + offset, size) == 0);
24732472

24742473
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
2474+
/* While parsing the outer CH accept a match against any
2475+
* echConfig publicName */
24752476
if (!matched && ech != NULL && !ssl->options.echProcessingInner) {
24762477
workingConfig = ech->echConfig;
24772478
while (workingConfig != NULL) {
@@ -2482,9 +2483,17 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
24822483
break;
24832484
workingConfig = workingConfig->next;
24842485
}
2486+
2487+
/* If a publicName is matched then this SNI is not something that should
2488+
* be forcibly cached */
2489+
if (matched)
2490+
cacheOnly = 0;
24852491
}
24862492
#endif
24872493

2494+
if (!matched)
2495+
matched = cacheOnly;
2496+
24882497
if (matched ||
24892498
(sni != NULL && (sni->options & WOLFSSL_SNI_ANSWER_ON_MISMATCH))) {
24902499
int matchStat;
@@ -13703,6 +13712,7 @@ static int TLSX_ECH_Use(WOLFSSL_EchConfig* echConfig, TLSX** extensions,
1370313712
XFREE(ech, heap, DYNAMIC_TYPE_TMP_BUFFER);
1370413713
return MEMORY_E;
1370513714
}
13715+
ForceZero(ech->hpke, sizeof(Hpke));
1370613716
ret = wc_HpkeInit(ech->hpke, ech->kemId, ech->cipherSuite.kdfId,
1370713717
ech->cipherSuite.aeadId, heap);
1370813718
/* setup the ephemeralKey */
@@ -16417,11 +16427,11 @@ int TLSX_PopulateExtensions(WOLFSSL* ssl, byte isServer)
1641716427

1641816428
#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
1641916429
/* Returns 1 if the extensions should be hidden for this write */
16420-
static int TLSX_EchShouldHideInner(WOLFSSL* ssl, WOLFSSL_ECH* ech)
16430+
static int TLSX_EchShouldHideInner(WOLFSSL_ECH* ech)
1642116431
{
1642216432
if (ech == NULL || ech->type != ECH_TYPE_OUTER)
1642316433
return 0;
16424-
return ssl->options.echAccepted || ech->innerCount == 0;
16434+
return 1;
1642516435
}
1642616436

1642716437
/* Swap matching extension types between *sslExts and *echExts.
@@ -16442,7 +16452,7 @@ static word16 TLSX_EchSwapExtensions(TLSX** sslExts, TLSX** echExts,
1644216452
TLSX* inner;
1644316453
TLSX** outerLink;
1644416454
TLSX** innerLink;
16445-
word16 appended = 0;
16455+
word16 prepended = 0;
1644616456

1644716457
/* unhook popCount nodes off *sslExts head into chunk.
1644816458
* Head-prepend undoes the reversal caused by appending onto sslExts. */
@@ -16477,19 +16487,19 @@ static word16 TLSX_EchSwapExtensions(TLSX** sslExts, TLSX** echExts,
1647716487
*outerLink = outer->next;
1647816488
outer->next = *sslExts;
1647916489
*sslExts = outer;
16480-
appended++;
16490+
prepended++;
1648116491
}
1648216492
}
1648316493

1648416494
/* outerLink is at the tail of *echExts; append the chunk */
1648516495
*outerLink = chunk;
1648616496

16487-
return appended;
16497+
return prepended;
1648816498
}
1648916499

1649016500
/* If ECH is accepted, delete ech->extensions
16491-
* If rejected, replace matching ssl->extensions with ech->extensions, appending
16492-
* to head if necessary */
16501+
* If rejected, replace matching ssl->extensions with ech->extensions,
16502+
* prepending to head if necessary */
1649316503
void TLSX_EchReplaceExtensions(WOLFSSL* ssl, byte accepted)
1649416504
{
1649516505
TLSX* echX;
@@ -16618,25 +16628,20 @@ static int TLSX_GetSizeWithEch(WOLFSSL* ssl, byte* semaphore, byte msgType,
1661816628
TLSX* echX = NULL;
1661916629
WOLFSSL_ECH* ech = NULL;
1662016630
word16 count = 0;
16621-
word16 appended = 0;
16631+
word16 prepended = 0;
1662216632
byte installed = 0;
1662316633

1662416634
if (ssl->extensions)
1662516635
echX = TLSX_Find(ssl->extensions, TLSX_ECH);
16626-
if (echX == NULL && ssl->ctx && ssl->ctx->extensions)
16627-
echX = TLSX_Find(ssl->ctx->extensions, TLSX_ECH);
1662816636
if (echX != NULL)
1662916637
ech = (WOLFSSL_ECH*)echX->data;
1663016638

16631-
if (TLSX_EchShouldHideInner(ssl, ech)) {
16632-
appended = TLSX_EchSwapExtensions(&ssl->extensions,
16639+
if (TLSX_EchShouldHideInner(ech)) {
16640+
prepended = TLSX_EchSwapExtensions(&ssl->extensions,
1663316641
&ech->extensions, 0);
1663416642
installed = 1;
1663516643
}
1663616644

16637-
if (echX != NULL)
16638-
ech = (WOLFSSL_ECH*)echX->data;
16639-
1664016645
/* If ECH won't be written exclude it from the size calculation */
1664116646
if (!ssl->options.echAccepted && ech != NULL && ech->innerCount != 0) {
1664216647
TURN_ON(semaphore, TLSX_ToSemaphore(echX->type));
@@ -16653,9 +16658,13 @@ static int TLSX_GetSizeWithEch(WOLFSSL* ssl, byte* semaphore, byte msgType,
1665316658
if (ret == 0 && ssl->ctx && ssl->ctx->extensions)
1665416659
ret = TLSX_GetSize(ssl->ctx->extensions, semaphore, msgType, pLength);
1665516660

16656-
if (installed)
16657-
(void)TLSX_EchSwapExtensions(&ssl->extensions, &ech->extensions,
16658-
appended);
16661+
if (installed) {
16662+
prepended = TLSX_EchSwapExtensions(&ssl->extensions, &ech->extensions,
16663+
prepended);
16664+
if (ret == 0 && prepended != 0) {
16665+
ret = BAD_STATE_E;
16666+
}
16667+
}
1665916668
return ret;
1666016669
}
1666116670
#endif
@@ -16789,18 +16798,16 @@ static int TLSX_WriteWithEch(WOLFSSL* ssl, byte* output, byte* semaphore,
1678916798
int ret = 0;
1679016799
TLSX* echX = NULL;
1679116800
WOLFSSL_ECH* ech = NULL;
16792-
word16 appended = 0;
16801+
word16 prepended = 0;
1679316802
byte installed = 0;
1679416803

1679516804
if (ssl->extensions)
1679616805
echX = TLSX_Find(ssl->extensions, TLSX_ECH);
16797-
if (echX == NULL && ssl->ctx && ssl->ctx->extensions)
16798-
echX = TLSX_Find(ssl->ctx->extensions, TLSX_ECH);
1679916806
if (echX != NULL)
1680016807
ech = (WOLFSSL_ECH*)echX->data;
1680116808

16802-
if (TLSX_EchShouldHideInner(ssl, ech)) {
16803-
appended = TLSX_EchSwapExtensions(&ssl->extensions,
16809+
if (TLSX_EchShouldHideInner(ech)) {
16810+
prepended = TLSX_EchSwapExtensions(&ssl->extensions,
1680416811
&ech->extensions, 0);
1680516812
installed = 1;
1680616813
}
@@ -16853,7 +16860,7 @@ static int TLSX_WriteWithEch(WOLFSSL* ssl, byte* output, byte* semaphore,
1685316860
}
1685416861

1685516862
/* only write ECH if there is a shot at acceptance */
16856-
if (ret == 0 && echX != NULL &&
16863+
if (ret == 0 && ech != NULL &&
1685716864
(ssl->options.echAccepted || ech->innerCount == 0)) {
1685816865
/* turn off and write it last */
1685916866
TURN_OFF(semaphore, TLSX_ToSemaphore(echX->type));
@@ -16869,9 +16876,13 @@ static int TLSX_WriteWithEch(WOLFSSL* ssl, byte* output, byte* semaphore,
1686916876
}
1687016877
}
1687116878

16872-
if (installed)
16873-
(void)TLSX_EchSwapExtensions(&ssl->extensions, &ech->extensions,
16874-
appended);
16879+
if (installed) {
16880+
prepended = TLSX_EchSwapExtensions(&ssl->extensions, &ech->extensions,
16881+
prepended);
16882+
if (ret == 0 && prepended != 0) {
16883+
ret = BAD_STATE_E;
16884+
}
16885+
}
1687516886
return ret;
1687616887
}
1687716888
#endif

‎src/tls13.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4589,7 +4589,7 @@ typedef struct Sch13Args {
45894589
word32 length;
45904590
#if defined(HAVE_ECH)
45914591
int clientRandomOffset;
4592-
int preXLength;
4592+
word32 preXLength;
45934593
word32 expandedInnerLen;
45944594
WOLFSSL_ECH* ech;
45954595
#endif
@@ -4783,6 +4783,8 @@ int SendTls13ClientHello(WOLFSSL* ssl)
47834783
#if defined(HAVE_ECH)
47844784
if (ssl->echConfigs != NULL && !ssl->options.disableECH) {
47854785
TLSX* echX = TLSX_Find(ssl->extensions, TLSX_ECH);
4786+
void* hostName = NULL;
4787+
word16 nameLen;
47864788
if (echX == NULL)
47874789
return WOLFSSL_FATAL_ERROR;
47884790

@@ -4801,7 +4803,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
48014803

48024804
/* set the type to inner */
48034805
args->ech->type = ECH_TYPE_INNER;
4804-
args->preXLength = (int)args->length;
4806+
args->preXLength = args->length;
48054807

48064808
/* get expanded inner size (used for transcript) */
48074809
ret = TLSX_GetRequestSize(ssl, client_hello, &args->length);
@@ -4831,8 +4833,9 @@ int SendTls13ClientHello(WOLFSSL* ssl)
48314833
return ret;
48324834

48334835
/* calculate padding (RFC 9849, section 6.1.3) */
4834-
if (args->ech->privateName != NULL) {
4835-
word16 nameLen = (word16)XSTRLEN(args->ech->privateName);
4836+
nameLen = TLSX_SNI_GetRequest(ssl->extensions,
4837+
WOLFSSL_SNI_HOST_NAME, &hostName, 1);
4838+
if (hostName != NULL) {
48364839
if (nameLen > args->ech->echConfig->maxNameLen)
48374840
args->ech->paddingLen = 0;
48384841
else
@@ -4854,7 +4857,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
48544857
return BUFFER_E;
48554858

48564859
/* restore the length to pre-ClientHelloInner computations */
4857-
args->length = (word32)args->preXLength;
4860+
args->length = args->preXLength;
48584861
}
48594862
}
48604863
#endif

‎tests/api.c‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13430,6 +13430,8 @@ static int test_wolfSSL_Tls13_ECH_params_b64(void)
1343013430
const char* b64BadCiph = "AEX+DQBBFAAgACBuAoQI8+liEVYQbXKBDeVgTmF2rfXuKO2knhwrN7jgTgAE/v4AAQASY2xvdWRmbGFyZS1lY2guY29tAAA=";
1343113431
/* ech configs with unrecognized mandatory extension */
1343213432
const char* b64Mandatory = "AEn+DQBFFAAgACBuAoQI8+liEVYQbXKBDeVgTmF2rfXuKO2knhwrN7jgTgAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAT6+gAA";
13433+
/* ech configs with unrecognized mandatory extension first */
13434+
const char* b64MandatoryFirst = "AJD+DQBGAQAgACCjR6+Qn9UYkMaWdXZzsby88vXFhPHJ2tWCDHQJLvMkEgAEAAEAAQATZWNoLXB1YmxpYy1uYW1lLmNvbQAE+voAAP4NAEICACAAIDDOry602zn7HwOn02yWPyLtC49sXhxDxlCXlMEBgGBeAAQAAQABABNlY2gtcHVibGljLW5hbWUuY29tAAA=";
1343313435
/* ech configs with bad version first */
1343413436
const char* b64BadVers1 = "AIz+HQBCAQAgACCjR6+Qn9UYkMaWdXZzsby88vXFhPHJ2tWCDHQJLvMkEgAEAAEAAQATZWNoLXB1YmxpYy1uYW1lLmNvbQAA/g0AQgIAIAAgMM6vLrTbOfsfA6fTbJY/Iu0Lj2xeHEPGUJeUwQGAYF4ABAABAAEAE2VjaC1wdWJsaWMtbmFtZS5jb20AAA==";
1343513437
/* ech configs with bad version second */
@@ -13497,6 +13499,20 @@ static int test_wolfSSL_Tls13_ECH_params_b64(void)
1349713499
ExpectIntNE(WOLFSSL_SUCCESS, wolfSSL_SetEchConfigsBase64(ssl,
1349813500
b64Mandatory, (word32)XSTRLEN(b64Mandatory)));
1349913501

13502+
/* unrecognized mandatory extension */
13503+
ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_CTX_SetEchConfigsBase64(ctx,
13504+
b64MandatoryFirst, (word32)XSTRLEN(b64MandatoryFirst)));
13505+
ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_SetEchConfigsBase64(ssl,
13506+
b64MandatoryFirst, (word32)XSTRLEN(b64MandatoryFirst)));
13507+
ExpectIntEQ(2, ctx->echConfigs->configId);
13508+
ExpectIntEQ(2, ssl->echConfigs->configId);
13509+
13510+
/* clear configs */
13511+
wolfSSL_CTX_SetEchEnable(ctx, 0);
13512+
wolfSSL_CTX_SetEchEnable(ctx, 1);
13513+
wolfSSL_SetEchEnable(ssl, 0);
13514+
wolfSSL_SetEchEnable(ssl, 1);
13515+
1350013516
/* bad version first, should only have config 2 set */
1350113517
ExpectIntEQ(WOLFSSL_SUCCESS, wolfSSL_CTX_SetEchConfigsBase64(ctx,
1350213518
b64BadVers1, (word32)XSTRLEN(b64BadVers1)));
@@ -13889,9 +13905,7 @@ static int test_wolfSSL_Tls13_ECH_all_algos(void)
1388913905
return EXPECT_RESULT();
1389013906
}
1389113907

13892-
/* Test ECH when no private SNI is set
13893-
* SNI is by default permissive so these should pass
13894-
* (inner SNI is not required by ECH, only the outer SNI is required) */
13908+
/* Test ECH when no private SNI is set */
1389513909
static int test_wolfSSL_Tls13_ECH_no_private_name(void)
1389613910
{
1389713911
EXPECT_DECLS;
@@ -13929,14 +13943,19 @@ static int test_wolfSSL_Tls13_ECH_no_private_name(void)
1392913943

1393013944
ExpectIntEQ(test_ssl_memio_setup(&test_ctx), TEST_SUCCESS);
1393113945

13946+
/* SNI is permissive by default, force a failure when SNI is absent */
13947+
wolfSSL_SNI_SetOptions(test_ctx.s_ssl, WOLFSSL_SNI_HOST_NAME,
13948+
WOLFSSL_SNI_ABORT_ON_ABSENCE);
13949+
1393213950
ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, echCbTestConfigs,
1393313951
echCbTestConfigsLen), WOLFSSL_SUCCESS);
1393413952

1393513953
ExpectIntNE(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), TEST_SUCCESS);
13954+
/* server fails before sending response to client */
1393613955
ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.c_ssl),
1393713956
WOLFSSL_ECH_STATUS_REJECTED);
1393813957
ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.s_ssl),
13939-
WOLFSSL_ECH_STATUS_REJECTED);
13958+
WOLFSSL_ECH_STATUS_ACCEPTED);
1394013959

1394113960
test_ssl_memio_cleanup(&test_ctx);
1394213961

@@ -13954,11 +13973,13 @@ static int test_wolfSSL_Tls13_ECH_no_private_name(void)
1395413973
ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, echCbTestConfigs,
1395513974
echCbTestConfigsLen), WOLFSSL_SUCCESS);
1395613975

13957-
ExpectIntNE(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), TEST_SUCCESS);
13976+
/* it is odd to have no private SNI but it's not necessarily an issue,
13977+
* there are other methods that could be used to route the connection */
13978+
ExpectIntEQ(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), TEST_SUCCESS);
1395813979
ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.c_ssl),
13959-
WOLFSSL_ECH_STATUS_REJECTED);
13980+
WOLFSSL_ECH_STATUS_ACCEPTED);
1396013981
ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.s_ssl),
13961-
WOLFSSL_ECH_STATUS_REJECTED);
13982+
WOLFSSL_ECH_STATUS_ACCEPTED);
1396213983

1396313984
test_ssl_memio_cleanup(&test_ctx);
1396413985

0 commit comments

Comments
 (0)