Skip to content

Sign in through keyboard-interactive, explain rejections, and dial each address with its own timeout - #177

Open
Sadykhzadeh wants to merge 6 commits into
mainfrom
fix/ssh-connect-compat
Open

Sadykhzadeh wants to merge 6 commits into
mainfrom
fix/ssh-connect-compat

Conversation

@Sadykhzadeh

@Sadykhzadeh Sadykhzadeh commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

Closes #176. All changes are in core/src/connection.rs and its tests; session.rs is untouched, so this doesn't overlap the #140 stack.

Sign-in

  • Which methods the server accepts: a Password host first sends a none request, as OpenSSH does, to learn the server's methods.

    • If password is offered, it is used.
    • Otherwise keyboard-interactive is used.
  • Keyboard-interactive is answered the way OpenSSH answers a PAM password prompt:

    • The password goes only to a round with a single prompt that isn't echoed, and only once.
    • Rounds with no prompts get an empty reply.
    • A round asking for more (a one-time code, several fields, a visible prompt) is refused without sending the password. The error names the prompts: the server asks for more than the password ("Password:", "Verification code:"); Clavyn cannot answer additional sign-in prompts such as one-time codes yet.
    • At most 5 rounds, all within the existing auth timeout.
  • A rejected password is not retried through keyboard-interactive. Both methods check the same credential, so a second attempt can't succeed. It would only count twice against MaxAuthTries and fail2ban.

  • Why a sign-in failed: the message keeps the authentication rejected by server: prefix, then says one of:

    • the server rejected the password (it accepts: password, publickey)
    • the server does not accept password sign-in (it accepts: publickey)
    • the server accepted this step but requires another: publickey; Clavyn cannot complete multi-step sign-in yet

    The frontend only displays this text and never matches on it.

Connecting

  • Dialing: the name is resolved with lookup_host, and each address gets its own 5 s in resolver order. The overall 20 s cap still covers resolve, dial and the handshake. The stream then goes to russh::client::connect_stream, so host key checking is unchanged.
  • A failed dial lists every address and its reason, e.g. [2001:db8::1]:22: timed out after 5s; 192.0.2.7:22: Connection refused.
  • TCP_NODELAY is set on the dialed socket. russh ignores config.nodelay for a stream it didn't open itself.

Tests

Each fails against main (or, for the resend test, against the earlier commit on this branch):

  • Loopback sign-in tests (ssh_roundtrip.rs):
    • a_password_is_answered_through_keyboard_interactive
    • a_second_factor_prompt_is_refused_without_sending_the_password
    • a_rejected_password_is_not_sent_a_second_time
    • a_rejected_password_names_the_methods_the_server_accepts
    • a_server_without_password_sign_in_says_so
    • a_partial_success_is_reported_as_a_further_step
  • Dialer tests (connection.rs):
    • a_silent_address_does_not_hold_up_the_next_one: a black-hole address, then a loopback listener; must finish in under 10 s.
    • dialed_streams_disable_nagle
    • a_failed_dial_names_every_address

Trade-offs and limits

  • The none probe costs one round trip per password sign-in.
  • One-time codes and other MFA prompts still can't be answered; the user gets a clear message instead of a generic failure. Answering them needs prompts in the UI, and the auth timeout would have to stop covering the time the user spends typing.
  • Tested only against russh's in-process server, not a real OpenSSH or FreeBSD sshd.

Test plan

  • cargo test -p clavyn-core: 121 + 5 + 32 + 15 passed
  • cargo clippy -p clavyn-core --all-targets: nothing new
  • node scripts/verify/check-comments.mjs: ok
  • No UI change.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

russh's connect tries the resolved addresses one after another with no
per-address bound, so a host whose first address is a black-holed IPv6
route spent the whole 20 s connect timeout there and never reached its
working IPv4 address. Resolve the name ourselves, give each address 5 s,
keep the overall cap, and hand the stream to connect_stream. A failed
dial now says which addresses were tried and why each failed.

The dialed socket also gets TCP_NODELAY, as OpenSSH sets for interactive
sessions; with Nagle on, keystrokes waited on the server's delayed ACK and
typing lagged noticeably on high-latency links.
Password hosts only ever tried the password method, so servers that
disable it and take the password through keyboard-interactive (FreeBSD's
default sshd, many PAM setups) refused them outright. Probe with "none"
first like OpenSSH, then use password and/or keyboard-interactive as the
server offers them. The password is only given to a single hidden prompt,
once; prompts for anything more, such as a one-time code, are refused with
an explanation instead of being sent the password.

