Conversation
LinuxJedi
commented
Apr 8, 2026
LinuxJedi
commented
- Fix oct2dec typo
- Fix leak in linked-list
- Fix Nucleus month handling
- Fix DoDisconnect to signal connection termination
- fix DoChannelOpen failure response and add regression test
- Validate the host key signature algorithm name in DoKexDhReply()
F-2069
F-2070
DoDisconnect was returning WS_SUCCESS after receiving SSH_MSG_DISCONNECT, allowing the session to continue processing packets. Per RFC 4253 §11.1, no further data should be accepted after a disconnect message. Add new WS_DISCONNECT error code and return it from DoDisconnect so callers tear down the connection immediately. F-605
Send SSH_MSG_CHANNEL_OPEN_FAILURE for unclassified channel open errors instead of incorrectly falling back to SSH_MSG_REQUEST_FAILURE. Normalize OPEN_OK error cases to an administrative-prohibited channel open failure with a generic description, and add white-box regressions covering callback rejection plus optional direct-tcpip and agent-null paths. F-2076
The client-side KEXDH_REPLY path was parsing the signature blob name and skipping over it without checking that it matched the negotiated host key algorithm. That allowed an RSA server to negotiate rsa-sha2-256 or rsa-sha2-512 but send a signature blob labeled ssh-rsa instead. Fix this by comparing the signature blob name against the expected signature type derived from handshake->pubKeyId before verifying the signature bytes. Add regress coverage that drives an in-memory client/server handshake, rewrites the server's first KEXDH_REPLY on the wire, and verifies the client rejects rsa-sha2-256 and rsa-sha2-512 replies whose signature blob name is downgraded to ssh-rsa. F-2077
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple static-analysis and correctness findings across wolfSSH core and regression tests, tightening protocol handling and fixing a few platform-specific and memory-management issues.
Changes:
- Add a dedicated disconnect error code and propagate disconnect as a terminal condition.
- Harden KEXDH reply processing by validating the signature algorithm name against the negotiated hostkey algorithm (with regression coverage).
- Fix SFTP handle-list unlinking and correct Nucleus
mktime()month handling; also fix octal digit validation and channel-open failure responses.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
wolfssh/error.h |
Adds WS_DISCONNECT and updates WS_LAST_E to include the new terminal error code. |
src/internal.c |
Implements disconnect propagation, validates KEXDH reply signature name, fixes wolfSSH_oct2dec() digit check, and ensures channel-open failures send CHANNEL_OPEN_FAIL. |
src/wolfsftp.c |
Fixes handle-list head removal logic and corrects Nucleus month conversion for mktime(). |
tests/regress.c |
Adds regression coverage for channel-open failure responses and KEXDH reply signature-name downgrade rejection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #908
Scan targets checked: wolfssh-bugs, wolfssh-compliance, wolfssh-consttime, wolfssh-defaults, wolfssh-mutation, wolfssh-proptest, wolfssh-src, wolfssh-zeroize
Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
