Fix private key file permissions and several CLI correctness bugs by julek-wolfssl · Pull Request #298 · wolfSSL/wolfCLU

julek-wolfssl · 2026-09-28T17:14:34Z

Security:

Add wolfCLU_FileOpenOwner/wolfCLU_BioOpenOwner helpers that create new files with mode 0600 on POSIX (via direct open(), gated by WOLFCLU_POSIX_FILE).
Use these helpers instead of fopen/BIO_new_file for private key output in genkey, pkcs8, rsa, dsaparam -genkey, pkcs12, req -keyout, pkey, dhparam -genkey, ecparam -genkey, and the XMSS state file, so outputs are no longer world-readable under umask 022.
-out without a file name in rsa, pkcs12, and pkey now fails instead of silently falling back to stdout.
Serialize XMSS signing with an flock() on the .priv state file to prevent concurrent signers from reusing a one-time key.

Bug fixes:

s_server: fail on compiled-out protocol versions instead of continuing with a NULL method/ctx, and propagate the failure as a non-zero exit code.
s_server: compare the throughput remainder as size_t before narrowing to int, fixing an infinite echo loop for remainders that are multiples of 2^32.
parseAlgo: reject abbreviated/unsupported cipher modes instead of silently leaving the algorithm uninitialised.
wolfCLU_GetStdinPassword: return an empty password on EOF instead of reading uninitialized memory; pkcs12, pkcs8, and req now check this result.
Legacy (Camellia) encrypt with explicit -key/-iv: mark the pad count in the header so decrypt strips padding correctly.
x509: only force version 3 when signing a CSR (-req), so -text on a v1 cert reports the correct version.
x509 -fingerprint: fail with an error when SHA-1/hashing is unavailable instead of exiting 0 with no output.
-subj parsing: reset the entry encoding per field so a value after C= isn't incorrectly encoded as PrintableString.
basicConstraints: parse as comma-separated NAME:VALUE pairs, fixing dropped pathlen and ignored CA:TRUE, and fail on malformed entries.
pkcs8 -topk8 -outform DER: write proper PKCS#8 DER instead of double-wrapping the traditional key format.
req -verify: verify -x509-generated certificates as certificates rather than CSRs.

Private keys were opened with fopen/BIO_new_file, so under umask 022 they were world-readable. Add wolfCLU_FileOpenOwner/wolfCLU_BioOpenOwner, which create new files with mode 0600 on POSIX, and use them for all genkey private outputs and the XMSS state file. WOLFCLU_POSIX_FILE enables the POSIX path on the platforms where wolfSSL uses it for wc_fopen_owner_only(), and only when wolfSSL defines XFDOPEN and XCLOSE. open() is called directly: wolfSSL's open wrappers are WOLFSSL_LOCAL and not exported from the library. F-8096

pkcs8 -out always holds a private key but was opened with BIO_new_file, so under umask 022 it was world-readable. Use wolfCLU_BioOpenOwner so new files are created 0600 on POSIX. F-9854

rsa opened -out before knowing if a private key would be written, so under umask 022 the key file was world-readable. Open -out after option parsing and use wolfCLU_BioOpenOwner unless the output is public only. -out without a file name fails instead of falling back to stdout. F-9855

dsaparam -genkey wrote the DSA private key through wolfSSL_BIO_new_file, so under umask 022 the file was world-readable. Open the output with wolfCLU_BioOpenOwner when -genkey is given. Params-only output is unchanged. F-9856

pkcs12 opened -out with BIO_new_file, so extracted private keys were world-readable under umask 022. Defer opening until options are parsed and use wolfCLU_BioOpenOwner unless -nokeys is given. -out without a file name fails instead of falling back to stdout. F-9861

req wrote the generated private key with BIO_new_file, so under umask 022 it was world-readable. Open -keyout with wolfCLU_BioOpenOwner so a new key file gets mode 0600, encrypted or not. F-9860

server_test continued with a NULL method and ctx when the requested version was not compiled in. Treat a NULL method or ctx as fatal and propagate server_test's return code so s_server exits non-zero. F-12925

ServerEchoData cast the size_t throughput remainder to int before min(). Remainders that are multiples of 2^32 truncated to 0 and the echo loop spun forever. Compare against the block size as size_t first. F-9845

Mode prefixes like "c" or "cb" passed validation but set no cipher, so encrypt/decrypt used an uninitialised alg and wrote garbage output. Require an exact mode, start alg at WOLFCLU_ALGO_NONE and fail if no cipher was chosen. Initialise alg in wolfCLU_setup too. F-11081

wolfCLU_GetStdinPassword ran strlen on the buffer after fgets failed, reading uninitialised caller buffers. It now returns an empty password of size 0 on failure. pkcs12, pkcs8 and req check the result instead of encrypting or decrypting with a stale or garbage password. F-12121