A rejection used to read "authentication rejected by server" and nothing
else. It now names the methods the server still accepts, says when the
server does not offer password or publickey sign-in at all, and reports a
partial success as a further step Clavyn cannot complete yet.
A server that offers password and rejects it would check the same
credential through keyboard-interactive, so the retry could not succeed
and only counted twice against MaxAuthTries and fail2ban.
@greptile-apps

greptile-apps Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

TREX TREX could not finish testing

  • Plan what to test
  • Set up the environment and test the changed flows
  • Post test results

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Support keyboard-interactive SSH sign-in and bounded address dialing

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Support password sign-in through keyboard-interactive without resending rejected credentials or
 answering additional-factor prompts.
• Explain authentication rejections using the methods and next steps reported by the server.
• Bound each address dial, report individual failures, and disable Nagle for interactive
 connections.
Diagram

graph TD
  Resolve["Resolve host"] --> Dial["Bounded TCP dial"] --> SSH["SSH handshake"] --> Choice{"Host auth type"} --> Password["Probe password methods"] --> Outcome["Session or error"]
  Password -->|interactive only| Interactive["Answer safe prompts"] --> Outcome
  Choice -->|key host| PublicKey["Public key auth"] --> Outcome
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Race addresses with Happy Eyeballs
  • ➕ Can reach a working IPv4 address sooner when IPv6 connections stall.
  • ➖ Adds concurrent connection management and departs from strict resolver-order dialing.

Recommendation: Keep the PR's sequential, per-address timeout approach: it fixes starvation within the overall connection cap while preserving predictable attempt order. Consider Happy Eyeballs separately if the five-second delay before a fallback address proves problematic.

Files changed (2) +474 / -16

Bug fix (1) +258 / -13
connection.rsBound address dialing and broaden password authentication +258/-13

Bound address dialing and broaden password authentication

• Resolves and dials addresses with individual five-second bounds, enables TCP_NODELAY, and passes the stream to russh under the existing overall timeout. Password hosts discover supported methods before authenticating, safely handle keyboard-interactive prompts, and report server-provided rejection details. Unit tests cover address fallback, socket settings, and dial failures.

core/src/connection.rs

Tests (1) +216 / -3
ssh_roundtrip.rsExercise SSH sign-in decisions against loopback servers +216/-3

Exercise SSH sign-in decisions against loopback servers

• Generalizes the in-process server helper and adds sign-in modes for keyboard-interactive passwords, additional prompts, rejected credentials, unavailable methods, and partial success. Roundtrip tests verify both successful sign-in and that unsafe or duplicate password responses are not sent.

core/tests/ssh_roundtrip.rs

@qodo-code-review

qodo-code-review Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Code Review by Qodo


View medium (1)
Remediation recommended
1. Password copies remain in memory after sign-in 🐞 ⭐
Description
keyboard_interactive_password creates an ordinary owned String with password.to_owned() and
passes it to russh without a way to wipe that copy. Every keyboard-interactive password response
therefore leaves an additional secret allocation to be freed without zeroization, unlike the
deliberately wiped private-key buffer used in the same connection path.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Keyboard-interactive sign-in creates an owned password response that is dropped without zeroization.
## Fix Focus Areas
- core/src/connection.rs[394-415]
## Recommended Fix
Ensure the owned response is wiped after russh serializes it. Because the response API takes ownership of `Vec<String>`, use or add an API that zeroizes the consumed response rather than wiping only a separate local copy.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


Comment thread core/src/connection.rs Resolved
Comment thread core/src/connection.rs Resolved
The loop answered five rounds but never looked at the reply to the fifth,
so a server that accepted there was reported as asking for too many
rounds.
Comment thread core/src/connection.rs Resolved
Comment thread core/src/connection.rs Resolved
… a credential nobody sent

Only an address with a fallback after it needs the per-address cap; the
last or only one was cut off at 5 s where it used to get the full 20.
A server that named no method at all was reported as having rejected
the password, even when the password was never sent.
Comment thread core/src/connection.rs Resolved
They rely on an address that swallows the handshake; a network that
refuses it quickly would make them pass or fail for the wrong reason.
Comment thread core/src/connection.rs Resolved
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.

SSH sign-in: no keyboard-interactive, unexplained rejections, Nagle on, and a dead IPv6 address blocks IPv4

1 participant