add public sni handling to public ext

sebastian-carpenter · sebastian-carpenter · commit 9675b89603fe · 2026-06-01T13:56:37.000-06:00
diff --git a/src/tls.c b/src/tls.c
@@ -2367,8 +2367,7 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
 #if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
     TLSX* echX = NULL;
     WOLFSSL_ECH* ech = NULL;
-    WOLFSSL_EchConfig* workingConfig;
-    word16 privateNameLen;
+    WOLFSSL_EchConfig* workingConfig = NULL;
 #endif
 #endif /* !NO_WOLFSSL_SERVER */
     TLSX *extension = TLSX_Find(ssl->extensions, TLSX_SERVER_NAME);
@@ -2409,13 +2408,7 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     }
 #endif
 
-#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
-    if ((!extension || !extension->data) ||
-            (ech != NULL && ech->sniState == ECH_INNER_SNI &&
-             ech->privateName == NULL)) {
-#else
     if (!extension || !extension->data) {
-#endif
         /* This will keep SNI even though TLSX_UseSNI has not been called.
          * Enable it so that the received sni is available to functions
          * that use a custom callback when SNI is received.
@@ -2463,28 +2456,6 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     if (!cacheOnly && !(sni = TLSX_SNI_Find((SNI*)extension->data, type)))
         return 0; /* not using this type of SNI. */
 
-#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 */
-        ech->sniState = ECH_INNER_SNI_ATTEMPT;
-        if (sni != NULL){
-            sni->status = WOLFSSL_SNI_NO_MATCH;
-        }
-    }
-    else if (ech != NULL && ech->sniState == ECH_OUTER_SNI &&
-            ech->privateName == NULL && sni != NULL){
-        /* save the private SNI before it is overwritten by the public SNI */
-        privateNameLen = (word16)XSTRLEN(sni->data.host_name) + 1;
-        ech->privateName = (char*)XMALLOC(privateNameLen, ssl->heap,
-            DYNAMIC_TYPE_TMP_BUFFER);
-        if (ech->privateName == NULL)
-            return MEMORY_E;
-        XMEMCPY((char*)ech->privateName, sni->data.host_name,
-            privateNameLen);
-    }
-#endif
-
 #if defined(WOLFSSL_TLS13)
     /* Don't process the second ClientHello SNI extension if there
      * was problems with the first.
@@ -2493,14 +2464,6 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
         return 0;
 #endif
 
-#if defined(HAVE_ECH)
-    if (ech != NULL && ech->sniState == ECH_INNER_SNI_ATTEMPT &&
-            ech->privateName != NULL) {
-        matched = cacheOnly || (XSTRLEN(ech->privateName) == size &&
-            XSTRNCMP(ech->privateName, (const char*)input + offset, size) == 0);
-    }
-    else
-#endif
     {
         const char* hostName = (sni != NULL) ? sni->data.host_name : NULL;
         matched = cacheOnly || (hostName != NULL &&
@@ -2509,16 +2472,14 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     }
 
 #if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
-    if (!matched && ech != NULL && ech->sniState == ECH_OUTER_SNI) {
+    if (!matched && ech != NULL && !ssl->options.echProcessingInner) {
         workingConfig = ech->echConfig;
         while (workingConfig != NULL) {
             matched = XSTRLEN(workingConfig->publicName) == size &&
                 XSTRNCMP(workingConfig->publicName,
                 (const char*)input + offset, size) == 0;
-
             if (matched)
                 break;
-
             workingConfig = workingConfig->next;
         }
     }
@@ -2527,8 +2488,14 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
     if (matched ||
             (sni != NULL && (sni->options & WOLFSSL_SNI_ANSWER_ON_MISMATCH))) {
         int matchStat;
-        int r = TLSX_UseSNI(&ssl->extensions, type, input + offset, size,
-                                                                     ssl->heap);
+        int r;
+        TLSX** writeList = &ssl->extensions;
+#if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
+        if (workingConfig != NULL)
+            writeList = &ech->extensions;
+#endif
+
+        r = TLSX_UseSNI(writeList, type, input + offset, size, ssl->heap);
 
         if (r != WOLFSSL_SUCCESS)
             return r; /* throws error. */
@@ -2546,10 +2513,14 @@ static int TLSX_SNI_Parse(WOLFSSL* ssl, const byte* input, word16 length,
             matchStat = WOLFSSL_SNI_FAKE_MATCH;
         }
 
-        TLSX_SNI_SetStatus(ssl->extensions, type, (byte)matchStat);
+        TLSX_SNI_SetStatus(*writeList, type, (byte)matchStat);
 
-        if (!cacheOnly)
-            TLSX_SetResponse(ssl, TLSX_SERVER_NAME);
+        if (!cacheOnly) {
+            extension = TLSX_Find(*writeList, TLSX_SERVER_NAME);
+
+            if (extension)
+                extension->resp = 1;
+        }
     }
     else if ((sni == NULL) ||
             !(sni->options & WOLFSSL_SNI_CONTINUE_ON_MISMATCH)) {
diff --git a/src/tls13.c b/src/tls13.c
@@ -13678,18 +13678,12 @@ int DoTls13HandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
                 *inOutIdx = echInOutIdx;
                 /* call again with the inner hello */
                 if (ret == 0) {
-                    if (((WOLFSSL_ECH*)echX->data)->sniState == ECH_OUTER_SNI) {
-                        ((WOLFSSL_ECH*)echX->data)->sniState = ECH_INNER_SNI;
-                    }
-
                     ssl->options.echProcessingInner = 1;
                     ret = DoTls13ClientHello(ssl,
                         ((WOLFSSL_ECH*)echX->data)->innerClientHello,
                         &echInOutIdx,
                         ((WOLFSSL_ECH*)echX->data)->innerClientHelloLen);
                     ssl->options.echProcessingInner = 0;
-
-                    ((WOLFSSL_ECH*)echX->data)->sniState = ECH_SNI_DONE;
                 }
                 if (ret == 0 && ((WOLFSSL_ECH*)echX->data)->state !=
                         ECH_PARSED_INTERNAL) {
diff --git a/tests/api.c b/tests/api.c
@@ -13696,9 +13696,14 @@ static int test_ech_server_sni_callback(WOLFSSL* ssl, int* ad, void* arg)
 
     /* reached by *_disable_conn test: expect name to be the public SNI when
      * client has ECH enabled, otherwise it should be the private SNI */
-    if (arg != NULL && *(int*)arg == 1 &&
-            XSTRCMP(name, echCbTestPublicName) == 0) {
-        return 0;
+    if (arg != NULL && *(int*)arg == 1) {
+        if (XSTRCMP(name, echCbTestPublicName) == 0) {
+            return 0;
+        }
+        else {
+            *ad = WOLFSSL_AD_UNRECOGNIZED_NAME;
+            return fatal_return;
+        }
     }
     else if (XSTRCMP(name, echCbTestPrivateName) == 0) {
         return 0;
@@ -13884,7 +13889,9 @@ static int test_wolfSSL_Tls13_ECH_all_algos(void)
     return EXPECT_RESULT();
 }
 
-/* Test ECH when no private SNI is set */
+/* Test ECH when no private SNI is set
+ * SNI is by default permissive so these should pass
+ *   (inner SNI is not required by ECH, only the outer SNI is required) */
 static int test_wolfSSL_Tls13_ECH_no_private_name(void)
 {
     EXPECT_DECLS;
@@ -13927,9 +13934,9 @@ static int test_wolfSSL_Tls13_ECH_no_private_name(void)
 
     ExpectIntNE(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), TEST_SUCCESS);
     ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.c_ssl),
-        WOLFSSL_ECH_STATUS_NOT_OFFERED);
+        WOLFSSL_ECH_STATUS_REJECTED);
     ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.s_ssl),
