X509: cache decoded public key, give get0_pubkey borrowed semantics by julek-wolfssl · Pull Request #11428 · wolfSSL/wolfssl

julek-wolfssl · 2026-09-10T15:58:54Z

Previously wolfSSL_X509_get_pubkey() (and X509_get0_pubkey(), which
mapped to it) built a brand-new WOLFSSL_EVP_PKEY on every call,
leaking the decoded key on each invocation for callers that followed
OpenSSL's get0 borrowed-pointer contract.

Decode the public key once into the certificate's WOLFSSL_X509_PUBKEY
member, lazily on first use, and free it with the certificate.
wolfSSL_X509_get_pubkey() returns the cached key with a new
reference, matching X509_get_pubkey().
Add wolfSSL_X509_get0_pubkey() (returns without a reference) and map
X509_get0_pubkey() / X509_REQ_get0_pubkey() to it.
Add wolfSSL_X509_PUBKEY_get0() (X509_PUBKEY_get0).
wolfSSL_X509_get_X509_PUBKEY() now decodes the key too, so
X509_PUBKEY_get() and X509_PUBKEY_get0_param() work outside
OPENSSL_ALL.
get0_param no longer dereferences a missing key, and maps the stored
key OID to a NID before creating the algorithm object; certificates
without a public key don't cache an empty one.
The lazily decoded key is published via compare-and-exchange (like the
context private key cache) so concurrent first calls on a shared
certificate don't leak a key; the EC public point's internal copy is
marked as set after SetECKeyExternal() to avoid rebuilding it on
shared reads.
wolfSSL_X509_set_pubkey() and re-decoding a certificate drop the
cached key; set_pubkey keeps it when handed the same key and
refreshes the key's algorithm OID, algorithm object, and curve OID.
key.algor is now freed in every configuration that can allocate it,
not only OPENSSL_ALL.

Callers that freed the result of X509_get0_pubkey() must stop doing
so; this is noted in the ChangeLog.

Copilot

🟡 Changes recommended

Unresolved findings affect build guards, cache lifecycle, concurrent initialization, and API const correctness.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates X.509 public-key access to lazily cache decoded keys and match OpenSSL ownership semantics.

Changes:

Adds cached owned and borrowed public-key accessors.
Updates cache invalidation, cleanup, and EC key handling.
Adds compatibility mappings, tests, documentation, and ChangeLog guidance.

File summaries

File	Summary
`wolfssl/ssl.h`	Declares new public-key APIs.
`wolfssl/openssl/ssl.h`	Adds OpenSSL compatibility mappings.
`tests/api/test_ossl_x509_pk.h`	Registers the new tests.
`tests/api/test_ossl_x509_pk.c`	Tests ownership and caching behavior.
`src/x509.c`	Implements key caching and access semantics.
`src/pk_ec.c`	Preserves synchronized EC public-point state.
`src/internal.c`	Updates cache cleanup and invalidation.
`doc/dox_comments/header_files/ssl.h`	Documents the new APIs.
`ChangeLog.md`	Records the behavioral change.

Review details

Suppressed comments (4)

src/internal.c:15264

Moving the old cache invalidation under only OPENSSL_EXTRA_X509_SMALL removes it from OPENSSL_ALL, even though the OPENSSL_ALL block below still creates x509->key.pkey. Re-decoding or reusing an X509 then overwrites the cached pointer without releasing the old key. This guard needs to cover the cache's full set of configurations, including OPENSSL_ALL/full OPENSSL_EXTRA.
src/x509.c:6614
This comment says the borrowed key is valid for the entire certificate lifetime, but wolfSSL_X509_set_pubkey() and certificate re-decoding can free or replace x509->key.pkey before the certificate is freed. Document the pointer as valid only until the certificate is modified or freed, and keep the generated API documentation consistent.

