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
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion rustica-agent/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ serde = "1.0.97"
serde_derive = "1.0"
sha2 = "0.9.2"
# For Production
sshcerts = { version = "0.14.0" }
sshcerts = { version = "0.14.1" }
# For Development
# sshcerts = { path = "../../sshcerts", features = [
# "yubikey-support",
Expand Down
15 changes: 15 additions & 0 deletions rustica-agent/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use async_trait::async_trait;
use rustica::key::U2FAttestation;

use config::{Options, UpdatableConfiguration};
use sshagent::constraints::Constraint;

pub use config::Config;
use serde_derive::{Deserialize, Serialize};
Expand Down Expand Up @@ -301,6 +302,20 @@ impl SshAgentHandler for Handler {
Ok(Response::Success)
}

async fn add_identity_constrained(
&self,
private_key: PrivateKey,
constraints: Vec<Constraint>,
) -> Result<Response, AgentError> {
trace!("Add Identity Constrained call");
if !constraints.is_empty() {
trace!("Key is being added with constraints");
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_identity_constrained currently accepts and stores the key even when constraints is non-empty, effectively ignoring lifetime/confirm/extension constraints. This can widen key usage vs what the client requested (e.g., confirm-required or limited lifetime), which is a security footgun. Either enforce the supported constraints here (or store them alongside the key for later enforcement), or reject non-empty constraints until enforcement is implemented (return Response::Failure).

Suggested change
trace!("Key is being added with constraints");
trace!("Key is being added with constraints, which are not currently supported");
// Until constraints are properly enforced or stored for later enforcement,
// reject attempts to add constrained identities to avoid silently widening key usage.
return Ok(Response::Failure);

Copilot uses AI. Check for mistakes.
}
let public_key = private_key.pubkey.encode();
self.identities.lock().await.insert(public_key, private_key);
Ok(Response::Success)
}

async fn identities(&self) -> Result<Response, AgentError> {
trace!("Identities call");
// We start building identies with the manually loaded keys
Expand Down
42 changes: 42 additions & 0 deletions rustica-agent/src/sshagent/constraints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use sshcerts::ssh::Reader;

use crate::sshagent::error::ParsingError;

#[derive(Debug)]
pub enum Constraint {
Lifetime(u32),
Confirm,
Extension(String, Vec<u8>),
}

pub fn parse_constraints(buf: &[u8]) -> ParsingError<Vec<Constraint>> {
let mut constraints = Vec::new();
let mut reader = Reader::new(buf);
let total_bytes = buf.len();
while reader.get_offset() < total_bytes {
let constraint_type = reader
.read_raw_bytes(1)
.map_err(|_| "Failed to read constraint type")?[0];
match constraint_type {
1 => {
constraints.push(Constraint::Lifetime(
reader
.read_u32()
.map_err(|_| "Failed to read u32 for lifetime")?,
));
}
2 => constraints.push(Constraint::Confirm),
255 => {
let ext_name = reader
.read_string()
.map_err(|_| "Failed to read string for extension name")?;
let ext_data = reader
.read_bytes()
.map_err(|_| "Failed to read bytes for extension data")?;
constraints.push(Constraint::Extension(ext_name, ext_data));
}
_ => return Err("Unknown constraint type".into()),
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_constraints returns a generic "Unknown constraint type" error without including the actual byte value, which makes debugging wire-compat issues harder. Consider incorporating the constraint_type value in the error details (and possibly the offset) so logs clearly show what was rejected.

Suggested change
_ => return Err("Unknown constraint type".into()),
_ => {
let offset = reader.get_offset().saturating_sub(1);
return Err(format!(
"Unknown constraint type {} at offset {}",
constraint_type, offset
)
.into());
}

Copilot uses AI. Check for mistakes.
}
}
Ok(constraints)
}
14 changes: 14 additions & 0 deletions rustica-agent/src/sshagent/handler.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::sshagent::constraints::Constraint;

use super::protocol::Request;
use super::protocol::Response;

Expand All @@ -9,6 +11,11 @@ use sshcerts::PrivateKey;
#[async_trait]
pub trait SshAgentHandler: Send + Sync {
async fn add_identity(&self, key: PrivateKey) -> HandleResult<Response>;
async fn add_identity_constrained(
&self,
key: PrivateKey,
constraints: Vec<Constraint>,
) -> HandleResult<Response>;
Comment on lines +17 to +18
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new required method to the public SshAgentHandler trait is a breaking change for any external implementers. Consider providing a default implementation for add_identity_constrained (e.g., delegate to add_identity or return Response::Failure) to preserve compatibility, or bump the crate version appropriately if this is intended to be breaking.

Suggested change
constraints: Vec<Constraint>,
) -> HandleResult<Response>;
_constraints: Vec<Constraint>,
) -> HandleResult<Response> {
// Default behavior: ignore constraints and treat this as a regular identity add.
self.add_identity(key).await
}

