From 1fd306575e28dfc3235ec9d2403e361e44847a2e Mon Sep 17 00:00:00 2001 From: framefilter Date: Fri, 9 Oct 2026 20:00:24 -0400 Subject: [PATCH 01/35] import: lock the Molto2 import behavior with golden tests (re #170) The parsers feeding the Molto2 slot programmer are about to gain wider siblings for keys that store HOTP, other TOTP periods and SHA-512. Pin what the Molto2 path accepts, skips and reports today before anything moves: the exact Debug of every accepted entry and the exact text of every error, across Aegis, 2FAS, otpauth lists, parse_any, Google Authenticator payloads and single otpauth:// URIs. Any refactor that changes a Molto2 import result now fails here first. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU --- crates/keyroost-import/tests/molto_golden.rs | 348 +++++++++++++++++++ 1 file changed, 348 insertions(+) create mode 100644 crates/keyroost-import/tests/molto_golden.rs diff --git a/crates/keyroost-import/tests/molto_golden.rs b/crates/keyroost-import/tests/molto_golden.rs new file mode 100644 index 00000000..7dc72574 --- /dev/null +++ b/crates/keyroost-import/tests/molto_golden.rs @@ -0,0 +1,348 @@ +//! Golden tests locking the Molto2 import path's observable behavior. +//! +//! The narrow parsers (`aegis::parse`, `twofas::parse`, `parse_otpauth_list`, +//! `parse_any`, `migration::parse_payload`, `otpauth::parse`) feed the Molto2 +//! slot programmer. Wider parsers for other keys are built beside them; this +//! file pins every result the Molto2 path produces today — the exact `Debug` +//! of each accepted entry and the exact text of each error — so any change to +//! what a Molto2 import accepts, skips or reports shows up here first. + +#![cfg(feature = "bulk")] + +use keyroost_import::{aegis, migration, otpauth, parse_bulk_any, parse_otpauth_list, twofas}; + +/// Render a parse result: `Ok` as the `Debug` of the value, `Err` as the +/// error's `Display` text, prefixed so the two can't be confused. +fn render(r: Result) -> String { + match r { + Ok(v) => format!("Ok {:?}", v), + Err(e) => format!("Err {}", e), + } +} + +/// Fail with every mismatch at once (the actual value printed in full), so a +/// behavior change shows its whole footprint rather than the first case. +fn assert_no_mismatches(mismatches: &[String]) { + assert!(mismatches.is_empty(), "\n{}", mismatches.join("\n")); +} + +// ---- Aegis --------------------------------------------------------------- + +const AEGIS_TOTP: &str = r#"{"type":"totp","name":"alice@example.com","issuer":"GitHub","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":6,"period":30}}"#; +const AEGIS_HOTP: &str = r#"{"type":"hotp","name":"counter","issuer":"Bank","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":6,"counter":5}}"#; +const AEGIS_STEAM: &str = r#"{"type":"steam","name":"gamer","issuer":"Steam","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":5,"period":30}}"#; +const AEGIS_DIGITS7: &str = r#"{"type":"totp","name":"seven","issuer":"Odd","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":7,"period":30}}"#; +const AEGIS_PERIOD45: &str = r#"{"type":"totp","name":"slow","issuer":"Slow","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":6,"period":45}}"#; +const AEGIS_SHA512: &str = r#"{"type":"totp","name":"wide","issuer":"Wide","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA512","digits":6,"period":30}}"#; +const AEGIS_BAD_B32: &str = r#"{"type":"totp","name":"bad","issuer":"Bad","info":{"secret":"NOT_BASE32!!","algo":"SHA1","digits":6,"period":30}}"#; +// 103 base32 'A's decode to 64 bytes: one past the Molto2's 63-byte cap. +const AEGIS_SECRET64: &str = r#"{"type":"totp","name":"long","issuer":"Long","info":{"secret":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","algo":"SHA1","digits":6,"period":30}}"#; +const AEGIS_EMPTY_ISSUER: &str = r#"{"type":"totp","name":"bob","issuer":"","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA256","digits":8,"period":60}}"#; + +fn aegis_doc(entries: &[&str]) -> String { + format!( + r#"{{"version":1,"header":{{"slots":null,"params":null}},"db":{{"version":2,"entries":[{}]}}}}"#, + entries.join(",") + ) +} + +/// The full corpus, in this order. The first entry the Molto2 path can't +/// take aborts the whole file (digits 7, at index 3). +fn aegis_corpus() -> String { + aegis_doc(&[ + AEGIS_TOTP, + AEGIS_HOTP, + AEGIS_STEAM, + AEGIS_DIGITS7, + AEGIS_PERIOD45, + AEGIS_SHA512, + AEGIS_BAD_B32, + AEGIS_SECRET64, + AEGIS_EMPTY_ISSUER, + ]) +} + +#[test] +fn aegis_golden() { + let cases: &[(String, &str)] = &[ + ( + aegis_corpus(), + r#"Err entry #3: digits=7 not supported by Molto2 (4, 6, 8, or 10)"#, + ), + ( + aegis_doc(&[AEGIS_TOTP]), + r#"Ok [BulkEntry { issuer: Some("GitHub"), account: Some("alice@example.com"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }]"#, + ), + (aegis_doc(&[AEGIS_HOTP]), r#"Err no entries found"#), + (aegis_doc(&[AEGIS_STEAM]), r#"Err no entries found"#), + ( + aegis_doc(&[AEGIS_DIGITS7]), + r#"Err entry #0: digits=7 not supported by Molto2 (4, 6, 8, or 10)"#, + ), + ( + aegis_doc(&[AEGIS_TOTP, AEGIS_HOTP, AEGIS_DIGITS7]), + r#"Err entry #2: digits=7 not supported by Molto2 (4, 6, 8, or 10)"#, + ), + ( + aegis_doc(&[AEGIS_PERIOD45]), + r#"Err entry #0: period=45s not supported by Molto2 (30 or 60)"#, + ), + ( + aegis_doc(&[AEGIS_SHA512]), + r#"Err entry #0: algorithm "SHA512" not supported by Molto2 (SHA1 or SHA256 only)"#, + ), + ( + aegis_doc(&[AEGIS_BAD_B32]), + r#"Err entry #0: `secret` is not valid base32"#, + ), + ( + aegis_doc(&[AEGIS_SECRET64]), + r#"Err entry #0: `secret` is not valid base32"#, + ), + ( + aegis_doc(&[AEGIS_EMPTY_ISSUER]), + r#"Ok [BulkEntry { issuer: None, account: Some("bob"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds60 }]"#, + ), + ( + aegis_doc(&[AEGIS_TOTP, AEGIS_HOTP, AEGIS_STEAM, AEGIS_EMPTY_ISSUER]), + r#"Ok [BulkEntry { issuer: Some("GitHub"), account: Some("alice@example.com"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }, BulkEntry { issuer: None, account: Some("bob"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds60 }]"#, + ), + ( + r#"{"db":"ZW5jcnlwdGVk","header":{}}"#.to_owned(), + r#"Err encrypted vault: Aegis export is encrypted; supply a password (CLI: --password env:NAME or --password stdin)"#, + ), + ]; + let mut mismatches = Vec::new(); + for (json, want) in cases { + let got = render(aegis::parse(json)); + if got != *want { + mismatches.push(format!("aegis::parse({json}) -> {got:?}")); + } + } + assert_no_mismatches(&mismatches); +} + +// ---- 2FAS ---------------------------------------------------------------- + +const TWOFAS_PLAIN: &str = r#"{"services":[{"name":"Stripe","secret":"JBSWY3DPEHPK3PXP","otp":{"account":"ops@example.com","issuer":"Stripe","digits":8,"period":60,"algorithm":"SHA256","tokenType":"TOTP"}},{"name":"NoIssuer","secret":"JBSWY3DPEHPK3PXP","otp":{"account":"me"}}]}"#; +const TWOFAS_ENCRYPTED: &str = r#"{"servicesEncrypted":"c2VjcmV0","services":[]}"#; +const TWOFAS_HOTP: &str = r#"{"services":[{"name":"Bank","secret":"JBSWY3DPEHPK3PXP","otp":{"account":"me","issuer":"Bank","digits":6,"counter":3,"algorithm":"SHA1","tokenType":"HOTP"}}]}"#; +const TWOFAS_MIXED: &str = r#"{"services":[{"name":"Bank","secret":"JBSWY3DPEHPK3PXP","otp":{"account":"me","issuer":"Bank","counter":3,"tokenType":"HOTP"}},{"name":"Mail","secret":"JBSWY3DPEHPK3PXP","otp":{"account":"you","tokenType":"TOTP"}}]}"#; + +#[test] +fn twofas_golden() { + let cases: &[(&str, &str)] = &[ + ( + TWOFAS_PLAIN, + r#"Ok [BulkEntry { issuer: Some("Stripe"), account: Some("ops@example.com"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds60 }, BulkEntry { issuer: Some("NoIssuer"), account: Some("me"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }]"#, + ), + ( + TWOFAS_ENCRYPTED, + r#"Err encrypted vault: 2FAS export is encrypted; re-export without a password"#, + ), + (TWOFAS_HOTP, r#"Err no entries found"#), + ( + TWOFAS_MIXED, + r#"Ok [BulkEntry { issuer: Some("Mail"), account: Some("you"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }]"#, + ), + ]; + let mut mismatches = Vec::new(); + for (json, want) in cases { + let got = render(twofas::parse(json)); + if got != *want { + mismatches.push(format!("twofas::parse({json}) -> {got:?}")); + } + } + assert_no_mismatches(&mismatches); +} + +// ---- otpauth list ---------------------------------------------------------- + +const LIST_OK: &str = "# exported list\n\notpauth://totp/GitHub:me@example.com?secret=JBSWY3DPEHPK3PXP\n \n # indented comment\notpauth://totp/GitLab?secret=JBSWY3DPEHPK3PXP&algorithm=SHA256&digits=8&period=60\n"; +const LIST_HOTP: &str = "# one HOTP line\notpauth://totp/A?secret=JBSWY3DPEHPK3PXP\notpauth://hotp/B?secret=JBSWY3DPEHPK3PXP&counter=2\n"; +const LIST_PERIOD45: &str = + "\n# blank line above\n\notpauth://totp/Slow?secret=JBSWY3DPEHPK3PXP&period=45\n"; +const LIST_MIGRATION: &str = "otpauth-migration://offline?data=CgA%3D\n"; + +#[test] +fn otpauth_list_golden() { + let cases: &[(&str, &str)] = &[ + ( + LIST_OK, + r#"Ok [BulkEntry { issuer: Some("GitHub"), account: Some("me@example.com"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }, BulkEntry { issuer: None, account: Some("GitLab"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds60 }]"#, + ), + ( + LIST_HOTP, + r#"Err entry #2: unsupported OTP type "hotp" (only totp is supported)"#, + ), + ( + LIST_PERIOD45, + r#"Err entry #3: period=45s not supported by Molto2 (30 or 60)"#, + ), + (LIST_MIGRATION, r#"Err entry #0: not an otpauth:// URI"#), + ("# only a comment\n\n", r#"Err no entries found"#), + ]; + let mut mismatches = Vec::new(); + for (text, want) in cases { + let got = render(parse_otpauth_list(text)); + if got != *want { + mismatches.push(format!("parse_otpauth_list({text:?}) -> {got:?}")); + } + } + assert_no_mismatches(&mismatches); +} + +// ---- parse_any ------------------------------------------------------------- + +#[test] +fn parse_any_golden() { + let cases: &[(String, &str)] = &[ + ( + aegis_doc(&[AEGIS_TOTP, AEGIS_HOTP]), + r#"Ok [BulkEntry { issuer: Some("GitHub"), account: Some("alice@example.com"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }]"#, + ), + (aegis_corpus(), r#"Err no entries found"#), + ( + TWOFAS_PLAIN.to_owned(), + r#"Ok [BulkEntry { issuer: Some("Stripe"), account: Some("ops@example.com"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds60 }, BulkEntry { issuer: Some("NoIssuer"), account: Some("me"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }]"#, + ), + ( + TWOFAS_ENCRYPTED.to_owned(), + r#"Err encrypted vault: 2FAS export is encrypted; re-export without a password"#, + ), + (TWOFAS_HOTP.to_owned(), r#"Err no entries found"#), + ( + r#"{"db":"ZW5jcnlwdGVk","header":{}}"#.to_owned(), + r#"Err encrypted vault: Aegis export is encrypted; supply a password (CLI: --password env:NAME or --password stdin)"#, + ), + (r#"{"neither":true}"#.to_owned(), r#"Err no entries found"#), + ( + LIST_OK.to_owned(), + r#"Ok [BulkEntry { issuer: Some("GitHub"), account: Some("me@example.com"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }, BulkEntry { issuer: None, account: Some("GitLab"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds60 }]"#, + ), + ( + LIST_PERIOD45.to_owned(), + r#"Err entry #3: period=45s not supported by Molto2 (30 or 60)"#, + ), + ]; + let mut mismatches = Vec::new(); + for (text, want) in cases { + let got = render(parse_bulk_any(text)); + if got != *want { + mismatches.push(format!("parse_any({text:?}) -> {got:?}")); + } + } + assert_no_mismatches(&mismatches); +} + +// ---- Google Authenticator migration payload ------------------------------- + +/// One `OtpParameters` message, built field by field. +fn otp_params( + secret: &[u8], + name: &str, + issuer: &str, + algo: u8, + digits: u8, + typ: u8, + counter: u8, +) -> Vec { + let mut m = vec![0x0a, secret.len() as u8]; + m.extend(secret); + m.extend([0x12, name.len() as u8]); + m.extend(name.as_bytes()); + m.extend([0x1a, issuer.len() as u8]); + m.extend(issuer.as_bytes()); + m.extend([0x20, algo, 0x28, digits, 0x30, typ, 0x38, counter]); + m +} + +fn payload(params: &[Vec], batch: Option<(u8, u8)>) -> Vec { + let mut p = Vec::new(); + for m in params { + p.extend([0x0a, m.len() as u8]); + p.extend(m); + } + p.extend([0x10, 1]); + if let Some((index, size)) = batch { + p.extend([0x18, size, 0x20, index]); + } + p +} + +/// TOTP, HOTP (counter 7), SHA-512, 8 digits, MD5 and a 64-byte secret. +fn migration_corpus() -> Vec { + payload( + &[ + otp_params(b"0123456789", "Acme:alice", "Acme", 1, 1, 2, 0), + otp_params(b"counterkey", "Legacy", "", 1, 1, 1, 7), + otp_params(b"0123456789", "wide", "Wide", 3, 1, 2, 0), + otp_params(b"0123456789", "Eight:bob", "", 2, 2, 2, 0), + otp_params(b"0123456789", "old", "Old", 4, 1, 2, 0), + otp_params(&[0x41; 64], "long", "", 1, 1, 2, 0), + ], + Some((1, 3)), + ) +} + +#[test] +fn migration_golden() { + let cases: &[(Vec, &str)] = &[ + ( + migration_corpus(), + r#"Ok Migration { entries: [BulkEntry { issuer: Some("Acme"), account: Some("alice"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }, BulkEntry { issuer: Some("Eight"), account: Some("bob"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds30 }], skipped: [Skipped { label: "Legacy", reason: "HOTP is not supported by the Molto2" }, Skipped { label: "Wide:wide", reason: "only SHA-1 / SHA-256 are supported" }, Skipped { label: "Old:old", reason: "only SHA-1 / SHA-256 are supported" }, Skipped { label: "long", reason: "secret must be 1..=63 bytes" }], batch: Some((1, 3)) }"#, + ), + ( + payload(&[otp_params(b"counterkey", "Legacy", "", 1, 1, 1, 7)], None), + r#"Ok Migration { entries: [], skipped: [Skipped { label: "Legacy", reason: "HOTP is not supported by the Molto2" }], batch: None }"#, + ), + ( + payload(&[], None), + r#"Err unsupported format: migration payload contains no accounts"#, + ), + ( + vec![0x0a, 0xff], + r#"Err unsupported format: malformed migration protobuf"#, + ), + ]; + let mut mismatches = Vec::new(); + for (buf, want) in cases { + let got = render(migration::parse_payload(buf)); + if got != *want { + mismatches.push(format!("parse_payload({buf:02x?}) -> {got:?}")); + } + } + assert_no_mismatches(&mismatches); +} + +// ---- otpauth:// ------------------------------------------------------------ + +#[test] +fn otpauth_golden() { + let cases: &[(&str, &str)] = &[ + ("otpauth://totp/Acme:alice%40example.com?secret=JBSWY3DPEHPK3PXP&issuer=Acme+Co&algorithm=SHA256&digits=8&period=60", r#"Ok OtpAuth { issuer: Some("Acme Co"), account: Some("alice@example.com"), secret: [10 bytes], algorithm: Sha256, digits: Eight, time_step: Seconds60 }"#), + ("otpauth://totp/plain?secret=JBSWY3DPEHPK3PXP", r#"Ok OtpAuth { issuer: None, account: Some("plain"), secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }"#), + ("otpauth://totp/:?secret=JBSWY3DPEHPK3PXP&issuer=", r#"Ok OtpAuth { issuer: None, account: None, secret: [10 bytes], algorithm: Sha1, digits: Six, time_step: Seconds30 }"#), + ("otpauth://hotp/X:a?secret=JBSWY3DPEHPK3PXP&counter=5", r#"Err unsupported OTP type "hotp" (only totp is supported)"#), + ("otpauth://steam/X?secret=JBSWY3DPEHPK3PXP", r#"Err unsupported OTP type "steam" (only totp is supported)"#), + ("otpauth://totp/X?secret=JBSWY3DPEHPK3PXP&period=45", r#"Err period=45s not supported by Molto2 (30 or 60)"#), + ("otpauth://totp/X?secret=JBSWY3DPEHPK3PXP&period=0", r#"Err period=0s not supported by Molto2 (30 or 60)"#), + ("otpauth://totp/X?secret=JBSWY3DPEHPK3PXP&digits=7", r#"Err digits=7 not supported by Molto2 (4, 6, 8, or 10)"#), + ("otpauth://totp/X?secret=JBSWY3DPEHPK3PXP&digits=x", r#"Err malformed URI: digits"#), + ("otpauth://totp/X?secret=JBSWY3DPEHPK3PXP&algorithm=SHA512", r#"Err algorithm "SHA512" not supported by Molto2 (SHA1 or SHA256 only)"#), + ("otpauth://totp/X?secret=JBSWY3DPEHPK3PXP&algorithm=md5", r#"Err algorithm "MD5" not supported by Molto2 (SHA1 or SHA256 only)"#), + ("otpauth://totp/X?secret=NOT_BASE32!!", r#"Err `secret` is not valid base32"#), + ("otpauth://totp/X", r#"Err URI is missing the `secret` parameter"#), + ("otpauth://totp?secret=JBSWY3DPEHPK3PXP", r#"Err malformed URI: missing label"#), + ("otpauth://totp/%ZZ?secret=JBSWY3DPEHPK3PXP", r#"Err malformed URI: label percent-encoding"#), + ("https://example.com", r#"Err not an otpauth:// URI"#), + ]; + let mut mismatches = Vec::new(); + for (uri, want) in cases { + let got = render(otpauth::parse(uri)); + if got != *want { + mismatches.push(format!("otpauth::parse({uri}) -> {got:?}")); + } + } + assert_no_mismatches(&mismatches); +} From 45395fdc5634ba79fe9c41632c170a81e4e58552 Mon Sep 17 00:00:00 2001 From: framefilter Date: Fri, 9 Oct 2026 20:06:54 -0400 Subject: [PATCH 02/35] import: carry HOTP, any TOTP period and SHA-512 in a wide entry model; Molto2 path unchanged (re #170) OATH keys and Token2 OTP storage can hold entries a Molto2 slot can't: HOTP with its counter, TOTP periods other than 30/60 s, SHA-512, other digit counts and longer seeds. The existing parsers drop or reject those before any key is chosen, and stop a whole file at the first entry the Molto2 can't take. Add a format-neutral ImportEntry/ImportFile model and wide parsers for otpauth:// URIs, Aegis, 2FAS, otpauth lists (now also accepting Google Authenticator migration lines), migration payloads and QR screenshots. Every entry in a file becomes an item, in file order, either usable or carrying a plain reason (Steam/mOTP/Yandex, MD5, a bad secret, ...); only bad JSON, an encrypted export, or an empty file fail the whole file. Per-key limits are applied later, against the chosen destination. The narrow Molto2 parsers keep their own mapping and error order; the JSON structs are shared, and the new counter fields read leniently so a file the Molto2 path accepted can't start failing. The golden tests from the previous commit pass unchanged. Seeds stay in wipe-on-drop buffers and Debug shows only their length. The wide parsers join the fuzz targets. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU --- crates/keyroost-import/src/bulk.rs | 449 +++++++++++++++++++++++- crates/keyroost-import/src/entry.rs | 353 +++++++++++++++++++ crates/keyroost-import/src/lib.rs | 12 +- crates/keyroost-import/src/migration.rs | 264 ++++++++++++-- crates/keyroost-import/src/otpauth.rs | 198 ++++++++++- crates/keyroost-qr/src/lib.rs | 127 ++++++- crates/keyroost-qr/tests/roundtrip.rs | 29 ++ fuzz/fuzz_targets/bulk_import.rs | 5 + fuzz/fuzz_targets/migration.rs | 2 + fuzz/fuzz_targets/otpauth.rs | 1 + fuzz/fuzz_targets/qr_image.rs | 1 + 11 files changed, 1388 insertions(+), 53 deletions(-) create mode 100644 crates/keyroost-import/src/entry.rs diff --git a/crates/keyroost-import/src/bulk.rs b/crates/keyroost-import/src/bulk.rs index 3b08201e..81e8f71c 100644 --- a/crates/keyroost-import/src/bulk.rs +++ b/crates/keyroost-import/src/bulk.rs @@ -6,6 +6,10 @@ use keyroost_proto::commands::{DisplayTimeout, HmacAlgo, OtpDigits, ProfileConfig, TimeStep}; use serde::Deserialize; +use crate::entry::{ + decode_secret, finish, hash_from_name, name_label, type_from_name, Format, ImportEntry, + ImportFile, ImportItem, OtpKind, TypeName, Unusable, +}; use crate::otpauth::{parse as parse_otpauth, OtpAuth, OtpAuthError}; /// One normalized entry ready to be programmed into a Molto2 profile slot. @@ -184,6 +188,10 @@ pub mod aegis { algo: Option, digits: Option, period: Option, + /// HOTP counter; read only by the wide mapping. Lenient so a value + /// of an unexpected shape never fails a file the Molto2 path reads. + #[serde(default, deserialize_with = "super::lenient_u64")] + counter: Option, } // `build_entry` copies the seed into a zeroize-on-drop `BulkEntry`; wipe @@ -195,25 +203,24 @@ pub mod aegis { } } - pub fn parse(json: &str) -> Result, BulkError> { + /// Read the plaintext `db`, or the encrypted-vault error. Shared by the + /// narrow (Molto2) and wide mappings. + fn read_db(json: &str) -> Result { let root: Root = serde_json::from_str(json)?; - let db: Db = match root.db { - DbField::Plain(db) => db, + match root.db { + DbField::Plain(db) => Ok(db), DbField::Encrypted => { #[cfg(feature = "encrypted")] - { - return Err(BulkError::Encrypted( - "Aegis export is encrypted; supply a password (CLI: --password env:NAME or --password stdin)", - )); - } + let msg = "Aegis export is encrypted; supply a password (CLI: --password env:NAME or --password stdin)"; #[cfg(not(feature = "encrypted"))] - { - return Err(BulkError::Encrypted( - "Aegis export is encrypted; build with --features encrypted or re-export plaintext", - )); - } + let msg = "Aegis export is encrypted; build with --features encrypted or re-export plaintext"; + Err(BulkError::Encrypted(msg)) } - }; + } + } + + pub fn parse(json: &str) -> Result, BulkError> { + let db = read_db(json)?; let mut out = Vec::with_capacity(db.entries.len()); for (i, e) in db.entries.into_iter().enumerate() { if !e.typ.eq_ignore_ascii_case("totp") { @@ -236,6 +243,45 @@ pub mod aegis { Ok(out) } + /// Every entry of a plaintext Aegis export, in file order, each either + /// usable by some key or marked with why not. Whole-file errors: bad + /// JSON, an encrypted vault (decrypt first), or no entries at all. + pub fn parse_wide(json: &str) -> Result { + let db = read_db(json)?; + let items: Vec = db + .entries + .into_iter() + .enumerate() + .map(|(i, e)| { + let label = name_label(e.issuer.as_deref(), e.name.as_deref()) + .unwrap_or_else(|| format!("entry {}", i + 1)); + let entry = wide_entry( + &e.typ, + e.issuer.clone(), + e.name.clone(), + &e.info.secret, + e.info.algo.as_deref(), + e.info.digits.unwrap_or(6), + e.info.period.unwrap_or(30), + e.info.counter.unwrap_or(0), + ); + ImportItem { + index: i, + label, + entry, + } + }) + .collect(); + if items.is_empty() { + return Err(BulkError::EmptyFile); + } + Ok(ImportFile { + format: Format::Aegis, + items, + batch: None, + }) + } + /// Detect an encrypted Aegis vault without parsing the whole structure. /// `Ok(true)` means an `encrypted` decrypt is needed; `Ok(false)` means /// the file is already plaintext. Cheap and side-effect-free: the `db` @@ -322,15 +368,25 @@ pub mod twofas { algorithm: Option, #[serde(rename = "tokenType")] token_type: Option, + /// HOTP counter; read only by the wide mapping (lenient, as for + /// Aegis). + #[serde(default, deserialize_with = "super::lenient_u64")] + counter: Option, } - pub fn parse(json: &str) -> Result, BulkError> { + /// Read the plaintext `services`, or the encrypted-export error. + fn read_root(json: &str) -> Result { let root: Root = serde_json::from_str(json)?; if root.services_encrypted.is_some() && root.services.is_empty() { return Err(BulkError::Encrypted( "2FAS export is encrypted; re-export without a password", )); } + Ok(root) + } + + pub fn parse(json: &str) -> Result, BulkError> { + let root = read_root(json)?; let mut out = Vec::with_capacity(root.services.len()); for (i, mut s) in root.services.into_iter().enumerate() { // 2FAS includes Steam etc; skip non-TOTP. @@ -357,6 +413,47 @@ pub mod twofas { } Ok(out) } + + /// Every service of a plaintext 2FAS export, in file order (see + /// [`aegis::parse_wide`] for the contract). + pub fn parse_wide(json: &str) -> Result { + let root = read_root(json)?; + let items: Vec = root + .services + .iter() + .enumerate() + .map(|(i, s)| { + // Same issuer rule as the narrow mapping: the otp issuer, + // else the service name. + let issuer = s.otp.issuer.clone().or_else(|| s.name.clone()); + let label = name_label(issuer.as_deref(), s.otp.account.as_deref()) + .unwrap_or_else(|| format!("entry {}", i + 1)); + let entry = wide_entry( + s.otp.token_type.as_deref().unwrap_or("TOTP"), + issuer, + s.otp.account.clone(), + &s.secret, + s.otp.algorithm.as_deref(), + s.otp.digits.unwrap_or(6), + s.otp.period.unwrap_or(30), + s.otp.counter.unwrap_or(0), + ); + ImportItem { + index: i, + label, + entry, + } + }) + .collect(); + if items.is_empty() { + return Err(BulkError::EmptyFile); + } + Ok(ImportFile { + format: Format::TwoFas, + items, + batch: None, + }) + } } /// One otpauth:// URI per line; blank lines and `#` comments allowed. Useful @@ -378,6 +475,146 @@ pub fn parse_otpauth_list(text: &str) -> Result, BulkError> { Ok(out) } +/// One entry per line, like [`parse_otpauth_list`], but nothing a line +/// holds fails the file: each non-blank, non-`#` line becomes an item (index +/// = line index), unusable lines carrying their reason. A Google +/// Authenticator `otpauth-migration://` line contributes all of its entries +/// under that line's index. Only an empty list is a whole-file error. +pub fn parse_list_wide(text: &str) -> Result { + let mut items = Vec::new(); + let mut batch = None; + for (i, raw) in text.lines().enumerate() { + let line = raw.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + if crate::migration::is_migration_uri(line) { + match crate::migration::parse_wide(line) { + Ok(file) => { + batch = batch.or(file.batch); + items.extend(file.items.into_iter().map(|mut item| { + item.index = i; + item + })); + } + Err(_) => items.push(ImportItem { + index: i, + label: format!("line {}", i + 1), + entry: Err(Unusable::Malformed( + "the Google Authenticator export couldn't be read", + )), + }), + } + continue; + } + let (name, entry) = crate::otpauth::parse_entry_labeled(line); + items.push(ImportItem { + index: i, + label: name.unwrap_or_else(|| format!("line {}", i + 1)), + entry, + }); + } + if items.is_empty() { + return Err(BulkError::EmptyFile); + } + Ok(ImportFile { + format: Format::OtpauthList, + items, + batch, + }) +} + +/// The wide counterpart of [`parse_any`]: JSON goes to Aegis (an encrypted +/// vault is reported as such), then 2FAS; anything else is read as a list. +pub fn parse_any_wide(text: &str) -> Result { + if text.trim_start().starts_with('{') { + match aegis::parse_wide(text) { + Ok(f) => return Ok(f), + Err(e @ BulkError::Encrypted(_)) => return Err(e), + Err(_) => {} + } + return twofas::parse_wide(text); + } + parse_list_wide(text) +} + +/// Map one JSON-export entry to the wide model. Checks run type, secret, +/// hash, digits, period — the Molto2 path's order with the type first. +#[allow(clippy::too_many_arguments)] +fn wide_entry( + typ: &str, + issuer: Option, + account: Option, + secret_b32: &str, + algo: Option<&str>, + digits: u32, + period: u32, + counter: u64, +) -> Result { + let kind = type_from_name(typ)?; + let secret = decode_secret(secret_b32)?; + let hash = hash_from_name(algo.unwrap_or("SHA1"))?; + let kind = match kind { + TypeName::Totp => OtpKind::Totp { period }, + TypeName::Hotp => OtpKind::Hotp { counter }, + }; + finish(issuer, account, secret, hash, digits, kind) +} + +/// `Option` from any JSON value: a non-negative integer becomes `Some`, +/// anything else (string, float, null, ...) `None` — never an error. +fn lenient_u64<'de, D: serde::Deserializer<'de>>(d: D) -> Result, D::Error> { + struct V; + impl<'de> serde::de::Visitor<'de> for V { + type Value = Option; + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("any JSON value") + } + fn visit_u64(self, v: u64) -> Result { + Ok(Some(v)) + } + fn visit_i64(self, v: i64) -> Result { + Ok(u64::try_from(v).ok()) + } + fn visit_f64(self, _: f64) -> Result { + Ok(None) + } + fn visit_bool(self, _: bool) -> Result { + Ok(None) + } + fn visit_str(self, _: &str) -> Result { + Ok(None) + } + fn visit_unit(self) -> Result { + Ok(None) + } + fn visit_none(self) -> Result { + Ok(None) + } + fn visit_some>(self, d: D) -> Result { + d.deserialize_any(self) + } + fn visit_seq>( + self, + mut a: A, + ) -> Result { + while a.next_element::()?.is_some() {} + Ok(None) + } + fn visit_map>( + self, + mut a: A, + ) -> Result { + while a + .next_entry::()? + .is_some() + {} + Ok(None) + } + } + d.deserialize_any(V) +} + /// Try each known format on `bytes` and return the first that parses. pub fn parse_any(bytes: &str) -> Result, BulkError> { if bytes.trim_start().starts_with('{') { @@ -562,4 +799,186 @@ mod tests { assert_eq!(parse_any(twofas_like).unwrap().len(), 1); assert_eq!(parse_any(list).unwrap().len(), 1); } + + // ---- wide parsers ---------------------------------------------------- + + use crate::entry::HashAlg; + + /// The same corpus as `tests/molto_golden.rs`, in the same order. + const WIDE_AEGIS: &str = r#"{"version":1,"header":{"slots":null,"params":null},"db":{"version":2,"entries":[ + {"type":"totp","name":"alice@example.com","issuer":"GitHub","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":6,"period":30}}, + {"type":"hotp","name":"counter","issuer":"Bank","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":6,"counter":5}}, + {"type":"steam","name":"gamer","issuer":"Steam","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":5,"period":30}}, + {"type":"totp","name":"seven","issuer":"Odd","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":7,"period":30}}, + {"type":"totp","name":"slow","issuer":"Slow","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA1","digits":6,"period":45}}, + {"type":"totp","name":"wide","issuer":"Wide","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA512","digits":6,"period":30}}, + {"type":"totp","name":"bad","issuer":"Bad","info":{"secret":"NOT_BASE32!!","algo":"SHA1","digits":6,"period":30}}, + {"type":"totp","name":"long","issuer":"Long","info":{"secret":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","algo":"SHA1","digits":6,"period":30}}, + {"type":"totp","name":"bob","issuer":"","info":{"secret":"JBSWY3DPEHPK3PXP","algo":"SHA256","digits":8,"period":60}} + ]}}"#; + + /// `(index, label, kind or unusable reason)` per item, for compact asserts. + fn shape(file: &crate::entry::ImportFile) -> Vec<(usize, String, Result)> { + file.items + .iter() + .map(|i| { + ( + i.index, + i.label.clone(), + i.entry.as_ref().map(|e| e.kind).map_err(Clone::clone), + ) + }) + .collect() + } + + #[test] + fn aegis_wide_keeps_every_entry_in_order() { + let f = aegis::parse_wide(WIDE_AEGIS).unwrap(); + assert_eq!(f.format, Format::Aegis); + assert_eq!(f.batch, None); + let totp30 = Ok(OtpKind::Totp { period: 30 }); + assert_eq!( + shape(&f), + vec![ + (0, "GitHub:alice@example.com".into(), totp30.clone()), + (1, "Bank:counter".into(), Ok(OtpKind::Hotp { counter: 5 })), + (2, "Steam:gamer".into(), Err(Unusable::Type("steam".into()))), + (3, "Odd:seven".into(), totp30.clone()), + (4, "Slow:slow".into(), Ok(OtpKind::Totp { period: 45 })), + (5, "Wide:wide".into(), totp30.clone()), + (6, "Bad:bad".into(), Err(Unusable::BadSecret)), + (7, "Long:long".into(), totp30), + (8, "bob".into(), Ok(OtpKind::Totp { period: 60 })), + ] + ); + let e = |i: usize| f.items[i].entry.as_ref().unwrap(); + assert_eq!(e(3).digits, 7); + assert_eq!(e(5).hash, HashAlg::Sha512); + assert_eq!(e(7).secret.len(), 64); + assert_eq!(e(8).issuer, None); + assert_eq!(e(8).hash, HashAlg::Sha256); + assert_eq!(e(8).digits, 8); + assert_eq!(f.usable(), 7); + } + + #[test] + fn aegis_wide_file_errors() { + assert!(matches!( + aegis::parse_wide(r#"{"db":"ZW5jcnlwdGVk","header":{}}"#), + Err(BulkError::Encrypted(_)) + )); + assert!(matches!( + aegis::parse_wide(r#"{"db":{"entries":[]}}"#), + Err(BulkError::EmptyFile) + )); + assert!(matches!(aegis::parse_wide("{"), Err(BulkError::Json(_)))); + // A file of only unusable entries is still a file of items. + let f = aegis::parse_wide( + r#"{"db":{"entries":[{"type":"motp","name":"","issuer":"","info":{"secret":"JBSWY3DP"}}]}}"#, + ) + .unwrap(); + assert_eq!(f.usable(), 0); + assert_eq!( + shape(&f), + vec![(0, "entry 1".into(), Err(Unusable::Type("motp".into())))] + ); + } + + #[test] + fn twofas_wide_reads_hotp_counter() { + let json = r#"{"services":[ + {"name":"Bank","secret":"JBSWY3DPEHPK3PXP","otp":{"account":"me","issuer":"Bank","digits":6,"counter":3,"algorithm":"SHA1","tokenType":"HOTP"}}, + {"name":"Mail","secret":"JBSWY3DPEHPK3PXP","otp":{"account":"you","period":20,"algorithm":"SHA512"}}, + {"name":"Steam","secret":"JBSWY3DPEHPK3PXP","otp":{"tokenType":"STEAM"}} + ]}"#; + let f = twofas::parse_wide(json).unwrap(); + assert_eq!(f.format, Format::TwoFas); + assert_eq!( + shape(&f), + vec![ + (0, "Bank:me".into(), Ok(OtpKind::Hotp { counter: 3 })), + (1, "Mail:you".into(), Ok(OtpKind::Totp { period: 20 })), + (2, "Steam".into(), Err(Unusable::Type("STEAM".into()))), + ] + ); + assert_eq!(f.items[1].entry.as_ref().unwrap().hash, HashAlg::Sha512); + assert!(matches!( + twofas::parse_wide(r#"{"servicesEncrypted":"x","services":[]}"#), + Err(BulkError::Encrypted(_)) + )); + assert!(matches!( + twofas::parse_wide(r#"{"services":[]}"#), + Err(BulkError::EmptyFile) + )); + } + + #[test] + fn list_wide_line_errors_are_items_not_file_errors() { + let text = "# header\n\notpauth://totp/A:x?secret=JBSWY3DPEHPK3PXP&period=45\n \notpauth://hotp/B?secret=JBSWY3DPEHPK3PXP&counter=2\nhello there\notpauth://totp/C:y?secret=JBSWY3DP&digits=11\n"; + let f = parse_list_wide(text).unwrap(); + assert_eq!(f.format, Format::OtpauthList); + assert_eq!( + shape(&f), + vec![ + (2, "A:x".into(), Ok(OtpKind::Totp { period: 45 })), + (4, "B".into(), Ok(OtpKind::Hotp { counter: 2 })), + ( + 5, + "line 6".into(), + Err(Unusable::Malformed("not an otpauth:// URI")) + ), + (6, "C:y".into(), Err(Unusable::Digits(11))), + ] + ); + assert!(matches!( + parse_list_wide("# nothing\n\n"), + Err(BulkError::EmptyFile) + )); + } + + #[test] + fn list_wide_accepts_migration_lines() { + let migration = crate::migration::tests::uri_for(&crate::migration::tests::payload(&[ + crate::migration::tests::otp_params(b"0123456789", "Acme:alice", "Acme", 1, 1, 2), + crate::migration::tests::otp_params(b"counterkey", "Legacy", "", 1, 1, 1), + ])); + let text = format!( + "otpauth://totp/A?secret=JBSWY3DPEHPK3PXP\n{}\notpauth-migration://offline?data=!!!\n", + migration + ); + let f = parse_list_wide(&text).unwrap(); + assert_eq!( + shape(&f), + vec![ + (0, "A".into(), Ok(OtpKind::Totp { period: 30 })), + (1, "Acme:alice".into(), Ok(OtpKind::Totp { period: 30 })), + (1, "Legacy".into(), Ok(OtpKind::Hotp { counter: 0 })), + ( + 2, + "line 3".into(), + Err(Unusable::Malformed( + "the Google Authenticator export couldn't be read" + )) + ), + ] + ); + } + + #[test] + fn parse_any_wide_routes_by_shape() { + let aegis_like = r#"{"db": {"entries":[{"type":"hotp","name":"a","issuer":"X","info":{"secret":"JBSWY3DP","counter":1}}]}}"#; + let twofas_like = r#"{"services":[{"name":"X","secret":"JBSWY3DP","otp":{}}]}"#; + assert_eq!(parse_any_wide(aegis_like).unwrap().format, Format::Aegis); + assert_eq!(parse_any_wide(twofas_like).unwrap().format, Format::TwoFas); + assert_eq!( + parse_any_wide("otpauth://hotp/X?secret=JBSWY3DP") + .unwrap() + .format, + Format::OtpauthList + ); + assert!(matches!( + parse_any_wide(r#"{"db":"ZW5j"}"#), + Err(BulkError::Encrypted(_)) + )); + } } diff --git a/crates/keyroost-import/src/entry.rs b/crates/keyroost-import/src/entry.rs new file mode 100644 index 00000000..c8d61682 --- /dev/null +++ b/crates/keyroost-import/src/entry.rs @@ -0,0 +1,353 @@ +//! Format-neutral authenticator entries: everything an export can carry, +//! before any key's limits are applied. Destinations filter these against +//! their own rules; the Molto2 path keeps its own narrow `BulkEntry`. + +use zeroize::Zeroizing; + +/// Longest decoded seed an entry may carry. Generous on purpose: per-key +/// limits are applied later, against the destination, not here. +pub(crate) const MAX_SECRET_BYTES: usize = 255; + +/// HMAC hash named by the entry. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum HashAlg { + Sha1, + Sha256, + Sha512, +} + +/// Time- or counter-based, with the parameter that goes with it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OtpKind { + /// TOTP with its period in seconds (>= 1). + Totp { period: u32 }, + /// HOTP with the next counter value from the export. + Hotp { counter: u64 }, +} + +/// One usable entry. The seed is wiped on drop. +pub struct ImportEntry { + /// Issuer; an empty string in the file becomes `None`. + pub issuer: Option, + /// Account name; an empty string in the file becomes `None`. + pub account: Option, + /// Decoded seed, 1..=255 bytes. + pub secret: Zeroizing>, + pub hash: HashAlg, + /// Code length, 1..=10. + pub digits: u8, + pub kind: OtpKind, +} + +/// Manual Debug so a stray `{:?}` can't print the seed; only its length is +/// shown. +impl core::fmt::Debug for ImportEntry { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("ImportEntry") + .field("issuer", &self.issuer) + .field("account", &self.account) + .field("secret", &format_args!("[{} bytes]", self.secret.len())) + .field("hash", &self.hash) + .field("digits", &self.digits) + .field("kind", &self.kind) + .finish() + } +} + +/// Why an entry in a file can't go to any key. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Unusable { + /// Missing, empty, over 255 bytes, or not valid base32. + BadSecret, + /// Steam, mOTP, Yandex, ... — the type as the file names it. + Type(String), + /// MD5 or a name keyroost doesn't know. + Hash(String), + /// Digits outside 1..=10. + Digits(u32), + /// Period of 0. + Period(u32), + /// A list line that isn't an otpauth:// URI, or a URI missing a required part. + Malformed(&'static str), +} + +impl core::fmt::Display for Unusable { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Unusable::BadSecret => { + write!(f, "the secret is missing, too long or isn't valid base32") + } + Unusable::Type(t) => { + let name = match t.to_ascii_lowercase().as_str() { + "steam" => "Steam".to_owned(), + "motp" => "mOTP".to_owned(), + "yandex" => "Yandex".to_owned(), + _ => t.clone(), + }; + write!(f, "{} codes aren't supported", name) + } + Unusable::Hash(h) => write!(f, "{} isn't supported", h), + Unusable::Digits(d) => write!(f, "{}-digit codes aren't supported", d), + Unusable::Period(p) => write!(f, "a {}-second period isn't supported", p), + Unusable::Malformed(what) => write!(f, "{}", what), + } + } +} + +impl std::error::Error for Unusable {} + +/// One entry of a file, in file order. +#[derive(Debug)] +pub struct ImportItem { + /// Position in the file: the entry index for JSON formats, the line index + /// for lists (the same index the narrow path reports in + /// `BulkError::EntryRejected`). + pub index: usize, + /// "issuer:account" as the file names it, for display only (callers + /// sanitize before printing). "line N" / "entry N" (counting from 1) + /// when there is no name. + pub label: String, + pub entry: Result, +} + +/// Which kind of file the entries came from. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Format { + Aegis, + AegisEncrypted, + TwoFas, + OtpauthList, + QrImage, + GoogleExport, +} + +/// Everything read from one file. +#[derive(Debug)] +pub struct ImportFile { + pub format: Format, + pub items: Vec, + /// `(index, total)` when this is one QR of a multi-QR Google export. + pub batch: Option<(u32, u32)>, +} + +impl ImportFile { + /// How many items carry an entry some key could take. + pub fn usable(&self) -> usize { + self.items.iter().filter(|i| i.entry.is_ok()).count() + } +} + +/// `"issuer:account"`, or whichever of the two is present, or `None`. +pub(crate) fn name_label(issuer: Option<&str>, account: Option<&str>) -> Option { + let issuer = issuer.filter(|s| !s.is_empty()); + let account = account.filter(|s| !s.is_empty()); + match (issuer, account) { + (Some(i), Some(a)) => Some(format!("{}:{}", i, a)), + (Some(one), None) | (None, Some(one)) => Some(one.to_owned()), + (None, None) => None, + } +} + +/// Map a hash name as exports spell it (case-insensitive). +pub(crate) fn hash_from_name(name: &str) -> Result { + match name.to_ascii_uppercase().as_str() { + "SHA1" => Ok(HashAlg::Sha1), + "SHA256" => Ok(HashAlg::Sha256), + "SHA512" => Ok(HashAlg::Sha512), + other => Err(Unusable::Hash(other.to_owned())), + } +} + +/// The kind a type name stands for, before its parameter is known. +pub(crate) enum TypeName { + Totp, + Hotp, +} + +/// `totp` / `hotp` (case-insensitive); anything else is unusable, named as +/// the file names it. +pub(crate) fn type_from_name(name: &str) -> Result { + if name.eq_ignore_ascii_case("totp") { + Ok(TypeName::Totp) + } else if name.eq_ignore_ascii_case("hotp") { + Ok(TypeName::Hotp) + } else { + Err(Unusable::Type(name.to_owned())) + } +} + +/// Decode a base32 seed into a wipe-on-drop buffer within the wide range. +pub(crate) fn decode_secret(secret_b32: &str) -> Result>, Unusable> { + let secret = Zeroizing::new( + keyroost_proto::codec::base32_decode(secret_b32).map_err(|_| Unusable::BadSecret)?, + ); + check_secret(secret) +} + +/// Enforce the wide seed range on raw bytes. +pub(crate) fn check_secret(secret: Zeroizing>) -> Result>, Unusable> { + if secret.is_empty() || secret.len() > MAX_SECRET_BYTES { + return Err(Unusable::BadSecret); + } + Ok(secret) +} + +/// Validate digits and the kind's parameter, in that order, and assemble the +/// entry. The secret and hash are already checked by the caller (secret +/// first, then hash — the same order the Molto2 path reports problems in). +pub(crate) fn finish( + issuer: Option, + account: Option, + secret: Zeroizing>, + hash: HashAlg, + digits: u32, + kind: OtpKind, +) -> Result { + if !(1..=10).contains(&digits) { + return Err(Unusable::Digits(digits)); + } + if let OtpKind::Totp { period: 0 } = kind { + return Err(Unusable::Period(0)); + } + Ok(ImportEntry { + issuer: issuer.filter(|s| !s.is_empty()), + account: account.filter(|s| !s.is_empty()), + secret, + hash, + digits: digits as u8, + kind, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample() -> ImportEntry { + ImportEntry { + issuer: Some("Acme".into()), + account: Some("alice".into()), + // "Hello!\xde\xad\xbe\xef" plus six more bytes: 16 in all. + secret: Zeroizing::new(b"Hello!\xde\xad\xbe\xef012345".to_vec()), + hash: HashAlg::Sha1, + digits: 6, + kind: OtpKind::Totp { period: 30 }, + } + } + + #[test] + fn debug_never_shows_secret() { + let e = sample(); + let dbg = format!("{:?}", e); + assert!(dbg.contains("[16 bytes]"), "{dbg}"); + // Neither the base32 nor the hex spelling of the seed, nor its bytes. + assert!(!dbg.contains("JBSWY3DP"), "{dbg}"); + assert!(!dbg.to_ascii_lowercase().contains("48656c6c6f"), "{dbg}"); + assert!(!dbg.contains("Hello"), "{dbg}"); + assert!(!dbg.contains("72, 101"), "{dbg}"); + // The item wrapper's Debug goes through the same impl. + let item = ImportItem { + index: 0, + label: "Acme:alice".into(), + entry: Ok(sample()), + }; + assert!(format!("{:?}", item).contains("[16 bytes]")); + } + + #[test] + fn secret_over_255_bytes_is_bad_secret() { + assert!(check_secret(Zeroizing::new(vec![1u8; 255])).is_ok()); + assert_eq!( + check_secret(Zeroizing::new(vec![1u8; 256])).unwrap_err(), + Unusable::BadSecret + ); + assert_eq!( + check_secret(Zeroizing::new(Vec::new())).unwrap_err(), + Unusable::BadSecret + ); + // 408 base32 chars decode to 255 bytes; 410 to 256. + assert_eq!(decode_secret(&"A".repeat(408)).unwrap().len(), 255); + assert_eq!( + decode_secret(&"A".repeat(410)).unwrap_err(), + Unusable::BadSecret + ); + } + + #[test] + fn unusable_reasons_read_plainly() { + assert_eq!( + Unusable::Type("steam".into()).to_string(), + "Steam codes aren't supported" + ); + assert_eq!( + Unusable::Type("MOTP".into()).to_string(), + "mOTP codes aren't supported" + ); + assert_eq!( + Unusable::Type("yandex".into()).to_string(), + "Yandex codes aren't supported" + ); + assert_eq!( + Unusable::BadSecret.to_string(), + "the secret is missing, too long or isn't valid base32" + ); + assert_eq!( + Unusable::Hash("MD5".into()).to_string(), + "MD5 isn't supported" + ); + assert_eq!( + Unusable::Digits(7).to_string(), + "7-digit codes aren't supported" + ); + assert_eq!( + Unusable::Period(0).to_string(), + "a 0-second period isn't supported" + ); + } + + #[test] + fn finish_checks_digits_then_period() { + let s = || Zeroizing::new(vec![1u8; 10]); + let totp0 = OtpKind::Totp { period: 0 }; + assert_eq!( + finish(None, None, s(), HashAlg::Sha1, 11, totp0).unwrap_err(), + Unusable::Digits(11) + ); + assert_eq!( + finish(None, None, s(), HashAlg::Sha1, 6, totp0).unwrap_err(), + Unusable::Period(0) + ); + let e = finish( + Some(String::new()), + Some("a".into()), + s(), + HashAlg::Sha512, + 10, + OtpKind::Hotp { counter: 0 }, + ) + .unwrap(); + assert_eq!(e.issuer, None); + assert_eq!(e.digits, 10); + } + + #[test] + fn usable_counts_ok_items() { + let file = ImportFile { + format: Format::OtpauthList, + items: vec![ + ImportItem { + index: 0, + label: "a".into(), + entry: Ok(sample()), + }, + ImportItem { + index: 1, + label: "line 2".into(), + entry: Err(Unusable::BadSecret), + }, + ], + batch: None, + }; + assert_eq!(file.usable(), 1); + } +} diff --git a/crates/keyroost-import/src/lib.rs b/crates/keyroost-import/src/lib.rs index 4079c781..acb041bf 100644 --- a/crates/keyroost-import/src/lib.rs +++ b/crates/keyroost-import/src/lib.rs @@ -2,16 +2,26 @@ //! (with the `bulk` feature) Aegis / 2FAS plaintext JSON parsers. //! The `encrypted` feature additionally pulls in scrypt + AES-GCM for //! decrypting password-protected Aegis vaults. +//! +//! Two layers of parsers sit side by side. The narrow ones (`parse`, +//! `aegis::parse`, ...) produce `BulkEntry` values limited to what a Molto2 +//! slot stores. The wide ones (`parse_entry`, `aegis::parse_wide`, ...) +//! produce an [`ImportFile`] of every entry in the file — HOTP, any TOTP +//! period, SHA-512 — each marked usable or with the reason it isn't, for +//! keys whose limits are checked later. +pub mod entry; pub mod otpauth; +pub use entry::{Format, HashAlg, ImportEntry, ImportFile, ImportItem, OtpKind, Unusable}; pub use otpauth::{parse as parse_otpauth, OtpAuth, OtpAuthError}; #[cfg(feature = "bulk")] pub mod bulk; #[cfg(feature = "bulk")] pub use bulk::{ - aegis, parse_any as parse_bulk_any, parse_otpauth_list, twofas, BulkEntry, BulkError, + aegis, parse_any as parse_bulk_any, parse_any_wide, parse_list_wide, parse_otpauth_list, + twofas, BulkEntry, BulkError, }; /// Google Authenticator `otpauth-migration://` export parsing. Lives under diff --git a/crates/keyroost-import/src/migration.rs b/crates/keyroost-import/src/migration.rs index c5b5b494..4927a386 100644 --- a/crates/keyroost-import/src/migration.rs +++ b/crates/keyroost-import/src/migration.rs @@ -29,6 +29,10 @@ //! accounts shouldn't lose 19 because one is HOTP. use crate::bulk::{BulkEntry, BulkError}; +use crate::entry::{ + check_secret, finish, name_label, Format, HashAlg, ImportEntry, ImportFile, ImportItem, + OtpKind, Unusable, +}; use keyroost_proto::codec::base64_decode; use keyroost_proto::commands::{HmacAlgo, OtpDigits, TimeStep}; @@ -57,6 +61,11 @@ pub fn is_migration_uri(uri: &str) -> bool { /// Parse an `otpauth-migration://offline?data=…` URI. pub fn parse(uri: &str) -> Result { + parse_payload(&decode_uri(uri)?) +} + +/// The raw protobuf payload of a migration URI, in a wipe-on-drop buffer. +fn decode_uri(uri: &str) -> Result>, BulkError> { let uri = uri.trim(); let rest = uri.strip_prefix("otpauth-migration://offline") @@ -85,7 +94,117 @@ pub fn parse(uri: &str) -> Result { base64_decode(&data) .map_err(|_| BulkError::UnsupportedFormat("migration data is not valid base64"))?, ); - parse_payload(&payload) + Ok(payload) +} + +/// Every account of an `otpauth-migration://offline?data=…` URI, in +/// payload order, in the wide model: HOTP keeps its counter (field 7) and +/// SHA-512 is carried; MD5, unknown enums and out-of-range secrets become +/// unusable items. Whole-file errors are the same as [`parse`]'s. +pub fn parse_wide(uri: &str) -> Result { + parse_payload_wide(&decode_uri(uri)?) +} + +/// [`parse_wide`] over the raw protobuf `MigrationPayload`. +pub fn parse_payload_wide(buf: &[u8]) -> Result { + let mut items = Vec::new(); + let (mut batch_size, mut batch_index) = (None, None); + let mut r = Reader(buf); + while let Some((field, wire)) = r.key()? { + match (field, wire) { + (1, 2) => { + let index = items.len(); + let (label, entry) = convert_wide(r.bytes()?)?; + items.push(ImportItem { + index, + label: label.unwrap_or_else(|| format!("entry {}", index + 1)), + entry, + }); + } + (3, 0) => batch_size = Some(r.varint()? as u32), + (4, 0) => batch_index = Some(r.varint()? as u32), + _ => r.skip(wire)?, + } + } + if items.is_empty() { + return Err(BulkError::UnsupportedFormat( + "migration payload contains no accounts", + )); + } + Ok(ImportFile { + format: Format::GoogleExport, + items, + batch: batch_of(batch_index, batch_size), + }) +} + +fn batch_of(index: Option, size: Option) -> Option<(u32, u32)> { + match (index, size) { + (Some(i), Some(n)) if n > 1 => Some((i, n)), + _ => None, + } +} + +/// One `OtpParameters` message in the wide model, with its display name. +#[allow(clippy::type_complexity)] +fn convert_wide(buf: &[u8]) -> Result<(Option, Result), BulkError> { + let mut secret: &[u8] = &[]; + let mut name = String::new(); + let mut issuer = String::new(); + let (mut algorithm, mut digits, mut typ, mut counter) = (0u64, 0u64, 0u64, 0u64); + let mut r = Reader(buf); + while let Some((field, wire)) = r.key()? { + match (field, wire) { + (1, 2) => secret = r.bytes()?, + (2, 2) => name = String::from_utf8_lossy(r.bytes()?).into_owned(), + (3, 2) => issuer = String::from_utf8_lossy(r.bytes()?).into_owned(), + (4, 0) => algorithm = r.varint()?, + (5, 0) => digits = r.varint()?, + (6, 0) => typ = r.varint()?, + (7, 0) => counter = r.varint()?, + _ => r.skip(wire)?, + } + } + + // "Issuer:account" names: prefer the explicit issuer field. + let (label_issuer, account) = crate::otpauth::split_label(&name); + let issuer = if issuer.is_empty() { + label_issuer + } else { + Some(issuer) + } + .filter(|s| !s.is_empty()); + let account = account.filter(|s| !s.is_empty()); + let label = name_label(issuer.as_deref(), account.as_deref()); + + let entry = (|| { + // type: 0 (unspecified) is TOTP, matching other importers. + let kind = match typ { + 0 | 2 => OtpKind::Totp { period: 30 }, // GA exports are always 30 s + 1 => OtpKind::Hotp { counter }, + other => return Err(Unusable::Type(format!("type {}", other))), + }; + let secret = check_secret(zeroize::Zeroizing::new(secret.to_vec()))?; + let hash = match algorithm { + 0 | 1 => HashAlg::Sha1, // unspecified defaults to SHA-1 + 2 => HashAlg::Sha256, + 3 => HashAlg::Sha512, + 4 => return Err(Unusable::Hash("MD5".to_owned())), + other => return Err(Unusable::Hash(format!("algorithm {}", other))), + }; + // An enum, not a count: 1 = six, 2 = eight. + let digits = match digits { + 0 | 1 => 6, + 2 => 8, + _ => { + return Err(Unusable::Malformed( + "the digit count isn't one keyroost knows", + )) + } + }; + finish(issuer, account, secret, hash, digits, kind) + })(); + Ok((label, entry)) } /// Parse the raw protobuf `MigrationPayload`. @@ -110,10 +229,7 @@ pub fn parse_payload(buf: &[u8]) -> Result { } } - let batch = match (batch_index, batch_size) { - (Some(i), Some(n)) if n > 1 => Some((i, n)), - _ => None, - }; + let batch = batch_of(batch_index, batch_size); if entries.is_empty() && skipped.is_empty() { return Err(BulkError::UnsupportedFormat( "migration payload contains no accounts", @@ -267,12 +383,12 @@ fn percent_decode_keep_plus(s: &str) -> Option { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; /// Hand-built protobuf: the test constructs the bytes field-by-field so /// the expected layout is visible (and independent of any encoder). - fn otp_params( + pub(crate) fn otp_params( secret: &[u8], name: &str, issuer: &str, @@ -293,7 +409,7 @@ mod tests { m } - fn payload(params: &[Vec]) -> Vec { + pub(crate) fn payload(params: &[Vec]) -> Vec { let mut p = Vec::new(); for m in params { p.extend([0x0a, m.len() as u8]); @@ -305,35 +421,121 @@ mod tests { p } + /// `otpauth-migration://offline?data=…` for a payload, base64 encoded + /// and percent-encoded the way a QR-embedded URI would be. + pub(crate) fn uri_for(p: &[u8]) -> String { + // tiny local encoder for the tests + const A: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut s = String::new(); + for c in p.chunks(3) { + let b = [c[0], *c.get(1).unwrap_or(&0), *c.get(2).unwrap_or(&0)]; + let n = u32::from_be_bytes([0, b[0], b[1], b[2]]); + for i in 0..=c.len() { + s.push(A[(n >> (18 - 6 * i) & 0x3f) as usize] as char); + } + for _ in c.len()..3 { + s.push('='); + } + } + format!( + "otpauth-migration://offline?data={}", + s.replace('+', "%2B") + .replace('/', "%2F") + .replace('=', "%3D") + ) + } + + /// One `OtpParameters` with a HOTP counter (field 7) appended. + fn otp_params_counter( + secret: &[u8], + name: &str, + issuer: &str, + algo: u8, + typ: u8, + counter: u8, + ) -> Vec { + let mut m = otp_params(secret, name, issuer, algo, 1, typ); + m.extend([0x38, counter]); // field 7, wire 0 + m + } + + #[test] + fn migration_wide_keeps_hotp_and_sha512() { + let p = payload(&[ + otp_params(b"0123456789", "Acme:alice", "Acme", 1, 1, 2), + otp_params_counter(b"counterkey", "Legacy", "", 1, 1, 7), + otp_params(b"0123456789", "wide", "Wide", 3, 2, 2), + otp_params(&[0x41; 64], "long", "", 2, 1, 0), + otp_params(b"", "", "", 1, 1, 2), + ]); + let f = parse_wide(&uri_for(&p)).unwrap(); + assert_eq!(f.format, Format::GoogleExport); + assert_eq!(f.batch, None); + let labels: Vec<_> = f + .items + .iter() + .map(|i| (i.index, i.label.as_str())) + .collect(); + assert_eq!( + labels, + [ + (0, "Acme:alice"), + (1, "Legacy"), + (2, "Wide:wide"), + (3, "long"), + (4, "entry 5") + ] + ); + let e = |i: usize| f.items[i].entry.as_ref().unwrap(); + assert_eq!(e(0).kind, OtpKind::Totp { period: 30 }); + assert_eq!(&e(0).secret[..], b"0123456789"); + assert_eq!(e(0).issuer.as_deref(), Some("Acme")); + assert_eq!(e(0).account.as_deref(), Some("alice")); + assert_eq!(e(1).kind, OtpKind::Hotp { counter: 7 }); + assert_eq!(e(1).issuer, None); + assert_eq!(e(1).account.as_deref(), Some("Legacy")); + assert_eq!(e(2).hash, HashAlg::Sha512); + assert_eq!(e(2).digits, 8); + assert_eq!(e(3).secret.len(), 64); + assert_eq!(e(3).hash, HashAlg::Sha256); + assert_eq!(f.items[4].entry.as_ref().unwrap_err(), &Unusable::BadSecret); + } + + #[test] + fn migration_wide_md5_is_unusable() { + let mut p = payload(&[ + otp_params(b"0123456789", "old", "Old", 4, 1, 2), + otp_params(b"0123456789", "odd", "", 9, 1, 2), + otp_params(b"0123456789", "typ", "", 1, 1, 5), + ]); + p.extend([0x18, 3, 0x20, 1]); // batch 1 of 3 (overrides the 1-of-1) + let f = parse_wide(&uri_for(&p)).unwrap(); + assert_eq!(f.batch, Some((1, 3))); + assert_eq!( + f.items[0].entry.as_ref().unwrap_err(), + &Unusable::Hash("MD5".into()) + ); + assert!(matches!( + f.items[1].entry.as_ref().unwrap_err(), + Unusable::Hash(_) + )); + assert!(matches!( + f.items[2].entry.as_ref().unwrap_err(), + Unusable::Type(_) + )); + assert_eq!(f.usable(), 0); + // Whole-file problems stay whole-file errors. + assert!(parse_wide("otpauth-migration://offline?data=!!!").is_err()); + assert!(parse_wide(&uri_for(&payload(&[]))).is_err()); + } + #[test] fn migration_round_trip() { let p = payload(&[ otp_params(b"0123456789", "Acme:alice", "Acme", 1, 1, 2), otp_params(b"counterkey", "Legacy", "", 1, 1, 1), // HOTP → skipped ]); - let b64 = { - // tiny local encoder for the test - const A: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; - let mut s = String::new(); - for c in p.chunks(3) { - let b = [c[0], *c.get(1).unwrap_or(&0), *c.get(2).unwrap_or(&0)]; - let n = u32::from_be_bytes([0, b[0], b[1], b[2]]); - for i in 0..=c.len() { - s.push(A[(n >> (18 - 6 * i) & 0x3f) as usize] as char); - } - for _ in c.len()..3 { - s.push('='); - } - } - s - }; - // percent-encode the padding the way a QR-embedded URI would. - let uri = format!( - "otpauth-migration://offline?data={}", - b64.replace('+', "%2B") - .replace('/', "%2F") - .replace('=', "%3D") - ); + let uri = uri_for(&p); let m = parse(&uri).expect("parse"); assert_eq!(m.entries.len(), 1); diff --git a/crates/keyroost-import/src/otpauth.rs b/crates/keyroost-import/src/otpauth.rs index 982a7c63..55b96bb7 100644 --- a/crates/keyroost-import/src/otpauth.rs +++ b/crates/keyroost-import/src/otpauth.rs @@ -10,6 +10,8 @@ use keyroost_proto::codec::base32_decode; use keyroost_proto::commands::{DisplayTimeout, HmacAlgo, OtpDigits, ProfileConfig, TimeStep}; +use crate::entry::{ImportEntry, Unusable}; + #[non_exhaustive] #[derive(Debug, PartialEq, Eq)] pub enum OtpAuthError { @@ -202,11 +204,7 @@ pub fn parse(uri: &str) -> Result { } // Label may be "Issuer:account" — split on the first colon if present. - let (label_issuer, account) = match label.split_once(':') { - Some((i, a)) => (Some(i.trim().to_owned()), Some(a.trim().to_owned())), - None if label.is_empty() => (None, None), - None => (None, Some(label.trim().to_owned())), - }; + let (label_issuer, account) = split_label(&label); // Prefer the explicit issuer= query param over the label prefix. let issuer = issuer_param.or(label_issuer).filter(|s| !s.is_empty()); @@ -222,6 +220,117 @@ pub fn parse(uri: &str) -> Result { }) } +/// Parse an otpauth:// URI into a format-neutral [`ImportEntry`]: HOTP with +/// its counter, any TOTP period, SHA-1/256/512, 1–10 digits and seeds up to +/// 255 bytes. Same percent-decoding and label rules as [`parse`]; problems +/// are reported secret first, then hash, digits and period. +pub fn parse_entry(uri: &str) -> Result { + parse_entry_labeled(uri).1 +} + +/// [`parse_entry`] plus the name the URI carries (`issuer:account`, or +/// whichever half it has), even when the entry itself is unusable — so a +/// list or a QR screenshot can say which account failed. +pub fn parse_entry_labeled(uri: &str) -> (Option, Result) { + let rest = match uri.strip_prefix("otpauth://") { + Some(r) => r, + None => return (None, Err(Unusable::Malformed("not an otpauth:// URI"))), + }; + let (typ_label, query) = rest.split_once('?').unwrap_or((rest, "")); + let (typ, label_raw) = match typ_label.split_once('/') { + Some(parts) => parts, + None => return (None, Err(Unusable::Malformed("the URI has no label"))), + }; + // Same label rule as `parse`: a path segment keeps a literal '+'. + let label = match percent_decode_keep_plus(label_raw) { + Ok(l) => l, + Err(()) => { + return ( + None, + Err(Unusable::Malformed( + "the label's percent-encoding is invalid", + )), + ) + } + }; + let (label_issuer, account) = split_label(&label); + + // Collect the parameters first, then validate in a fixed order. The + // decoded base32 text is the seed in another spelling: Zeroizing, so it + // is wiped on every exit (including a duplicate `secret=`). + let mut secret_b32: Option> = None; + let mut issuer_param: Option = None; + let mut algorithm: Option = None; + let mut digits: u32 = 6; + let mut period: u32 = 30; + let mut counter: u64 = 0; + let mut malformed: Option<&'static str> = None; + for kv in query.split('&').filter(|s| !s.is_empty()) { + let (k, v) = kv.split_once('=').unwrap_or((kv, "")); + let v = match percent_decode(v) { + Ok(v) => zeroize::Zeroizing::new(v), + Err(()) => { + malformed.get_or_insert("a parameter's percent-encoding is invalid"); + continue; + } + }; + match k { + "secret" => secret_b32 = Some(v), + "issuer" => issuer_param = Some(v.to_string()), + "algorithm" => algorithm = Some(v.to_string()), + "digits" => match v.parse() { + Ok(n) => digits = n, + Err(_) => { + malformed.get_or_insert("the digits value isn't a number"); + } + }, + "period" => match v.parse() { + Ok(n) => period = n, + Err(_) => { + malformed.get_or_insert("the period value isn't a number"); + } + }, + "counter" => match v.parse() { + Ok(n) => counter = n, + Err(_) => { + malformed.get_or_insert("the counter value isn't a number"); + } + }, + _ => {} // image, color, ... carry nothing a key stores + } + } + + // Prefer the explicit issuer= query param over the label prefix. + let issuer = issuer_param.or(label_issuer).filter(|s| !s.is_empty()); + let account = account.filter(|s| !s.is_empty()); + let name = crate::entry::name_label(issuer.as_deref(), account.as_deref()); + + let entry = (|| { + let kind = crate::entry::type_from_name(typ)?; + if let Some(what) = malformed { + return Err(Unusable::Malformed(what)); + } + let secret = crate::entry::decode_secret(secret_b32.as_deref().map_or("", |s| s))?; + let hash = crate::entry::hash_from_name(algorithm.as_deref().unwrap_or("SHA1"))?; + let kind = match kind { + crate::entry::TypeName::Totp => crate::entry::OtpKind::Totp { period }, + crate::entry::TypeName::Hotp => crate::entry::OtpKind::Hotp { counter }, + }; + crate::entry::finish(issuer, account, secret, hash, digits, kind) + })(); + (name, entry) +} + +/// Split an `Issuer:account` label on its first colon (both halves trimmed); +/// a label without a colon is the account. +pub(crate) fn split_label(label: &str) -> (Option, Option) { + match label.split_once(':') { + Some((i, a)) => (Some(i.trim().to_owned()), Some(a.trim().to_owned())), + None if label.is_empty() => (None, None), + None => (None, Some(label.trim().to_owned())), + } +} + /// Percent-decode a query value: `+` decodes to a space (form-encoding). fn percent_decode(s: &str) -> Result { percent_decode_impl(s, true) @@ -286,6 +395,7 @@ fn truncate_bytes(s: &str, max: usize) -> &str { #[cfg(test)] mod tests { use super::*; + use crate::entry::{HashAlg, OtpKind}; #[test] fn rejects_non_otpauth() { @@ -392,6 +502,84 @@ mod tests { assert_eq!(p.suggested_title(), "ABCDEFGHIJKL"); // 12 bytes } + #[test] + fn parse_entry_accepts_hotp_with_counter() { + let e = parse_entry("otpauth://hotp/X:a?secret=JBSWY3DPEHPK3PXP&counter=5").unwrap(); + assert_eq!(e.kind, OtpKind::Hotp { counter: 5 }); + assert_eq!(e.issuer.as_deref(), Some("X")); + assert_eq!(e.account.as_deref(), Some("a")); + assert_eq!(&e.secret[..], b"Hello!\xde\xad\xbe\xef"); + assert_eq!(e.hash, HashAlg::Sha1); + assert_eq!(e.digits, 6); + // No counter in the URI starts at 0. + let e = parse_entry("otpauth://HOTP/X?secret=JBSWY3DPEHPK3PXP").unwrap(); + assert_eq!(e.kind, OtpKind::Hotp { counter: 0 }); + } + + #[test] + fn parse_entry_accepts_any_period_and_sha512() { + let e = parse_entry( + "otpauth://totp/Co%20Inc:alice%2Bwork?secret=JBSWY3DPEHPK3PXP&period=45&algorithm=SHA512&digits=7", + ) + .unwrap(); + assert_eq!(e.kind, OtpKind::Totp { period: 45 }); + assert_eq!(e.hash, HashAlg::Sha512); + assert_eq!(e.digits, 7); + // Same label rules as `parse`: '+' in the label is literal. + assert_eq!(e.issuer.as_deref(), Some("Co Inc")); + assert_eq!(e.account.as_deref(), Some("alice+work")); + // The issuer= query param wins and form-decodes '+'. + let e = + parse_entry("otpauth://totp/Old:bob?secret=JBSWY3DPEHPK3PXP&issuer=Acme+Co").unwrap(); + assert_eq!(e.issuer.as_deref(), Some("Acme Co")); + assert_eq!(e.kind, OtpKind::Totp { period: 30 }); + // A 64-byte secret (too long for a Molto2) is fine here. + let e = parse_entry(&format!("otpauth://totp/x?secret={}", "A".repeat(103))).unwrap(); + assert_eq!(e.secret.len(), 64); + } + + #[test] + fn parse_entry_unusable_reasons() { + let p = |q: &str| parse_entry(&format!("otpauth://totp/x?{}", q)).unwrap_err(); + assert_eq!(p("secret=JBSWY3DP&period=0"), Unusable::Period(0)); + assert_eq!(p("secret=JBSWY3DP&digits=11"), Unusable::Digits(11)); + assert_eq!(p("secret=JBSWY3DP&digits=0"), Unusable::Digits(0)); + assert_eq!( + p("secret=JBSWY3DP&algorithm=MD5"), + Unusable::Hash("MD5".into()) + ); + assert_eq!(p("secret=NOT_BASE32!!"), Unusable::BadSecret); + assert_eq!(p("issuer=x"), Unusable::BadSecret); + assert_eq!(p("secret="), Unusable::BadSecret); + assert!(matches!( + p("secret=JBSWY3DP&digits=x"), + Unusable::Malformed(_) + )); + assert!(matches!( + p("secret=JBSWY3DP&period=-1"), + Unusable::Malformed(_) + )); + assert_eq!( + parse_entry("otpauth://steam/x?secret=JBSWY3DP").unwrap_err(), + Unusable::Type("steam".into()) + ); + assert!(matches!( + parse_entry("https://example.com").unwrap_err(), + Unusable::Malformed(_) + )); + assert!(matches!( + parse_entry("otpauth://totp?secret=JBSWY3DP").unwrap_err(), + Unusable::Malformed(_) + )); + // Secret problems are reported before hash and digit problems, + // matching the Molto2 path's order. + assert_eq!(p("secret=!!&algorithm=MD5&digits=11"), Unusable::BadSecret); + assert_eq!( + p("secret=JBSWY3DP&digits=11&algorithm=MD5"), + Unusable::Hash("MD5".into()) + ); + } + #[test] fn percent_decoded_label_with_plus() { let p = diff --git a/crates/keyroost-qr/src/lib.rs b/crates/keyroost-qr/src/lib.rs index 53c5bb98..52292f1d 100644 --- a/crates/keyroost-qr/src/lib.rs +++ b/crates/keyroost-qr/src/lib.rs @@ -15,7 +15,7 @@ //! to delete the screenshot afterwards. use keyroost_import::migration; -use keyroost_import::{BulkEntry, OtpAuth}; +use keyroost_import::{BulkEntry, Format, ImportFile, ImportItem, OtpAuth, Unusable}; /// Refuse images larger than this many pixels before allocating the /// grayscale buffer. 16 MP comfortably covers any screenshot (a 4K display @@ -212,6 +212,83 @@ pub fn entries_from_image(bytes: &[u8]) -> Result { Ok(import) } +/// Decode a QR image into the wide, format-neutral model: every account in +/// every code, in order — HOTP, any TOTP period and SHA-512 included — each +/// usable or marked with why not. A damaged Google Authenticator code +/// becomes an unusable item rather than discarding the other codes. Same +/// whole-image errors as [`entries_from_image`]: no 2FA code at all is +/// [`QrError::NotOtpauth`], and only damaged codes is [`QrError::Payload`]. +pub fn entries_from_image_wide(bytes: &[u8]) -> Result { + let texts = texts_from_image(bytes)?; + file_from_texts(&texts) +} + +/// The routing half of [`entries_from_image_wide`], over decoded payloads. +fn file_from_texts(texts: &[String]) -> Result { + let mut items: Vec = Vec::new(); + let mut batch = None; + let mut any_otpauth = false; + let mut any_migration = false; + let mut any_parsed = false; + let mut payload_err: Option = None; + for text in texts { + let text = text.trim(); + if migration::is_migration_uri(text) { + any_otpauth = true; + any_migration = true; + match migration::parse_wide(text) { + Ok(file) => { + any_parsed = true; + batch = batch.or(file.batch); + items.extend(file.items); + } + Err(e) => { + payload_err.get_or_insert(e.to_string()); + items.push(ImportItem { + index: 0, + label: "damaged QR code".into(), + entry: Err(Unusable::Malformed("QR payload could not be read")), + }); + } + } + } else if text.starts_with("otpauth://") { + any_otpauth = true; + any_parsed = true; + let (label, entry) = keyroost_import::otpauth::parse_entry_labeled(text); + items.push(ImportItem { + index: 0, + label: label.unwrap_or_default(), + entry, + }); + } + } + if !any_otpauth { + return Err(QrError::NotOtpauth); + } + if !any_parsed { + if let Some(e) = payload_err { + return Err(QrError::Payload(e)); + } + } + // One running index across every code in the image; nameless items are + // named by their position. + for (i, item) in items.iter_mut().enumerate() { + item.index = i; + if item.label.is_empty() { + item.label = format!("entry {}", i + 1); + } + } + Ok(ImportFile { + format: if any_migration { + Format::GoogleExport + } else { + Format::QrImage + }, + items, + batch, + }) +} + /// True when `bytes` starts with a PNG or JPEG signature — lets callers /// route files by content rather than extension. pub fn looks_like_image(bytes: &[u8]) -> bool { @@ -312,6 +389,54 @@ fn rgb_luma(p: &[u8]) -> u8 { mod tests { use super::*; + fn texts(list: &[&str]) -> Vec { + list.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn wide_damaged_code_is_an_item_not_a_failure() { + let f = file_from_texts(&texts(&[ + "otpauth://hotp/Acme:alice?secret=JBSWY3DPEHPK3PXP&counter=4", + "otpauth-migration://offline?data=!!!", + "otpauth://steam/Game?secret=JBSWY3DPEHPK3PXP", + "https://not.a.code", + ])) + .unwrap(); + assert_eq!(f.format, Format::GoogleExport); + let shape: Vec<_> = f + .items + .iter() + .map(|i| (i.index, i.label.as_str(), i.entry.as_ref().err().cloned())) + .collect(); + assert_eq!( + shape, + [ + (0, "Acme:alice", None), + ( + 1, + "damaged QR code", + Some(Unusable::Malformed("QR payload could not be read")) + ), + (2, "Game", Some(Unusable::Type("steam".into()))), + ] + ); + } + + #[test] + fn wide_whole_image_errors_match_the_narrow_path() { + assert!(matches!( + file_from_texts(&texts(&["https://example.com"])), + Err(QrError::NotOtpauth) + )); + assert!(matches!( + file_from_texts(&texts(&["otpauth-migration://offline?data=!!!"])), + Err(QrError::Payload(_)) + )); + let f = file_from_texts(&texts(&["otpauth://totp/X?secret=JBSWY3DP&period=45"])).unwrap(); + assert_eq!(f.format, Format::QrImage); + assert_eq!(f.usable(), 1); + } + #[test] fn rejects_non_images() { assert!(matches!( diff --git a/crates/keyroost-qr/tests/roundtrip.rs b/crates/keyroost-qr/tests/roundtrip.rs index 3ea9d582..f9ecc42c 100644 --- a/crates/keyroost-qr/tests/roundtrip.rs +++ b/crates/keyroost-qr/tests/roundtrip.rs @@ -33,3 +33,32 @@ fn google_authenticator_migration_png_round_trips() { assert!(import.skipped.is_empty()); assert_eq!(import.batch, None); } + +#[test] +fn wide_enrollment_png_reads_one_item() { + let img = include_bytes!("fixtures/enroll.png"); + let file = keyroost_qr::entries_from_image_wide(img).expect("decode png"); + assert_eq!(file.format, keyroost_import::Format::QrImage); + assert_eq!(file.items.len(), 1); + assert_eq!(file.items[0].label, "Acme:alice"); + let e = file.items[0].entry.as_ref().expect("usable"); + assert_eq!(&e.secret[..], b"Hello!\xde\xad\xbe\xef"); + assert_eq!(file.batch, None); +} + +#[test] +fn wide_migration_png_is_a_google_export() { + let img = include_bytes!("fixtures/migration.png"); + let file = keyroost_qr::entries_from_image_wide(img).expect("decode migration"); + assert_eq!(file.format, keyroost_import::Format::GoogleExport); + assert_eq!(file.usable(), 1); + assert_eq!(file.items[0].label, "Acme:alice"); +} + +#[test] +fn wide_keeps_the_narrow_errors() { + assert!(matches!( + keyroost_qr::entries_from_image_wide(b"not an image"), + Err(keyroost_qr::QrError::UnsupportedImage) + )); +} diff --git a/fuzz/fuzz_targets/bulk_import.rs b/fuzz/fuzz_targets/bulk_import.rs index b8905da9..078d3a7d 100644 --- a/fuzz/fuzz_targets/bulk_import.rs +++ b/fuzz/fuzz_targets/bulk_import.rs @@ -6,5 +6,10 @@ fuzz_target!(|data: &[u8]| { if let Ok(s) = std::str::from_utf8(data) { let _ = keyroost_import::parse_bulk_any(s); let _ = keyroost_import::aegis::is_encrypted(s); + // The wide parsers read the same attacker-supplied files. + let _ = keyroost_import::parse_any_wide(s); + let _ = keyroost_import::parse_list_wide(s); + let _ = keyroost_import::aegis::parse_wide(s); + let _ = keyroost_import::twofas::parse_wide(s); } }); diff --git a/fuzz/fuzz_targets/migration.rs b/fuzz/fuzz_targets/migration.rs index 224fef61..8fede209 100644 --- a/fuzz/fuzz_targets/migration.rs +++ b/fuzz/fuzz_targets/migration.rs @@ -4,7 +4,9 @@ use libfuzzer_sys::fuzz_target; fuzz_target!(|data: &[u8]| { let _ = keyroost_import::migration::parse_payload(data); + let _ = keyroost_import::migration::parse_payload_wide(data); if let Ok(s) = std::str::from_utf8(data) { let _ = keyroost_import::migration::parse(s); + let _ = keyroost_import::migration::parse_wide(s); } }); diff --git a/fuzz/fuzz_targets/otpauth.rs b/fuzz/fuzz_targets/otpauth.rs index 0215d4db..b73401ea 100644 --- a/fuzz/fuzz_targets/otpauth.rs +++ b/fuzz/fuzz_targets/otpauth.rs @@ -5,5 +5,6 @@ use libfuzzer_sys::fuzz_target; fuzz_target!(|data: &[u8]| { if let Ok(s) = std::str::from_utf8(data) { let _ = keyroost_import::parse_otpauth(s); + let _ = keyroost_import::otpauth::parse_entry_labeled(s); } }); diff --git a/fuzz/fuzz_targets/qr_image.rs b/fuzz/fuzz_targets/qr_image.rs index a01438a3..7316391e 100644 --- a/fuzz/fuzz_targets/qr_image.rs +++ b/fuzz/fuzz_targets/qr_image.rs @@ -4,4 +4,5 @@ use libfuzzer_sys::fuzz_target; fuzz_target!(|data: &[u8]| { let _ = keyroost_qr::entries_from_image(data); + let _ = keyroost_qr::entries_from_image_wide(data); }); From 66cb36f81528bac05120e8103cea621aecf25364 Mon Sep 17 00:00:00 2001 From: framefilter Date: Fri, 9 Oct 2026 20:10:56 -0400 Subject: [PATCH 03/35] import: classify dropped files and read every authenticator export through one entry point (re #170) The import flow starts from a file the person drops or picks, and that file may be a certificate, a key or something else entirely. Guessing a destination from such a file would be wrong, and the CLI and GUI each sniffing formats on their own would drift apart. classify() sorts a file by content (and, for binary DER and OpenPGP, by extension) into an authenticator export, recognized key material, or unknown, without decrypting or decoding anything and without its JSON probes materializing a seed. Key material comes with a plain pointer to where keyroost imports it (PIV certificates, OpenPGP RSA keys) or that it doesn't, each ending "Nothing was changed." open_file() in keyroost-qr is the single reader both front ends will use: it refuses files over 16 MiB, asks for the Aegis password when a vault needs one, decrypts into a wipe-on-drop buffer, reads QR screenshots, and returns the wide entry model. keyroost-qr now enables keyroost-import's existing `encrypted` feature for the vault path; no new crates. Both functions join the fuzz targets. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Skvxkb6eMzytjPDmL6mPYU --- crates/keyroost-import/src/bulk.rs | 43 +++ crates/keyroost-import/src/classify.rs | 443 +++++++++++++++++++++++++ crates/keyroost-import/src/lib.rs | 1 + crates/keyroost-qr/Cargo.toml | 4 +- crates/keyroost-qr/src/file.rs | 268 +++++++++++++++ crates/keyroost-qr/src/lib.rs | 4 + fuzz/fuzz_targets/bulk_import.rs | 3 + fuzz/fuzz_targets/qr_image.rs | 2 + 8 files changed, 767 insertions(+), 1 deletion(-) create mode 100644 crates/keyroost-import/src/classify.rs create mode 100644 crates/keyroost-qr/src/file.rs diff --git a/crates/keyroost-import/src/bulk.rs b/crates/keyroost-import/src/bulk.rs index 81e8f71c..c5128acc 100644 --- a/crates/keyroost-import/src/bulk.rs +++ b/crates/keyroost-import/src/bulk.rs @@ -374,6 +374,49 @@ pub mod twofas { counter: Option, } + /// Whether `json` looks like a 2FAS export, without materializing any + /// service (or seed): `Some(true)` for an encrypted export (the + /// `servicesEncrypted` blob and no plaintext services), `Some(false)` + /// for a plaintext one, `None` when it is neither (or not a JSON object). + pub(crate) fn probe(json: &str) -> Option { + use serde::de::{IgnoredAny, SeqAccess, Visitor}; + /// Number of elements in a JSON array, drained without allocating. + struct Count(usize); + impl<'de> Deserialize<'de> for Count { + fn deserialize>(d: D) -> Result { + struct V; + impl<'de> Visitor<'de> for V { + type Value = Count; + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a JSON array") + } + fn visit_seq>(self, mut a: A) -> Result { + let mut n = 0; + while a.next_element::()?.is_some() { + n += 1; + } + Ok(Count(n)) + } + } + d.deserialize_seq(V) + } + } + #[derive(Deserialize)] + struct ProbeRoot { + services: Option, + #[serde(rename = "servicesEncrypted")] + services_encrypted: Option, + } + let root: ProbeRoot = serde_json::from_str(json).ok()?; + let services = root.services.map(|c| c.0); + match (services, root.services_encrypted.is_some()) { + (None, false) => None, + (Some(n), true) if n > 0 => Some(false), + (_, true) => Some(true), + (Some(_), false) => Some(false), + } + } + /// Read the plaintext `services`, or the encrypted-export error. fn read_root(json: &str) -> Result { let root: Root = serde_json::from_str(json)?; diff --git a/crates/keyroost-import/src/classify.rs b/crates/keyroost-import/src/classify.rs new file mode 100644 index 00000000..5118ac82 --- /dev/null +++ b/crates/keyroost-import/src/classify.rs @@ -0,0 +1,443 @@ +//! Sorting a dropped or opened file before anything reads it as an import: +//! an authenticator export, recognized key material (a certificate, a key, +//! ...) that belongs somewhere else, or something keyroost doesn't read. +//! +//! Classification looks only at the bytes it is given (plus, for binary +//! DER and OpenPGP files, the file name's extension). It never decrypts a +//! vault or decodes an image, and its JSON probes never materialize an entry +//! or a seed. The JSON probes need the `bulk` feature; without it every JSON +//! file is [`FileKind::Unknown`]. + +/// An authenticator export keyroost can read. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AuthFormat { + Aegis, + AegisEncrypted, + TwoFas, + TwoFasEncrypted, + OtpauthList, + /// A PNG or JPEG image (a QR screenshot, maybe). + Image, +} + +/// Key material keyroost recognizes but doesn't import as authenticator +/// codes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum KeyMaterial { + Certificate, + CertificateRequest, + PrivateKey, + OpenPgpKey, + SshKey, + Pkcs12, +} + +/// What a file is, as far as importing goes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FileKind { + Authenticator(AuthFormat), + KeyMaterial(KeyMaterial), + Unknown, +} + +/// Shown for a file that is neither an authenticator export nor recognized +/// key material. +pub const UNKNOWN_FILE: &str = + "keyroost can't import this file. It reads Aegis, 2FAS and otpauth:// list exports and QR screenshots."; + +impl KeyMaterial { + /// "a certificate", "an OpenPGP key", ... + pub fn what(self) -> &'static str { + match self { + KeyMaterial::Certificate => "a certificate", + KeyMaterial::CertificateRequest => "a certificate request", + KeyMaterial::PrivateKey => "a private key", + KeyMaterial::OpenPgpKey => "an OpenPGP key", + KeyMaterial::SshKey => "an SSH key", + KeyMaterial::Pkcs12 => "a PKCS #12 bundle", + } + } + + /// Where keyroost imports it, or that it doesn't, in GUI words. Each ends + /// "Nothing was changed." + pub fn gui_pointer(self) -> &'static str { + match self { + KeyMaterial::Certificate => { + "This looks like a certificate. To put it on a key, open the key's PIV tab \ + and use Import certificate. Nothing was changed." + } + KeyMaterial::PrivateKey => { + "This looks like a private key. keyroost imports RSA-2048 private keys in a \ + key's OpenPGP tab (Import key from file). Nothing was changed." + } + KeyMaterial::CertificateRequest => { + "This looks like a certificate request. keyroost can't import this kind of \ + file. Nothing was changed." + } + KeyMaterial::OpenPgpKey => { + "This looks like an OpenPGP key. keyroost can't import this kind of file. \ + Nothing was changed." + } + KeyMaterial::SshKey => { + "This looks like an SSH key. keyroost can't import this kind of file. \ + Nothing was changed." + } + KeyMaterial::Pkcs12 => { + "This looks like a PKCS #12 bundle. keyroost can't import this kind of file. \ + Nothing was changed." + } + } + } + + /// The same in CLI words. + pub fn cli_pointer(self) -> &'static str { + match self { + KeyMaterial::Certificate => { + "This looks like a certificate. To put it on a key, use: \ + keyroostctl -d KEY piv cert import --slot SLOT --in FILE. Nothing was changed." + } + KeyMaterial::PrivateKey => { + "This looks like a private key. keyroost imports RSA-2048 private keys with: \ + keyroostctl -d KEY openpgp key import --in FILE. Nothing was changed." + } + // No command imports these, so the wording is the GUI's. + other => other.gui_pointer(), + } + } +} + +/// Sort a file by content (and, for binary DER, its extension). Never +/// decrypts or decodes images; reads at most the bytes given. +pub fn classify(bytes: &[u8], file_name: Option<&str>) -> FileKind { + // 1. Images, by magic bytes. + if bytes.starts_with(b"\x89PNG\r\n\x1a\n") || bytes.starts_with(&[0xFF, 0xD8, 0xFF]) { + return FileKind::Authenticator(AuthFormat::Image); + } + + let text = core::str::from_utf8(bytes).ok(); + if let Some(text) = text { + // 2. JSON: an Aegis or 2FAS export, or nothing keyroost reads. + if text.trim_start().starts_with('{') { + return classify_json(text); + } + // 3. Any otpauth line makes it a list (bad lines are reported per + // line by the list parser). + if text + .lines() + .map(str::trim_start) + .any(|l| l.starts_with("otpauth://") || l.starts_with("otpauth-migration://")) + { + return FileKind::Authenticator(AuthFormat::OtpauthList); + } + } + + // 4. PEM armor, by label. + if let Some(kind) = pem_kind(bytes) { + return FileKind::KeyMaterial(kind); + } + + // 5. An OpenSSH public key line. + let first_line = bytes + .split(|&b| b == b'\n') + .next() + .unwrap_or_default() + .trim_ascii_start(); + const SSH_PREFIXES: [&[u8]; 5] = [ + b"ssh-ed25519 ", + b"ssh-rsa ", + b"ecdsa-sha2-", + b"sk-ssh-", + b"sk-ecdsa-", + ]; + if SSH_PREFIXES.iter().any(|p| first_line.starts_with(p)) { + return FileKind::KeyMaterial(KeyMaterial::SshKey); + } + + let ext = extension(file_name); + // Binary: not UTF-8, or carrying control bytes no text file has (DER + // always does: its INTEGER/OCTET STRING/OID tags are 0x02/0x04/0x06). + let binary = text.is_none() || bytes.iter().any(|&b| b < 0x09 || (0x0e..0x20).contains(&b)); + // 6. DER (an ASN.1 SEQUENCE), named by its extension. + if binary && bytes.first() == Some(&0x30) { + let kind = match ext.as_deref() { + Some("cer" | "crt" | "der") => Some(KeyMaterial::Certificate), + Some("p12" | "pfx") => Some(KeyMaterial::Pkcs12), + Some("key") => Some(KeyMaterial::PrivateKey), + Some("csr") => Some(KeyMaterial::CertificateRequest), + _ => None, + }; + if let Some(kind) = kind { + return FileKind::KeyMaterial(kind); + } + } + + // 7. Binary OpenPGP. + if binary && matches!(ext.as_deref(), Some("gpg" | "pgp")) { + return FileKind::KeyMaterial(KeyMaterial::OpenPgpKey); + } + + FileKind::Unknown +} + +#[cfg(feature = "bulk")] +fn classify_json(text: &str) -> FileKind { + match crate::bulk::aegis::is_encrypted(text) { + Ok(true) => return FileKind::Authenticator(AuthFormat::AegisEncrypted), + Ok(false) => return FileKind::Authenticator(AuthFormat::Aegis), + Err(_) => {} + } + match crate::bulk::twofas::probe(text) { + Some(true) => FileKind::Authenticator(AuthFormat::TwoFasEncrypted), + Some(false) => FileKind::Authenticator(AuthFormat::TwoFas), + None => FileKind::Unknown, + } +} + +#[cfg(not(feature = "bulk"))] +fn classify_json(_text: &str) -> FileKind { + FileKind::Unknown +} + +/// The first recognized `-----BEGIN