With -key/-inkey and -iv, the legacy (Camellia) encrypt path wrote an all-zero salt, so decrypt left the block padding in the output. Set salt[0] to the pad count so decrypt strips it. Old files decrypt as before. F-11082

wolfCLU_certSetup set v3 on every parsed cert before checking -req, so -text on a v1 cert reported "Version: 3". Check reqFlag first. certs/server-ecc-v1-cert.pem made with OpenSSL 3.0.13: openssl x509 -req (no extensions) from an ecc-key.pem CSR, signed by ca-ecc-cert.pem. F-12924

wc_HashGetDigestSize's negative result was stored unsigned, and wc_Hash or missing-DER failures never set ret, so -fingerprint exited 0 with no output. Keep the digest size signed, reject <= 0, and return WOLFCLU_FATAL_ERROR with a message on each failure. F-12122

wolfCLU_ParseX509NameString kept CTC_PRINTABLE after a C= entry, so later entries (e.g. a CN with '_' or '@') were encoded as PrintableString. Choose the encoding per entry: C is PrintableString, others UTF8String. Declare the per-entry variables inside the loop so no value can carry over from a previous entry. F-9842

"CA:TRUE, pathlen:0" was split on ':' only, so CA stayed FALSE and pathlen was dropped without an error. Split on ',' then at the first ':', trim whitespace, set pathlen the way wolfSSL_X509_add_ext reads it, and fail on unknown or malformed entries instead of emitting a wrong extension. F-11080

The DER branch always used the traditional encoder, so -topk8 -nocrypt -outform DER wrote RSAPrivateKey/ECPrivateKey. For -topk8, re-encode the key and wrap it with wolfSSL_i2d_PKCS8_PKEY, so PKCS#8 DER input is not double wrapped. Zero the key buffers before freeing. F-9841

pkey opened -out before knowing if a private key would be written, so under umask 022 the converted key file was world-readable. Open -out after option parsing and use wolfCLU_BioOpenOwner unless -pubout/-pubin. -out without a file name fails instead of falling back to stdout. F-9859

dhparam -genkey wrote the DH private key through wolfSSL_BIO_new_file, so under umask 022 the file was world-readable. Open the output with wolfCLU_BioOpenOwner when -genkey is given. Params-only output is unchanged. F-9857

ecparam -genkey wrote the EC private key through wolfSSL_BIO_new_file, so under umask 022 the file was world-readable. Open the output with wolfCLU_BioOpenOwner when -genkey is given. Params-only output is unchanged. F-9858

req -verify always called wolfSSL_X509_REQ_verify, so a certificate made with -x509 was parsed as a CSR and failed. Use wolfSSL_X509_verify for generated certs, and verify with the req/cert public key, since wolfSSL cannot verify with an RSA private key object. F-9846

Concurrent signers could reload the same XMSS state and reuse a one-time key. Hold an flock() on the .priv file (opened read-write, as NFS needs) from reload until the new state is saved and fsynced. On CIFS mounts without nobrl the lock is mandatory and signing fails closed. flock(), fsync() and fileno() are called directly: wolfSSL has no wrappers for them. F-12944

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved critical server null-dereference paths and a moderate pathlen overflow issue block approval.

Review effort: Lite
Findings: 2 · 1

Open (3)

Null ssl cleanup crashes on unsupported protocol path · New
Callback registration can dereference null SSL context · New
pathlen parsing can overflow before bounds validation · New

What changed in this PR

This PR hardens private-key output permissions and fixes CLI, X.509, PKCS#8, encryption, server, and XMSS behavior.

Changes:

Adds owner-only file creation and XMSS state locking.
Fixes parsing, password handling, certificate, encryption, and PKCS#8 behavior.
Expands regression and permission tests.

Outstanding findings include two critical s_server null-dereference paths and a moderate pathlen integer-overflow issue.

