ECH tidying

Author: sebastian-carpenter

src/tls.c

@@ -2358,7 +2358,7 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
 #if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
     if (ech != NULL && ech->sniState == ECH_INNER_SNI){
         /* SNI status is carried over from processing the outer hello so it is
-        * necessary to clear it before processing the inner hello */
+         * necessary to clear it before processing the inner hello */
         ech->sniState = ECH_INNER_SNI_ATTEMPT;
         if (sni != NULL){
             sni->status = WOLFSSL_SNI_NO_MATCH;
@@ -13340,11 +13340,23 @@ static int TLSX_ECH_Write(WOLFSSL_ECH* ech, byte msgType, byte* writeBuf,
 
     WOLFSSL_MSG("TLSX_ECH_Write");
     if (msgType == hello_retry_request) {
-        /* reserve space to write the confirmation to */
-        *offset += ECH_ACCEPT_CONFIRMATION_SZ;
-        /* set confBuf */
-        ech->confBuf = writeBuf;
-        return 0;
+        WC_ALLOC_VAR_EX(rng, WC_RNG, 1, NULL, DYNAMIC_TYPE_RNG, ret = MEMORY_E);
+        if (ret == 0) {
+            ret = wc_InitRng(rng);
+        }
+        if (ret == 0) {
+            /* randomize confirmation in case ech is rejected */
+            ret = wc_RNG_GenerateBlock(rng, writeBuf,
+                    ECH_ACCEPT_CONFIRMATION_SZ);
+            wc_FreeRng(rng);
+        }
+        if (ret == 0) {
+            *offset += ECH_ACCEPT_CONFIRMATION_SZ;
+            ech->confBuf = writeBuf;
+        }
+
+        WC_FREE_VAR_EX(rng, NULL, DYNAMIC_TYPE_RNG);
+        return ret;
     }
     if (ech->state == ECH_WRITE_NONE || ech->state == ECH_PARSED_INTERNAL)
         return 0;
@@ -13567,7 +13579,7 @@ static int TLSX_ECH_CheckInnerPadding(WOLFSSL* ssl, WOLFSSL_ECH* ech)
  * returns 0 on success and otherwise failure.
  */
 static const byte* TLSX_ECH_FindOuterExtension(const byte* outerCh,
-    word32 chLen, word16 extType, word16* extLen, word16* extOffset,
+    word32 chLen, word16 extType, word32* extLen, word32* extOffset,
     word16* extensionsStart, word16* extensionsLen)
 {
     word32 idx = *extOffset;
@@ -13647,8 +13659,8 @@ static int TLSX_ECH_CopyOuterExtensions(const byte* outerCh, word32 outerChLen,
 {
     int ret = 0;
     word16 refType;
-    word16 outerExtLen;
-    word16 outerExtOffset = 0;
+    word32 outerExtLen;
+    word32 outerExtOffset = 0;
     word16 extsStart;
     word16 extsLen;
     const byte* outerExtData;

src/tls13.c

@@ -4054,7 +4054,6 @@ static int EchCalcAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
 #endif
     return ret;
 }
-
 #endif
 
 #ifndef NO_WOLFSSL_CLIENT
@@ -5065,12 +5064,20 @@ static int Dtls13ClientDoDowngrade(WOLFSSL* ssl)
 #endif /* WOLFSSL_DTLS13 && !WOLFSSL_NO_CLIENT*/
 
 #if defined(HAVE_ECH)
-/* check if the server accepted ech or not, return status */
+/* Calculate ECH acceptance and verify the server accepted ECH.
+ *
+ * ssl          SSL/TLS object.
+ * label        Ascii string describing ECH acceptance type.
+ * labelSz      Length of label excluding NULL character.
+ * input        The buffer to calculate confirmation off of.
+ * acceptOffset Where the 8 ECH confirmation bytes start.
+ * helloSz      Size of hello message.
+ * returns 0 on success and otherwise failure.
+ */
 static int EchCheckAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
-    const byte* input, int acceptOffset, int helloSz)
+    const byte* input, int acceptOffset, int helloSz, byte msgType)
 {
     int ret = 0;
-    int isHrr = 0;
     int headerSz;
     HS_Hashes* tmpHashes;
     byte acceptConfirmation[ECH_ACCEPT_CONFIRMATION_SZ];
@@ -5084,13 +5091,8 @@ static int EchCheckAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
     headerSz = HANDSHAKE_HEADER_SZ;
 #endif
 
-    if (labelSz == ECH_HRR_ACCEPT_CONFIRMATION_LABEL_SZ &&
-        XMEMCMP(label, echHrrAcceptConfirmationLabel, labelSz) == 0) {
-        isHrr = 1;
-    }
-
     ret = EchCalcAcceptance(ssl, label, labelSz, input, acceptOffset, helloSz,
-            isHrr, acceptConfirmation);
+            msgType == hello_retry_request, acceptConfirmation);
 
     tmpHashes = ssl->hsHashes;
     ssl->hsHashes = ssl->hsHashesEch;
@@ -5105,7 +5107,7 @@ static int EchCheckAcceptance(WOLFSSL* ssl, byte* label, word16 labelSz,
 
             /* after HRR, hsHashesEch must contain:
              * message_hash(ClientHelloInner1) || HRR (actual, not zeros) */
-            if (isHrr) {
+            if (msgType == hello_retry_request) {
                 ret = HashRaw(ssl, input, helloSz + headerSz);
             }
             /* normal TLS code will calculate transcript of ServerHello */
@@ -5663,7 +5665,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
     }
 
 #if defined(HAVE_ECH)
-    /* check for acceptConfirmation, must be done after hashes restart */
+    /* check for acceptConfirmation */
     if (ssl->echConfigs != NULL && !ssl->options.disableECH) {
         args->echX = TLSX_Find(ssl->extensions, TLSX_ECH);
         /* account for hrr extension instead of server random */
@@ -5680,7 +5682,8 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
         /* check acceptance */
         if (ret == 0) {
             ret = EchCheckAcceptance(ssl, args->acceptLabel,
-                args->acceptLabelSz, input, args->acceptOffset, helloSz);
+                args->acceptLabelSz, input, args->acceptOffset, helloSz,
+                args->extMsgType);
         }
         if (ret != 0)
             return ret;

tests/api.c

@@ -14210,9 +14210,9 @@ static int test_wolfSSL_Tls13_ECH_params_b64(void)
     /* base64 ech configs from cloudflare-ech.com (these are good configs) */
     const char* b64Valid = "AEX+DQBBFAAgACBuAoQI8+liEVYQbXKBDeVgTmF2rfXuKO2knhwrN7jgTgAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA=";
     /* ech configs with bad/unsupported algorithm */
-    const char* b64BadAlgo = "AEX+DQBBFP//ACBuAoQI8+liEVYQbXKBDeVgTmF2rfXuKO2knhwrN7jgTgAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA=";
+    const char* b64BadAlgo = "AEX+DQBBFP7+ACBuAoQI8+liEVYQbXKBDeVgTmF2rfXuKO2knhwrN7jgTgAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA=";
     /* ech configs with bad/unsupported ciphersuite */
-    const char* b64BadCiph = "AEX+DQBBFAAgACBuAoQI8+liEVYQbXKBDeVgTmF2rfXuKO2knhwrN7jgTgAE//8AAQASY2xvdWRmbGFyZS1lY2guY29tAAA=";
+    const char* b64BadCiph = "AEX+DQBBFAAgACBuAoQI8+liEVYQbXKBDeVgTmF2rfXuKO2knhwrN7jgTgAE/v4AAQASY2xvdWRmbGFyZS1lY2guY29tAAA=";
     /* ech configs with bad version first */
     const char* b64BadVers1 = "AIz+HQBCAQAgACCjR6+Qn9UYkMaWdXZzsby88vXFhPHJ2tWCDHQJLvMkEgAEAAEAAQATZWNoLXB1YmxpYy1uYW1lLmNvbQAA/g0AQgIAIAAgMM6vLrTbOfsfA6fTbJY/Iu0Lj2xeHEPGUJeUwQGAYF4ABAABAAEAE2VjaC1wdWJsaWMtbmFtZS5jb20AAA==";
     /* ech configs with bad version second */

wolfcrypt/src/hpke.c

@@ -464,12 +464,34 @@ static int wc_HpkeLabeledExtract(Hpke* hpke, byte* suite_id,
 {
     int ret;
     byte* labeled_ikm_p;
+    word32 remaining;
     WC_DECLARE_VAR(labeled_ikm, byte, MAX_HPKE_LABEL_SZ, 0);
 
     if (hpke == NULL) {
         return BAD_FUNC_ARG;
     }
 
+    /* check that sum of len's will not overflow */
+    remaining = MAX_HPKE_LABEL_SZ;
+    if ((word32)HPKE_VERSION_STR_LEN > remaining) {
+        return BUFFER_E;
+    }
+    remaining -= (word32)HPKE_VERSION_STR_LEN;
+
+    if (suite_id_len > remaining) {
+        return BUFFER_E;
+    }
+    remaining -= suite_id_len;
+
+    if (label_len > remaining) {
+        return BUFFER_E;
+    }
+    remaining -= label_len;
+
+    if (ikm_len > remaining) {
+        return BUFFER_E;
+    }
+
     WC_ALLOC_VAR_EX(labeled_ikm, byte, MAX_HPKE_LABEL_SZ, hpke->heap,
         DYNAMIC_TYPE_TMP_BUFFER, return MEMORY_E);
 
@@ -511,12 +533,39 @@ static int wc_HpkeLabeledExpand(Hpke* hpke, byte* suite_id, word32 suite_id_len,
 {
     int ret;
     byte* labeled_info_p;
+    word32 remaining;
     WC_DECLARE_VAR(labeled_info, byte, MAX_HPKE_LABEL_SZ, 0);
 
     if (hpke == NULL) {
         return BAD_FUNC_ARG;
     }
 
+    /* check that sum of len's will not overflow */
+    remaining = MAX_HPKE_LABEL_SZ;
+    if (2u > remaining){
+        return BUFFER_E;
+    }
+    remaining -= 2u;
+
+    if ((word32)HPKE_VERSION_STR_LEN > remaining) {
+        return BUFFER_E;
+    }
+    remaining -= (word32)HPKE_VERSION_STR_LEN;
+
+    if (suite_id_len > remaining) {
+        return BUFFER_E;
+    }
+    remaining -= suite_id_len;
+
+    if (label_len > remaining) {
+        return BUFFER_E;
+    }
+    remaining -= label_len;
+
+    if (infoSz > remaining) {
+        return BUFFER_E;
+    }
+
     WC_ALLOC_VAR_EX(labeled_info, byte, MAX_HPKE_LABEL_SZ, hpke->heap,
         DYNAMIC_TYPE_TMP_BUFFER, return MEMORY_E);