From ccc8068b056d8c8794141157a751a3289dd74800 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 13 Jul 2026 21:39:57 +0000 Subject: [PATCH 1/2] X509_STORE_CTX_set0_crls: implement for OpenVPN OpenVPN master keeps CRLs in its own stack and passes them to each verification with X509_STORE_CTX_set0_crls from its cert verify callback. CRLs are no longer loaded into the store. - Add wolfSSL_X509_STORE_CTX_set0_crls. The ctx borrows the stack. - Check the ctx CRLs in X509StoreVerifyCert. They can revoke a cert the CertManager accepted and can satisfy the CRL requirement when the CertManager has no CRL loaded. The check runs after the date override handling so that a revocation is not masked by an overridden date error. A stale CRL in the stack does not fail the check when another CRL vouches for the cert. - Add CheckCertCRLFromCm to check a cert against a caller-owned CRL using the cm of the store for CRL signature verification. The CRL object is not modified and the cached verification result of the entries is not used because it is only valid for the owning cm. - Pass the good result of the cert verify callback to the following verify callbacks in DoVerifyCallback. In OpenSSL the cert verify callback replaces chain verification so the verify callbacks only see its result. OpenVPN needs this to run its per-cert verification. - Re-add OpenVPN master to CI testing. --- .github/workflows/openvpn.yml | 2 +- src/crl.c | 115 ++++++++++++++------- src/internal.c | 4 + src/x509_str.c | 95 +++++++++++++++++ tests/api/test_ossl_x509_str.c | 180 +++++++++++++++++++++++++++++++++ tests/api/test_ossl_x509_str.h | 6 ++ wolfssl/crl.h | 2 + wolfssl/openssl/ssl.h | 1 + wolfssl/ssl.h | 10 ++ 9 files changed, 378 insertions(+), 37 deletions(-) diff --git a/.github/workflows/openvpn.yml b/.github/workflows/openvpn.yml index 644909654c5..ef139eaaff3 100644 --- a/.github/workflows/openvpn.yml +++ b/.github/workflows/openvpn.yml @@ -43,7 +43,7 @@ jobs: strategy: fail-fast: false matrix: - ref: [ release/2.6, v2.6.19 ] + ref: [ master, release/2.6, v2.6.19 ] name: ${{ matrix.ref }} if: ${{ (github.repository_owner == 'wolfssl') && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) }} runs-on: ubuntu-24.04 diff --git a/src/crl.c b/src/crl.c index 6c242eb6ee5..8b9e3269ac5 100644 --- a/src/crl.c +++ b/src/crl.c @@ -456,7 +456,10 @@ static int FindRevokedSerial(RevokedCert* rc, byte* serial, int serialSz, return ret; } -static int VerifyCRLE(const WOLFSSL_CRL* crl, CRL_Entry* crle) +/* cacheResult should only be set when cm is the owning cm of crl. A cached + * result is not valid for other cms because they can trust different CAs. */ +static int VerifyCRLE(const WOLFSSL_CRL* crl, CRL_Entry* crle, + WOLFSSL_CERT_MANAGER* cm, int cacheResult) { Signer* ca = NULL; SignatureCtx sigCtx; @@ -464,11 +467,11 @@ static int VerifyCRLE(const WOLFSSL_CRL* crl, CRL_Entry* crle) #ifndef NO_SKID if (crle->extAuthKeyIdSet) - ca = GetCA(crl->cm, crle->extAuthKeyId); + ca = GetCA(cm, crle->extAuthKeyId); if (ca == NULL) - ca = GetCAByName(crl->cm, crle->issuerHash); + ca = GetCAByName(cm, crle->issuerHash); #else /* NO_SKID */ - ca = GetCA(crl->cm, crle->issuerHash); + ca = GetCA(cm, crle->issuerHash); #endif /* NO_SKID */ if (ca == NULL) { WOLFSSL_MSG("Did NOT find CRL issuer CA"); @@ -484,18 +487,21 @@ static int VerifyCRLE(const WOLFSSL_CRL* crl, CRL_Entry* crle) #endif ca, crl->heap); - if (ret == 0) { - crle->verified = 1; - } - else { - crle->verified = ret; + if (cacheResult) { + if (ret == 0) { + crle->verified = 1; + } + else { + crle->verified = ret; + } } return ret; } static int CheckCertCRLList(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, - int serialSz, byte* serialHash, int *pFoundEntry) + int serialSz, byte* serialHash, int *pFoundEntry, + WOLFSSL_CERT_MANAGER* cm) { CRL_Entry* crle; int foundEntry = 0; @@ -512,27 +518,38 @@ static int CheckCertCRLList(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, WOLFSSL_MSG("Found CRL Entry on list"); - if (crle->verified == 0) { - if (wc_LockMutex(&crle->verifyMutex) != 0) { - WOLFSSL_MSG("wc_LockMutex failed"); + if (cm != crl->cm) { + /* cm is not the owning cm of crl. The cached result is only + * valid for the owning cm. Verify again without caching. */ + ret = VerifyCRLE(crl, crle, cm, 0); + if (ret != 0) { + WOLFSSL_MSG("Cannot use CRL as it didn't verify"); break; } + } + else { + if (crle->verified == 0) { + if (wc_LockMutex(&crle->verifyMutex) != 0) { + WOLFSSL_MSG("wc_LockMutex failed"); + break; + } - /* A different thread may have verified the entry while we were - * waiting for the mutex. */ - if (crle->verified == 0) - ret = VerifyCRLE(crl, crle); + /* A different thread may have verified the entry while we + * were waiting for the mutex. */ + if (crle->verified == 0) + ret = VerifyCRLE(crl, crle, cm, 1); - wc_UnLockMutex(&crle->verifyMutex); + wc_UnLockMutex(&crle->verifyMutex); - if (ret != 0) - break; - } + if (ret != 0) + break; + } - if (crle->verified < 0) { - WOLFSSL_MSG("Cannot use CRL as it didn't verify"); - ret = crle->verified; - break; + if (crle->verified < 0) { + WOLFSSL_MSG("Cannot use CRL as it didn't verify"); + ret = crle->verified; + break; + } } WOLFSSL_MSG("Checking next date validity"); @@ -569,9 +586,12 @@ static int CheckCertCRLList(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, return ret; } -int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, +/* cm is the CertManager to use for CRL signature verification and + * callbacks. It can differ from crl->cm when checking CRLs the app supplied + * with X509_STORE_CTX_set0_crls. The caller-owned crl is not modified. */ +static int CheckCertCRLCm(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, int serialSz, byte* serialHash, const byte* extCrlInfo, - int extCrlInfoSz, void* issuerName) + int extCrlInfoSz, void* issuerName, WOLFSSL_CERT_MANAGER* cm) { int foundEntry = 0; int ret = 0; @@ -592,7 +612,7 @@ int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, #endif ret = CheckCertCRLList(crl, issuerHash, serial, serialSz, serialHash, - &foundEntry); + &foundEntry, cm); #ifdef HAVE_CRL_IO if (foundEntry == 0) { @@ -606,7 +626,7 @@ int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, else if (cbRet >= 0) { /* try again */ ret = CheckCertCRLList(crl, issuerHash, serial, serialSz, - serialHash, &foundEntry); + serialHash, &foundEntry, cm); } } } @@ -623,13 +643,13 @@ int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, /* and try again checking Cert in the CRL list. */ /* When not set the folder or not use hash_dir, do nothing. */ if ((foundEntry == 0) && (ret != WC_NO_ERR_TRACE(OCSP_WANT_READ))) { - if (crl->cm != NULL && crl->cm->x509_store_p != NULL) { - int loadRet = LoadCertByIssuer(crl->cm->x509_store_p, + if (cm != NULL && cm->x509_store_p != NULL) { + int loadRet = LoadCertByIssuer(cm->x509_store_p, (WOLFSSL_X509_NAME*)issuerName, X509_LU_CRL); if (loadRet == WOLFSSL_SUCCESS) { /* try again */ ret = CheckCertCRLList(crl, issuerHash, serial, serialSz, - serialHash, &foundEntry); + serialHash, &foundEntry, cm); } } } @@ -640,7 +660,7 @@ int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, ret = CRL_MISSING; } - if (crl->cm != NULL && crl->cm->cbMissingCRL) { + if (cm != NULL && cm->cbMissingCRL) { char url[256]; WOLFSSL_MSG("Issuing missing CRL callback"); @@ -655,11 +675,11 @@ int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, } } - crl->cm->cbMissingCRL(url); + cm->cbMissingCRL(url); } - if (crl->cm != NULL && crl->cm->crlCb && - crl->cm->crlCb(ret, crl, crl->cm, crl->cm->crlCbCtx)) { + if (cm != NULL && cm->crlCb && + cm->crlCb(ret, crl, cm, cm->crlCbCtx)) { if (ret != 0) WOLFSSL_MSG("Overriding CRL error"); ret = 0; @@ -669,6 +689,14 @@ int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, return ret; } +int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, + int serialSz, byte* serialHash, const byte* extCrlInfo, + int extCrlInfoSz, void* issuerName) +{ + return CheckCertCRLCm(crl, issuerHash, serial, serialSz, serialHash, + extCrlInfo, extCrlInfoSz, issuerName, crl->cm); +} + /* Is the cert ok with CRL, return 0 on success */ int CheckCertCRL(WOLFSSL_CRL* crl, DecodedCert* cert) { @@ -681,6 +709,21 @@ int CheckCertCRL(WOLFSSL_CRL* crl, DecodedCert* cert) NULL, cert->extCrlInfo, cert->extCrlInfoSz, issuerName); } +/* Check cert against crl using cm for CRL signature verification. Does not + * modify crl, so crl can be a caller-owned object shared between threads. + * Return 0 on success. */ +int CheckCertCRLFromCm(WOLFSSL_CERT_MANAGER* cm, WOLFSSL_CRL* crl, + DecodedCert* cert) +{ +#if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) + void* issuerName = cert->issuerName; +#else + void* issuerName = NULL; +#endif + return CheckCertCRLCm(crl, cert->issuerHash, cert->serial, cert->serialSz, + NULL, cert->extCrlInfo, cert->extCrlInfoSz, issuerName, cm); +} + #ifdef HAVE_CRL_UPDATE_CB static void SetCrlInfo(CRL_Entry* entry, CrlInfo *info) { diff --git a/src/internal.c b/src/internal.c index 5914aa42033..5c211ca386c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -15565,6 +15565,10 @@ int DoVerifyCallback(WOLFSSL_CERT_MANAGER* cm, WOLFSSL* ssl, int cert_err, if (cert_err != 0) { WOLFSSL_MSG("Verify Cert callback overriding error!"); ret = 0; + /* The app's cert verify callback replaces chain + * verification in OpenSSL. Pass its good result on to + * the following verify callbacks. */ + verify_ok = 1; } } else { diff --git a/src/x509_str.c b/src/x509_str.c index 710d18b74c0..d39f84ae5e3 100644 --- a/src/x509_str.c +++ b/src/x509_str.c @@ -203,6 +203,9 @@ int wolfSSL_X509_STORE_CTX_init(WOLFSSL_X509_STORE_CTX* ctx, #endif ctx->ctxIntermediates = sk; +#ifdef HAVE_CRL + ctx->crls = NULL; +#endif if (ctx->chain != NULL) { wolfSSL_sk_X509_free(ctx->chain); ctx->chain = NULL; @@ -262,6 +265,20 @@ void wolfSSL_X509_STORE_CTX_cleanup(WOLFSSL_X509_STORE_CTX* ctx) } +#ifdef HAVE_CRL +/* Set the CRLs to use during certificate verification. The stack is not + * copied. The caller keeps ownership and has to keep the stack valid as long + * as it is set on the ctx. */ +void wolfSSL_X509_STORE_CTX_set0_crls(WOLFSSL_X509_STORE_CTX *ctx, + WOLF_STACK_OF(WOLFSSL_X509_CRL) *sk) +{ + WOLFSSL_ENTER("wolfSSL_X509_STORE_CTX_set0_crls"); + if (ctx != NULL) { + ctx->crls = sk; + } +} +#endif + void wolfSSL_X509_STORE_CTX_trusted_stack(WOLFSSL_X509_STORE_CTX *ctx, WOLF_STACK_OF(WOLFSSL_X509) *sk) { @@ -430,6 +447,67 @@ static int X509StoreVerifyCertDate(WOLFSSL_X509_STORE_CTX* ctx, int ret) } #endif /* NO_ASN_TIME */ +#ifdef HAVE_CRL +/* Check ctx->current_cert against the CRLs set with + * X509_STORE_CTX_set0_crls. + * Returns WOLFSSL_SUCCESS if a CRL for the cert's issuer is in the stack and + * the cert is not revoked. Returns CRL_MISSING if the stack has no CRL for + * the issuer. Returns a negative error on revocation or CRL failure. */ +static int X509StoreCheckCtxCrls(WOLFSSL_X509_STORE_CTX* ctx) +{ + int ret = WC_NO_ERR_TRACE(CRL_MISSING); + int found = 0; + int dateErr = 0; + int i; + int numCrls; + WC_DECLARE_VAR(cert, DecodedCert, 1, 0); + + numCrls = wolfSSL_sk_X509_CRL_num(ctx->crls); + if (numCrls <= 0) + return ret; + + WC_ALLOC_VAR_EX(cert, DecodedCert, 1, ctx->heap, DYNAMIC_TYPE_DCERT, + return MEMORY_E); + + InitDecodedCert(cert, ctx->current_cert->derCert->buffer, + ctx->current_cert->derCert->length, ctx->heap); + /* The cert signature is verified by the CertManager. Only the issuer and + * serial info is needed here. */ + if (ParseCertRelative(cert, CERT_TYPE, NO_VERIFY, ctx->store->cm, NULL) + == 0) { + /* Check all CRLs in the stack. A revocation in any of them wins over + * a CRL that does not list the cert, like in the CertManager. */ + for (i = 0; i < numCrls; i++) { + WOLFSSL_X509_CRL* crl = wolfSSL_sk_X509_CRL_value(ctx->crls, i); + if (crl == NULL) + continue; + /* Use the store's cm to verify the CRL. The caller-owned crl is + * not modified. */ + ret = CheckCertCRLFromCm(ctx->store->cm, crl, cert); + if (ret == 0) + found = 1; + else if (ret == WC_NO_ERR_TRACE(CRL_CERT_DATE_ERR)) + dateErr = 1; /* stale CRL, another CRL can still vouch */ + else if (ret != WC_NO_ERR_TRACE(CRL_MISSING)) + break; + } + } + FreeDecodedCert(cert); + WC_FREE_VAR_EX(cert, ctx->heap, DYNAMIC_TYPE_DCERT); + + if (ret == 0 || ret == WC_NO_ERR_TRACE(CRL_MISSING) || + ret == WC_NO_ERR_TRACE(CRL_CERT_DATE_ERR)) { + if (found) + ret = WOLFSSL_SUCCESS; + else if (dateErr) + ret = WC_NO_ERR_TRACE(CRL_CERT_DATE_ERR); + else + ret = WC_NO_ERR_TRACE(CRL_MISSING); + } + return ret; +} +#endif /* HAVE_CRL */ + static int X509StoreVerifyCert(WOLFSSL_X509_STORE_CTX* ctx) { int ret = WC_NO_ERR_TRACE(WOLFSSL_FAILURE); @@ -444,6 +522,23 @@ static int X509StoreVerifyCert(WOLFSSL_X509_STORE_CTX* ctx) /* update return value with any date validation overrides */ ret = X509StoreVerifyCertDate(ctx, ret); #endif +#ifdef HAVE_CRL + /* Consult the CRLs set with X509_STORE_CTX_set0_crls after the date + * overrides. They can revoke a cert the CertManager accepted, also + * one whose date error was overridden, and can satisfy a CRL + * requirement the CertManager's own CRL store could not. */ + if (ctx->crls != NULL && ctx->store->cm->crlEnabled && + (ret == WOLFSSL_SUCCESS || + ret == WC_NO_ERR_TRACE(CRL_MISSING))) { + int crlRet = X509StoreCheckCtxCrls(ctx); + if (crlRet == WOLFSSL_SUCCESS) { + ret = WOLFSSL_SUCCESS; + } + else if (crlRet != WC_NO_ERR_TRACE(CRL_MISSING)) { + ret = crlRet; + } + } +#endif SetupStoreCtxError(ctx, ret); #if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) if (ctx->store->verify_cb) diff --git a/tests/api/test_ossl_x509_str.c b/tests/api/test_ossl_x509_str.c index 85b4b3fb5c6..2257fa5ed35 100644 --- a/tests/api/test_ossl_x509_str.c +++ b/tests/api/test_ossl_x509_str.c @@ -2778,6 +2778,186 @@ int test_wolfSSL_X509_STORE_set_get_crl(void) return EXPECT_RESULT(); } +#if defined(OPENSSL_EXTRA) && defined(HAVE_CRL) && !defined(NO_RSA) && \ + !defined(NO_FILESYSTEM) && !defined(WOLFSSL_CRL_ALLOW_MISSING_CDP) +/* Load a CRL from a PEM file and push it onto sk. */ +static int test_set0_crls_push_crl(STACK_OF(X509_CRL)* sk, const char* file) +{ + EXPECT_DECLS; + X509_CRL* crl = NULL; + XFILE fp = XBADFILE; + + ExpectTrue((fp = XFOPEN(file, "rb")) != XBADFILE); + ExpectNotNull(crl = (X509_CRL*)PEM_read_X509_CRL(fp, (X509_CRL**)NULL, + NULL, NULL)); + if (fp != XBADFILE) + XFCLOSE(fp); + ExpectIntGT(sk_X509_CRL_push(sk, crl), 0); + if (EXPECT_RESULT() != TEST_SUCCESS) + X509_CRL_free(crl); + return EXPECT_RESULT(); +} +#endif + +int test_wolfSSL_X509_STORE_CTX_set0_crls(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && defined(HAVE_CRL) && !defined(NO_RSA) && \ + !defined(NO_FILESYSTEM) && !defined(WOLFSSL_CRL_ALLOW_MISSING_CDP) + X509_STORE* store = NULL; + X509_STORE_CTX* storeCtx = NULL; + X509* ca = NULL; + X509* cert = NULL; + X509* revoked = NULL; + STACK_OF(X509_CRL)* crls = NULL; + const char caCert[] = "./certs/ca-cert.pem"; + const char srvCert[] = "./certs/server-cert.pem"; + const char srvRevokedCert[] = "./certs/server-revoked-cert.pem"; + const char crlPem[] = "./certs/crl/crl.pem"; + const char crlRevoked[] = "./certs/crl/crl.revoked"; + + ExpectNotNull(store = X509_STORE_new()); + ExpectNotNull(ca = wolfSSL_X509_load_certificate_file(caCert, + SSL_FILETYPE_PEM)); + ExpectIntEQ(X509_STORE_add_cert(store, ca), SSL_SUCCESS); + ExpectIntEQ(X509_STORE_set_flags(store, + X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL), SSL_SUCCESS); + ExpectNotNull(cert = wolfSSL_X509_load_certificate_file(srvCert, + SSL_FILETYPE_PEM)); + ExpectNotNull(revoked = wolfSSL_X509_load_certificate_file(srvRevokedCert, + SSL_FILETYPE_PEM)); + ExpectNotNull(storeCtx = X509_STORE_CTX_new()); + + ExpectNotNull(crls = sk_X509_CRL_new_null()); + ExpectIntEQ(test_set0_crls_push_crl(crls, crlPem), TEST_SUCCESS); + + /* No CRLs available. Verification has to fail. */ + ExpectIntEQ(X509_STORE_CTX_init(storeCtx, store, cert, NULL), SSL_SUCCESS); + ExpectIntNE(X509_verify_cert(storeCtx), SSL_SUCCESS); + ExpectIntEQ(X509_STORE_CTX_get_error(storeCtx), + WOLFSSL_X509_V_ERR_UNABLE_TO_GET_CRL); + + /* The CRL from the ctx satisfies the CRL check. */ + ExpectIntEQ(X509_STORE_CTX_init(storeCtx, store, cert, NULL), SSL_SUCCESS); + X509_STORE_CTX_set0_crls(storeCtx, crls); + ExpectIntEQ(X509_verify_cert(storeCtx), SSL_SUCCESS); + + /* X509_STORE_CTX_init clears the CRLs again. */ + ExpectIntEQ(X509_STORE_CTX_init(storeCtx, store, cert, NULL), SSL_SUCCESS); + ExpectIntNE(X509_verify_cert(storeCtx), SSL_SUCCESS); + ExpectIntEQ(X509_STORE_CTX_get_error(storeCtx), + WOLFSSL_X509_V_ERR_UNABLE_TO_GET_CRL); + + /* A revocation in any CRL of the stack is found. */ + ExpectIntEQ(test_set0_crls_push_crl(crls, crlRevoked), TEST_SUCCESS); + ExpectIntEQ(X509_STORE_CTX_init(storeCtx, store, revoked, NULL), + SSL_SUCCESS); + X509_STORE_CTX_set0_crls(storeCtx, crls); + ExpectIntNE(X509_verify_cert(storeCtx), SSL_SUCCESS); + ExpectIntEQ(X509_STORE_CTX_get_error(storeCtx), + WOLFSSL_X509_V_ERR_CERT_REVOKED); + + /* The ctx does not own the CRL stack. Freeing it here must not lead to a + * double free when the ctx is freed. */ + sk_X509_CRL_pop_free(crls, X509_CRL_free); + X509_STORE_CTX_free(storeCtx); + X509_STORE_free(store); + X509_free(revoked); + X509_free(cert); + X509_free(ca); +#endif + return EXPECT_RESULT(); +} + +#if defined(HAVE_SSL_MEMIO_TESTS_DEPENDENCIES) && defined(OPENSSL_ALL) && \ + defined(HAVE_CRL) && !defined(WOLFSSL_CRL_ALLOW_MISSING_CDP) +static STACK_OF(X509_CRL)* test_set0_crls_stack; +static int test_set0_crls_preverify; /* preverify_ok seen at depth 0 */ +static int test_set0_crls_error; /* error seen when preverify_ok == 0 */ + +/* Mimics OpenVPN's cert_verify_callback. */ +static int test_set0_crls_cert_verify_cb(X509_STORE_CTX* ctx, void* arg) +{ + (void)arg; + X509_STORE_CTX_set0_crls(ctx, test_set0_crls_stack); + return X509_verify_cert(ctx); +} + +/* Mimics OpenVPN's verify_callback. */ +static int test_set0_crls_verify_cb(int preverify_ok, X509_STORE_CTX* ctx) +{ + if (X509_STORE_CTX_get_error_depth(ctx) == 0) + test_set0_crls_preverify = preverify_ok; + if (!preverify_ok) { + test_set0_crls_error = X509_STORE_CTX_get_error(ctx); + return 0; + } + return 1; +} + +static int test_set0_crls_ctx_ready(WOLFSSL_CTX* ctx) +{ + EXPECT_DECLS; + X509_STORE* store = NULL; + + SSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER, test_set0_crls_verify_cb); + SSL_CTX_set_cert_verify_callback(ctx, test_set0_crls_cert_verify_cb, NULL); + ExpectNotNull(store = SSL_CTX_get_cert_store(ctx)); + ExpectIntEQ(X509_STORE_set_flags(store, + X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL), SSL_SUCCESS); + return EXPECT_RESULT(); +} +#endif + +/* Mimics OpenVPN's CRL handling. OpenVPN keeps the CRLs in its own stack and + * passes them to each verification with X509_STORE_CTX_set0_crls from the + * cert verify callback. No CRLs are ever loaded into the store. */ +int test_wolfSSL_X509_STORE_CTX_set0_crls_handshake(void) +{ + EXPECT_DECLS; +#if defined(HAVE_SSL_MEMIO_TESTS_DEPENDENCIES) && defined(OPENSSL_ALL) && \ + defined(HAVE_CRL) && !defined(WOLFSSL_CRL_ALLOW_MISSING_CDP) + test_ssl_cbf client_cbs; + test_ssl_cbf server_cbs; + + /* The CRL stack covers the whole chain. The handshake succeeds and the + * verify callback sees the good result of the cert verify callback. */ + XMEMSET(&client_cbs, 0, sizeof(client_cbs)); + XMEMSET(&server_cbs, 0, sizeof(server_cbs)); + client_cbs.ctx_ready = test_set0_crls_ctx_ready; + ExpectNotNull(test_set0_crls_stack = sk_X509_CRL_new_null()); + ExpectIntEQ(test_set0_crls_push_crl(test_set0_crls_stack, + "./certs/crl/crl.pem"), TEST_SUCCESS); + test_set0_crls_preverify = -1; + test_set0_crls_error = 0; + ExpectIntEQ(test_wolfSSL_client_server_nofail_memio(&client_cbs, + &server_cbs, NULL), TEST_SUCCESS); + ExpectIntEQ(test_set0_crls_preverify, 1); + sk_X509_CRL_pop_free(test_set0_crls_stack, X509_CRL_free); + test_set0_crls_stack = NULL; + + /* The server presents a revoked cert. The handshake has to fail. */ + XMEMSET(&client_cbs, 0, sizeof(client_cbs)); + XMEMSET(&server_cbs, 0, sizeof(server_cbs)); + client_cbs.ctx_ready = test_set0_crls_ctx_ready; + server_cbs.certPemFile = "./certs/server-revoked-cert.pem"; + server_cbs.keyPemFile = "./certs/server-revoked-key.pem"; + ExpectNotNull(test_set0_crls_stack = sk_X509_CRL_new_null()); + ExpectIntEQ(test_set0_crls_push_crl(test_set0_crls_stack, + "./certs/crl/crl.pem"), TEST_SUCCESS); + ExpectIntEQ(test_set0_crls_push_crl(test_set0_crls_stack, + "./certs/crl/crl.revoked"), TEST_SUCCESS); + test_set0_crls_preverify = -1; + test_set0_crls_error = 0; + ExpectIntEQ(test_wolfSSL_client_server_nofail_memio(&client_cbs, + &server_cbs, NULL), -1001); + ExpectIntEQ(test_set0_crls_error, WOLFSSL_X509_V_ERR_CERT_REVOKED); + sk_X509_CRL_pop_free(test_set0_crls_stack, X509_CRL_free); + test_set0_crls_stack = NULL; +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_X509_CA_num(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_x509_str.h b/tests/api/test_ossl_x509_str.h index 5f1cdc9e08c..d320133b627 100644 --- a/tests/api/test_ossl_x509_str.h +++ b/tests/api/test_ossl_x509_str.h @@ -46,6 +46,8 @@ int test_wolfSSL_X509_STORE_load_locations(void); int test_X509_STORE_get0_objects(void); int test_wolfSSL_X509_STORE_get1_certs(void); int test_wolfSSL_X509_STORE_set_get_crl(void); +int test_wolfSSL_X509_STORE_CTX_set0_crls(void); +int test_wolfSSL_X509_STORE_CTX_set0_crls_handshake(void); int test_wolfSSL_X509_CA_num(void); int test_X509_STORE_No_SSL_CTX(void); int test_wolfSSL_CTX_set_cert_store(void); @@ -82,6 +84,10 @@ int test_wolfSSL_CTX_set_cert_store(void); TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_get0_objects), \ TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_X509_STORE_get1_certs), \ TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_X509_STORE_set_get_crl), \ + TEST_DECL_GROUP("ossl_x509_store", \ + test_wolfSSL_X509_STORE_CTX_set0_crls), \ + TEST_DECL_GROUP("ossl_x509_store", \ + test_wolfSSL_X509_STORE_CTX_set0_crls_handshake), \ TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_X509_CA_num), \ TEST_DECL_GROUP("ossl_x509_store", test_X509_STORE_No_SSL_CTX), \ TEST_DECL_GROUP("ossl_x509_store", test_wolfSSL_CTX_set_cert_store) diff --git a/wolfssl/crl.h b/wolfssl/crl.h index 14a007ce459..0a1c72d7d5e 100644 --- a/wolfssl/crl.h +++ b/wolfssl/crl.h @@ -48,6 +48,8 @@ WOLFSSL_LOCAL int CheckCertCRL(WOLFSSL_CRL* crl, DecodedCert* cert); WOLFSSL_LOCAL int CheckCertCRL_ex(WOLFSSL_CRL* crl, byte* issuerHash, byte* serial, int serialSz, byte* serialHash, const byte* extCrlInfo, int extCrlInfoSz, void* issuerName); +WOLFSSL_LOCAL int CheckCertCRLFromCm(WOLFSSL_CERT_MANAGER* cm, + WOLFSSL_CRL* crl, DecodedCert* cert); #ifdef HAVE_CRL_UPDATE_CB WOLFSSL_LOCAL int GetCRLInfo(WOLFSSL_CRL* crl, CrlInfo* info, const byte* buff, long sz, int type); diff --git a/wolfssl/openssl/ssl.h b/wolfssl/openssl/ssl.h index cdc8ea64adb..d76d49edfb3 100644 --- a/wolfssl/openssl/ssl.h +++ b/wolfssl/openssl/ssl.h @@ -771,6 +771,7 @@ typedef WOLFSSL_X509_NAME_ENTRY X509_NAME_ENTRY; #define X509_STORE_CTX_get0_store wolfSSL_X509_STORE_CTX_get0_store #define X509_STORE_CTX_get0_cert wolfSSL_X509_STORE_CTX_get0_cert #define X509_STORE_CTX_trusted_stack wolfSSL_X509_STORE_CTX_trusted_stack +#define X509_STORE_CTX_set0_crls wolfSSL_X509_STORE_CTX_set0_crls #define X509_STORE_set_verify_cb(s, c) \ wolfSSL_X509_STORE_set_verify_cb((WOLFSSL_X509_STORE *)(s), (WOLFSSL_X509_STORE_CTX_verify_cb)(c)) diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index b71810d3181..8c2443423c8 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -722,6 +722,12 @@ struct WOLFSSL_X509_STORE_CTX { WOLF_STACK_OF(WOLFSSL_X509)* setTrustedSk;/* A trusted stack override * set with * X509_STORE_CTX_trusted_stack */ +#ifdef HAVE_CRL + WOLF_STACK_OF(WOLFSSL_X509_CRL)* crls; /* CRLs to use during verification, + * set with + * X509_STORE_CTX_set0_crls. + * Not owned by this ctx. */ +#endif #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ }; @@ -2458,6 +2464,10 @@ WOLFSSL_API int wolfSSL_X509_STORE_CTX_init(WOLFSSL_X509_STORE_CTX* ctx, WOLFSSL_API void wolfSSL_X509_STORE_CTX_cleanup(WOLFSSL_X509_STORE_CTX* ctx); WOLFSSL_API void wolfSSL_X509_STORE_CTX_trusted_stack(WOLFSSL_X509_STORE_CTX *ctx, WOLF_STACK_OF(WOLFSSL_X509) *sk); +#ifdef HAVE_CRL +WOLFSSL_API void wolfSSL_X509_STORE_CTX_set0_crls(WOLFSSL_X509_STORE_CTX *ctx, + WOLF_STACK_OF(WOLFSSL_X509_CRL) *sk); +#endif WOLFSSL_API WOLFSSL_ASN1_TIME* wolfSSL_X509_CRL_get_lastUpdate(WOLFSSL_X509_CRL* crl); WOLFSSL_API int wolfSSL_X509_CRL_set_lastUpdate(WOLFSSL_X509_CRL* crl, From 1a32f1b9afcc91b15ac14680ed2ca341ae5563a4 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 14 Jul 2026 10:46:57 +0000 Subject: [PATCH 2/2] X509_STORE_CTX: chain owns references to its certs Fixes a heap-use-after-free found by ASAN in the sanitize-asan CI config. wolfSSL_X509_verify_cert filled ctx->chain with borrowed pointers while CleanupStoreCtxCallback frees the chain members it expects from X509_STORE_CTX_get_chain. Calling X509_verify_cert from a cert verify callback, like OpenVPN does, freed the certs twice. Take a reference for each cert pushed onto the chain and free the members whenever the chain is freed, like OpenSSL. Also address review comments: - Pass a cert verify callback failure on to the following verify callbacks as preverify_ok = 0. - Only define X509_STORE_CTX_set0_crls with HAVE_CRL. --- src/internal.c | 3 +++ src/x509_str.c | 35 +++++++++++++++++++++++++++-------- wolfssl/openssl/ssl.h | 2 ++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5c211ca386c..0ad498a6530 100644 --- a/src/internal.c +++ b/src/internal.c @@ -15573,6 +15573,9 @@ int DoVerifyCallback(WOLFSSL_CERT_MANAGER* cm, WOLFSSL* ssl, int cert_err, } else { verifyFail = 1; + /* Pass the failure on to the following verify + * callbacks. */ + verify_ok = 0; } } #endif diff --git a/src/x509_str.c b/src/x509_str.c index d39f84ae5e3..7bd2eb22f11 100644 --- a/src/x509_str.c +++ b/src/x509_str.c @@ -92,7 +92,7 @@ void wolfSSL_X509_STORE_CTX_free(WOLFSSL_X509_STORE_CTX* ctx) ctx->param = NULL; if (ctx->chain != NULL) { - wolfSSL_sk_X509_free(ctx->chain); + wolfSSL_sk_X509_pop_free(ctx->chain, NULL); } if (ctx->owned != NULL) { wolfSSL_sk_X509_pop_free(ctx->owned, NULL); @@ -207,7 +207,7 @@ int wolfSSL_X509_STORE_CTX_init(WOLFSSL_X509_STORE_CTX* ctx, ctx->crls = NULL; #endif if (ctx->chain != NULL) { - wolfSSL_sk_X509_free(ctx->chain); + wolfSSL_sk_X509_pop_free(ctx->chain, NULL); ctx->chain = NULL; } #ifdef SESSION_CERTS @@ -663,6 +663,22 @@ static int X509StoreRemoveCert(WOLFSSL_STACK *stack, WOLFSSL_X509 *cert) { } +/* Push x509 onto the ctx chain with its own reference, like OpenSSL. + * The chain owns a reference to each of its certs. */ +static int X509StoreChainPush(WOLF_STACK_OF(WOLFSSL_X509)* chain, + WOLFSSL_X509* x509) +{ + int ret = WC_NO_ERR_TRACE(WOLFSSL_FAILURE); + + if (x509 == NULL || wolfSSL_X509_up_ref(x509) != WOLFSSL_SUCCESS) + return ret; + ret = wolfSSL_sk_X509_push(chain, x509) > 0 ? WOLFSSL_SUCCESS : + WC_NO_ERR_TRACE(WOLFSSL_FAILURE); + if (ret != WOLFSSL_SUCCESS) + wolfSSL_X509_free(x509); + return ret; +} + /* Current certificate failed, but it is possible there is an * alternative cert with the same subject key which will work. * Retry until all possible candidate certs are exhausted. */ @@ -677,6 +693,9 @@ static int X509VerifyCertSetupRetry(WOLFSSL_X509_STORE_CTX* ctx, WOLFSSL_TEMP_CA); X509StoreMoveCert(certs, failed, ctx->current_cert); ctx->current_cert = wolfSSL_sk_X509_pop(ctx->chain); + /* Release the chain's reference. The cert stays valid through its + * original owner. */ + wolfSSL_X509_free(ctx->current_cert); if (*depth < origDepth) *depth += 1; @@ -907,7 +926,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) } if (ctx->chain != NULL) { - wolfSSL_sk_X509_free(ctx->chain); + wolfSSL_sk_X509_pop_free(ctx->chain, NULL); } ctx->chain = wolfSSL_sk_X509_new_null(); if (ctx->chain == NULL) { @@ -941,7 +960,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) ctx->current_cert); if (ret == WOLFSSL_SUCCESS) { if (ctx->current_cert == issuer) { - wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert); + X509StoreChainPush(ctx->chain, ctx->current_cert); break; } @@ -988,7 +1007,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) continue; } /* Add it to the current chain and look at the issuer cert next */ - wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert); + X509StoreChainPush(ctx->chain, ctx->current_cert); ctx->current_cert = issuer; } else if (ret == WC_NO_ERR_TRACE(WOLFSSL_FAILURE)) { @@ -1025,7 +1044,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) (ctx->store->param->flags & WOLFSSL_PARTIAL_CHAIN))) && X509StoreCertIsTrusted(ctx->store, ctx->current_cert, origTrustedSk)) { - wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert); + X509StoreChainPush(ctx->chain, ctx->current_cert); /* Clear error set by the failed X509StoreVerifyCert * attempt; the partial-chain fallback accepted the * chain at a caller-trusted certificate. */ @@ -1046,7 +1065,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) } /* Cert verified, finish building the chain */ - wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert); + X509StoreChainPush(ctx->chain, ctx->current_cert); issuer = NULL; #ifdef WOLFSSL_SIGNER_DER_CERT x509GetIssuerFromCM(&issuer, ctx->store->cm, ctx->current_cert); @@ -1064,7 +1083,7 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx) } #endif if (issuer != NULL) { - wolfSSL_sk_X509_push(ctx->chain, issuer); + X509StoreChainPush(ctx->chain, issuer); } done = 1; diff --git a/wolfssl/openssl/ssl.h b/wolfssl/openssl/ssl.h index d76d49edfb3..f550eb5aa33 100644 --- a/wolfssl/openssl/ssl.h +++ b/wolfssl/openssl/ssl.h @@ -771,7 +771,9 @@ typedef WOLFSSL_X509_NAME_ENTRY X509_NAME_ENTRY; #define X509_STORE_CTX_get0_store wolfSSL_X509_STORE_CTX_get0_store #define X509_STORE_CTX_get0_cert wolfSSL_X509_STORE_CTX_get0_cert #define X509_STORE_CTX_trusted_stack wolfSSL_X509_STORE_CTX_trusted_stack +#ifdef HAVE_CRL #define X509_STORE_CTX_set0_crls wolfSSL_X509_STORE_CTX_set0_crls +#endif #define X509_STORE_set_verify_cb(s, c) \ wolfSSL_X509_STORE_set_verify_cb((WOLFSSL_X509_STORE *)(s), (WOLFSSL_X509_STORE_CTX_verify_cb)(c))