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.

…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.

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.

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


            x509->key.algor = NULL;
        }
+    #endif
+    #ifdef OPENSSL_EXTRA_X509_SMALL


+        if (key != NULL) {
+        #ifdef WOLFSSL_ATOMIC_OPS
+            WOLFSSL_EVP_PKEY* current = NULL;
+            if (!wolfSSL_Atomic_Ptr_CompareExchange(
+                    (void* volatile*)&x509->key.pkey, (void**)&current, key)) {
+                wolfSSL_EVP_PKEY_free(key);
+                key = current;
+            }
+        #else
+            x509->key.pkey = key;
+        #endif
+            x509->key.pubKeyOID = x509->pubKeyOID;
+        }


 WOLFSSL_API int wolfSSL_X509_CRL_set_nextUpdate(WOLFSSL_X509_CRL* crl,
                                                const WOLFSSL_ASN1_TIME* time);
 WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509);
+WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_X509_get0_pubkey(WOLFSSL_X509* x509);


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

MemBrowse Memory Report

gcc-arm-cortex-m3

FLASH: .text +104 B (+0.1%, 125,879 B / 262,144 B, total: 48% used)

gcc-arm-cortex-m4

FLASH: .text +128 B (+0.1%, 205,085 B / 262,144 B, total: 78% used)

gcc-arm-cortex-m4-dtls13

FLASH: .text +128 B (+0.1%, 187,324 B / 1,048,576 B, total: 18% used)

gcc-arm-cortex-m4-openssl-compat

FLASH: .rodata +24 B, .text +384 B (+0.1%, 784,188 B / 1,048,576 B, total: 75% used)

gcc-arm-cortex-m4-pq

FLASH: .text +128 B (+0.0%, 302,216 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m4-rsa-only

FLASH: .text +64 B (+0.0%, 332,960 B / 1,048,576 B, total: 32% used)

gcc-arm-cortex-m4-tls12

FLASH: .text +64 B (+0.1%, 126,643 B / 262,144 B, total: 48% used)

gcc-arm-cortex-m4-tls13

FLASH: .text +64 B (+0.0%, 242,703 B / 262,144 B, total: 93% used)

gcc-arm-cortex-m7

FLASH: .text +128 B (+0.1%, 205,085 B / 262,144 B, total: 78% used)

gcc-arm-cortex-m7-pq

FLASH: .text +128 B (+0.0%, 303,176 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m7-tls13

FLASH: .text +128 B (+0.1%, 242,767 B / 262,144 B, total: 93% used)

linuxkm-pie

Data: __patchable_function_entries +32 B (+0.1%, 27,072 B)

linuxkm-standard

Data: __patchable_function_entries +48 B (+0.1%, 49,896 B)
No memory changes detected for:
gcc-arm-cortex-m0plus
gcc-arm-cortex-m4-baremetal
gcc-arm-cortex-m4-crypto-only
gcc-arm-cortex-m4-min-ecc
gcc-arm-cortex-m4-pkcs7
gcc-arm-cortex-m4-sp-math
stm32-sim-stm32h753

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.

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

julek-wolfssl added 3 commits September 11, 2026 05:33

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

e40149f

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.

Navigation Menu

Uh oh!

X509: cache decoded public key, give get0_pubkey borrowed semantics - #11428

X509: cache decoded public key, give get0_pubkey borrowed semantics#11428
julek-wolfssl wants to merge 4 commits into
wolfSSL:masterfrom
julek-wolfssl:fix/get0-pubkey-borrowed

julek-wolfssl commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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

Uh oh!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Sep 10, 2026 •

edited

Loading