Merge pull request #10374 from kareem-wolfssl/zd21699

Enable all-zero shared secret check for Curve448/25519 by default.  Ensure post_handshake_auth extension was sent before accepting post-handshake CertificateRequest message.

Author: SparkiDev

.wolfssl_known_macro_extras

@@ -752,7 +752,6 @@ WOLFSSL_ECC_BLIND_K
 WOLFSSL_ECC_GEN_REJECT_SAMPLING
 WOLFSSL_ECC_NO_SMALL_STACK
 WOLFSSL_ECC_SIGALG_PARAMS_NULL_ALLOWED
-WOLFSSL_ECDHX_SHARED_NOT_ZERO
 WOLFSSL_ECDSA_MATCH_HASH
 WOLFSSL_ECDSA_SET_K_ONE_LOOP
 WOLFSSL_EC_POINT_CMP_JACOBIAN
@@ -831,6 +830,7 @@ WOLFSSL_NO_DH186
 WOLFSSL_NO_DILITHIUM_LEGACY_GATES
 WOLFSSL_NO_DILITHIUM_LEGACY_NAMES
 WOLFSSL_NO_DTLS_SIZE_CHECK
+WOLFSSL_NO_ECDHX_SHARED_ZERO_CHECK
 WOLFSSL_NO_ETM_ALERT
 WOLFSSL_NO_FENCE
 WOLFSSL_NO_ISSUERHASH_TDPEER

doc/dox_comments/header_files/ssl.h

@@ -14232,10 +14232,13 @@ int  wolfSSL_CTX_allow_post_handshake_auth(WOLFSSL_CTX* ctx);
     This is useful when connecting to a web server that has some pages that
     require client authentication and others that don't.
 
+    This function must be called before wolfSSL_connect() on the WOLFSSL object.
+
     \param [in,out] ssl a pointer to a WOLFSSL structure, created using wolfSSL_new().
 
     \return BAD_FUNC_ARG if ssl is NULL or not using TLS v1.3.
     \return SIDE_ERROR if called with a server.
+    \return BAD_STATE_E if called after the handshake has started.
     \return 0 if successful.
 
     _Example_

src/tls13.c

@@ -13189,6 +13189,21 @@ static int SanityCheckTls13MsgReceived(WOLFSSL* ssl, byte type)
                 WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
                 return OUT_OF_ORDER_E;
             }
+            /* RFC 8446 4.6.2: A client that receives a post-handshake
+             * CertificateRequest message without having sent the
+             * "post_handshake_auth" extension MUST send an
+             * "unexpected_message" fatal alert. wolfSSL_allow_post_handshake_auth()
+             * must be called before wolfSSL_connect() so postHandshakeAuth
+             * reflects whether the extension was offered. */
+            if (ssl->options.serverState >= SERVER_FINISHED_COMPLETE &&
+                ssl->options.clientState == CLIENT_FINISHED_COMPLETE &&
+                !ssl->options.postHandshakeAuth) {
+                WOLFSSL_MSG("Post-handshake CertificateRequest received "
+                            "without having sent post_handshake_auth "
+                            "extension");
+                WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
+                return OUT_OF_ORDER_E;
+            }
         #endif
         #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK)
             /* Server's authenticating with PSK must not send this. */
@@ -14889,14 +14904,20 @@ int wolfSSL_CTX_allow_post_handshake_auth(WOLFSSL_CTX* ctx)
  *
  * ssl  The SSL/TLS object.
  * returns BAD_FUNC_ARG when ssl is NULL, or not using TLS v1.3,
- * SIDE_ERROR when not a client and 0 on success.
+ * SIDE_ERROR when not a client, BAD_STATE_E when called after the handshake
+ * has started, and 0 on success.
+ *
+ * Must be called before wolfSSL_connect() so the post_handshake_auth
+ * extension can be included in the ClientHello.
  */
 int wolfSSL_allow_post_handshake_auth(WOLFSSL* ssl)
 {
     if (ssl == NULL || !IsAtLeastTLSv1_3(ssl->version))
         return BAD_FUNC_ARG;
     if (ssl->options.side == WOLFSSL_SERVER_END)
         return SIDE_ERROR;
+    if (ssl->options.handShakeState != NULL_STATE)
+        return BAD_STATE_E;
 
     ssl->options.postHandshakeAuth = 1;
 

tests/api/test_curve25519.c

@@ -353,6 +353,51 @@ int test_wc_curve25519_shared_secret_ex(void)
     return EXPECT_RESULT();
 } /* END test_wc_curve25519_shared_secret_ex */
 
