Skip to content

fix: raise wolfSSH crypto minimum - #1101

Open
MarkAtwood wants to merge 1 commit into
wolfSSL:masterfrom
MarkAtwood:fix/wolfssh-crypto-minimums
Open

fix: raise wolfSSH crypto minimum#1101
MarkAtwood wants to merge 1 commit into
wolfSSL:masterfrom
MarkAtwood:fix/wolfssh-crypto-minimums

Conversation

@MarkAtwood

@MarkAtwood MarkAtwood commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

This change raises a cryptographic minimum and rejects peers below it by design — good hardening, but calling it out explicitly since it affects interop.

  • RSA user-authentication keys must be ≥ 2048 bits (Fenrir F-6517)
    Per NIST SP 800-131A; matches OpenSSH's floor. The DoUserAuthRequestRsa path previously enforced no minimum (the cert path DoUserAuthRequestRsaCert already did). Added the missing check, and introduced a WOLFSSH_RSA_MIN_KEY_BITS define (default 2048) that both RSA auth paths now use, so the minimum is uniform and configurable. Both paths count the modulus bits with mp_count_bits() rather than scaling the encrypt size, which rounds up to a byte and would let a 2041-2047 bit key through. Smaller keys fail with WS_CERT_KEY_SIZE_E.

Compatibility: deployments still using sub-2048-bit RSA keys will be rejected and must regenerate.

Split out from #1097 so the uncontroversial protocol fixes there can merge independently of this policy decision. Build-verified (--enable-all) against wolfSSL. Reported by static analysis (Fenrir F-6517).

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

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

Raises wolfSSH’s baseline cryptographic acceptance thresholds to improve security posture, rejecting peers that negotiate below the new minimums (with build-time overrides intended for deployments that must relax them).

Changes:

  • Increase DH group-exchange minimum modulus size from 1024 to 2048 bits (WOLFSSH_DEFAULT_GEXDH_MIN).
  • Enforce a minimum RSA public-key size for user authentication (new WOLFSSH_RSA_MIN_KEY_BITS, default 2048) and reject smaller keys with WS_CERT_KEY_SIZE_E.

Reviewed changes

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

File Description
wolfssh/internal.h Raises default DH GEX minimum and introduces WOLFSSH_RSA_MIN_KEY_BITS build-time minimum.
src/internal.c Adds RSA user-auth raw public-key size enforcement using WOLFSSH_RSA_MIN_KEY_BITS.

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

Comment thread wolfssh/internal.h
Comment thread src/internal.c Outdated

@aidangarske aidangarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐺 Skoll Code Review

Overall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped

Posted findings

  • [Low] RSA user-auth minimum accepts 2041-2047 bit keyssrc/internal.c:8217-8221
  • [Medium] Add regression coverage for the new crypto minimumssrc/internal.c:8217-8222, wolfssh/internal.h:513-521

Review generated by Skoll.

Comment thread src/internal.c
Comment thread src/internal.c
@ejohnstown
ejohnstown force-pushed the fix/wolfssh-crypto-minimums branch from ac14a5c to c99f52c Compare July 28, 2026 05:38
@ejohnstown ejohnstown changed the title fix: raise wolfSSH crypto minimums fix: raise wolfSSH crypto minimum Jul 28, 2026
Raise a cryptographic minimum to current guidance. It rejects peers below
the new floor by design.

- Reject RSA user-authentication keys smaller than 2048 bits with
  WS_CERT_KEY_SIZE_E, per NIST SP 800-131A. The floor is configurable via
  a new WOLFSSH_RSA_MIN_KEY_BITS define (default 2048).

Issue: F-6517
@ejohnstown
ejohnstown force-pushed the fix/wolfssh-crypto-minimums branch from c99f52c to 87a83a5 Compare July 28, 2026 05:52
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.

6 participants