Skip to content

openssl compat: cut EVP, EVP_PKEY and PEM overhead, and fix defects found - #11689

Open
SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:openssl_compat_api_perf_fixes_1
Open

SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:openssl_compat_api_perf_fixes_1

Conversation

@SparkiDev

@SparkiDev SparkiDev commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Description

Performance:

  • Size WOLFSSL_SHA3_CTX's holder in pointers, not bytes. It reserved sizeof(wc_Sha3) pointers for a 440 byte object, so WOLFSSL_Hasher goes 3520 -> 448 bytes and WOLFSSL_EVP_MD_CTX 3552 -> 944. A static assert now catches the same mistake.
  • Resolve digests and ciphers by table pointer before comparing names, and return the table's own pointer from the accessors. EVP_sha256() 47.4 -> 1.2ns, EVP_MD_type 64.7 -> 4.3ns, EVP_CIPHER_nid 101.2 -> 8.1ns.
  • Copy only the fields past the hash union in EVP_MD_CTX_copy_ex, and leave the cipher union alone in EVP_CIPHER_CTX_init.
  • Set up an EVP_PKEY's RNG, an RSA key's blinding RNG and an EVP_PKEY's DER encoding on first use rather than at creation. Seeding a DRBG cost more than everything else about making a key put together. EVP_PKEY_new + free 13366 -> 34ns, RSA_new + free 13748 -> 230ns, EVP_PKEY_set1_RSA 13994 -> 45ns. Certificate parsing roughly halves as a result: d2i_X509 45978 -> 17198ns, X509_dup 45945 -> 17116ns, both now faster than OpenSSL. set1_DSA still encodes immediately, as WOLFSSL_DSA is not reference counted and the pkey only borrows the caller's key.
  • Keep the decoded public key on the X509 and hand out a reference, as OpenSSL does. X509_get_pubkey + free 27692 -> 9.8ns, against 9.6ns.
  • Give public key operations the shared global RNG instead of seeding one per call, where the instance locks itself or the build is single threaded. WOLFSSL_PK_LOCAL_RNG forces an RNG per call. EVP_DigestSign with ECDSA P-256 32291 -> 17008ns, against 18852ns.
  • Encode base64 without a call per output character, and hand whole blocks to Base64_Encode() rather than one 48 byte line at a time. EVP_EncodeUpdate 6446 -> 1855ns per 4KB. Skip the call into Base64_SkipNewline() for characters needing no skipping: EVP_DecodeUpdate 37059 -> 16194ns. What remains there is the constant time character decode, which is deliberate; wc_PemToDer() already picks the table decoder by PEM type.
  • Search cipher_tbl before the alias table in EVP_get_cipherbyname(), since over half the aliases only spell a name already there in another case, and compare without the platform's strcasecmp: 330 -> 67ns. The comparison is ASCII only, so it no longer follows the locale and works on platforms whose XSTRCASECMP is strcmp.
  • Find the PEM footer in a memory BIO's buffer and read up to it, rather than reading a byte at a time through the whole input. Loading a certificate from a file that carries a text dump alongside it 100311 -> 23301ns, against 33006ns.

Fixes:

  • AES-CCM through EVP now behaves as OpenSSL's does: the payload length comes from the first Update() carrying payload, AAD before the length is refused, and Final() has nothing left to do. Of 376 observables compared against OpenSSL - the tag and IV length matrices, the defaults, length inference over 48 sizes, and an encrypt/decrypt matrix with the tag then corrupted - 375 agree. The one that differs, EVP_CIPHER_iv_length() reporting CCM_NONCE_MIN_SZ where OpenSSL reports 12, is unchanged by this work.
  • EVP_get_cipherbyname(NULL) and EVP_get_digestbyname(NULL) dereferenced the name; OpenSSL returns NULL.
  • EVP_EncodeUpdate() wrote the block it was holding and then wrote over it, so a caller feeding data in pieces got wrong bytes, including bytes never written.
  • EVP_PKEY_keygen() on DH read the parameters after replacing the key object they came from, which is the same object when a caller passes its own key back in.
  • EVP_PKEY_set1_EC_KEY() with an empty EC_KEY now succeeds. It reported failure only as a side effect of encoding eagerly; OpenSSL returns 1 and the empty key is caught when the encoding is asked for.
  • Remove three GMULT macros naming functions that do not exist.

