Move HRR group restore and check out of extension parsing

Author: Frauschi

src/tls.c

@@ -7248,55 +7248,6 @@ static int TLSX_Cookie_Write(Cookie* cookie, byte* output, byte msgType,
     return 0;
 }
 
-#if defined(WOLFSSL_DTLS13) && defined (WOLFSSL_SEND_HRR_COOKIE)
-/* Extract the key share group from the cookie and store it in the
- * ssl session for later checks.
- *
- * ssl    The SSL/TLS object.
- * returns 0 on success and other values indicate failure.
- */
-static int TLSX_Cookie_RestoreHrrGroup(WOLFSSL* ssl)
-{
-    TLSX* extension;
-    Cookie* cookie;
-#ifndef NO_SHA256
-    byte macSz = WC_SHA256_DIGEST_SIZE;
-#elif defined(WOLFSSL_SHA384)
-    byte macSz = WC_SHA384_DIGEST_SIZE;
-#elif defined(WOLFSSL_TLS13_SHA512)
-    byte macSz = WC_SHA512_DIGEST_SIZE;
-#elif defined(WOLFSSL_SM3)
-    byte macSz = WC_SM3_DIGEST_SIZE;
-#else
-    #error "No digest available to use with HMAC for cookies."
-#endif /* NO_SHA */
-
-    extension = TLSX_Find(ssl->extensions, TLSX_COOKIE);
-    if (extension == NULL)
-        return 0;
-
-    cookie = (Cookie*)extension->data;
-    if (cookie == NULL)
-        return 0;
-
-    /* Cookie Data = Hash Len | Hash | CS | KeyShare Group (optional) | MAC */
-
-    /* Check if the cookie has a key share group */
-    if (cookie->data[0] + 4 + macSz < cookie->len) {
-        word16 keyShareGroup = 0;
-        ato16(cookie->data + 3 + cookie->data[0], &keyShareGroup);
-
-        /* The key share group in the cookie is the group selected by the
-         * server in the HelloRetryRequest. Hence, the client must use this
-         * group in the second ClientHello.
-         */
-        ssl->hrr_keyshare_group = keyShareGroup;
-    }
-
-    return 0;
-}
-#endif /* WOLFSSL_DTLS13 && WOLFSSL_SEND_HRR_COOKIE */
-
 /* Parse the Cookie extension.
  * In messages: ClientHello and HelloRetryRequest.
  *
@@ -7340,19 +7291,11 @@ static int TLSX_Cookie_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     if (extension == NULL) {
 #ifdef WOLFSSL_DTLS13
         if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version)) {
-            int ret = 0;
             /* Allow a cookie extension with DTLS 1.3 because it is possible
              * that a different SSL instance sent the cookie but we are now
              * receiving it. */
-            ret = TLSX_Cookie_Use(ssl, input + idx, len, NULL, 0, 0,
-                                  &ssl->extensions);
-    #if defined(WOLFSSL_SEND_HRR_COOKIE)
-            if (ret == 0 && ssl->options.dtlsStateful) {
-                /* Try to extract a HRR key share group from the cookie */
-                ret = TLSX_Cookie_RestoreHrrGroup(ssl);
-            }
-    #endif
-            return ret;
+            return TLSX_Cookie_Use(ssl, input + idx, len, NULL, 0, 0,
+                                   &ssl->extensions);
         }
         else
 #endif
@@ -10274,20 +10217,6 @@ int TLSX_KeyShare_Parse_ClientHello(const WOLFSSL* ssl,
         offset += ret;
     }
 
-    if (ssl->hrr_keyshare_group != 0) {
-        /*
-         * https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.8
-         *   when sending the new ClientHello, the client MUST
-         *   replace the original "key_share" extension with one containing only a
-         *   new KeyShareEntry for the group indicated in the selected_group field
-         *   of the triggering HelloRetryRequest
-         */
-        if (seenGroupsCnt != 1 || seenGroups[0] != ssl->hrr_keyshare_group) {
-            WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA);
-            return BAD_KEY_SHARE_DATA;
-        }
-    }
-
     return 0;
 }
 

src/tls13.c

@@ -6436,6 +6436,17 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie)
     cookieData = cookie->data;
     idx = OPAQUE8_LEN;
 
+#ifdef WOLFSSL_DTLS13
+    /* Restore the HRR key share group from the cookie.
+     * Cookie Data = Hash Len (1B) | Hash | CS (2B) | KS Group (2B, optional)
+     */
+    if (cookieDataSz == hashSz + 5) {
+        word16 keyShareGroup = 0;
+        ato16(cookieData + hashSz + 3, &keyShareGroup);
+        ssl->hrr_keyshare_group = keyShareGroup;
+    }
+#endif /* WOLFSSL_DTLS13 */
+
     /* Restart handshake hash with synthetic message hash. */
     AddTls13HandShakeHeader(header, hashSz, 0, 0, message_hash, ssl);
 
@@ -7015,6 +7026,29 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
     }
 #endif
 
+#ifdef HAVE_SUPPORTED_CURVES
+    if (ssl->hrr_keyshare_group != 0) {
+        /*
+         * https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.8
+         *   when sending the new ClientHello, the client MUST
+         *   replace the original "key_share" extension with one containing only
+         *   a new KeyShareEntry for the group indicated in the selected_group
+         *   field of the triggering HelloRetryRequest.
+         */
+        TLSX* extension = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE);
+        if (extension != NULL) {
+            KeyShareEntry* kse = (KeyShareEntry*)extension->data;
+            /* Exactly one KeyShareEntry with the HRR group must be present. */
+            if (kse == NULL || kse->next != NULL ||
+                                        kse->group != ssl->hrr_keyshare_group) {
+                ERROR_OUT(BAD_KEY_SHARE_DATA, exit_dch);
+            }
+        }
+        else
+            ERROR_OUT(BAD_KEY_SHARE_DATA, exit_dch);
+    }
+#endif
+
 #if defined(HAVE_ECH)
     /* hash clientHelloInner to hsHashesEch independently since it can't include
      * the HRR */