Skip to content

New API for CRL unknown extension callback #10961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anhu wants to merge 3 commits into
base: master
Choose a base branch
from
Open

New API for CRL unknown extension callback #10961

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b0e0b9
New API for CRL unknown extension callback
anhu Jul 21, 2026
7ea5bb6
Set to NULL for non-template ASN
anhu Jul 21, 2026
73f1a7b
More tests
anhu Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/crl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,13 @@ int BufferLoadCRL(WOLFSSL_CRL* crl, const byte* buff, long sz, int type,
}

@Frauschi Frauschi Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [Medium] New CRL unknown-extension callback is a silent no-op on every OpenSSL-compat CRL load path
💡 SUGGEST

Propagation of the new callback into the DecodedCRL is gated on crl->cm != NULL. Several public CRL-loading paths parse through a WOLFSSL_CRL whose cm is NULL, so the setter has no effect there:

  • wolfSSL_d2i_X509_CRL() (src/x509.c:9480) does InitCRL(newcrl, NULL) then BufferLoadCRL(newcrl, ...). This is the terminus for d2i_X509_CRL, d2i_X509_CRL_bio, d2i_X509_CRL_fp, PEM_read_bio_X509_CRL and wolfSSL_X509_load_crl_file (src/x509.c:9352, 9373).
  • wolfSSL_X509_crl_new() / wolfSSL_X509_CRL_new() (src/crl.c:2417) likewise pass NULL.
  • wolfSSL_X509_STORE_add_crl() (src/crl.c:1543) only splices an already-parsed entry list into store->cm->crl; attaching the CRL to a cert manager afterwards cannot retroactively apply the callback.

Concrete caller path: an OPENSSL_EXTRA/OPENSSL_ALL application calls wolfSSL_CertManagerSetCRLUnknownExtCallbackEx(cm, cb, ctx) (returns WOLFSSL_SUCCESS), then loads its CRL the OpenSSL way (PEM_read_bio_X509_CRL + X509_STORE_add_crl, or X509_STORE_load_locations). The CRL with a critical unknown extension is still rejected with ASN_CRIT_EXT_E inside d2i, which returns NULL — the callback never fires and X509_STORE_add_crl is never reached. Nothing in the return value or the header comment signals that the registration had no effect. The wolfSSL-native paths (wolfSSL_CertManagerLoadCRL*, ProcessFile, the CRL monitor via InitCRL(tmp, crl->cm), HAVE_CRL_IO) do work, so behaviour depends on which of two equivalent-looking load APIs the caller picked.

Recommendation: Either add an internal d2i variant that takes the store/cert manager (with wolfSSL_X509_load_crl_file and the X509_LOOKUP helpers passing ctx->store->cm), or add a WOLFSSL_X509_CRL-level setter so a standalone CRL object can carry the callback. At minimum, state the limitation explicitly in the ssl.h doc comment: only CRLs parsed through this cert manager are covered; CRLs parsed with d2i_X509_CRL/PEM_read_bio_X509_CRL are not.

InitDecodedCRL(dcrl, crl->heap);
#ifdef WC_ASN_UNKNOWN_EXT_CB
if (crl->cm != NULL) {
dcrl->unknownExtCallback = crl->cm->crlUnknownExtCallback;
dcrl->unknownExtCallbackEx = crl->cm->crlUnknownExtCallbackEx;
dcrl->unknownExtCallbackExCtx = crl->cm->crlUnknownExtCallbackExCtx;
}
#endif
ret = ParseCRL(currentEntry->certs, dcrl, myBuffer, (word32)sz,
verify, crl->cm);

Expand Down Expand Up @@ -1224,6 +1231,13 @@ int GetCRLInfo(WOLFSSL_CRL* crl, CrlInfo* info, const byte* buff,
}