/* Returns the public key of x509 without a new reference.
 *
 * returns a pointer to the WOLFSSL_EVP_PKEY on success and NULL on fail.
 * The key is valid for the lifetime of x509 and must not be freed.

src/x509.c:6581

When WOLFSSL_ATOMIC_OPS is unavailable, this fallback is selected even for non-SINGLE_THREADED builds and is only a plain assignment. Concurrent first calls can overwrite the cache, leak the losing decoded EVP_PKEY, and race on the cached pointer, despite the helper's concurrency guarantee. Protect this path with a mutex or use another synchronized cache mechanism.

            x509->key.pkey = key;

wolfssl/ssl.h:6251

OpenSSL declares X509_PUBKEY_get0(const X509_PUBKEY *); this new wrapper takes a mutable pointer, so code using a const X509_PUBKEY cannot call the compatibility macro without discarding const. Make both the declaration and implementation accept const WOLFSSL_X509_PUBKEY*.

WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get0(WOLFSSL_X509_PUBKEY* key);

Files reviewed: 9/9 changed files
Comments generated: 4
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-09-10T16:54:13Z

MemBrowse Memory Report

gcc-arm-cortex-m0plus

FLASH: .text +276 B (+1.9%, 69,147 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m3

FLASH: .text +272 B (+1.0%, 129,133 B / 262,144 B, total: 49% used)

gcc-arm-cortex-m4

FLASH: .text +192 B (+0.6%, 207,852 B / 262,144 B, total: 79% used)

gcc-arm-cortex-m4-baremetal

FLASH: .text +192 B (+1.7%, 71,523 B / 262,144 B, total: 27% used)

gcc-arm-cortex-m4-crypto-only

FLASH: .text +256 B (+0.7%, 180,829 B / 262,144 B, total: 69% used)

gcc-arm-cortex-m4-dtls13

FLASH: .rodata +1,024 B, .text +1,408 B (+1.3%, 192,260 B / 1,048,576 B, total: 18% used)

gcc-arm-cortex-m4-min-ecc

FLASH: .text +192 B (+1.9%, 66,309 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-openssl-compat

FLASH: .rodata +1,016 B, .text +1,664 B (+0.3%, 790,868 B / 1,048,576 B, total: 75% used)

gcc-arm-cortex-m4-pkcs7

FLASH: .text +1,984 B (+1.4%, 221,598 B / 262,144 B, total: 85% used)

gcc-arm-cortex-m4-pq

FLASH: .rodata +1,048 B, .text +2,944 B (+1.3%, 308,272 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m4-rsa-only

FLASH: .rodata +1,024 B, .text +1,728 B (+0.8%, 338,544 B / 1,048,576 B, total: 32% used)

gcc-arm-cortex-m4-sp-math

FLASH: .text +192 B (+1.9%, 66,309 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-tls12

FLASH: .text +320 B (+1.0%, 129,917 B / 262,144 B, total: 50% used)

gcc-arm-cortex-m4-tls13

FLASH: .text +1,472 B (+1.0%, 247,198 B / 262,144 B, total: 94% used)

gcc-arm-cortex-m7

FLASH: .text +256 B (+0.6%, 207,852 B / 262,144 B, total: 79% used)

gcc-arm-cortex-m7-pq

FLASH: .rodata +1,048 B, .text +2,880 B (+1.3%, 309,168 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m7-tls13

FLASH: .text +1,472 B (+1.0%, 247,198 B / 262,144 B, total: 94% used)

linuxkm-pie

Data: __patchable_function_entries +216 B (+0.8%, 28,600 B)

linuxkm-standard

Data: __patchable_function_entries +296 B (+0.6%, 51,464 B)

stm32-sim-stm32h753

FLASH: .text +3,848 B (+2.0%, 194,964 B / 2,097,152 B, total: 9% used)

julek-wolfssl · 2026-09-29T05:19:55Z

retest this please

…ntics wolfSSL_X509_get_pubkey() built a new WOLFSSL_EVP_PKEY, including a freshly decoded RSA or EC key, on every call, and X509_get0_pubkey() mapped to the same function. Callers following the OpenSSL get0 contract, which returns a pointer owned by the certificate, leaked the whole decoded key graph on each call. Decode the public key once into the certificate's existing WOLFSSL_X509_PUBKEY member, lazily on first use (OPENSSL_ALL already fills it at parse time), and free it with the certificate. wolfSSL_X509_get_pubkey() now returns that key with a new reference, matching X509_get_pubkey(). Add wolfSSL_X509_get0_pubkey(), which returns it without a reference, and map X509_get0_pubkey() and X509_REQ_get0_pubkey() to it. Add wolfSSL_X509_PUBKEY_get0() (X509_PUBKEY_get0). wolfSSL_X509_get_X509_PUBKEY() decodes the key too, so X509_PUBKEY_get() and X509_PUBKEY_get0_param() work outside OPENSSL_ALL. get0_param no longer dereferences a missing key and maps the stored key OID to a NID before creating the algorithm object. A certificate without a public key does not cache an empty key. The lazily decoded key is published with a compare-and-exchange, the same way the context private key cache is, so concurrent first calls on a shared certificate do not leak a key. The EC public point's internal copy is marked as set after SetECKeyExternal(), so readers of a shared key do not rebuild it. wolfSSL_X509_set_pubkey() and re-decoding a certificate drop the cached key; set_pubkey keeps it when handed that very key, and refreshes the key's algorithm OID, algorithm object and curve OID. The key.algor member is now freed in every configuration that can allocate it, not only OPENSSL_ALL. Callers that freed the result of X509_get0_pubkey() must stop; the ChangeLog records this.

d2iTryMlDsaKey() left keyIdx at 0 when the size-keyed raw import path matched, so d2i_make_pkey() copied nothing and the resulting EVP PKEY carried pkey.ptr == NULL and pkey_sz == 0. Under OPENSSL_ALL the certificate parser fills x509->key.pkey through wolfSSL_d2i_PUBKEY(), and an ML-DSA certificate stores the raw public key, so the cached key had no key material. Now that wolfSSL_X509_get_pubkey() returns that cached key, wolfSSL_X509_verify() and wolfSSL_X509_REQ_verify() had nothing to verify against and failed. Raw bytes carry no length prefix, so the whole input is the key.

Set x509->key.pubKeyOID before the compare-and-exchange publishes x509->key.pkey, so a concurrent X509_PUBKEY_get0_param() never sees a key next to an OID of 0. X509_get0_pubkey() takes a const X509* in OpenSSL. Take a const WOLFSSL_X509* and cast for the lazy cache, the same way wolfSSL_X509_get_X509_PUBKEY() does.

C89 forbids mixed declarations and code. Windows (C2275) and the -Wdeclaration-after-statement Jenkins configs both rejected the declaration inside the WOLFSSL_ATOMIC_OPS block.

philljj · 2026-10-01T21:59:04Z

note: maybe redundant with

Compat layer: fix memory leak in X509_get0_pubkey #11467

padelsbach · 2026-10-02T00:50:13Z

This PR has several independent changes. Please post separate PRs in the future.

padelsbach · 2026-10-02T04:11:42Z

+{
+    WOLFSSL_ENTER("wolfSSL_X509_get0_pubkey");
+    /* The cache is the only thing written, like X509_get_X509_PUBKEY(). */
+    return X509CachedPubKey((WOLFSSL_X509*)x509);


