Skip to content

Commit 8a7d327

Browse files
sebastian-carpentersebastian-carpenter
committed
ECH fixes F-293, F-201, F-358, F-203
1 parent 032dbe6 commit 8a7d327

2 files changed

Lines changed: 227 additions & 83 deletions

File tree

‎src/ssl_ech.c‎

Lines changed: 126 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -490,68 +490,61 @@ int SetEchConfigsEx(WOLFSSL_EchConfig** outputConfigs, void* heap,
490490
const byte* echConfigs, word32 echConfigsLen)
491491
{
492492
int ret = 0;
493-
int i;
493+
word32 configIdx;
494+
word32 idx;
494495
int j;
495496
word16 totalLength;
496497
word16 version;
497498
word16 length;
498499
word16 hpkePubkeyLen;
499500
word16 cipherSuitesLen;
500-
word16 publicNameLen;
501+
word16 extensionsLen;
502+
byte publicNameLen;
501503
WOLFSSL_EchConfig* configList = NULL;
502504
WOLFSSL_EchConfig* workingConfig = NULL;
503505
WOLFSSL_EchConfig* lastConfig = NULL;
504-
byte* echConfig = NULL;
506+
const byte* echConfig = NULL;
505507

506-
if (outputConfigs == NULL || echConfigs == NULL || echConfigsLen == 0)
508+
if (outputConfigs == NULL || echConfigs == NULL || echConfigsLen < 2)
507509
return BAD_FUNC_ARG;
508510

509511
/* check that the total length is well formed */
510512
ato16(echConfigs, &totalLength);
511-
512513
if (totalLength != echConfigsLen - 2) {
513514
return WOLFSSL_FATAL_ERROR;
514515
}
515-
516-
/* skip the total length uint16_t */
517-
i = 2;
516+
configIdx = 2;
518517

519518
do {
520-
echConfig = (byte*)echConfigs + i;
519+
if (configIdx + 4 > echConfigsLen) {
520+
ret = BUFFER_E;
521+
break;
522+
}
523+
echConfig = echConfigs + configIdx;
521524
ato16(echConfig, &version);
522525
ato16(echConfig + 2, &length);
523526

524-
/* if the version does not match */
525-
if (version != TLSX_ECH) {
526-
/* we hit the end of the configs */
527-
if ( (word32)i + 2 >= echConfigsLen ) {
528-
break;
529-
}
530-
531-
/* skip this config, +4 for version and length */
532-
i += length + 4;
533-
continue;
534-
}
535-
536-
/* check if the length will overrun the buffer */
537-
if ((word32)i + length + 4 > echConfigsLen) {
527+
if (configIdx + length + 4 > echConfigsLen) {
528+
ret = BUFFER_E;
538529
break;
539530
}
531+
else if (version != TLSX_ECH) {
532+
/* skip this config and try the next one */
533+
configIdx += length + 4;
534+
continue;
535+
}
540536

541537
if (workingConfig == NULL) {
542538
workingConfig =
543539
(WOLFSSL_EchConfig*)XMALLOC(sizeof(WOLFSSL_EchConfig), heap,
544-
DYNAMIC_TYPE_TMP_BUFFER);
540+
DYNAMIC_TYPE_TMP_BUFFER);
545541
configList = workingConfig;
546-
if (workingConfig != NULL) {
547-
workingConfig->next = NULL;
548-
}
549542
}
550543
else {
551544
lastConfig = workingConfig;
552545
workingConfig->next =
553-
(WOLFSSL_EchConfig*)XMALLOC(sizeof(WOLFSSL_EchConfig),
554-
heap, DYNAMIC_TYPE_TMP_BUFFER);
546+
(WOLFSSL_EchConfig*)XMALLOC(sizeof(WOLFSSL_EchConfig), heap,
547+
DYNAMIC_TYPE_TMP_BUFFER);
555548
workingConfig = workingConfig->next;
556549
}
557550

@@ -566,8 +559,8 @@ int SetEchConfigsEx(WOLFSSL_EchConfig** outputConfigs, void* heap,
566559
workingConfig->rawLen = length + 4;
567560

568561
/* raw body */
569-
workingConfig->raw = (byte*)XMALLOC(workingConfig->rawLen,
570-
heap, DYNAMIC_TYPE_TMP_BUFFER);
562+
workingConfig->raw = (byte*)XMALLOC(workingConfig->rawLen, heap,
563+
DYNAMIC_TYPE_TMP_BUFFER);
571564
if (workingConfig->raw == NULL) {
572565
ret = MEMORY_E;
573566
break;
@@ -578,24 +571,56 @@ int SetEchConfigsEx(WOLFSSL_EchConfig** outputConfigs, void* heap,
578571
/* skip over version and length */
579572
echConfig += 4;
580573

574+
idx = 5;
575+
if (idx >= length) {
576+
ret = BUFFER_E;
577+
break;
578+
}
579+
581580
/* configId, 1 byte */
582-
workingConfig->configId = *(echConfig);
581+
workingConfig->configId = *echConfig;
583582
echConfig++;
584583
/* kemId, 2 bytes */
585584
ato16(echConfig, &workingConfig->kemId);
586585
echConfig += 2;
587586
/* hpke public_key length, 2 bytes */
588587
ato16(echConfig, &hpkePubkeyLen);
589588
echConfig += 2;
589+
590590
/* hpke public_key */
591-
if (hpkePubkeyLen > HPKE_Npk_MAX) {
591+
if (hpkePubkeyLen > HPKE_Npk_MAX || hpkePubkeyLen == 0) {
592592
ret = BUFFER_E;
593593
break;
594594
}
595+
idx += hpkePubkeyLen;
596+
if (idx >= length) {
597+
ret = BUFFER_E;
598+
break;
599+
}
600+
595601
XMEMCPY(workingConfig->receiverPubkey, echConfig, hpkePubkeyLen);
596602
echConfig += hpkePubkeyLen;
603+
597604
/* cipherSuitesLen */
605+
idx += 2;
606+
if (idx >= length) {
607+
ret = BUFFER_E;
608+
break;
609+
}
598610
ato16(echConfig, &cipherSuitesLen);
611+
if (cipherSuitesLen == 0 || cipherSuitesLen % 4 != 0 ||
612+
cipherSuitesLen >= 1024) {
613+
/* numCipherSuites is a byte so only 256 ciphersuites (each 4 bytes)
614+
* can be accessed */
615+
ret = BUFFER_E;
616+
break;
617+
}
618+
619+
idx += cipherSuitesLen;
620+
if (idx >= length) {
621+
ret = BUFFER_E;
622+
break;
623+
}
599624

600625
workingConfig->cipherSuites = (EchCipherSuite*)XMALLOC(cipherSuitesLen,
601626
heap, DYNAMIC_TYPE_TMP_BUFFER);
@@ -605,51 +630,100 @@ int SetEchConfigsEx(WOLFSSL_EchConfig** outputConfigs, void* heap,
605630
}
606631

607632
echConfig += 2;
608-
workingConfig->numCipherSuites = cipherSuitesLen / 4;
633+
workingConfig->numCipherSuites = (byte)(cipherSuitesLen / 4);
609634
/* cipherSuites */
610635
for (j = 0; j < workingConfig->numCipherSuites; j++) {
611-
ato16(echConfig + j * 4, &workingConfig->cipherSuites[j].kdfId);
612-
ato16(echConfig + j * 4 + 2,
613-
&workingConfig->cipherSuites[j].aeadId);
636+
ato16(echConfig, &workingConfig->cipherSuites[j].kdfId);
637+
ato16(echConfig + 2, &workingConfig->cipherSuites[j].aeadId);
638+
echConfig += 4;
614639
}
615-
echConfig += cipherSuitesLen;
640+
616641
/* ignore the maximum name length */
642+
idx++;
643+
if (idx >= length) {
644+
ret = BUFFER_E;
645+
break;
646+
}
617647
echConfig++;
648+
618649
/* publicNameLen */
619-
publicNameLen = *(echConfig);
650+
idx++;
651+
if (idx >= length) {
652+
ret = BUFFER_E;
653+
break;
654+
}
655+
656+
publicNameLen = *echConfig;
657+
if (publicNameLen == 0) {
658+
ret = BUFFER_E;
659+
break;
660+
}
661+
662+
idx += publicNameLen;
663+
if (idx >= length) {
664+
ret = BUFFER_E;
665+
break;
666+
}
667+
echConfig++;
668+
620669
workingConfig->publicName = (char*)XMALLOC(publicNameLen + 1,
621670
heap, DYNAMIC_TYPE_TMP_BUFFER);
622671
if (workingConfig->publicName == NULL) {
623672
ret = MEMORY_E;
624673
break;
625674
}
626-
echConfig++;
675+
627676
/* publicName */
628677
XMEMCPY(workingConfig->publicName, echConfig, publicNameLen);
629-
/* null terminated */
630-
workingConfig->publicName[publicNameLen] = 0;
678+
workingConfig->publicName[publicNameLen] = '\0';
679+
echConfig += publicNameLen;
680+
681+
/* TODO: Parse ECHConfigExtension */
682+
/* --> for now just ignore it */
683+
idx += 2;
684+
if (idx > length) {
685+
ret = BUFFER_E;
686+
break;
687+
}
688+
ato16(echConfig, &extensionsLen);
631689

632-
/* add length to go to next config, +4 for version and length */
633-
i += length + 4;
690+
idx += extensionsLen;
691+
if (idx != length) {
692+
ret = BUFFER_E;
693+
break;
694+
}
634695

635696
/* check that we support this config */
636697
for (j = 0; j < HPKE_SUPPORTED_KEM_LEN; j++) {
637698
if (hpkeSupportedKem[j] == workingConfig->kemId)
638699
break;
639700
}
640701

641-
/* if we don't support the kem or at least one cipher suite */
702+
/* KEM or ciphersuite not supported, free this config and then try to
703+
* parse another */
642704
if (j >= HPKE_SUPPORTED_KEM_LEN ||
643-
EchConfigGetSupportedCipherSuite(workingConfig) < 0)
644-
{
645-
XFREE(workingConfig->cipherSuites, heap,
646-
DYNAMIC_TYPE_TMP_BUFFER);
647-
XFREE(workingConfig->publicName, heap,
648-
DYNAMIC_TYPE_TMP_BUFFER);
705+
EchConfigGetSupportedCipherSuite(workingConfig) < 0) {
706+
XFREE(workingConfig->cipherSuites, heap, DYNAMIC_TYPE_TMP_BUFFER);
707+
XFREE(workingConfig->publicName, heap, DYNAMIC_TYPE_TMP_BUFFER);
649708
XFREE(workingConfig->raw, heap, DYNAMIC_TYPE_TMP_BUFFER);
709+
XFREE(workingConfig, heap, DYNAMIC_TYPE_TMP_BUFFER);
650710
workingConfig = lastConfig;
711+
712+
if (workingConfig != NULL) {
713+
workingConfig->next = NULL;
714+
}
715+
else {
716+
/* if one (or more) of the leading configs are unsupported then
717+
* this case will be hit */
718+
configList = NULL;
719+
}
651720
}
652-
} while ((word32)i < echConfigsLen);
721+
722+
configIdx += 4 + length;
723+
} while (configIdx < echConfigsLen);
724+
if (ret == 0 && configIdx != echConfigsLen){
725+
ret = BUFFER_E;
726+
}
653727

654728
/* if we found valid configs */
655729
if (ret == 0 && configList != NULL) {
@@ -659,15 +733,13 @@ int SetEchConfigsEx(WOLFSSL_EchConfig** outputConfigs, void* heap,
659733
}
660734

661735
workingConfig = configList;
662-
663736
while (workingConfig != NULL) {
664737
lastConfig = workingConfig;
665738
workingConfig = workingConfig->next;
666739

667740
XFREE(lastConfig->cipherSuites, heap, DYNAMIC_TYPE_TMP_BUFFER);
668741
XFREE(lastConfig->publicName, heap, DYNAMIC_TYPE_TMP_BUFFER);
669742
XFREE(lastConfig->raw, heap, DYNAMIC_TYPE_TMP_BUFFER);
670-
671743
XFREE(lastConfig, heap, DYNAMIC_TYPE_TMP_BUFFER);
672744
}
673745

0 commit comments

Comments
 (0)