Skip to content

Export a constant-time compare to applications - #1337

Open
ejohnstown wants to merge 5 commits into
wolfSSL:masterfrom
ejohnstown:ct-compare
Open

ejohnstown wants to merge 5 commits into
wolfSSL:masterfrom
ejohnstown:ct-compare

Conversation

@ejohnstown

Copy link
Copy Markdown
Contributor

wolfSSH_ConstantCompare() gives userauth callbacks the library's
constant-time compare. Like WS_FORCEZERO(), the internal
WS_CONSTANTCOMPARE() maps to wolfCrypt's wc_ConstantCompare() on
wolfSSL v5.9.4 and later and to a volatile local loop otherwise.

  • New public API; it needs a manual entry and a ChangeLog line.
  • wolfsshd calls the export, which also fixes its --disable-inline
    link failure.
  • The echoserver and tpmcertserver compare secrets with it, and the
    echoserver wipes its digest buffer on every return.
  • The echoserver frees the DecodedCert in FPKI userauth and no longer
    reads past the username in PwMapNew.

Issue: F-7513, F-7514, F-8858, F-12630, F-13997, F-13329

Userauth callbacks compare passwords and digests outside the library,
where the internal ConstantCompare() is hidden. Export a wrapper, and
add WS_CONSTANTCOMPARE(), which, like WS_FORCEZERO(), maps to
wolfCrypt's wc_ConstantCompare() when the wolfSSL build provides it
(v5.9.4 and later) and to the local loop otherwise.

- misc.c: build the local loop only as the fallback, with a volatile
  accumulator
- internal.c: compare MACs and the CBC check block with the macro
- ssh.h: point the userauth callback documentation at the export
- api.c: test match, mismatch, length and argument handling

Issue: F-7513, F-7514
auth.c called the library-internal ConstantCompare(), which does not
resolve when the library is built with --disable-inline. Call
wolfSSH_ConstantCompare() instead.

Issue: F-7514
The echoserver and tpmcertserver userauth callbacks compare the stored
digest or password with wolfSSH_ConstantCompare(). The echoserver
callback now wipes its digest buffer on every return.

Issue: F-8858, F-12630
wsUserAuth parses the peer certificate to print its FASC-N and UUID.
Free the DecodedCert before leaving that block, on success and on
failure alike.

Issue: F-13997
The copy read one byte past the username; the terminator is written
explicitly after it.

Issue: F-13329
Copilot AI balanced review requested due to automatic review settings October 22, 2026 20:49

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 6 of 9 in-scope changed file(s) opened by the reviewer; not opened: apps/wolfsshd/auth.c, src/internal.c, wolfssh/ssh.h

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

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.

3 participants