AI finds that wolfCLU uses this, and since it does not increment the refcount, there becomes a double free in wolfSSL_EVP_PKEY_free. Can you check?

Confirmed and fix is to update the call in wolfclu to use a call that requries a free call.

Fixed in wolfSSL/wolfCLU#300

padelsbach · 2026-10-02T04:18:14Z

 WOLFSSL_API WOLFSSL_X509_PUBKEY *wolfSSL_X509_get_X509_PUBKEY(const WOLFSSL_X509* x509);
 WOLFSSL_API int wolfSSL_X509_PUBKEY_get0_param(WOLFSSL_ASN1_OBJECT **ppkalg, const unsigned char **pk, int *ppklen, WOLFSSL_X509_ALGOR **pa, WOLFSSL_X509_PUBKEY *pub);
 WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get(WOLFSSL_X509_PUBKEY* key);
+WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get0(WOLFSSL_X509_PUBKEY* key);


Param should be const WOLFSSL_X509_PUBKEY* to match openssl

padelsbach · 2026-10-02T04:25:35Z

    }

+    /* Decode the key so pkey is usable through the returned object. */
+    (void)X509CachedPubKey((WOLFSSL_X509*)x509);


This line casts away const. This can lead to undefined behavior, memory faults, or other bad outcomes. At least add a comment about why this must be done.