InitDecodedCRL(dcrl, crl->heap);
#ifdef WC_ASN_UNKNOWN_EXT_CB
if (crl->cm != NULL) {
dcrl->unknownExtCallback = crl->cm->crlUnknownExtCallback;
dcrl->unknownExtCallbackEx = crl->cm->crlUnknownExtCallbackEx;
dcrl->unknownExtCallbackExCtx = crl->cm->crlUnknownExtCallbackExCtx;
}
#endif
ret = ParseCRL(crle->certs, dcrl, myBuffer, (word32)sz,
0, crl->cm);
if (ret != 0 && !(ret == WC_NO_ERR_TRACE(ASN_CRL_NO_SIGNER_E))) {
Expand Down
25 changes: 25 additions & 0 deletions src/ssl_certman.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,31 @@ void wolfSSL_CertManagerSetUnknownExtCallback(WOLFSSL_CERT_MANAGER* cm,
}

}

#ifdef HAVE_CRL
int wolfSSL_CertManagerSetCRLUnknownExtCallback(WOLFSSL_CERT_MANAGER* cm,
wc_UnknownExtCallback cb)
{
WOLFSSL_ENTER("wolfSSL_CertManagerSetCRLUnknownExtCallback");
if (cm == NULL) {
return BAD_FUNC_ARG;
}
cm->crlUnknownExtCallback = cb;
return WOLFSSL_SUCCESS;
}

int wolfSSL_CertManagerSetCRLUnknownExtCallbackEx(WOLFSSL_CERT_MANAGER* cm,
wc_UnknownExtCallbackEx cb, void* ctx)
{
WOLFSSL_ENTER("wolfSSL_CertManagerSetCRLUnknownExtCallbackEx");
if (cm == NULL) {
return BAD_FUNC_ARG;
}
cm->crlUnknownExtCallbackEx = cb;
cm->crlUnknownExtCallbackExCtx = ctx;
return WOLFSSL_SUCCESS;
}
#endif /* HAVE_CRL */
#endif /* WC_ASN_UNKNOWN_EXT_CB */

#if (!defined(NO_WOLFSSL_CLIENT) || !defined(WOLFSSL_NO_CLIENT_AUTH)) || \
Expand Down
196 changes: 184 additions & 12 deletions tests/api/test_certman.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2846,12 +2846,10 @@ int test_wolfSSL_CRL_critical_idp(void)
return EXPECT_RESULT();
}

