SSH keyboard-interactive #6508
SSH keyboard-interactive #6508
Conversation
WalkthroughAdds keyboard-interactive SSH authentication alongside existing password auth by registering an ssh.KeyboardInteractiveChallenge callback that logs prompts and auto-answers username/password prompts using the supplied credentials while retaining direct password authentication. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant SSHC as SSH Client
participant Server as SSH Server
App->>SSHC: Connect(host, user, password)
Note over SSHC: Configure auth methods:\n- Password\n- Keyboard-Interactive (callback logs prompts & supplies answers)
SSHC->>Server: Start authentication
alt Server requests direct password
SSHC->>Server: Send password
else Server uses keyboard-interactive
Server->>SSHC: Challenge(questions=[...])
SSHC->>SSHC: Callback invoked for each question (logs prompt)
alt Prompts match username/password patterns
SSHC->>Server: Respond with username/password as provided
else
SSHC->>Server: Respond with empty/default per callback
end
end
Server-->>SSHC: Auth success/failure
SSHC-->>App: Connection result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/js/libs/ssh/ssh.go (1)
237-242: Consider returning an error for unsupported multi-question scenarios.The callback returns an empty answer slice when
len(questions) != 1, which may cause authentication to fail silently. Consider returning an explicit error to make the limitation clear.Apply this diff:
cb := func(user, instruction string, questions []string, echos []bool) (answers []string, err error) { if len(questions) == 1 { return []string{opts.Password}, nil } - return []string{}, nil + return nil, fmt.Errorf("keyboard-interactive with %d questions not supported", len(questions)) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/js/libs/ssh/ssh.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code using go fmt
Run static analysis with go vet
Files:
pkg/js/libs/ssh/ssh.go
🔇 Additional comments (1)
pkg/js/libs/ssh/ssh.go (1)
237-242: Verify that blindly answering with the password is acceptable.The callback responds with the password for any single-question prompt without inspecting the question text. While this is documented as "best-effort," it could pose a security risk if the server asks an unexpected question (e.g., "Are you sure you want to delete all data?").
Consider whether the implementation should:
- Validate that the question is password-related (e.g., contains "password" or "passphrase")
- Add logging to track what questions are being answered
- Document this behavior more explicitly in code comments
If this best-effort approach is intentional for broad compatibility, add a comment explaining the trade-off.
|
To answer Rabbit's question, the keyboard-interactive protocol sometimes asks no questions, in which case we return no answers and the authentication flow continues. It would be an error to break the authentication flow there, as the user may be prompted for a password in a later stage. I tested this on ubuntu:24.04 container using the |
|
@coderabbitai create an issue for this PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 54 @dogancanbakir: I've created issue #6514 to track this PR. The issue includes:
You can view it here: #6514 |
dogancanbakir
requested review from
dwisiswant0
and removed request for
dogancanbakir
There was a problem hiding this comment.
minor:
- It (
len(questions) == 1expr) blindly assumed it was a password prompt. It could send a password to non-password questions like "Enter username:" or "Accept terms? (y/n)". I mean, what about servers that ask username + password in separate questions? - Password fields usually do not display the input characters, which makes the
echosparam a reliable indicator of this behavior. - No analysis of question text content. Couldn't distinguish between different types of single questions. Which comes with some unresolved questions:
- What if servers use non-English prompts?
- What if they use unusual question formats?
- What about servers that send empty questions or instructions?
nit:
- When keyboard-interactive fails, users get a generic "auth failed" with no indication that it was due to complex auth requirements.
- No way to log when keyboard-interactive auth is attempted vs. when it fails due to complexity.
|
In general, this MR does not intend to implement full keyboard-interactive fuzzing. Adding such feature would require also designing API that would allow to specify questions/answers in templates. This MR does not intend to do that. This MR only adds support in cases when
Although keyboard-interactive support would be nice to have in Nuclei, I have no interest in implementing that. The common usecases for keyboard-interactive are requiring TOTP on login. So I think most of the points raised are out of scope. I think this MR is useful and enhances Nuclei in the current state without the nice to have features you described above. Nevertheless I still try to respond to them below:
These are theoretical issues. Username in SSH is provided before the authentication flow starts, asking for it during the flow doesn't make sense. Accepting terms? I have never seen anything like this ever.
That's probably true.
I don't check questions in this MR. I don't log the questions either.
That's true. The user is only notified if the authentication was successful and there is a server with a weak password. I don't think it is interesting / useful to know if the auth failed due to other reasons. I could log the questions though, if you think it would be useful
I don't understand the distinction here. |
SSH servers are not limited to OpenSSH. Some use Cisco, F5, etc, and with custom auth flows. Even if you've never encountered them prompts, they exist1 in some VPN gateways and/or enterprise access controls. My point is, Implementing a reliable and cautious approach is more suitable than relying on assumptions that could result in UB.
This is an API-level change, and it should NOT be treated/scoped/reasoned as something that only applies to a specific template or case. Footnotes
|
|
I see. So let's only handle out password if the question matches case-insensitive |
Yea that works. |
by: * implement regex-based prompt matching for password variants. * add support for filling username prompts in keyboard interactive challenges. * improve debug logging with structured output. this addresses issues with servers using non-standard prompt formats and provides better visibility into auth failures. Signed-off-by: Dwi Siswanto <[email protected]>
4 participants
Proposed changes
Adds best-effort support for SSH keyboard-interactive protocol. Although the protocol supports interactive questions, this MR adds support for a single question to which it responds with the password.
Checklist
Summary by CodeRabbit
New Features
Refactor