Portability:

  • Add XMEMCHR, with wc_memchr() for builds naming their own string functions through STRING_USER that have no memchr among them.

Tests:

  • Known answer vectors for the base64 encoders, generated independently of wolfSSL. testwolfcrypt passed with the encoder deliberately corrupted, as it only checked return codes.
  • EVP_EncodeUpdate() over every chunking of ten sizes straddling the 48 byte block, and PEM_read_bio_X509() reading several certificates from one BIO while leaving nothing behind after one.
  • EVP_get_cipherbyname() over canonical names, alternative spellings, every casing of each, and NULL.
  • One RSA private key operation per freshly decoded key: sharing a key lets the first operation provide the blinding RNG and hides a missing one in the rest. Plus EVP_PKEY_set1_DSA() followed by the caller releasing its key.
  • The reference counting behind the cached X509 public key: outliving the certificate and the caller, replacement, and duplication.
  • wc_memchr(), guarded by USE_WOLF_MEMCHR so it runs where that function is built.

Testing

Tested on ARM64 with SM algorithms.

Note: wolfsm fixes required:
wolfSSL/wolfsm#37

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

MemBrowse Memory Report · Explore what changed →

gcc-arm-cortex-m0plus

  • FLASH: .text +132 B (+0.2%, 70,935 B / 262,144 B, total: 27% used)

gcc-arm-cortex-m3

  • FLASH: .text +144 B (+0.1%, 131,905 B / 262,144 B, total: 50% used)

