Repository navigation
Conversation
The SSH agent protocol was published as RFC 9987. Point the crate documentation, code comments and README at the RFC instead of draft-miller-ssh-agent-14 (section 3.x of the draft is section 5.x of the RFC). While here: - fix the section label of the session-bind extension link, which said 8.8.1 while the URL already pointed at 8.1.1; - cite RFC 9987 section 5.1 (not 4.1) in the unknown-request test and quote the RFC wording; - point the key constraint decoder comment at the constraint type registry (section 8.2) instead of draft-12. Refs wiktor-k#116 Signed-off-by: anyingiit <49945850+anyingiit@users.noreply.github.com>
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.
Requested by winshyon · project thread
Refs #116
This covers the first half of #116: the crate docs, comments and README now point at RFC 9987 instead of
draft-miller-ssh-agent-14. It also reports what I found when I checked the implementation against the RFC (below).Documentation changes
[draft-miller-ssh-agent-14 § 3.x](…)links now read[RFC9987 § 5.x](https://www.rfc-editor.org/rfc/rfc9987.html#section-5.x). I checked every anchor against the RFC text.request.rs: the session-bind link was labelled§ 8.8.1, but its URL already pointed at#section-8.1.1. The label now says 8.1.1.tests/unknown_request.rscited § 4.1 for the "unknown request type →SSH_AGENT_FAILURE" rule. The rule is in RFC 9987 § 5.1, and the quoted text now uses the RFC wording ("MUST also be sent").KeyConstraint::decodecomment pointed at draft-12 § 5.2. It now points at the constraint type registry in RFC 9987 § 8.2.There are no code changes.
Implementation check (not fixed here)
Unknown key constraint closes the connection. RFC 9987 § 5.2.7 says an agent that does not recognise a constraint "MUST abort parsing, refuse the request, and return an SSH_AGENT_FAILURE message". draft-14 already said the agent must refuse the request. Today
KeyConstraint::decodereturnsErr(KeyError::AlgorithmUnknown)for an unknown type byte. That makes the codec fail, soadapter.try_next().await?inhandle_socketreturns an error and the connection is dropped. NoSSH_AGENT_FAILUREis sent.I confirmed this with a throw-away test that is not part of this PR. It sent
tests/messages/req-add-identity-constrained-lifetime.binwith the constraint type byte changed to3through the sameagent::handle_socketsetup astests/unknown_request.rs. The client got no response, and the next request on the same stream got none either.Fixing it means changing how the decoder reports this error, or adding an "unknown constraint" variant, so the agent can reply with
Failure. I'd rather leave that design choice to you. I'm happy to send a follow-up PR once you say which way you prefer.Unknown requests and unsupported known requests already get
SSH_AGENT_FAILURE, because Return correct response for unknown messages, add handler for unknown messages toSession(extends #112) #114 andSessiondefault toUnsupportedCommand→Response::Failure. This matches § 5.1.So #116 should stay open for the implementation part.
Testing
cargo test --all: all tests pass.cargo clippy --workspace --no-deps --all-targets -- -D warnings: clean.codespell --config .codespellrcon the tree and on the commit message: clean.cargo fmt --all -- --check(stable): clean. The diff only touches comments and doc text.cargo doc --no-deps: no new warnings. The one existing warning is the redundant link target atsrc/agent.rs:269, which is unchanged.The commit is signed off (DCO).
This change was prepared with AI assistance (Claude Code). I reviewed the diff and checked every section number and anchor against the RFC text myself.
🤖 Generated with Claude Code