TLSv1.3 PSK binders: always use id protection

Removed WOLFSSL_PSK_ID_PROTECTION from use as it is now on by default.
Always check whether the server has a certificate (not a CA chain).
If there is a certificate then continue, otherwise, report a binder
error.

Added test to ensure binder error returned and alert sent when no
NO_CERT. test_tls13_bad_psk_binder already tested no certificate.

Allowed memio test harness to be built when NO_CERT is defined.

Author: SparkiDev

src/tls13.c

@@ -57,7 +57,6 @@
  * WOLFSSL_PSK_ONE_ID:       Single PSK identity per connect       default: off
  * WOLFSSL_PSK_MULTI_ID_PER_CS: Multiple PSK IDs per cipher suite default: off
  * WOLFSSL_PRIORITIZE_PSK:   Prioritize PSK over ciphersuite order default: off
- * WOLFSSL_PSK_ID_PROTECTION: Enable PSK identity protection       default: off
  *
  * TLS 1.3 Session Tickets:
  * WOLFSSL_TICKET_HAVE_ID:   Session tickets include ID            default: off
@@ -6527,20 +6526,15 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 inputSz,
     }
 
     if (current == NULL) {
-#ifdef WOLFSSL_PSK_ID_PROTECTION
     #ifndef NO_CERTS
-        if (ssl->buffers.certChainCnt != 0) {
+        if (ssl->buffers.certificate != NULL) {
             ret = 0;
             goto cleanup;
         }
     #endif
         WOLFSSL_ERROR_VERBOSE(BAD_BINDER);
         ret = BAD_BINDER;
         goto cleanup;
-#else
-        ret = 0;
-        goto cleanup;
-#endif
     }
 
     *first = (current == ext->data);
@@ -6817,14 +6811,12 @@ static int CheckPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz,
         TLSX_Remove(&ssl->extensions, TLSX_CERT_WITH_EXTERN_PSK, ssl->heap);
         ssl->options.certWithExternPsk = 0;
 #endif
-#ifdef WOLFSSL_PSK_ID_PROTECTION
     #ifndef NO_CERTS
-        if (ssl->buffers.certChainCnt != 0)
+        if (ssl->buffers.certificate != NULL)
             return 0;
     #endif
         WOLFSSL_ERROR_VERBOSE(BAD_BINDER);
         return BAD_BINDER;
-#endif
     }
 
     WOLFSSL_LEAVE("CheckPreSharedKeys", ret);

tests/api/test_tls13.c

@@ -1775,6 +1775,121 @@ int test_tls13_bad_psk_binder(void)
 }
 
 
+#if defined(WOLFSSL_TLS13) && \
+    defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_NO_CERTS) && !defined(NO_PSK)
+static unsigned int test_tls13_psk_no_cert_client_cb(WOLFSSL* ssl,
+        const char* hint, char* identity, unsigned int id_max_len,
+        unsigned char* key, unsigned int key_max_len)
+{
+    (void)ssl;
+    (void)hint;
+    (void)key_max_len;
+
+    /* Offer a PSK so the client sends a pre_shared_key extension. */
+    XSTRNCPY(identity, "Client_identity", id_max_len);
+    key[0] = 0x20;
+    return 1;
+}
+
+static unsigned int test_tls13_psk_no_cert_server_cb(WOLFSSL* ssl,
+        const char* id, unsigned char* key, unsigned int key_max_len)
+{
+    (void)ssl;
+    (void)id;
+    (void)key;
+    (void)key_max_len;
+
+    /* Reject every identity so the server finds no matching PSK. */
+    return 0;
+}
+#endif
+
+/* When no offered PSK matches and the server has no certificate to fall back
+ * to, the server must abort the handshake with BAD_BINDER rather than silently
+ * continuing. This covers both cases:
+ *   - NO_CERTS defined: the certificate fall-back branch is compiled out.
+ *   - certificates compiled in but none loaded: ssl->buffers.certificate is
+ *     NULL, so the same branch is taken at runtime.
+ * It uses the cert-less memio harness so it builds and runs in either
+ * configuration. */
+int test_tls13_psk_no_cert_bad_binder(void)
+{
+    EXPECT_DECLS;
+#if defined(WOLFSSL_TLS13) && \
+    defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_NO_CERTS) && !defined(NO_PSK)
+    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;
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+
+    /* Don't use test_memio_setup(): it loads a default server certificate,
+     * which would let the server fall back to a certificate handshake. Build
+     * the contexts by hand so the server has no certificate loaded. */
+    ExpectNotNull(ctx_c = wolfSSL_CTX_new(wolfTLSv1_3_client_method()));
+    ExpectNotNull(ctx_s = wolfSSL_CTX_new(wolfTLSv1_3_server_method()));
+    if (ctx_c != NULL) {
+        wolfSSL_SetIORecv(ctx_c, test_memio_read_cb);
+        wolfSSL_SetIOSend(ctx_c, test_memio_write_cb);
+    }
+    if (ctx_s != NULL) {
+        wolfSSL_SetIORecv(ctx_s, test_memio_read_cb);
+        wolfSSL_SetIOSend(ctx_s, test_memio_write_cb);
+    }
+
+    ExpectNotNull(ssl_c = wolfSSL_new(ctx_c));
+    ExpectNotNull(ssl_s = wolfSSL_new(ctx_s));
+    if (ssl_c != NULL) {
+        wolfSSL_SetIOWriteCtx(ssl_c, &test_ctx);
+        wolfSSL_SetIOReadCtx(ssl_c, &test_ctx);
+    }
+    if (ssl_s != NULL) {
+        wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx);
+        wolfSSL_SetIOReadCtx(ssl_s, &test_ctx);
+    }
+
+    wolfSSL_set_psk_client_callback(ssl_c, test_tls13_psk_no_cert_client_cb);
+    wolfSSL_set_psk_server_callback(ssl_s, test_tls13_psk_no_cert_server_cb);
+
+    /* Confirm the precondition: the server really has no certificate. */
+#ifndef NO_CERTS
+    if (ssl_s != NULL) {
+        ExpectNull(ssl_s->buffers.certificate);
+    }
+#endif
+
+    /* Client sends ClientHello (with PSK) and waits for the response. */
+    ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)),
+        WOLFSSL_ERROR_WANT_READ);
+
+    /* Server processes ClientHello: no PSK matches and no certificate is
+     * available, so it must abort with BAD_BINDER. */
+    ExpectIntNE(wolfSSL_accept(ssl_s), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_get_error(ssl_s, WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)),
+        WC_NO_ERR_TRACE(BAD_BINDER));
+
+    /* Client reads the server's alert: BAD_BINDER maps to a fatal
+     * illegal_parameter alert (see TranslateErrorToAlert). */
+    ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_get_error(ssl_c, WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)),
+        WC_NO_ERR_TRACE(FATAL_ERROR));
+    ExpectIntEQ(wolfSSL_get_alert_history(ssl_c, &h), WOLFSSL_SUCCESS);
+    ExpectIntEQ(h.last_rx.code, illegal_parameter);
+    ExpectIntEQ(h.last_rx.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();
+}
+
+
 #if defined(HAVE_RPK) && !defined(NO_TLS) && !defined(NO_WOLFSSL_CLIENT) && \
     !defined(NO_WOLFSSL_SERVER)
 