Copilot uses AI. Check for mistakes.
async fn identities(&self) -> HandleResult<Response>;
async fn sign_request(
&self,
Expand All @@ -29,6 +36,13 @@ pub trait SshAgentHandler: Send + Sync {
.await
}
Request::AddIdentity { private_key } => self.add_identity(private_key).await,
Request::AddIdentityConstrained {
private_key,
constraints,
} => {
self.add_identity_constrained(private_key, constraints)
.await
}
Request::Unknown => Ok(Response::Failure),
}
}
Expand Down
9 changes: 5 additions & 4 deletions rustica-agent/src/sshagent/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
extern crate byteorder;

mod agent;
mod protocol;
mod handler;
pub mod constraints;
pub mod error;
mod handler;
mod protocol;
Comment on lines 3 to +7
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making the entire constraints module public exposes internal parsing helpers (parse_constraints) as part of the crate API. If consumers only need the Constraint type (e.g., to implement SshAgentHandler), consider keeping the module private and re-exporting Constraint from sshagent (and making parse_constraints pub(crate)), which reduces public API surface while still allowing the type to be referenced.

Copilot uses AI. Check for mistakes.

pub use handler::SshAgentHandler;
pub use agent::Agent;
pub use handler::SshAgentHandler;
pub use protocol::Identity;
pub use protocol::Response;
pub use protocol::Identity;
26 changes: 24 additions & 2 deletions rustica-agent/src/sshagent/protocol.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use byteorder::{BigEndian, WriteBytesExt};
use sshcerts::ssh::Reader;
use tokio::{
io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt},
net::UnixStream,
Expand Down Expand Up @@ -31,11 +32,11 @@ impl MessageRequest {
17 => MessageRequest::AddIdentity,
18 => MessageRequest::RemoveIdentity,
19 => MessageRequest::RemoveAllIdentities,
25 => MessageRequest::AddIdConstrained,
20 => MessageRequest::AddSmartcardKey,
21 => MessageRequest::RemoveSmartcardKey,
22 => MessageRequest::Lock,
23 => MessageRequest::Unlock,
25 => MessageRequest::AddIdConstrained,
26 => MessageRequest::AddSmartcardKeyConstrained,
27 => MessageRequest::Extension,
_ => MessageRequest::Unknown,
Expand Down Expand Up @@ -76,6 +77,10 @@ pub enum Request {
AddIdentity {
private_key: sshcerts::PrivateKey,
},
AddIdentityConstrained {
private_key: sshcerts::PrivateKey,
constraints: Vec<super::constraints::Constraint>,
},
Unknown,
}

Expand All @@ -99,7 +104,24 @@ impl Request {
},
MessageRequest::RemoveIdentity => Ok(Request::Unknown),
MessageRequest::RemoveAllIdentities => Ok(Request::Unknown),
MessageRequest::AddIdConstrained => Ok(Request::Unknown),
MessageRequest::AddIdConstrained => {
let mut reader = Reader::new(buf);

let private_key = match sshcerts::PrivateKey::read_private_key(&mut reader) {
Ok(private_key) => private_key,
Err(_) => return Ok(Request::Unknown),
};

let constraints_buf = &buf[reader.get_offset()..];
let constraints = match super::constraints::parse_constraints(&constraints_buf) {
Ok(constraints) => constraints,
Err(_) => return Ok(Request::Unknown),
};
Ok(Request::AddIdentityConstrained {
private_key,
constraints,
})
}
MessageRequest::AddSmartcardKey => Ok(Request::Unknown),
MessageRequest::RemoveSmartcardKey => Ok(Request::Unknown),
MessageRequest::Lock => Ok(Request::Unknown),
Expand Down
2 changes: 1 addition & 1 deletion rustica/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ serde = { version = "1.0", features = ["derive"] }
# "yubikey-lite",
# ] }
# For Development
sshcerts = { version = "0.14.0", default-features = false, features = [
sshcerts = { version = "0.14.1", default-features = false, features = [
"fido-lite",
"x509-support",
"yubikey-lite",
Expand Down
Loading