int test_wolfSSL_CRL_unknown_critical_ext(void)
{
EXPECT_DECLS;
#if !defined(NO_CERTS) && defined(HAVE_CRL) && !defined(NO_RSA)

static const unsigned char ca_cert[] = {
/* claim-root CA, DER. Shared by the unknown-critical-CRL-extension tests
* below. */
static const unsigned char crl_unk_ca_der[] = {
0x30, 0x82, 0x03, 0x1b, 0x30, 0x82, 0x02, 0x03, 0xa0, 0x03, 0x02,
0x01, 0x02, 0x02, 0x14, 0x1e, 0x25, 0xc1, 0x5d, 0x6f, 0x02, 0x21,
0xa0, 0xf0, 0x14, 0x15, 0x9c, 0x3b, 0x4d, 0x1d, 0x73, 0x16, 0x00,
Expand Down Expand Up @@ -2927,10 +2925,10 @@ int test_wolfSSL_CRL_unknown_critical_ext(void)
0xa2, 0xb0, 0x3e, 0x61, 0x16, 0x69, 0x8f
};

/* CRL with critical obsolete extension OID 2.5.29.1, 422 bytes DER.
* OID 2.5.29.1 is the old X.509v2 Authority Key Identifier, permanently
* superseded by 2.5.29.35. No implementation will ever support it. */
static const unsigned char crl_obsolete_critical[] = {
/* CRL with a critical obsolete CRL-level extension OID 2.5.29.1, 422 bytes
* DER. OID 2.5.29.1 is the old X.509v2 Authority Key Identifier, permanently
* superseded by 2.5.29.35. No implementation will ever support it. */
static const unsigned char crl_obsolete_critical[] = {
0x30, 0x82, 0x01, 0xa6, 0x30, 0x81, 0x8f, 0x02, 0x01, 0x01, 0x30,
0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01,
0x0b, 0x05, 0x00, 0x30, 0x15, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03,
Expand Down Expand Up @@ -2970,13 +2968,18 @@ int test_wolfSSL_CRL_unknown_critical_ext(void)
0xff, 0xad, 0x4d, 0x6e, 0x10, 0x73, 0x68, 0xfa, 0x54, 0x9e, 0xdc,
0x34, 0x70, 0xe4, 0x5d, 0x9e, 0x7c, 0xfa, 0x59, 0x97, 0xde, 0x35,
0x17, 0xbb, 0xaf, 0xa0, 0x28, 0x78, 0x13, 0xbf
};
};
#endif /* !NO_CERTS && HAVE_CRL && !NO_RSA */

int test_wolfSSL_CRL_unknown_critical_ext(void)
{
EXPECT_DECLS;
#if !defined(NO_CERTS) && defined(HAVE_CRL) && !defined(NO_RSA)
WOLFSSL_CERT_MANAGER* cm = NULL;

ExpectNotNull(cm = wolfSSL_CertManagerNew());
ExpectIntEQ(wolfSSL_CertManagerLoadCABuffer(cm, ca_cert,
sizeof(ca_cert), WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerLoadCABuffer(cm, crl_unk_ca_der,
sizeof(crl_unk_ca_der), WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECKALL),
WOLFSSL_SUCCESS);

Expand Down Expand Up @@ -3014,6 +3017,175 @@ int test_wolfSSL_CRL_unknown_critical_entry_ext(void)
return EXPECT_RESULT();
}

#if !defined(NO_CERTS) && defined(HAVE_CRL) && !defined(NO_RSA) && \

@Frauschi Frauschi Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] No test coverage for the non-Ex setter wolfSSL_CertManagerSetCRLUnknownExtCallback
🔧 NIT test

All three new tests exercise only the ...Ex variant (wolfSSL_CertManagerSetCRLUnknownExtCallbackEx). The non-Ex public API wolfSSL_CertManagerSetCRLUnknownExtCallback (which sets crlUnknownExtCallback and drives the dcrl->unknownExtCallback(...) branch in both parse paths) is never called by any test, so that branch is uncovered.

Suggestion: Add a case that registers the non-Ex callback via wolfSSL_CertManagerSetCRLUnknownExtCallback and asserts it fires (the non-Ex callback signature omits the ctx argument).

!defined(NO_FILESYSTEM) && defined(WC_ASN_UNKNOWN_EXT_CB)
/* Counter context plus the OID we observed on the unknown extension. */
typedef struct CRLUnkExtCtx {
int calls;
int sawCritical;
int oidMatched;
} CRLUnkExtCtx;

/* CRL Reason OID 2.5.29.21 (last component for the entry-ext test) and
* obsolete X.509v2 AKI 2.5.29.1 (last component for the CRL-level test). */
static int crl_unk_ext_cb_accept(const word16* oid, word32 oidSz,
int crit, const unsigned char* der, word32 derSz, void* ctxIn)
{
CRLUnkExtCtx* ctx = (CRLUnkExtCtx*)ctxIn;
(void)der;
(void)derSz;
if (ctx != NULL) {
ctx->calls++;
if (crit)
ctx->sawCritical = 1;
/* Expect OID arc starts at {2, 5, 29, ...}. */
if (oidSz >= 4 && oid[0] == 2 && oid[1] == 5 && oid[2] == 29)
ctx->oidMatched = 1;
}
return 0; /* accept */
}

static int crl_unk_ext_cb_reject(const word16* oid, word32 oidSz,
int crit, const unsigned char* der, word32 derSz, void* ctxIn)
{
(void)oid; (void)oidSz; (void)crit; (void)der; (void)derSz;
if (ctxIn != NULL)
((CRLUnkExtCtx*)ctxIn)->calls++;
return ASN_PARSE_E;
}

static int crl_unk_ext_cb_reject_positive(const word16* oid, word32 oidSz,
int crit, const unsigned char* der, word32 derSz, void* ctxIn)
{
(void)oid; (void)oidSz; (void)crit; (void)der; (void)derSz;
if (ctxIn != NULL)
((CRLUnkExtCtx*)ctxIn)->calls++;
return 1;
}
#endif

/* Callback rescues a critical unknown CRL ENTRY extension that would
* otherwise be rejected per RFC 5280 5.3. The fixture
* crl_critical_entry.pem carries OID 2.5.29.1 in a revoked entry. */
int test_wolfSSL_CRL_unknown_ext_cb_rescues_critical_entry_ext(void)
{
EXPECT_DECLS;
#if !defined(NO_CERTS) && defined(HAVE_CRL) && !defined(NO_RSA) && \
!defined(NO_FILESYSTEM) && defined(WC_ASN_UNKNOWN_EXT_CB)
WOLFSSL_CERT_MANAGER* cm = NULL;
CRLUnkExtCtx ctx = { 0, 0, 0 };

ExpectNotNull(cm = wolfSSL_CertManagerNew());
ExpectIntEQ(wolfSSL_CertManagerLoadCA(cm,
"./certs/crl/extra-crls/claim-root.pem", NULL), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECKALL),
WOLFSSL_SUCCESS);

/* Register a callback that accepts any unknown extension. */
ExpectIntEQ(wolfSSL_CertManagerSetCRLUnknownExtCallbackEx(cm,
crl_unk_ext_cb_accept, &ctx), WOLFSSL_SUCCESS);

/* Without the callback, this load fails (see
* test_wolfSSL_CRL_unknown_critical_entry_ext above). With an
* accepting callback registered, the load must succeed. */
ExpectIntEQ(wolfSSL_CertManagerLoadCRLFile(cm,
"./certs/crl/extra-crls/crl_critical_entry.pem", WOLFSSL_FILETYPE_PEM),
WOLFSSL_SUCCESS);

/* Callback must have fired at least once on the unknown critical OID. */
ExpectIntGT(ctx.calls, 0);
ExpectIntEQ(ctx.sawCritical, 1);
ExpectIntEQ(ctx.oidMatched, 1);

wolfSSL_CertManagerFree(cm);
#endif
return EXPECT_RESULT();
}

/* Callback rescues a critical unknown CRL-LEVEL extension. Drives the
* dispatch in ParseCRL_Extensions (as opposed to the entry-extension test
* above, which drives ParseCRL_EntryExtensions) using the same embedded
* crl_obsolete_critical fixture that test_wolfSSL_CRL_unknown_critical_ext
* proves is rejected without a callback. */
int test_wolfSSL_CRL_unknown_ext_cb_rescues_critical_crl_ext(void)
{
EXPECT_DECLS;
#if !defined(NO_CERTS) && defined(HAVE_CRL) && !defined(NO_RSA) && \
!defined(NO_FILESYSTEM) && defined(WC_ASN_UNKNOWN_EXT_CB)
WOLFSSL_CERT_MANAGER* cm = NULL;
CRLUnkExtCtx ctx = { 0, 0, 0 };

/* Accepting callback: the load must succeed and the callback must have
* seen the critical unknown OID. */
ExpectNotNull(cm = wolfSSL_CertManagerNew());
ExpectIntEQ(wolfSSL_CertManagerLoadCABuffer(cm, crl_unk_ca_der,
sizeof(crl_unk_ca_der), WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECKALL),
WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerSetCRLUnknownExtCallbackEx(cm,
crl_unk_ext_cb_accept, &ctx), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerLoadCRLBuffer(cm, crl_obsolete_critical,
sizeof(crl_obsolete_critical), WOLFSSL_FILETYPE_ASN1),
WOLFSSL_SUCCESS);
ExpectIntGT(ctx.calls, 0);
ExpectIntEQ(ctx.sawCritical, 1);
ExpectIntEQ(ctx.oidMatched, 1);
wolfSSL_CertManagerFree(cm);
cm = NULL;

/* Rejecting callback: the same CRL must fail to load. */
ctx.calls = 0;
ExpectNotNull(cm = wolfSSL_CertManagerNew());
ExpectIntEQ(wolfSSL_CertManagerLoadCABuffer(cm, crl_unk_ca_der,
sizeof(crl_unk_ca_der), WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECKALL),
WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerSetCRLUnknownExtCallbackEx(cm,
crl_unk_ext_cb_reject, &ctx), WOLFSSL_SUCCESS);
ExpectIntNE(wolfSSL_CertManagerLoadCRLBuffer(cm, crl_obsolete_critical,
sizeof(crl_obsolete_critical), WOLFSSL_FILETYPE_ASN1),
WOLFSSL_SUCCESS);
ExpectIntGT(ctx.calls, 0);

wolfSSL_CertManagerFree(cm);
#endif
return EXPECT_RESULT();
}

/* A callback that rejects with a positive value must not make the load
* report success. WOLFSSL_SUCCESS is 1, and BufferLoadCRL converts its
* error code with "ret ? ret : WOLFSSL_SUCCESS", so a positive callback
* return that propagates unsanitized out of ParseCRL is indistinguishable
* from success while the CRL entry has been discarded. */
int test_wolfSSL_CRL_unknown_ext_cb_positive_return_fails_load(void)
{
EXPECT_DECLS;
#if !defined(NO_CERTS) && defined(HAVE_CRL) && !defined(NO_RSA) && \
!defined(NO_FILESYSTEM) && defined(WC_ASN_UNKNOWN_EXT_CB)
WOLFSSL_CERT_MANAGER* cm = NULL;
CRLUnkExtCtx ctx = { 0, 0, 0 };
int rc;

ExpectNotNull(cm = wolfSSL_CertManagerNew());
ExpectIntEQ(wolfSSL_CertManagerLoadCA(cm,
"./certs/crl/extra-crls/claim-root.pem", NULL), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECKALL),
WOLFSSL_SUCCESS);

ExpectIntEQ(wolfSSL_CertManagerSetCRLUnknownExtCallbackEx(cm,
crl_unk_ext_cb_reject_positive, &ctx), WOLFSSL_SUCCESS);

rc = wolfSSL_CertManagerLoadCRLFile(cm,
"./certs/crl/extra-crls/crl_critical_entry.pem", WOLFSSL_FILETYPE_PEM);
ExpectIntNE(rc, WOLFSSL_SUCCESS);
ExpectIntLT(rc, 0);
ExpectIntGT(ctx.calls, 0);

wolfSSL_CertManagerFree(cm);
#endif
return EXPECT_RESULT();
}