x509->key needs to be computed here

padelsbach · 2026-10-02T04:36:46Z

+        return WOLFSSL_FAILURE;
+    }

    if (!pub->algor) {


AI says there is now a race condition in this block since x509->key.pubKeyOID is now set in X509CachedPubKey. If this block fails in nid2obj for example, pub->algor is not fully built, but subsequent calls succeed. Not sure if this is valid

padelsbach · 2026-10-02T04:37:54Z

        if (rc == 0) {
            isMlDsa = 1;
+            /* Raw bytes carry no length prefix; the whole input is the key. */
+            keyIdx = inSz;


Is there a test case for this?

X509_PUBKEY_get0() takes a const X509_PUBKEY* in OpenSSL; match it. Note why X509_get_X509_PUBKEY() and X509_get0_pubkey() cast away const: OpenSSL takes a const X509 in both, and only the lazily decoded key cache is written. Document that the borrowed key lasts until the certificate is freed or its public key is changed.

X509_PUBKEY_get0_param() stored the new X509_ALGOR in pub->algor before filling in its algorithm. When creating the object failed, later calls found the half built algor and returned a NULL algorithm with success. Build it locally, free it on failure, and store it only when complete.

Decode the raw ML-DSA-44 public key and check that the EVP PKEY holds all of it. Without the keyIdx fix the key had no material.

X509 objects are not designed for concurrent use. Their refcount manages lifetime, not thread sharing. Store the lazily decoded key with plain assignment instead of compare-and-exchange.

Copilot AI lite review requested due to automatic review settings September 10, 2026 15:58

julek-wolfssl self-assigned this Sep 10, 2026

Copilot started reviewing on behalf of julek-wolfssl September 10, 2026 15:59 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/x509.c Outdated

Comment thread src/internal.c

Comment thread src/x509.c

Comment thread wolfssl/ssl.h Outdated

julek-wolfssl force-pushed the fix/get0-pubkey-borrowed branch from e40149f to 62567ec Compare September 15, 2026 10:33

julek-wolfssl assigned wolfSSL-Bot and unassigned julek-wolfssl Sep 16, 2026

julek-wolfssl force-pushed the fix/get0-pubkey-borrowed branch 2 times, most recently from 3b5d5d3 to eb536e8 Compare September 28, 2026 11:20

julek-wolfssl added 4 commits September 30, 2026 05:43

Fix CI: hoist the atomic CAS temporary to the top of X509CachedPubKey

1c5f7e0

C89 forbids mixed declarations and code. Windows (C2275) and the -Wdeclaration-after-statement Jenkins configs both rejected the declaration inside the WOLFSSL_ATOMIC_OPS block.

julek-wolfssl force-pushed the fix/get0-pubkey-borrowed branch from eb536e8 to 1c5f7e0 Compare September 30, 2026 09:05

philljj self-assigned this Oct 1, 2026

philljj requested a review from padelsbach October 1, 2026 21:52

philljj assigned padelsbach Oct 1, 2026

padelsbach mentioned this pull request Oct 1, 2026

Compat layer: fix memory leak in X509_get0_pubkey #11467

Closed

1 of 4 tasks

padelsbach requested changes Oct 2, 2026

View reviewed changes

julek-wolfssl added 4 commits October 2, 2026 06:18

Address review: test d2i_PUBKEY with a raw ML-DSA public key

030d134

Decode the raw ML-DSA-44 public key and check that the EVP PKEY holds all of it. Without the keyIdx fix the key had no material.

Address review: use plain assignment for the cached X509 key

f1b5dbc

X509 objects are not designed for concurrent use. Their refcount manages lifetime, not thread sharing. Store the lazily decoded key with plain assignment instead of compare-and-exchange.

julek-wolfssl requested a review from padelsbach October 5, 2026 12:56

Conversation

julek-wolfssl commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

Uh oh!

julek-wolfssl commented Sep 29, 2026

Uh oh!

philljj commented Oct 1, 2026

Uh oh!

padelsbach commented Oct 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions Bot commented Sep 10, 2026 •

edited

Loading