gcc-arm-cortex-m4

  • FLASH: .text +448 B (+0.2%, 209,484 B / 262,144 B, total: 80% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text +128 B (+0.2%, 73,655 B / 262,144 B, total: 28% used)

gcc-arm-cortex-m4-crypto-only

  • FLASH: .text +448 B (+0.2%, 182,163 B / 262,144 B, total: 69% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text +128 B (+0.1%, 196,120 B / 1,048,576 B, total: 19% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text +128 B (+0.2%, 68,249 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .rodata +64 B, .text -512 B (-0.1%, 800,420 B / 1,048,576 B, total: 76% used)
  • RAM: .bss +16 B (+0.0%, 121,788 B / 262,144 B, total: 46% used)

gcc-arm-cortex-m4-pkcs7

  • FLASH: .text +448 B (+0.2%, 223,316 B / 262,144 B, total: 85% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text +448 B (+0.1%, 313,296 B / 1,048,576 B, total: 30% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text +448 B (+0.1%, 344,144 B / 1,048,576 B, total: 33% used)

gcc-arm-cortex-m4-sp-math

  • FLASH: .text +128 B (+0.2%, 68,249 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text +128 B (+0.1%, 132,689 B / 262,144 B, total: 51% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text +448 B (+0.2%, 250,082 B / 262,144 B, total: 95% used)

gcc-arm-cortex-m7

  • FLASH: .text +512 B (+0.2%, 209,548 B / 262,144 B, total: 80% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text +448 B (+0.1%, 314,128 B / 1,048,576 B, total: 30% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text +448 B (+0.2%, 250,082 B / 262,144 B, total: 95% used)

stm32-sim-stm32h753

  • FLASH: .text +512 B (+0.3%, 204,588 B / 2,097,152 B, total: 10% used)

zephyr-frdm_rw612

  • FLASH: log_const_area -8 B, rodata -8 B, text +2,120 B (+0.7%, 284,568 B / 67,108,864 B, total: 0% used)

Catch memory regressions in CI with MemBrowse. Free for open source.

Copilot AI left a comment

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.

🟡 Changes recommended

Portability failures, unsafe failure paths, RNG lifetime defects, and mutex initialization races remain unresolved.

10 open findings
What changed in this PR

Optimizes OpenSSL compatibility APIs while correcting EVP, PEM, CCM, Base64, key-lifecycle, and X509 caching behavior.

Changes:

  • Defers costly RNG and DER initialization and caches decoded X509 public keys.
  • Optimizes cipher/digest lookup, Base64 processing, and PEM BIO parsing.
  • Expands compatibility and regression coverage.
File Description
wolfssl/​wolfcrypt/​types.h Adds XMEMCHR portability support.
wolfssl/​wolfcrypt/​coding.h Declares streaming Base64 decoding.
wolfssl/​ssl.h Tracks deferred PKEY state.
wolfssl/​openssl/​sha3.h Corrects SHA3 holder sizing.
wolfssl/​openssl/​rsa.h Tracks deferred RSA RNG state.
wolfssl/​openssl/​evp.h Adds CCM message state.
wolfssl/​internal.h Adds cached X509 key support.
wolfcrypt/​src/​wc_port.c Implements wc_memchr.
wolfcrypt/​src/​evp_pk.c Integrates deferred DER generation.
wolfcrypt/​src/​coding.c Optimizes Base64 encoding and decoding.
wolfcrypt/​src/​evp.c Implements EVP optimizations and compatibility fixes.
wolfcrypt/​src/​aes.c Removes invalid GMULT macros.
tests/​api/​test_port.c Tests wc_memchr.
tests/​api/​test_ossl_x509_pk.h Registers X509 cache tests.
tests/​api/​test_ossl_x509_pk.c Tests cached-key reference handling.
tests/​api/​test_ossl_x509_io.h Registers PEM BIO tests.
tests/​api/​test_ossl_x509_io.c Tests multi-certificate BIO parsing.
tests/​api/​test_ossl_rsa.h Registers lazy-RNG tests.
tests/​api/​test_ossl_rsa.c Tests deferred RSA blinding RNGs.
tests/​api/​test_evp.h Registers chunked Base64 tests.
tests/​api/​test_evp.c Tests chunk-independent encoding.
tests/​api/​test_evp_pkey.h Registers deferred-DER tests.
tests/​api/​test_evp_pkey.c Tests lazy PKEY encoding and ownership.
tests/​api/​test_evp_digest.h Registers digest regression tests.
tests/​api/​test_evp_digest.c Tests digest context lookup and copying.
tests/​api/​test_evp_cipher.h Registers cipher and CCM tests.
tests/​api/​test_evp_cipher.c Tests CCM semantics and cipher lookup.
tests/​api/​test_coding.h Registers Base64 vectors.
tests/​api/​test_coding.c Adds Base64 known-answer tests.
src/​x509.c Caches keys and optimizes PEM parsing.
src/​ssl.c Enables shared RNG use where safe.
src/​ssl_p7p12.c Ensures DER before PKCS operations.
src/​ssl_load.c Ensures DER before loading keys.
src/​ssl_crypto.c Adds SHA3 size validation.
src/​pk.c Ensures DER before key serialization.
src/​pk_rsa.c Defers RSA blinding RNG creation.
src/​internal.c Releases cached X509 keys.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread src/pk_rsa.c Resolved
Comment thread src/ssl_crypto.c Resolved
Comment thread src/x509.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread src/x509.c Resolved
Comment thread tests/api/test_evp_pkey.c Resolved
Comment thread tests/api/test_evp_cipher.c Resolved
Comment thread tests/api/test_evp_cipher.c Resolved
Comment thread tests/api/test_evp_cipher.c Resolved
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch 6 times, most recently from 35ad6ac to 2295f64 Compare October 9, 2026 01:54
@SparkiDev
SparkiDev requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot October 9, 2026 03:19

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11689

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs
Failed targets: wolfssl-src, wolfssl-bugs
Coverage: 12 of 24 in-scope changed file(s) opened by the reviewer; not opened: src/ssl_crypto.c, src/ssl_load.c, src/ssl_p7p12.c, tests/api.c, tests/api/test_coding.c, tests/api/test_evp.c, tests/api/test_evp_cipher.c, wolfssl/internal.h, wolfssl/openssl/evp.h, wolfssl/openssl/rsa.h and 2 more

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Lite

⚠️ Review incomplete: the spending limit was reached. Do not treat this as a clean review.

Comment thread wolfcrypt/src/evp.c Resolved
Comment thread src/pk.c Resolved

Copilot AI left a comment

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.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

7 open findings
10 resolved since last review

🧠 Review effort: Lite

Comment thread wolfcrypt/src/coding.c Resolved
Comment thread src/pk.c Resolved
Comment thread src/pk.c Resolved
Comment thread src/ssl_load.c Resolved
Comment thread src/ssl_load.c Resolved
Comment thread wolfcrypt/src/evp_pk.c Resolved
Comment thread wolfcrypt/src/evp_pk.c Resolved
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch from 2295f64 to 3da039a Compare October 9, 2026 05:28
@SparkiDev
SparkiDev requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot October 9, 2026 05:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Comment thread wolfcrypt/src/evp.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread src/ssl_p7p12.c Resolved

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11689

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs
Failed targets: wolfssl-src, wolfssl-bugs
Coverage: 13 of 24 in-scope changed file(s) opened by the reviewer; not opened: src/ssl_crypto.c, tests/api.c, tests/api/test_coding.c, tests/api/test_evp.c, tests/api/test_evp_cipher.c, wolfcrypt/src/aes.c, wolfssl/internal.h, wolfssl/openssl/rsa.h, wolfssl/openssl/sha3.h, wolfssl/ssl.h and 1 more

Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Lite

⚠️ Review incomplete: the spending limit was reached. Do not treat this as a clean review.

Comment thread wolfcrypt/src/evp.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread src/ssl.c Resolved
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch from 3da039a to cb36a34 Compare October 9, 2026 06:46
@SparkiDev
SparkiDev requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot October 9, 2026 06:46

Copilot AI left a comment

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.

🟡 Changes recommended

Lazy DER failure paths can leak resources, crash on empty EC keys, or corrupt retry metadata, and one supported test configuration does not compile portably.

4 open findings
4 resolved since last review

🧠 Review effort: Balanced

Comment thread src/pk.c Resolved
Comment thread src/pk.c Resolved
Comment thread tests/api/test_evp_cipher.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11689

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 13 of 32 in-scope changed file(s) opened by the reviewer; not opened: src/internal.c, src/ssl_crypto.c, tests/api.c, tests/api/test_coding.c, tests/api/test_evp.c, tests/api/test_evp_digest.c, tests/api/test_evp_pkey.c, tests/api/test_ossl_pem.c, tests/api/test_ossl_rsa.c, tests/api/test_ossl_x509_io.c and 9 more

Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Lite

⚠️ Review incomplete: the spending limit was reached. Do not treat this as a clean review.

Comment thread wolfcrypt/src/evp.c Resolved
Comment thread src/pk.c Resolved
Comment thread tests/api/test_evp_cipher.c Resolved
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch from cb36a34 to dcf8a6c Compare October 9, 2026 08:04
@SparkiDev
SparkiDev requested a balanced review from Copilot October 9, 2026 08:04
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch from 2d2788a to c66794e Compare October 29, 2026 06:55
@SparkiDev SparkiDev assigned wolfSSL-Bot and unassigned SparkiDev Oct 29, 2026
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch from c66794e to 31c544b Compare November 5, 2026 02:25
@SparkiDev
SparkiDev requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot November 5, 2026 02:25
@SparkiDev SparkiDev self-assigned this Nov 5, 2026

Copilot AI left a comment

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.

🟡 Changes recommended

The new streaming rollback paths lack chunked whitespace tests for incomplete base64 quartets, leaving input-consumption regressions that could lose or duplicate decoded bytes undetected.

1 open issue
2 resolved since last review

Review effort: Balanced

Comment thread wolfcrypt/src/coding.c Resolved

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11689

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): .wolfssl_known_macro_extras, tests/api/test_coding.h, tests/api/test_evp.h, tests/api/test_evp_cipher.h, tests/api/test_evp_digest.h, tests/api/test_evp_pkey.h, tests/api/test_ossl_rsa.h, tests/api/test_ossl_x509_io.h
Coverage: 16 of 30 in-scope changed file(s) opened by the reviewer; not opened: tests/api.c, tests/api/test_coding.c, tests/api/test_evp.c, tests/api/test_evp_cipher.c, tests/api/test_evp_digest.c, tests/api/test_evp_pkey.c, tests/api/test_ossl_rsa.c, tests/api/test_ossl_x509_io.c, tests/api/test_port.c, wolfssl/internal.h and 4 more

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Lite

Comment thread wolfcrypt/src/evp.c Resolved
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch 2 times, most recently from 9e19be4 to 0fc4008 Compare November 5, 2026 04:08
@SparkiDev SparkiDev removed their assignment Nov 5, 2026
@philljj

philljj commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

@claude review

@philljj
philljj requested a review from julek-wolfssl November 6, 2026 19:11
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread src/x509.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread wolfcrypt/src/evp.c Resolved
Comment thread src/ssl_crypto.c Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Findings marked 🟡 are optional suggestions and need no follow-up push.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🔴 wolfcrypt/src/evp.c:10177 — FIPS builds: RSA keys attached with EVP_PKEY_set1_RSA() can no longer be used for private decryption. EVP_PKEY_decrypt and RSA_private_decrypt now fail because the key has no blinding RNG. set1_RSA now lends the pkey's RNG only when key->rngInited is set (evp.c:10169). Under HAVE_FIPS, wolfssl_rsa_ensure_rng() never sets that flag, so the key keeps a NULL rng. Fix: under HAVE_FIPS, set1_RSA should still give the RSA key an RNG (EvpPkeyRng(pkey)) when it has none, or ensure_rng should provide one in FIPS builds as well.

    Why this was flagged

    In a HAVE_FIPS build with OPENSSL_EXTRA and WC_RSA_BLINDING (the harden default), wolfSSL_RSA_new_ex() has never set an RNG, because that block is guarded by !HAVE_FIPS. On the base branch, wolfSSL_EVP_PKEY_set1_RSA() ran if (key->ownRng == 0) wc_RsaSetRNG(internal, &pkey->rng). ownRng is always 0 in FIPS, so every RSA key attached that way got the pkey's seeded RNG. The new code at evp.c:10169 lends it only when key->rngInited && key->ownRng == 0. wolfssl_rsa_ensure_rng() (pk_rsa.c:288) does nothing under HAVE_FIPS and never sets rngInited, so the lend never happens. wolfSSL_EVP_PKEY_decrypt (evp.c:3295) then calls wolfSSL_RSA_private_decrypt(). That function skips its new ensure_rng check under HAVE_FIPS (pk_rsa.c:3745) and calls wc_RsaPrivateDecrypt(), which takes key->rng (rsa.c:4438). With a NULL rng, blinding fails with MISSING_RNG_E (mp_rand or the SP path), and decryption that worked on the base now returns -1. Signing is unaffected because it passes an RNG explicitly through WOLFSSL_RSA_GetRNG().

    Verification: On the base, set1_RSA ran if (key->ownRng == 0) wc_RsaSetRNG(internal, &pkey->rng) unconditionally (base evp.c:9646-9653). The head lends the RNG only if (key->rngInited && (key->ownRng == 0)) (evp.c:10169). wolfssl_rsa_ensure_rng() does nothing under HAVE_FIPS, so rngInited is never set in FIPS builds. So RSA_private_decrypt returns -1.

@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch 2 times, most recently from f55123d to 840012d Compare November 9, 2026 07:14
@SparkiDev
SparkiDev requested a balanced review from Copilot November 9, 2026 07:23
@SparkiDev

Copy link
Copy Markdown
Contributor Author

@claude review

Copilot AI left a comment

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.

🟡 Changes recommended

The test requires successful RSA serialization in FIPS/self-test builds without WOLFSSL_KEY_TO_DER, causing failures when the encoder is intentionally unavailable.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread tests/api/test_evp_pkey.c Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread wolfcrypt/src/evp.c Resolved
…ound

Performance:

 - Size WOLFSSL_SHA3_CTX's holder in pointers, not bytes. It reserved
   sizeof(wc_Sha3) pointers for a 440 byte object, so WOLFSSL_Hasher goes
   3520 -> 448 bytes and WOLFSSL_EVP_MD_CTX 3552 -> 944. A static assert now
   catches the same mistake.
 - Resolve digests and ciphers by table pointer before comparing names, and
   return the table's own pointer from the accessors. EVP_sha256()
   47.4 -> 1.2ns, EVP_MD_type 64.7 -> 4.3ns, EVP_CIPHER_nid 101.2 -> 8.1ns.
 - Copy only the fields past the hash union in EVP_MD_CTX_copy_ex, and leave
   the cipher union alone in EVP_CIPHER_CTX_init.
 - Set up an EVP_PKEY's RNG, an RSA key's blinding RNG and an EVP_PKEY's DER
   encoding on first use rather than at creation. Seeding a DRBG cost more
   than everything else about making a key put together.
   EVP_PKEY_new + free 13366 -> 34ns, RSA_new + free 13748 -> 230ns,
   EVP_PKEY_set1_RSA 13994 -> 45ns. Certificate parsing roughly halves as a
   result: d2i_X509 45978 -> 17198ns, X509_dup 45945 -> 17116ns, both now
   faster than OpenSSL. set1_DSA still encodes immediately, as WOLFSSL_DSA is
   not reference counted and the pkey only borrows the caller's key.
 - Keep the decoded public key on the X509 and hand out a reference, as
   OpenSSL does. X509_get_pubkey + free 27692 -> 9.8ns, against 9.6ns.
 - Give public key operations the shared global RNG instead of seeding one per
   call, where the instance locks itself or the build is single threaded.
   WOLFSSL_PK_LOCAL_RNG forces an RNG per call. EVP_DigestSign with ECDSA
   P-256 32291 -> 17008ns, against 18852ns.
 - Encode base64 without a call per output character, and hand whole blocks to
   Base64_Encode() rather than one 48 byte line at a time. EVP_EncodeUpdate
   6446 -> 1855ns per 4KB. Skip the call into Base64_SkipNewline() for
   characters needing no skipping: EVP_DecodeUpdate 37059 -> 16194ns. What
   remains there is the constant time character decode, which is deliberate;
   wc_PemToDer() already picks the table decoder by PEM type.
 - Search cipher_tbl before the alias table in EVP_get_cipherbyname(), since
   over half the aliases only spell a name already there in another case, and
   compare without the platform's strcasecmp: 330 -> 67ns. The comparison is
   ASCII only, so it no longer follows the locale and works on platforms whose
   XSTRCASECMP is strcmp.
 - Find the PEM footer in a memory BIO's buffer and read up to it, rather than
   reading a byte at a time through the whole input. Loading a certificate
   from a file that carries a text dump alongside it 100311 -> 23301ns,
   against 33006ns.

Fixes:

 - AES-CCM through EVP now behaves as OpenSSL's does: the payload length comes
   from the first Update() carrying payload, AAD before the length is refused,
   and Final() has nothing left to do. Of 376 observables compared against
   OpenSSL - the tag and IV length matrices, the defaults, length inference
   over 48 sizes, and an encrypt/decrypt matrix with the tag then corrupted -
   375 agree. The one that differs, EVP_CIPHER_iv_length() reporting
   CCM_NONCE_MIN_SZ where OpenSSL reports 12, is unchanged by this work.
 - EVP_get_cipherbyname(NULL) and EVP_get_digestbyname(NULL) dereferenced the
   name; OpenSSL returns NULL.
 - EVP_EncodeUpdate() wrote the block it was holding and then wrote over it,
   so a caller feeding data in pieces got wrong bytes, including bytes never
   written.
 - EVP_PKEY_keygen() on DH read the parameters after replacing the key object
   they came from, which is the same object when a caller passes its own key
   back in.
 - EVP_PKEY_set1_EC_KEY() with an empty EC_KEY now succeeds. It reported
   failure only as a side effect of encoding eagerly; OpenSSL returns 1 and
   the empty key is caught when the encoding is asked for.
 - Remove three GMULT macros naming functions that do not exist.

Portability:

 - Add XMEMCHR, with wc_memchr() for builds naming their own string functions
   through STRING_USER that have no memchr among them.

Tests:

 - Known answer vectors for the base64 encoders, generated independently of
   wolfSSL. testwolfcrypt passed with the encoder deliberately corrupted, as
   it only checked return codes.
 - EVP_EncodeUpdate() over every chunking of ten sizes straddling the 48 byte
   block, and PEM_read_bio_X509() reading several certificates from one BIO
   while leaving nothing behind after one.
 - EVP_get_cipherbyname() over canonical names, alternative spellings, every
   casing of each, and NULL.
 - One RSA private key operation per freshly decoded key: sharing a key lets
   the first operation provide the blinding RNG and hides a missing one in the
   rest. Plus EVP_PKEY_set1_DSA() followed by the caller releasing its key.
 - The reference counting behind the cached X509 public key: outliving the
   certificate and the caller, replacement, and duplication.
 - wc_memchr(), guarded by USE_WOLF_MEMCHR so it runs where that function is
   built.
@SparkiDev
SparkiDev force-pushed the openssl_compat_api_perf_fixes_1 branch from 840012d to 88a01d3 Compare November 9, 2026 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants