refactor(host): extract native_pairing/sanitize.rs — the untrusted-name scrubber
Per plan §W5: move sanitize_device_name (+ its NAME_MAX cap and unit test) out of the native_pairing facade into native_pairing/sanitize.rs. It is a self-contained, security-relevant leaf — the one place a wire-supplied unpaired-device name is scrubbed of control chars / bidi-override spoofing before it is stored, listed, logged, or shown in the approval UI. Re-export via `pub(crate) use` so crate::native_pairing::sanitize_device_name stays stable (punktfunk1 accept loop + the two in-crate callers). Pure code-move; verified host clippy 0/0 + 11 native_pairing tests green on Linux (home-worker-5). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -196,47 +196,11 @@ fn random_pin() -> String {
|
|||||||
format!("{:04}", rand::thread_rng().gen_range(0..10_000u32))
|
format!("{:04}", rand::thread_rng().gen_range(0..10_000u32))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Sanitize a client-supplied device name before it's stored, listed, or logged. The name comes
|
/// The untrusted-device-name sanitizer lives in its own module (plan §W5); re-exported so
|
||||||
/// straight off the wire (the `Hello`/`PairRequest` of an *unpaired* device), so it's untrusted: a
|
/// `crate::native_pairing::sanitize_device_name` stays stable (the `punktfunk1` accept loop
|
||||||
/// hostile LAN device could embed terminal escapes / control characters (log + console injection) or
|
/// reaches it there).
|
||||||
/// bidi overrides (`U+202E` etc.) to make a malicious device *look* like a trusted one in the
|
mod sanitize;
|
||||||
/// approval UI. Strip C0/C1 controls and Unicode bidi/format controls, collapse whitespace, trim, and
|
pub(crate) use sanitize::sanitize_device_name;
|
||||||
/// cap the length; an empty/all-control name falls back to a fingerprint-derived label.
|
|
||||||
pub(crate) fn sanitize_device_name(name: &str, fp_hex: &str) -> String {
|
|
||||||
let cleaned: String = name
|
|
||||||
.chars()
|
|
||||||
.map(|c| if c == '\t' || c == '\n' { ' ' } else { c })
|
|
||||||
.filter(|&c| {
|
|
||||||
!c.is_control()
|
|
||||||
// Bidi/format controls that could spoof or reorder the displayed name.
|
|
||||||
&& !('\u{202A}'..='\u{202E}').contains(&c) // LRE..RLO/PDF
|
|
||||||
&& !('\u{2066}'..='\u{2069}').contains(&c) // LRI..PDI
|
|
||||||
&& c != '\u{200E}' // LRM
|
|
||||||
&& c != '\u{200F}' // RLM
|
|
||||||
&& c != '\u{061C}' // ALM
|
|
||||||
&& c != '\u{FEFF}' // BOM / zero-width no-break space
|
|
||||||
})
|
|
||||||
.collect();
|
|
||||||
// Collapse internal whitespace runs, trim, cap at the wire limit.
|
|
||||||
let collapsed = cleaned.split_whitespace().collect::<Vec<_>>().join(" ");
|
|
||||||
let mut trimmed = collapsed.as_str();
|
|
||||||
while trimmed.len() > NAME_MAX {
|
|
||||||
let mut cut = NAME_MAX;
|
|
||||||
while !trimmed.is_char_boundary(cut) {
|
|
||||||
cut -= 1;
|
|
||||||
}
|
|
||||||
trimmed = &trimmed[..cut];
|
|
||||||
}
|
|
||||||
let trimmed = trimmed.trim();
|
|
||||||
if trimmed.is_empty() {
|
|
||||||
format!("device {}", &fp_hex[..8.min(fp_hex.len())])
|
|
||||||
} else {
|
|
||||||
trimmed.to_string()
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Max stored device-name length (matches the `Hello` wire cap, `quic::HELLO_NAME_MAX`).
|
|
||||||
const NAME_MAX: usize = 64;
|
|
||||||
|
|
||||||
impl NativePairing {
|
impl NativePairing {
|
||||||
/// Load the trust store. `store_path = None` uses the default config path. If `arm_at_start`
|
/// Load the trust store. `store_path = None` uses the default config path. If `arm_at_start`
|
||||||
@@ -816,24 +780,6 @@ mod tests {
|
|||||||
let _ = std::fs::remove_file(&p);
|
let _ = std::fs::remove_file(&p);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn sanitize_strips_control_and_bidi() {
|
|
||||||
// ANSI escape + newline + a bidi override that could spoof the displayed name.
|
|
||||||
let dirty = "\u{1b}]0;evil\u{07}Good\nDevice\u{202E}xfp";
|
|
||||||
let clean = sanitize_device_name(dirty, "deadbeef00");
|
|
||||||
assert!(!clean.contains('\u{1b}') && !clean.contains('\n') && !clean.contains('\u{202E}'));
|
|
||||||
// ESC dropped (']' survives), BEL dropped, '\n'→space (Good Device), RLO dropped (no space).
|
|
||||||
assert_eq!(clean, "]0;evilGood Devicexfp");
|
|
||||||
// All-control / empty → fingerprint-derived fallback.
|
|
||||||
assert_eq!(
|
|
||||||
sanitize_device_name("\u{1b}\u{07}", "deadbeef00"),
|
|
||||||
"device deadbeef"
|
|
||||||
);
|
|
||||||
assert_eq!(sanitize_device_name(" ", "abc"), "device abc");
|
|
||||||
// Over-long names cap at a char boundary.
|
|
||||||
assert!(sanitize_device_name(&"x".repeat(200), "ab").len() <= 64);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn pairing_clears_a_pending_knock() {
|
fn pairing_clears_a_pending_knock() {
|
||||||
let p = temp();
|
let p = temp();
|
||||||
|
|||||||
@@ -0,0 +1,69 @@
|
|||||||
|
//! Sanitize a client-supplied device name before it is stored, listed, logged, or shown in the
|
||||||
|
//! pairing-approval UI. The name arrives on the wire from an *unpaired* device, so it is untrusted
|
||||||
|
//! (terminal-escape / control-char injection, bidi-override spoofing of a trusted-looking name) —
|
||||||
|
//! this is the one place that scrubs it. Split out of the `native_pairing` facade (plan §W5).
|
||||||
|
|
||||||
|
/// Sanitize a client-supplied device name before it's stored, listed, or logged. The name comes
|
||||||
|
/// straight off the wire (the `Hello`/`PairRequest` of an *unpaired* device), so it's untrusted: a
|
||||||
|
/// hostile LAN device could embed terminal escapes / control characters (log + console injection) or
|
||||||
|
/// bidi overrides (`U+202E` etc.) to make a malicious device *look* like a trusted one in the
|
||||||
|
/// approval UI. Strip C0/C1 controls and Unicode bidi/format controls, collapse whitespace, trim, and
|
||||||
|
/// cap the length; an empty/all-control name falls back to a fingerprint-derived label.
|
||||||
|
pub(crate) fn sanitize_device_name(name: &str, fp_hex: &str) -> String {
|
||||||
|
let cleaned: String = name
|
||||||
|
.chars()
|
||||||
|
.map(|c| if c == '\t' || c == '\n' { ' ' } else { c })
|
||||||
|
.filter(|&c| {
|
||||||
|
!c.is_control()
|
||||||
|
// Bidi/format controls that could spoof or reorder the displayed name.
|
||||||
|
&& !('\u{202A}'..='\u{202E}').contains(&c) // LRE..RLO/PDF
|
||||||
|
&& !('\u{2066}'..='\u{2069}').contains(&c) // LRI..PDI
|
||||||
|
&& c != '\u{200E}' // LRM
|
||||||
|
&& c != '\u{200F}' // RLM
|
||||||
|
&& c != '\u{061C}' // ALM
|
||||||
|
&& c != '\u{FEFF}' // BOM / zero-width no-break space
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
// Collapse internal whitespace runs, trim, cap at the wire limit.
|
||||||
|
let collapsed = cleaned.split_whitespace().collect::<Vec<_>>().join(" ");
|
||||||
|
let mut trimmed = collapsed.as_str();
|
||||||
|
while trimmed.len() > NAME_MAX {
|
||||||
|
let mut cut = NAME_MAX;
|
||||||
|
while !trimmed.is_char_boundary(cut) {
|
||||||
|
cut -= 1;
|
||||||
|
}
|
||||||
|
trimmed = &trimmed[..cut];
|
||||||
|
}
|
||||||
|
let trimmed = trimmed.trim();
|
||||||
|
if trimmed.is_empty() {
|
||||||
|
format!("device {}", &fp_hex[..8.min(fp_hex.len())])
|
||||||
|
} else {
|
||||||
|
trimmed.to_string()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Max stored device-name length (matches the `Hello` wire cap, `quic::HELLO_NAME_MAX`).
|
||||||
|
const NAME_MAX: usize = 64;
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sanitize_strips_control_and_bidi() {
|
||||||
|
// ANSI escape + newline + a bidi override that could spoof the displayed name.
|
||||||
|
let dirty = "\u{1b}]0;evil\u{07}Good\nDevice\u{202E}xfp";
|
||||||
|
let clean = sanitize_device_name(dirty, "deadbeef00");
|
||||||
|
assert!(!clean.contains('\u{1b}') && !clean.contains('\n') && !clean.contains('\u{202E}'));
|
||||||
|
// ESC dropped (']' survives), BEL dropped, '\n'→space (Good Device), RLO dropped (no space).
|
||||||
|
assert_eq!(clean, "]0;evilGood Devicexfp");
|
||||||
|
// All-control / empty → fingerprint-derived fallback.
|
||||||
|
assert_eq!(
|
||||||
|
sanitize_device_name("\u{1b}\u{07}", "deadbeef00"),
|
||||||
|
"device deadbeef"
|
||||||
|
);
|
||||||
|
assert_eq!(sanitize_device_name(" ", "abc"), "device abc");
|
||||||
|
// Over-long names cap at a char boundary.
|
||||||
|
assert!(sanitize_device_name(&"x".repeat(200), "ab").len() <= 64);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user