int test_wolfSSL_CertManagerCheckOCSPResponse(void)
{
EXPECT_DECLS;
Expand Down
9 changes: 9 additions & 0 deletions tests/api/test_certman.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ int test_wolfSSL_CRL_duplicate_extensions(void);
int test_wolfSSL_CRL_critical_idp(void);
int test_wolfSSL_CRL_unknown_critical_ext(void);
int test_wolfSSL_CRL_unknown_critical_entry_ext(void);
int test_wolfSSL_CRL_unknown_ext_cb_rescues_critical_entry_ext(void);
int test_wolfSSL_CRL_unknown_ext_cb_rescues_critical_crl_ext(void);
int test_wolfSSL_CRL_unknown_ext_cb_positive_return_fails_load(void);
int test_wolfSSL_CertManagerCheckOCSPResponse(void);
int test_various_pathlen_chains(void);
int test_wolfSSL_CertManagerRejectMD5Cert(void);
Expand Down Expand Up @@ -82,6 +85,12 @@ int test_wolfSSL_CertManagerNameConstraint_skid_disambiguates(void);
TEST_DECL_GROUP("certman", test_wolfSSL_CRL_critical_idp), \
TEST_DECL_GROUP("certman", test_wolfSSL_CRL_unknown_critical_ext), \
TEST_DECL_GROUP("certman", test_wolfSSL_CRL_unknown_critical_entry_ext), \
TEST_DECL_GROUP("certman", \
test_wolfSSL_CRL_unknown_ext_cb_rescues_critical_entry_ext), \
TEST_DECL_GROUP("certman", \
test_wolfSSL_CRL_unknown_ext_cb_rescues_critical_crl_ext), \
TEST_DECL_GROUP("certman", \
test_wolfSSL_CRL_unknown_ext_cb_positive_return_fails_load), \
TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerCheckOCSPResponse), \
TEST_DECL_GROUP("certman", test_various_pathlen_chains), \
TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerRejectMD5Cert), \
Expand Down
Loading
Loading