Skip the redundant signature verification for normal CAs. by yosuke-wolfssl · Pull Request #11892 · wolfSSL/wolfssl

yosuke-wolfssl · 2026-10-26T01:48:54Z

Problem

PR #11334 introduced selfSigned and its verifier ConfirmSelfSignature.
It increased the memory usage for CA loads unconditionally.
The PR recognized the runtime cost only.

Fix

a CA_TYPE cert now skips ConfirmSelfSignature() only when its AKID is set and equals its SKID, and Key Usage allows keyCertSign.
Also, ProcessPeerCertParse() sets selfSigned on the small-cert path when the signature verified with the certificate's own key. wc_CheckCertSignature_ex() is added for the purpose.

Test

make check with --enable-all: pass.

Verification (peak heap)

Built the low_ecc and low_rsa2048 configs from nightly-memusage-v2, plus
--enable-trackmemory, and ran one mutual-auth TLS 1.2 handshake between
examples/server/server and examples/client/client. Numbers are the
peak Bytes line each process prints on exit (whole process, CA load
included); repeated runs gave identical numbers.

ECC: server -c certs/server-ecc.pem -k certs/ecc-key.pem -A certs/client-ecc-cert.pem,
client -A certs/ca-ecc-cert.pem -c certs/client-ecc-cert.pem -k certs/ecc-client-key.pem
RSA: default certs on both sides (DHE-RSA)

Peak bytes	before	after
low_ecc client	10313	9123
low_ecc server	11241	9425
low_rsa2048 client	19895	17355
low_rsa2048 server	21513	21169

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

github-actions · 2026-10-26T01:52:21Z

MemBrowse Memory Report

gcc-arm-cortex-m0plus

FLASH: .text +68 B (+0.1%, 70,499 B / 262,144 B, total: 27% used)

gcc-arm-cortex-m3

FLASH: .text +48 B (+0.0%, 130,789 B / 262,144 B, total: 50% used)

gcc-arm-cortex-m4-baremetal

FLASH: .text +64 B (+0.1%, 73,015 B / 262,144 B, total: 28% used)

gcc-arm-cortex-m4-crypto-only

FLASH: .text +64 B (+0.0%, 182,527 B / 262,144 B, total: 70% used)

gcc-arm-cortex-m4-dtls13

FLASH: .text +64 B (+0.0%, 194,328 B / 1,048,576 B, total: 19% used)

gcc-arm-cortex-m4-min-ecc

FLASH: .text +64 B (+0.1%, 67,673 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-openssl-compat

FLASH: .text +128 B (+0.0%, 796,140 B / 1,048,576 B, total: 76% used)

gcc-arm-cortex-m4-pkcs7

FLASH: .text +64 B (+0.0%, 223,616 B / 262,144 B, total: 85% used)

gcc-arm-cortex-m4-pq

FLASH: .text +64 B (+0.0%, 311,780 B / 1,048,576 B, total: 30% used)

gcc-arm-cortex-m4-rsa-only

FLASH: .text +64 B (+0.0%, 343,128 B / 1,048,576 B, total: 33% used)

gcc-arm-cortex-m4-sp-math

FLASH: .text +64 B (+0.1%, 67,673 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-tls13

FLASH: .text +64 B (+0.0%, 249,294 B / 262,144 B, total: 95% used)

gcc-arm-cortex-m7-pq

FLASH: .text +64 B (+0.0%, 312,676 B / 1,048,576 B, total: 30% used)

linuxkm-standard

Data: __patchable_function_entries +8 B (+0.0%, 52,048 B)

stm32-sim-stm32h753

FLASH: .text +128 B (+0.1%, 205,680 B / 2,097,152 B, total: 10% used)
No memory changes detected for:
gcc-arm-cortex-m4
gcc-arm-cortex-m4-tls12
gcc-arm-cortex-m7
gcc-arm-cortex-m7-tls13
linuxkm-pie

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11892

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer

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

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11892

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 1 of 3 in-scope changed file(s) opened by the reviewer; not opened: tests/api.c, wolfssl/wolfcrypt/asn.h

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

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11892

Scan targets checked: wolfssl-src, wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Coverage: 1 of 2 in-scope changed file(s) opened by the reviewer; not opened: tests/api.c

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

Frauschi

Thanks for chasing down the memory cost from #11334. Skipping the self-signature check for AKID == SKID roots looks sound for the common case. A few comments: the VERIFY_NAME change sets selfSigned from a key that isn't guaranteed to be the one that verified the signature, the skip is a bit broader than the root case it targets, and the new tests don't exercise the changed path yet.

Frauschi · 2026-10-26T12:29:55Z

                }
            #endif /* WOLFSSL_DUAL_ALG_CERTS */
            }