tests/api/test_tls13.h

@@ -61,6 +61,7 @@ int test_tls13_hrr_bad_cookie(void);
 int test_tls13_zero_inner_content_type(void);
 int test_tls13_downgrade_sentinel(void);
 int test_tls13_serverhello_bad_cipher_suites(void);
+int test_tls13_psk_no_cert_bad_binder(void);
 int test_tls13_cert_with_extern_psk_apis(void);
 int test_tls13_cert_with_extern_psk_handshake(void);
 int test_tls13_cert_with_extern_psk_requires_key_share(void);
@@ -113,6 +114,7 @@ int test_tls13_cipher_fuzz_aes128_ccm_8_sha256(void);
     TEST_DECL_GROUP("tls13", test_tls13_zero_inner_content_type), \
     TEST_DECL_GROUP("tls13", test_tls13_downgrade_sentinel), \
     TEST_DECL_GROUP("tls13", test_tls13_serverhello_bad_cipher_suites), \
+    TEST_DECL_GROUP("tls13", test_tls13_psk_no_cert_bad_binder), \
     TEST_DECL_GROUP("tls13", test_tls13_cert_with_extern_psk_apis), \
     TEST_DECL_GROUP("tls13", test_tls13_cert_with_extern_psk_handshake), \
     TEST_DECL_GROUP("tls13", test_tls13_cert_with_extern_psk_requires_key_share), \

tests/utils.c

@@ -23,7 +23,7 @@
 #include <tests/utils.h>
 #include <wolfssl/wolfcrypt/error-crypt.h>
 
-#ifdef HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES
+#ifdef HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_BUILD
 
 /* This set of memio functions allows for more fine tuned control of the TLS
  * connection operations. For new tests, try to use ssl_memio first. */
@@ -784,7 +784,7 @@ int test_memio_setup(struct test_memio_ctx *ctx,
                                method_s, NULL, 0, NULL, 0, NULL, 0);
 }
 
-#endif /* HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES */
+#endif /* HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_BUILD */
 
 #if !defined(NO_FILESYSTEM) && defined(OPENSSL_EXTRA) && \
     defined(DEBUG_UNIT_TEST_CERTS)

tests/utils.h

@@ -32,11 +32,24 @@ extern char tmpDirName[16];
 extern const char* currentTestName;
 #endif
 
-#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \
-    (!defined(NO_RSA) || defined(HAVE_RPK)) && \
-    !defined(NO_WOLFSSL_SERVER) && !defined(NO_WOLFSSL_CLIENT) && \
+/* Base dependencies for the manual memio test harness. The harness itself does
+ * not require certificate support, so cert-less tests (e.g. PSK-only) can use
+ * it through this narrower macro. */
+#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_WOLFSSL_CLIENT) && \
     (!defined(WOLFSSL_NO_TLS12) || defined(WOLFSSL_TLS13))
+#define HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_NO_CERTS
+#define HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_BUILD
+#endif
+
+/* Full dependencies: the base harness plus certificate support. Most memio
+ * tests set up a certificate-based handshake and must use this macro. */
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_NO_CERTS) && \
+    !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \
+    (!defined(NO_RSA) || defined(HAVE_RPK))
 #define HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES
+#endif
+
+#ifdef HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_BUILD
 #define TEST_MEMIO_BUF_SZ (64 * 1024)
 #define TEST_MEMIO_MAX_MSGS 32
 
@@ -85,7 +98,7 @@ int test_memio_move_message(struct test_memio_ctx *ctx, int client,
 int test_memio_drop_message(struct test_memio_ctx *ctx, int client, int msg_pos);
 int test_memio_modify_message_len(struct test_memio_ctx *ctx, int client, int msg_pos, int new_len);
 int test_memio_remove_from_buffer(struct test_memio_ctx *ctx, int client, int off, int sz);
-#endif
+#endif /* HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES_BUILD */
 
 #if !defined(NO_FILESYSTEM) && defined(OPENSSL_EXTRA) && \
     defined(DEBUG_UNIT_TEST_CERTS)