File	Description
`wolfclu/genkey/clu_genkey.h`	Declares XMSS locking helpers.
`wolfclu/clu_optargs.h`	Adds an empty-algorithm sentinel.
`wolfclu/clu_header_main.h`	Declares secure file helpers.
`tests/x509/x509-req-test.py`	Tests CSR, verification, and permissions.
`tests/x509/x509-process-test.py`	Tests v1 certificate handling.
`tests/wolfclu_test.py`	Adds PTY test support.
`tests/server/server-test.py`	Tests unsupported protocol versions.
`tests/pkey/rsa-test.py`	Tests RSA output and permissions.
`tests/pkey/pkey-test.py`	Tests PKEY output and permissions.
`tests/pkey/ecparam-test.py`	Tests EC output permissions.
`tests/pkcs/pkcs8-test.py`	Tests PKCS#8 DER and passwords.
`tests/pkcs/pkcs12-test.py`	Tests PKCS#12 output and passwords.
`tests/genkey_sign_ver/genkey-sign-ver-test.py`	Tests XMSS locking and permissions.
`tests/encrypt/enc-test.py`	Tests cipher and padding fixes.
`tests/dsa/dsa-test.py`	Tests DSA output permissions.
`tests/dh/dh-test.py`	Tests DH output permissions.
`src/x509/clu_request_setup.c`	Fixes certificate verification and key output.
`src/x509/clu_config.c`	Parses basic constraints.
`src/x509/clu_cert_setup.c`	Fixes certificate versions and fingerprints.
`src/tools/clu_funcs.c`	Implements secure files, parsing, and EOF handling.
`src/sign-verify/clu_sign.c`	Serializes XMSS signing.
`src/server/server.c`	Fixes protocol failures and throughput sizing.
`src/server/clu_server_setup.c`	Propagates server failures.
`src/pkey/clu_rsa.c`	Secures RSA output and validates `-out`.
`src/pkey/clu_pkey.c`	Secures PKEY output and validates `-out`.
`src/pkcs/clu_pkcs8.c`	Fixes PKCS#8 DER conversion and passwords.
`src/pkcs/clu_pkcs12.c`	Secures output and handles password EOF.
`src/genkey/clu_genkey.c`	Secures key files and XMSS persistence.
`src/ecparam/clu_ecparam.c`	Secures generated EC keys.
`src/dsa/clu_dsa.c`	Secures generated DSA keys.
`src/dh/clu_dh.c`	Secures generated DH keys.
`src/crypto/clu_encrypt.c`	Records explicit-key padding.
`src/crypto/clu_crypto_setup.c`	Initializes algorithm selection safely.
`certs/server-ecc-v1-cert.pem`	Adds a v1 certificate fixture.

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

julek-wolfssl added 21 commits September 25, 2026 17:13

Create pkcs8 output files with owner-only permissions

4fbddc9

pkcs8 -out always holds a private key but was opened with BIO_new_file, so under umask 022 it was world-readable. Use wolfCLU_BioOpenOwner so new files are created 0600 on POSIX. F-9854

Create dsaparam -genkey output with owner-only permissions

cb79142

dsaparam -genkey wrote the DSA private key through wolfSSL_BIO_new_file, so under umask 022 the file was world-readable. Open the output with wolfCLU_BioOpenOwner when -genkey is given. Params-only output is unchanged. F-9856

Create req -keyout private key with owner-only permissions

73543d4

req wrote the generated private key with BIO_new_file, so under umask 022 it was world-readable. Open -keyout with wolfCLU_BioOpenOwner so a new key file gets mode 0600, encrypted or not. F-9860

Fail s_server on compiled-out protocol versions

6606924

server_test continued with a NULL method and ctx when the requested version was not compiled in. Treat a NULL method or ctx as fatal and propagate server_test's return code so s_server exits non-zero. F-12925

Bound s_server throughput remainder before narrowing to int

86cec9c

ServerEchoData cast the size_t throughput remainder to int before min(). Remainders that are multiples of 2^32 truncated to 0 and the echo loop spun forever. Compare against the block size as size_t first. F-9845

Mark padding in legacy encrypt header for explicit key/IV

8f799e7

With -key/-inkey and -iv, the legacy (Camellia) encrypt path wrote an all-zero salt, so decrypt left the block padding in the output. Set salt[0] to the pad count so decrypt strips it. Old files decrypt as before. F-11082

Create dhparam -genkey output with owner-only permissions

815bd89

dhparam -genkey wrote the DH private key through wolfSSL_BIO_new_file, so under umask 022 the file was world-readable. Open the output with wolfCLU_BioOpenOwner when -genkey is given. Params-only output is unchanged. F-9857

Create ecparam -genkey output with owner-only permissions

e013169

ecparam -genkey wrote the EC private key through wolfSSL_BIO_new_file, so under umask 022 the file was world-readable. Open the output with wolfCLU_BioOpenOwner when -genkey is given. Params-only output is unchanged. F-9858

Copilot AI lite review requested due to automatic review settings September 28, 2026 17:14

julek-wolfssl self-assigned this Sep 28, 2026

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

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Comment thread src/server/server.c

Comment thread src/server/server.c

Comment thread src/x509/clu_config.c

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

Navigation Menu

Fix private key file permissions and several CLI correctness bugs - #298

Fix private key file permissions and several CLI correctness bugs#298
julek-wolfssl wants to merge 21 commits into
wolfSSL:mainfrom
julek-wolfssl:fenrir-wolfclu-fixes

julek-wolfssl commented Sep 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

julek-wolfssl commented Sep 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants