Merge pull request #10702 from Frauschi/zd21992

Various fixes

Author: JacobBarthelmeh

src/bio.c

@@ -120,6 +120,16 @@ static int wolfSSL_BIO_MEMORY_read(WOLFSSL_BIO* bio, void* buf, int len)
     WOLFSSL_ENTER("wolfSSL_BIO_MEMORY_read");
     }
 
+    /* Reject a negative length up front. Callers are expected to validate, but
+     * guarding here too prevents a negative len from defeating the signed
+     * bounds checks below (sz/memSz comparisons) and reaching XMEMCPY with a
+     * length of (size_t)-1. Return the same error the public wolfSSL_BIO_read()
+     * does for a negative length rather than silently reporting 0 bytes read. A
+     * zero length is handled correctly by the logic below (copies nothing). */
+    if (len < 0) {
+        return WOLFSSL_BIO_ERROR;
+    }
+
     sz = wolfSSL_BIO_pending(bio);
     if (sz > 0) {
         int memSz;

src/internal.c

@@ -15690,31 +15690,29 @@ PRAGMA_GCC_DIAG_POP
     ret = ParseCertRelative(args->dCert, certType, verify, SSL_CM(ssl), extraSigners);
 
 #if defined(HAVE_RPK)
-    /* if cert type has negotiated with peer, confirm the cert received has
-     * the same type.
-     */
-    if (ret == 0 ) {
-        if (ssl->options.side ==  WOLFSSL_CLIENT_END) {
-            if (ssl->options.rpkState.received_ServerCertTypeCnt == 1) {
+    /* Confirm the received certificate's form (X.509 vs raw public key) matches
+     * the type negotiated with the peer. A raw public key (RFC 7250) has no
+     * chain, so ParseCertRelative() accepts it without any trust verification;
+     * it must only be accepted when RPK was negotiated for this peer. When no
+     * type was negotiated the default is X.509 (RFC 7250/8446), so an
+     * un-negotiated bare key is rejected. The negotiated type is the received
+     * server cert type (client) or the selected client cert type (server). */
+    if (ret == 0) {
+        cType = WOLFSSL_CERT_TYPE_X509;
+        if (ssl->options.side == WOLFSSL_CLIENT_END) {
+            if (ssl->options.rpkState.received_ServerCertTypeCnt == 1)
                 cType = ssl->options.rpkState.received_ServerCertTypes[0];
-                if ((cType == WOLFSSL_CERT_TYPE_RPK && !args->dCert->isRPK) ||
-                    (cType == WOLFSSL_CERT_TYPE_X509 && args->dCert->isRPK)) {
-                    /* cert type mismatch */
-                    WOLFSSL_MSG("unsupported certificate type received");
-                    ret = UNSUPPORTED_CERTIFICATE;
-                }
-            }
         }
         else if (ssl->options.side == WOLFSSL_SERVER_END) {
-            if (ssl->options.rpkState.received_ClientCertTypeCnt == 1) {
+            if (ssl->options.rpkState.sending_ClientCertTypeCnt == 1)
                 cType = ssl->options.rpkState.sending_ClientCertTypes[0];
-                if ((cType == WOLFSSL_CERT_TYPE_RPK && !args->dCert->isRPK) ||
-                    (cType == WOLFSSL_CERT_TYPE_X509 && args->dCert->isRPK)) {
-                    /* cert type mismatch */
-                    WOLFSSL_MSG("unsupported certificate type received");
-                    ret = UNSUPPORTED_CERTIFICATE;
-                }
-            }
+        }
+
+        if ((cType == WOLFSSL_CERT_TYPE_RPK && !args->dCert->isRPK) ||
+            (cType != WOLFSSL_CERT_TYPE_RPK && args->dCert->isRPK)) {
+            /* cert type mismatch - includes an un-negotiated raw public key */
+            WOLFSSL_MSG("unsupported certificate type received");
+            ret = UNSUPPORTED_CERTIFICATE;
         }
     }
 #endif /* HAVE_RPK */

src/ssl_certman.c

@@ -3205,10 +3205,15 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify)
     }
 #ifndef ALLOW_INVALID_CERTSIGN
     else if (ret == 0 && cert->isCA == 1 && type != WOLFSSL_USER_CA &&
-        type != WOLFSSL_TEMP_CA && !cert->selfSigned &&
+        !cert->selfSigned && cert->extKeyUsageSet &&
         (cert->extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) {
-        /* Intermediate CA certs are required to have the keyCertSign
-        * extension set. User loaded root certs are not. */
+        /* Intermediate CA certs - including chain-supplied temporary CAs
+        * (WOLFSSL_TEMP_CA) added while building a path - are required to have
+        * the keyCertSign key usage when a Key Usage extension is present.
+        * Only operator-loaded root certs (WOLFSSL_USER_CA) and self-signed
+        * roots are exempt. Per RFC 5280 an absent Key Usage extension implies
+        * all usages, so only enforce this when the extension is actually
+        * present (extKeyUsageSet). */
         WOLFSSL_MSG("\tDoesn't have key usage certificate signing");
         ret = NOT_CA_ERROR;
     }

src/ssl_p7p12.c

@@ -816,6 +816,19 @@ int wolfSSL_PKCS7_verify(PKCS7* pkcs7, WOLFSSL_STACK* certs,
     if (ret != 0)
         return WOLFSSL_FAILURE;
 
+    /* Reject a degenerate (certs-only) PKCS#7 with no verified signer. Such an
+     * object has empty signerInfos, so wc_PKCS7_VerifySignedData() succeeds
+     * without authenticating the content. pkcs7.verifyCert is only set once a
+     * signer's signature has actually been verified, so a NULL value here means
+     * the content carries no valid signature and must not be reported as
+     * verified - regardless of PKCS7_NOVERIFY, which only suppresses signer
+     * certificate chain validation, not the requirement that a signature exist.
+     */
+    if (p7->pkcs7.verifyCert == NULL) {
+        WOLFSSL_MSG("PKCS7 has no verified signer (degenerate/certs-only)");
+        return WOLFSSL_FAILURE;
+    }
+
     if ((flags & PKCS7_NOVERIFY) != PKCS7_NOVERIFY) {
         /* Verify signer certificates */
         if (store == NULL || store->cm == NULL) {

src/tls13.c

@@ -11835,7 +11835,10 @@ int DoTls13Finished(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
 #endif
         if (
         #ifdef WOLFSSL_POST_HANDSHAKE_AUTH
-            !ssl->options.verifyPostHandshake &&
+            /* Exempt only the initial handshake; a pending post-handshake
+             * CertificateRequest (certReqCtx != NULL) still requires a peer
+             * certificate and a valid CertificateVerify. */
+            (!ssl->options.verifyPostHandshake || ssl->certReqCtx != NULL) &&
         #endif
             (!ssl->options.havePeerCert || !ssl->options.havePeerVerify)) {
             ret = NO_PEER_CERT; /* NO_PEER_VERIFY */
@@ -13507,7 +13510,19 @@ static int SanityCheckTls13MsgReceived(WOLFSSL* ssl, byte type)
                  */
                 if (ssl->options.verifyPeer &&
                 #ifdef WOLFSSL_POST_HANDSHAKE_AUTH
-                    !ssl->options.verifyPostHandshake &&
+                    /* The post-handshake-auth exemption is only valid during
+                     * the initial handshake. On the server, once a
+                     * post-handshake CertificateRequest is outstanding
+                     * (certReqCtx != NULL), a Certificate is required again.
+                     * Scoped to the server: certReqCtx means something
+                     * different on the client (a received request) and the
+                     * client does not process an inbound Finished in that
+                     * state. Whether an empty Certificate is then accepted
+                     * follows the verify mode (FAIL_IF_NO_PEER_CERT), exactly
+                     * as for first-handshake client authentication. */
+                    (!ssl->options.verifyPostHandshake ||
+                     (ssl->options.side == WOLFSSL_SERVER_END &&
+                      ssl->certReqCtx != NULL)) &&
                 #endif
                                            !ssl->msgsReceived.got_certificate) {
                     WOLFSSL_MSG("Finished received out of order - "

tests/api/test_ecc.c

@@ -578,6 +578,110 @@ int test_wc_ecc_shared_secret(void)
     return EXPECT_RESULT();
 } /* END tests_wc_ecc_shared_secret */
 
+#if defined(HAVE_ECC) && defined(HAVE_ECC_DHE) && !defined(WC_NO_RNG) && \
+    (defined(HAVE_ECC384) || defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES))
+/* Verify the output-buffer size contract of wc_ecc_shared_secret() at the
+ * field-size boundary. The single-precision (SP) math secret generators for
+ * P-384/P-521 historically validated the caller's buffer against the wrong
+ * length (e.g. P-521 checked 65 but writes 66), so a buffer declared one byte
+ * short of the field size slipped past the check and was overwritten. Assert
+ * that fieldSz-1 is rejected with BUFFER_E and fieldSz succeeds, for whichever
+ * math backend is built.
+ *
+ * Coverage note: this drives the blocking generators only (wc_ecc_shared_secret
+ * is synchronous). The fix also corrected the non-blocking (_nb) variants
+ * (sp_ecc_secret_gen_384_nb / _521_nb), which need WOLFSSL_SP_NONBLOCK plus the
+ * specialized SP build and are not exercised here. Of the blocking cases only
+ * P-521 (65->66) actually fails without the fix; P-384 already used 48, so its
+ * case is a guard against regression rather than a reproduction. */
+static int ecc_shared_secret_size_bound(WC_RNG* rng, int curveId, int fieldSz)
+{
+    EXPECT_DECLS;
+    ecc_key key;
+    ecc_key pub;
+    byte    out[80]; /* >= P-521 field size (66) */
+    word32  outlen;
+    int     keyInit = 0, pubInit = 0;
+    int     ret;
+
+    XMEMSET(&key, 0, sizeof(key));
+    XMEMSET(&pub, 0, sizeof(pub));
+
+    ExpectIntEQ(wc_ecc_init(&key), 0);
+    if (EXPECT_SUCCESS()) keyInit = 1;
+    ExpectIntEQ(wc_ecc_init(&pub), 0);
+    if (EXPECT_SUCCESS()) pubInit = 1;
+
+    ret = wc_ecc_make_key_ex(rng, fieldSz, &key, curveId);
+#if defined(WOLFSSL_ASYNC_CRYPT)
+    ret = wc_AsyncWait(ret, &key.asyncDev, WC_ASYNC_FLAG_NONE);
+#endif
+    ExpectIntEQ(ret, 0);
+
+    ret = wc_ecc_make_key_ex(rng, fieldSz, &pub, curveId);
+#if defined(WOLFSSL_ASYNC_CRYPT)
+    ret = wc_AsyncWait(ret, &pub.asyncDev, WC_ASYNC_FLAG_NONE);
+#endif
+    ExpectIntEQ(ret, 0);
+
+#if defined(ECC_TIMING_RESISTANT) && (!defined(HAVE_FIPS) || \
+    (!defined(HAVE_FIPS_VERSION) || (HAVE_FIPS_VERSION != 2))) && \
+    !defined(HAVE_SELFTEST)
+    ExpectIntEQ(wc_ecc_set_rng(&key, rng), 0);
+#endif
+
+    /* One byte short of the field size: must be rejected, not written past. */
+    outlen = (word32)(fieldSz - 1);
+    ExpectIntEQ(wc_ecc_shared_secret(&key, &pub, out, &outlen),
+        WC_NO_ERR_TRACE(BUFFER_E));
+
+    /* Exactly the field size: must succeed and report the field size. */
+    outlen = (word32)fieldSz;
+    ExpectIntEQ(wc_ecc_shared_secret(&key, &pub, out, &outlen), 0);
+    ExpectIntEQ(outlen, (word32)fieldSz);
+
+    if (pubInit)
+        wc_ecc_free(&pub);
+    if (keyInit)
+        wc_ecc_free(&key);
+    return EXPECT_RESULT();
+}
+#endif
+
+/*
+ * Testing wc_ecc_shared_secret() output buffer bounds at the field-size edge.
+ */
+int test_wc_ecc_shared_secret_size_bounds(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_ECC) && defined(HAVE_ECC_DHE) && !defined(WC_NO_RNG) && \
+    (defined(HAVE_ECC384) || defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES))
+    WC_RNG rng;
+    int    rngInit = 0;
+
+    XMEMSET(&rng, 0, sizeof(rng));
+    PRIVATE_KEY_UNLOCK();
+    ExpectIntEQ(wc_InitRng(&rng), 0);
+    if (EXPECT_SUCCESS())
+        rngInit = 1;
+
+#if defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES)
+    ExpectIntEQ(ecc_shared_secret_size_bound(&rng, ECC_SECP384R1, 48), 1);
+#endif
+#if defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES)
+    ExpectIntEQ(ecc_shared_secret_size_bound(&rng, ECC_SECP521R1, 66), 1);
+#endif
+
+    if (rngInit)
+        DoExpectIntEQ(wc_FreeRng(&rng), 0);
+#ifdef FP_ECC
+    wc_ecc_fp_free();
+#endif
+    PRIVATE_KEY_LOCK();
+#endif
+    return EXPECT_RESULT();
+}
+
 /*
  * testint wc_ecc_export_x963()
  */

tests/api/test_ecc.h

@@ -36,6 +36,7 @@ int test_wc_ecc_size(void);
 int test_wc_ecc_params(void);
 int test_wc_ecc_signVerify_hash(void);
 int test_wc_ecc_shared_secret(void);
+int test_wc_ecc_shared_secret_size_bounds(void);
 int test_wc_ecc_export_x963(void);
 int test_wc_ecc_export_x963_ex(void);
 int test_wc_ecc_import_x963(void);
@@ -74,6 +75,7 @@ int test_wc_EccPrivateKeyToDer(void);
     TEST_DECL_GROUP("ecc", test_wc_ecc_params),                         \
     TEST_DECL_GROUP("ecc", test_wc_ecc_signVerify_hash),                \
     TEST_DECL_GROUP("ecc", test_wc_ecc_shared_secret),                  \
+    TEST_DECL_GROUP("ecc", test_wc_ecc_shared_secret_size_bounds),      \
     TEST_DECL_GROUP("ecc", test_wc_ecc_export_x963),                    \
     TEST_DECL_GROUP("ecc", test_wc_ecc_export_x963_ex),                 \
     TEST_DECL_GROUP("ecc", test_wc_ecc_import_x963),                    \

tests/api/test_ossl_bio.c

@@ -980,6 +980,45 @@ int test_wolfSSL_BIO_write(void)
     return EXPECT_RESULT();
 }
 
+/* A negative length must never defeat the memory-BIO read bounds check and
+ * reach XMEMCPY with (size_t)-1. This exercises the public BIO_read() boundary
+ * (which rejects a negative length before dispatch); the matching guard in the
+ * static wolfSSL_BIO_MEMORY_read() sink is defense-in-depth and not separately
+ * reachable through the public API. Verify a negative length is rejected with
+ * an error without copying, a zero length reads nothing, and the pending data
+ * is left intact and still readable. */
+int test_wolfSSL_BIO_read_negative_len(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA)
+    BIO*  bio = NULL;
+    char  msg[] = "negative length test";
+    int   msgLen = (int)XSTRLEN(msg);
+    char  out[64];
+
+    ExpectNotNull(bio = BIO_new(BIO_s_mem()));
+    ExpectIntEQ(BIO_write(bio, msg, msgLen), msgLen);
+
+    /* Negative length: must be rejected with an error, not a wild copy and not
+     * a silent 0-byte read. */
+    XMEMSET(out, 0, sizeof(out));
+    ExpectIntLT(BIO_read(bio, out, -1), 0);
+    /* Data must be untouched - still all pending. */
+    ExpectIntEQ(BIO_pending(bio), msgLen);
+
+    /* Zero length: nothing read, data still pending. */
+    ExpectIntEQ(BIO_read(bio, out, 0), 0);
+    ExpectIntEQ(BIO_pending(bio), msgLen);
+
+    /* A normal read still returns the intact message. */
+    ExpectIntEQ(BIO_read(bio, out, (int)sizeof(out)), msgLen);
+    ExpectIntEQ(XMEMCMP(out, msg, msgLen), 0);
+
+    BIO_free(bio);
+#endif
+    return EXPECT_RESULT();
+}
+
 
 int test_wolfSSL_BIO_printf(void)
 {

tests/api/test_ossl_bio.h

@@ -35,6 +35,7 @@ int test_wolfSSL_BIO_datagram(void);
 int test_wolfSSL_BIO_s_null(void);
 int test_wolfSSL_BIO_accept(void);
 int test_wolfSSL_BIO_write(void);
+int test_wolfSSL_BIO_read_negative_len(void);
 int test_wolfSSL_BIO_printf(void);
 int test_wolfSSL_BIO_f_md(void);
 int test_wolfSSL_BIO_up_ref(void);
@@ -55,6 +56,7 @@ int test_wolfSSL_BIO_get_init(void);
     TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_should_retry),   \
     TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_s_null),         \
     TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_write),          \
+    TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_read_negative_len), \
     TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_printf),         \
     TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_f_md),           \
     TEST_DECL_GROUP("ossl_bio", test_wolfSSL_BIO_up_ref),         \