-        WOLFSSL_ECH_STATUS_NOT_OFFERED);
+        WOLFSSL_ECH_STATUS_REJECTED);
 
     test_ssl_memio_cleanup(&test_ctx);
 
@@ -13949,9 +13956,9 @@ static int test_wolfSSL_Tls13_ECH_no_private_name(void)
 
     ExpectIntNE(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), TEST_SUCCESS);
     ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.c_ssl),
-        WOLFSSL_ECH_STATUS_NOT_OFFERED);
+        WOLFSSL_ECH_STATUS_REJECTED);
     ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.s_ssl),
-        WOLFSSL_ECH_STATUS_NOT_OFFERED);
+        WOLFSSL_ECH_STATUS_REJECTED);
 
     test_ssl_memio_cleanup(&test_ctx);
 
@@ -14058,6 +14065,39 @@ static int test_wolfSSL_Tls13_ECH_bad_configs_ex(int hrr, int sniCb)
 
     test_ssl_memio_cleanup(&test_ctx);
 
+
+    /* verify with double public SNI */
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+
+    test_ctx.s_cb.method = wolfTLSv1_3_server_method;
+    test_ctx.c_cb.method = wolfTLSv1_3_client_method;
+
+    test_ctx.s_cb.ctx_ready = test_ech_server_ctx_ready;
+    test_ctx.s_cb.ssl_ready = test_ech_server_ssl_ready;
+
+    ExpectIntEQ(test_ssl_memio_setup(&test_ctx), TEST_SUCCESS);
+
+    /* set public SNI for private SNI on client */
+    ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, echCbTestConfigs,
+        echCbTestConfigsLen), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_UseSNI(test_ctx.c_ssl, WOLFSSL_SNI_HOST_NAME,
+        echCbTestPublicName, (word16)XSTRLEN(echCbTestPublicName)),
+            WOLFSSL_SUCCESS);
+
+    if (hrr) {
+        ExpectIntEQ(wolfSSL_NoKeyShares(test_ctx.c_ssl), WOLFSSL_SUCCESS);
+    }
+    if (sniCb) {
+        wolfSSL_CTX_set_servername_callback(test_ctx.s_ctx,
+            test_ech_server_sni_callback);
+    }
+
+    ExpectIntNE(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), TEST_SUCCESS);
+    ExpectIntEQ(test_ctx.c_ssl->options.echAccepted, 0);
+
+    test_ssl_memio_cleanup(&test_ctx);
+
     return EXPECT_RESULT();
 }
 
@@ -14608,6 +14648,132 @@ static int test_wolfSSL_Tls13_ECH_disable_conn_ex(int enableServer,
     return EXPECT_RESULT();
 }
 
+static const byte* test_find_bytes(const char* needle,
+    const byte* haystack, int hayLen)
+{
+    int needleLen = (int)XSTRLEN(needle);
+    int i;
+    if (hayLen < needleLen)
+        return NULL;
+    for (i = 0; i <= hayLen - needleLen; i++) {
+        if (XMEMCMP(haystack + i, needle, needleLen) == 0)
+            return haystack + i;
+    }
+    return NULL;
+}
+
+/* The public name must be visible and the private name must not be visible */
+static int test_wolfSSL_Tls13_ECH_wire_sni_ex(int hrr, int accept)
+{
+    EXPECT_DECLS;
+    test_ssl_memio_ctx test_ctx;
+    WOLFSSL_CTX* tempCtx = NULL;
+    byte badConfig[128];
+    word32 badConfigLen = sizeof(badConfig);
+    const char* expectedSni =
+        accept ? echCbTestPrivateName : echCbTestPublicName;
+    void* sniName = NULL;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+
+    test_ctx.s_cb.method = wolfTLSv1_3_server_method;
+    test_ctx.c_cb.method = wolfTLSv1_3_client_method;
+
+    test_ctx.s_cb.ctx_ready = test_ech_server_ctx_ready;
+    test_ctx.s_cb.ssl_ready = test_ech_server_ssl_ready;
+    /* Accept path uses the correct configs */
+    if (accept)
+        test_ctx.c_cb.ssl_ready = test_ech_client_ssl_ready;
+
+    ExpectIntEQ(test_ssl_memio_setup(&test_ctx), TEST_SUCCESS);
+
+    /* Reject path installs bad configs (with the correct public name) */
+    if (!accept) {
+        ExpectNotNull(tempCtx = wolfSSL_CTX_new(wolfTLSv1_3_server_method()));
+        ExpectIntEQ(wolfSSL_CTX_GenerateEchConfig(tempCtx, echCbTestPublicName,
+            0, 0, 0), WOLFSSL_SUCCESS);
+        ExpectIntEQ(wolfSSL_CTX_GetEchConfigs(tempCtx, badConfig,
+            &badConfigLen), WOLFSSL_SUCCESS);
+        wolfSSL_CTX_free(tempCtx);
+        ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, badConfig,
+            badConfigLen), WOLFSSL_SUCCESS);
+        ExpectIntEQ(wolfSSL_UseSNI(test_ctx.c_ssl, WOLFSSL_SNI_HOST_NAME,
+            echCbTestPrivateName, (word16)XSTRLEN(echCbTestPrivateName)),
+            WOLFSSL_SUCCESS);
+    }
+
+    if (hrr)
+        ExpectIntEQ(wolfSSL_NoKeyShares(test_ctx.c_ssl), WOLFSSL_SUCCESS);
+
+    /* On reject, client aborts with ech_required and won't send a cert. */
+    if (!accept) {
+        wolfSSL_set_verify(test_ctx.s_ssl, WOLFSSL_VERIFY_NONE, NULL);
+        wolfSSL_set_verify(test_ctx.c_ssl, WOLFSSL_VERIFY_PEER, NULL);
+    }
+
+    /* client writes CH1 into s_buff */
+    ExpectIntEQ(wolfSSL_connect(test_ctx.c_ssl), WOLFSSL_FATAL_ERROR);
+    ExpectIntEQ(wolfSSL_get_error(test_ctx.c_ssl, WOLFSSL_FATAL_ERROR),
+        WOLFSSL_ERROR_WANT_READ);
+
+    /* CH1 wire bytes */
+    ExpectNotNull(test_find_bytes(echCbTestPublicName, test_ctx.s_buff,
+        test_ctx.s_len));
+    ExpectNull(test_find_bytes(echCbTestPrivateName, test_ctx.s_buff,
+        test_ctx.s_len));
+
+    if (hrr) {
+        /* server consumes CH1 and writes HRR into c_buff */
+        ExpectIntEQ(wolfSSL_accept(test_ctx.s_ssl), WOLFSSL_FATAL_ERROR);
+        ExpectIntEQ(wolfSSL_get_error(test_ctx.s_ssl, WOLFSSL_FATAL_ERROR),
+            WOLFSSL_ERROR_WANT_READ);
+        ExpectIntEQ(test_ctx.s_ssl->options.serverState,
+            SERVER_HELLO_RETRY_REQUEST_COMPLETE);
+
+        /* client reads HRR from c_buff and writes CH2 into s_buff */
+        ExpectIntEQ(wolfSSL_connect(test_ctx.c_ssl), WOLFSSL_FATAL_ERROR);
+        ExpectIntEQ(wolfSSL_get_error(test_ctx.c_ssl, WOLFSSL_FATAL_ERROR),
+            WOLFSSL_ERROR_WANT_READ);
+
+        /* CH2 wire bytes: same property must hold */
+        ExpectNotNull(test_find_bytes(echCbTestPublicName, test_ctx.s_buff,
+            test_ctx.s_len));
+        ExpectNull(test_find_bytes(echCbTestPrivateName, test_ctx.s_buff,
+            test_ctx.s_len));
+    }
+
+    /* drive remaining rounds and verify the correct SNI is authoritative */
+    if (accept) {
+        ExpectIntEQ(test_ssl_memio_do_handshake(&test_ctx, 10, NULL),
+            TEST_SUCCESS);
+    }
+    else {
+        ExpectIntNE(test_ssl_memio_do_handshake(&test_ctx, 10, NULL),
+            TEST_SUCCESS);
+    }
+
+    ExpectIntEQ(test_ctx.c_ssl->options.echAccepted, accept ? 1 : 0);
+    wolfSSL_SNI_GetRequest(test_ctx.c_ssl, WOLFSSL_SNI_HOST_NAME, &sniName);
+    ExpectStrEQ((const char*)sniName, expectedSni);
+    sniName = NULL;
+    wolfSSL_SNI_GetRequest(test_ctx.s_ssl, WOLFSSL_SNI_HOST_NAME, &sniName);
+    ExpectStrEQ((const char*)sniName, expectedSni);
+
+    test_ssl_memio_cleanup(&test_ctx);
+
+    return EXPECT_RESULT();
+}
+
+static int test_wolfSSL_Tls13_ECH_wire_sni(void)
+{
+    EXPECT_DECLS;
+    ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(0, 0), TEST_SUCCESS);
+    ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(0, 1), TEST_SUCCESS);
+    ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(1, 0), TEST_SUCCESS);
+    ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(1, 1), TEST_SUCCESS);
+    return EXPECT_RESULT();
+}
+
 /* setup a server and client with ECH then disable on one, the other, or both.
  * Verifies that disabling ECH prevents ECH from being used and that the
  * public/private SNI's are verified correctly */
@@ -34456,6 +34622,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_wolfSSL_Tls13_ECH_new_config),
     TEST_DECL(test_wolfSSL_Tls13_ECH_trial_decrypt),
     TEST_DECL(test_wolfSSL_Tls13_ECH_GREASE),
+    TEST_DECL(test_wolfSSL_Tls13_ECH_wire_sni),
     TEST_DECL(test_wolfSSL_Tls13_ECH_disable_conn),
     TEST_DECL(test_wolfSSL_Tls13_ECH_long_SNI),
     TEST_DECL(test_wolfSSL_Tls13_ECH_HRR_rejection),
diff --git a/wolfssl/internal.h b/wolfssl/internal.h
