Skip to content

Commit 0040dd2

Browse files
HermanShermanBotHerman Sherman
andauthored
fix: 4 security issues from code audit (#334)
HIGH: handle_intro filter logic (AND→OR) — events with mismatched kind/type combos could slip through validation. MEDIUM: - Invite tokens: 3 bytes → 8 bytes (48-bit → 64-bit keyspace) - responder_health_set: add is_valid_slot_id() before path construction - Macaroon verify: constant-time sig comparison (prevent timing leak) Co-authored-by: Herman Sherman <herman@hermans.mac.mini.lan>
1 parent 51c28f7 commit 0040dd2

2 files changed

Lines changed: 23 additions & 5 deletions

File tree

src/macaroon.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl Macaroon {
5959

6060
pub fn verify(&self, root_key: &[u8], context: &VerifyContext) -> Result<()> {
6161
let expected = compute_signature(root_key, &self.identifier, &self.caveats)?;
62-
if self.signature != expected {
62+
if !constant_time_eq(self.signature.as_bytes(), expected.as_bytes()) {
6363
bail!("macaroon signature mismatch");
6464
}
6565
for caveat in &self.caveats {
@@ -121,3 +121,15 @@ fn parse_rfc3339(s: &str) -> Result<time::OffsetDateTime> {
121121
time::OffsetDateTime::parse(s, &time::format_description::well_known::Rfc3339)
122122
.map_err(|e| anyhow!("invalid RFC3339 timestamp {s:?}: {e}"))
123123
}
124+
125+
/// Constant-time comparison to avoid timing side-channels on signature checks.
126+
fn constant_time_eq(a: &[u8], b: &[u8]) -> bool {
127+
if a.len() != b.len() {
128+
return false;
129+
}
130+
let mut acc = 0u8;
131+
for (x, y) in a.iter().zip(b.iter()) {
132+
acc |= x ^ y;
133+
}
134+
acc == 0
135+
}

src/relay_server.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,9 +1544,8 @@ async fn invite_register(
15441544
.duration_since(UNIX_EPOCH)
15451545
.map(|d| d.as_secs())
15461546
.unwrap_or(0);
1547-
// 6-hex token → 16.7M space. Collision probability negligible at v0.5
1548-
// scale; if a collision happens (1 in 16M) we 409 and the caller retries.
1549-
let token = random_hex(3);
1547+
// 16-hex token → 2^64 space. Brute-force infeasible.
1548+
let token = random_hex(8);
15501549
let rec = InviteRecord {
15511550
token: token.clone(),
15521551
invite_url: req.invite_url,
@@ -1864,7 +1863,7 @@ async fn handle_intro(
18641863
// to the standard /v1/events/:slot_id with bearer auth.
18651864
let kind = req.event.get("kind").and_then(Value::as_u64).unwrap_or(0);
18661865
let type_str = req.event.get("type").and_then(Value::as_str).unwrap_or("");
1867-
if kind != 1100 && type_str != "pair_drop" && type_str != "agent_card" {
1866+
if kind != 1100 || (type_str != "pair_drop" && type_str != "agent_card") {
18681867
return (
18691868
StatusCode::BAD_REQUEST,
18701869
Json(json!({
@@ -2204,6 +2203,13 @@ async fn responder_health_set(
22042203
if let Err(resp) = check_token(&relay, &headers, &slot_id).await {
22052204
return resp;
22062205
}
2206+
if !is_valid_slot_id(&slot_id) {
2207+
return (
2208+
StatusCode::BAD_REQUEST,
2209+
Json(json!({"error": "invalid slot_id format"})),
2210+
)
2211+
.into_response();
2212+
}
22072213
let path = relay
22082214
.state_dir
22092215
.join("responder-health")

0 commit comments

Comments
 (0)