Merge pull request 'fix(qr): bind invite sig fields; cap decode' (#6) from wt/t_8b24e067 into main

Reviewed-on: #6

src/qr.rs

@@ -20,30 +20,51 @@ pub struct QrPayload {
     pub signed_prekey: Vec<u8>,
 }
 
-/// `onionwire:v1:k={pubkey_hex}:o={v3onion}:spk={signed_prekey}:sig={sign(k||o||spk)}`
+/// Cap invite text before hex decode. Honest v1 is a few hundred bytes.
+const MAX_INVITE: usize = 4096;
+const HEX_K: usize = 64;
+const HEX_SPK: usize = 64;
+const HEX_SIG: usize = 128;
+
+/// `onionwire:v1:k={pubkey_hex}:o={v3onion}:spk={signed_prekey}:sig={sign(v2)}`
 pub fn encode(identity_sk: &[u8], onion: &str, signed_prekey: &[u8]) -> Result<String> {
+    if signed_prekey.len() != 32 {
+        return Err(Error("spk must be 32 bytes".into()));
+    }
+    if !is_v3_onion(onion) {
+        return Err(Error("o must be a v3 onion".into()));
+    }
     let sk_bytes: [u8; 32] = identity_sk
         .try_into()
         .map_err(|_| Error("identity secret key must be 32 bytes".into()))?;
     let sk = SigningKey::from_bytes(&sk_bytes);
     let k = to_hex(&sk.verifying_key().to_bytes());
     let spk = to_hex(signed_prekey);
-    let msg = sign_msg(&k, onion, &spk);
+    let msg = sign_msg_v2(&k, onion, &spk);
     let sig = to_hex(&sk.sign(&msg).to_bytes());
     Ok(format!("onionwire:v1:k={k}:o={onion}:spk={spk}:sig={sig}"))
 }
 
 pub fn decode(raw: &str) -> Result<QrPayload> {
+    if raw.len() > MAX_INVITE {
+        return Err(Error("invite too long".into()));
+    }
     let (k, onion, spk, sig) = parse_fields(raw)?;
+    if !is_hex_len(&k, HEX_K) {
+        return Err(Error("k must be 32 bytes".into()));
+    }
+    if !is_hex_len(&spk, HEX_SPK) {
+        return Err(Error("spk must be 32 bytes".into()));
+    }
+    if !is_hex_len(&sig, HEX_SIG) {
+        return Err(Error("sig must be 64 bytes".into()));
+    }
+    if !is_v3_onion(&onion) {
+        return Err(Error("o must be a v3 onion".into()));
+    }
     let pubkey = from_hex(&k)?;
     let signed_prekey = from_hex(&spk)?;
     let sig_bytes = from_hex(&sig)?;
-    if pubkey.len() != 32 {
-        return Err(Error("k must be 32 bytes".into()));
-    }
-    if sig_bytes.len() != 64 {
-        return Err(Error("sig must be 64 bytes".into()));
-    }
     let pk_arr: [u8; 32] = pubkey
         .as_slice()
         .try_into()
@@ -54,9 +75,15 @@ pub fn decode(raw: &str) -> Result<QrPayload> {
         .map_err(|_| Error("sig must be 64 bytes".into()))?;
     let vk = VerifyingKey::from_bytes(&pk_arr).map_err(|e| Error(e.to_string()))?;
     let signature = Signature::from_bytes(&sig_arr);
-    let msg = sign_msg(&k, &onion, &spk);
-    vk.verify(&msg, &signature)
-        .map_err(|_| Error("bad signature".into()))?;
+    let ok_v2 = vk
+        .verify(&sign_msg_v2(&k, &onion, &spk), &signature)
+        .is_ok();
+    let ok_v1 = vk
+        .verify(&sign_msg_v1(&k, &onion, &spk), &signature)
+        .is_ok();
+    if !ok_v2 && !ok_v1 {
+        return Err(Error("bad signature".into()));
+    }
     Ok(QrPayload {
         pubkey,
         onion,
@@ -64,7 +91,30 @@ pub fn decode(raw: &str) -> Result<QrPayload> {
     })
 }
 
-fn sign_msg(k: &str, onion: &str, spk: &str) -> Vec<u8> {
+fn is_v3_onion(s: &str) -> bool {
+    let Some(addr) = s.strip_suffix(".onion") else {
+        return false;
+    };
+    addr.len() == 56 && addr.bytes().all(|b| matches!(b, b'a'..=b'z' | b'2'..=b'7'))
+}
+
+fn is_hex_len(s: &str, n: usize) -> bool {
+    s.len() == n && s.bytes().all(|c| c.is_ascii_hexdigit())
+}
+
+fn sign_msg_v2(k: &str, onion: &str, spk: &str) -> Vec<u8> {
+    let mut msg = Vec::with_capacity(19 + 3 + k.len() + onion.len() + spk.len());
+    msg.extend_from_slice(b"onionwire-invite-v1");
+    msg.push(0);
+    msg.extend_from_slice(k.as_bytes());
+    msg.push(0);
+    msg.extend_from_slice(onion.as_bytes());
+    msg.push(0);
+    msg.extend_from_slice(spk.as_bytes());
+    msg
+}
+
+fn sign_msg_v1(k: &str, onion: &str, spk: &str) -> Vec<u8> {
     let mut msg = Vec::with_capacity(k.len() + onion.len() + spk.len());
     msg.extend_from_slice(k.as_bytes());
     msg.extend_from_slice(onion.as_bytes());
@@ -118,7 +168,7 @@ fn to_hex(bytes: &[u8]) -> String {
 }
 
 fn from_hex(s: &str) -> Result<Vec<u8>> {
-    if s.is_empty() || !s.len().is_multiple_of(2) {
+    if s.len() > HEX_SIG || s.is_empty() || !s.len().is_multiple_of(2) {
         return Err(Error("invalid hex".into()));
     }
     if !s.bytes().all(|c| c.is_ascii_hexdigit()) {
@@ -129,3 +179,25 @@ fn from_hex(s: &str) -> Result<Vec<u8>> {
         .map(|i| u8::from_str_radix(&s[i..i + 2], 16).map_err(|_| Error("invalid hex".into())))
         .collect()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn v3() -> String {
+        format!("{}.onion", "a".repeat(56))
+    }
+
+    #[test]
+    fn well_formed_v1_concat_still_decodes() {
+        let sk = SigningKey::from_bytes(&[7u8; 32]);
+        let k = to_hex(&sk.verifying_key().to_bytes());
+        let onion = v3();
+        let spk = to_hex(&[9u8; 32]);
+        let sig = to_hex(&sk.sign(&sign_msg_v1(&k, &onion, &spk)).to_bytes());
+        let raw = format!("onionwire:v1:k={k}:o={onion}:spk={spk}:sig={sig}");
+        let p = decode(&raw).expect("legacy concat");
+        assert_eq!(p.onion, onion);
+        assert_eq!(p.signed_prekey, vec![9u8; 32]);
+    }
+}

tests/qr.rs

@@ -2,8 +2,8 @@
 
 use std::sync::{Mutex, MutexGuard};
 
-use onionwire::Store;
 use onionwire::qr;
+use onionwire::Store;
 
 static ENV_LOCK: Mutex<()> = Mutex::new(());
 
@@ -15,7 +15,7 @@ struct TempHome {
 impl TempHome {
     fn new() -> Self {
         let dir = tempfile::tempdir().expect("tempdir");
-        let guard = ENV_LOCK.lock().expect("env lock");
+        let guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         unsafe {
             std::env::set_var("ONIONWIRE_HOME", dir.path());
             std::env::set_var("ONIONWIRE_STORE_PASSPHRASE", "onionwire-test");
@@ -37,7 +37,37 @@ impl Drop for TempHome {
 }
 
 fn v3onion(tag: &str) -> String {
-    format!("{tag}.onion")
+    let mut addr = vec![b'a'; 56];
+    let bytes: Vec<u8> = tag
+        .bytes()
+        .map(|b| match b {
+            b'a'..=b'z' | b'2'..=b'7' => b,
+            _ => b'a',
+        })
+        .collect();
+    let n = bytes.len().min(56);
+    addr[..n].copy_from_slice(&bytes[..n]);
+    format!("{}.onion", String::from_utf8(addr).expect("base32"))
+}
+
+fn invite_fields(raw: &str) -> (String, String, String, String) {
+    let rest = raw.strip_prefix("onionwire:v1:").expect("prefix");
+    let mut k = String::new();
+    let mut o = String::new();
+    let mut spk = String::new();
+    let mut sig = String::new();
+    for part in rest.split(':') {
+        if let Some(v) = part.strip_prefix("k=") {
+            k = v.to_string();
+        } else if let Some(v) = part.strip_prefix("o=") {
+            o = v.to_string();
+        } else if let Some(v) = part.strip_prefix("spk=") {
+            spk = v.to_string();
+        } else if let Some(v) = part.strip_prefix("sig=") {
+            sig = v.to_string();
+        }
+    }
+    (k, o, spk, sig)
 }
 
 #[test]
@@ -119,3 +149,73 @@ fn unknown_k_decode_does_not_insert() {
     let _p = qr::decode(&raw).unwrap();
     assert_eq!(store.friend_count().unwrap(), 0);
 }
+
+#[test]
+fn shifted_o_spk_is_err() {
+    let _home = TempHome::new();
+    let store = Store::open().expect("open");
+    let me = store.self_identity().expect("self");
+    let onion = v3onion("honest");
+    let spk = [0xab; 32];
+    let raw = qr::encode(&me.identity_sk, &onion, &spk).expect("encode");
+    let (k, o, spk_hex, sig) = invite_fields(&raw);
+    assert_eq!(spk_hex.len(), 64);
+    let mutant = format!(
+        "onionwire:v1:k={k}:o={o}{}:spk={}:sig={sig}",
+        &spk_hex[..8],
+        &spk_hex[8..]
+    );
+    assert!(
+        qr::decode(&mutant).is_err(),
+        "shifted o/spk must not verify"
+    );
+}
+
+#[test]
+fn encode_rejects_spk_not_32() {
+    let _home = TempHome::new();
+    let store = Store::open().expect("open");
+    let me = store.self_identity().expect("self");
+    let onion = v3onion("spk");
+    assert!(qr::encode(&me.identity_sk, &onion, &[1u8; 31]).is_err());
+    assert!(qr::encode(&me.identity_sk, &onion, &[1u8; 33]).is_err());
+    assert!(qr::encode(&me.identity_sk, &onion, &[]).is_err());
+}
+
+#[test]
+fn encode_rejects_garbage_onion() {
+    let _home = TempHome::new();
+    let store = Store::open().expect("open");
+    let me = store.self_identity().expect("self");
+    let spk = [2u8; 32];
+    assert!(qr::encode(&me.identity_sk, "not-an-onion", &spk).is_err());
+    assert!(qr::encode(&me.identity_sk, "alice.onion", &spk).is_err());
+    assert!(qr::encode(&me.identity_sk, &format!("{}.onion", "A".repeat(56)), &spk).is_err());
+}
+
+#[test]
+fn decode_rejects_spk_len_and_garbage_onion() {
+    let _home = TempHome::new();
+    let store = Store::open().expect("open");
+    let me = store.self_identity().expect("self");
+    let raw = qr::encode(&me.identity_sk, &v3onion("ok"), &[3u8; 32]).expect("encode");
+    let (k, o, spk, sig) = invite_fields(&raw);
+    let short_spk = format!("onionwire:v1:k={k}:o={o}:spk={}:sig={sig}", &spk[..62]);
+    assert!(qr::decode(&short_spk).is_err());
+    let garbage_o = format!("onionwire:v1:k={k}:o=nope.onion:spk={spk}:sig={sig}");
+    assert!(qr::decode(&garbage_o).is_err());
+}
+
+#[test]
+fn huge_hex_is_err_quickly() {
+    let huge = format!(
+        "onionwire:v1:k={}:o=x:spk=yy:sig=zz",
+        "aa".repeat(1024 * 1024)
+    );
+    let t = std::time::Instant::now();
+    assert!(qr::decode(&huge).is_err());
+    assert!(
+        t.elapsed() < std::time::Duration::from_millis(250),
+        "decode must fail closed before a multi-MB alloc"
+    );
+}

tests/tui_chrome.rs

@@ -71,7 +71,7 @@ fn chrome_renders_at_80x24_and_120x40() {
     use ratatui::Terminal;
 
     let fp = "abcdef0123456789";
-    let onion = "abcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopq.onion";
+    let onion = "abcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopqrstuvwx.onion";
     for (w, h) in [(80u16, 24u16), (120, 40)] {
         let backend = TestBackend::new(w, h);
         let mut terminal = Terminal::new(backend).expect("terminal");
@@ -113,7 +113,7 @@ fn chrome_renders_at_80x24_and_120x40() {
 #[test]
 fn main_footer_keeps_help_at_80_and_120() {
     let fp = "abcdef0123456789deadbeef";
-    let onion = "abcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopq.onion";
+    let onion = "abcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopqrstuvwx.onion";
     let hints = main_footer_hints();
     assert!(hints.contains("? help"));
     for w in [80u16, 120] {
@@ -170,7 +170,7 @@ fn share_screen_shows_invite_not_qr_at_80x24() {
     use ratatui::Terminal;
 
     let sk = [7u8; 32];
-    let onion = "abcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopq.onion";
+    let onion = "abcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopqrstuvwx.onion";
     let payload = qr::encode(&sk, onion, &[1u8; 32]).expect("encode");
     assert!(payload.starts_with("onionwire:v1:"));