Conversation
Contributor
ejohnstown
commented
May 2, 2025
- Snip out some extraneous states from the server handshake tracking for accept.
- Change sending the keyboard interactive info response to a reaction to a request.
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the keyboard interactive authentication handling in the server by removing extraneous state entries and modifying how the keyboard interactive response is sent.
- Remove unnecessary keyboard interactive states from the state machine.
- Eliminate the repeated while-loop sending keyboard interactive responses in favor of a direct response call.
- Update internal state transitions to streamline the authentication process.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| wolfssh/internal.h | Removed extraneous keyboard interactive state definitions. |
| src/ssh.c | Deleted the while-loop that was managing keyboard interactive responses. |
| src/internal.c | Consolidated state transitions and modified DoUserAuthInfoRequest to call SendUserAuthKeyboardResponse directly. |
Comments suppressed due to low confidence (4)
wolfssh/internal.h:1154
- Removing the extraneous keyboard interactive states simplifies the state machine. Please verify that no other part of the codebase depends on these removed states.
- SERVER_USERAUTH_ACCEPT_KEYBOARD,
src/ssh.c:890
- The removal of the loop that repeatedly sends the keyboard interactive response streamlines the connection logic. Ensure that the new triggering mechanism for sending the response adequately covers all required scenarios.
while (ssh->serverState == SERVER_USERAUTH_ACCEPT_KEYBOARD) { ... }
src/internal.c:7898
- The refactoring to directly assign SERVER_USERAUTH_ACCEPT_DONE simplifies state progression. Please confirm that all necessary cleanup associated with a keyboard interactive session is still handled correctly.
- if (ssh->serverState == SERVER_USERAUTH_ACCEPT_KEYBOARD)
- ssh->serverState = SERVER_USERAUTH_ACCEPT_KEYBOARD_DONE;
- else
src/internal.c:8027
- Switching to an immediate call to SendUserAuthKeyboardResponse in DoUserAuthInfoRequest clarifies response handling. Make sure that the error handling and subsequent state transitions align with the overall authentication flow.
if (ret == WS_SUCCESS) { ret = SendUserAuthKeyboardResponse(ssh); }
1. Snip out some extraneous states from the server handshake tracking for accept. 2. Change sending the keyboard interactive info response to a reaction to a request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.