Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ pub trait Session: 'static + Sync + Send + Unpin {
None => Ok(Response::Success),
}
}
Request::Unknown(_) => return Ok(Response::Failure),
}
Ok(Response::Success)
}
Expand Down
17 changes: 16 additions & 1 deletion src/proto/message/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -77,6 +89,7 @@ impl Request {
Self::AddIdConstrained(_) => 25,
Self::AddSmartcardKeyConstrained(_) => 26,
Self::Extension(_) => 27,
Self::Unknown(command) => *command,
}
}
}
Expand All @@ -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)),
}
}
}
Expand All @@ -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()
Expand All @@ -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(())
Expand Down
112 changes: 112 additions & 0 deletions tests/unknown_request.rs
Original file line number Diff line number Diff line change
@@ -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<Vec<Identity>, 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<Vec<u8>> {
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);
}