Skip to content

Generic X.509 certificate helpers and secure file utilities#276

Open
stenslae wants to merge 4 commits into
wolfSSL:mainfrom
stenslae:pr-x509-only
Open

Generic X.509 certificate helpers and secure file utilities#276
stenslae wants to merge 4 commits into
wolfSSL:mainfrom
stenslae:pr-x509-only

Conversation

@stenslae

@stenslae stenslae commented Jul 15, 2026

Copy link
Copy Markdown
Member

Added helper functions to do secure file operations. Added simplified certificate parsing and mapping logic into helper functions. Existing modules were refactored to utilize new functions. Added CI tests for all new changes.

This allows for potential reduction in openssl compat layer reliance and new signature algos.

@stenslae stenslae self-assigned this Jul 15, 2026
@stenslae
stenslae requested review from wolfSSL-Fenrir-bot and removed request for wolfSSL-Fenrir-bot July 16, 2026 17:14

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src

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

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread tests/x509/cert_setup_unit_test.c
Comment thread src/tools/clu_funcs.c
Comment thread tests/x509/cert_setup_unit_test.c
Comment thread src/tools/clu_funcs.c

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src

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

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread tests/x509/cert_setup_unit_test.c
Comment thread src/genkey/clu_genkey.c Outdated
Comment thread tests/x509/cert_setup_unit_test.c
Comment thread tests/x509/cert_setup_unit_test.c
Comment thread src/genkey/clu_genkey.c Outdated
Comment thread tests/x509/cert_setup_unit_test.c

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src

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

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/genkey/clu_genkey.c Outdated
Comment thread src/genkey/clu_genkey.c Outdated

@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 #276

⚠️ An internal error occurred during the automated review. This error has been logged. Please contact the Fenrir team if you need assistance.

Error: GitHubAPIError

@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 #276

⚠️ An internal error occurred during the automated review. This error has been logged. Please contact the Fenrir team if you need assistance.

Error: GitHubAPIError

@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 #276

⚠️ An internal error occurred during the automated review. This error has been logged. Please contact the Fenrir team if you need assistance.

Error: GitHubAPIError

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src

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

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread tests/x509/cert_setup_unit_test.c Outdated
Comment thread src/tools/clu_funcs.c Outdated
Comment thread src/tools/clu_funcs.c Outdated
Comment thread src/tools/clu_funcs.c Outdated
Comment thread src/tools/clu_funcs.c Outdated
Comment thread tests/x509/cert_setup_unit_test.c Outdated

@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 #276

Scan targets checked: none
Failed targets: wolfclu-bugs, wolfclu-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 4
3 finding(s) posted as inline comments (see file-level comments below)

Low (1)

RSA private key DER buffer not zeroed when second i2d encoding call fails

Function: wolfCLU_RSA
Category: Sensitive data handling

The PR adds a derSz < 0 guard after the second wolfSSL_i2d_RSAPrivateKey(rsa, &pt) call to detect encoding failure, and a matching if (derSz > 0) guard before wolfCLU_ForceZero to prevent calling ForceZero with a negative size (which would be UB). The combination is logically correct for preventing UB, but it creates a gap: when the second i2d call fails, der has already been allocated to hold the private key DER and may contain partial RSA private key material written by the failing call. Because derSz is now negative, the if (derSz > 0) guard prevents zeroing, so that material is freed without being wiped. The allocation size from the first i2d call is not tracked separately, so it is unavailable for the cleanup. The private key path for public RSA output (wolfSSL_i2d_RSAPublicKey) and the analogous ECC path in _ECCpKeyPEMtoKey (src/pkey/clu_pkey.c:98) share the same pattern. In practice, the second i2d call almost never fails after the first succeeds, so the exploitable window is extremely narrow.

Recommendation: Track the allocation size separately (e.g. word32 allocSz = (word32)derSz; after the first i2d call), then use allocSz in the cleanup: if (allocSz > 0) wolfCLU_ForceZero(der, allocSz);. This ensures zeroing occurs regardless of whether the second i2d call succeeds.


This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/genkey/clu_genkey.c Outdated
Comment thread src/genkey/clu_genkey.c
Comment thread src/tools/clu_funcs.c
Comment thread src/genkey/clu_genkey.c
Comment thread src/tools/clu_funcs.c
Comment thread src/genkey/clu_genkey.c Outdated

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src

No new issues found in the changed files. ✅

@stenslae
stenslae marked this pull request as ready for review July 23, 2026 17:09

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src

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

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/pkcs/clu_pkcs12.c
Comment thread src/pkcs/clu_pkcs12.c

@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 #276

Scan targets checked: wolfclu-bugs, wolfclu-src

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

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/tools/clu_funcs.c
if (lstat(path, &st) == 0 && S_ISLNK(st.st_mode) && !ownerOnly) {
struct stat targetSt;
if (stat(path, &targetSt) == 0 && !S_ISREG(targetSt.st_mode)) {
return fopen(path, mode);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 [Low] TOCTOU between stat() and fopen() in non-secret symlink fallback path · File handling vulnerabilities

Between stat(path, &targetSt) (line 1331) and fopen(path, mode) (line 1332), an attacker with write access to the parent directory can swap the symlink target. If the original target was a non-regular file (e.g., /dev/null), the condition is satisfied and fopen follows the new, attacker-chosen target.

Fix: Use open(O_PATH) to pin the symlink inode, then openat(fd, "", O_EMPTY_PATH|O_WRONLY) to open through it atomically, or document that this is an accepted trade-off for the -out /dev/stdout use case.

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.

2 participants