Skip to content

fix: force-zero secret buffers before free#1099

Merged
JacobBarthelmeh merged 1 commit into
wolfSSL:masterfrom
MarkAtwood:fix/wolfssh-force_zero
Jul 23, 2026
Merged

fix: force-zero secret buffers before free#1099
JacobBarthelmeh merged 1 commit into
wolfSSL:masterfrom
MarkAtwood:fix/wolfssh-force_zero

Conversation

@MarkAtwood

Copy link
Copy Markdown
Contributor

Force-zero hardening across wolfSSH: scrub secret-bearing buffers before they are freed or leave scope, and fix one private-key leak on an error path. All changes build-verified together on Ubuntu 24.04 with ./configure --with-wolfssl=<all> --enable-all && make (exit 0). Reported by Fenrir static analysis.

  • #1278 src/internal.c KeyAgreeEcdhMlKem_client — ForceZero the ML-KEM private key in handshake->x before wc_MlKemKey_Free, on both success and error, mirroring the DH path.
  • #2082 src/agent.c wolfSSH_AGENT_ID_free — replace WMEMSET with ForceZero for the key buffer and the id struct so the compiler cannot elide the scrub.
  • #2496 src/internal.c SignHRsa/SignHEcdsa — ForceZero digest (and encSig in SignHRsa) before return.
  • #2497 src/internal.c BuildUserAuthRequest{Rsa,RsaCert,Ecc,EccCert} — ForceZero digest; also encDigest (in-scope) for the two RSA paths.
  • #2498 src/internal.c DoUserAuthRequestPublicKey — ForceZero digest before it leaves scope on all exits of the else block.
  • #2500 src/internal.c DoUserAuthRequestRsa — ForceZero encDigest before free (SMALL_STACK) / before scope exit (stack array).
  • #2501 src/internal.c DoUserAuthRequestRsaCert — same encDigest scrub.
  • #2886 src/internal.c SshResourceFree — ForceZero ssh->h and ssh->sessionId (and reset their sizes) alongside the already-zeroed KDF inputs before free.
  • #3453 src/internal.c SetHostPrivateKey — on the CTX-full reject branch, take ownership and ForceZero+WFREE der instead of leaking the private key (no double-free: the store path is unaffected).
  • #3680 src/internal.c ChannelDelete — ForceZero inputBuffer.buffer (NULL-guarded) before free to scrub residual plaintext.
  • #6277 src/wolfsftp.c wolfSSH_SFTP_ClearState GET/PUT and the GET/PUT _CLEANUP cases — ForceZero the SFTP get/put state structs before free (all four sites).
  • #6278 apps/wolfsshd/wolfsshd.c POSIX SHELL_Subsystem — ForceZero channelBuffer/shellBuffer on the data-carrying exit.

ForceZero is already used throughout these translation units; no new includes.

Copilot AI review requested due to automatic review settings July 10, 2026 00:31

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.

Pull request overview

This PR strengthens wolfSSH’s secret-handling hygiene by ensuring secret-bearing buffers are reliably scrubbed with ForceZero() before being freed or leaving scope, and by fixing a private-key leak on a context-full error path.

Changes:

  • Added ForceZero() scrubbing for authentication digests/signature buffers and key-exchange private-key material in src/internal.c.
  • Replaced non-forced zeroing (WMEMSET) with ForceZero() for agent identity buffers in src/agent.c, and scrubbed plaintext-carrying channel/SFTP state buffers before free.
  • Ensured wolfsshd subsystem stack buffers are scrubbed on exit.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/internal.c Scrubs KEX/auth/signature/session materials with ForceZero() and fixes a private-key leak when host-key slots are exhausted.
src/agent.c Uses ForceZero() to scrub agent identity key material and the ID struct before free.
src/wolfsftp.c Scrubs SFTP GET/PUT state structs before freeing them in multiple cleanup paths.
apps/wolfsshd/wolfsshd.c Scrubs subsystem I/O buffers (channelBuffer/shellBuffer) before returning.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MarkAtwood
MarkAtwood requested a review from ejohnstown July 10, 2026 00:59
@ejohnstown
ejohnstown force-pushed the fix/wolfssh-force_zero branch 2 times, most recently from 0849cc9 to a347660 Compare July 23, 2026 00:32
Comment thread apps/wolfsshd/wolfsshd.c Outdated
Zeroize key/digest/handshake buffers before they are freed or go out
of scope, and fix a private-key leak on the CTX-full reject path.
Build-verified with ./configure --enable-all (make exit 0).

- #1278 internal.c KeyAgreeEcdhMlKem_client: WS_FORCEZERO handshake->x
  ML-KEM private key before wc_MlKemKey_Free, matching the DH path.
- #2082 agent.c wolfSSH_AGENT_ID_free: WMEMSET -> WS_FORCEZERO for the
  key buffer and the id struct so scrubs are not optimized away.
- #2496 internal.c SignHRsa/SignHEcdsa: WS_FORCEZERO digest (and encSig
  in SignHRsa) before return.
- #2497 internal.c BuildUserAuthRequest Rsa/RsaCert/Ecc/EccCert:
  WS_FORCEZERO digest (and encDigest for the RSA paths).
- #2498 internal.c DoUserAuthRequestPublicKey: WS_FORCEZERO digest before
  it leaves scope on both success and failure exits.
- #2500 internal.c DoUserAuthRequestRsa: WS_FORCEZERO encDigest before
  free (both SMALL_STACK and stack-array paths).
- #2501 internal.c DoUserAuthRequestRsaCert: same encDigest scrub.
- #2886 internal.c SshResourceFree: WS_FORCEZERO ssh->h and ssh->sessionId
  (and reset sizes) alongside the already-zeroed KDF inputs.
- #3453 internal.c SetHostPrivateKey: on CTX-full reject, take ownership
  and WS_FORCEZERO+WFREE der instead of leaking the private key.
- #3680 internal.c ChannelDelete: WS_FORCEZERO inputBuffer before free.
- #6277 wolfsftp.c ClearState/GET/PUT cleanup: WS_FORCEZERO SFTP get/put
  state structs before free (all four sites).
- #6278 wolfsshd.c SHELL_Subsystem: WS_FORCEZERO channelBuffer/shellBuffer
  on the data-carrying exit.
@ejohnstown
ejohnstown force-pushed the fix/wolfssh-force_zero branch from a347660 to 31a0458 Compare July 23, 2026 20:39

@ejohnstown ejohnstown 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.

The original was good except for rebasing and changing everything to the new WS_FORCEZERO macro.

@JacobBarthelmeh
JacobBarthelmeh merged commit 216c1a0 into wolfSSL:master Jul 23, 2026
136 checks passed
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