ssh: simplify `radicle-ssh` crate implementation

Remove the internal buffer and simplify code around that.

Signed-off-by: Alexis Sellier <alexis@radicle.xyz>
This commit is contained in:
Alexis Sellier 2022-10-31 14:36:14 +01:00
parent 16b22d9178
commit f5212405c5
No known key found for this signature in database
3 changed files with 133 additions and 139 deletions

View File

@ -289,8 +289,7 @@ impl ExtendedSignature {
return Err(ExtendedSignatureError::MagicPreamble(preamble)); return Err(ExtendedSignatureError::MagicPreamble(preamble));
} }
let sig = ExtendedSignature::read(&mut reader)?; ExtendedSignature::read(&mut reader)
Ok(sig)
} }
pub fn to_armored(&self) -> Vec<u8> { pub fn to_armored(&self) -> Vec<u8> {
@ -316,7 +315,6 @@ mod test {
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use quickcheck_macros::quickcheck; use quickcheck_macros::quickcheck;
use zeroize::Zeroizing;
use super::{fmt, ExtendedSignature, SecretKey}; use super::{fmt, ExtendedSignature, SecretKey};
use crate as crypto; use crate as crypto;
@ -327,21 +325,21 @@ mod test {
#[derive(Clone, Default)] #[derive(Clone, Default)]
struct DummyStream { struct DummyStream {
incoming: Arc<Mutex<Zeroizing<Vec<u8>>>>, incoming: Arc<Mutex<Vec<u8>>>,
} }
impl ClientStream for DummyStream { impl ClientStream for DummyStream {
fn connect_socket<P>(_path: P) -> Result<AgentClient<Self>, Error> fn connect<P>(_path: P) -> Result<AgentClient<Self>, Error>
where where
P: AsRef<std::path::Path> + Send, P: AsRef<std::path::Path> + Send,
{ {
panic!("This function should never be called!") panic!("This function should never be called!")
} }
fn read_response(&mut self, buf: &mut Zeroizing<Vec<u8>>) -> Result<(), Error> { fn request(&mut self, buf: &[u8]) -> Result<Buffer, Error> {
*self.incoming.lock().unwrap() = buf.clone(); *self.incoming.lock().unwrap() = buf.to_vec();
Ok(()) Ok(Buffer::default())
} }
} }
@ -405,9 +403,9 @@ mod test {
let stream = DummyStream::default(); let stream = DummyStream::default();
let mut agent = AgentClient::connect(stream.clone()); let mut agent = AgentClient::connect(stream.clone());
let data: Zeroizing<Vec<u8>> = vec![1, 2, 3, 4, 5, 6, 7, 8, 9].into(); let data: Vec<u8> = vec![1, 2, 3, 4, 5, 6, 7, 8, 9];
agent.sign_request(&pk, data).ok(); agent.sign(&pk, &data).ok();
assert_eq!( assert_eq!(
stream.incoming.lock().unwrap().as_slice(), stream.incoming.lock().unwrap().as_slice(),

View File

@ -43,26 +43,22 @@ pub enum Error {
/// SSH agent client. /// SSH agent client.
pub struct AgentClient<S> { pub struct AgentClient<S> {
stream: S, stream: S,
buf: Buffer,
} }
// https://tools.ietf.org/html/draft-miller-ssh-agent-00#section-4.1 // https://tools.ietf.org/html/draft-miller-ssh-agent-00#section-4.1
impl<S> AgentClient<S> { impl<S> AgentClient<S> {
/// Connect to an SSH agent via the provided stream (on Unix, usually a Unix-domain socket). /// Connect to an SSH agent via the provided stream (on Unix, usually a Unix-domain socket).
pub fn connect(stream: S) -> Self { pub fn connect(stream: S) -> Self {
AgentClient { AgentClient { stream }
stream,
buf: Vec::new().into(),
}
} }
} }
pub trait ClientStream: Sized + Send + Sync { pub trait ClientStream: Sized + Send + Sync {
/// How to read the response from the stream /// Send an agent request to through the stream and read the response.
fn read_response(&mut self, buf: &mut Buffer) -> Result<(), Error>; fn request(&mut self, req: &[u8]) -> Result<Buffer, Error>;
/// How to connect the streaming socket /// How to connect the streaming socket
fn connect_socket<P>(path: P) -> Result<AgentClient<Self>, Error> fn connect<P>(path: P) -> Result<AgentClient<Self>, Error>
where where
P: AsRef<Path> + Send; P: AsRef<Path> + Send;
@ -72,11 +68,11 @@ pub trait ClientStream: Sized + Send + Sync {
} else { } else {
return Err(Error::EnvVar("SSH_AUTH_SOCK")); return Err(Error::EnvVar("SSH_AUTH_SOCK"));
}; };
match Self::connect_socket(var) { match Self::connect(var) {
Err(Error::Io(io_err)) if io_err.kind() == std::io::ErrorKind::NotFound => { Err(Error::Io(io_err)) if io_err.kind() == std::io::ErrorKind::NotFound => {
Err(Error::BadAuthSock) Err(Error::BadAuthSock)
} }
owise => owise, err => err,
} }
} }
} }
@ -89,37 +85,38 @@ impl<S: ClientStream> AgentClient<S> {
K: Encodable, K: Encodable,
K::Error: std::error::Error + Send + Sync + 'static, K::Error: std::error::Error + Send + Sync + 'static,
{ {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0);
buf.resize(4, 0);
if constraints.is_empty() { if constraints.is_empty() {
self.buf.push(msg::ADD_IDENTITY) buf.push(msg::ADD_IDENTITY)
} else { } else {
self.buf.push(msg::ADD_ID_CONSTRAINED) buf.push(msg::ADD_ID_CONSTRAINED)
} }
key.write(&mut self.buf); key.write(&mut buf);
if !constraints.is_empty() { if !constraints.is_empty() {
for cons in constraints { for cons in constraints {
match *cons { match *cons {
Constraint::KeyLifetime { seconds } => { Constraint::KeyLifetime { seconds } => {
self.buf.push(msg::CONSTRAIN_LIFETIME); buf.push(msg::CONSTRAIN_LIFETIME);
self.buf.deref_mut().write_u32::<BigEndian>(seconds)? buf.deref_mut().write_u32::<BigEndian>(seconds)?
} }
Constraint::Confirm => self.buf.push(msg::CONSTRAIN_CONFIRM), Constraint::Confirm => buf.push(msg::CONSTRAIN_CONFIRM),
Constraint::Extensions { Constraint::Extensions {
ref name, ref name,
ref details, ref details,
} => { } => {
self.buf.push(msg::CONSTRAIN_EXTENSION); buf.push(msg::CONSTRAIN_EXTENSION);
self.buf.extend_ssh_string(name); buf.extend_ssh_string(name);
self.buf.extend_ssh_string(details); buf.extend_ssh_string(details);
} }
} }
} }
} }
self.buf.write_len(); buf.write_len();
self.stream.read_response(&mut self.buf)?; self.stream.request(&buf)?;
Ok(()) Ok(())
} }
@ -132,67 +129,68 @@ impl<S: ClientStream> AgentClient<S> {
pin: &[u8], pin: &[u8],
constraints: &[Constraint], constraints: &[Constraint],
) -> Result<(), Error> { ) -> Result<(), Error> {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0);
buf.resize(4, 0);
if constraints.is_empty() { if constraints.is_empty() {
self.buf.push(msg::ADD_SMARTCARD_KEY) buf.push(msg::ADD_SMARTCARD_KEY)
} else { } else {
self.buf.push(msg::ADD_SMARTCARD_KEY_CONSTRAINED) buf.push(msg::ADD_SMARTCARD_KEY_CONSTRAINED)
} }
self.buf.extend_ssh_string(id.as_bytes()); buf.extend_ssh_string(id.as_bytes());
self.buf.extend_ssh_string(pin); buf.extend_ssh_string(pin);
if !constraints.is_empty() { if !constraints.is_empty() {
self.buf buf.deref_mut()
.deref_mut()
.write_u32::<BigEndian>(constraints.len() as u32)?; .write_u32::<BigEndian>(constraints.len() as u32)?;
for cons in constraints { for cons in constraints {
match *cons { match *cons {
Constraint::KeyLifetime { seconds } => { Constraint::KeyLifetime { seconds } => {
self.buf.push(msg::CONSTRAIN_LIFETIME); buf.push(msg::CONSTRAIN_LIFETIME);
self.buf.deref_mut().write_u32::<BigEndian>(seconds)?; buf.deref_mut().write_u32::<BigEndian>(seconds)?;
} }
Constraint::Confirm => self.buf.push(msg::CONSTRAIN_CONFIRM), Constraint::Confirm => buf.push(msg::CONSTRAIN_CONFIRM),
Constraint::Extensions { Constraint::Extensions {
ref name, ref name,
ref details, ref details,
} => { } => {
self.buf.push(msg::CONSTRAIN_EXTENSION); buf.push(msg::CONSTRAIN_EXTENSION);
self.buf.extend_ssh_string(name); buf.extend_ssh_string(name);
self.buf.extend_ssh_string(details); buf.extend_ssh_string(details);
} }
} }
} }
} }
self.buf.write_len(); buf.write_len();
self.stream.read_response(&mut self.buf)?; self.stream.request(&buf)?;
Ok(()) Ok(())
} }
/// Lock the agent, making it refuse to sign until unlocked. /// Lock the agent, making it refuse to sign until unlocked.
pub fn lock(&mut self, passphrase: &[u8]) -> Result<(), Error> { pub fn lock(&mut self, passphrase: &[u8]) -> Result<(), Error> {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0);
self.buf.push(msg::LOCK);
self.buf.extend_ssh_string(passphrase);
self.buf.write_len();
self.stream.read_response(&mut self.buf)?; buf.resize(4, 0);
buf.push(msg::LOCK);
buf.extend_ssh_string(passphrase);
buf.write_len();
self.stream.request(&buf)?;
Ok(()) Ok(())
} }
/// Unlock the agent, allowing it to sign again. /// Unlock the agent, allowing it to sign again.
pub fn unlock(&mut self, passphrase: &[u8]) -> Result<(), Error> { pub fn unlock(&mut self, passphrase: &[u8]) -> Result<(), Error> {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0); buf.resize(4, 0);
self.buf.push(msg::UNLOCK); buf.push(msg::UNLOCK);
self.buf.extend_ssh_string(passphrase); buf.extend_ssh_string(passphrase);
self.buf.write_len(); buf.write_len();
self.stream.read_response(&mut self.buf)?; self.stream.request(&buf)?;
Ok(()) Ok(())
} }
@ -204,16 +202,16 @@ impl<S: ClientStream> AgentClient<S> {
K: Encodable, K: Encodable,
K::Error: std::error::Error + Send + Sync + 'static, K::Error: std::error::Error + Send + Sync + 'static,
{ {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0); buf.resize(4, 0);
self.buf.push(msg::REQUEST_IDENTITIES); buf.push(msg::REQUEST_IDENTITIES);
self.buf.write_len(); buf.write_len();
self.stream.read_response(&mut self.buf)?;
let mut keys = Vec::new(); let mut keys = Vec::new();
if self.buf[0] == msg::IDENTITIES_ANSWER { let resp = self.stream.request(&buf)?;
let mut r = self.buf.reader(1);
if resp[0] == msg::IDENTITIES_ANSWER {
let mut r = resp.reader(1);
let n = r.read_u32()?; let n = r.read_u32()?;
for _ in 0..n { for _ in 0..n {
@ -231,26 +229,23 @@ impl<S: ClientStream> AgentClient<S> {
} }
/// Ask the agent to sign the supplied piece of data. /// Ask the agent to sign the supplied piece of data.
pub fn sign_request<K>(&mut self, public: &K, data: Buffer) -> Result<Signature, Error> pub fn sign<K>(&mut self, public: &K, data: &[u8]) -> Result<Signature, Error>
where where
K: Encodable + fmt::Debug, K: Encodable + fmt::Debug,
{ {
self.prepare_sign_request(public, &data); let req = self.prepare_sign_request(public, data);
self.stream.read_response(&mut self.buf)?; let resp = self.stream.request(&req)?;
if !self.buf.is_empty() && self.buf[0] == msg::SIGN_RESPONSE { if !resp.is_empty() && resp[0] == msg::SIGN_RESPONSE {
let mut signature: Signature = [0; 64]; self.read_signature(&resp)
self.write_signature(&mut signature)?; } else if !resp.is_empty() && resp[0] == msg::FAILURE {
Ok(signature)
} else if self.buf[0] == msg::FAILURE {
Err(Error::AgentFailure) Err(Error::AgentFailure)
} else { } else {
Err(Error::AgentProtocolError) Err(Error::AgentProtocolError)
} }
} }
fn prepare_sign_request<K>(&mut self, public: &K, data: &[u8]) fn prepare_sign_request<K>(&self, public: &K, data: &[u8]) -> Buffer
where where
K: Encodable + fmt::Debug, K: Encodable + fmt::Debug,
{ {
@ -264,27 +259,28 @@ impl<S: ClientStream> AgentClient<S> {
let total = 1 + pk.len() + 4 + data.len() + 4; let total = 1 + pk.len() + 4 + data.len() + 4;
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf buf.write_u32::<BigEndian>(total as u32)
.write_u32::<BigEndian>(total as u32)
.expect("Writing to a vector never fails"); .expect("Writing to a vector never fails");
self.buf.push(msg::SIGN_REQUEST); buf.push(msg::SIGN_REQUEST);
self.buf.extend_from_slice(&pk); buf.extend_from_slice(&pk);
self.buf.extend_ssh_string(data); buf.extend_ssh_string(data);
// Signature flags should be zero for ed25519. // Signature flags should be zero for ed25519.
self.buf.write_u32::<BigEndian>(0).unwrap(); buf.write_u32::<BigEndian>(0).unwrap();
buf
} }
fn write_signature(&self, data: &mut [u8]) -> Result<(), Error> { fn read_signature(&self, sig: &[u8]) -> Result<Signature, Error> {
let mut r = self.buf.reader(1); let mut r = sig.reader(1);
let mut resp = r.read_string()?.reader(0); let mut resp = r.read_string()?.reader(0);
let _t = resp.read_string()?; let _t = resp.read_string()?;
let sig = resp.read_string()?; let sig = resp.read_string()?;
data.copy_from_slice(sig); let mut out = [0; 64];
out.copy_from_slice(sig);
Ok(()) Ok(out)
} }
/// Ask the agent to remove a key from its memory. /// Ask the agent to remove a key from its memory.
@ -297,70 +293,71 @@ impl<S: ClientStream> AgentClient<S> {
let total = 1 + pk.len(); let total = 1 + pk.len();
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.write_u32::<BigEndian>(total as u32)?; buf.write_u32::<BigEndian>(total as u32)?;
self.buf.push(msg::REMOVE_IDENTITY); buf.push(msg::REMOVE_IDENTITY);
self.buf.extend_from_slice(&pk); buf.extend_from_slice(&pk);
self.stream.read_response(&mut self.buf)?; self.stream.request(&buf)?;
Ok(()) Ok(())
} }
/// Ask the agent to remove a smartcard from its memory. /// Ask the agent to remove a smartcard from its memory.
pub fn remove_smartcard_key(&mut self, id: &str, pin: &[u8]) -> Result<(), Error> { pub fn remove_smartcard_key(&mut self, id: &str, pin: &[u8]) -> Result<(), Error> {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0); buf.resize(4, 0);
self.buf.push(msg::REMOVE_SMARTCARD_KEY); buf.push(msg::REMOVE_SMARTCARD_KEY);
self.buf.extend_ssh_string(id.as_bytes()); buf.extend_ssh_string(id.as_bytes());
self.buf.extend_ssh_string(pin); buf.extend_ssh_string(pin);
self.buf.write_len(); buf.write_len();
self.stream.read_response(&mut self.buf)?; self.stream.request(&buf)?;
Ok(()) Ok(())
} }
/// Ask the agent to forget all known keys. /// Ask the agent to forget all known keys.
pub fn remove_all_identities(&mut self) -> Result<(), Error> { pub fn remove_all_identities(&mut self) -> Result<(), Error> {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0); buf.resize(4, 0);
self.buf.push(msg::REMOVE_ALL_IDENTITIES); buf.push(msg::REMOVE_ALL_IDENTITIES);
self.buf.write_len(); buf.write_len();
self.stream.read_response(&mut self.buf)?; self.stream.request(&buf)?;
Ok(()) Ok(())
} }
/// Send a custom message to the agent. /// Send a custom message to the agent.
pub fn extension(&mut self, typ: &[u8], ext: &[u8]) -> Result<(), Error> { pub fn extension(&mut self, typ: &[u8], ext: &[u8]) -> Result<(), Error> {
self.buf.zeroize(); let mut buf = Buffer::default();
self.buf.resize(4, 0);
self.buf.push(msg::EXTENSION);
self.buf.extend_ssh_string(typ);
self.buf.extend_ssh_string(ext);
self.buf.write_len();
self.stream.read_response(&mut self.buf)?; buf.resize(4, 0);
buf.push(msg::EXTENSION);
buf.extend_ssh_string(typ);
buf.extend_ssh_string(ext);
buf.write_len();
self.stream.request(&buf)?;
Ok(()) Ok(())
} }
/// Ask the agent what extensions about supported extensions. /// Ask the agent about supported extensions.
pub fn query_extension(&mut self, typ: &[u8], mut ext: Buffer) -> Result<bool, Error> { pub fn query_extension(&mut self, typ: &[u8], mut ext: Buffer) -> Result<bool, Error> {
self.buf.zeroize(); let mut req = Buffer::default();
self.buf.resize(4, 0);
self.buf.push(msg::EXTENSION);
self.buf.extend_ssh_string(typ);
self.buf.write_len();
self.stream.read_response(&mut self.buf)?; req.resize(4, 0);
req.push(msg::EXTENSION);
req.extend_ssh_string(typ);
req.write_len();
let mut r = self.buf.reader(1); let resp = self.stream.request(&req)?;
let mut r = resp.reader(1);
ext.extend(r.read_string()?); ext.extend(r.read_string()?);
Ok(!self.buf.is_empty() && self.buf[0] == msg::SUCCESS) Ok(!resp.is_empty() && resp[0] == msg::SUCCESS)
} }
} }
@ -384,33 +381,32 @@ impl ClientStream for TcpStream {
#[cfg(unix)] #[cfg(unix)]
impl ClientStream for UnixStream { impl ClientStream for UnixStream {
fn connect_socket<P>(path: P) -> Result<AgentClient<Self>, Error> fn connect<P>(path: P) -> Result<AgentClient<Self>, Error>
where where
P: AsRef<Path> + Send, P: AsRef<Path> + Send,
{ {
let stream = UnixStream::connect(path)?; let stream = UnixStream::connect(path)?;
Ok(AgentClient {
stream, Ok(AgentClient { stream })
buf: Vec::new().into(),
})
} }
fn read_response(&mut self, buf: &mut Buffer) -> Result<(), Error> { fn request(&mut self, msg: &[u8]) -> Result<Buffer, Error> {
let mut resp = Buffer::default();
// Write the message // Write the message
self.write_all(buf)?; self.write_all(msg)?;
self.flush()?; self.flush()?;
// Read the length // Read the length
buf.zeroize(); resp.resize(4, 0);
buf.resize(4, 0); self.read_exact(&mut resp)?;
self.read_exact(buf)?;
// Read the rest of the buffer // Read the rest of the buffer
let len = BigEndian::read_u32(buf) as usize; let len = BigEndian::read_u32(&resp) as usize;
buf.zeroize(); resp.zeroize();
buf.resize(len, 0); resp.resize(len, 0);
self.read_exact(buf)?; self.read_exact(&mut resp)?;
Ok(()) Ok(resp)
} }
} }

View File

@ -25,7 +25,7 @@ fn main() -> anyhow::Result<()> {
io::stdin().read_to_end(&mut stdin)?; io::stdin().read_to_end(&mut stdin)?;
let mut agent = ssh::agent::connect()?; let mut agent = ssh::agent::connect()?;
let sig = agent.sign_request(profile.id(), stdin.into())?; let sig = agent.sign(profile.id(), &stdin)?;
let sig = crypto::Signature::from(sig); let sig = crypto::Signature::from(sig);
println!("{}", &sig); println!("{}", &sig);