diff --git a/CLAUDE.md b/CLAUDE.md index 43b0254c..2422b21c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,6 +16,7 @@ cargo test --locked --release # tests in release mode (CI runs both) cargo test # run a single test by substring match cargo fmt --all -- --check # formatting check (CI gate) cargo clippy -- -D warnings # lint; warnings are errors (CI gate) +cargo clippy --all-targets -- -D warnings # also lints tests; kept clean too ``` CI (`.github/workflows/build.yml`) requires `cargo fmt`, `cargo clippy -D warnings`, and `cargo test` (debug + release) to pass across Linux/macOS/Windows. On Linux, building requires xcb dev libraries for clipboard support (see README "Other linux distributions"). @@ -41,25 +42,30 @@ commit-analyzer computes the next version, `ci/write_cargo_version.sh` writes it `main.rs` drives a fixed lifecycle for every invocation: -1. `init()` — resolves the DB path, and either initializes a new encrypted database on first run (prompting for a password) or loads the existing one. Returns `ReadResult = (OTPDatabase, key, salt)`. -2. `args_parser()` (`arguments/mod.rs`) — if a subcommand was given, dispatches to it; otherwise launches the interactive `dashboard()` (TUI). Each subcommand consumes the `OTPDatabase` and returns a (possibly modified) one. -3. Back in `main()`, if `database.is_modified()` the database is re-encrypted and written to disk. The derived `key` is zeroized before exit. +1. `init()` — resolves the DB path, and either initializes a new encrypted database on first run (prompting for a password) or loads the existing one via `storage::`. Returns `ReadResult = (OTPDatabase, key, salt)` (defined in `storage/mod.rs`). +2. `args_parser()` (`arguments/mod.rs`) — if a subcommand was given, dispatches to it; otherwise launches the interactive `dashboard()` (TUI). Each subcommand consumes the `OTPDatabase` and returns a (possibly modified) one. Exit codes: 1 for init/save errors, 2 for subcommand errors. +3. Back in `main()`, if `database.is_modified()` the database is re-encrypted and written to disk via `storage::save()`. The derived `key` is zeroized before exit. -The `OTPDatabase` is passed by value through the command layer; mutations set a `needs_modification` flag (via `mark_modified()`) that gates the final save. Secrets and keys use `zeroize` throughout — preserve zeroization when touching password/key handling. +The `OTPDatabase` is passed by value through the command layer; mutations set a private dirty flag (via `mark_modified()`/`clear_modified()`) that gates the final save — `mut_element()` deliberately does NOT mark, so callers mark only on real changes (no-op edits skip the rewrite). `storage::save()` clears the flag only after a successful write; this is also what makes `passwd` safe (it saves itself with a key from the new password, and the cleared flag stops `main()` from saving again with the old key). Secrets and keys use `zeroize` throughout — preserve zeroization when touching password/key handling. ## Key modules (`src/`) -- **`arguments/`** — Clap subcommands (`add`, `edit`, `list`, `delete`, `import`, `export`, `extract`, `passwd`). Each implements the `SubcommandExecutor` trait (`fn run_command(self, db: OTPDatabase) -> Result`), wired together with `enum_dispatch` on the `CotpSubcommands` enum. To add a subcommand: create the module, define an `Args` struct, implement `SubcommandExecutor`, and add a variant to `CotpSubcommands`. -- **`otp/`** — core domain. `otp_element.rs` holds `OTPElement` and `OTPDatabase` (serialization, save/encrypt, migrations). `algorithms/` has one generator per scheme (`totp`, `hotp`, `motp`, `steam`, `yandex`). `otp_type.rs` / `otp_algorithm.rs` are the enums; `from_otp_uri.rs` parses `otpauth://` URIs. +- **`arguments/`** — Clap subcommands (`add`, `edit`, `list`, `delete`, `import`, `export`, `extract`, `passwd`). Each implements the `SubcommandExecutor` trait (`fn run_command(self, db: OTPDatabase) -> Result`), wired together with `enum_dispatch` on the `CotpSubcommands` enum. To add a subcommand: create the module, define an `Args` struct, implement `SubcommandExecutor`, and add a variant to `CotpSubcommands`. The mutually exclusive import/export format flags map to exhaustive internal enums (`ImportFormat` in `import.rs`, `ExportKind` in `export.rs`) — add new formats there. `extract` matches issuer/label with a hand-rolled `*`/`?` wildcard matcher (no regex/glob crate). +- **`otp/`** — core domain. `otp_element.rs` holds `OTPElement` and `OTPDatabase`; the database is pure domain data (element accessors, dirty flag, sort) — persistence lives in `storage/`. `algorithms/` has one generator per scheme (`totp`, `hotp`, `motp`, `steam`, `yandex`); Yandex always uses HMAC-SHA256 regardless of the element's stored algorithm. `otp_type.rs` / `otp_algorithm.rs` are the enums, with `TryFrom<&str>` impls that reject unknown strings instead of defaulting; `from_otp_uri.rs` parses `otpauth://` URIs. +- **`storage/`** — persistence layer. Load: `get_elements_from_input`/`get_elements_from_stdin` → `read_from_file` (password prompt, decrypt, legacy-v1 fallback). Save: `save(db, key, salt, path)` / `save_with_pw(db, password, path)` — runs `migrate()` on every save, encrypts, writes with 0600 permissions on unix, zeroizes the plaintext JSON, and clears the dirty flag only after a successful write. - **`crypto/`** — `cryptography.rs` does Argon2id key derivation (config constants at top of file) + XChaCha20Poly1305 authenticated encryption; also AES-GCM for decrypting Aegis encrypted backups. `encrypted_database.rs` is the on-disk envelope. - **`importers/`** — one module per source app. `importer.rs::import_from_path::()` is the generic entry point: `T` must be `Deserialize + TryInto>`. Import selection happens in `arguments/import.rs`. Some sources (Authy, Microsoft Authenticator, FreeOTP) are pre-converted by Python scripts to `ConvertedJsonList` first (see below); others deserialize natively. Google Authenticator is handled natively by `google_authenticator.rs`, which parses `otpauth-migration://` export URIs (base64 protobuf `MigrationPayload`, decoded with `prost` using hand-declared message structs — no `.proto`/`protoc` build step). -- **`exporters/`** — `andotp`, `freeotp_plus`, `otp_uri`. `do_export::()` is the shared writer. -- **`interface/`** — the ratatui/crossterm TUI. `app.rs` holds mutable `App` state; `ui.rs` renders; `event.rs` is the input event loop (250ms tick); `handlers/` route key events by focus (`main_window`, `popup`, `search_bar`). The dashboard runs on `io::stderr()` so stdout stays clean for piping. +- **`exporters/`** — `andotp`, `freeotp_plus`, `otp_uri`. `do_export::()` is the shared writer (0600 permissions on unix). +- **`interface/`** — the ratatui/crossterm TUI. `app.rs` holds mutable `App` state (including the cached rendered QR code); `ui.rs` owns the terminal lifecycle (`Tui`) and all rendering, as free functions taking `&mut App`; `event.rs` is the input event loop (250ms tick); `handlers/` route key events by focus (`main_window`, `popup`, `search_bar`). The dashboard runs on `io::stderr()` so stdout stays clean for piping. + +## Dependency notes + +Errors use plain `eyre` (not color-eyre). All base32/hex/base64 codecs go through `data-encoding` (no `hex`/`base64` crates). `ratatui` and `qrcode` are built with trimmed feature sets, and `url`'s IDNA backend is pinned to the small unicode-rs `idna_adapter` — keep this binary-size budget in mind when adding dependencies. ## Database format & migrations - Default path resolution (`path.rs`): `--database-path` arg > `COTP_DB_PATH` env > `./db.cotp` (portable / debug builds always) > `$XDG_DATA_HOME/cotp/db.cotp` (auto-migrated from legacy `$HOME/.cotp/db.cotp` if present). The path is a `OnceLock` set once at startup. -- `CURRENT_DATABASE_VERSION` (in `otp_element.rs`) is the schema version. Legacy v1 was a bare `Vec`; `read_from_file` falls back to parsing that and converts via `From>`. Schema upgrades go in `otp/migrations/mod.rs` — add a `Migration { to_version, migration_function }` entry to `MIGRATIONS_LIST`; `migrate()` runs on every save. +- `CURRENT_DATABASE_VERSION` (in `otp_element.rs`) is the schema version. Legacy v1 was a bare `Vec`; `storage::read_from_file` falls back to parsing that and converts via `From>`. Schema upgrades go in `otp/migrations/mod.rs` — add a `Migration { to_version, migration_function }` entry to `MIGRATIONS_LIST`; `migrate()` runs on every save (from `storage::save`). ## Python converters (`converters/`) diff --git a/Cargo.lock b/Cargo.lock index 5880b171..71de7df5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,21 +2,6 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "addr2line" -version = "0.25.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b5d307320b3181d6d7954e663bd7c774a838b8220fe0593c86d9fb09f498b4b" -dependencies = [ - "gimli", -] - -[[package]] -name = "adler2" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" - [[package]] name = "aead" version = "0.6.1" @@ -188,21 +173,6 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" -[[package]] -name = "backtrace" -version = "0.3.76" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb531853791a215d7c62a30daf0dde835f381ab5de4589cfe7c649d2cbe92bd6" -dependencies = [ - "addr2line", - "cfg-if", - "libc", - "miniz_oxide", - "object", - "rustc-demangle", - "windows-link", -] - [[package]] name = "base64" version = "0.22.1" @@ -300,12 +270,6 @@ version = "1.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" -[[package]] -name = "byteorder-lite" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f1fe948ff07f4bd06c30984e69f5b4899c516a3ef74f34df92a2df2ab535495" - [[package]] name = "bytes" version = "1.12.1" @@ -423,33 +387,6 @@ version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c9ea0ac24bc397ab3c98583a3c9ba74fa56b09a4449bbe172b9b1ddb016027a" -[[package]] -name = "color-eyre" -version = "0.6.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5920befb47832a6d61ee3a3a846565cfa39b331331e68a3b1d1116630f2f26d" -dependencies = [ - "backtrace", - "color-spantrace", - "eyre", - "indenter", - "once_cell", - "owo-colors", - "tracing-error", -] - -[[package]] -name = "color-spantrace" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8b88ea9df13354b55bc7234ebcce36e6ef896aca2e42a15de9e10edce01b427" -dependencies = [ - "once_cell", - "owo-colors", - "tracing-core", - "tracing-error", -] - [[package]] name = "colorchoice" version = "1.0.5" @@ -524,20 +461,18 @@ dependencies = [ "aes-gcm", "assert_cmd", "assert_fs", - "base64", "chacha20poly1305", "clap", - "color-eyre", "copypasta-ext", "crossterm", "data-encoding", "derive_builder", "dirs", "enum_dispatch", + "eyre", "getrandom 0.4.3", - "globset", - "hex", "hmac", + "idna_adapter", "md-5", "predicates", "prost", @@ -876,18 +811,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.59.0", -] - -[[package]] -name = "displaydoc" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", + "windows-sys 0.61.2", ] [[package]] @@ -945,7 +869,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -1106,12 +1030,6 @@ dependencies = [ "polyval", ] -[[package]] -name = "gimli" -version = "0.32.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e629b9b98ef3dd8afe6ca2bd0f89306cec16d43d907889945bc5d6687f2f13c7" - [[package]] name = "globset" version = "0.4.19" @@ -1197,87 +1115,6 @@ dependencies = [ "typenum", ] -[[package]] -name = "icu_collections" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c6b649701667bbe825c3b7e6388cb521c23d88644678e83c0c4d0a621a34b43" -dependencies = [ - "displaydoc", - "potential_utf", - "yoke", - "zerofrom", - "zerovec", -] - -[[package]] -name = "icu_locale_core" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "edba7861004dd3714265b4db54a3c390e880ab658fec5f7db895fae2046b5bb6" -dependencies = [ - "displaydoc", - "litemap", - "tinystr", - "writeable", - "zerovec", -] - -[[package]] -name = "icu_normalizer" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f6c8828b67bf8908d82127b2054ea1b4427ff0230ee9141c54251934ab1b599" -dependencies = [ - "icu_collections", - "icu_normalizer_data", - "icu_properties", - "icu_provider", - "smallvec", - "zerovec", -] - -[[package]] -name = "icu_normalizer_data" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7aedcccd01fc5fe81e6b489c15b247b8b0690feb23304303a9e560f37efc560a" - -[[package]] -name = "icu_properties" -version = "2.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "020bfc02fe870ec3a66d93e677ccca0562506e5872c650f893269e08615d74ec" -dependencies = [ - "icu_collections", - "icu_locale_core", - "icu_properties_data", - "icu_provider", - "zerotrie", - "zerovec", -] - -[[package]] -name = "icu_properties_data" -version = "2.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "616c294cf8d725c6afcd8f55abc17c56464ef6211f9ed59cccffe534129c77af" - -[[package]] -name = "icu_provider" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85962cf0ce02e1e0a629cc34e7ca3e373ce20dda4c4d7294bbd0bf1fdb59e614" -dependencies = [ - "displaydoc", - "icu_locale_core", - "writeable", - "yoke", - "zerofrom", - "zerotrie", - "zerovec", -] - [[package]] name = "ident_case" version = "1.0.1" @@ -1297,13 +1134,9 @@ dependencies = [ [[package]] name = "idna_adapter" -version = "1.2.1" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3acae9609540aa318d1bc588455225fb2085b9ed0c4f6bd0d9d5bcd86f1a0344" -dependencies = [ - "icu_normalizer", - "icu_properties", -] +checksum = "cfdf4f5d937a025381f5ab13624b1c5f51414bfe5c9885663226eae8d6d39560" [[package]] name = "ignore" @@ -1321,18 +1154,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "image" -version = "0.25.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6506c6c10786659413faa717ceebcb8f70731c0a60cbae39795fdf114519c1a" -dependencies = [ - "bytemuck", - "byteorder-lite", - "moxcms", - "num-traits", -] - [[package]] name = "indenter" version = "0.3.4" @@ -1482,12 +1303,6 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" -[[package]] -name = "litemap" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6373607a59f0be73a39b6fe456b8192fcc3585f602af20751600e974dd455e77" - [[package]] name = "litrs" version = "1.0.0" @@ -1592,15 +1407,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" -[[package]] -name = "miniz_oxide" -version = "0.8.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fa76a2c86f704bdb222d66965fb3d63269ce38518b83cb0575fca855ebb6316" -dependencies = [ - "adler2", -] - [[package]] name = "mio" version = "1.2.2" @@ -1613,16 +1419,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "moxcms" -version = "0.7.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac9557c559cd6fc9867e122e20d2cbefc9ca29d80d027a8e39310920ed2f0a97" -dependencies = [ - "num-traits", - "pxfm", -] - [[package]] name = "nix" version = "0.24.3" @@ -1728,15 +1524,6 @@ dependencies = [ "objc", ] -[[package]] -name = "object" -version = "0.37.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff76201f031d8863c38aa7f905eca4f53abbfa15f609db4277d44cd8938f33fe" -dependencies = [ - "memchr", -] - [[package]] name = "once_cell" version = "1.21.4" @@ -1764,12 +1551,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "owo-colors" -version = "4.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d211803b9b6b570f68772237e415a029d5a50c65d382910b879fb19d3271f94d" - [[package]] name = "palette" version = "0.7.6" @@ -1928,12 +1709,6 @@ dependencies = [ "siphasher", ] -[[package]] -name = "pin-project-lite" -version = "0.2.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" - [[package]] name = "pkg-config" version = "0.3.32" @@ -1967,15 +1742,6 @@ version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c33a9471896f1c69cecef8d20cbe2f7accd12527ce60845ff44c153bb2a21b49" -[[package]] -name = "potential_utf" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b73949432f5e2a09657003c25bca5e19a0e9c84f8058ca374f49e0ebe605af77" -dependencies = [ - "zerovec", -] - [[package]] name = "powerfmt" version = "0.2.0" @@ -2044,20 +1810,11 @@ dependencies = [ "syn 2.0.119", ] -[[package]] -name = "pxfm" -version = "0.1.28" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5a041e753da8b807c9255f28de81879c78c876392ff2469cde94799b2896b9d" - [[package]] name = "qrcode" version = "0.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d68782463e408eb1e668cf6152704bd856c78c5b6417adaee3203d8f4c1fc9ec" -dependencies = [ - "image", -] [[package]] name = "quote" @@ -2110,7 +1867,6 @@ dependencies = [ "instability", "ratatui-core", "ratatui-crossterm", - "ratatui-macros", "ratatui-termina", "ratatui-termwiz", "ratatui-widgets", @@ -2151,16 +1907,6 @@ dependencies = [ "ratatui-core", ] -[[package]] -name = "ratatui-macros" -version = "0.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed7dc68daa7498a43e4d68e0eb078427e10c38fbcfbb1e42d955f1fa2140d814" -dependencies = [ - "ratatui-core", - "ratatui-widgets", -] - [[package]] name = "ratatui-termina" version = "0.1.0" @@ -2284,12 +2030,6 @@ dependencies = [ "crossbeam-utils", ] -[[package]] -name = "rustc-demangle" -version = "0.1.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b50b8869d9fc858ce7266cce0194bd74df58b9d0e3f6df3a9fc8eb470d95c09d" - [[package]] name = "rustc_version" version = "0.4.1" @@ -2322,7 +2062,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2462,15 +2202,6 @@ dependencies = [ "digest 0.11.3", ] -[[package]] -name = "sharded-slab" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" -dependencies = [ - "lazy_static", -] - [[package]] name = "signal-hook" version = "0.3.18" @@ -2542,12 +2273,6 @@ dependencies = [ "wayland-client", ] -[[package]] -name = "stable_deref_trait" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" - [[package]] name = "static_assertions" version = "1.1.0" @@ -2620,17 +2345,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "synstructure" -version = "0.13.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "tempfile" version = "3.27.0" @@ -2640,7 +2354,7 @@ dependencies = [ "fastrand", "once_cell", "rustix 1.1.4", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2798,15 +2512,6 @@ dependencies = [ "syn 3.0.3", ] -[[package]] -name = "thread_local" -version = "1.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f60246a4944f24f6e018aa17cdeffb7818b76356965d03b07d6a9886e8962185" -dependencies = [ - "cfg-if", -] - [[package]] name = "time" version = "0.3.47" @@ -2828,57 +2533,6 @@ version = "0.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7694e1cfe791f8d31026952abf09c69ca6f6fa4e1a1229e18988f06a04a12dca" -[[package]] -name = "tinystr" -version = "0.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42d3e9c45c09de15d06dd8acf5f4e0e399e85927b7f00711024eb7ae10fa4869" -dependencies = [ - "displaydoc", - "zerovec", -] - -[[package]] -name = "tracing" -version = "0.1.44" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" -dependencies = [ - "pin-project-lite", - "tracing-core", -] - -[[package]] -name = "tracing-core" -version = "0.1.36" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a" -dependencies = [ - "once_cell", - "valuable", -] - -[[package]] -name = "tracing-error" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b1581020d7a273442f5b45074a6a57d5757ad0a47dac0e9f0bd57b81936f3db" -dependencies = [ - "tracing", - "tracing-subscriber", -] - -[[package]] -name = "tracing-subscriber" -version = "0.3.22" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f30143827ddab0d256fd843b7a66d164e9f271cfa0dde49142c5ca0ca291f1e" -dependencies = [ - "sharded-slab", - "thread_local", - "tracing-core", -] - [[package]] name = "typenum" version = "1.20.1" @@ -2972,12 +2626,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "valuable" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" - [[package]] name = "version_check" version = "0.9.5" @@ -3251,7 +2899,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -3372,12 +3020,6 @@ version = "0.51.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7249219f66ced02969388cf2bb044a09756a083d0fab1e566056b04d9fbcaa5" -[[package]] -name = "writeable" -version = "0.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" - [[package]] name = "x11-clipboard" version = "0.7.1" @@ -3421,50 +3063,6 @@ version = "0.8.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3ae8337f8a065cfc972643663ea4279e04e7256de865aa66fe25cec5fb912d3f" -[[package]] -name = "yoke" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72d6e5c6afb84d73944e5cedb052c4680d5657337201555f9f2a16b7406d4954" -dependencies = [ - "stable_deref_trait", - "yoke-derive", - "zerofrom", -] - -[[package]] -name = "yoke-derive" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b659052874eb698efe5b9e8cf382204678a0086ebf46982b79d6ca3182927e5d" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", - "synstructure", -] - -[[package]] -name = "zerofrom" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50cc42e0333e05660c3587f3bf9d0478688e15d870fab3346451ce7f8c9fbea5" -dependencies = [ - "zerofrom-derive", -] - -[[package]] -name = "zerofrom-derive" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d71e5d6e06ab090c67b5e44993ec16b72dcbaabc526db883a360057678b48502" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", - "synstructure", -] - [[package]] name = "zeroize" version = "1.9.0" @@ -3485,39 +3083,6 @@ dependencies = [ "syn 2.0.119", ] -[[package]] -name = "zerotrie" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a59c17a5562d507e4b54960e8569ebee33bee890c70aa3fe7b97e85a9fd7851" -dependencies = [ - "displaydoc", - "yoke", - "zerofrom", -] - -[[package]] -name = "zerovec" -version = "0.11.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c28719294829477f525be0186d13efa9a3c602f7ec202ca9e353d310fb9a002" -dependencies = [ - "yoke", - "zerofrom", - "zerovec-derive", -] - -[[package]] -name = "zerovec-derive" -version = "0.11.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eadce39539ca5cb3985590102671f2567e659fca9666581ad3411d59207951f3" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "zmij" version = "1.0.23" diff --git a/Cargo.toml b/Cargo.toml index 4cce0f95..07926c7d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,18 +43,26 @@ getrandom = "0.4.3" rust-argon2 = "3.0.0" scrypt = "0.12.0" aes-gcm = "0.11.0" -hex = "0.4.3" -qrcode = "0.14.1" +qrcode = { version = "0.14.1", default-features = false } urlencoding = "2.1.3" -base64 = "0.22.1" md-5 = "0.11.0" -ratatui = { version = "0.30.2", features = ["all-widgets"] } +# Default features minus "all-widgets" (widget-calendar), which the TUI never +# uses and which drags the `time` crate into every build, and minus "macros", +# which cotp does not use either. +ratatui = { version = "0.30.2", default-features = false, features = [ + "crossterm", + "layout-cache", + "underline-color", +] } crossterm = "0.29.0" url = "2.5.8" -color-eyre = "0.6.5" +# Direct pin of url's IDNA backend to the unicode-rs adapter, keeping the much +# larger ICU4X tables out of the binary. cotp only parses otpauth:// URIs and +# never needs internationalized domain names. +idna_adapter = "~1.0" +eyre = "0.6.12" enum_dispatch = "0.3.13" derive_builder = "0.20.2" -globset = "0.4.19" prost = "0.14.4" [dev-dependencies] diff --git a/src/arguments/add.rs b/src/arguments/add.rs index 030978b3..35c00afa 100644 --- a/src/arguments/add.rs +++ b/src/arguments/add.rs @@ -1,7 +1,7 @@ use std::io::{self, BufRead}; use clap::{Args, value_parser}; -use color_eyre::eyre::{self, ErrReport, Result}; +use eyre::{self, ErrReport, Result}; use zeroize::Zeroize; @@ -41,7 +41,7 @@ pub struct AddArgs { short, long, default_value_t = 6, - default_value_if("type", "STEAM", "5"), + default_value_if("otp_type", "steam", "5"), value_parser=value_parser!(u64).range(1..=10) )] pub digits: u64, @@ -51,15 +51,15 @@ pub struct AddArgs { pub period: u64, /// HOTP counter - #[arg(short, long, required_if_eq("otp_type", "HOTP"))] + #[arg(short, long, required_if_eq("otp_type", "hotp"))] pub counter: Option, /// Yandex / MOTP pin #[arg( short, long, - required_if_eq("otp_type", "YANDEX"), - required_if_eq("otp_type", "MOTP") + required_if_eq("otp_type", "yandex"), + required_if_eq("otp_type", "motp") )] pub pin: Option, @@ -69,9 +69,9 @@ pub struct AddArgs { } impl SubcommandExecutor for AddArgs { - fn run_command(self, mut database: OTPDatabase) -> color_eyre::Result { + fn run_command(self, mut database: OTPDatabase) -> eyre::Result { let otp_element = if self.otp_uri { - let mut otp_uri = rpassword::prompt_password("Insert the otp uri: ").unwrap(); + let mut otp_uri = rpassword::prompt_password("Insert the otp uri: ")?; let result = OTPElement::from_otp_uri(otp_uri.as_str()); otp_uri.zeroize(); result? @@ -84,7 +84,24 @@ impl SubcommandExecutor for AddArgs { } } -fn get_from_args(matches: AddArgs) -> color_eyre::Result { +/// Backstop for the conditional clap rules above: enforce the per-type +/// invariants even if the declarative rules stop firing (e.g. because of an +/// arg id or value-case mismatch, which silently disables them). +fn validate_type_invariants(matches: &AddArgs) -> eyre::Result<()> { + match matches.otp_type { + OTPType::Hotp if matches.counter.is_none() => { + Err(eyre::eyre!("--counter is required for HOTP codes")) + } + OTPType::Yandex | OTPType::Motp if matches.pin.is_none() => Err(eyre::eyre!( + "--pin is required for {} codes", + matches.otp_type + )), + _ => Ok(()), + } +} + +fn get_from_args(matches: AddArgs) -> eyre::Result { + validate_type_invariants(&matches)?; let secret = if matches.take_secret_from_stdin { if let Some(password) = io::stdin().lock().lines().next() { password.map_err(ErrReport::from) @@ -110,3 +127,108 @@ fn map_args_to_code(secret: String, matches: AddArgs) -> Result { .pin(matches.pin) .build() } + +#[cfg(test)] +mod tests { + use clap::Parser; + + use super::{AddArgs, validate_type_invariants}; + use crate::otp::otp_type::OTPType; + + #[derive(Parser)] + struct TestParser { + #[command(flatten)] + args: AddArgs, + } + + fn parse(args: &[&str]) -> Result { + TestParser::try_parse_from(args).map(|parsed| parsed.args) + } + + #[test] + fn hotp_without_counter_is_rejected() { + let result = parse(&["add", "-l", "label", "-t", "hotp"]); + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().kind(), + clap::error::ErrorKind::MissingRequiredArgument + ); + } + + #[test] + fn hotp_with_counter_is_accepted() { + let args = parse(&["add", "-l", "label", "-t", "hotp", "-c", "42"]).unwrap(); + assert_eq!(args.counter, Some(42)); + } + + #[test] + fn yandex_without_pin_is_rejected() { + let result = parse(&["add", "-l", "label", "-t", "yandex"]); + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().kind(), + clap::error::ErrorKind::MissingRequiredArgument + ); + } + + #[test] + fn motp_without_pin_is_rejected() { + let result = parse(&["add", "-l", "label", "-t", "motp"]); + assert!(result.is_err()); + assert_eq!( + result.err().unwrap().kind(), + clap::error::ErrorKind::MissingRequiredArgument + ); + } + + #[test] + fn yandex_with_pin_is_accepted() { + let args = parse(&["add", "-l", "label", "-t", "yandex", "-p", "5678"]).unwrap(); + assert_eq!(args.pin.as_deref(), Some("5678")); + } + + #[test] + fn steam_defaults_to_five_digits() { + let args = parse(&["add", "-l", "label", "-t", "steam"]).unwrap(); + assert_eq!(args.digits, 5); + } + + #[test] + fn steam_explicit_digits_are_kept() { + let args = parse(&["add", "-l", "label", "-t", "steam", "-d", "7"]).unwrap(); + assert_eq!(args.digits, 7); + } + + #[test] + fn totp_defaults_to_six_digits() { + let args = parse(&["add", "-l", "label"]).unwrap(); + assert_eq!(args.digits, 6); + assert_eq!(args.otp_type, OTPType::Totp); + } + + #[test] + fn backstop_rejects_hotp_without_counter() { + let mut args = parse(&["add", "-l", "label", "-t", "hotp", "-c", "42"]).unwrap(); + // Simulate the clap rule rotting away again + args.counter = None; + assert!(validate_type_invariants(&args).is_err()); + } + + #[test] + fn backstop_rejects_yandex_and_motp_without_pin() { + for otp_type in ["yandex", "motp"] { + let mut args = parse(&["add", "-l", "label", "-t", otp_type, "-p", "1234"]).unwrap(); + // Simulate the clap rule rotting away again + args.pin = None; + assert!(validate_type_invariants(&args).is_err()); + } + } + + #[test] + fn backstop_accepts_valid_combinations() { + let args = parse(&["add", "-l", "label", "-t", "hotp", "-c", "1"]).unwrap(); + assert!(validate_type_invariants(&args).is_ok()); + let args = parse(&["add", "-l", "label"]).unwrap(); + assert!(validate_type_invariants(&args).is_ok()); + } +} diff --git a/src/arguments/delete.rs b/src/arguments/delete.rs index 8ba40492..39e2951e 100644 --- a/src/arguments/delete.rs +++ b/src/arguments/delete.rs @@ -6,7 +6,7 @@ use std::fs::File; use std::fs::OpenOptions; use clap::Args; -use color_eyre::eyre::eyre; +use eyre::eyre; use crate::otp::otp_element::OTPDatabase; @@ -28,17 +28,34 @@ pub struct DeleteArgs { } impl SubcommandExecutor for DeleteArgs { - fn run_command(self, mut otp_database: OTPDatabase) -> color_eyre::Result { + fn run_command(self, mut otp_database: OTPDatabase) -> eyre::Result { if otp_database.elements_ref().is_empty() { return Err(eyre!("There are no elements to delete")); } - let index_to_delete = self - .index - .and_then(|i| i.checked_sub(1)) - // Match by issues or label if index filter is missing - .or_else(|| get_first_matching_element(&otp_database, &self)) - .ok_or(eyre!("No code has been found using the given arguments"))?; + let index_to_delete = match self.index { + // Indexes are 1-based, as shown by the list subcommand and the TUI. + // Reject 0 explicitly instead of silently falling through to the + // issuer/label matcher (which would target the first element). + Some(0) => { + return Err(eyre!( + "Invalid index 0: indexes are 1-based, use --index 1 for the first code" + )); + } + Some(index) => { + let real_index = index - 1; + if real_index >= otp_database.elements_ref().len() { + return Err(eyre!( + "{index} is an invalid index: the database contains {} codes", + otp_database.elements_ref().len() + )); + } + real_index + } + // Match by issuer or label if the index filter is missing + None => get_first_matching_element(&otp_database, &self) + .ok_or(eyre!("No code has been found using the given arguments"))?, + }; if let Some(element) = otp_database.elements_ref().get(index_to_delete) { print!( @@ -53,17 +70,19 @@ impl SubcommandExecutor for DeleteArgs { if output.trim().eq_ignore_ascii_case("y") { otp_database.delete_element(index_to_delete); - Ok(otp_database) } else { - Err(eyre!("Operation interrupt by the user")) + // Declining the confirmation is not an error: leave the + // database untouched and exit successfully + println!("Deletion aborted, no code has been removed"); } + Ok(otp_database) } else { Err(eyre!("Missing {}th code to delete", index_to_delete + 1)) } } } -fn read_confirmation_line() -> color_eyre::Result { +fn read_confirmation_line() -> eyre::Result { let mut output = String::with_capacity(1); if io::stdin().read_line(&mut output)? > 0 { diff --git a/src/arguments/edit.rs b/src/arguments/edit.rs index 398b57f2..2b73b04c 100644 --- a/src/arguments/edit.rs +++ b/src/arguments/edit.rs @@ -1,7 +1,10 @@ use clap::{Args, value_parser}; -use color_eyre::eyre::eyre; +use eyre::eyre; -use crate::otp::{otp_algorithm::OTPAlgorithm, otp_element::OTPDatabase}; +use crate::otp::{ + otp_algorithm::OTPAlgorithm, + otp_element::{OTPDatabase, OTPElement, OTPElementBuilder}, +}; use super::SubcommandExecutor; @@ -45,10 +48,11 @@ pub struct EditArgs { } impl SubcommandExecutor for EditArgs { - fn run_command(self, mut database: OTPDatabase) -> color_eyre::Result { + fn run_command(self, mut database: OTPDatabase) -> eyre::Result { let secret = self .change_secret - .then(|| rpassword::prompt_password("Insert the secret: ").unwrap()); + .then(|| rpassword::prompt_password("Insert the secret: ")) + .transpose()?; // User provides row number from dashboard which is equal to the array index plus one let index = self.index; @@ -60,6 +64,7 @@ impl SubcommandExecutor for EditArgs { match database.mut_element(real_index) { Some(element) => { + let unmodified_element = element.clone(); if let Some(v) = self.issuer { element.issuer = v; } @@ -82,9 +87,13 @@ impl SubcommandExecutor for EditArgs { element.pin = self.pin; } if let Some(s) = secret { - element.secret = s; + element.secret = validate_secret(element, s)?; + } + // Only persist (re-encrypt and rewrite the database) if + // the edit actually changed something + if *element != unmodified_element { + database.mark_modified(); } - database.mark_modified(); } None => return Err(eyre!("No element found at index {index}")), } @@ -94,3 +103,22 @@ impl SubcommandExecutor for EditArgs { } } } + +/// Run the new secret through the same validation and case normalization that +/// `add` gets via OTPElementBuilder (base32/hex checks depending on the OTP +/// type), instead of persisting the raw string and only failing later at code +/// generation time. +fn validate_secret(element: &OTPElement, secret: String) -> eyre::Result { + let validated = OTPElementBuilder::default() + .secret(secret) + .issuer(element.issuer.as_str()) + .label(element.label.as_str()) + .digits(element.digits) + .type_(element.type_) + .algorithm(element.algorithm) + .period(element.period) + .counter(element.counter) + .pin(element.pin.clone()) + .build()?; + Ok(validated.secret.clone()) +} diff --git a/src/arguments/export.rs b/src/arguments/export.rs index 04fb1818..d8ef3a94 100644 --- a/src/arguments/export.rs +++ b/src/arguments/export.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use clap::Args; -use color_eyre::eyre::eyre; +use eyre::WrapErr; use crate::{ exporters::{do_export, otp_uri::OtpUriList}, @@ -42,39 +42,65 @@ pub struct ExportFormat { pub freeotp_plus: bool, } -impl Default for ExportFormat { - fn default() -> Self { - Self { - cotp: true, - andotp: false, - otp_uri: false, - freeotp_plus: false, - } +/// The export format selected on the command line, derived from the mutually +/// exclusive [`ExportFormat`] flags. +#[derive(Clone, Copy)] +enum ExportKind { + Cotp, + Andotp, + OtpUri, + FreeOtpPlus, +} + +impl ExportFormat { + /// Maps the mutually exclusive clap flags to the selected export format. + /// + /// The clap `ArgGroup` on [`ExportFormat`] (`multiple = false`) combined + /// with the `Option` flattening in [`ExportArgs`] guarantees exactly one + /// flag is set whenever this struct is present, so exactly one entry of + /// the table below is enabled. + fn kind(&self) -> ExportKind { + let flag_table = [ + (self.cotp, ExportKind::Cotp), + (self.andotp, ExportKind::Andotp), + (self.otp_uri, ExportKind::OtpUri), + (self.freeotp_plus, ExportKind::FreeOtpPlus), + ]; + flag_table + .into_iter() + .find_map(|(enabled, kind)| enabled.then_some(kind)) + .expect("clap ArgGroup guarantees exactly one export format flag") } } impl SubcommandExecutor for ExportArgs { - fn run_command(self, database: OTPDatabase) -> color_eyre::Result { - let export_format = self.format.unwrap_or_default(); + fn run_command(self, database: OTPDatabase) -> eyre::Result { + // Exporting to the cotp format when no flag is given keeps the + // historical default behavior. + let export_kind = self + .format + .as_ref() + .map_or(ExportKind::Cotp, ExportFormat::kind); let exported_path = if self.path.is_dir() { self.path.join("exported.cotp") } else { self.path }; - if export_format.cotp { - do_export(&database, exported_path) - } else if export_format.andotp { - let andotp: &Vec = (&database).into(); - do_export(&andotp, exported_path) - } else if export_format.otp_uri { - let otp_uri_list: OtpUriList = (&database).into(); - do_export(&otp_uri_list, exported_path) - } else if export_format.freeotp_plus { - let freeotp_plus: FreeOTPPlusJson = (&database).try_into()?; - do_export(&freeotp_plus, exported_path) - } else { - unreachable!("Unreachable code"); + match export_kind { + ExportKind::Cotp => do_export(&database, exported_path), + ExportKind::Andotp => { + let andotp: &[OTPElement] = (&database).into(); + do_export(&andotp, exported_path) + } + ExportKind::OtpUri => { + let otp_uri_list: OtpUriList = (&database).into(); + do_export(&otp_uri_list, exported_path) + } + ExportKind::FreeOtpPlus => { + let freeotp_plus: FreeOTPPlusJson = (&database).try_into()?; + do_export(&freeotp_plus, exported_path) + } } .map(|path| { println!( @@ -83,6 +109,6 @@ impl SubcommandExecutor for ExportArgs { ); database }) - .map_err(|e| eyre!("An error occurred while exporting database: {e}")) + .wrap_err("An error occurred while exporting database") } } diff --git a/src/arguments/extract.rs b/src/arguments/extract.rs index 0f271303..5a704e3e 100644 --- a/src/arguments/extract.rs +++ b/src/arguments/extract.rs @@ -1,8 +1,7 @@ use crate::otp::otp_element::OTPDatabase; use crate::{clipboard, otp::otp_element::OTPElement}; use clap::Args; -use color_eyre::eyre::eyre; -use globset::{GlobBuilder, GlobMatcher}; +use eyre::eyre; use super::SubcommandExecutor; @@ -12,11 +11,11 @@ pub struct ExtractArgs { #[arg(short, long, required_unless_present_any = ["issuer", "label"])] pub index: Option, - /// Code issuer, may be a glob pattern + /// Code issuer, may be a wildcard pattern (`*` and `?`) #[arg(short = 's', long, required_unless_present_any = ["index", "label"])] pub issuer: Option, - /// Code label, may be a glob pattern + /// Code label, may be a wildcard pattern (`*` and `?`) #[arg(short, long, required_unless_present_any = ["index", "issuer"])] pub label: Option, @@ -25,52 +24,75 @@ pub struct ExtractArgs { pub copy_to_clipboard: bool, } -// Contains glob filters for each field we can filter on -struct ExtractFilterGlob { - issuer_glob: Option, - label_glob: Option, +// Contains wildcard filters for each field we can filter on +struct ExtractFilter { + issuer_pattern: Option, + label_pattern: Option, index: Option, } -impl TryFrom for ExtractFilterGlob { - type Error = color_eyre::eyre::ErrReport; +impl TryFrom for ExtractFilter { + type Error = eyre::ErrReport; fn try_from(value: ExtractArgs) -> Result { - let issuer_glob = if let Some(issuer) = value.issuer { - Some(create_matcher(&issuer)?) - } else { - None - }; - - let label_glob = if let Some(label) = value.label { - Some(create_matcher(&label)?) - } else { - None - }; + if value.index == Some(0) { + return Err(eyre!( + "Invalid index 0: indexes are 1-based, use --index 1 for the first code" + )); + } Ok(Self { - issuer_glob, - label_glob, + issuer_pattern: value.issuer, + label_pattern: value.label, index: value.index, }) } } -fn create_matcher( - glob: &str, -) -> Result>::Error> { - Ok(GlobBuilder::new(glob) - .case_insensitive(true) - .build()? - .compile_matcher()) +/// Case-insensitive wildcard matching of the whole `text` against `pattern`, +/// where `*` matches any (possibly empty) sequence of characters and `?` +/// matches exactly one character. +/// +/// This replaces the former globset-based matcher, which pulled the whole +/// regex engine into the binary for this simple use case. +fn wildcard_match(pattern: &str, text: &str) -> bool { + let pattern: Vec = pattern.to_lowercase().chars().collect(); + let text: Vec = text.to_lowercase().chars().collect(); + + // Iterative two-pointer matching with backtracking to the last `*` + let mut p = 0; // position in pattern + let mut t = 0; // position in text + let mut star: Option = None; // position of the last `*` seen + let mut star_t = 0; // position in text when the last `*` was seen + + while t < text.len() { + if p < pattern.len() && (pattern[p] == '?' || pattern[p] == text[t]) { + p += 1; + t += 1; + } else if p < pattern.len() && pattern[p] == '*' { + star = Some(p); + star_t = t; + p += 1; + } else if let Some(star_p) = star { + // Backtrack: let the last `*` consume one more character + p = star_p + 1; + star_t += 1; + t = star_t; + } else { + return false; + } + } + + // Only trailing `*`s may remain in the pattern + pattern[p..].iter().all(|&c| c == '*') } impl SubcommandExecutor for ExtractArgs { - fn run_command(self, otp_database: OTPDatabase) -> color_eyre::Result { + fn run_command(self, otp_database: OTPDatabase) -> eyre::Result { let copy_to_clipboard = self.copy_to_clipboard; - let globbed: ExtractFilterGlob = self.try_into()?; + let filter: ExtractFilter = self.try_into()?; - let first_with_filters = find_match(&otp_database, globbed); + let first_with_filters = find_match(&otp_database, filter); if let Some(otp) = first_with_filters { let code = otp.get_otp_code()?; @@ -86,27 +108,29 @@ impl SubcommandExecutor for ExtractArgs { } } -fn find_match(otp_database: &OTPDatabase, globbed: ExtractFilterGlob) -> Option<&OTPElement> { +fn find_match(otp_database: &OTPDatabase, filter: ExtractFilter) -> Option<&OTPElement> { otp_database - .elements + .elements_ref() .iter() .enumerate() - .find(|(index, code)| filter_extract(&globbed, *index, code)) + .find(|(index, code)| filter_extract(&filter, *index, code)) .map(|(_, code)| code) } -fn filter_extract(args: &ExtractFilterGlob, index: usize, candidate: &OTPElement) -> bool { - let match_by_index = args.index.is_none_or(|i| i == index); +fn filter_extract(args: &ExtractFilter, index: usize, candidate: &OTPElement) -> bool { + // The user-facing index is 1-based (like list, edit, delete and the TUI), + // while `index` here is the 0-based position in the database + let match_by_index = args.index.is_none_or(|i| i.checked_sub(1) == Some(index)); let match_by_issuer = args - .issuer_glob + .issuer_pattern .as_ref() - .is_none_or(|issuer| issuer.is_match(&candidate.issuer)); + .is_none_or(|issuer| wildcard_match(issuer, &candidate.issuer)); let match_by_label = args - .label_glob + .label_pattern .as_ref() - .is_none_or(|label| label.is_match(&candidate.label)); + .is_none_or(|label| wildcard_match(label, &candidate.label)); match_by_index && match_by_issuer && match_by_label } @@ -119,7 +143,7 @@ mod tests { otp::otp_element::{OTPDatabase, OTPElementBuilder}, }; - use super::find_match; + use super::{find_match, wildcard_match}; #[test] fn test_glob_filtering_good_issuer() { @@ -293,6 +317,139 @@ mod tests { assert!(found_match.is_none()); } + #[test] + fn test_index_filtering_is_one_based() { + // Arrange + let mut otp_database = OTPDatabase::default(); + otp_database.add_element( + OTPElementBuilder::default() + .issuer("first-issuer") + .label("first-label") + .secret("AA") + .build() + .unwrap(), + ); + + otp_database.add_element( + OTPElementBuilder::default() + .issuer("second-issuer") + .label("second-label") + .secret("AA") + .build() + .unwrap(), + ); + + // Act / Assert: --index 1 must return the FIRST element + let filter = ExtractArgs { + index: Some(1), + ..Default::default() + }; + let found_match = find_match(&otp_database, filter.try_into().unwrap()); + assert_eq!("first-issuer", found_match.unwrap().issuer); + + // Act / Assert: --index 2 must return the SECOND element + let filter = ExtractArgs { + index: Some(2), + ..Default::default() + }; + let found_match = find_match(&otp_database, filter.try_into().unwrap()); + assert_eq!("second-issuer", found_match.unwrap().issuer); + } + + #[test] + fn test_index_out_of_range_matches_nothing() { + // Arrange + let mut otp_database = OTPDatabase::default(); + otp_database.add_element( + OTPElementBuilder::default() + .issuer("first-issuer") + .label("first-label") + .secret("AA") + .build() + .unwrap(), + ); + + let filter = ExtractArgs { + index: Some(2), + ..Default::default() + }; + + // Act + let found_match = find_match(&otp_database, filter.try_into().unwrap()); + + // Assert + assert!(found_match.is_none()); + } + + #[test] + fn test_index_zero_is_rejected() { + // Arrange + let filter = ExtractArgs { + index: Some(0), + ..Default::default() + }; + + // Act + let result: Result = filter.try_into(); + + // Assert + assert!(result.is_err()); + } + + #[test] + fn test_wildcard_match_literal() { + assert!(wildcard_match("test", "test")); + assert!(!wildcard_match("test", "test2")); + assert!(!wildcard_match("test", "tes")); + } + + #[test] + fn test_wildcard_match_empty_pattern() { + assert!(wildcard_match("", "")); + assert!(!wildcard_match("", "a")); + } + + #[test] + fn test_wildcard_match_star() { + assert!(wildcard_match("*", "")); + assert!(wildcard_match("*", "anything")); + assert!(wildcard_match("test-*", "test-issuer")); + assert!(wildcard_match("*issuer", "test-issuer")); + assert!(wildcard_match("t*t*r", "test-issuer")); + assert!(!wildcard_match("t*t*z", "test-issuer")); + } + + #[test] + fn test_wildcard_match_star_collapse() { + assert!(wildcard_match("***", "anything")); + assert!(wildcard_match("a**b", "ab")); + assert!(wildcard_match("a**b", "a-whatever-b")); + assert!(!wildcard_match("a**b", "a-whatever-c")); + } + + #[test] + fn test_wildcard_match_question_mark() { + assert!(wildcard_match("?", "a")); + assert!(!wildcard_match("?", "")); + assert!(!wildcard_match("?", "ab")); + assert!(wildcard_match("te?t", "test")); + assert!(!wildcard_match("te?t", "tet")); + assert!(wildcard_match("?*", "abc")); + } + + #[test] + fn test_wildcard_match_case_insensitive() { + assert!(wildcard_match("TeSt-iSS*", "test-issuer")); + assert!(wildcard_match("test", "TEST")); + } + + #[test] + fn test_wildcard_match_unicode_case() { + assert!(wildcard_match("über*", "ÜBERtest")); + assert!(wildcard_match("ÜBER*", "übertest")); + assert!(wildcard_match("caf?", "CAFÉ")); + } + #[test] fn test_glob_filtering_case_insensitive() { // Arrange diff --git a/src/arguments/import.rs b/src/arguments/import.rs index c9273e46..aa8c0f98 100644 --- a/src/arguments/import.rs +++ b/src/arguments/import.rs @@ -1,7 +1,9 @@ +use std::fs::read_to_string; use std::path::PathBuf; use clap::Args; -use color_eyre::eyre::eyre; +use eyre::eyre; +use zeroize::Zeroize; use crate::{ exporters::otp_uri::OtpUriList, @@ -12,6 +14,7 @@ use crate::{ importer::import_from_path, }, otp::otp_element::{OTPDatabase, OTPElement}, + utils, }; use super::SubcommandExecutor; @@ -75,37 +78,98 @@ pub struct BackupType { pub otp_uri: bool, } +/// The backup format selected on the command line, derived from the mutually +/// exclusive [`BackupType`] flags. +#[derive(Clone, Copy)] +enum ImportFormat { + Cotp, + Andotp, + Aegis, + AegisEncrypted, + FreeOtpPlus, + FreeOtp, + GoogleAuthenticator, + Authy, + AuthyExported, + MicrosoftAuthenticator, + OtpUri, +} + +impl BackupType { + /// Maps the mutually exclusive clap flags to the selected import format. + /// + /// The clap `ArgGroup` on [`BackupType`] (`required = true, multiple = + /// false`) guarantees exactly one flag is set, so exactly one entry of + /// the table below is enabled. + fn format(&self) -> ImportFormat { + let flag_table = [ + (self.cotp, ImportFormat::Cotp), + (self.andotp, ImportFormat::Andotp), + (self.aegis, ImportFormat::Aegis), + (self.aegis_encrypted, ImportFormat::AegisEncrypted), + (self.freeotp_plus, ImportFormat::FreeOtpPlus), + (self.freeotp, ImportFormat::FreeOtp), + (self.google_authenticator, ImportFormat::GoogleAuthenticator), + (self.authy, ImportFormat::Authy), + (self.authy_exported, ImportFormat::AuthyExported), + ( + self.microsoft_authenticator, + ImportFormat::MicrosoftAuthenticator, + ), + (self.otp_uri, ImportFormat::OtpUri), + ]; + flag_table + .into_iter() + .find_map(|(enabled, format)| enabled.then_some(format)) + .expect("clap ArgGroup guarantees exactly one import format flag") + } +} + impl SubcommandExecutor for ImportArgs { - fn run_command(self, mut database: OTPDatabase) -> color_eyre::Result { + fn run_command(self, mut database: OTPDatabase) -> eyre::Result { let path = self.path; - let backup_type = self.backup_type; - - let result = if backup_type.cotp { - import_from_path::(path) - } else if backup_type.andotp { - import_from_path::>(path) - } else if backup_type.aegis { - import_from_path::(path) - } else if backup_type.aegis_encrypted { - import_from_path::(path) - } else if backup_type.freeotp_plus { - import_from_path::(path) - } else if backup_type.authy_exported { - import_from_path::(path) - } else if backup_type.google_authenticator { - import_from_google_authenticator(path) - } else if backup_type.authy || backup_type.microsoft_authenticator || backup_type.freeotp { - import_from_path::(path) - } else if backup_type.otp_uri { - import_from_path::(path) - } else { - return Err(eyre!("Invalid arguments provided")); + let result = match self.backup_type.format() { + ImportFormat::Cotp => import_from_path::(path), + ImportFormat::Andotp => import_from_path::>(path), + ImportFormat::Aegis => import_from_path::(path), + ImportFormat::AegisEncrypted => import_aegis_encrypted(path), + ImportFormat::FreeOtpPlus => import_from_path::(path), + ImportFormat::AuthyExported => import_from_path::(path), + ImportFormat::GoogleAuthenticator => import_from_google_authenticator(path), + // Authy, Microsoft Authenticator and FreeOTP backups are + // pre-converted by the Python scripts in converters/ into the + // same intermediate JSON shape. + ImportFormat::Authy | ImportFormat::MicrosoftAuthenticator | ImportFormat::FreeOtp => { + import_from_path::(path) + } + ImportFormat::OtpUri => import_from_path::(path), }; - let elements = result.map_err(|e| eyre!("{e}"))?; + let elements = result?; database.add_all(elements); Ok(database) } } + +/// Imports an encrypted Aegis backup, prompting the user for the backup +/// password before decrypting it. +fn import_aegis_encrypted(path: PathBuf) -> eyre::Result> { + let json = read_to_string(path)?; + let encrypted: AegisEncryptedDatabase = serde_json::from_str(json.as_str()).map_err(|e| { + eyre!( + "Invalid JSON import format. + Please check the file you are trying to import. For further information please check these guidelines: + https://github.com/replydev/cotp?tab=readme-ov-file#migration-from-other-apps + + Specific error: {e}" + ) + })?; + + let mut password = utils::try_password("Insert your Aegis password: ", 0)?; + let result = encrypted.decrypt(password.as_str()); + password.zeroize(); + + result +} diff --git a/src/arguments/list.rs b/src/arguments/list.rs index dd53c261..d2144090 100644 --- a/src/arguments/list.rs +++ b/src/arguments/list.rs @@ -1,5 +1,5 @@ use clap::Args; -use color_eyre::eyre::{Result, eyre}; +use eyre::eyre; use serde::Serialize; use crate::otp::otp_element::{OTPDatabase, OTPElement}; @@ -43,38 +43,46 @@ struct JsonOtpList<'a> { otp_code: String, } -impl<'a> TryFrom<&'a OTPElement> for JsonOtpList<'a> { - type Error = color_eyre::eyre::Error; - - fn try_from(value: &'a OTPElement) -> Result { - let otp_code = value.get_otp_code()?; - Ok(JsonOtpList { +impl<'a> From<&'a OTPElement> for JsonOtpList<'a> { + fn from(value: &'a OTPElement) -> Self { + // Degrade per-element like the table view does: an uncomputable code + // (e.g. HOTP missing its counter) must not make the whole listing fail + let otp_code = value + .get_otp_code() + .unwrap_or_else(|error| error.to_string()); + JsonOtpList { issuer: &value.issuer, label: &value.label, otp_code, - }) + } } } const NO_ISSUER_TEXT: &str = ""; impl SubcommandExecutor for ListArgs { - fn run_command(self, otp_database: OTPDatabase) -> color_eyre::Result { + fn run_command(self, otp_database: OTPDatabase) -> eyre::Result { if self.format.unwrap_or_default().json { let json_elements = otp_database - .elements + .elements_ref() .iter() - .map(TryInto::try_into) - .collect::>>()?; + .map(Into::into) + .collect::>(); let stringified = serde_json::to_string_pretty(&json_elements) .map_err(|e| eyre!("Error during JSON serialization: {:?}", e))?; - print!("{stringified}"); + println!("{stringified}"); } else { - if otp_database.elements.is_empty() { + if otp_database.elements_ref().is_empty() { println!("No elements to list"); return Ok(otp_database); } + + const ISSUER_HEADER: &str = "Issuer"; + const LABEL_HEADER: &str = "Label"; + + // Clamp column widths to at least the header lengths so short + // issuers/labels can never underflow the padding computation let issuer_width = calculate_width(&otp_database, |element| { let issuer_length = element.issuer.chars().count(); if issuer_length > 0 { @@ -82,37 +90,32 @@ impl SubcommandExecutor for ListArgs { } else { NO_ISSUER_TEXT.chars().count() } - }); + }) + .max(ISSUER_HEADER.chars().count()); let label_width = - calculate_width(&otp_database, |element| element.label.chars().count()); + calculate_width(&otp_database, |element| element.label.chars().count()) + .max(LABEL_HEADER.chars().count()); println!( - "{0: <6} {1} {2} {3: <10}", - "Index", - "Issuer".to_owned() + " ".repeat(issuer_width - 6).as_ref(), - "Label".to_owned() + " ".repeat(label_width - 5).as_ref(), - "OTP", + "{0: <6} {1: usize, { otp_database - .elements + .elements_ref() .iter() .map(get_number_of_chars) .max() diff --git a/src/arguments/mod.rs b/src/arguments/mod.rs index d4de5148..d630a8f4 100644 --- a/src/arguments/mod.rs +++ b/src/arguments/mod.rs @@ -1,9 +1,9 @@ use crate::otp::otp_element::OTPDatabase; use crate::{arguments::extract::ExtractArgs, dashboard}; use clap::{Parser, Subcommand}; -use color_eyre::eyre::eyre; use delete::DeleteArgs; use enum_dispatch::enum_dispatch; +use eyre::eyre; use self::{ add::AddArgs, edit::EditArgs, export::ExportArgs, import::ImportArgs, list::ListArgs, @@ -22,7 +22,7 @@ mod passwd; /// Common trait the all the Subcommands must implement to define the command logic #[enum_dispatch] pub trait SubcommandExecutor { - fn run_command(self, otp_database: OTPDatabase) -> color_eyre::Result; + fn run_command(self, otp_database: OTPDatabase) -> eyre::Result; } /// Main structure defining the Clap argument for the cotp commandline utility @@ -61,7 +61,7 @@ pub enum CotpSubcommands { Passwd(PasswdArgs), } -pub fn args_parser(matches: CotpArgs, read_result: OTPDatabase) -> color_eyre::Result { +pub fn args_parser(matches: CotpArgs, read_result: OTPDatabase) -> eyre::Result { if let Some(command) = matches.command { command.run_command(read_result) } else { diff --git a/src/arguments/passwd.rs b/src/arguments/passwd.rs index 1ca741fb..680f2151 100644 --- a/src/arguments/passwd.rs +++ b/src/arguments/passwd.rs @@ -1,7 +1,7 @@ use clap::Args; use zeroize::Zeroize; -use crate::{otp::otp_element::OTPDatabase, utils}; +use crate::{otp::otp_element::OTPDatabase, path::DATABASE_PATH, storage, utils}; use super::SubcommandExecutor; @@ -9,9 +9,11 @@ use super::SubcommandExecutor; pub struct PasswdArgs; impl SubcommandExecutor for PasswdArgs { - fn run_command(self, mut database: OTPDatabase) -> color_eyre::Result { - let mut new_password = utils::verified_password("New password: ", 8); - database.save_with_pw(&new_password)?; + fn run_command(self, mut database: OTPDatabase) -> eyre::Result { + let mut new_password = utils::try_verified_password("New password: ", 8)?; + // Saves with a key derived from the new password and clears the + // modified flag, so main() will not save again with the old key + storage::save_with_pw(&mut database, &new_password, DATABASE_PATH.get().unwrap())?; new_password.zeroize(); Ok(database) } diff --git a/src/clipboard.rs b/src/clipboard.rs index 2efea28d..e78ffa8e 100644 --- a/src/clipboard.rs +++ b/src/clipboard.rs @@ -1,11 +1,11 @@ -use base64::{Engine as _, engine::general_purpose}; -use color_eyre::eyre::eyre; use copypasta_ext::prelude::*; #[cfg(target_os = "linux")] use copypasta_ext::wayland_bin::WaylandBinClipboardContext; use copypasta_ext::x11_bin::ClipboardContext as BinClipboardContext; use copypasta_ext::x11_fork::ClipboardContext as ForkClipboardContext; use crossterm::style::Print; +use data_encoding::BASE64; +use eyre::eyre; use std::{env, io}; pub enum CopyType { @@ -13,7 +13,7 @@ pub enum CopyType { OSC52, } -pub fn copy_string_to_clipboard(content: &str) -> color_eyre::Result { +pub fn copy_string_to_clipboard(content: &str) -> eyre::Result { if ssh_clipboard(content) { Ok(CopyType::OSC52) } else if wayland_clipboard(content) || other_platform_clipboard(content) { @@ -27,11 +27,12 @@ fn ssh_clipboard(content: &str) -> bool { env_var_set("SSH_CONNECTION") // We do not use copypasta_ext::osc52 module because we have enabled terminal raw mode, so we print with crossterm utilities // Check https://github.com/timvisee/rust-clipboard-ext/blob/371df19d2f961882a21c957f396d1e24548d1f28/src/osc52.rs#L92 + // Write to stderr: the TUI owns stderr, while stdout must stay clean for piping && crossterm::execute!( - io::stdout(), + io::stderr(), Print(format!( "\x1B]52;c;{}\x07", - general_purpose::STANDARD.encode(content) + BASE64.encode(content.as_bytes()) )) ) .is_ok() diff --git a/src/crypto/cryptography.rs b/src/crypto/cryptography.rs index 3ca47f8f..1e141715 100644 --- a/src/crypto/cryptography.rs +++ b/src/crypto/cryptography.rs @@ -1,8 +1,8 @@ use argon2::{Config, ThreadMode, Variant, Version}; use chacha20poly1305::aead::Aead; use chacha20poly1305::{KeyInit, XChaCha20Poly1305, XNonce}; -use color_eyre::eyre::{ErrReport, eyre}; use data_encoding::BASE64; +use eyre::{ErrReport, eyre}; use super::encrypted_database::EncryptedDatabase; @@ -18,14 +18,18 @@ const KEY_DERIVATION_CONFIG: Config = Config { secret: &[], ad: &[], hash_length: XCHACHA20_POLY1305_KEY_LENGTH as u32, - thread_mode: ThreadMode::Sequential, + // Parallel only changes the execution strategy: the derived key depends on + // the lane count (4), not on how many threads compute the lanes, so + // existing databases decrypt identically (see + // test_derived_key_unchanged_by_thread_mode). + thread_mode: ThreadMode::Parallel, }; -pub fn argon_derive_key(password_bytes: &[u8], salt: &[u8]) -> color_eyre::Result> { +pub fn argon_derive_key(password_bytes: &[u8], salt: &[u8]) -> eyre::Result> { argon2::hash_raw(password_bytes, salt, &KEY_DERIVATION_CONFIG).map_err(ErrReport::from) } -pub fn gen_salt() -> color_eyre::Result<[u8; ARGON2ID_SALT_LENGTH]> { +pub fn gen_salt() -> eyre::Result<[u8; ARGON2ID_SALT_LENGTH]> { let mut salt: [u8; ARGON2ID_SALT_LENGTH] = [0; ARGON2ID_SALT_LENGTH]; getrandom::fill(&mut salt).map_err(|e| eyre!(e))?; Ok(salt) @@ -35,7 +39,7 @@ pub fn encrypt_string_with_key( plain_text: &str, key: &Vec, salt: &[u8], -) -> color_eyre::Result { +) -> eyre::Result { let aead = XChaCha20Poly1305::new_from_slice(key.as_slice()) .map_err(|e| eyre!("Invalid encryption key length: {e}"))?; let mut nonce_bytes: [u8; XCHACHA20_POLY1305_NONCE_LENGTH] = @@ -58,17 +62,19 @@ pub fn encrypt_string_with_key( pub fn decrypt_string( encrypted_text: &str, password: &str, -) -> color_eyre::Result<(String, Vec, Vec)> { +) -> eyre::Result<(String, Vec, Vec)> { //encrypted text is an encrypted database json serialized object let encrypted_database: EncryptedDatabase = serde_json::from_str(encrypted_text) .map_err(|e| eyre!("Error during encrypted database deserialization: {e}"))?; let nonce = BASE64 .decode(encrypted_database.nonce().as_bytes()) - .expect("Cannot decode Base64 nonce"); + .map_err(|e| eyre!("database file is corrupted: cannot decode Base64 nonce: {e}"))?; let cipher_text = BASE64 .decode(encrypted_database.cipher().as_bytes()) - .expect("Cannot decode Base64 cipher"); - let salt = BASE64.decode(encrypted_database.salt().as_bytes()).unwrap(); + .map_err(|e| eyre!("database file is corrupted: cannot decode Base64 cipher: {e}"))?; + let salt = BASE64 + .decode(encrypted_database.salt().as_bytes()) + .map_err(|e| eyre!("database file is corrupted: cannot decode Base64 salt: {e}"))?; let key: Vec = argon_derive_key(password.as_bytes(), salt.as_slice())?; @@ -98,4 +104,49 @@ mod tests { decrypt_string(&serde_json::to_string(&encrypted).unwrap(), "pa$$w0rd").unwrap(); assert_eq!(String::from("Secret data@#[]ò"), decrypted); } + + /// The expected value below was captured from the previous + /// `ThreadMode::Sequential` configuration. It must never change: the lane + /// count (4) is the Argon2 hash parameter, while the thread mode is only + /// the execution strategy, so switching to `ThreadMode::Parallel` must + /// derive the exact same key and keep existing databases decryptable. + #[test] + fn test_derived_key_unchanged_by_thread_mode() { + let key = argon_derive_key(b"pa$$w0rd", b"0123456789abcdef").unwrap(); + let hex: String = key.iter().map(|b| format!("{b:02x}")).collect(); + assert_eq!( + hex, + "1bae31857cc03a6fbb54f463a991e09e3294bdf3b8c44e4ddcec4ec1d7d6a4a7" + ); + } + + #[test] + fn test_decrypt_invalid_base64_nonce_returns_error() { + let corrupted = + r#"{"version":1,"nonce":"!!!not-base64!!!","salt":"c2FsdA==","cipher":"Y2lwaGVy"}"#; + let result = decrypt_string(corrupted, "pa$$w0rd"); + let error = result.expect_err("corrupted nonce must not panic"); + assert!(error.to_string().contains("database file is corrupted")); + assert!(error.to_string().contains("nonce")); + } + + #[test] + fn test_decrypt_invalid_base64_cipher_returns_error() { + let corrupted = + r#"{"version":1,"nonce":"bm9uY2U=","salt":"c2FsdA==","cipher":"!!!not-base64!!!"}"#; + let result = decrypt_string(corrupted, "pa$$w0rd"); + let error = result.expect_err("corrupted cipher must not panic"); + assert!(error.to_string().contains("database file is corrupted")); + assert!(error.to_string().contains("cipher")); + } + + #[test] + fn test_decrypt_invalid_base64_salt_returns_error() { + let corrupted = + r#"{"version":1,"nonce":"bm9uY2U=","salt":"!!!not-base64!!!","cipher":"Y2lwaGVy"}"#; + let result = decrypt_string(corrupted, "pa$$w0rd"); + let error = result.expect_err("corrupted salt must not panic"); + assert!(error.to_string().contains("database file is corrupted")); + assert!(error.to_string().contains("salt")); + } } diff --git a/src/exporters/andotp.rs b/src/exporters/andotp.rs index a5a48469..b23fa1b7 100644 --- a/src/exporters/andotp.rs +++ b/src/exporters/andotp.rs @@ -1,14 +1,15 @@ use crate::otp::otp_element::{OTPDatabase, OTPElement}; +/// andOTP backups are plain JSON arrays of OTP elements type AndOtpDatabase = Vec; impl From for AndOtpDatabase { fn from(value: OTPDatabase) -> Self { - value.elements + value.into_elements() } } -impl<'a> From<&'a OTPDatabase> for &'a AndOtpDatabase { +impl<'a> From<&'a OTPDatabase> for &'a [OTPElement] { fn from(value: &'a OTPDatabase) -> Self { - &value.elements + value.elements_ref() } } diff --git a/src/exporters/freeotp_plus.rs b/src/exporters/freeotp_plus.rs index ece11376..7ce831cd 100644 --- a/src/exporters/freeotp_plus.rs +++ b/src/exporters/freeotp_plus.rs @@ -1,5 +1,5 @@ -use color_eyre::eyre::{ErrReport, Result}; use data_encoding::BASE32_NOPAD; +use eyre::{ErrReport, Result}; use crate::{ importers::freeotp_plus::{FreeOTPElement, FreeOTPPlusJson}, @@ -10,7 +10,7 @@ impl TryFrom<&OTPDatabase> for FreeOTPPlusJson { type Error = ErrReport; fn try_from(otp_database: &OTPDatabase) -> Result { otp_database - .elements + .elements_ref() .iter() .map(TryInto::try_into) .collect::, ErrReport>>() diff --git a/src/exporters/mod.rs b/src/exporters/mod.rs index 04ee935f..1098aeb6 100644 --- a/src/exporters/mod.rs +++ b/src/exporters/mod.rs @@ -1,5 +1,10 @@ -use std::{fs::File, io::Write, path::PathBuf}; +use std::{ + fs::OpenOptions, + io::{self, Write}, + path::{Path, PathBuf}, +}; +use eyre::eyre; use serde::Serialize; use zeroize::Zeroize; @@ -7,22 +12,88 @@ pub mod andotp; pub mod freeotp_plus; pub mod otp_uri; -pub fn do_export(to_be_saved: &T, exported_path: PathBuf) -> Result +pub fn do_export(to_be_saved: &T, exported_path: PathBuf) -> eyre::Result where T: ?Sized + Serialize, { - match serde_json::to_string(to_be_saved) { - Ok(mut contents) => { - if contents == "[]" { - return Err("No contents to export, skipping...".to_owned()); - } - let mut file = File::create(&exported_path).expect("Cannot create file"); - let contents_bytes = contents.as_bytes(); - file.write_all(contents_bytes) - .expect("Failed to write contents"); - contents.zeroize(); + let mut contents = match serde_json::to_string(to_be_saved) { + Ok(contents) => contents, + Err(e) => return Err(eyre!("Failed to serialize the export: {e}")), + }; + if contents == "[]" { + contents.zeroize(); + return Err(eyre!("No contents to export, skipping...")); + } + let write_result = write_secret_file(&exported_path, contents.as_bytes()); + contents.zeroize(); + match write_result { + Ok(()) => { + eprintln!( + "Warning: the exported file contains your OTP secrets in PLAIN TEXT. Keep it safe and delete it as soon as it is no longer needed." + ); Ok(exported_path) } - Err(e) => Err(format!("{e:?}")), + Err(e) => Err(eyre!( + "Cannot export to file {}: {e}", + exported_path.display() + )), + } +} + +/// Writes the plain text export, creating the file with owner-only permissions +/// (0600) on Unix so other local users cannot read the exported secrets. +fn write_secret_file(path: &Path, contents: &[u8]) -> io::Result<()> { + let mut options = OpenOptions::new(); + options.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options.open(path)?; + file.write_all(contents) +} + +#[cfg(test)] +mod tests { + use super::do_export; + + #[test] + fn export_error_is_propagated_instead_of_panicking() { + let result = do_export( + &vec!["some content"], + std::path::PathBuf::from("/nonexistent-dir/never/created/export.json"), + ); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .starts_with("Cannot export to file") + ); + } + + #[test] + fn empty_export_is_skipped() { + let empty: Vec = vec![]; + let result = do_export(&empty, std::path::PathBuf::from("unused.json")); + assert_eq!( + result.unwrap_err().to_string(), + "No contents to export, skipping..." + ); + } + + #[cfg(unix)] + #[test] + fn exported_file_is_created_with_owner_only_permissions() { + use std::os::unix::fs::PermissionsExt; + + let dir = std::env::temp_dir().join(format!("cotp-export-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("export.json"); + let exported = do_export(&vec!["secret"], path.clone()).unwrap(); + let mode = std::fs::metadata(&exported).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + std::fs::remove_dir_all(&dir).unwrap(); } } diff --git a/src/exporters/otp_uri.rs b/src/exporters/otp_uri.rs index eacb0ed9..7d63d2c2 100644 --- a/src/exporters/otp_uri.rs +++ b/src/exporters/otp_uri.rs @@ -9,7 +9,7 @@ pub struct OtpUriList { impl<'a> From<&'a OTPDatabase> for OtpUriList { fn from(value: &'a OTPDatabase) -> Self { let items: Vec = value - .elements + .elements_ref() .iter() .map(super::super::otp::otp_element::OTPElement::get_otpauth_uri) .collect(); diff --git a/src/importers/aegis.rs b/src/importers/aegis.rs index a510a5a9..a6daa5c5 100644 --- a/src/importers/aegis.rs +++ b/src/importers/aegis.rs @@ -1,8 +1,8 @@ -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use crate::otp::{otp_algorithm::OTPAlgorithm, otp_element::OTPElement, otp_type::OTPType}; -#[derive(Serialize, Deserialize)] +#[derive(Deserialize)] pub struct AegisJson { //version: u64, //header: AegisHeader, @@ -10,19 +10,12 @@ pub struct AegisJson { } #[derive(Deserialize)] -#[allow(dead_code)] -struct AegisHeader { - //slots: Option, - //params: Option, -} - -#[derive(Serialize, Deserialize)] pub(crate) struct AegisDb { //version: u64, entries: Vec, } -#[derive(Serialize, Deserialize)] +#[derive(Deserialize)] struct AegisElement { r#type: String, //uuid: String, @@ -32,43 +25,128 @@ struct AegisElement { info: AegisInfo, } -impl From for OTPElement { - fn from(value: AegisElement) -> Self { - OTPElement { +impl TryFrom for OTPElement { + type Error = eyre::Report; + + fn try_from(value: AegisElement) -> Result { + let type_ = OTPType::try_from(value.r#type.as_str())?; + let algorithm = OTPAlgorithm::try_from(value.info.algo.as_str())?; + Ok(OTPElement { secret: value.info.secret, issuer: value.issuer, label: value.name, digits: value.info.digits, - type_: OTPType::from(value.r#type.as_str()), - algorithm: OTPAlgorithm::from(value.info.algo.as_str()), + type_, + algorithm, period: value.info.period.unwrap_or(30), counter: value.info.counter, - pin: None, - } + pin: value.info.pin, + }) } } impl TryFrom for Vec { - type Error = String; + type Error = eyre::Report; fn try_from(aegis_db: AegisDb) -> Result { - Ok(aegis_db.entries.into_iter().map(Into::into).collect()) + aegis_db + .entries + .into_iter() + .map(TryInto::try_into) + .collect() } } impl TryFrom for Vec { - type Error = String; + type Error = eyre::Report; fn try_from(aegis_json: AegisJson) -> Result { aegis_json.db.try_into() } } -#[derive(Serialize, Deserialize)] +#[derive(Deserialize)] struct AegisInfo { secret: String, algo: String, digits: u64, period: Option, counter: Option, + #[serde(default)] + pin: Option, +} + +#[cfg(test)] +mod tests { + use super::AegisJson; + use crate::otp::{otp_algorithm::OTPAlgorithm, otp_element::OTPElement, otp_type::OTPType}; + + #[test] + fn test_pin_is_imported() { + let json = r#"{ + "version": 1, + "header": {"slots": null, "params": null}, + "db": { + "version": 2, + "entries": [ + { + "type": "yandex", + "uuid": "00000000-0000-0000-0000-000000000000", + "name": "Label", + "issuer": "Issuer", + "info": { + "secret": "AAAAAAAAAAAAAAAA", + "algo": "SHA256", + "digits": 8, + "period": 30, + "pin": "1234" + } + }, + { + "type": "totp", + "uuid": "00000000-0000-0000-0000-000000000001", + "name": "Label2", + "issuer": "Issuer2", + "info": { + "secret": "BBBBBBBBBBBBBBBB", + "algo": "SHA1", + "digits": 6, + "period": 30 + } + } + ] + } + }"#; + + let deserialized: AegisJson = serde_json::from_str(json).unwrap(); + let elements: Vec = deserialized.try_into().unwrap(); + + assert_eq!( + vec![ + OTPElement { + secret: "AAAAAAAAAAAAAAAA".to_string(), + issuer: "Issuer".to_string(), + label: "Label".to_string(), + digits: 8, + type_: OTPType::Yandex, + algorithm: OTPAlgorithm::Sha256, + period: 30, + counter: None, + pin: Some("1234".to_string()), + }, + OTPElement { + secret: "BBBBBBBBBBBBBBBB".to_string(), + issuer: "Issuer2".to_string(), + label: "Label2".to_string(), + digits: 6, + type_: OTPType::Totp, + algorithm: OTPAlgorithm::Sha1, + period: 30, + counter: None, + pin: None, + } + ], + elements + ); + } } diff --git a/src/importers/aegis_encrypted.rs b/src/importers/aegis_encrypted.rs index 2132663a..2b52640c 100644 --- a/src/importers/aegis_encrypted.rs +++ b/src/importers/aegis_encrypted.rs @@ -1,12 +1,11 @@ use aes_gcm::aead::{Aead, Nonce}; use aes_gcm::{Aes256Gcm, KeyInit}; // Or `Aes128Gcm` -use data_encoding::BASE64; -use hex::FromHex; +use data_encoding::{BASE64, DecodeError, HEXLOWER_PERMISSIVE}; +use eyre::eyre; use serde::Deserialize; use zeroize::Zeroize; use crate::otp::otp_element::OTPElement; -use crate::utils; use scrypt::{Params, scrypt}; use super::aegis::AegisDb; @@ -43,47 +42,51 @@ struct AegisEncryptedSlot { //repaired: Option, } -impl TryFrom for Vec { - type Error = String; - - fn try_from(aegis_encrypted: AegisEncryptedDatabase) -> Result { - let mut password = utils::password("Insert your Aegis password: ", 0); - let master_key: Option> = get_master_key(&aegis_encrypted, &password); - password.zeroize(); +impl AegisEncryptedDatabase { + /// Decrypts the backup contents using the given password and maps the + /// entries into `OTPElement` values. + pub fn decrypt(self, password: &str) -> eyre::Result> { + let master_key: Option> = get_master_key(&self, password); match master_key { Some(mut master_key) => { let content = BASE64 - .decode(aegis_encrypted.db.as_bytes()) - .map_err(|e| format!("Error during base64 decoding: {e:?}"))?; + .decode(self.db.as_bytes()) + .map_err(|e| eyre!("Error during base64 decoding: {e}"))?; let cipher = Aes256Gcm::new_from_slice(master_key.as_slice()) - .map_err(|e| format!("Invalid master key length: {e:?}"))?; + .map_err(|e| eyre!("Invalid master key length: {e}"))?; master_key.zeroize(); - let nonce_bytes = Vec::from_hex(&aegis_encrypted.header.params.nonce) - .expect("Failed to parse hex nonce"); + let nonce_bytes = decode_hex(&self.header.params.nonce) + .map_err(|e| eyre!("Failed to parse hex nonce: {e}"))?; let nonce = Nonce::::try_from(nonce_bytes.as_slice()) - .map_err(|e| format!("Invalid nonce length: {e:?}"))?; + .map_err(|e| eyre!("Invalid nonce length: {e}"))?; let payload = [ content, - Vec::from_hex(&aegis_encrypted.header.params.tag) - .expect("Failed to parse hex tag"), + decode_hex(&self.header.params.tag) + .map_err(|e| eyre!("Failed to parse hex tag: {e}"))?, ] .concat(); let decrypted_db = cipher .decrypt(&nonce, payload.as_slice()) - .map_err(|e| format!("Failed to derive master key: {e:?}"))?; + .map_err(|e| eyre!("Failed to derive master key: {e}"))?; map_results(decrypted_db) } - None => Err("Failed to derive master key".to_string()), + None => Err(eyre!("Failed to derive master key")), } } } +/// Decodes a hex string, accepting both lower- and uppercase digits like the +/// previously used `hex` crate did (Aegis itself writes lowercase). +fn decode_hex(input: &str) -> Result, DecodeError> { + HEXLOWER_PERMISSIVE.decode(input.as_bytes()) +} + fn get_master_key(aegis_encrypted: &AegisEncryptedDatabase, password: &str) -> Option> { let mut master_key: Option> = None; for slot in aegis_encrypted @@ -103,26 +106,50 @@ fn get_master_key(aegis_encrypted: &AegisEncryptedDatabase, password: &str) -> O master_key } -fn map_results(decrypted_db: Vec) -> Result, String> { - let json = String::from_utf8(decrypted_db) - .map_err(|e| format!("Failed to decode from utf-8 bytes: {e:?}"))?; +fn map_results(decrypted_db: Vec) -> eyre::Result> { + let mut json = match String::from_utf8(decrypted_db) { + Ok(json) => json, + Err(e) => { + let error = eyre!("Failed to decode from utf-8 bytes: {}", e.utf8_error()); + e.into_bytes().zeroize(); + return Err(error); + } + }; - serde_json::from_str::(json.as_str()) - .map_err(|e| e.to_string()) - .and_then(TryInto::try_into) + let result = serde_json::from_str::(json.as_str()) + .map_err(eyre::Report::from) + .and_then(TryInto::try_into); + json.zeroize(); + result } -fn get_params(slot: &AegisEncryptedSlot) -> Result { - let n = slot.n.unwrap(); - let p = slot.p.unwrap(); - let r = slot.r.unwrap(); +fn get_params(slot: &AegisEncryptedSlot) -> eyre::Result { + let n = slot + .n + .ok_or(eyre!("Missing scrypt parameter n in backup slot"))?; + let p = slot + .p + .ok_or(eyre!("Missing scrypt parameter p in backup slot"))?; + let r = slot + .r + .ok_or(eyre!("Missing scrypt parameter r in backup slot"))?; - Params::new((n as f32).log2() as u8, r, p) - .map_err(|e| format!("Error during scrypt params creation: {e:?}")) + if !n.is_power_of_two() { + return Err(eyre!( + "Invalid scrypt parameter n: {n} is not a power of two" + )); + } + + Params::new(n.trailing_zeros() as u8, r, p) + .map_err(|e| eyre!("Error during scrypt params creation: {e}")) } -fn calc_master_key(slot: &AegisEncryptedSlot, password: &str) -> Result, String> { - let salt = Vec::from_hex(slot.salt.as_ref().unwrap()).expect("Failed to parse hex salt"); +fn calc_master_key(slot: &AegisEncryptedSlot, password: &str) -> eyre::Result> { + let salt_hex = slot + .salt + .as_ref() + .ok_or(eyre!("Missing salt in backup slot"))?; + let salt = decode_hex(salt_hex).map_err(|e| eyre!("Failed to parse hex salt: {e}"))?; let mut output: [u8; 32] = [0; 32]; let params = get_params(slot)?; @@ -132,23 +159,174 @@ fn calc_master_key(slot: &AegisEncryptedSlot, password: &str) -> Result, ¶ms, output.as_mut_slice(), ) { - return Err(format!("Error during scrypt key derivation: {e:?}")); + return Err(eyre!("Error during scrypt key derivation: {e}")); } let cipher = Aes256Gcm::new_from_slice(output.as_slice()) - .map_err(|e| format!("Invalid derived key length: {e:?}"))?; + .map_err(|e| eyre!("Invalid derived key length: {e}"))?; output.zeroize(); let cipher_text = [ - Vec::from_hex(&slot.key).expect("Failed to parse hex key"), - Vec::from_hex(&slot.key_params.tag).expect("Failed to parse hex tag"), + decode_hex(&slot.key).map_err(|e| eyre!("Failed to parse hex key: {e}"))?, + decode_hex(&slot.key_params.tag).map_err(|e| eyre!("Failed to parse hex tag: {e}"))?, ] .concat(); - let nonce_bytes = Vec::from_hex(&slot.key_params.nonce).expect("Failed to parse hex nonce"); + let nonce_bytes = + decode_hex(&slot.key_params.nonce).map_err(|e| eyre!("Failed to parse hex nonce: {e}"))?; let nonce = Nonce::::try_from(nonce_bytes.as_slice()) - .map_err(|e| format!("Invalid nonce length: {e:?}"))?; + .map_err(|e| eyre!("Invalid nonce length: {e}"))?; cipher .decrypt(&nonce, cipher_text.as_slice()) - .map_err(|e| format!("Failed to derive master key: {e:?}")) + .map_err(|e| eyre!("Failed to derive master key: {e}")) +} + +#[cfg(test)] +mod tests { + use super::{AegisEncryptedDatabase, AegisEncryptedSlot, calc_master_key, get_params}; + + fn slot_from_json(json: &str) -> AegisEncryptedSlot { + serde_json::from_str(json).expect("Invalid test slot JSON") + } + + #[test] + fn decrypt_with_no_usable_slot_returns_error() { + // The only type-1 slot is malformed (missing scrypt parameters), so no + // master key can be derived and decrypt must fail gracefully. + let database: AegisEncryptedDatabase = serde_json::from_str( + r#"{ + "version": 1, + "header": { + "slots": [ + { + "type": 1, + "key": "00", + "key_params": {"nonce": "00", "tag": "00"}, + "salt": "00" + } + ], + "params": {"nonce": "00", "tag": "00"} + }, + "db": "AAAA" + }"#, + ) + .expect("Invalid test database JSON"); + + let result = database.decrypt("password"); + assert_eq!( + "Failed to derive master key", + result.unwrap_err().to_string() + ); + } + + #[test] + fn missing_scrypt_params_return_error() { + let slot = slot_from_json( + r#"{ + "type": 1, + "key": "00", + "key_params": {"nonce": "00", "tag": "00"}, + "salt": "00" + }"#, + ); + + let result = calc_master_key(&slot, "password"); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("Missing scrypt parameter") + ); + } + + #[test] + fn missing_salt_returns_error() { + let slot = slot_from_json( + r#"{ + "type": 1, + "key": "00", + "key_params": {"nonce": "00", "tag": "00"}, + "n": 2, + "r": 8, + "p": 1 + }"#, + ); + + let result = calc_master_key(&slot, "password"); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("Missing salt")); + } + + #[test] + fn non_power_of_two_n_returns_error() { + let slot = slot_from_json( + r#"{ + "type": 1, + "key": "00", + "key_params": {"nonce": "00", "tag": "00"}, + "n": 15000, + "r": 8, + "p": 1, + "salt": "00" + }"#, + ); + + let result = get_params(&slot); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("not a power of two") + ); + } + + #[test] + fn non_hex_salt_returns_error() { + let slot = slot_from_json( + r#"{ + "type": 1, + "key": "00", + "key_params": {"nonce": "00", "tag": "00"}, + "n": 2, + "r": 8, + "p": 1, + "salt": "not-hex" + }"#, + ); + + let result = calc_master_key(&slot, "password"); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("Failed to parse hex salt") + ); + } + + #[test] + fn non_hex_key_returns_error() { + let slot = slot_from_json( + r#"{ + "type": 1, + "key": "zz", + "key_params": {"nonce": "00", "tag": "00"}, + "n": 2, + "r": 8, + "p": 1, + "salt": "00" + }"#, + ); + + let result = calc_master_key(&slot, "password"); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("Failed to parse hex key") + ); + } } diff --git a/src/importers/authy_remote_debug.rs b/src/importers/authy_remote_debug.rs index 290e03f7..4c5607a4 100644 --- a/src/importers/authy_remote_debug.rs +++ b/src/importers/authy_remote_debug.rs @@ -1,96 +1,100 @@ -/* -Import from JSON file exported from a script executed from remote debugging. -For more information see https://gist.github.com/gboudreau/94bb0c11a6209c82418d01a59d958c93 -*/ - -use crate::otp::{otp_algorithm::OTPAlgorithm, otp_element::OTPElement, otp_type::OTPType}; -use serde::Deserialize; - -const URL_INDEX: usize = 3; -const PARAMETERS_INDEX: usize = 1; -const DIGITS_DEFAULT_VALUE: u64 = 6; - -#[derive(Deserialize)] -struct AuthyExportedJsonElement { - name: String, - secret: String, - uri: String, -} - -// Newtype pattern to bypass compiler check for impl From for Vec -// https://rust-unofficial.github.io/patterns/patterns/behavioural/newtype.html -#[derive(Deserialize)] -pub struct AuthyExportedList(Vec); - -impl AuthyExportedJsonElement { - pub fn get_type(&self) -> String { - let default_value = "totp"; - let args: Vec<&str> = self.uri.split('/').collect(); - String::from(*args.get(2).unwrap_or(&default_value)) - } - - pub fn get_digits(&self) -> u64 { - let args: Vec<&str> = self.uri.split('/').collect(); - args.get(URL_INDEX) - .and_then(|s| { - let mut args: Vec<&str> = s.split('?').collect(); - if args.get(PARAMETERS_INDEX).is_some() { - Some(args.swap_remove(PARAMETERS_INDEX)) - } else { - None - } - }) - .and_then(|s| { - let mut args: Vec<&str> = - s.split('&').filter(|s| s.starts_with("digits=")).collect(); - if !args.is_empty() { - Some(args.swap_remove(0)) - } else { - None - } - }) - .and_then(|s| s.parse::().ok()) - .unwrap_or(DIGITS_DEFAULT_VALUE) - } - - pub fn get_issuer(&self) -> String { - let default_value = ""; - let args: Vec<&str> = self.uri.split('/').collect(); - match args.get(3) { - Some(s) => { - let args: Vec<&str> = s.split('?').collect(); - let issuer = args.first().unwrap_or(&default_value); - match urlencoding::decode(issuer) { - Ok(r) => r.into_owned(), - Err(_e) => (*issuer).to_string(), - } - } - None => String::from(default_value), - } - } -} - -impl From for OTPElement { - fn from(input: AuthyExportedJsonElement) -> Self { - let type_ = OTPType::from(input.get_type().as_str()); - let counter: Option = (type_ == OTPType::Hotp).then_some(0); - let digits = input.get_digits(); - OTPElement { - secret: input.secret.to_uppercase().replace('=', ""), - issuer: input.get_issuer(), - label: input.name, - digits, - type_, - algorithm: OTPAlgorithm::Sha1, - period: 30, - counter, - pin: None, - } - } -} - -impl From for Vec { - fn from(exported_list: AuthyExportedList) -> Self { - exported_list.0.into_iter().map(Into::into).collect() - } -} +/* +Import from JSON file exported from a script executed from remote debugging. +For more information see https://gist.github.com/gboudreau/94bb0c11a6209c82418d01a59d958c93 +*/ + +use crate::otp::{otp_algorithm::OTPAlgorithm, otp_element::OTPElement, otp_type::OTPType}; +use serde::Deserialize; + +const URL_INDEX: usize = 3; +const PARAMETERS_INDEX: usize = 1; +const DIGITS_DEFAULT_VALUE: u64 = 6; + +#[derive(Deserialize)] +struct AuthyExportedJsonElement { + name: String, + secret: String, + uri: String, +} + +// Newtype pattern to bypass compiler check for impl From for Vec +// https://rust-unofficial.github.io/patterns/patterns/behavioural/newtype.html +#[derive(Deserialize)] +pub struct AuthyExportedList(Vec); + +impl AuthyExportedJsonElement { + pub fn get_type(&self) -> String { + let default_value = "totp"; + let args: Vec<&str> = self.uri.split('/').collect(); + String::from(*args.get(2).unwrap_or(&default_value)) + } + + pub fn get_digits(&self) -> u64 { + let args: Vec<&str> = self.uri.split('/').collect(); + args.get(URL_INDEX) + .and_then(|s| { + let mut args: Vec<&str> = s.split('?').collect(); + if args.get(PARAMETERS_INDEX).is_some() { + Some(args.swap_remove(PARAMETERS_INDEX)) + } else { + None + } + }) + .and_then(|s| { + let mut args: Vec<&str> = + s.split('&').filter(|s| s.starts_with("digits=")).collect(); + if !args.is_empty() { + Some(args.swap_remove(0)) + } else { + None + } + }) + .and_then(|s| s.parse::().ok()) + .unwrap_or(DIGITS_DEFAULT_VALUE) + } + + pub fn get_issuer(&self) -> String { + let default_value = ""; + let args: Vec<&str> = self.uri.split('/').collect(); + match args.get(3) { + Some(s) => { + let args: Vec<&str> = s.split('?').collect(); + let issuer = args.first().unwrap_or(&default_value); + match urlencoding::decode(issuer) { + Ok(r) => r.into_owned(), + Err(_e) => (*issuer).to_string(), + } + } + None => String::from(default_value), + } + } +} + +impl TryFrom for OTPElement { + type Error = eyre::Report; + + fn try_from(input: AuthyExportedJsonElement) -> Result { + let type_ = OTPType::try_from(input.get_type().as_str())?; + let counter: Option = (type_ == OTPType::Hotp).then_some(0); + let digits = input.get_digits(); + Ok(OTPElement { + secret: input.secret.to_uppercase().replace('=', ""), + issuer: input.get_issuer(), + label: input.name, + digits, + type_, + algorithm: OTPAlgorithm::Sha1, + period: 30, + counter, + pin: None, + }) + } +} + +impl TryFrom for Vec { + type Error = eyre::Report; + + fn try_from(exported_list: AuthyExportedList) -> Result { + exported_list.0.into_iter().map(TryInto::try_into).collect() + } +} diff --git a/src/importers/converted.rs b/src/importers/converted.rs index 9ad3ede4..2926de7f 100644 --- a/src/importers/converted.rs +++ b/src/importers/converted.rs @@ -14,21 +14,24 @@ struct ConvertedJson { counter: u64, } -impl From for OTPElement { - fn from(converted_json: ConvertedJson) -> Self { - let counter: Option = (OTPType::from(converted_json.type_.as_str()) == OTPType::Hotp) - .then_some(converted_json.counter); - OTPElement { +impl TryFrom for OTPElement { + type Error = eyre::Report; + + fn try_from(converted_json: ConvertedJson) -> Result { + let type_ = OTPType::try_from(converted_json.type_.as_str())?; + let algorithm = OTPAlgorithm::try_from(converted_json.algorithm.as_str())?; + let counter: Option = (type_ == OTPType::Hotp).then_some(converted_json.counter); + Ok(OTPElement { secret: converted_json.secret, issuer: converted_json.issuer.unwrap_or_default(), label: converted_json.label.unwrap_or_default(), digits: converted_json.digits, - type_: OTPType::from(converted_json.type_.as_str()), - algorithm: OTPAlgorithm::from(converted_json.algorithm.as_str()), + type_, + algorithm, period: 30, counter, pin: None, - } + }) } } @@ -37,8 +40,36 @@ impl From for OTPElement { pub struct ConvertedJsonList(Vec); impl TryFrom for Vec { - type Error = String; + type Error = eyre::Report; fn try_from(value: ConvertedJsonList) -> Result { - Ok(value.0.into_iter().map(Into::into).collect()) + value.0.into_iter().map(TryInto::try_into).collect() + } +} + +#[cfg(test)] +mod tests { + use super::ConvertedJsonList; + use crate::otp::otp_element::OTPElement; + + #[test] + fn unknown_type_in_import_file_is_a_clear_error() { + let json = r#"[ + { + "label": "Label", + "secret": "AAAAAAAAAAAAAAAA", + "issuer": "Issuer", + "type": "OCRA", + "algorithm": "SHA1", + "digits": 6, + "counter": 0 + } + ]"#; + + let deserialized: ConvertedJsonList = serde_json::from_str(json).unwrap(); + let result: eyre::Result> = deserialized.try_into(); + + let error = result.unwrap_err().to_string(); + assert!(error.contains("Unknown OTP type")); + assert!(error.contains("OCRA")); } } diff --git a/src/importers/freeotp_plus.rs b/src/importers/freeotp_plus.rs index fab6e723..c9d5c972 100644 --- a/src/importers/freeotp_plus.rs +++ b/src/importers/freeotp_plus.rs @@ -44,31 +44,31 @@ pub struct FreeOTPElement { pub r#type: String, } -impl From for OTPElement { - fn from(token: FreeOTPElement) -> Self { - let counter: Option = if token.algo.to_uppercase().as_str() == "HOTP" { - Some(token.counter) - } else { - None - }; - OTPElement { +impl TryFrom for OTPElement { + type Error = eyre::Report; + + fn try_from(token: FreeOTPElement) -> Result { + let type_ = OTPType::try_from(token.r#type.as_str())?; + let algorithm = OTPAlgorithm::try_from(token.algo.as_str())?; + let counter: Option = (type_ == OTPType::Hotp).then_some(token.counter); + Ok(OTPElement { counter, secret: encode_secret(&token.secret), issuer: token.issuer_ext, label: token.label, digits: token.digits, - type_: OTPType::from(token.r#type.as_str()), - algorithm: OTPAlgorithm::from(token.algo.as_str()), + type_, + algorithm, period: token.period, pin: None, - } + }) } } impl TryFrom for Vec { - type Error = String; + type Error = eyre::Report; fn try_from(freeotp: FreeOTPPlusJson) -> Result { - Ok(freeotp.tokens.into_iter().map(Into::into).collect()) + freeotp.tokens.into_iter().map(TryInto::try_into).collect() } } @@ -94,7 +94,7 @@ mod tests { use std::fs; use crate::otp::otp_element::OTPDatabase; - use color_eyre::Result; + use eyre::Result; use super::{FreeOTPPlusJson, encode_secret}; @@ -146,6 +146,28 @@ mod tests { ); } + #[test] + fn test_hotp_conversion_keeps_counter() { + let imported = import_from_path::(PathBuf::from( + "test_samples/freeotp_plus_hotp.json", + )); + + assert_eq!( + vec![OTPElement { + secret: "AAAAAAAAAAAAAAAA".to_string(), + issuer: "Example3".to_string(), + label: "Label3".to_string(), + digits: 6, + type_: OTPType::Hotp, + algorithm: OTPAlgorithm::Sha1, + period: 30, + counter: Some(4), + pin: None + }], + imported.unwrap() + ); + } + #[test] fn test_freeotp_export() { // Arrange diff --git a/src/importers/google_authenticator.rs b/src/importers/google_authenticator.rs index 7287a336..10903c0b 100644 --- a/src/importers/google_authenticator.rs +++ b/src/importers/google_authenticator.rs @@ -14,9 +14,8 @@ use std::{fs::read_to_string, path::PathBuf}; -use base64::{Engine as _, engine::general_purpose}; -use color_eyre::eyre::{Result, eyre}; -use data_encoding::BASE32_NOPAD; +use data_encoding::{BASE32_NOPAD, BASE64, BASE64_NOPAD}; +use eyre::{Result, eyre}; use prost::Message; use url::Url; @@ -105,9 +104,12 @@ fn parse_migration_uri(uri: &str) -> Result> { .ok_or_else(|| eyre!("Missing 'data' parameter in otpauth-migration URI"))?; // The url crate already percent-decodes the value, so we can decode the - // raw base64 (standard alphabet, with padding) directly. - let decoded = general_purpose::STANDARD + // raw base64 (standard alphabet) directly. Some QR scanners strip the + // trailing `=` padding when extracting the URI, so fall back to unpadded + // decoding instead of rejecting those exports. + let decoded = BASE64 .decode(data.as_bytes()) + .or_else(|_| BASE64_NOPAD.decode(data.as_bytes())) .map_err(|e| eyre!("Invalid base64 in otpauth-migration data: {e}"))?; let payload = MigrationPayload::decode(decoded.as_slice()) @@ -172,7 +174,7 @@ mod tests { otp_parameters: entries, } .encode_to_vec(); - let data = general_purpose::STANDARD.encode(&payload); + let data = BASE64.encode(&payload); let encoded = urlencoding::encode(&data).into_owned(); format!("otpauth-migration://offline?data={encoded}") } @@ -285,6 +287,35 @@ mod tests { assert!(err.to_string().contains("No otpauth-migration")); } + #[test] + fn accepts_unpadded_base64_data() { + // Some QR scanners strip the trailing '=' padding from the migration + // URI; the importer must still decode it. + // Vary the label length so at least one payload is not a multiple of + // three bytes and therefore carries '=' padding in its base64 form. + let uri = (0..3) + .map(|i| { + build_uri(vec![otp_parameters( + b"Hello", + &"a".repeat(10 + i), + "Example", + 1, + 1, + 2, + 0, + )]) + }) + .find(|uri| uri.ends_with("%3D")) + .expect("at least one fixture must carry base64 padding"); + let unpadded = uri.trim_end_matches("%3D"); + assert_ne!(uri, unpadded); + + let elements = import_from_string(unpadded).unwrap(); + + assert_eq!(elements.len(), 1); + assert_eq!(elements[0].secret, "JBSWY3DP"); + } + #[test] fn errors_on_invalid_base64() { let err = import_from_string("otpauth-migration://offline?data=not*base64").unwrap_err(); @@ -294,7 +325,7 @@ mod tests { #[test] fn errors_on_invalid_protobuf() { // Valid base64 but not a valid protobuf message. - let data = general_purpose::STANDARD.encode([0xff, 0xff, 0xff, 0xff]); + let data = BASE64.encode(&[0xff, 0xff, 0xff, 0xff]); let encoded = urlencoding::encode(&data).into_owned(); let uri = format!("otpauth-migration://offline?data={encoded}"); let err = import_from_string(&uri).unwrap_err(); diff --git a/src/importers/importer.rs b/src/importers/importer.rs index d51cd22b..60caa594 100644 --- a/src/importers/importer.rs +++ b/src/importers/importer.rs @@ -1,6 +1,6 @@ -use std::{fmt::Debug, fs::read_to_string, path::PathBuf}; +use std::{fs::read_to_string, path::PathBuf}; -use color_eyre::eyre::{Result, eyre}; +use eyre::{Result, eyre}; use serde::Deserialize; use crate::otp::otp_element::OTPElement; @@ -9,7 +9,7 @@ use crate::otp::otp_element::OTPElement; pub fn import_from_path(path: PathBuf) -> Result> where T: for<'a> Deserialize<'a> + TryInto>, - >>::Error: Debug, + >>::Error: Into, { let json = read_to_string(path)?; let deserialized: T = serde_json::from_str(json.as_str()).map_err(|e| { @@ -17,11 +17,10 @@ where "Invalid JSON import format. Please check the file you are trying to import. For further information please check these guidelines: https://github.com/replydev/cotp?tab=readme-ov-file#migration-from-other-apps - - Specific error: {:?}", - e + + Specific error: {e}" ) })?; - let mapped: Vec = deserialized.try_into().map_err(|e| eyre!("{:?}", e))?; + let mapped: Vec = deserialized.try_into().map_err(Into::into)?; Ok(mapped) } diff --git a/src/importers/otp_uri.rs b/src/importers/otp_uri.rs index 761dd859..ebaff201 100644 --- a/src/importers/otp_uri.rs +++ b/src/importers/otp_uri.rs @@ -1,7 +1,7 @@ use crate::exporters::otp_uri::OtpUriList; use crate::otp::from_otp_uri::FromOtpUri; use crate::otp::otp_element::OTPElement; -use color_eyre::eyre::ErrReport; +use eyre::ErrReport; impl TryFrom for Vec { type Error = ErrReport; diff --git a/src/interface/app.rs b/src/interface/app.rs index 757cea1b..9c5fb8b7 100644 --- a/src/interface/app.rs +++ b/src/interface/app.rs @@ -1,22 +1,14 @@ use std::error; +use std::time::{SystemTime, UNIX_EPOCH}; use crate::interface::enums::Focus; use crate::interface::enums::Page; -use crate::interface::enums::Page::{Main, Qrcode}; -use crate::otp::otp_element::OTPDatabase; -use ratatui::Frame; -use ratatui::layout::Rect; -use ratatui::layout::{Alignment, Constraint, Direction, Layout}; -use ratatui::style::{Color, Modifier, Style}; -use ratatui::widgets::{Block, Borders, Cell, Clear, Gauge, Paragraph, Row, Table, Wrap}; +use crate::otp::otp_element::{OTPDatabase, OTPElement}; use crate::interface::stateful_table::{StatefulTable, fill_table}; use crate::utils::percentage; use super::enums::PopupAction; -use super::popup::centered_rect; - -const LARGE_APPLICATION_WIDTH: u16 = 75; /// Application result type. pub type AppResult = Result>; @@ -27,10 +19,12 @@ const DEFAULT_QRCODE_LABEL: &str = "Press enter to copy the OTP URI code"; pub struct App<'a> { /// Is the application running? pub running: bool, - title: String, + pub(crate) title: String, pub(crate) table: StatefulTable, pub(crate) database: &'a mut OTPDatabase, - progress: u16, + /// Time step of each element at the last tick, used to detect when an + /// element crosses its own period boundary and its code must be renewed + last_steps: Vec, /// Text to print replacing the percentage pub(crate) label_text: String, pub(crate) print_percentage: bool, @@ -41,6 +35,10 @@ pub struct App<'a> { /// Info text in the `QRCode` page pub(crate) qr_code_page_label: &'static str, + + /// Cached rendered QR code for the `QRCode` page, keyed by the index of + /// the element it was generated from + pub(crate) qrcode_cache: Option<(usize, String)>, } pub struct Popup { @@ -61,8 +59,8 @@ impl<'a> App<'a> { running: true, title, table: StatefulTable::new(database.elements_ref()), + last_steps: element_steps(database.elements_ref()), database, - progress: percentage(), label_text: String::new(), print_percentage: true, current_page: Page::default(), @@ -70,11 +68,12 @@ impl<'a> App<'a> { focus: Focus::MainPage, popup: Popup { text: String::new(), - action: PopupAction::EditOtp, + action: PopupAction::default(), percent_x: 60, percent_y: 20, }, qr_code_page_label: DEFAULT_QRCODE_LABEL, + qrcode_cache: None, } } @@ -82,234 +81,63 @@ impl<'a> App<'a> { self.current_page = Page::default(); self.print_percentage = true; self.qr_code_page_label = DEFAULT_QRCODE_LABEL; + self.qrcode_cache = None; } /// Handles the tick event of the terminal. pub fn tick(&mut self, force_update: bool) { - // Update progress bar - let new_progress = percentage(); - // Check for new cycle - if force_update || new_progress < self.progress { + let steps = element_steps(self.database.elements_ref()); + // Regenerate the codes when any element crossed its own period + // boundary, so elements with a period != 30 seconds (e.g. 60s TOTP, + // 10s MOTP) are refreshed on time too + if force_update || steps != self.last_steps { // Update codes self.table.items.clear(); fill_table(&mut self.table, self.database.elements_ref()); + // Elements may have changed (e.g. HOTP counter increment or + // deletion), so the cached QR code may be stale + self.qrcode_cache = None; } - self.progress = new_progress; - } - - /// Renders the user interface widgets. - pub fn render(&mut self, frame: &mut Frame<'_>) { - match &self.current_page { - Main => self.render_main_page(frame), - Qrcode => self.render_qrcode_page(frame), - } + self.last_steps = steps; } - fn render_qrcode_page(&self, frame: &mut Frame<'_>) { - let paragraph = self - .table + /// Percentage of the current period cycle elapsed for the selected + /// element, falling back to the global 30 seconds cycle if no element is + /// selected + pub(crate) fn progress(&self) -> u16 { + self.table .state .selected() .and_then(|index| self.database.elements_ref().get(index)) - .map_or_else( - || { - Paragraph::new("No element is selected") - .block(Block::default().title("Nope").borders(Borders::ALL)) - .style(Style::default().fg(Color::White).bg(Color::Reset)) - .alignment(Alignment::Center) - .wrap(Wrap { trim: true }) - }, - |element| { - let title = if element.label.is_empty() { - element.issuer.clone() - } else { - format!("{} - {}", element.issuer, element.label) - }; - Paragraph::new(format!( - "{}\n{}", - element.get_qrcode(), - self.qr_code_page_label - )) - .block(Block::default().title(title).borders(Borders::ALL)) - .style(Style::default().fg(Color::White).bg(Color::Reset)) - .alignment(Alignment::Center) - .wrap(Wrap { trim: true }) - }, - ); - Self::render_paragraph(frame, paragraph); - } - - fn render_paragraph(frame: &mut Frame<'_>, paragraph: Paragraph) { - let rects = Layout::default() - .direction(Direction::Vertical) - .constraints([Constraint::Percentage(100)].as_ref()) - .split(frame.area()); - - frame.render_widget(paragraph, rects[0]); + .map_or_else(percentage, |element| period_percentage(element.period)) } +} - fn render_main_page(&mut self, frame: &mut Frame<'_>) { - let height = frame.area().height; - let rects = Layout::default() - .direction(Direction::Vertical) - .constraints( - [ - Constraint::Length(3), // Search bar - Constraint::Length(height - 8), // Table + Info Box - Constraint::Length(1), // Progress bar - ] - .as_ref(), - ) - .margin(2) - .split(frame.area()); - - let search_bar_title = "Press CTRL + F to search a code..."; - let search_bar = Paragraph::new(&*self.search_query) - .block( - Block::default() - .title(search_bar_title) - .borders(Borders::ALL) - .border_style(Style::default().fg(if self.focus == Focus::SearchBar { - Color::LightRed - } else { - Color::White - })), - ) - .style(Style::default().fg(Color::White).bg(Color::Reset)) - .alignment(Alignment::Center) - .wrap(Wrap { trim: true }); - - let progress_label = if self.print_percentage { - format!("{}%", self.progress) - } else { - self.label_text.clone() - }; - let progress_bar = Gauge::default() - .block(Block::default()) - .gauge_style( - Style::default() - .bg(Color::White) - .fg(Color::DarkGray) - .add_modifier(Modifier::BOLD), - ) - .percent(self.progress) - .label(progress_label); - - frame.render_widget(search_bar, rects[0]); - self.render_table_box(frame, rects[1]); - frame.render_widget(progress_bar, rects[2]); - if self.focus == Focus::Popup { - self.render_alert(frame); - } - } - - fn render_alert(&mut self, frame: &mut Frame<'_>) { - let block = Block::default().title("Alert").borders(Borders::ALL); - let paragraph = Paragraph::new(&*self.popup.text) - .block(block) - .alignment(Alignment::Center) - .wrap(Wrap { trim: true }); - let area = centered_rect(self.popup.percent_x, self.popup.percent_y, frame.area()); - frame.render_widget(Clear, area); - //this clears out the background - frame.render_widget(paragraph, area); - } - - fn render_table_box(&mut self, frame: &mut Frame<'_>, area: Rect) { - let constraints = if Self::is_large_application(frame) { - vec![Constraint::Percentage(80), Constraint::Percentage(20)] - } else { - vec![Constraint::Percentage(100)] - }; - let chunks = Layout::default() - .constraints(constraints) - .direction(Direction::Horizontal) - .split(area); - - let header_cells = ["Id", "Issuer", "Label", "OTP"] - .iter() - .map(|h| Cell::from(*h).style(Style::default().fg(Color::Black))); - let header = Row::new(header_cells) - .style( - Style::default() - .bg(Color::White) - .add_modifier(Modifier::BOLD), - ) - .height(1) - .bottom_margin(1); - let rows = self.table.items.iter().map(|item| { - Row::new(item.cells()) - .height(item.height()) - .bottom_margin(1) - }); - - const TABLE_WIDTHS: &[Constraint] = &[ - Constraint::Percentage(5), - Constraint::Percentage(35), - Constraint::Percentage(35), - Constraint::Percentage(25), - ]; - - let t = Table::new(rows, TABLE_WIDTHS) - .header(header) - .block( - Block::default() - .borders(Borders::TOP | Borders::BOTTOM) - .title(self.title.as_str()), - ) - .row_highlight_style( - Style::default() - .bg(Color::White) - .fg(Color::Black) - .add_modifier(Modifier::BOLD), - ) - .highlight_symbol("-> "); - - let selected_element = self - .table - .state - .selected() - .and_then(|i| self.database.get_element(i)); +/// Milliseconds elapsed since the Unix epoch +fn current_millis() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_millis() as u64) + .unwrap_or(0) +} - let mut text = if let Some(element) = selected_element { - format!( - " - Type: {} - Algorithm: {} - Period: {} {} - Counter: {} - Pin: {} - ", - element.type_, - element.algorithm, - element.period, - if element.period == 1u64 { - "second" - } else { - "seconds" - }, - element - .counter - .map_or_else(|| String::from("N/A"), |e| e.to_string()), - element.pin.clone().unwrap_or_else(|| String::from("N/A")) - ) - } else { - String::new() - }; +/// Index of the current time step for the given period in seconds (the T +/// value of RFC 6238). A period of 0 is treated as 1 to avoid a division by +/// zero +fn current_step(period: u64) -> u64 { + (current_millis() / 1000) / period.max(1) +} - text.push_str("\n\n Press '?' to get help\n"); - let paragraph = Paragraph::new(text) - .block(Block::default().title("Code info").borders(Borders::ALL)) - .style(Style::default().fg(Color::White).bg(Color::Reset)) - .alignment(Alignment::Left) - .wrap(Wrap { trim: true }); - frame.render_stateful_widget(t, chunks[0], &mut self.table.state); - if Self::is_large_application(frame) { - frame.render_widget(paragraph, chunks[1]); - } - } +/// Percentage of the current cycle elapsed for the given period in seconds +fn period_percentage(period: u64) -> u16 { + let period_millis = period.max(1) * 1000; + ((current_millis() % period_millis) * 100 / period_millis) as u16 +} - fn is_large_application(frame: &mut Frame<'_>) -> bool { - frame.area().width >= LARGE_APPLICATION_WIDTH - } +/// The current time step of every element, in database order +fn element_steps(elements: &[OTPElement]) -> Vec { + elements + .iter() + .map(|element| current_step(element.period)) + .collect() } diff --git a/src/interface/enums.rs b/src/interface/enums.rs index 373bb849..9fc8e6be 100644 --- a/src/interface/enums.rs +++ b/src/interface/enums.rs @@ -5,10 +5,10 @@ pub enum Focus { Popup, } -#[derive(Eq, PartialEq, Debug)] +#[derive(Eq, PartialEq, Debug, Default)] pub enum PopupAction { - EditOtp, DeleteOtp, + #[default] GeneralInfo, SaveBeforeQuit, } diff --git a/src/interface/event.rs b/src/interface/event.rs index 2726fe17..1fe0d86f 100644 --- a/src/interface/event.rs +++ b/src/interface/event.rs @@ -6,23 +6,14 @@ use crossterm::event::{self, Event as CrosstermEvent, KeyEvent, KeyEventKind}; use crate::interface::app::AppResult; -/// Terminal events. +/// Terminal events the dashboard reacts to. Everything else read from the +/// terminal (mouse, resize, focus, paste) is ignored at the source. #[derive(Clone, Debug)] pub enum Event { /// Terminal tick. Tick, /// Key press. Key(KeyEvent), - /// Mouse click/scroll. - Mouse(()), - /// Terminal resize. - Resize((), ()), - /// Focus gained - FocusGained(), - /// Focus lost - FocusLost(), - /// Paste text - Paste(()), } /// Terminal event handler. @@ -49,27 +40,29 @@ impl EventHandler { .unwrap_or(tick_rate); if event::poll(timeout).expect("no events available") { - match event::read().expect("unable to read event") { - CrosstermEvent::Key(e) => { - // Workaround to fix double input on Windows - // Please check https://github.com/crossterm-rs/crossterm/issues/752 - if e.kind == KeyEventKind::Press { - sender.send(Event::Key(e)) - } else { - Ok(()) - } + let send_result = match event::read().expect("unable to read event") { + // Workaround to fix double input on Windows + // Please check https://github.com/crossterm-rs/crossterm/issues/752 + CrosstermEvent::Key(e) if e.kind == KeyEventKind::Press => { + sender.send(Event::Key(e)) } - CrosstermEvent::Mouse(_e) => sender.send(Event::Mouse(())), - CrosstermEvent::Resize(_w, _h) => sender.send(Event::Resize((), ())), - CrosstermEvent::FocusGained => sender.send(Event::FocusGained()), - CrosstermEvent::FocusLost => sender.send(Event::FocusLost()), - CrosstermEvent::Paste(_e) => sender.send(Event::Paste(())), + // Mouse, resize, focus and paste events are irrelevant + // to the dashboard: drop them here instead of routing + // dead variants through the channel. + _ => Ok(()), + }; + if send_result.is_err() { + // The receiver has been dropped: the dashboard has + // exited, so stop the event thread gracefully. + break; } - .expect("failed to send terminal event"); } if last_tick.elapsed() >= tick_rate { - sender.send(Event::Tick).expect("failed to send tick event"); + if sender.send(Event::Tick).is_err() { + // Receiver dropped, see above. + break; + } last_tick = Instant::now(); } } diff --git a/src/interface/handlers/main_window.rs b/src/interface/handlers/main_window.rs index ec9ca955..6b04799c 100644 --- a/src/interface/handlers/main_window.rs +++ b/src/interface/handlers/main_window.rs @@ -22,8 +22,13 @@ pub(super) fn main_handler(key_event: KeyEvent, app: &mut App) { handle_exit(app); } - // exit application on Ctrl-D - KeyCode::Char('d' | 'D' | 'c') => { + // exit application on Ctrl-C + KeyCode::Char('c' | 'C') if key_event.modifiers == KeyModifiers::CONTROL => { + handle_exit(app); + } + + // exit application on Ctrl-D, delete the selected code on plain D + KeyCode::Char('d' | 'D') => { if key_event.modifiers == KeyModifiers::CONTROL { handle_exit(app); } else if app.table.state.selected().is_some() { @@ -124,8 +129,9 @@ fn handle_counter_switch(app: &mut App, increment: bool) { && let Some(element) = app.database.mut_element(selected) && element.type_ == OTPType::Hotp { - // safe to unwrap because the element type is HOTP - let counter = element.counter.unwrap(); + // HOTP elements may lack a counter (e.g. imported from an otpauth URI + // without one), so fall back to 0 instead of panicking + let counter = element.counter.unwrap_or(0); element.counter = if increment { Some(counter.saturating_add(1)) } else { diff --git a/src/interface/handlers/mod.rs b/src/interface/handlers/mod.rs index 41fdd32a..f7b04f79 100644 --- a/src/interface/handlers/mod.rs +++ b/src/interface/handlers/mod.rs @@ -41,15 +41,12 @@ pub(super) fn handle_exit(app: &mut App) { pub(crate) fn copy_selected_code_to_clipboard(app: &mut App) -> String { match app.table.state.selected() { Some(selected) => match app.table.items.get(selected) { - Some(element) => match element.values.get(3) { - Some(otp_code) => match copy_string_to_clipboard(otp_code) { - Ok(result) => match result { - CopyType::Native => "Copied!".to_string(), - CopyType::OSC52 => "Remote copied!".to_string(), - }, - _ => "Cannot copy".to_string(), + Some(element) => match copy_string_to_clipboard(&element.otp_code) { + Ok(result) => match result { + CopyType::Native => "Copied!".to_string(), + CopyType::OSC52 => "Remote copied!".to_string(), }, - None => "Cannot get OTP Code column".to_string(), + _ => "Cannot copy".to_string(), }, None => format!("Cannot fetch element from index: {selected}"), }, diff --git a/src/interface/handlers/popup.rs b/src/interface/handlers/popup.rs index da032310..7c545556 100644 --- a/src/interface/handlers/popup.rs +++ b/src/interface/handlers/popup.rs @@ -7,7 +7,6 @@ use crate::interface::{ pub(super) fn popup_handler(key_event: KeyEvent, app: &mut App) { match app.popup.action { - PopupAction::EditOtp => todo!(), PopupAction::DeleteOtp => match key_event.code { KeyCode::Char('y' | 'Y') => { if let Err(e) = delete_selected_code(app) { @@ -34,7 +33,7 @@ pub(super) fn popup_handler(key_event: KeyEvent, app: &mut App) { app.running = false; } KeyCode::Char('n' | 'N') => { - app.database.needs_modification = false; + app.database.clear_modified(); app.running = false; } KeyCode::Esc => { diff --git a/src/interface/handlers/search_bar.rs b/src/interface/handlers/search_bar.rs index eaa00d4d..443a9345 100644 --- a/src/interface/handlers/search_bar.rs +++ b/src/interface/handlers/search_bar.rs @@ -42,61 +42,35 @@ pub(super) fn search_bar_handler(key_event: KeyEvent, app: &mut App) { } fn search_and_select(app: &mut App) { - // Check for issuer - for iter in app.table.items.iter().enumerate() { - let (index, row) = iter; - if row - .values - .get(1) - .unwrap() - .to_lowercase() - .starts_with(&app.search_query.to_lowercase()) - { - app.table.state.select(Some(index)); - return; - } - } - // Check for label - for iter in app.table.items.iter().enumerate() { - let (index, row) = iter; - if row - .values - .get(2) - .unwrap() - .to_lowercase() - .starts_with(&app.search_query.to_lowercase()) - { - app.table.state.select(Some(index)); - return; - } - } - // Check if issuer contains the query - for iter in app.table.items.iter().enumerate() { - let (index, row) = iter; - if row - .values - .get(1) - .unwrap() - .to_lowercase() - .contains(&app.search_query.to_lowercase()) - { - app.table.state.select(Some(index)); - return; - } - } - // Check if label contains the query - for iter in app.table.items.iter().enumerate() { - let (index, row) = iter; - if row - .values - .get(2) - .unwrap() - .to_lowercase() - .contains(&app.search_query.to_lowercase()) - { - app.table.state.select(Some(index)); - return; - } + let query = app.search_query.to_lowercase(); + // Single ranked pass over the rows: an issuer prefix match wins over a + // label prefix match, which wins over an issuer substring match, which + // wins over a label substring match; ties are broken by row order + let best_match = app + .table + .items + .iter() + .enumerate() + .filter_map(|(index, row)| { + let issuer = row.issuer.to_lowercase(); + let label = row.label.to_lowercase(); + let rank = if issuer.starts_with(&query) { + 0 + } else if label.starts_with(&query) { + 1 + } else if issuer.contains(&query) { + 2 + } else if label.contains(&query) { + 3 + } else { + return None; + }; + Some((rank, index)) + }) + .min_by_key(|&(rank, index)| (rank, index)); + + if let Some((_, index)) = best_match { + app.table.state.select(Some(index)); } // TODO Handle if no search results } diff --git a/src/interface/popup.rs b/src/interface/popup.rs index f542e345..a578cdde 100644 --- a/src/interface/popup.rs +++ b/src/interface/popup.rs @@ -7,9 +7,9 @@ pub(crate) fn centered_rect(percent_x: u16, percent_y: u16, r: Rect) -> Rect { .direction(Direction::Vertical) .constraints( [ - Constraint::Percentage((100 - percent_y) / 2), + Constraint::Percentage(100u16.saturating_sub(percent_y) / 2), Constraint::Percentage(percent_y), - Constraint::Percentage((100 - percent_y) / 2), + Constraint::Percentage(100u16.saturating_sub(percent_y) / 2), ] .as_ref(), ) @@ -19,9 +19,9 @@ pub(crate) fn centered_rect(percent_x: u16, percent_y: u16, r: Rect) -> Rect { .direction(Direction::Horizontal) .constraints( [ - Constraint::Percentage((100 - percent_x) / 2), + Constraint::Percentage(100u16.saturating_sub(percent_x) / 2), Constraint::Percentage(percent_x), - Constraint::Percentage((100 - percent_x) / 2), + Constraint::Percentage(100u16.saturating_sub(percent_x) / 2), ] .as_ref(), ) diff --git a/src/interface/row.rs b/src/interface/row.rs index b6d164c7..79090df1 100644 --- a/src/interface/row.rs +++ b/src/interface/row.rs @@ -3,17 +3,37 @@ use ratatui::style::Style; use ratatui::widgets::Cell; pub(crate) struct Row { - pub(crate) values: Vec, + pub(crate) id: String, + pub(crate) issuer: String, + pub(crate) label: String, + pub(crate) otp_code: String, has_error: bool, } impl Row { - pub(crate) fn new(values: Vec, has_error: bool) -> Self { - Row { values, has_error } + pub(crate) fn new( + id: String, + issuer: String, + label: String, + otp_code: String, + has_error: bool, + ) -> Self { + Row { + id, + issuer, + label, + otp_code, + has_error, + } } + + fn columns(&self) -> [&String; 4] { + [&self.id, &self.issuer, &self.label, &self.otp_code] + } + pub fn height(&self) -> u16 { (self - .values + .columns() .iter() .map(|content| content.chars().filter(|c| *c == '\n').count()) .max() @@ -22,7 +42,7 @@ impl Row { } pub fn cells(&self) -> Vec> { - self.values + self.columns() .iter() .map(|c| { let style = if self.has_error { diff --git a/src/interface/stateful_table.rs b/src/interface/stateful_table.rs index 3d399d95..f8f26b40 100644 --- a/src/interface/stateful_table.rs +++ b/src/interface/stateful_table.rs @@ -69,15 +69,13 @@ pub fn fill_table(table: &mut StatefulTable, elements: &[OTPElement]) { let error = result.is_err(); table.items.push(Row::new( - vec![ - (i + 1).to_string(), - element.issuer.clone(), - label, - match result { - Ok(code) => code, - Err(e) => e.to_string(), - }, - ], + (i + 1).to_string(), + element.issuer.clone(), + label, + match result { + Ok(code) => code, + Err(e) => e.to_string(), + }, error, )); } diff --git a/src/interface/ui.rs b/src/interface/ui.rs index e822bad3..b1ee992d 100644 --- a/src/interface/ui.rs +++ b/src/interface/ui.rs @@ -2,11 +2,19 @@ use std::io; use crossterm::event::{DisableMouseCapture, EnableMouseCapture}; use crossterm::terminal::{self, EnterAlternateScreen, LeaveAlternateScreen}; -use ratatui::Terminal; +use ratatui::layout::{Alignment, Constraint, Direction, Layout, Rect}; use ratatui::prelude::Backend; +use ratatui::style::{Color, Modifier, Style}; +use ratatui::widgets::{Block, Borders, Cell, Clear, Gauge, Paragraph, Row, Table, Wrap}; +use ratatui::{Frame, Terminal}; use crate::interface::app::{App, AppResult}; +use crate::interface::enums::Focus; +use crate::interface::enums::Page::{Main, Qrcode}; use crate::interface::event::EventHandler; +use crate::interface::popup::centered_rect; + +const LARGE_APPLICATION_WIDTH: u16 = 75; /// Representation of a terminal user interface. /// @@ -43,12 +51,12 @@ impl Tui { /// [`Draw`] the terminal interface by [`rendering`] the widgets. /// /// [`Draw`]: tui::Terminal::draw - /// [`rendering`]: crate::app::App::render + /// [`rendering`]: render pub fn draw(&mut self, app: &mut App) -> AppResult<()> where ::Error: 'static, { - self.terminal.draw(|frame| app.render(frame))?; + self.terminal.draw(|frame| render(app, frame))?; Ok(()) } @@ -65,3 +73,230 @@ impl Tui { Ok(()) } } + +/// Renders the user interface widgets. +pub fn render(app: &mut App, frame: &mut Frame<'_>) { + match &app.current_page { + Main => render_main_page(app, frame), + Qrcode => render_qrcode_page(app, frame), + } +} + +fn render_qrcode_page(app: &mut App, frame: &mut Frame<'_>) { + let selected_index = app + .table + .state + .selected() + .filter(|index| *index < app.database.elements_ref().len()); + + let paragraph = if let Some(index) = selected_index { + // Building the QR code (URI + matrix + unicode rendering) is + // expensive, so cache the rendered string and rebuild it only + // when the selection changes + let cache_is_valid = matches!(&app.qrcode_cache, Some((cached, _)) if *cached == index); + if !cache_is_valid { + let qrcode = app.database.elements_ref()[index].get_qrcode(); + app.qrcode_cache = Some((index, qrcode)); + } + let element = &app.database.elements_ref()[index]; + let qrcode = app + .qrcode_cache + .as_ref() + .map(|(_, qrcode)| qrcode.as_str()) + .unwrap_or_default(); + let title = if element.label.is_empty() { + element.issuer.clone() + } else { + format!("{} - {}", element.issuer, element.label) + }; + Paragraph::new(format!("{}\n{}", qrcode, app.qr_code_page_label)) + .block(Block::default().title(title).borders(Borders::ALL)) + .style(Style::default().fg(Color::White).bg(Color::Reset)) + .alignment(Alignment::Center) + .wrap(Wrap { trim: true }) + } else { + Paragraph::new("No element is selected") + .block(Block::default().title("Nope").borders(Borders::ALL)) + .style(Style::default().fg(Color::White).bg(Color::Reset)) + .alignment(Alignment::Center) + .wrap(Wrap { trim: true }) + }; + render_paragraph(frame, paragraph); +} + +fn render_paragraph(frame: &mut Frame<'_>, paragraph: Paragraph) { + let rects = Layout::default() + .direction(Direction::Vertical) + .constraints([Constraint::Percentage(100)].as_ref()) + .split(frame.area()); + + frame.render_widget(paragraph, rects[0]); +} + +fn render_main_page(app: &mut App, frame: &mut Frame<'_>) { + let height = frame.area().height; + let rects = Layout::default() + .direction(Direction::Vertical) + .constraints( + [ + Constraint::Length(3), // Search bar + Constraint::Length(height.saturating_sub(8)), // Table + Info Box + Constraint::Length(1), // Progress bar + ] + .as_ref(), + ) + .margin(2) + .split(frame.area()); + + let search_bar_title = "Press CTRL + F to search a code..."; + let search_bar = Paragraph::new(&*app.search_query) + .block( + Block::default() + .title(search_bar_title) + .borders(Borders::ALL) + .border_style(Style::default().fg(if app.focus == Focus::SearchBar { + Color::LightRed + } else { + Color::White + })), + ) + .style(Style::default().fg(Color::White).bg(Color::Reset)) + .alignment(Alignment::Center) + .wrap(Wrap { trim: true }); + + // The gauge tracks the period of the selected element, so a 60s TOTP + // or a 10s MOTP shows its actual remaining time + let progress = app.progress(); + let progress_label = if app.print_percentage { + format!("{progress}%") + } else { + app.label_text.clone() + }; + let progress_bar = Gauge::default() + .block(Block::default()) + .gauge_style( + Style::default() + .bg(Color::White) + .fg(Color::DarkGray) + .add_modifier(Modifier::BOLD), + ) + .percent(progress) + .label(progress_label); + + frame.render_widget(search_bar, rects[0]); + render_table_box(app, frame, rects[1]); + frame.render_widget(progress_bar, rects[2]); + if app.focus == Focus::Popup { + render_alert(app, frame); + } +} + +fn render_alert(app: &mut App, frame: &mut Frame<'_>) { + let block = Block::default().title("Alert").borders(Borders::ALL); + let paragraph = Paragraph::new(&*app.popup.text) + .block(block) + .alignment(Alignment::Center) + .wrap(Wrap { trim: true }); + let area = centered_rect(app.popup.percent_x, app.popup.percent_y, frame.area()); + frame.render_widget(Clear, area); + //this clears out the background + frame.render_widget(paragraph, area); +} + +fn render_table_box(app: &mut App, frame: &mut Frame<'_>, area: Rect) { + let constraints = if is_large_application(frame) { + vec![Constraint::Percentage(80), Constraint::Percentage(20)] + } else { + vec![Constraint::Percentage(100)] + }; + let chunks = Layout::default() + .constraints(constraints) + .direction(Direction::Horizontal) + .split(area); + + let header_cells = ["Id", "Issuer", "Label", "OTP"] + .iter() + .map(|h| Cell::from(*h).style(Style::default().fg(Color::Black))); + let header = Row::new(header_cells) + .style( + Style::default() + .bg(Color::White) + .add_modifier(Modifier::BOLD), + ) + .height(1) + .bottom_margin(1); + let rows = app.table.items.iter().map(|item| { + Row::new(item.cells()) + .height(item.height()) + .bottom_margin(1) + }); + + const TABLE_WIDTHS: &[Constraint] = &[ + Constraint::Percentage(5), + Constraint::Percentage(35), + Constraint::Percentage(35), + Constraint::Percentage(25), + ]; + + let t = Table::new(rows, TABLE_WIDTHS) + .header(header) + .block( + Block::default() + .borders(Borders::TOP | Borders::BOTTOM) + .title(app.title.as_str()), + ) + .row_highlight_style( + Style::default() + .bg(Color::White) + .fg(Color::Black) + .add_modifier(Modifier::BOLD), + ) + .highlight_symbol("-> "); + + let selected_element = app + .table + .state + .selected() + .and_then(|i| app.database.get_element(i)); + + let mut text = if let Some(element) = selected_element { + format!( + " + Type: {} + Algorithm: {} + Period: {} {} + Counter: {} + Pin: {} + ", + element.type_, + element.algorithm, + element.period, + if element.period == 1u64 { + "second" + } else { + "seconds" + }, + element + .counter + .map_or_else(|| String::from("N/A"), |e| e.to_string()), + element.pin.clone().unwrap_or_else(|| String::from("N/A")) + ) + } else { + String::new() + }; + + text.push_str("\n\n Press '?' to get help\n"); + let paragraph = Paragraph::new(text) + .block(Block::default().title("Code info").borders(Borders::ALL)) + .style(Style::default().fg(Color::White).bg(Color::Reset)) + .alignment(Alignment::Left) + .wrap(Wrap { trim: true }); + frame.render_stateful_widget(t, chunks[0], &mut app.table.state); + if is_large_application(frame) { + frame.render_widget(paragraph, chunks[1]); + } +} + +fn is_large_application(frame: &mut Frame<'_>) -> bool { + frame.area().width >= LARGE_APPLICATION_WIDTH +} diff --git a/src/main.rs b/src/main.rs index 2acd3193..f73391d4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,17 +1,17 @@ #![forbid(unsafe_code)] use arguments::{CotpArgs, args_parser}; use clap::Parser; -use color_eyre::eyre::eyre; use interface::app::AppResult; use interface::event::{Event, EventHandler}; use interface::handlers::handle_key_events; use interface::ui::Tui; -use otp::otp_element::{CURRENT_DATABASE_VERSION, OTPDatabase}; -use path::init_path; +use otp::otp_element::OTPDatabase; +use path::{DATABASE_PATH, init_path}; use ratatui::Terminal; use ratatui::prelude::CrosstermBackend; -use reading::{ReadResult, get_elements_from_input, get_elements_from_stdin}; -use std::{io, vec}; +use std::io; +use std::process::ExitCode; +use storage::{ReadResult, get_elements_from_input, get_elements_from_stdin}; use zeroize::Zeroize; mod arguments; @@ -22,72 +22,69 @@ mod importers; mod interface; mod otp; mod path; -mod reading; +mod storage; mod utils; -fn init(args: &CotpArgs) -> color_eyre::Result { +fn init(args: &CotpArgs) -> eyre::Result { init_path(args); - match utils::init_app() { - Ok(first_run) => { - if first_run { - // Let's initialize the database file - let mut pw = utils::verified_password("Choose a password: ", 8); - let mut database = OTPDatabase { - version: CURRENT_DATABASE_VERSION, - elements: vec![], - ..Default::default() - }; - let save_result = database.save_with_pw(&pw); - pw.zeroize(); - save_result.map(|(key, salt)| (database, key, salt.to_vec())) - } else if args.password_from_stdin { - get_elements_from_stdin() - } else { - get_elements_from_input() - } - } - Err(()) => Err(eyre!("An error occurred during database creation")), + let first_run = utils::init_app()?; + if first_run { + // Let's initialize the database file + let mut pw = utils::try_verified_password("Choose a password: ", 8)?; + let mut database = OTPDatabase::default(); + let save_result = storage::save_with_pw(&mut database, &pw, DATABASE_PATH.get().unwrap()); + pw.zeroize(); + save_result.map(|(key, salt)| (database, key, salt.to_vec())) + } else if args.password_from_stdin { + get_elements_from_stdin() + } else { + get_elements_from_input() } } -fn main() -> AppResult<()> { - color_eyre::install()?; - +fn main() -> ExitCode { let cotp_args: CotpArgs = CotpArgs::parse(); let (database, mut key, salt) = match init(&cotp_args) { Ok(v) => v, Err(e) => { - println!("{e}"); - std::process::exit(-1); + // "{e:#}" prints the whole eyre error chain, e.g. + // "outer context: root cause", keeping the root cause visible. + eprintln!("An error occurred: {e:#}"); + return ExitCode::from(1); } }; let mut reowned_database = match args_parser(cotp_args, database) { Ok(d) => d, Err(e) => { - eprintln!("An error occurred: {e}"); + eprintln!("An error occurred: {e:#}"); key.zeroize(); - std::process::exit(-2) + return ExitCode::from(2); } }; - let error_code = if reowned_database.is_modified() { - match reowned_database.save(&key, &salt) { + let exit_code = if reowned_database.is_modified() { + match storage::save( + &mut reowned_database, + &key, + &salt, + DATABASE_PATH.get().unwrap(), + ) { Ok(()) => { println!("Modifications have been persisted"); - 0 + ExitCode::SUCCESS } _ => { eprintln!("An error occurred during database overwriting"); - -1 + ExitCode::from(1) } } } else { - 0 + ExitCode::SUCCESS }; key.zeroize(); - std::process::exit(error_code) + exit_code } fn dashboard(mut database: OTPDatabase) -> AppResult { @@ -112,11 +109,6 @@ fn dashboard(mut database: OTPDatabase) -> AppResult { match tui.events.next()? { Event::Tick => app.tick(false), Event::Key(key_event) => handle_key_events(key_event, &mut app), - Event::Mouse(()) - | Event::Resize((), ()) - | Event::FocusGained() - | Event::FocusLost() - | Event::Paste(()) => {} } } diff --git a/src/otp/algorithms/motp_maker.rs b/src/otp/algorithms/motp_maker.rs index 165cde7a..e7d7da71 100644 --- a/src/otp/algorithms/motp_maker.rs +++ b/src/otp/algorithms/motp_maker.rs @@ -1,7 +1,10 @@ +use data_encoding::HEXLOWER; use md5::{Digest, Md5}; use std::time::SystemTime; -pub fn motp(secret: &str, pin: &str, period: u64, digits: usize) -> String { +use crate::otp::otp_error::OtpError; + +pub fn motp(secret: &str, pin: &str, period: u64, digits: usize) -> Result { let seconds = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) .unwrap() @@ -10,7 +13,17 @@ pub fn motp(secret: &str, pin: &str, period: u64, digits: usize) -> String { get_motp_code(secret, pin, period, digits, seconds) } -fn get_motp_code(secret: &str, pin: &str, period: u64, digits: usize, seconds: u64) -> String { +fn get_motp_code( + secret: &str, + pin: &str, + period: u64, + digits: usize, + seconds: u64, +) -> Result { + if period == 0 { + return Err(OtpError::InvalidPeriod); + } + // TODO MOTP Secrets are hex encoded, so do not use BASE32 at all let hex_secret = secret; let counter = seconds / period; @@ -18,8 +31,8 @@ fn get_motp_code(secret: &str, pin: &str, period: u64, digits: usize, seconds: u let mut md5_hasher = Md5::new(); md5_hasher.update(data.as_bytes()); - let code = hex::encode(md5_hasher.finalize()); - code.as_str()[0..digits].to_owned() + let code = HEXLOWER.encode(&md5_hasher.finalize()); + Ok(code.as_str()[0..digits].to_owned()) } #[cfg(test)] @@ -33,7 +46,7 @@ mod tests { assert_eq!( "e7d8b6".to_string(), - get_motp_code("e3152afee62599c8", "1234", 10, 6, seconds) + get_motp_code("e3152afee62599c8", "1234", 10, 6, seconds).unwrap() ); } } diff --git a/src/otp/algorithms/totp_maker.rs b/src/otp/algorithms/totp_maker.rs index c82d4157..f2e9b9e4 100644 --- a/src/otp/algorithms/totp_maker.rs +++ b/src/otp/algorithms/totp_maker.rs @@ -20,6 +20,9 @@ fn generate_totp( time_step: u64, skew: i64, ) -> Result { + if time_step == 0 { + return Err(OtpError::InvalidPeriod); + } hotp(secret, algorithm, ((time as i64 + skew) as u64) / time_step) } diff --git a/src/otp/algorithms/yandex_otp_maker.rs b/src/otp/algorithms/yandex_otp_maker.rs index b5bad7f7..a26b80b6 100644 --- a/src/otp/algorithms/yandex_otp_maker.rs +++ b/src/otp/algorithms/yandex_otp_maker.rs @@ -3,11 +3,8 @@ use std::time::SystemTime; use data_encoding::BASE32_NOPAD; -use hmac::EagerHash; -use sha1::{Digest, Sha1}; -use sha2::{Sha256, Sha512}; +use sha2::{Digest, Sha256}; -use crate::otp::otp_algorithm::OTPAlgorithm; use crate::otp::otp_error::OtpError; use super::hotp_maker::hotp_hash; @@ -15,40 +12,32 @@ use super::hotp_maker::hotp_hash; const EN_ALPHABET_LENGTH: u64 = 26; const SECRET_LENGTH: usize = 16; -pub fn yandex( - secret: &str, - pin: &str, - period: u64, - digits: usize, - algorithm: OTPAlgorithm, -) -> Result { +/// Yandex OTP codes are always calculated with HMAC-SHA256, regardless of the +/// algorithm stored on the element. The Yandex.Key app (and Aegis' reference +/// implementation this port is based on) exclusively use HMAC-SHA256. +/// +/// Honoring the element's algorithm field was also unsound: HMAC-SHA1 output +/// is only 20 bytes, while the dynamic offset below can reach 15, making +/// `hash[offset..offset + 8]` read out of bounds for offsets 13..=15. +pub fn yandex(secret: &str, pin: &str, period: u64, digits: usize) -> Result { let seconds = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) .unwrap() .as_secs(); - match algorithm { - OTPAlgorithm::Sha256 => { - calculate_yandex_code::(secret, pin, period, digits, seconds) - } - - OTPAlgorithm::Sha512 => { - calculate_yandex_code::(secret, pin, period, digits, seconds) - } - - _ => calculate_yandex_code::(secret, pin, period, digits, seconds), - } + calculate_yandex_code(secret, pin, period, digits, seconds) } -fn calculate_yandex_code( +fn calculate_yandex_code( secret: &str, pin: &str, period: u64, digits: usize, seconds: u64, -) -> Result -where - D: EagerHash, -{ +) -> Result { + if period == 0 { + return Err(OtpError::InvalidPeriod); + } + let decoded_secret = match BASE32_NOPAD.decode(secret.as_bytes()) { Ok(r) => r, Err(e) => return Err(OtpError::SecretEncoding(e.kind, e.position)), @@ -73,7 +62,7 @@ where } let counter: u64 = seconds / period; - let mut period_hash = hotp_hash::(key_hash, counter); + let mut period_hash = hotp_hash::(key_hash, counter); // calculate offset let offset: usize = match period_hash.last() { @@ -81,13 +70,14 @@ where None => return Err(OtpError::InvalidOffset), } as usize; - period_hash[offset] &= 0x7f; + *period_hash.get_mut(offset).ok_or(OtpError::InvalidOffset)? &= 0x7f; - // calculate code - let code_bytes: [u8; 8] = match period_hash[offset..offset + 8].try_into() { - Ok(x) => x, - Err(_) => return Err(OtpError::InvalidDigest), - }; + // calculate code, bounds-checked as defense in depth + let code_bytes: [u8; 8] = period_hash + .get(offset..offset + 8) + .ok_or(OtpError::InvalidDigest)? + .try_into() + .map_err(|_| OtpError::InvalidDigest)?; let code = u64::from_be_bytes(code_bytes); @@ -110,8 +100,6 @@ fn to_yandex_string(mut code: u64, digits: usize) -> String { #[cfg(test)] mod tests { - use sha2::Sha256; - use super::calculate_yandex_code; #[test] @@ -119,7 +107,7 @@ mod tests { let seconds: u64 = 1641559648; assert_eq!( - calculate_yandex_code::( + calculate_yandex_code( "6SB2IKNM6OBZPAVBVTOHDKS4FAAAAAAADFUTQMBTRY", "5239", 30, diff --git a/src/otp/from_otp_uri.rs b/src/otp/from_otp_uri.rs index 04400ff1..594fb6af 100644 --- a/src/otp/from_otp_uri.rs +++ b/src/otp/from_otp_uri.rs @@ -1,16 +1,25 @@ -use color_eyre::eyre::ErrReport; +use eyre::ErrReport; use url::Url; -use super::{otp_algorithm::OTPAlgorithm, otp_element::OTPElement, otp_type::OTPType}; +use super::{ + otp_algorithm::OTPAlgorithm, + otp_element::{OTPElement, OTPElementBuilder}, + otp_type::OTPType, +}; pub trait FromOtpUri: Sized { - fn from_otp_uri(otp_uri: &str) -> color_eyre::Result; + fn from_otp_uri(otp_uri: &str) -> eyre::Result; } impl FromOtpUri for OTPElement { - fn from_otp_uri(otp_uri: &str) -> color_eyre::Result { - let decoded = urlencoding::decode(otp_uri).map_err(ErrReport::from)?; - let parsed_uri = Url::parse(&decoded).map_err(ErrReport::from)?; + fn from_otp_uri(otp_uri: &str) -> eyre::Result { + // Parse the raw URI: percent-decoding must only ever happen on the + // individual components. Decoding the whole URI up front turns encoded + // structural characters into real ones (e.g. "%23" -> "#" makes the + // rest of the URI a fragment, "%26" -> "&" splits a query value) and + // decodes every query value twice, corrupting values that contain a + // literal "%25". + let parsed_uri = Url::parse(otp_uri).map_err(ErrReport::from)?; let otp_type = parsed_uri .host_str() @@ -18,10 +27,15 @@ impl FromOtpUri for OTPElement { let (issuer, label) = get_issuer_and_label(&parsed_uri)?; + // The secret is taken as-is: case normalization is applied by + // OTPElementBuilder depending on the OTP type. Base32 secrets + // (TOTP/HOTP/Steam/Yandex) are uppercased, while MOTP secrets are hex + // strings fed as text into MD5, so their case must not be folded to + // uppercase or the generated codes would be wrong. let secret = parsed_uri .query_pairs() .find(|(k, _v)| k == "secret") - .map(|(_k, v)| v.to_uppercase()) + .map(|(_k, v)| v.to_string()) .ok_or(ErrReport::msg("Secret not found in OTP Uri"))?; let algorithm = parsed_uri @@ -44,50 +58,56 @@ impl FromOtpUri for OTPElement { .find(|(k, _v)| k == "counter") .and_then(|(_k, v)| v.parse::().ok()); - Ok(OTPElement { - secret, - issuer, - label, - digits, - type_: OTPType::from(otp_type.as_str()), - algorithm: OTPAlgorithm::from(algorithm.as_str()), - period, - counter, - pin: None, - }) + let pin = parsed_uri + .query_pairs() + .find(|(k, _v)| k == "pin") + .map(|(_k, v)| v.to_string()); + + // Build through OTPElementBuilder so its validation (secret encoding, + // period, digits) applies to URI imports too. The type must be set + // after the secret, so the builder can normalize the secret case. + OTPElementBuilder::default() + .secret(secret) + .type_(OTPType::try_from(otp_type.as_str())?) + .issuer(issuer) + .label(label) + .digits(digits) + .algorithm(OTPAlgorithm::try_from(algorithm.as_str())?) + .period(period) + .counter(counter) + .pin(pin) + .build() } } -fn get(parsed_uri: &Url) -> color_eyre::Result> { - let first_segment: Vec = parsed_uri +/// Extracts the "issuer:label" parts from the first path segment of the URI. +/// +/// The raw segment is percent-decoded first, then split on ':'. Decoding +/// before splitting keeps the historical behavior of treating an encoded +/// colon ("%3A") as the issuer/label separator (see GH issue 548). +fn issuer_label_segments(parsed_uri: &Url) -> eyre::Result> { + let raw_segment = parsed_uri .path_segments() - .map(Iterator::collect::>) .ok_or(ErrReport::msg("Failed to collect path segments"))? - .first() - .ok_or(ErrReport::msg("No path segments found"))? + .next() + .ok_or(ErrReport::msg("No path segments found"))?; + + let decoded = urlencoding::decode(raw_segment) + .map_err(ErrReport::from)? + .into_owned(); + + Ok(decoded .split(':') - .collect::>() - .into_iter() .map(std::borrow::ToOwned::to_owned) - .collect(); - Ok(first_segment) + .collect()) } -fn get_issuer_and_label(parsed_uri: &Url) -> color_eyre::Result<(String, String)> { +fn get_issuer_and_label(parsed_uri: &Url) -> eyre::Result<(String, String)> { // Find the first path segments, OTP Uris should not have others - let first_segment = get(parsed_uri)?; - - let first = first_segment.first().and_then(|v| { - urlencoding::decode(v.as_str()) - .map(std::borrow::Cow::into_owned) - .ok() - }); + let first_segment = issuer_label_segments(parsed_uri)?; - let second = first_segment.get(1).and_then(|v| { - urlencoding::decode(v) - .map(std::borrow::Cow::into_owned) - .ok() - }); + let first = first_segment.first().cloned(); + let second = first_segment.get(1).cloned(); match (first, second) { (Some(i), Some(l)) => Ok((i, l)), @@ -102,3 +122,65 @@ fn get_issuer_and_label(parsed_uri: &Url) -> color_eyre::Result<(String, String) _ => Err(ErrReport::msg("No label found in OTP uri")), } } + +#[cfg(test)] +mod tests { + use super::FromOtpUri; + use crate::otp::otp_element::OTPElement; + + #[test] + fn test_encoded_hash_in_label_does_not_break_query_parsing() { + // "%23" must stay part of the label; pre-decoding the whole URI turned + // it into "#", making everything after it a fragment and losing the + // secret. + let uri = "otpauth://totp/C%23Corp?secret=JBSWY3DPEHPK3PXP"; + + let element = OTPElement::from_otp_uri(uri).unwrap(); + + assert_eq!("C#Corp", element.label); + assert_eq!("JBSWY3DPEHPK3PXP", element.secret); + } + + #[test] + fn test_encoded_question_mark_in_label_does_not_break_query_parsing() { + let uri = "otpauth://totp/Que%3FStion?secret=JBSWY3DPEHPK3PXP"; + + let element = OTPElement::from_otp_uri(uri).unwrap(); + + assert_eq!("Que?Stion", element.label); + assert_eq!("JBSWY3DPEHPK3PXP", element.secret); + } + + #[test] + fn test_encoded_ampersand_in_label_and_query_value() { + let uri = "otpauth://totp/A%26B?secret=JBSWY3DPEHPK3PXP&issuer=C%26D"; + + let element = OTPElement::from_otp_uri(uri).unwrap(); + + assert_eq!("A&B", element.label); + assert_eq!("C&D", element.issuer); + assert_eq!("JBSWY3DPEHPK3PXP", element.secret); + } + + #[test] + fn test_percent_encoded_label_is_decoded_exactly_once() { + // Label text "50%off" is encoded as "50%25off" and must not be + // decoded twice. + let uri = "otpauth://totp/50%25off?secret=JBSWY3DPEHPK3PXP"; + + let element = OTPElement::from_otp_uri(uri).unwrap(); + + assert_eq!("50%off", element.label); + } + + #[test] + fn test_double_encoded_label_keeps_literal_percent_sequence() { + // Label text literally containing "%25" is encoded as "%2525" and + // must decode back to "%25", not to "%". + let uri = "otpauth://totp/x%2525y?secret=JBSWY3DPEHPK3PXP"; + + let element = OTPElement::from_otp_uri(uri).unwrap(); + + assert_eq!("x%25y", element.label); + } +} diff --git a/src/otp/migrations/mod.rs b/src/otp/migrations/mod.rs index 18b04265..b14a16e7 100644 --- a/src/otp/migrations/mod.rs +++ b/src/otp/migrations/mod.rs @@ -1,26 +1,29 @@ use super::otp_element::OTPDatabase; struct Migration<'a> { to_version: u16, // Database version which we are migrating on - migration_function: &'a dyn Fn(&mut OTPDatabase) -> color_eyre::Result<()>, // Function to execute the migration + migration_function: &'a dyn Fn(&mut OTPDatabase) -> eyre::Result<()>, // Function to execute the migration } +/// Migrations must be kept sorted by ascending `to_version`; `migrate` relies +/// on this ordering and asserts it in debug builds. const MIGRATIONS_LIST: [Migration; 1] = [Migration { to_version: 2, migration_function: &migrate_to_2, }]; -fn migrate_to_2(database: &mut OTPDatabase) -> color_eyre::Result<()> { +fn migrate_to_2(database: &mut OTPDatabase) -> eyre::Result<()> { database.version = 2; Ok(()) } -pub fn migrate(database: &mut OTPDatabase) -> color_eyre::Result<()> { - let mut binding = MIGRATIONS_LIST; - let migrations = binding.as_mut(); - migrations.sort_unstable_by_key(|c1| c1.to_version); - for i in migrations { - if database.version < i.to_version { +pub fn migrate(database: &mut OTPDatabase) -> eyre::Result<()> { + debug_assert!( + MIGRATIONS_LIST.is_sorted_by_key(|m| m.to_version), + "MIGRATIONS_LIST must be sorted by to_version" + ); + for migration in &MIGRATIONS_LIST { + if database.version < migration.to_version { // Do the migration - (i.migration_function)(database)?; + (migration.migration_function)(database)?; } } Ok(()) diff --git a/src/otp/otp_algorithm.rs b/src/otp/otp_algorithm.rs index 79fc96c6..5b6d937c 100644 --- a/src/otp/otp_algorithm.rs +++ b/src/otp/otp_algorithm.rs @@ -26,13 +26,21 @@ impl fmt::Display for OTPAlgorithm { } } -impl From<&str> for OTPAlgorithm { - fn from(s: &str) -> Self { +impl TryFrom<&str> for OTPAlgorithm { + type Error = eyre::Report; + + /// Parses an OTP algorithm name case-insensitively, rejecting unknown + /// values instead of silently defaulting to SHA1 (which would generate + /// wrong codes for entries using a different, unsupported algorithm). + fn try_from(s: &str) -> Result { match s.to_uppercase().as_str() { - "SHA256" => Self::Sha256, - "SHA512" => Self::Sha512, - "MD5" => Self::Md5, - _ => Self::Sha1, + "SHA1" => Ok(Self::Sha1), + "SHA256" => Ok(Self::Sha256), + "SHA512" => Ok(Self::Sha512), + "MD5" => Ok(Self::Md5), + _ => Err(eyre::eyre!( + "Unknown OTP algorithm: {s:?} (expected one of SHA1, SHA256, SHA512, MD5)" + )), } } } @@ -42,3 +50,30 @@ impl Zeroize for OTPAlgorithm { *self = OTPAlgorithm::Sha1; } } + +#[cfg(test)] +mod tests { + use super::OTPAlgorithm; + + #[test] + fn known_algorithms_parse_case_insensitively() { + assert_eq!(OTPAlgorithm::Sha1, OTPAlgorithm::try_from("sha1").unwrap()); + assert_eq!(OTPAlgorithm::Sha1, OTPAlgorithm::try_from("SHA1").unwrap()); + assert_eq!( + OTPAlgorithm::Sha256, + OTPAlgorithm::try_from("Sha256").unwrap() + ); + assert_eq!( + OTPAlgorithm::Sha512, + OTPAlgorithm::try_from("sha512").unwrap() + ); + assert_eq!(OTPAlgorithm::Md5, OTPAlgorithm::try_from("md5").unwrap()); + } + + #[test] + fn unknown_algorithm_is_an_error_instead_of_defaulting_to_sha1() { + let error = OTPAlgorithm::try_from("crc32").unwrap_err(); + assert!(error.to_string().contains("Unknown OTP algorithm")); + assert!(error.to_string().contains("crc32")); + } +} diff --git a/src/otp/otp_element.rs b/src/otp/otp_element.rs index a8ff6e60..a50ec9a4 100644 --- a/src/otp/otp_element.rs +++ b/src/otp/otp_element.rs @@ -1,11 +1,8 @@ -use color_eyre::eyre::{ErrReport, eyre}; use derive_builder::Builder; -use std::{fs::File, io::Write, vec}; +use eyre::{ErrReport, eyre}; -use crate::crypto::cryptography::{argon_derive_key, encrypt_string_with_key, gen_salt}; use crate::otp::otp_error::OtpError; -use crate::path::DATABASE_PATH; -use data_encoding::BASE32_NOPAD; +use data_encoding::{BASE32_NOPAD, HEXLOWER_PERMISSIVE}; use qrcode::QrCode; use qrcode::render::unicode; use serde::{Deserialize, Serialize}; @@ -16,7 +13,6 @@ use super::{ hotp_maker::hotp, motp_maker::motp, steam_otp_maker::steam, totp_maker::totp, yandex_otp_maker::yandex, }, - migrations::migrate, otp_algorithm::OTPAlgorithm, otp_type::OTPType, }; @@ -26,9 +22,12 @@ pub const CURRENT_DATABASE_VERSION: u16 = 2; #[derive(Serialize, Deserialize, PartialEq, Hash)] pub struct OTPDatabase { pub(crate) version: u16, - pub(crate) elements: Vec, + elements: Vec, + /// Dirty flag gating the final save in `main()`. Private on purpose: it is + /// only toggled through [`OTPDatabase::mark_modified`] and + /// [`OTPDatabase::clear_modified`]. #[serde(skip)] - pub(crate) needs_modification: bool, + needs_modification: bool, } impl From> for OTPDatabase { @@ -57,36 +56,6 @@ impl OTPDatabase { self.needs_modification } - pub fn save(&mut self, key: &Vec, salt: &[u8]) -> color_eyre::Result<()> { - self.needs_modification = false; - migrate(self)?; - match self.overwrite_database_key(key, salt) { - Ok(()) => Ok(()), - Err(e) => Err(ErrReport::from(e)), - } - } - - fn overwrite_database_key(&self, key: &Vec, salt: &[u8]) -> Result<(), std::io::Error> { - let json: &str = &serde_json::to_string(&self)?; - let encrypted = encrypt_string_with_key(json, key, salt).unwrap(); - let mut file = File::create(DATABASE_PATH.get().unwrap())?; - match serde_json::to_string(&encrypted) { - Ok(content) => { - file.write_all(content.as_bytes())?; - file.sync_all()?; - Ok(()) - } - Err(e) => Err(std::io::Error::from(e)), - } - } - - pub fn save_with_pw(&mut self, password: &str) -> color_eyre::Result<(Vec, [u8; 16])> { - let salt = gen_salt()?; - let key = argon_derive_key(password.as_bytes(), &salt)?; - self.save(&key, &salt)?; - Ok((key, salt)) - } - pub fn add_all(&mut self, mut elements: Vec) { self.mark_modified(); self.elements.append(&mut elements); @@ -97,10 +66,17 @@ impl OTPDatabase { self.elements.push(element); } + /// Marks the database as dirty so `main()` persists it on exit. pub fn mark_modified(&mut self) { self.needs_modification = true; } + /// Discards the dirty flag so the database will NOT be persisted on exit + /// (e.g. the user answered "don't save" when quitting the dashboard). + pub fn clear_modified(&mut self) { + self.needs_modification = false; + } + pub fn delete_element(&mut self, index: usize) { self.mark_modified(); self.elements.remove(index); @@ -110,10 +86,21 @@ impl OTPDatabase { &self.elements } + /// Consumes the database and returns its elements, for exporters that + /// need owned values. + pub fn into_elements(self) -> Vec { + self.elements + } + pub fn get_element(&self, i: usize) -> Option<&OTPElement> { self.elements.get(i) } + /// Mutable access to an element. Deliberately does NOT mark the database + /// as modified: callers must call [`Self::mark_modified`] themselves once + /// they know an actual change happened, which lets no-op edits (e.g. + /// `cotp edit` with values identical to the current ones) skip the + /// re-encryption and rewrite of the database file. pub fn mut_element(&mut self, i: usize) -> Option<&mut OTPElement> { self.elements.get_mut(i) } @@ -127,9 +114,7 @@ impl OTPDatabase { } } -#[derive( - Serialize, Deserialize, Builder, Clone, PartialEq, Eq, Debug, Hash, Zeroize, ZeroizeOnDrop, -)] +#[derive(Serialize, Deserialize, Builder, Clone, PartialEq, Eq, Hash, Zeroize, ZeroizeOnDrop)] #[builder( setter(into), build_fn(validate = "Self::validate", error = "ErrReport") @@ -154,6 +139,24 @@ pub struct OTPElement { pub pin: Option, } +/// Hand-written Debug implementation which redacts the secret and the pin, so +/// they cannot leak into logs, error reports or test output. +impl std::fmt::Debug for OTPElement { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("OTPElement") + .field("secret", &"***") + .field("issuer", &self.issuer) + .field("label", &self.label) + .field("digits", &self.digits) + .field("type_", &self.type_) + .field("algorithm", &self.algorithm) + .field("period", &self.period) + .field("counter", &self.counter) + .field("pin", &self.pin.as_ref().map(|_| "***")) + .finish() + } +} + static ALLOWED_DIGITS_RANGE: std::ops::RangeInclusive = 1..=10; impl OTPElement { @@ -172,16 +175,28 @@ impl OTPElement { uri.push_str("&counter="); uri.push_str(self.counter.unwrap_or(0).to_string().as_str()); } + + // Yandex / MOTP codes cannot be generated without their pin, so it + // must survive an export / import round-trip + if let Some(pin) = &self.pin { + uri.push_str("&pin="); + uri.push_str(&urlencoding::encode(pin)); + } uri } pub fn get_qrcode(&self) -> String { - QrCode::new(self.get_otpauth_uri()) - .unwrap() - .render::() - .dark_color(unicode::Dense1x2::Light) - .light_color(unicode::Dense1x2::Dark) - .build() + // The otpauth URI can exceed the maximum QR code capacity (e.g. very + // long labels or secrets). Return a printable message instead of + // panicking, since this is rendered inside the TUI. + match QrCode::new(self.get_otpauth_uri()) { + Ok(qrcode) => qrcode + .render::() + .dark_color(unicode::Dense1x2::Light) + .light_color(unicode::Dense1x2::Dark) + .build(), + Err(_) => String::from("Cannot render QR code: data too long"), + } } pub fn get_otp_code(&self) -> Result { @@ -210,17 +225,16 @@ impl OTPElement { pin.as_str(), self.period, self.digits as usize, - self.algorithm, ), None => Err(OtpError::MissingPin), }, OTPType::Motp => match &self.pin { - Some(pin) => Ok(motp( + Some(pin) => motp( &self.secret, pin.as_str(), self.period, self.digits as usize, - )), + ), None => Err(OtpError::MissingPin), }, } @@ -272,9 +286,18 @@ impl OTPElementBuilder { return Err(eyre!("Secret must not be empty",)); } + if self.period == Some(0) { + return Err(eyre!("Period must be greater than zero",)); + } + + if self.digits == Some(0) { + return Err(eyre!("Digits must be greater than zero",)); + } + // Validate secret encoding match self.type_.unwrap_or_default() { - OTPType::Motp => hex::decode(self.secret.as_ref().unwrap()) + OTPType::Motp => HEXLOWER_PERMISSIVE + .decode(self.secret.as_ref().unwrap().as_bytes()) .map(|_| {}) .map_err(|e| eyre!("Invalid hex secret: {e}")), _ => BASE32_NOPAD @@ -450,6 +473,53 @@ mod test { assert_eq!("aaaf", result.unwrap().secret); } + #[test] + fn test_zero_period_is_rejected_by_builder() { + let result = OTPElementBuilder::default() + .secret("AA") + .label("label") + .issuer("") + .period(0u64) + .build(); + + assert_eq!( + "Period must be greater than zero", + result.unwrap_err().to_string() + ); + } + + #[test] + fn test_zero_digits_is_rejected_by_builder() { + let result = OTPElementBuilder::default() + .secret("AA") + .label("label") + .issuer("") + .digits(0u64) + .build(); + + assert_eq!( + "Digits must be greater than zero", + result.unwrap_err().to_string() + ); + } + + #[test] + fn test_zero_period_returns_error_instead_of_panicking() { + let element = OTPElement { + secret: "xr5gh44x7bprcqgrdtulafeevt5rxqlbh5wvked22re43dh2d4mapv5g".to_uppercase(), + issuer: String::from("IssuerText"), + label: String::from("LabelText"), + digits: 6, + type_: Totp, + algorithm: Sha1, + period: 0, + counter: None, + pin: None, + }; + + assert_eq!(Err(OtpError::InvalidPeriod), element.get_otp_code()); + } + #[test] fn invalid_secret_hex() { let result = OTPElementBuilder::default() @@ -460,11 +530,86 @@ mod test { .build(); assert_eq!( - "Invalid hex secret: Odd number of digits", + "Invalid hex secret: invalid length at 2", result.unwrap_err().to_string() ); } + fn assert_generation_relevant_fields_eq(expected: &OTPElement, actual: &OTPElement) { + assert_eq!(expected.secret, actual.secret); + assert_eq!(expected.type_, actual.type_); + assert_eq!(expected.algorithm, actual.algorithm); + assert_eq!(expected.digits, actual.digits); + assert_eq!(expected.period, actual.period); + assert_eq!(expected.counter, actual.counter); + assert_eq!(expected.pin, actual.pin); + } + + #[test] + fn test_otp_uri_round_trip_totp() { + let element = OTPElementBuilder::default() + .secret("xr5gh44x7bprcqgrdtulafeevt5rxqlbh5wvked22re43dh2d4mapv5g") + .issuer("IssuerText") + .label("LabelText") + .build() + .unwrap(); + + let round_tripped = OTPElement::from_otp_uri(&element.get_otpauth_uri()).unwrap(); + + assert_generation_relevant_fields_eq(&element, &round_tripped); + } + + #[test] + fn test_otp_uri_round_trip_motp_preserves_lowercase_secret_and_pin() { + let element = OTPElementBuilder::default() + .secret("e3152afee62599c8") + .type_(OTPType::Motp) + .issuer("IssuerText") + .label("LabelText") + .period(10u64) + .pin("1234".to_string()) + .build() + .unwrap(); + + let round_tripped = OTPElement::from_otp_uri(&element.get_otpauth_uri()).unwrap(); + + assert_generation_relevant_fields_eq(&element, &round_tripped); + // MOTP secrets are hex text hashed with MD5: uppercasing them changes + // the generated codes + assert_eq!("e3152afee62599c8", round_tripped.secret); + assert_eq!(Some("1234".to_string()), round_tripped.pin); + } + + #[test] + fn test_otp_uri_round_trip_yandex_preserves_pin() { + let element = OTPElementBuilder::default() + .secret("6SB2IKNM6OBZPAVBVTOHDKS4FAAAAAAADFUTQMBTRY") + .type_(OTPType::Yandex) + .issuer("Yandex") + .label("LabelText") + .digits(8u64) + .pin("5239".to_string()) + .build() + .unwrap(); + + let round_tripped = OTPElement::from_otp_uri(&element.get_otpauth_uri()).unwrap(); + + assert_generation_relevant_fields_eq(&element, &round_tripped); + assert_eq!(Some("5239".to_string()), round_tripped.pin); + // Both must generate a code, not fail with a missing pin + assert_eq!(element.get_otp_code(), round_tripped.get_otp_code()); + } + + #[test] + fn test_from_otp_uri_rejects_invalid_base32_secret() { + // Construction goes through OTPElementBuilder, so its validation + // applies to URI imports too + // "aaa" has an invalid BASE32 length and "1" is not in the alphabet + let otp_uri = "otpauth://totp/Label?secret=aa1"; + + assert!(OTPElement::from_otp_uri(otp_uri).is_err()); + } + #[test] fn gh_issue_548_invalid_otp_uri_label_url_encoded() { // Arrange diff --git a/src/otp/otp_error.rs b/src/otp/otp_error.rs index f8aefd9d..73c5ec7d 100644 --- a/src/otp/otp_error.rs +++ b/src/otp/otp_error.rs @@ -10,6 +10,7 @@ pub enum OtpError { InvalidOffset, // Invalid offset InvalidDigest, // Invalid digest InvalidDigits, // Invalid Digits value (too high or low) + InvalidPeriod, // Invalid period value (zero) } impl Display for OtpError { @@ -24,6 +25,7 @@ impl Display for OtpError { OtpError::InvalidOffset => f.write_str("Invalid offset"), OtpError::ShortSecret => f.write_str("Secret length less than 16 bytes"), OtpError::InvalidDigits => f.write_str("Digits value too high or low"), + OtpError::InvalidPeriod => f.write_str("Period value must be greater than zero"), } } } diff --git a/src/otp/otp_type.rs b/src/otp/otp_type.rs index 4a845446..57af8bd0 100644 --- a/src/otp/otp_type.rs +++ b/src/otp/otp_type.rs @@ -38,14 +38,22 @@ impl fmt::Display for OTPType { } } -impl From<&str> for OTPType { - fn from(s: &str) -> Self { +impl TryFrom<&str> for OTPType { + type Error = eyre::Report; + + /// Parses an OTP type name case-insensitively, rejecting unknown values + /// instead of silently defaulting to TOTP (which would generate wrong + /// codes for entries of a different, unsupported type). + fn try_from(s: &str) -> Result { match s.to_uppercase().as_str() { - "HOTP" => Self::Hotp, - "STEAM" => Self::Steam, - "YANDEX" => Self::Yandex, - "MOTP" => Self::Motp, - _ => Self::Totp, + "TOTP" => Ok(Self::Totp), + "HOTP" => Ok(Self::Hotp), + "STEAM" => Ok(Self::Steam), + "YANDEX" => Ok(Self::Yandex), + "MOTP" => Ok(Self::Motp), + _ => Err(eyre::eyre!( + "Unknown OTP type: {s:?} (expected one of TOTP, HOTP, STEAM, YANDEX, MOTP)" + )), } } } @@ -55,3 +63,25 @@ impl Zeroize for OTPType { *self = OTPType::Totp; } } + +#[cfg(test)] +mod tests { + use super::OTPType; + + #[test] + fn known_types_parse_case_insensitively() { + assert_eq!(OTPType::Totp, OTPType::try_from("totp").unwrap()); + assert_eq!(OTPType::Totp, OTPType::try_from("TOTP").unwrap()); + assert_eq!(OTPType::Hotp, OTPType::try_from("HoTp").unwrap()); + assert_eq!(OTPType::Steam, OTPType::try_from("steam").unwrap()); + assert_eq!(OTPType::Yandex, OTPType::try_from("YANDEX").unwrap()); + assert_eq!(OTPType::Motp, OTPType::try_from("Motp").unwrap()); + } + + #[test] + fn unknown_type_is_an_error_instead_of_defaulting_to_totp() { + let error = OTPType::try_from("otp-2000").unwrap_err(); + assert!(error.to_string().contains("Unknown OTP type")); + assert!(error.to_string().contains("otp-2000")); + } +} diff --git a/src/reading.rs b/src/reading.rs deleted file mode 100644 index eb752fed..00000000 --- a/src/reading.rs +++ /dev/null @@ -1,63 +0,0 @@ -use crate::crypto; -use crate::otp::otp_element::{OTPDatabase, OTPElement}; -use crate::path::DATABASE_PATH; -use crate::utils; -use color_eyre::eyre::{ErrReport, eyre}; -use std::fs::read_to_string; -use std::io::{self, BufRead}; -use zeroize::Zeroize; - -pub type ReadResult = (OTPDatabase, Vec, Vec); - -pub fn get_elements_from_input() -> color_eyre::Result { - let pw = utils::password("Password: ", 8); - get_elements_with_password(pw) -} - -pub fn get_elements_from_stdin() -> color_eyre::Result { - if let Some(password) = io::stdin().lock().lines().next() { - return get_elements_with_password(password?); - } - Err(eyre!("Failure during stdin reading")) -} - -fn get_elements_with_password(mut password: String) -> color_eyre::Result { - let (elements, key, salt) = read_from_file(&password)?; - password.zeroize(); - Ok((elements, key, salt)) -} - -pub fn read_decrypted_text(password: &str) -> color_eyre::Result<(String, Vec, Vec)> { - let encrypted_contents = - read_to_string(DATABASE_PATH.get().unwrap()).map_err(ErrReport::from)?; - if encrypted_contents.is_empty() { - return match delete_db() { - Ok(()) => Err(eyre!( - "Your database file was empty, please restart to create a new one.", - )), - Err(_) => Err(eyre!( - "Your database file is empty, please remove it manually and restart.", - )), - }; - } - //rust close files at the end of the function - crypto::cryptography::decrypt_string(&encrypted_contents, password) -} - -pub fn read_from_file(password: &str) -> color_eyre::Result { - match read_decrypted_text(password) { - Ok((mut contents, key, salt)) => { - let mut database: OTPDatabase = serde_json::from_str(&contents) - .or_else(|_| serde_json::from_str::>(&contents).map(Into::into)) - .map_err(ErrReport::from)?; - contents.zeroize(); - database.sort(); - Ok((database, key, salt)) - } - Err(e) => Err(e), - } -} - -fn delete_db() -> io::Result<()> { - std::fs::remove_file(DATABASE_PATH.get().unwrap()) -} diff --git a/src/storage/mod.rs b/src/storage/mod.rs new file mode 100644 index 00000000..354369b2 --- /dev/null +++ b/src/storage/mod.rs @@ -0,0 +1,143 @@ +//! Persistence layer for the OTP database. +//! +//! [`OTPDatabase`] holds only domain data; this module owns everything about +//! moving it to and from disk: password prompting, decryption and +//! deserialization on load (including the legacy v1 format fallback), and +//! migration, encryption and the actual filesystem write on save. + +use std::fs::{File, read_to_string}; +use std::io::{self, BufRead, Write}; +use std::path::Path; + +use eyre::{ErrReport, eyre}; +use zeroize::Zeroize; + +use crate::crypto::cryptography::{ + argon_derive_key, decrypt_string, encrypt_string_with_key, gen_salt, +}; +use crate::otp::migrations::migrate; +use crate::otp::otp_element::{OTPDatabase, OTPElement}; +use crate::path::DATABASE_PATH; +use crate::utils; + +pub type ReadResult = (OTPDatabase, Vec, Vec); + +pub fn get_elements_from_input() -> eyre::Result { + let pw = utils::try_password("Password: ", 8)?; + get_elements_with_password(pw) +} + +pub fn get_elements_from_stdin() -> eyre::Result { + if let Some(password) = io::stdin().lock().lines().next() { + return get_elements_with_password(password?); + } + Err(eyre!("Failure during stdin reading")) +} + +fn get_elements_with_password(mut password: String) -> eyre::Result { + let read_result = read_from_file(DATABASE_PATH.get().unwrap(), &password); + password.zeroize(); + read_result +} + +fn read_decrypted_text(path: &Path, password: &str) -> eyre::Result<(String, Vec, Vec)> { + let encrypted_contents = read_to_string(path).map_err(ErrReport::from)?; + if encrypted_contents.is_empty() { + // Do not delete the file here: silently destroying a user file from a + // read path is surprising and irreversible. An empty file can also be + // the leftover of an interrupted write, in which case the user may + // want to restore a backup instead of starting over. + return Err(eyre!( + "Your database file at {path:?} is empty or corrupted. If you have a backup, restore it over that path; otherwise remove the file manually and restart cotp to initialize a new database.", + )); + } + //rust close files at the end of the function + decrypt_string(&encrypted_contents, password) +} + +fn read_from_file(path: &Path, password: &str) -> eyre::Result { + let (mut contents, key, salt) = read_decrypted_text(path, password)?; + let mut database: OTPDatabase = serde_json::from_str(&contents) + .or_else(|_| serde_json::from_str::>(&contents).map(Into::into)) + .map_err(ErrReport::from)?; + contents.zeroize(); + database.sort(); + Ok((database, key, salt)) +} + +/// Encrypts the database with the given key and writes it to `path`. +/// +/// `migrate()` runs on every save, so the written database is always at the +/// current schema version. The modified flag is cleared only AFTER a +/// successful write, so a failed save leaves the database marked dirty. +/// +/// Clearing the flag on success is also what makes the `passwd` flow safe: +/// `passwd` persists the database itself via [`save_with_pw`] (new salt + key +/// derived from the new password), and the cleared flag makes the final +/// `is_modified()` check in `main()` skip its own save — which would +/// otherwise re-encrypt the database with the OLD key and silently undo the +/// password change. +pub fn save( + database: &mut OTPDatabase, + key: &Vec, + salt: &[u8], + path: &Path, +) -> eyre::Result<()> { + migrate(database)?; + encrypt_and_write(database, key, salt, path)?; + database.clear_modified(); + Ok(()) +} + +/// Derives a fresh salt + key from `password` and saves the database with +/// them, returning both so the caller can keep using the new key. +pub fn save_with_pw( + database: &mut OTPDatabase, + password: &str, + path: &Path, +) -> eyre::Result<(Vec, [u8; 16])> { + let salt = gen_salt()?; + let key = argon_derive_key(password.as_bytes(), &salt)?; + save(database, &key, &salt, path)?; + Ok((key, salt)) +} + +fn encrypt_and_write( + database: &OTPDatabase, + key: &Vec, + salt: &[u8], + path: &Path, +) -> eyre::Result<()> { + // The plaintext JSON contains every secret in the database: wipe it + // from memory as soon as it has been encrypted + let mut json = serde_json::to_string(database)?; + let encrypted = encrypt_string_with_key(&json, key, salt); + json.zeroize(); + let encrypted = encrypted?; + let mut file = create_database_file(path)?; + let content = serde_json::to_string(&encrypted)?; + file.write_all(content.as_bytes())?; + file.sync_all()?; + Ok(()) +} + +/// Creates (or truncates) the database file. +/// +/// On unix the file is created with mode 0600 so other users cannot read it. +/// The database is encrypted, so this is defense in depth rather than a +/// confidentiality requirement. +#[cfg(unix)] +fn create_database_file(path: &Path) -> std::io::Result { + use std::os::unix::fs::OpenOptionsExt; + std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(path) +} + +#[cfg(not(unix))] +fn create_database_file(path: &Path) -> std::io::Result { + File::create(path) +} diff --git a/src/utils.rs b/src/utils.rs index fb328dc6..6d6b7fb5 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,17 +1,37 @@ +use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; +use eyre::eyre; +use zeroize::Zeroize; + use crate::path::DATABASE_PATH; -pub fn init_app() -> Result { +pub fn init_app() -> eyre::Result { let db_path = DATABASE_PATH.get().unwrap(); // Safe to unwrap because we initialize - let db_dir = db_path.parent().unwrap(); - if !db_dir.exists() { - if let Err(_e) = std::fs::create_dir_all(db_dir) { - return Err(()); - } - return Ok(true); + + // Decide whether this is a first run from the database file itself: relying on + // the parent directory is wrong for bare relative paths (e.g. `-d db.cotp`), + // whose parent is the empty path and never "exists", which previously caused an + // existing database to be re-initialized and overwritten. + if db_path.exists() { + return Ok(false); + } + + // First run: make sure the parent directory exists. An empty parent means the + // current working directory. + let db_dir = match db_path.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent, + _ => Path::new("."), + }; + if !db_dir.exists() + && let Err(e) = std::fs::create_dir_all(db_dir) + { + return Err(eyre!( + "Cannot create the database directory {}: {e}", + db_dir.display() + )); } - Ok(!db_path.exists()) + Ok(true) } pub fn millis_before_next_step() -> u64 { @@ -25,25 +45,41 @@ pub fn percentage() -> u16 { (millis_before_next_step() * 100 / 30000) as u16 } -pub fn password(message: &str, minimum_length: usize) -> String { +/// Prompts for a password of at least `minimum_length` characters. +/// +/// Reading the password can fail (e.g. stdin is not a TTY): the error is +/// propagated to the caller instead of exiting the process. +pub fn try_password(message: &str, minimum_length: usize) -> std::io::Result { loop { - let password = rpassword::prompt_password(message).unwrap(); + let mut password = rpassword::prompt_password(message)?; if password.chars().count() < minimum_length { + password.zeroize(); println!("Please insert a password with at least {minimum_length} digits."); continue; } - return password; + return Ok(password); } } -pub fn verified_password(message: &str, minimum_length: usize) -> String { +/// Like [`try_password`], but asks the user to retype the password until both +/// entries match. +pub fn try_verified_password(message: &str, minimum_length: usize) -> std::io::Result { loop { - let password = password(message, minimum_length); - let verify_password = rpassword::prompt_password("Retype the same password: ").unwrap(); - if password != verify_password { + let mut password = try_password(message, minimum_length)?; + let mut verify_password = match rpassword::prompt_password("Retype the same password: ") { + Ok(verify_password) => verify_password, + Err(e) => { + password.zeroize(); + return Err(e); + } + }; + let matching = password == verify_password; + verify_password.zeroize(); + if !matching { + password.zeroize(); println!("Passwords do not match"); continue; } - return password; + return Ok(password); } } diff --git a/test_samples/freeotp_plus_hotp.json b/test_samples/freeotp_plus_hotp.json new file mode 100644 index 00000000..32674162 --- /dev/null +++ b/test_samples/freeotp_plus_hotp.json @@ -0,0 +1,28 @@ +{ + "tokenOrder": [ + "Example3:Label3" + ], + "tokens": [ + { + "algo": "SHA1", + "counter": 4, + "digits": 6, + "issuerExt": "Example3", + "label": "Label3", + "period": 30, + "secret": [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0 + ], + "type": "HOTP" + } + ] + } diff --git a/tests/add_integration_tests.rs b/tests/add_integration_tests.rs index cfd5900e..9a6a4993 100644 --- a/tests/add_integration_tests.rs +++ b/tests/add_integration_tests.rs @@ -1,9 +1,23 @@ #[cfg(not(target_os = "windows"))] // TODO, Integration tests currently does not work on Windows mod add_integration_tests { use assert_cmd::cargo::cargo_bin_cmd; + use assert_fs::TempDir; + use assert_fs::prelude::*; use predicates::{ord::eq, str::is_empty}; use test_case::test_case; + const FIXTURE_DIR: &str = "test_samples/cli_integration_test"; + const FIXTURE_NAME: &str = "empty_database"; + + /// Copies the committed database fixture into a temporary directory so + /// tests never mutate files tracked by git + fn temp_database() -> (TempDir, std::path::PathBuf) { + let temp = TempDir::new().unwrap(); + temp.copy_from(FIXTURE_DIR, &[FIXTURE_NAME]).unwrap(); + let database_path = temp.child(FIXTURE_NAME).path().to_path_buf(); + (temp, database_path) + } + #[test] fn add_without_label_should_fail() { // Arrange / Act @@ -30,12 +44,15 @@ For more information, try '--help'. #[test_case("-l" ; "Short subcommand")] #[test_case("--label" ; "Long subcommand")] fn add_with_label_should_work(label_arg: &str) { - // Arrange / Act + // Arrange + let (_temp, database_path) = temp_database(); + + // Act let mut command = cargo_bin_cmd!("cotp"); let assertion = command .arg("--password-stdin") .arg("--database-path") - .arg("test_samples/cli_integration_test/empty_database") + .arg(database_path) .arg("add") .arg(label_arg) .arg("test") diff --git a/tests/cli_integration_tests.rs b/tests/cli_integration_tests.rs index c1550c9b..080d2f97 100644 --- a/tests/cli_integration_tests.rs +++ b/tests/cli_integration_tests.rs @@ -17,6 +17,20 @@ mod cli_integration_test { .stderr(is_empty()); } + #[test] + fn test_delete_without_selector_is_rejected() { + // Arrange / Act: no --index, --issuer or --label is provided, so clap + // must reject the invocation instead of letting the matcher fall + // through to an empty-string match deleting the first element. + let mut command = cargo_bin_cmd!("cotp"); + let assertion = command.arg("delete").assert(); + + // Assert + assertion + .failure() + .stderr(is_match("required arguments were not provided").unwrap()); + } + #[test] fn test_help_subcommand() { // Arrange / Act diff --git a/tests/init_integration_tests.rs b/tests/init_integration_tests.rs new file mode 100644 index 00000000..1276e0e5 --- /dev/null +++ b/tests/init_integration_tests.rs @@ -0,0 +1,55 @@ +#[cfg(not(target_os = "windows"))] // TODO, Integration tests currently does not work on Windows +mod init_integration_tests { + use assert_cmd::cargo::cargo_bin_cmd; + use assert_fs::TempDir; + use assert_fs::prelude::*; + use predicates::str::contains; + + const FIXTURE_DIR: &str = "test_samples/cli_integration_test"; + const FIXTURE_NAME: &str = "empty_database"; + const FIXTURE_PASSWORD: &str = "12345678"; + + /// Regression test: when `--database-path` is a bare relative filename, an + /// existing database must be loaded instead of being treated as a first run + /// (which used to prompt for a new password and overwrite it with an empty + /// database). + #[test] + fn existing_database_with_bare_relative_path_survives() { + // Arrange: put an existing populated-able database in a temp working dir + let temp = TempDir::new().unwrap(); + temp.copy_from(FIXTURE_DIR, &[FIXTURE_NAME]).unwrap(); + + // Act 1: first invocation with a bare relative -d filename adds an element + let mut command = cargo_bin_cmd!("cotp"); + let assertion = command + .current_dir(temp.path()) + .arg("--password-stdin") + .arg("--database-path") + .arg(FIXTURE_NAME) + .arg("add") + .arg("--label") + .arg("relative-path-test") + .arg("--secret-stdin") + .write_stdin(format!("{FIXTURE_PASSWORD}\nAA\n")) + .assert(); + assertion + .success() + .stdout(contains("Modifications have been persisted")); + + // Act 2: second invocation with the same bare relative -d filename must + // load the existing database (not re-initialize it) and still contain + // the element added by the first run + let mut command = cargo_bin_cmd!("cotp"); + let assertion = command + .current_dir(temp.path()) + .arg("--password-stdin") + .arg("--database-path") + .arg(FIXTURE_NAME) + .arg("list") + .write_stdin(format!("{FIXTURE_PASSWORD}\n")) + .assert(); + + // Assert + assertion.success().stdout(contains("relative-path-test")); + } +}