diff --git a/src/agent.rs b/src/agent.rs index ef4160a..3605577 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -288,6 +288,7 @@ pub trait Session: 'static + Sync + Send + Unpin { None => Ok(Response::Success), } } + Request::Unknown(_) => return Ok(Response::Failure), } Ok(Response::Success) } diff --git a/src/proto/message/request.rs b/src/proto/message/request.rs index 16511b3..7fbfbe3 100644 --- a/src/proto/message/request.rs +++ b/src/proto/message/request.rs @@ -57,6 +57,18 @@ pub enum Request { /// Send a vendor-specific message via the agent protocol, /// identified by an *extension type*. Extension(Extension), + + /// A request message of an unknown type. + /// + /// The carried value is the raw protocol message identifier (type byte) + /// that could not be parsed. Because the message body format of unknown + /// types is undefined, the payload following the type byte is skipped. + /// + /// Agents should reply with [`Response::Failure`](crate::proto::Response::Failure) (per + /// [draft-miller-ssh-agent-14 § 4.1](https://www.ietf.org/archive/id/draft-miller-ssh-agent-14.html#section-4.1)) + /// and keep the connection open, matching the behaviour of OpenSSH's + /// `ssh-agent`. + Unknown(u8), } impl Request { @@ -77,6 +89,7 @@ impl Request { Self::AddIdConstrained(_) => 25, Self::AddSmartcardKeyConstrained(_) => 26, Self::Extension(_) => 27, + Self::Unknown(command) => *command, } } } @@ -100,7 +113,7 @@ impl Decode for Request { 25 => AddIdentityConstrained::decode(reader).map(Self::AddIdConstrained), 26 => AddSmartcardKeyConstrained::decode(reader).map(Self::AddSmartcardKeyConstrained), 27 => Extension::decode(reader).map(Self::Extension), - command => Err(Error::UnsupportedCommand { command }), + command => Ok(Self::Unknown(command)), } } } @@ -121,6 +134,7 @@ impl Encode for Request { Self::AddIdConstrained(key) => key.encoded_len()?, Self::AddSmartcardKeyConstrained(key) => key.encoded_len()?, Self::Extension(extension) => extension.encoded_len()?, + Self::Unknown(_) => 0, }; [message_id_len, payload_len].checked_sum() @@ -143,6 +157,7 @@ impl Encode for Request { Self::AddIdConstrained(identity) => identity.encode(writer)?, Self::AddSmartcardKeyConstrained(key) => key.encode(writer)?, Self::Extension(extension) => extension.encode(writer)?, + Self::Unknown(_) => {} }; Ok(()) diff --git a/tests/unknown_request.rs b/tests/unknown_request.rs new file mode 100644 index 0000000..77f8042 --- /dev/null +++ b/tests/unknown_request.rs @@ -0,0 +1,112 @@ +//! Integration test: the agent must reply `SSH_AGENT_FAILURE` to requests of an +//! unknown type and keep the connection open, instead of dropping it. +//! +//! This mirrors the behaviour of OpenSSH's `ssh-agent` and is required by +//! [draft-miller-ssh-agent-14 § 4.1](https://www.ietf.org/archive/id/draft-miller-ssh-agent-14.html#section-4.1): +//! +//! > SSH_AGENT_FAILURE messages are also sent in reply to requests with unknown types. +//! +//! Some clients (for example Ruby's `net-ssh`) probe the agent with a legacy +//! `SSH2_AGENT_REQUEST_VERSION` (type 1) message that most agents do not +//! implement. Such probes must not kill the connection. + +use std::os::unix::net::UnixStream as StdUnixStream; +use std::time::Duration; + +use ssh_agent_lib::agent::{listen, Session}; +use ssh_agent_lib::error::AgentError; +use ssh_agent_lib::proto::{Identity, Request, Response}; +use ssh_encoding::{Decode, Encode}; + +#[derive(Clone, Default)] +struct DummyAgent; + +#[ssh_agent_lib::async_trait] +impl Session for DummyAgent { + async fn request_identities(&mut self) -> Result, AgentError> { + Ok(Vec::new()) + } +} + +fn spawn_agent(socket_path: &std::path::Path) -> std::thread::JoinHandle<()> { + let socket_path = socket_path.to_path_buf(); + std::thread::spawn(move || { + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async move { + let listener = tokio::net::UnixListener::bind(&socket_path).unwrap(); + listen(listener, DummyAgent).await.unwrap(); + }); + }) +} + +/// Wait until the agent is accepting connections on `socket_path`. +fn wait_for_socket(socket_path: &std::path::Path) { + for _ in 0..100 { + if StdUnixStream::connect(socket_path).is_ok() { + return; + } + std::thread::sleep(Duration::from_millis(10)); + } + panic!("timed out waiting for agent socket"); +} + +/// Write a raw agent request frame (length prefix + body) and read the raw +/// response frame, mimicking how low-level clients such as `net-ssh` talk to +/// the agent. Returns `None` if the connection was closed without a reply. +fn raw_roundtrip(stream: &mut StdUnixStream, body: &[u8]) -> Option> { + use std::io::{Read, Write}; + + let mut request = Vec::new(); + (body.len() as u32).encode(&mut request).unwrap(); + request.extend_from_slice(body); + stream.write_all(&request).unwrap(); + + let mut header = [0u8; 4]; + if stream.read_exact(&mut header).is_err() { + return None; + } + let len = u32::from_be_bytes(header) as usize; + let mut response = vec![0u8; len]; + stream.read_exact(&mut response).ok()?; + Some(response) +} + +#[test] +fn unknown_request_type_replies_failure_and_keeps_connection_open() { + let socket_path = + std::env::temp_dir().join(format!("ssh-agent-lib-unknown-{}.sock", std::process::id())); + let _ = std::fs::remove_file(&socket_path); + + let handle = spawn_agent(&socket_path); + wait_for_socket(&socket_path); + + let mut stream = StdUnixStream::connect(&socket_path).unwrap(); + + // `SSH2_AGENT_REQUEST_VERSION` (message type 1) with a "2.0" payload, + // as sent by `net-ssh` during agent negotiation. Message type 1 is not a + // supported SSH agent command. + let request_body = [1u8, 0, 0, 0, 3, b'2', b'.', b'0']; + let response = raw_roundtrip(&mut stream, &request_body) + .expect("agent must reply instead of closing the connection"); + + // The response must be an SSH_AGENT_FAILURE (message type 5) with no body. + let mut rest = &response[..]; + let decoded = Response::decode(&mut rest).unwrap(); + assert_eq!(decoded, Response::Failure); + assert_eq!(response[0], 5, "expected SSH_AGENT_FAILURE (5)"); + + // The connection must stay usable: issue a supported request afterwards. + let mut request = Vec::new(); + Request::RequestIdentities.encode(&mut request).unwrap(); + let response = raw_roundtrip(&mut stream, &request) + .expect("connection must remain open after an unknown request type"); + let mut rest = &response[..]; + let decoded = Response::decode(&mut rest).unwrap(); + assert!(matches!(decoded, Response::IdentitiesAnswer(_))); + + drop(stream); + // The agent keeps accepting connections, so detach instead of joining. + drop(handle); + + let _ = std::fs::remove_file(&socket_path); +}