diff --git a/.github/configs/os-check-linux.json b/.github/configs/os-check-linux.json index 23dfc3a4e2..35f4d15510 100644 --- a/.github/configs/os-check-linux.json +++ b/.github/configs/os-check-linux.json @@ -16,6 +16,9 @@ "configure": ["--enable-all", "--enable-asn=original"]}, {"name": "all-certgencache", "minutes": 8.3, "configure": ["--enable-all", "--enable-certgencache"]}, +{"name": "all-certsigncb", "minutes": 8.3, + "comment": "Only --enable-certsigncb sets WOLFSSL_CERT_SIGN_CB, so without this entry wc_SignCert_cb() and its test are never compiled anywhere in CI. Minutes is seeded from the sibling --enable-all entries; refresh from a run's step summary.", + "configure": ["--enable-all", "--enable-certsigncb"]}, {"name": "all-dtls13-frag-ch-no-mlkem", "minutes": 8.2, "configure": ["--enable-all", "--enable-dtls13", "--enable-dtls-frag-ch", "--disable-mlkem"]}, diff --git a/tests/api.c b/tests/api.c index d13c0ff014..0694450747 100644 --- a/tests/api.c +++ b/tests/api.c @@ -25740,11 +25740,14 @@ static int mockSignCbError(const byte* in, word32 inLen, byte* out, } #endif -#ifdef WOLFSSL_CERT_SIGN_CB +/* Bytes kept past the advertised capacity to catch a write past the end. */ +#define SIGN_CERT_CB_GUARD_SZ 16 + static int test_wc_SignCert_cb(void) { EXPECT_DECLS; -#if defined(WOLFSSL_CERT_GEN) && !defined(NO_ASN_TIME) +#if defined(WOLFSSL_CERT_SIGN_CB) && defined(WOLFSSL_CERT_GEN) && \ + !defined(NO_ASN_TIME) #ifdef HAVE_ECC /* Test with ECC key */ @@ -25851,6 +25854,10 @@ static int test_wc_SignCert_cb(void) MockSignCtx signCtx; DecodedCert decodedCert; int ret; + int bodySz = 0; + int exactSz = 0; + int i; + static const byte fixedSerial[] = { 0x01, 0x02, 0x03, 0x04 }; XMEMSET(&rng, 0, sizeof(WC_RNG)); XMEMSET(&key, 0, sizeof(RsaKey)); @@ -25913,6 +25920,45 @@ static int test_wc_SignCert_cb(void) /* Callback returning error */ ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der, FOURK_BUF, RSA_TYPE, mockSignCbError, &signCtx, &rng), BAD_STATE_E); + + /* Buffer bounds. wc_SignCert_cb() appends the signatureAlgorithm and + * signatureValue as well as the outer SEQUENCE, so a capacity below + * the exact encoding size has to be rejected rather than overrun. The + * serial is fixed so both rebuilt bodies encode identically. */ + XMEMCPY(cert.serial, fixedSerial, sizeof(fixedSerial)); + cert.serialSz = (int)sizeof(fixedSerial); + ExpectIntGT(bodySz = wc_MakeCert(&cert, der, FOURK_BUF, &key, NULL, + &rng), 0); + ExpectIntGT(exactSz = wc_SignCert_cb(bodySz, cert.sigType, der, + FOURK_BUF, RSA_TYPE, mockSignCb, &signCtx, &rng), 0); + /* The guard region below is written past the advertised capacity, so + * it has to stay inside der. */ + ExpectIntLT(exactSz + SIGN_CERT_CB_GUARD_SZ, FOURK_BUF); + + ExpectIntGT(bodySz = wc_MakeCert(&cert, der, FOURK_BUF, &key, NULL, + &rng), 0); + if (EXPECT_SUCCESS()) { + XMEMSET(der + exactSz - 1, 0xA5, SIGN_CERT_CB_GUARD_SZ); + } + ExpectIntEQ(wc_SignCert_cb(bodySz, cert.sigType, der, + (word32)(exactSz - 1), RSA_TYPE, mockSignCb, &signCtx, &rng), + WC_NO_ERR_TRACE(BUFFER_E)); + for (i = 0; i < SIGN_CERT_CB_GUARD_SZ; i++) { + ExpectIntEQ(der[exactSz - 1 + i], 0xA5); + } + + /* The check must not be over-conservative either, so require the exact + * size to be accepted and the bytes above it left alone. */ + ExpectIntGT(bodySz = wc_MakeCert(&cert, der, FOURK_BUF, &key, NULL, + &rng), 0); + if (EXPECT_SUCCESS()) { + XMEMSET(der + exactSz, 0xA5, SIGN_CERT_CB_GUARD_SZ); + } + ExpectIntEQ(wc_SignCert_cb(bodySz, cert.sigType, der, + (word32)exactSz, RSA_TYPE, mockSignCb, &signCtx, &rng), exactSz); + for (i = 0; i < SIGN_CERT_CB_GUARD_SZ; i++) { + ExpectIntEQ(der[exactSz + i], 0xA5); + } #ifdef HAVE_ECC /* Invalid keyType */ ExpectIntEQ(wc_SignCert_cb(cert.bodySz, cert.sigType, der, @@ -25931,10 +25977,11 @@ static int test_wc_SignCert_cb(void) } #endif /* !NO_RSA && WOLFSSL_KEY_GEN */ -#endif /* WOLFSSL_CERT_GEN && !NO_ASN_TIME */ +#endif /* WOLFSSL_CERT_SIGN_CB && WOLFSSL_CERT_GEN && !NO_ASN_TIME */ return EXPECT_RESULT(); } -#endif /* WOLFSSL_CERT_SIGN_CB */ + +#undef SIGN_CERT_CB_GUARD_SZ static int test_ERR_load_crypto_strings(void) { @@ -38766,9 +38813,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_NameConstraints_SubtreeMinMax), TEST_DECL(test_ParseSerial0FixtureMatrix), TEST_DECL(test_MakeCertWithCaFalse), -#ifdef WOLFSSL_CERT_SIGN_CB TEST_DECL(test_wc_SignCert_cb), -#endif TEST_DECL(test_wc_SetAcmeIdentifierExt), TEST_DECL(test_wc_SetKeyUsage), TEST_DECL(test_wc_SetAuthKeyIdFromPublicKey_ex), diff --git a/tests/api/test_asn.c b/tests/api/test_asn.c index c42124a458..33ad775ad1 100644 --- a/tests/api/test_asn.c +++ b/tests/api/test_asn.c @@ -2518,6 +2518,276 @@ int test_ToTraditional_ex_mldsa_bad_params(void) return EXPECT_RESULT(); } +/* What wc_SignCert() bounds testing needs regardless of the signing algorithm. + * Each algorithm then gates on its own key type and certificate buffers, so an + * RSA-less build still gets the ECDSA sweep and vice versa. */ +#if defined(WOLFSSL_CERT_GEN) && !defined(NO_SHA256) && !defined(WC_NO_RNG) && \ + !defined(NO_ASN_TIME) && !defined(NO_ASN_CRYPT) + #if !defined(NO_RSA) && defined(USE_CERT_BUFFERS_2048) + #define TEST_SIGN_CERT_BOUNDS_RSA + #endif + #if defined(HAVE_ECC) && defined(USE_CERT_BUFFERS_256) + #define TEST_SIGN_CERT_BOUNDS_ECC + #endif +#endif + +#if defined(TEST_SIGN_CERT_BOUNDS_RSA) || defined(TEST_SIGN_CERT_BOUNDS_ECC) + +#define SIGN_CERT_SCRATCH_SZ 4096 +/* Number of capacities below the exact encoding size to try. Has to be wider + * than the AlgorithmIdentifier plus BIT STRING header that a sequence-headers + * only estimate leaves out. */ +#define SIGN_CERT_BAND_SZ 24 +/* Bytes kept past the advertised capacity to catch a write past the end. */ +#define SIGN_CERT_GUARD_SZ 32 +#define SIGN_CERT_GUARD_BYTE 0xA5 + +#ifdef TEST_SIGN_CERT_BOUNDS_RSA +/* Build the certificate body used by test_wc_SignCert_buffer_bounds(). + * wc_SignCert() rewrites the buffer in place, so it has to be rebuilt for + * every capacity tried. Returns the body size or a negative error. + * + * The serial is fixed rather than left for wc_MakeCert() to generate. A + * generated serial is random, and GenerateInteger() does not shrink its length + * after dropping leading zero bytes, so the promoted byte can carry the MSB and + * make the encoder pad the INTEGER with an extra 0x00. That changes the body + * size for about one certificate in 250, which would make the swept capacities + * below disagree with the reference size. */ +static int test_wc_SignCert_makeBody(Cert* cert, RsaKey* key, WC_RNG* rng, + byte* out, word32 outSz) +{ + static const byte serial[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08 }; + int ret; + + ret = wc_InitCert(cert); + if (ret != 0) + return ret; + + cert->sigType = CTC_SHA256wRSA; + cert->isCA = 0; + XMEMCPY(cert->serial, serial, sizeof(serial)); + cert->serialSz = (int)sizeof(serial); + XSTRNCPY(cert->subject.country, "US", CTC_NAME_SIZE); + XSTRNCPY(cert->subject.state, "MT", CTC_NAME_SIZE); + XSTRNCPY(cert->subject.org, "wolfSSL", CTC_NAME_SIZE); + XSTRNCPY(cert->subject.commonName, "signcert-bounds", CTC_NAME_SIZE); + + return wc_MakeCert(cert, out, outSz, key, NULL, rng); +} +#endif /* TEST_SIGN_CERT_BOUNDS_RSA */ + +#ifdef TEST_SIGN_CERT_BOUNDS_ECC +/* ECDSA r and s are DER INTEGERs whose length changes with the leading zero + * bytes of each new signature, so an ECDSA encoding size measured once does not + * repeat. The sweep copes with that by asserting an invariant that holds for + * either outcome rather than a fixed return, so only the accept case needs a + * capacity clear of anything the jitter can reach. */ +#define SIGN_CERT_ECC_SLACK 8 + +/* ECDSA counterpart of test_wc_SignCert_makeBody(). */ +static int test_wc_SignCert_makeBodyEcc(Cert* cert, ecc_key* key, WC_RNG* rng, + byte* out, word32 outSz) +{ + static const byte serial[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08 }; + int ret; + + ret = wc_InitCert(cert); + if (ret != 0) + return ret; + + cert->sigType = CTC_SHA256wECDSA; + cert->isCA = 0; + XMEMCPY(cert->serial, serial, sizeof(serial)); + cert->serialSz = (int)sizeof(serial); + XSTRNCPY(cert->subject.country, "US", CTC_NAME_SIZE); + XSTRNCPY(cert->subject.state, "MT", CTC_NAME_SIZE); + XSTRNCPY(cert->subject.org, "wolfSSL", CTC_NAME_SIZE); + XSTRNCPY(cert->subject.commonName, "signcert-bounds-ecc", CTC_NAME_SIZE); + + return wc_MakeCert(cert, out, outSz, NULL, key, rng); +} +#endif /* TEST_SIGN_CERT_BOUNDS_ECC */ +#endif /* TEST_SIGN_CERT_BOUNDS_RSA || TEST_SIGN_CERT_BOUNDS_ECC */ + +/* + * wc_SignCert() must never write past the capacity it was given. + * + * SignCert() hands the buffer to AddSignature(), which appends the + * signatureAlgorithm AlgorithmIdentifier and the signatureValue BIT STRING as + * well as wrapping everything in the outer SEQUENCE. A size check that only + * accounts for sequence headers under-counts by the algorithm identifier and + * bit string header, so capacities in a narrow band just below the exact + * encoding size get accepted and overrun. + */ +int test_wc_SignCert_buffer_bounds(void) +{ + EXPECT_DECLS; +#if defined(TEST_SIGN_CERT_BOUNDS_RSA) || defined(TEST_SIGN_CERT_BOUNDS_ECC) + WC_RNG rng; + Cert cert; + byte* scratch = NULL; + byte* buf = NULL; + int rngInit = 0; + int bodySz = 0; + int cap; + int i; +#ifdef TEST_SIGN_CERT_BOUNDS_RSA + RsaKey key; + word32 idx = 0; + int keyInit = 0; + int exactSz = 0; +#endif +#ifdef TEST_SIGN_CERT_BOUNDS_ECC + ecc_key eccKey; + word32 eccIdx = 0; + int eccInit = 0; + int eccExactSz = 0; + int eccSignedSz = 0; + int k; +#endif + + XMEMSET(&rng, 0, sizeof(rng)); + XMEMSET(&cert, 0, sizeof(cert)); + + ExpectIntEQ(wc_InitRng(&rng), 0); + if (EXPECT_SUCCESS()) rngInit = 1; + + ExpectNotNull(scratch = (byte*)XMALLOC(SIGN_CERT_SCRATCH_SZ, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectNotNull(buf = (byte*)XMALLOC(SIGN_CERT_SCRATCH_SZ, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + +#ifdef TEST_SIGN_CERT_BOUNDS_RSA + XMEMSET(&key, 0, sizeof(key)); + ExpectIntEQ(wc_InitRsaKey_ex(&key, HEAP_HINT, testDevId), 0); + if (EXPECT_SUCCESS()) keyInit = 1; + ExpectIntEQ(wc_RsaPrivateKeyDecode(server_key_der_2048, &idx, &key, + sizeof_server_key_der_2048), 0); + + /* Sign into a roomy buffer once to learn the exact encoding size. */ + ExpectIntGT(bodySz = test_wc_SignCert_makeBody(&cert, &key, &rng, scratch, + SIGN_CERT_SCRATCH_SZ), 0); + ExpectIntGT(exactSz = wc_SignCert(bodySz, cert.sigType, scratch, + SIGN_CERT_SCRATCH_SZ, &key, NULL, &rng), 0); + ExpectIntLT(exactSz + SIGN_CERT_GUARD_SZ, SIGN_CERT_SCRATCH_SZ); + + /* Every capacity below the exact size has to be rejected, and rejected + * without touching a byte beyond it. */ + for (cap = exactSz - SIGN_CERT_BAND_SZ; cap < exactSz; cap++) { + if (!EXPECT_SUCCESS()) break; + + ExpectIntGT(bodySz = test_wc_SignCert_makeBody(&cert, &key, &rng, + scratch, SIGN_CERT_SCRATCH_SZ), 0); + if (!EXPECT_SUCCESS()) break; + + XMEMCPY(buf, scratch, (size_t)bodySz); + XMEMSET(buf + cap, SIGN_CERT_GUARD_BYTE, SIGN_CERT_GUARD_SZ); + + ExpectIntEQ(wc_SignCert(bodySz, cert.sigType, buf, (word32)cap, &key, + NULL, &rng), WC_NO_ERR_TRACE(BUFFER_E)); + + for (i = 0; i < SIGN_CERT_GUARD_SZ; i++) { + ExpectIntEQ(buf[cap + i], SIGN_CERT_GUARD_BYTE); + } + } + + /* The exact size still has to be accepted - the check must not be made + * conservative instead of correct. */ + ExpectIntGT(bodySz = test_wc_SignCert_makeBody(&cert, &key, &rng, scratch, + SIGN_CERT_SCRATCH_SZ), 0); + if (EXPECT_SUCCESS() && (buf != NULL)) { + XMEMCPY(buf, scratch, (size_t)bodySz); + XMEMSET(buf + exactSz, SIGN_CERT_GUARD_BYTE, SIGN_CERT_GUARD_SZ); + } + ExpectIntEQ(wc_SignCert(bodySz, cert.sigType, buf, (word32)exactSz, &key, + NULL, &rng), exactSz); + for (i = 0; i < SIGN_CERT_GUARD_SZ; i++) { + ExpectIntEQ(buf[exactSz + i], SIGN_CERT_GUARD_BYTE); + } +#endif /* TEST_SIGN_CERT_BOUNDS_RSA */ + +#ifdef TEST_SIGN_CERT_BOUNDS_ECC + /* Same sweep for ECDSA. IsSigAlgoNoParams() drops the NULL parameters from + * the AlgorithmIdentifier, so the under-count an estimate makes has a + * different width here than it does for RSA. */ + XMEMSET(&eccKey, 0, sizeof(eccKey)); + ExpectIntEQ(wc_ecc_init_ex(&eccKey, HEAP_HINT, testDevId), 0); + if (EXPECT_SUCCESS()) eccInit = 1; + ExpectIntEQ(wc_EccPrivateKeyDecode(ecc_key_der_256, &eccIdx, &eccKey, + sizeof_ecc_key_der_256), 0); + + for (k = 1; k < SIGN_CERT_BAND_SZ; k++) { + if (!EXPECT_SUCCESS()) break; + + /* Measured fresh every iteration: the previous signature's length + * says nothing about the next one's. */ + ExpectIntGT(bodySz = test_wc_SignCert_makeBodyEcc(&cert, &eccKey, &rng, + scratch, SIGN_CERT_SCRATCH_SZ), 0); + ExpectIntGT(eccExactSz = wc_SignCert(bodySz, cert.sigType, scratch, + SIGN_CERT_SCRATCH_SZ, NULL, &eccKey, &rng), 0); + if (!EXPECT_SUCCESS()) break; + + cap = eccExactSz - k; + ExpectIntGT(bodySz = test_wc_SignCert_makeBodyEcc(&cert, &eccKey, &rng, + scratch, SIGN_CERT_SCRATCH_SZ), 0); + if (!EXPECT_SUCCESS()) break; + + XMEMCPY(buf, scratch, (size_t)bodySz); + XMEMSET(buf + cap, SIGN_CERT_GUARD_BYTE, SIGN_CERT_GUARD_SZ); + + eccSignedSz = wc_SignCert(bodySz, cert.sigType, buf, (word32)cap, NULL, + &eccKey, &rng); + /* The signature this call produces is not the one the capacity was + * derived from, so a capacity below the measured size is not always + * too small. Both outcomes are legitimate; what must hold either way + * is that nothing was written past the capacity. */ + ExpectTrue((eccSignedSz == WC_NO_ERR_TRACE(BUFFER_E)) || + ((eccSignedSz > 0) && (eccSignedSz <= cap))); + + for (i = 0; i < SIGN_CERT_GUARD_SZ; i++) { + ExpectIntEQ(buf[cap + i], SIGN_CERT_GUARD_BYTE); + } + } + + /* A capacity above anything the signature length can reach still has to be + * accepted, so the check is not merely conservative. */ + ExpectIntGT(bodySz = test_wc_SignCert_makeBodyEcc(&cert, &eccKey, &rng, + scratch, SIGN_CERT_SCRATCH_SZ), 0); + ExpectIntGT(eccExactSz = wc_SignCert(bodySz, cert.sigType, scratch, + SIGN_CERT_SCRATCH_SZ, NULL, &eccKey, &rng), 0); + cap = eccExactSz + SIGN_CERT_ECC_SLACK; + ExpectIntGT(bodySz = test_wc_SignCert_makeBodyEcc(&cert, &eccKey, &rng, + scratch, SIGN_CERT_SCRATCH_SZ), 0); + if (EXPECT_SUCCESS() && (buf != NULL)) { + XMEMCPY(buf, scratch, (size_t)bodySz); + XMEMSET(buf + cap, SIGN_CERT_GUARD_BYTE, SIGN_CERT_GUARD_SZ); + } + ExpectIntGT(eccSignedSz = wc_SignCert(bodySz, cert.sigType, buf, + (word32)cap, NULL, &eccKey, &rng), 0); + ExpectIntLE(eccSignedSz, cap); + for (i = 0; i < SIGN_CERT_GUARD_SZ; i++) { + ExpectIntEQ(buf[cap + i], SIGN_CERT_GUARD_BYTE); + } +#endif /* TEST_SIGN_CERT_BOUNDS_ECC */ + + XFREE(buf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(scratch, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); +#ifdef TEST_SIGN_CERT_BOUNDS_ECC + if (eccInit) + wc_ecc_free(&eccKey); +#endif +#ifdef TEST_SIGN_CERT_BOUNDS_RSA + if (keyInit) + wc_FreeRsaKey(&key); +#endif + if (rngInit) + wc_FreeRng(&rng); +#endif /* TEST_SIGN_CERT_BOUNDS_RSA || TEST_SIGN_CERT_BOUNDS_ECC */ + return EXPECT_RESULT(); +} + /* * MC/DC wave 2 - decision-targeted negative paths for PKCS#8 wrap/parse * and RSA key decode. Targets argument-check, short-buffer, and diff --git a/tests/api/test_asn.h b/tests/api/test_asn.h index 8798bbc6aa..243f5dfe65 100644 --- a/tests/api/test_asn.h +++ b/tests/api/test_asn.h @@ -43,6 +43,7 @@ int test_ToTraditional_ex_handcrafted(void); int test_ToTraditional_ex_roundtrip(void); int test_ToTraditional_ex_negative(void); int test_ToTraditional_ex_mldsa_bad_params(void); +int test_wc_SignCert_buffer_bounds(void); int test_wc_AsnDecisionCoverage(void); int test_wc_AsnFeatureCoverage(void); @@ -66,6 +67,7 @@ int test_wc_AsnFeatureCoverage(void); TEST_DECL_GROUP("asn", test_ToTraditional_ex_roundtrip), \ TEST_DECL_GROUP("asn", test_ToTraditional_ex_negative), \ TEST_DECL_GROUP("asn", test_ToTraditional_ex_mldsa_bad_params), \ + TEST_DECL_GROUP("asn", test_wc_SignCert_buffer_bounds), \ TEST_DECL_GROUP("asn", test_wc_AsnDecisionCoverage), \ TEST_DECL_GROUP("asn", test_wc_AsnFeatureCoverage) diff --git a/tests/api/test_evp_pkey.c b/tests/api/test_evp_pkey.c index 6fbfb281c2..f0d2b838d4 100644 --- a/tests/api/test_evp_pkey.c +++ b/tests/api/test_evp_pkey.c @@ -1428,6 +1428,342 @@ int test_wolfSSL_EVP_PKEY_keygen(void) return EXPECT_RESULT(); } +/* + * wolfSSL_EVP_PKEY_keygen() has to replace whatever the caller supplied + * EVP_PKEY already holds. + * + * The RSA branch used to pass &pkey->pkey.ptr straight to + * wolfSSL_i2d_RSAPrivateKey(). A non-NULL pointer there means "caller supplied + * buffer" to i2d, so the freshly generated key DER was written into the DER + * already on the pkey without a size check, and the pointer was left pointing + * past the encoding. + */ +int test_wolfSSL_EVP_PKEY_keygen_reuse(void) +{ + EXPECT_DECLS; +/* wolfSSL_i2d_PrivateKey() needs OPENSSL_EXTRA plus !NO_ASN and !NO_PWDBASED, + * and nothing here is OPENSSL_ALL only. */ +#if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN) && \ + defined(USE_CERT_BUFFERS_2048) && !defined(HAVE_SELFTEST) && \ + !defined(NO_ASN) && !defined(NO_PWDBASED) + WOLFSSL_EVP_PKEY* pkey = NULL; + WOLFSSL_EVP_PKEY* decoded = NULL; + EVP_PKEY_CTX* ctx = NULL; + const unsigned char* in; + unsigned char* der = NULL; + int derSz = 0; +/* load_file() is compiled only when certificates are available. */ +#if defined(HAVE_PKCS8) && !defined(NO_FILESYSTEM) && !defined(NO_CERTS) + byte* p8 = NULL; + size_t p8Sz = 0; +#endif + + /* Populate the pkey with a public key DER much smaller than the private + * key DER that keygen will produce. */ + in = client_keypub_der_2048; + ExpectNotNull(pkey = wolfSSL_d2i_PUBKEY(NULL, &in, + (long)sizeof_client_keypub_der_2048)); + + ExpectNotNull(ctx = EVP_PKEY_CTX_new(pkey, NULL)); + ExpectIntEQ(EVP_PKEY_keygen_init(ctx), WOLFSSL_SUCCESS); + ExpectIntEQ(EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 2048), WOLFSSL_SUCCESS); + ExpectIntEQ(EVP_PKEY_keygen(ctx, &pkey), WOLFSSL_SUCCESS); + + /* The DER on the pkey has to be the generated private key, held in a + * buffer that starts where pkey.ptr points. */ + ExpectIntGT(derSz = wolfSSL_i2d_PrivateKey(pkey, &der), 0); + in = der; + ExpectNotNull(decoded = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, &in, + (long)derSz)); +#if defined(WOLFSSL_ERROR_CODE_OPENSSL) + ExpectIntEQ(EVP_PKEY_cmp(pkey, decoded), 1); +#else + ExpectIntEQ(EVP_PKEY_cmp(pkey, decoded), 0); +#endif + + XFREE(der, NULL, DYNAMIC_TYPE_OPENSSL); + der = NULL; + EVP_PKEY_free(decoded); + decoded = NULL; + EVP_PKEY_CTX_free(ctx); + ctx = NULL; + EVP_PKEY_free(pkey); + pkey = NULL; + +#if defined(HAVE_PKCS8) && !defined(NO_FILESYSTEM) && !defined(NO_CERTS) + /* Same again, seeded from a PKCS#8 wrapped key. That gives the pkey a + * non-zero pkcs8HeaderSz, which does not describe the bare PKCS#1 + * encoding keygen installs, so it has to be cleared. */ + ExpectIntEQ(load_file("./certs/server-keyPkcs8.der", &p8, &p8Sz), 0); + in = p8; + ExpectNotNull(pkey = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, &in, + (long)p8Sz)); + + ExpectNotNull(ctx = EVP_PKEY_CTX_new(pkey, NULL)); + ExpectIntEQ(EVP_PKEY_keygen_init(ctx), WOLFSSL_SUCCESS); + ExpectIntEQ(EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 2048), WOLFSSL_SUCCESS); + ExpectIntEQ(EVP_PKEY_keygen(ctx, &pkey), WOLFSSL_SUCCESS); + + ExpectIntGT(derSz = wolfSSL_i2d_PrivateKey(pkey, &der), 0); + in = der; + ExpectNotNull(decoded = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, &in, + (long)derSz)); +#if defined(WOLFSSL_ERROR_CODE_OPENSSL) + ExpectIntEQ(EVP_PKEY_cmp(pkey, decoded), 1); +#else + ExpectIntEQ(EVP_PKEY_cmp(pkey, decoded), 0); +#endif + + XFREE(der, NULL, DYNAMIC_TYPE_OPENSSL); + XFREE(p8, NULL, DYNAMIC_TYPE_TMP_BUFFER); + EVP_PKEY_free(decoded); + EVP_PKEY_CTX_free(ctx); + EVP_PKEY_free(pkey); +#endif /* HAVE_PKCS8 && !NO_FILESYSTEM && !NO_CERTS */ +#endif + return EXPECT_RESULT(); +} + +/* + * Replacing a PKCS#8 wrapped key on an EVP_PKEY with an EC key that carries no + * wrapper has to drop pkcs8HeaderSz along with the encoding it described. + * + * ECC_populate_EVP_PKEY() writes a bare SEC1 ECPrivateKey when the EC key has + * no header size of its own. Everything that exports the key, from + * wolfSSL_EVP_PKEY_get_der() to the PKCS#8 encryption in + * wolfSSL_PEM_write_bio_PKCS8PrivateKey(), skips pkcs8HeaderSz bytes of the + * stored buffer, so a size left from the previous key makes them start inside + * the new encoding and hand out a truncated one. + */ +int test_wolfSSL_EVP_PKEY_set1_EC_KEY_no_pkcs8(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && !defined(NO_FILESYSTEM) && \ + !defined(NO_CERTS) && !defined(NO_ASN) && !defined(NO_PWDBASED) + WOLFSSL_EVP_PKEY* wrapped = NULL; + WOLFSSL_EVP_PKEY* fresh = NULL; + WOLFSSL_EC_KEY* ec = NULL; + const unsigned char* in; + unsigned char* wrappedDer = NULL; + unsigned char* freshDer = NULL; + int wrappedSz = 0; + int freshSz = 0; + byte* buf = NULL; + size_t bufSz = 0; + + /* Seed a pkey from a PKCS#8 wrapped key so that pkcs8HeaderSz starts out + * non-zero. */ + ExpectIntEQ(load_file("./certs/ecc-keyPkcs8.der", &buf, &bufSz), 0); + in = buf; + ExpectNotNull(wrapped = wolfSSL_d2i_PrivateKey(EVP_PKEY_EC, NULL, &in, + (long)bufSz)); + + /* A generated key carries no PKCS#8 header, so setting it takes the + * traditional branch of ECC_populate_EVP_PKEY(). */ + ExpectNotNull(ec = wolfSSL_EC_KEY_new_by_curve_name(NID_X9_62_prime256v1)); + ExpectIntEQ(wolfSSL_EC_KEY_generate_key(ec), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_EVP_PKEY_set1_EC_KEY(wrapped, ec), WOLFSSL_SUCCESS); + + /* The same key on a pkey that never held a wrapped one is the reference: + * both have to export the identical encoding. */ + ExpectNotNull(fresh = wolfSSL_EVP_PKEY_new()); + ExpectIntEQ(wolfSSL_EVP_PKEY_set1_EC_KEY(fresh, ec), WOLFSSL_SUCCESS); + + ExpectIntGT(freshSz = wolfSSL_i2d_PrivateKey(fresh, &freshDer), 0); + ExpectIntGT(wrappedSz = wolfSSL_i2d_PrivateKey(wrapped, &wrappedDer), 0); + ExpectIntEQ(wrappedSz, freshSz); + ExpectNotNull(wrappedDer); + ExpectNotNull(freshDer); + ExpectBufEQ(wrappedDer, freshDer, (size_t)freshSz); + + XFREE(wrappedDer, NULL, DYNAMIC_TYPE_OPENSSL); + XFREE(freshDer, NULL, DYNAMIC_TYPE_OPENSSL); + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + wolfSSL_EC_KEY_free(ec); + wolfSSL_EVP_PKEY_free(fresh); + wolfSSL_EVP_PKEY_free(wrapped); +#endif + return EXPECT_RESULT(); +} + +/* + * Replacing the key on an EVP_PKEY with one whose DER is SHORTER has to shrink + * the stored encoding without writing past the new buffer. + * + * Under WOLFSSL_NO_REALLOC, PopulateRSAEvpPkeyDer() and ECC_populate_EVP_PKEY() + * emulate XREALLOC by allocating a buffer sized for the new encoding and + * copying pkey_sz bytes, the size of the OLD one, into it. The CI job + * opensslextra-norealloc-asan builds exactly that configuration under ASan, so + * this test is where such an over-copy gets caught. + */ +int test_wolfSSL_EVP_PKEY_set1_shrinking_der(void) +{ + EXPECT_DECLS; +/* settings.h defines WOLFSSL_KEY_TO_DER only when RSA is enabled, so gating the + * whole test on it would compile the ECC half out of an RSA-less build, and + * that half is the only coverage for the ECC_populate_EVP_PKEY() over-copy. + * Gate on the union and keep the per-algorithm guards inside. */ +#if defined(OPENSSL_EXTRA) && !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \ + !defined(NO_ASN) && !defined(NO_PWDBASED) && \ + ((!defined(NO_RSA) && defined(WOLFSSL_KEY_TO_DER)) || defined(HAVE_ECC)) + const unsigned char* in; + byte* buf = NULL; + size_t bufSz = 0; +#if !defined(NO_RSA) && defined(WOLFSSL_KEY_TO_DER) + WOLFSSL_EVP_PKEY* rsaPkey = NULL; + WOLFSSL_RSA* rsaPub = NULL; + int rsaPrivSz = 0; +#endif +#ifdef HAVE_ECC + WOLFSSL_EVP_PKEY* ecPkey = NULL; + WOLFSSL_EC_KEY* ecPriv = NULL; + WOLFSSL_EC_KEY* ecPub = NULL; + int ecPrivSz = 0; +#endif + +#if !defined(NO_RSA) && defined(WOLFSSL_KEY_TO_DER) + /* Private key DER first, then a public-only key whose DER is about a + * quarter of the size. */ + ExpectIntEQ(load_file("./certs/client-key.der", &buf, &bufSz), 0); + in = buf; + ExpectNotNull(rsaPkey = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, &in, + (long)bufSz)); + ExpectIntGT(rsaPrivSz = wolfSSL_i2d_PrivateKey(rsaPkey, NULL), 0); + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + buf = NULL; + + ExpectIntEQ(load_file("./certs/client-keyPub.der", &buf, &bufSz), 0); + in = buf; + ExpectNotNull(rsaPub = wolfSSL_d2i_RSAPublicKey(NULL, &in, (long)bufSz)); + ExpectIntEQ(wolfSSL_EVP_PKEY_set1_RSA(rsaPkey, rsaPub), WOLFSSL_SUCCESS); + /* Confirm the stored encoding really did shrink, so the test keeps + * exercising the direction that overruns. */ + ExpectIntLT(wolfSSL_i2d_PrivateKey(rsaPkey, NULL), rsaPrivSz); + + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + buf = NULL; + wolfSSL_RSA_free(rsaPub); + wolfSSL_EVP_PKEY_free(rsaPkey); +#endif + +#ifdef HAVE_ECC + /* Same shape for ECC: the private key encoding is longer than the public + * one for the same curve. The seed is PKCS#8 wrapped rather than a bare + * SEC1 key, so pkcs8HeaderSz starts non-zero and the exact size assertion + * below catches a header size carried over onto the public encoding. */ + ExpectIntEQ(load_file("./certs/ecc-keyPkcs8.der", &buf, &bufSz), 0); + in = buf; + ExpectNotNull(ecPkey = wolfSSL_d2i_PrivateKey(EVP_PKEY_EC, NULL, &in, + (long)bufSz)); + ExpectIntGT(ecPrivSz = wolfSSL_i2d_PrivateKey(ecPkey, NULL), 0); + ExpectNotNull(ecPriv = wolfSSL_EVP_PKEY_get1_EC_KEY(ecPkey)); + + /* A key holding only the public point takes ECC_populate_EVP_PKEY's + * public branch. */ + ExpectNotNull(ecPub = wolfSSL_EC_KEY_new_by_curve_name( + NID_X9_62_prime256v1)); + ExpectIntEQ(wolfSSL_EC_KEY_set_public_key(ecPub, + wolfSSL_EC_KEY_get0_public_key(ecPriv)), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_EVP_PKEY_set1_EC_KEY(ecPkey, ecPub), WOLFSSL_SUCCESS); + ExpectIntLT(wolfSSL_i2d_PrivateKey(ecPkey, NULL), ecPrivSz); + /* Exact rather than "smaller", so that an export starting at a stale + * pkcs8HeaderSz shows up as a size mismatch instead of passing. */ + ExpectIntEQ(wolfSSL_i2d_PrivateKey(ecPkey, NULL), + wc_EccPublicKeyDerSize((ecc_key*)ecPub->internal, 1)); + + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + buf = NULL; + wolfSSL_EC_KEY_free(ecPub); + wolfSSL_EC_KEY_free(ecPriv); + wolfSSL_EVP_PKEY_free(ecPkey); +#endif +#endif /* OPENSSL_EXTRA && WOLFSSL_KEY_TO_DER && !NO_FILESYSTEM */ + return EXPECT_RESULT(); +} + +/* + * wolfSSL_EVP_PKEY_get1_EC_KEY() hands the caller a reference of its own, so + * releasing it has to leave the copy the EVP_PKEY holds intact and usable. + * + * This covers the path where the pkey already carries the EC_KEY, which is + * what every decode entry point produces. It passes with and without the + * reference fix in the branch that builds the key instead, since no decode + * path reaches that branch; it is here to pin the reference contract rather + * than as a regression test for it. + */ +int test_wolfSSL_EVP_PKEY_get1_EC_KEY_reuse(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && !defined(NO_FILESYSTEM) && \ + !defined(NO_CERTS) && !defined(NO_ASN) && !defined(NO_PWDBASED) + WOLFSSL_EVP_PKEY* pkey = NULL; + WOLFSSL_EC_KEY* ec1 = NULL; + WOLFSSL_EC_KEY* ec2 = NULL; + const unsigned char* in; + byte* buf = NULL; + size_t bufSz = 0; + + /* A pkey decoded from DER carries no EC_KEY yet, so the first get1 is the + * call that builds and caches one. */ + ExpectIntEQ(load_file("./certs/ecc-client-key.der", &buf, &bufSz), 0); + in = buf; + ExpectNotNull(pkey = wolfSSL_d2i_PrivateKey(EVP_PKEY_EC, NULL, &in, + (long)bufSz)); + + ExpectNotNull(ec1 = wolfSSL_EVP_PKEY_get1_EC_KEY(pkey)); + wolfSSL_EC_KEY_free(ec1); + ec1 = NULL; + + /* The pkey still holds a live key, so this neither reads freed memory nor + * returns NULL. */ + ExpectNotNull(ec2 = wolfSSL_EVP_PKEY_get1_EC_KEY(pkey)); + ExpectNotNull(wolfSSL_EC_KEY_get0_public_key(ec2)); + wolfSSL_EC_KEY_free(ec2); + + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + wolfSSL_EVP_PKEY_free(pkey); +#endif + return EXPECT_RESULT(); +} + +/* + * EVP_PKEY_keygen() on a pkey that already carries a DH object has to release + * it rather than overwrite the only reference to it. + */ +int test_wolfSSL_EVP_PKEY_keygen_dh_reuse(void) +{ + EXPECT_DECLS; +#if (defined(OPENSSL_ALL) || defined(WOLFSSL_QT) || \ + defined(WOLFSSL_OPENSSH)) && !defined(NO_DH) && \ + defined(WOLFSSL_DH_EXTRA) && !defined(NO_FILESYSTEM) && \ + !defined(NO_CERTS) && (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) + WOLFSSL_EVP_PKEY* pkey = NULL; + WOLFSSL_DH* dh = NULL; + EVP_PKEY_CTX* ctx = NULL; + byte* buf = NULL; + size_t bufSz = 0; + + ExpectIntEQ(load_file("./certs/dh2048.der", &buf, &bufSz), 0); + ExpectNotNull(dh = wolfSSL_DH_new()); + ExpectIntEQ(wolfSSL_DH_LoadDer(dh, buf, (int)bufSz), WOLFSSL_SUCCESS); + + /* set1 leaves the pkey holding a reference of its own, which keygen has to + * release when it installs the generated key. */ + ExpectNotNull(pkey = wolfSSL_EVP_PKEY_new()); + ExpectIntEQ(wolfSSL_EVP_PKEY_set1_DH(pkey, dh), WOLFSSL_SUCCESS); + + ExpectNotNull(ctx = EVP_PKEY_CTX_new(pkey, NULL)); + ExpectIntEQ(EVP_PKEY_keygen_init(ctx), WOLFSSL_SUCCESS); + ExpectIntEQ(EVP_PKEY_keygen(ctx, &pkey), WOLFSSL_SUCCESS); + + EVP_PKEY_CTX_free(ctx); + wolfSSL_DH_free(dh); + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + wolfSSL_EVP_PKEY_free(pkey); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_EVP_SignInit_ex(void) { EXPECT_DECLS; @@ -3112,3 +3448,91 @@ int test_wolfSSL_EVP_PKEY_encoded_public_key(void) return EXPECT_RESULT(); } + +int test_wolfSSL_d2i_PrivateKey_reuse_resets_state(void) +{ + EXPECT_DECLS; +/* wolfSSL_d2i_PrivateKey_EVP() and wolfSSL_PEM_write_bio_PKCS8PrivateKey() are + * both OPENSSL_ALL, and the latter also needs PKCS#8 and a password based key + * derivation. */ +#if defined(OPENSSL_ALL) && defined(HAVE_PKCS8) && !defined(NO_PWDBASED) && \ + defined(HAVE_ECC) && !defined(NO_RSA) && !defined(NO_FILESYSTEM) && \ + !defined(NO_BIO) && defined(WOLFSSL_DER_TO_PEM) + WOLFSSL_EVP_PKEY* pkey = NULL; + unsigned char* rsaDer = NULL; + unsigned char* eccDer = NULL; + unsigned char* freshPem = NULL; + const unsigned char* p = NULL; + unsigned char* q = NULL; + WOLFSSL_BIO* bio = NULL; + char* pem = NULL; + int rsaSz = 0; + int eccSz = 0; + int freshSz = 0; + XFILE f = XBADFILE; + + ExpectNotNull(rsaDer = (unsigned char*)XMALLOC(4096, NULL, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectNotNull(eccDer = (unsigned char*)XMALLOC(4096, NULL, + DYNAMIC_TYPE_TMP_BUFFER)); + + ExpectTrue((f = XFOPEN("./certs/server-keyPkcs8.der", "rb")) != XBADFILE); + ExpectIntGT(rsaSz = (int)XFREAD(rsaDer, 1, 4096, f), 0); + if (f != XBADFILE) { + XFCLOSE(f); + f = XBADFILE; + } + ExpectTrue((f = XFOPEN("./certs/ecc-key.der", "rb")) != XBADFILE); + ExpectIntGT(eccSz = (int)XFREAD(eccDer, 1, 4096, f), 0); + if (f != XBADFILE) { + XFCLOSE(f); + f = XBADFILE; + } + + /* Baseline: decode the traditional ECC key into a new object and record + * its PKCS#8 PEM. */ + q = eccDer; + ExpectNotNull(pkey = wolfSSL_d2i_PrivateKey_EVP(NULL, &q, (long)eccSz)); + ExpectIntEQ(wolfSSL_EVP_PKEY_id(pkey), WC_EVP_PKEY_EC); + ExpectNotNull(bio = wolfSSL_BIO_new(wolfSSL_BIO_s_mem())); + ExpectIntGT(wolfSSL_PEM_write_bio_PKCS8PrivateKey(bio, pkey, NULL, NULL, 0, + NULL, NULL), 0); + ExpectIntGT(freshSz = (int)wolfSSL_BIO_get_mem_data(bio, &pem), 0); + ExpectNotNull(freshPem = (unsigned char*)XMALLOC((size_t)freshSz, NULL, + DYNAMIC_TYPE_TMP_BUFFER)); + if (freshPem != NULL && pem != NULL) { + XMEMCPY(freshPem, pem, (size_t)freshSz); + } + wolfSSL_BIO_free(bio); + bio = NULL; + wolfSSL_EVP_PKEY_free(pkey); + pkey = NULL; + + /* A PKCS#8 RSA key records a non-zero pkcs8HeaderSz. */ + p = rsaDer; + ExpectNotNull(pkey = wolfSSL_d2i_PrivateKey(WC_EVP_PKEY_RSA, NULL, &p, + (long)rsaSz)); + ExpectIntGT((int)pkey->pkcs8HeaderSz, 0); + + /* Reusing that object for the traditional ECC key must clear it, so the + * re-encoded key is not sliced at the previous key's header offset. */ + q = eccDer; + ExpectNotNull(wolfSSL_d2i_PrivateKey_EVP(&pkey, &q, (long)eccSz)); + ExpectIntEQ(wolfSSL_EVP_PKEY_id(pkey), WC_EVP_PKEY_EC); + ExpectIntEQ((int)pkey->pkcs8HeaderSz, 0); + + /* The reused object must encode exactly like the fresh one. */ + ExpectNotNull(bio = wolfSSL_BIO_new(wolfSSL_BIO_s_mem())); + ExpectIntGT(wolfSSL_PEM_write_bio_PKCS8PrivateKey(bio, pkey, NULL, NULL, 0, + NULL, NULL), 0); + ExpectIntEQ((int)wolfSSL_BIO_get_mem_data(bio, &pem), freshSz); + ExpectBufEQ(pem, freshPem, freshSz); + + wolfSSL_BIO_free(bio); + wolfSSL_EVP_PKEY_free(pkey); + XFREE(freshPem, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(eccDer, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(rsaDer, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_evp_pkey.h b/tests/api/test_evp_pkey.h index 60647ac049..65df74ce18 100644 --- a/tests/api/test_evp_pkey.h +++ b/tests/api/test_evp_pkey.h @@ -51,6 +51,11 @@ int test_wolfSSL_EVP_PKEY_paramgen(void); int test_wolfSSL_EVP_PKEY_param_check(void); int test_wolfSSL_EVP_PKEY_keygen_init(void); int test_wolfSSL_EVP_PKEY_keygen(void); +int test_wolfSSL_EVP_PKEY_keygen_reuse(void); +int test_wolfSSL_EVP_PKEY_set1_EC_KEY_no_pkcs8(void); +int test_wolfSSL_EVP_PKEY_set1_shrinking_der(void); +int test_wolfSSL_EVP_PKEY_get1_EC_KEY_reuse(void); +int test_wolfSSL_EVP_PKEY_keygen_dh_reuse(void); int test_wolfSSL_EVP_SignInit_ex(void); int test_wolfSSL_EVP_PKEY_sign_verify_rsa(void); int test_wolfSSL_EVP_PKEY_sign_verify_dsa(void); @@ -69,6 +74,7 @@ int test_wolfSSL_EVP_PKEY_ed448(void); int test_wolfSSL_EVP_PKEY_x25519(void); int test_wolfSSL_EVP_PKEY_x448(void); int test_wolfSSL_EVP_PKEY_encoded_public_key(void); +int test_wolfSSL_d2i_PrivateKey_reuse_resets_state(void); #define TEST_EVP_PKEY_DECLS \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_CTX_new_id), \ @@ -99,6 +105,11 @@ int test_wolfSSL_EVP_PKEY_encoded_public_key(void); TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_param_check), \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_keygen_init), \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_keygen), \ + TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_keygen_reuse), \ + TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_set1_EC_KEY_no_pkcs8), \ + TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_set1_shrinking_der), \ + TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_get1_EC_KEY_reuse), \ + TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_keygen_dh_reuse), \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_SignInit_ex), \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_sign_verify_rsa), \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_sign_verify_dsa), \ @@ -116,6 +127,7 @@ int test_wolfSSL_EVP_PKEY_encoded_public_key(void); TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_ed448), \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_x25519), \ TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_x448), \ - TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_encoded_public_key) + TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_encoded_public_key), \ + TEST_DECL_GROUP("evp_pkey", test_wolfSSL_d2i_PrivateKey_reuse_resets_state) #endif /* WOLFCRYPT_TEST_EVP_PKEY_H */ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 198fdcbecf..eb0ca1830e 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -30951,6 +30951,7 @@ static int SignCert(int requestSz, int sType, byte* buf, word32 buffSz, { int sigSz = 0; int ret; + int totalSz; void* heap = NULL; /* The signature buffer is sized from the key at runtime. */ int maxSigSz; @@ -30962,6 +30963,11 @@ static int SignCert(int requestSz, int sType, byte* buf, word32 buffSz, if (requestSz < 0) return requestSz; + /* MakeSignature() hashes requestSz bytes out of buf below, so bound the + * read against the buffer before signing rather than only the write. */ + if ((word32)requestSz > buffSz) + return BUFFER_E; + /* Async crypto reuses the signing key's embedded CertSignCtx; only RSA and * ECC keys carry one. */ if (rsaKey) { @@ -31032,7 +31038,13 @@ static int SignCert(int requestSz, int sType, byte* buf, word32 buffSz, #endif if (sigSz >= 0) { - if (requestSz + MAX_SEQ_SZ * 2 + sigSz > (int)buffSz) + /* AddSignature() takes no buffer size, and writes the + * signatureAlgorithm and signatureValue on top of the outer SEQUENCE, + * so ask it for the exact size rather than estimating. */ + totalSz = AddSignature(NULL, requestSz, certSignCtx->sig, sigSz, sType); + if (totalSz < 0) + sigSz = totalSz; + else if ((word32)totalSz > buffSz) sigSz = BUFFER_E; else sigSz = AddSignature(buf, requestSz, certSignCtx->sig, sigSz, @@ -31343,6 +31355,7 @@ int wc_SignCert_cb(int requestSz, int sType, byte* buf, word32 buffSz, WC_RNG* rng) { int sigSz = 0; + int totalSz; word32 sigCap = MAX_ENCODED_CLASSIC_SIG_SZ; CertSignCtx certSignCtx_lcl; CertSignCtx* certSignCtx = &certSignCtx_lcl; @@ -31388,6 +31401,12 @@ int wc_SignCert_cb(int requestSz, int sType, byte* buf, word32 buffSz, return requestSz; } + /* MakeSignatureCb() hashes requestSz bytes out of buf below, so bound the + * read against the buffer before signing rather than only the write. */ + if ((word32)requestSz > buffSz) { + return BUFFER_E; + } + /* keyType is restricted to RSA_TYPE/ECC_TYPE above, so the signature is * a classic (non-PQC) one and fits MAX_ENCODED_CLASSIC_SIG_SZ. */ #ifndef WOLFSSL_NO_MALLOC @@ -31420,12 +31439,14 @@ int wc_SignCert_cb(int requestSz, int sType, byte* buf, word32 buffSz, #endif if (sigSz >= 0) { - /* Check buffer has room for signature structure. This is an estimate - * using MAX_SEQ_SZ * 2 to account for sequence headers and algorithm - * identifier overhead. For precise sizing, call AddSignature with - * NULL buffer first, but this estimate matches the existing pattern - * used in SignCert. */ - if (requestSz + MAX_SEQ_SZ * 2 + sigSz > (int)buffSz) { + /* AddSignature() takes no buffer size, and writes the + * signatureAlgorithm and signatureValue on top of the outer SEQUENCE, + * so ask it for the exact size rather than estimating. */ + totalSz = AddSignature(NULL, requestSz, certSignCtx->sig, sigSz, sType); + if (totalSz < 0) { + sigSz = totalSz; + } + else if ((word32)totalSz > buffSz) { sigSz = BUFFER_E; } else { diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index 3b70de2dbe..37f6ba5155 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -3803,6 +3803,9 @@ int wolfSSL_EVP_PKEY_keygen_init(WOLFSSL_EVP_PKEY_CTX *ctx) #ifdef HAVE_ECC static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key); #endif +#if !defined(NO_RSA) && defined(WOLFSSL_KEY_TO_DER) +static int PopulateRSAEvpPkeyDer(WOLFSSL_EVP_PKEY *pkey); +#endif int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, WOLFSSL_EVP_PKEY **ppkey) @@ -3810,6 +3813,12 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, int ret = WC_NO_ERR_TRACE(WOLFSSL_FAILURE); int ownPkey = 0; WOLFSSL_EVP_PKEY* pkey; +#if defined(WOLFSSL_KEY_GEN) && !defined(NO_RSA) + WOLFSSL_RSA* rsaTmp; +#endif +#if !defined(NO_DH) && (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) + WOLFSSL_DH* dhTmp; +#endif WOLFSSL_ENTER("wolfSSL_EVP_PKEY_keygen"); @@ -3843,13 +3852,19 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, switch (pkey->type) { #if defined(WOLFSSL_KEY_GEN) && !defined(NO_RSA) case WC_EVP_PKEY_RSA: - pkey->rsa = wolfSSL_RSA_generate_key(ctx->nbits, WC_RSA_EXPONENT, + rsaTmp = wolfSSL_RSA_generate_key(ctx->nbits, WC_RSA_EXPONENT, NULL, NULL); - if (pkey->rsa) { + if (rsaTmp != NULL) { + if (pkey->rsa != NULL && pkey->ownRsa == 1) + wolfSSL_RSA_free(pkey->rsa); + pkey->rsa = rsaTmp; pkey->ownRsa = 1; - pkey->pkey_sz = wolfSSL_i2d_RSAPrivateKey(pkey->rsa, - (unsigned char**)&pkey->pkey.ptr); - ret = WOLFSSL_SUCCESS; + /* PopulateRSAEvpPkeyDer() wraps the encoding in PKCS#8 only + * when the RSA key carries a header size, so take the new + * key's value. A size left over from a wrapped predecessor + * would make the export paths slice bytes off the encoding. */ + pkey->pkcs8HeaderSz = rsaTmp->pkcs8HeaderSz; + ret = PopulateRSAEvpPkeyDer(pkey); } break; #endif @@ -3859,11 +3874,15 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, * prior call to wolfSSL_EVP_PKEY_paramgen. */ if (pkey->ecc == NULL) { pkey->ecc = wolfSSL_EC_KEY_new_by_curve_name(ctx->curveNID); + /* Ownership is claimed where the key is created. Claiming it + * for a key that was already on the pkey would hand the free + * to a second owner. */ + if (pkey->ecc != NULL) + pkey->ownEcc = 1; } if (pkey->ecc) { ret = wolfSSL_EC_KEY_generate_key(pkey->ecc); if (ret == WOLFSSL_SUCCESS) { - pkey->ownEcc = 1; if (ECC_populate_EVP_PKEY(pkey, pkey->ecc) != WOLFSSL_SUCCESS) ret = WOLFSSL_FAILURE; } @@ -3872,8 +3891,13 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, #endif #if !defined(NO_DH) && (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) case WC_EVP_PKEY_DH: - pkey->dh = wolfSSL_DH_new(); - if (pkey->dh) { + dhTmp = wolfSSL_DH_new(); + if (dhTmp != NULL) { + /* A caller supplied pkey may already carry a DH object, so + * release it rather than overwrite the only reference to it. */ + if (pkey->dh != NULL && pkey->ownDh == 1) + wolfSSL_DH_free(pkey->dh); + pkey->dh = dhTmp; pkey->ownDh = 1; /* load DH params from CTX */ ret = wolfSSL_DH_LoadDer(pkey->dh, @@ -9280,23 +9304,53 @@ static int PopulateRSAEvpPkeyDer(WOLFSSL_EVP_PKEY *pkey) } #ifdef WOLFSSL_NO_REALLOC + /* The new buffer can be smaller than the old encoding, and the encoding + * below fills it completely, so nothing is carried over. Allocate before + * the old buffer is touched, so that a failure here leaves the encoding + * held so far in place. */ derBuf = (byte*)XMALLOC((size_t)derSz, pkey->heap, DYNAMIC_TYPE_DER); if (derBuf != NULL) { - XMEMCPY(derBuf, pkey->pkey.ptr, (size_t)pkey->pkey_sz); + /* On a private key the outgoing buffer holds a full RSA DER, so wipe + * it before it is returned to the allocator. The size is dropped with + * the contents so a failure below cannot leave pkey_sz describing a + * buffer that no longer holds an encoding. */ + if (pkey->pkey.ptr != NULL && pkey->pkey_sz > 0) { + ForceZero(pkey->pkey.ptr, (word32)pkey->pkey_sz); + } XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_DER); pkey->pkey.ptr = NULL; + pkey->pkey_sz = 0; } #else + /* XREALLOC consumes the old pointer, so the buffer has to be wiped before + * the call: on a private key it holds a full RSA DER. Nothing is carried + * over, as the encoding below fills the new buffer completely. The size is + * dropped with the contents so a failure below cannot leave pkey_sz + * describing a buffer that no longer holds an encoding. */ + if (pkey->pkey.ptr != NULL && pkey->pkey_sz > 0) { + ForceZero(pkey->pkey.ptr, (word32)pkey->pkey_sz); + pkey->pkey_sz = 0; + } + derBuf = (byte*)XREALLOC(pkey->pkey.ptr, (size_t)derSz, pkey->heap, DYNAMIC_TYPE_DER); #endif if (derBuf == NULL) { WOLFSSL_MSG("PopulateRSAEvpPkeyDer malloc failed"); + if (pkey->pkey_sz == 0) { + /* No encoding is described any more, so the header size has to go + * as well or the export paths subtract it from zero and + * underflow. */ + pkey->pkcs8HeaderSz = 0; + } return WOLFSSL_FAILURE; } - /* Old pointer is invalid from this point on */ + /* Old pointer is invalid from this point on. The new buffer holds no + * encoding yet and can be smaller than the old one, so drop the old size + * rather than let a failure below return with the two out of step. */ pkey->pkey.ptr = (char*)derBuf; + pkey->pkey_sz = 0; if (rsa->type == RSA_PRIVATE) { ret = wc_RsaKeyToDer(rsa, derBuf, (word32)derSz); @@ -9314,10 +9368,15 @@ static int PopulateRSAEvpPkeyDer(WOLFSSL_EVP_PKEY *pkey) if (derBuf != NULL) { ret = wc_CreatePKCS8Key(derBuf, &sz, keyBuf, (word32)keySz, RSAk, NULL, 0); + /* keyBuf holds the unwrapped private key. */ + ForceZero(keyBuf, (word32)keySz); XFREE(keyBuf, pkey->heap, DYNAMIC_TYPE_DER); pkey->pkey.ptr = (char*)derBuf; } else { + /* The encoding is abandoned but keyBuf stays on the pkey, + * so do not leave the key material behind in it. */ + ForceZero(keyBuf, (word32)keySz); ret = MEMORY_E; } derSz = (int)sz; @@ -9334,6 +9393,8 @@ static int PopulateRSAEvpPkeyDer(WOLFSSL_EVP_PKEY *pkey) if (ret < 0) { WOLFSSL_MSG("PopulateRSAEvpPkeyDer failed"); + /* As above: pkey_sz is zero here, so the header size cannot stay. */ + pkey->pkcs8HeaderSz = 0; return WOLFSSL_FAILURE; } else { @@ -9571,6 +9632,17 @@ WOLFSSL_EC_KEY* wolfSSL_EVP_PKEY_get1_EC_KEY(WOLFSSL_EVP_PKEY* key) WOLFSSL_EC_KEY_LOAD_PUBLIC) != WOLFSSL_SUCCESS) { wolfSSL_EC_KEY_free(local); + /* The pkey must not keep a pointer to the freed key. */ + key->ecc = NULL; + local = NULL; + } + } + if (local != NULL) { + /* The key was created with a single reference. It stays on the + * pkey, so that one belongs to the pkey and the caller needs + * its own, which it releases as the get1 contract requires. */ + key->ownEcc = 1; + if (wolfSSL_EC_KEY_up_ref(local) != WOLFSSL_SUCCESS) { local = NULL; } } @@ -9785,6 +9857,13 @@ static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key) if (derBuf) { if (wc_EccKeyToPKCS8(ecc, derBuf, (word32*)&derSz) >= 0) { if (pkey->pkey.ptr) { + /* The outgoing buffer can hold a private key + * encoding, so wipe it before it is returned to + * the allocator. */ + if (pkey->pkey_sz > 0) { + ForceZero(pkey->pkey.ptr, + (word32)pkey->pkey_sz); + } XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_OPENSSL); } pkey->pkey_sz = (int)derSz; @@ -9824,10 +9903,21 @@ static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key) if (derBuf) { if (wc_EccKeyToDer(ecc, derBuf, (word32)derSz) >= 0) { if (pkey->pkey.ptr) { + /* As above, the outgoing buffer can hold a + * private key encoding. */ + if (pkey->pkey_sz > 0) { + ForceZero(pkey->pkey.ptr, + (word32)pkey->pkey_sz); + } XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_OPENSSL); } pkey->pkey_sz = (int)derSz; pkey->pkey.ptr = (char*)derBuf; + /* The encoding carries no PKCS#8 wrapper, so a header + * size left from a wrapped predecessor has to go with + * it, or the export paths start inside the new + * encoding. */ + pkey->pkcs8HeaderSz = 0; return WOLFSSL_SUCCESS; } else { @@ -9841,13 +9931,44 @@ static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key) else if (ecc->type == ECC_PUBLICKEY) { if ((derSz = wc_EccPublicKeyDerSize(ecc, 1)) > 0) { #ifdef WOLFSSL_NO_REALLOC - derBuf = (byte*)XMALLOC((size_t)derSz, pkey->heap, DYNAMIC_TYPE_OPENSSL); + /* The encoding below fills the new buffer, so nothing is carried + * over from the old one. It can be smaller than the old encoding. + * Allocate before the old buffer is touched, so that a failure + * here leaves the encoding held so far in place. */ + derBuf = (byte*)XMALLOC((size_t)derSz, pkey->heap, + DYNAMIC_TYPE_OPENSSL); if (derBuf != NULL) { - XMEMCPY(derBuf, pkey->pkey.ptr, (size_t)pkey->pkey_sz); + /* The buffer being released can hold a private key encoding, + * so wipe it first. The size is dropped with the contents so + * a failure below cannot leave pkey_sz describing a buffer + * that no longer holds an encoding. A SubjectPublicKeyInfo + * carries no PKCS#8 wrapper, so the header size has to go + * with it as well, or a header size left from a wrapped + * predecessor would make the export paths start inside the + * new encoding. */ + if (pkey->pkey.ptr != NULL && pkey->pkey_sz > 0) { + ForceZero(pkey->pkey.ptr, (word32)pkey->pkey_sz); + } XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_OPENSSL); pkey->pkey.ptr = NULL; + pkey->pkey_sz = 0; + pkey->pkcs8HeaderSz = 0; } #else + /* XREALLOC consumes the old pointer, so the buffer has to be + * wiped before the call: it can hold a private key encoding. The + * size is dropped with the contents so a failure below cannot + * leave pkey_sz describing a buffer that no longer holds an + * encoding. A SubjectPublicKeyInfo carries no PKCS#8 wrapper, so + * the header size has to go with it as well, or a header size + * left from a wrapped predecessor would make the export paths + * start inside the new encoding. */ + if (pkey->pkey.ptr != NULL && pkey->pkey_sz > 0) { + ForceZero(pkey->pkey.ptr, (word32)pkey->pkey_sz); + } + pkey->pkey_sz = 0; + pkey->pkcs8HeaderSz = 0; + derBuf = (byte*)XREALLOC(pkey->pkey.ptr, (size_t)derSz, pkey->heap, DYNAMIC_TYPE_OPENSSL); #endif @@ -9858,7 +9979,6 @@ static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key) XFREE(derBuf, pkey->heap, DYNAMIC_TYPE_OPENSSL); derBuf = NULL; pkey->pkey.ptr = NULL; - pkey->pkey_sz = 0; } } } @@ -12263,8 +12383,12 @@ void wolfSSL_EVP_PKEY_free(WOLFSSL_EVP_PKEY* key) wc_FreeRng(&key->rng); if (key->pkey.ptr != NULL) { + /* Holds the private key DER for a private pkey. */ + if (key->pkey_sz > 0) + ForceZero(key->pkey.ptr, (word32)key->pkey_sz); XFREE(key->pkey.ptr, key->heap, DYNAMIC_TYPE_PUBLIC_KEY); key->pkey.ptr = NULL; + key->pkey_sz = 0; } switch(key->type) { diff --git a/wolfcrypt/src/evp_pk.c b/wolfcrypt/src/evp_pk.c index 73b9ddc3f6..7a81f0d94a 100644 --- a/wolfcrypt/src/evp_pk.c +++ b/wolfcrypt/src/evp_pk.c @@ -51,11 +51,36 @@ static int d2i_make_pkey(WOLFSSL_EVP_PKEY** out, const unsigned char* mem, word32 memSz, int priv, int type) { WOLFSSL_EVP_PKEY* pkey; + char* prevData = NULL; + int prevSz = 0; int ret = 1; /* Get or create the EVP PKEY object. */ if (*out != NULL) { pkey = *out; + /* Hold on to the data of the key this object held before. It is + * disposed of once the new key data has been copied in, as the caller + * may be decoding out of it. */ + prevData = pkey->pkey.ptr; + prevSz = pkey->pkey_sz; + pkey->pkey.ptr = NULL; + pkey->pkey_sz = 0; + #ifdef OPENSSL_EXTRA + /* Dispose of the key object of the key this object held before. The + * type is about to change and wolfSSL_EVP_PKEY_free() only disposes of + * the object matching the type set. */ + clearEVPPkeyKeys(pkey); + #endif + /* Drop metadata describing the key this object held before, so a + * reused object decodes to the same state as a new one. */ + pkey->pkcs8HeaderSz = 0; + pkey->save_type = 0; + #ifdef HAVE_ECC + pkey->pkey_curve = 0; + #endif + #ifdef WOLFSSL_HAVE_MLDSA + WOLFSSL_ATOMIC_STORE(pkey->mldsaOID, 0); + #endif } else { pkey = wolfSSL_EVP_PKEY_new(); @@ -68,9 +93,11 @@ static int d2i_make_pkey(WOLFSSL_EVP_PKEY** out, const unsigned char* mem, /* Set the size and allocate memory for key data to be copied into. */ pkey->pkey_sz = (int)memSz; if (memSz > 0) { - pkey->pkey.ptr = (char*)XMALLOC((size_t)memSz, NULL, + pkey->pkey.ptr = (char*)XMALLOC((size_t)memSz, pkey->heap, priv ? DYNAMIC_TYPE_PRIVATE_KEY : DYNAMIC_TYPE_PUBLIC_KEY); if (pkey->pkey.ptr == NULL) { + /* No encoding held - do not describe one. */ + pkey->pkey_sz = 0; ret = 0; } if (ret == 1) { @@ -78,6 +105,14 @@ static int d2i_make_pkey(WOLFSSL_EVP_PKEY** out, const unsigned char* mem, XMEMCPY(pkey->pkey.ptr, mem, memSz); } } + /* The data of the key held before is no longer referenced. */ + if (prevData != NULL) { + if (prevSz > 0) { + ForceZero(prevData, (word32)prevSz); + } + XFREE(prevData, pkey->heap, + priv ? DYNAMIC_TYPE_PRIVATE_KEY : DYNAMIC_TYPE_PUBLIC_KEY); + } if (ret == 1) { /* Set key type passed in and return object. */ pkey->type = type;