fix: unknown request types should return failure - #112
Open
cquintana92 wants to merge 1 commit into
Open
Conversation
cquintana92
force-pushed
the
fix/unknown-request-types-return-failure
branch
2 times, most recently
from
August 7, 2026 10:49
f66c5ad to
81948a9
Compare
Signed-off-by: Carlos Quintana <carlos@cquintana.dev>
cquintana92
force-pushed
the
fix/unknown-request-types-return-failure
branch
from
August 12, 2026 14:12
81948a9 to
5089da4
Compare
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.
Fix: Reply
SSH_AGENT_FAILUREto unknown request types instead of closing the connectionIssue
When a client sends a request whose message type is not recognised, the agent closes the connection without replying. Per draft-miller-ssh-agent-14 § 3.1:
OpenSSH's
ssh-agentbehaves that way, but agents built on this crate currently do not, because the connection is dropped at the codec layer before any session handling runs.Impact
Some clients probe the agent with a legacy request type that is not implemented by modern agents. For example Ruby's
net-sshopens negotiation withSSH2_AGENT_REQUEST_VERSION(message type1). When the agent closes the socket instead of replying,net-sshraises aFrozenErrorand deploy tools built on it cannot connect at all, even thoughssh/ssh-addwork fine against the same agent.Root cause
The framing
Codecdecodes every incoming frame into aRequestbefore theSessionis invoked. For an unknown message type,Request::decodereturnedErr(UnsupportedCommand). That decoder error is fatal to the stream:tokio_util'sFramedImplenters an errored state after a decode error and yieldsEOFon the next poll, so the socket is torn down beforeSession::handleis ever called. NoSessionoverride can prevent this with the current code.Proposed solution
Request::Unknown(u8)variant that captures the raw message type byte. Unknown message types now decode successfully (the payload following the type byte is skipped, bounded by themessage length prefix), encode back to the original type byte, and report the correctmessage_id.Session::handleimplementation, respond toRequest::UnknownwithResponse::Failure(i.e.SSH_AGENT_FAILURE, message type 5) and keep the connection open, matching OpenSSH's behaviour.The default implementation suits all existing agents; because
handleis the documented override point, users who need custom handling for a particular unknown type can still intercept it themselves.Testing
New integration test
tests/unknown_request.rsstarts a real agent over a Unix socket, replays the exactnet-sshprobe (SSH2_AGENT_REQUEST_VERSION, type 1, body"2.0"), and asserts that:SSH_AGENT_FAILURE(message type 5) instead of closing the connection.SSH_AGENTC_REQUEST_IDENTITIES).The test fails against the previous behaviour (connection closed without a reply) and passes with this change.
Validation
cargo testreports all suites pass (unit, roundtrip, doc, new integration test)cargo clippy --workspace --no-deps --all-targets -- -D warningscomes cleancargo fmt --checkcomes cleanCompatibility
This is a backward-compatible additive change:
Requestgains one new variant (the compiler will flag exhaustive matches, which is desirable since callers must now decide how to handle unknown messages). No existing behaviour changes.