+            else if (verify == VERIFY_NAME) {


This assumes cert->ca holds the key that verified the signature, which isn't guaranteed here. Under WOLFSSL_SMALL_CERT_VERIFY the check was done by wc_CheckCertSignature(), which looks up its own CA in the cert manager only, while ParseCertRelative() can take cert->ca from the CSR v2 pending list first. Also, if sigRet failed and the verify callback overrides it, selfSigned still gets set from a plain key compare.

selfSigned exempts a chain CA from CheckChainCAExtKeyUsage(), so it should only come from a verified signature. --enable-lowresource turns on WOLFSSL_SMALL_CERT_VERIFY, so this branch is probably part of your numbers and just dropping it isn't free. Could the small-cert path pass down the key it actually verified with (and nothing when sigRet != 0), and compare against that?

Done as suggested. The small-cert path now calls the new wc_CheckCertSignature_ex(), which reports the key of the CA that verified the signature. selfSigned is set from that key only when sigRet == 0, and ParseCertRelative() no longer derives it under VERIFY_NAME. Peak heap is unchanged.

Frauschi · 2026-10-26T12:29:56Z

            XMEMCMP(sigKey, cert->publicKey, cert->pubKeySize) == 0;
    }
-    else if (cert->selfIssued) {
+    else if (cert->selfIssued && (type != CA_TYPE || !cert->extAuthKeyIdSet ||


A genuinely self-signed root whose AKID doesn't match its SKID now ends up with selfSigned = 0: this skips ConfirmSelfSignature(), and UserRootEndsNcWalk() doesn't mark it either. Think of a renewed root whose AKID still names the old key, or one whose AKID was computed with a different key-ID method than ours. The name-constraint walk then continues past it and can pick up another same-DN CA's constraints, or fall back to a name lookup under NO_SKID, which is the case test_NameConstraints_SameDnRoots is about. Worst case is a spurious ASN_NAME_INVALID_E, not a bypass.

Could the skip be limited to AKID == SKID? That's the case the memory saving targets, and anything else keeps the full check.

Done: the skip now needs AKID == SKID; anything else gets the full check. test_NameConstraints_RootAkid covers a renewed root with a stale AKID.

Frauschi · 2026-10-26T12:29:56Z


+/* Returns 1 when a user-trusted CA is a root that ends the name-constraint
+ * walk without its self-signature being verified. */
+static int UserRootEndsNcWalk(const DecodedCert* cert)


I don't think this is needed. When the root has an AKID, FindSignerByAkidOrName() treats it as authoritative, and AlreadySigner() dedups by SKID (by subject name under NO_SKID), so for an AKID == SKID root the walk finds the root itself and stops on ncParent == ncSigner. I forced this to return 0 in an --enable-all build and the whole API suite still passed.

Dropping it keeps Signer::selfSigned meaning what it says (the asn.h comment change can go too), and there's no second copy of the KU/AKID condition from ParseCertRelative() to keep in sync.

Agreed, removed along with the asn.h comment change.

Frauschi · 2026-10-26T12:29:56Z

+/* Two trusted roots share a DN and carry no AKID, so a name lookup from one
+ * root can return the other. Each root's leaf must be checked against its own
+ * root's nameConstraints only, whichever root the lookup returns. */
+static int test_NameConstraints_SameDnRoots(void)


Both new tests pass without the library change: I reverted ssl_certman.c, asn.c and asn.h to the base commit and they still pass. SameDnRoots builds roots without an AKID, so they still go through ConfirmSelfSignature(), and in RolloverCa K already got selfSigned = 0 from the old check.

Could you add a case for the path this PR changes? A trusted root with AKID == SKID and nameConstraints, plus a self-signed root whose AKID doesn't match its SKID, would cover it.

Added test_NameConstraints_RootAkid. Its renewed-root case fails without the AKID == SKID condition.

Frauschi · 2026-10-26T12:29:56Z

+
+/* R's nameConstraints must reach leaves of K, a trusted key-rollover CA under
+ * R, with or without an AKID in K. */
+static int test_NameConstraints_RolloverCa(void)


These two tests load two CAs with the same DN, so the guard needs !defined(NO_SKID). Under NO_SKID, AlreadySigner() and HashSigner() key on the subject name, so the second CA isn't added and both tests would fail. I haven't built that config, so please double-check.

Doen as suggested

- a CA_TYPE cert now skips ConfirmSelfSignature() only when its AKID is set and equals its SKID, and Key Usage allows keyCertSign - wc_CheckCertSignature_ex() reports the key of the CA that verified the signature; wc_CheckCertSignature() calls it without one. - CheckCertSignature_ex() in asn.c and asn_orig.c fills that key. - ProcessPeerCertParse() sets selfSigned on the small-cert path when the signature verified with the certificate's own key.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11892

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): doc/dox_comments/header_files/asn.h
Coverage: 5 of 5 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

yosuke-wolfssl · 2026-10-27T04:06:56Z

Hi @Frauschi ,
Thank you for reviewing. I addressed all issues you mentioned.

yosuke-wolfssl self-assigned this Oct 26, 2026

Copilot AI balanced review requested due to automatic review settings October 26, 2026 01:48

Copilot AI reviewed Oct 26, 2026

yosuke-wolfssl force-pushed the fix/ConfirmSelfSignature branch from b64333d to b147ee5 Compare October 26, 2026 02:05

yosuke-wolfssl requested a review from wolfSSL-Fenrir-bot October 26, 2026 02:08

yosuke-wolfssl changed the title ~~Skip the full signature verification for normal CAs.~~ Skip the redundant signature verification for normal CAs. Oct 26, 2026

wolfSSL-Fenrir-bot previously requested changes Oct 26, 2026

View reviewed changes

Comment thread wolfcrypt/src/asn.c Outdated

yosuke-wolfssl force-pushed the fix/ConfirmSelfSignature branch from b147ee5 to b23f009 Compare October 26, 2026 02:52

yosuke-wolfssl requested a review from wolfSSL-Fenrir-bot October 26, 2026 02:52

wolfSSL-Fenrir-bot previously requested changes Oct 26, 2026

View reviewed changes

Comment thread src/ssl_certman.c Outdated

yosuke-wolfssl force-pushed the fix/ConfirmSelfSignature branch from b23f009 to ef7749a Compare October 26, 2026 04:22

yosuke-wolfssl requested a review from wolfSSL-Fenrir-bot October 26, 2026 04:27

wolfSSL-Fenrir-bot previously requested changes Oct 26, 2026

View reviewed changes

Comment thread src/ssl_certman.c Outdated

yosuke-wolfssl force-pushed the fix/ConfirmSelfSignature branch from ef7749a to 15797c4 Compare October 26, 2026 05:03

yosuke-wolfssl assigned wolfSSL-Bot and unassigned yosuke-wolfssl Oct 26, 2026

Frauschi reviewed Oct 26, 2026

View reviewed changes

Frauschi assigned yosuke-wolfssl Oct 26, 2026

yosuke-wolfssl force-pushed the fix/ConfirmSelfSignature branch from 15797c4 to 300fce8 Compare October 27, 2026 02:30

yosuke-wolfssl requested a review from wolfSSL-Fenrir-bot October 27, 2026 02:30

yosuke-wolfssl force-pushed the fix/ConfirmSelfSignature branch from 300fce8 to 89a2464 Compare October 27, 2026 02:41

wolfSSL-Fenrir-bot reviewed Oct 27, 2026

View reviewed changes

yosuke-wolfssl assigned Frauschi and unassigned wolfSSL-Bot Oct 27, 2026

yosuke-wolfssl removed their assignment Oct 27, 2026

Conversation

yosuke-wolfssl commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Test

Verification (peak heap)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

MemBrowse Memory Report

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11892

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11892

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11892

Uh oh!

Uh oh!

Frauschi left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11892

Uh oh!

yosuke-wolfssl commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yosuke-wolfssl commented Oct 26, 2026 •

edited

Loading

github-actions Bot commented Oct 26, 2026 •

edited

Loading