+/*
+ * Testing that wc_curve25519_shared_secret_ex rejects an all-zero shared
+ * secret (RFC 7748 section 6.1). This is the default behavior; users that
+ * need the legacy behavior can opt out with WOLFSSL_NO_ECDHX_SHARED_ZERO_CHECK.
+ */
+int test_wc_curve25519_shared_secret_zero_check(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_CURVE25519) && defined(HAVE_CURVE25519_KEY_IMPORT) && \
+    !defined(WOLFSSL_NO_ECDHX_SHARED_ZERO_CHECK)
+    curve25519_key private_key;
+    curve25519_key public_key;
+    WC_RNG         rng;
+    byte           out[CURVE25519_KEYSIZE];
+    word32         outLen = sizeof(out);
+    /* All-zero public key is a low-order point that yields an all-zero
+     * shared secret for any private key. */
+    byte           zero_pub[CURVE25519_KEYSIZE];
+
+    XMEMSET(&rng, 0, sizeof(WC_RNG));
+    XMEMSET(zero_pub, 0, sizeof(zero_pub));
+
+    ExpectIntEQ(wc_curve25519_init(&private_key), 0);
+    ExpectIntEQ(wc_curve25519_init(&public_key), 0);
+    ExpectIntEQ(wc_InitRng(&rng), 0);
+#ifdef WOLFSSL_CURVE25519_BLINDING
+    ExpectIntEQ(wc_curve25519_set_rng(&private_key, &rng), 0);
+#endif
+
+    ExpectIntEQ(wc_curve25519_make_key(&rng, CURVE25519_KEYSIZE, &private_key),
+        0);
+    ExpectIntEQ(wc_curve25519_import_public_ex(zero_pub, sizeof(zero_pub),
+        &public_key, EC25519_LITTLE_ENDIAN), 0);
+
+    ExpectIntEQ(wc_curve25519_shared_secret_ex(&private_key, &public_key, out,
+        &outLen, EC25519_BIG_ENDIAN),
+        WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
+
+    DoExpectIntEQ(wc_FreeRng(&rng), 0);
+    wc_curve25519_free(&private_key);
+    wc_curve25519_free(&public_key);
+#endif
+    return EXPECT_RESULT();
+} /* END test_wc_curve25519_shared_secret_zero_check */
+
 /*
  * Testing wc_curve25519_make_pub
  */

tests/api/test_curve25519.h

@@ -30,6 +30,7 @@ int test_wc_curve25519_export_key_raw(void);
 int test_wc_curve25519_export_key_raw_ex(void);
 int test_wc_curve25519_make_key(void);
 int test_wc_curve25519_shared_secret_ex(void);
+int test_wc_curve25519_shared_secret_zero_check(void);
 int test_wc_curve25519_make_pub(void);
 int test_wc_curve25519_export_public_ex(void);
 int test_wc_curve25519_export_private_raw_ex(void);
@@ -45,6 +46,7 @@ int test_wc_Curve25519KeyToDer_oneasymkey_version(void);
     TEST_DECL_GROUP("curve25519", test_wc_curve25519_export_key_raw_ex),       \
     TEST_DECL_GROUP("curve25519", test_wc_curve25519_make_key),                \
     TEST_DECL_GROUP("curve25519", test_wc_curve25519_shared_secret_ex),        \
+    TEST_DECL_GROUP("curve25519", test_wc_curve25519_shared_secret_zero_check),\
     TEST_DECL_GROUP("curve25519", test_wc_curve25519_make_pub),                \
     TEST_DECL_GROUP("curve25519", test_wc_curve25519_export_public_ex),        \
     TEST_DECL_GROUP("curve25519", test_wc_curve25519_export_private_raw_ex),   \

tests/api/test_curve448.c

@@ -116,6 +116,48 @@ int test_wc_curve448_shared_secret_ex(void)
     return EXPECT_RESULT();
 } /* END test_wc_curve448_shared_secret_ex */
 
+/*
+ * Testing that wc_curve448_shared_secret_ex rejects an all-zero shared
+ * secret (RFC 7748 section 6.2). This is the default behavior; users that
+ * need the legacy behavior can opt out with WOLFSSL_NO_ECDHX_SHARED_ZERO_CHECK.
+ */
+int test_wc_curve448_shared_secret_zero_check(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_CURVE448) && defined(HAVE_CURVE448_KEY_IMPORT) && \
+    defined(HAVE_CURVE448_SHARED_SECRET) && \
+    !defined(WOLFSSL_NO_ECDHX_SHARED_ZERO_CHECK)
+    curve448_key private_key;
+    curve448_key public_key;
+    WC_RNG       rng;
+    byte         out[CURVE448_KEY_SIZE];
+    word32       outLen = sizeof(out);
+    /* All-zero public key is a low-order point that yields an all-zero
+     * shared secret for any private key. */
+    byte         zero_pub[CURVE448_PUB_KEY_SIZE];
+
+    XMEMSET(&rng, 0, sizeof(WC_RNG));
+    XMEMSET(zero_pub, 0, sizeof(zero_pub));
+
+    ExpectIntEQ(wc_curve448_init(&private_key), 0);
+    ExpectIntEQ(wc_curve448_init(&public_key), 0);
+    ExpectIntEQ(wc_InitRng(&rng), 0);
+
+    ExpectIntEQ(wc_curve448_make_key(&rng, CURVE448_KEY_SIZE, &private_key), 0);
+    ExpectIntEQ(wc_curve448_import_public_ex(zero_pub, sizeof(zero_pub),
+        &public_key, EC448_LITTLE_ENDIAN), 0);
+
+    ExpectIntEQ(wc_curve448_shared_secret_ex(&private_key, &public_key, out,
+        &outLen, EC448_BIG_ENDIAN),
+        WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
+
+    DoExpectIntEQ(wc_FreeRng(&rng), 0);
+    wc_curve448_free(&private_key);
+    wc_curve448_free(&public_key);
+#endif
+    return EXPECT_RESULT();
+} /* END test_wc_curve448_shared_secret_zero_check */
+
 /*
  * Testing test_wc_curve448_export_public_ex
  */

tests/api/test_curve448.h

@@ -26,6 +26,7 @@
 
 int test_wc_curve448_make_key(void);
 int test_wc_curve448_shared_secret_ex(void);
+int test_wc_curve448_shared_secret_zero_check(void);
 int test_wc_curve448_export_public_ex(void);
 int test_wc_curve448_export_private_raw_ex(void);
 int test_wc_curve448_export_key_raw(void);
@@ -39,6 +40,7 @@ int test_wc_Curve448PrivateKeyToDer_oneasymkey_version(void);
 #define TEST_CURVE448_DECLS                                                 \
     TEST_DECL_GROUP("curve448", test_wc_curve448_make_key),                 \
     TEST_DECL_GROUP("curve448", test_wc_curve448_shared_secret_ex),         \
+    TEST_DECL_GROUP("curve448", test_wc_curve448_shared_secret_zero_check), \
     TEST_DECL_GROUP("curve448", test_wc_curve448_export_public_ex),         \
     TEST_DECL_GROUP("curve448", test_wc_curve448_export_private_raw_ex),    \
     TEST_DECL_GROUP("curve448", test_wc_curve448_export_key_raw),           \

tests/api/test_tls13.c

@@ -5575,6 +5575,127 @@ int test_tls13_zero_inner_content_type(void)
     return EXPECT_RESULT();
 }
 
+/* RFC 8446 Section 4.6.2: A client that receives a post-handshake
+ * CertificateRequest message without having sent the "post_handshake_auth"
+ * extension MUST send an "unexpected_message" fatal alert.
+ *
+ * This test completes a TLS 1.3 handshake in which the client never enabled
+ * post-handshake auth (so no extension was sent in the ClientHello), then
+ * forces the server to transmit a post-handshake CertificateRequest anyway.
+ * The client must reject the message with an unexpected_message fatal
+ * alert. */
+int test_tls13_post_handshake_auth_no_ext(void)
+{
+    EXPECT_DECLS;
+#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_POST_HANDSHAKE_AUTH) && \
+    defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
+    WOLFSSL_CTX *ctx_c = NULL;
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_c = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+    WOLFSSL_ALERT_HISTORY h;
+    char readBuf[8];
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    XMEMSET(&h, 0, sizeof(h));
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+        wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
+    /* Keep the post-handshake byte stream simple by suppressing the server's
+     * NewSessionTicket so the only post-handshake record from the server is
+     * the CertificateRequest under test. */
+    ExpectIntEQ(wolfSSL_no_ticket_TLSv13(ssl_s), 0);
+
+    /* Intentionally do NOT call wolfSSL_allow_post_handshake_auth() on the
+     * client so the post_handshake_auth extension is omitted from the
+     * ClientHello. */
+    ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+
+    /* The server's wolfSSL_request_certificate() refuses to send a
+     * post-handshake CertificateRequest unless its postHandshakeAuth flag is
+     * set (normally set when parsing the client's extension). To exercise
+     * the receive-side check we are testing, simulate a server that sends
+     * one anyway by toggling the flag directly. */
+    if (ssl_s != NULL)
+        ssl_s->options.postHandshakeAuth = 1;
+    /* OPENSSL_COMPATIBLE_DEFAULTS may leave groupMessages set on ssl_s; that
+     * suppresses SendBuffered() for the post-handshake CertificateRequest. */
+    ExpectIntEQ(wolfSSL_clear_group_messages(ssl_s), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_request_certificate(ssl_s), WOLFSSL_SUCCESS);
+    ExpectIntGT(test_ctx.c_len, 0);
+
+    /* The client must reject the unsolicited CertificateRequest. */
+    ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, (int)sizeof(readBuf)),
+        WOLFSSL_FATAL_ERROR);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
+        WC_NO_ERR_TRACE(OUT_OF_ORDER_E));
+
+    /* And the client must transmit a fatal unexpected_message alert. */
+    ExpectIntEQ(wolfSSL_get_alert_history(ssl_c, &h), WOLFSSL_SUCCESS);
+    ExpectIntEQ(h.last_tx.code, unexpected_message);
+    ExpectIntEQ(h.last_tx.level, alert_fatal);
+
+    wolfSSL_free(ssl_c);
+    wolfSSL_CTX_free(ctx_c);
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
+/* wolfSSL_allow_post_handshake_auth() must be called before the handshake
+ * starts. A late call must fail with BAD_STATE_E and must not enable
+ * post-handshake authentication for the connection. */
+int test_tls13_post_handshake_auth_late_allow(void)
+{
+    EXPECT_DECLS;
+#if defined(WOLFSSL_TLS13) && defined(WOLFSSL_POST_HANDSHAKE_AUTH) && \
+    defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
+    !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
+    WOLFSSL_CTX *ctx_c = NULL;
+    WOLFSSL_CTX *ctx_s = NULL;
+    WOLFSSL *ssl_c = NULL;
+    WOLFSSL *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+    WOLFSSL_ALERT_HISTORY h;
+    char readBuf[8];
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    XMEMSET(&h, 0, sizeof(h));
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+        wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
+    ExpectIntEQ(wolfSSL_no_ticket_TLSv13(ssl_s), 0);
+    ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
+
+    /* Too late: handshake is complete. */
+    ExpectIntEQ(wolfSSL_allow_post_handshake_auth(ssl_c),
+        WC_NO_ERR_TRACE(BAD_STATE_E));
+
+    if (ssl_s != NULL)
+        ssl_s->options.postHandshakeAuth = 1;
+    /* OPENSSL_COMPATIBLE_DEFAULTS may leave groupMessages set on ssl_s; that
+     * suppresses SendBuffered() for the post-handshake CertificateRequest. */
+    ExpectIntEQ(wolfSSL_clear_group_messages(ssl_s), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_request_certificate(ssl_s), WOLFSSL_SUCCESS);
+    ExpectIntGT(test_ctx.c_len, 0);
+
+    ExpectIntEQ(wolfSSL_read(ssl_c, readBuf, (int)sizeof(readBuf)),
+        WOLFSSL_FATAL_ERROR);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
+        WC_NO_ERR_TRACE(OUT_OF_ORDER_E));
+    ExpectIntEQ(wolfSSL_get_alert_history(ssl_c, &h), WOLFSSL_SUCCESS);
+    ExpectIntEQ(h.last_tx.code, unexpected_message);
+    ExpectIntEQ(h.last_tx.level, alert_fatal);
+
+    wolfSSL_free(ssl_c);
+    wolfSSL_CTX_free(ctx_c);
+    wolfSSL_free(ssl_s);
+    wolfSSL_CTX_free(ctx_s);
+#endif
+    return EXPECT_RESULT();
+}
+
 /* Test that a TLS 1.3-capable client rejects downgrade sentinels in a
  * downgraded ServerHello random for both TLS 1.2 and TLS 1.1-or-lower. */
 int test_tls13_downgrade_sentinel(void)

tests/api/test_tls13.h

@@ -59,6 +59,8 @@ int test_tls13_corrupted_finished(void);
 int test_tls13_peerauth_failsafe(void);
 int test_tls13_hrr_bad_cookie(void);
 int test_tls13_zero_inner_content_type(void);
+int test_tls13_post_handshake_auth_no_ext(void);
+int test_tls13_post_handshake_auth_late_allow(void);
 int test_tls13_downgrade_sentinel(void);
 int test_tls13_serverhello_bad_cipher_suites(void);
 int test_tls13_cert_with_extern_psk_apis(void);
@@ -111,6 +113,8 @@ int test_tls13_cipher_fuzz_aes128_ccm_8_sha256(void);
     TEST_DECL_GROUP("tls13", test_tls13_peerauth_failsafe),    \
     TEST_DECL_GROUP("tls13", test_tls13_hrr_bad_cookie), \
     TEST_DECL_GROUP("tls13", test_tls13_zero_inner_content_type), \
+    TEST_DECL_GROUP("tls13", test_tls13_post_handshake_auth_no_ext), \
+    TEST_DECL_GROUP("tls13", test_tls13_post_handshake_auth_late_allow), \
     TEST_DECL_GROUP("tls13", test_tls13_downgrade_sentinel), \
     TEST_DECL_GROUP("tls13", test_tls13_serverhello_bad_cipher_suites), \
     TEST_DECL_GROUP("tls13", test_tls13_cert_with_extern_psk_apis), \