From 834df468180d2b16af5a02c5a94ccb69057983e4 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Sun, 9 Aug 2026 22:41:33 +0000 Subject: [PATCH] Implement secure password generation (#8) --- README.md | 3 + crates/storage/src/generate.rs | 275 ++++++++++++++ crates/storage/src/lib.rs | 1 + crates/storage/src/write.rs | 42 +++ crates/storage/tests/password_generation.rs | 380 ++++++++++++++++++++ docs/password-generation.md | 25 ++ 6 files changed, 726 insertions(+) create mode 100644 crates/storage/src/generate.rs create mode 100644 crates/storage/tests/password_generation.rs create mode 100644 docs/password-generation.md diff --git a/README.md b/README.md index bc9d56e..b053b2d 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,9 @@ Typed list/show/find/decrypted-grep models and secret presentation selection are documented in [`docs/read-domains.md`](docs/read-domains.md). Insert modes, concurrency-safe edit sessions, and the secure CLI editor-file boundary are documented in [`docs/write-domains.md`](docs/write-domains.md). +Unbiased password generation, character-set validation, in-place replacement, +and presentation actions are documented in +[`docs/password-generation.md`](docs/password-generation.md). ## Project layout diff --git a/crates/storage/src/generate.rs b/crates/storage/src/generate.rs new file mode 100644 index 0000000..53f00df --- /dev/null +++ b/crates/storage/src/generate.rs @@ -0,0 +1,275 @@ +//! Unbiased cryptographic password generation and storage orchestration. + +use std::{collections::BTreeSet, error::Error, fmt, num::NonZeroUsize}; + +use rand::{CryptoRng, RngCore, rngs::OsRng}; + +use crate::{ + command::{GenerateRequest, GeneratedPresentation}, + crypto::{CryptoError, KeyStore, SecretProvider}, + recipient::SigningPolicy, + repository::{EntryPath, Repository, RepositoryError, SecretBytes}, + write::{EntryCommitter, OverwriteDecision, VaultWriter, WriteError, WriteOutcome}, +}; + +pub const DEFAULT_PASSWORD_LENGTH: usize = 25; +pub const MAX_PASSWORD_LENGTH: usize = 4096; +pub const DEFAULT_CHARACTER_SET: &str = "!\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~"; +pub const ALPHANUMERIC_CHARACTER_SET: &str = + "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GeneratorConfig { + default_length: NonZeroUsize, + character_set: Vec, + alphanumeric_set: Vec, +} + +impl GeneratorConfig { + pub fn new(default_length: usize, character_set: &str) -> Result { + Ok(Self { + default_length: validate_length(default_length)?, + character_set: validate_character_set(character_set)?, + alphanumeric_set: validate_character_set(ALPHANUMERIC_CHARACTER_SET)?, + }) + } + + pub fn pass_defaults() -> Self { + Self::new(DEFAULT_PASSWORD_LENGTH, DEFAULT_CHARACTER_SET) + .expect("built-in generator defaults are valid") + } + + pub fn default_length(&self) -> NonZeroUsize { + self.default_length + } + + pub fn character_set(&self) -> &[char] { + &self.character_set + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum GeneratedChannel { + Terminal, + Clipboard, + QrCode, +} + +pub struct GenerateOutcome { + password: SecretBytes, + channel: GeneratedChannel, + write: WriteOutcome, +} + +impl GenerateOutcome { + pub fn password(&self) -> &SecretBytes { + &self.password + } + + pub fn channel(&self) -> GeneratedChannel { + self.channel + } + + pub fn write(&self) -> &WriteOutcome { + &self.write + } +} + +impl fmt::Debug for GenerateOutcome { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("GenerateOutcome") + .field("password", &self.password) + .field("channel", &self.channel) + .field("write", &self.write) + .finish() + } +} + +pub struct PasswordGenerator<'a> { + repository: &'a Repository, + keys: &'a KeyStore, + config: GeneratorConfig, +} + +impl<'a> PasswordGenerator<'a> { + pub fn new(repository: &'a Repository, keys: &'a KeyStore, config: GeneratorConfig) -> Self { + Self { + repository, + keys, + config, + } + } + + #[allow(clippy::too_many_arguments)] + pub fn generate( + &self, + request: &GenerateRequest, + overwrite: OverwriteDecision, + signing: Option<&SigningPolicy>, + provider: &mut impl SecretProvider, + committer: &mut impl EntryCommitter, + ) -> Result { + self.generate_with_rng(request, overwrite, signing, provider, committer, &mut OsRng) + } + + #[allow(clippy::too_many_arguments)] + pub fn generate_with_rng( + &self, + request: &GenerateRequest, + overwrite: OverwriteDecision, + signing: Option<&SigningPolicy>, + provider: &mut impl SecretProvider, + committer: &mut impl EntryCommitter, + rng: &mut R, + ) -> Result { + if request.force && request.in_place { + return Err(GenerateError::IncompatibleFlags); + } + let length = request.length.unwrap_or(self.config.default_length); + let length = validate_length(length.get())?; + let characters = if request.no_symbols { + &self.config.alphanumeric_set + } else { + &self.config.character_set + }; + let password = generate_password(rng, length, characters)?; + let path = EntryPath::parse(&request.entry)?; + let contents = if request.in_place { + let ciphertext = self.repository.read_entry(&path)?; + let existing = self.keys.decrypt(&ciphertext, provider)?; + replace_first_line(&password, &existing) + } else { + SecretBytes::new(password.expose().to_vec()) + }; + let write = VaultWriter::new(self.repository, self.keys).store_generated( + &path, + contents, + request.force || request.in_place, + overwrite, + signing, + committer, + )?; + Ok(GenerateOutcome { + password, + channel: match request.presentation { + GeneratedPresentation::Terminal => GeneratedChannel::Terminal, + GeneratedPresentation::Clipboard => GeneratedChannel::Clipboard, + GeneratedPresentation::QrCode => GeneratedChannel::QrCode, + }, + write, + }) + } +} + +fn generate_password( + rng: &mut R, + length: NonZeroUsize, + characters: &[char], +) -> Result { + if characters.is_empty() { + return Err(GenerateError::EmptyCharacterSet); + } + let mut password = String::with_capacity(length.get()); + for _ in 0..length.get() { + let range = characters.len() as u64; + let unbiased_limit = u64::MAX - (u64::MAX % range); + let value = loop { + let mut bytes = [0_u8; 8]; + rng.try_fill_bytes(&mut bytes) + .map_err(|_| GenerateError::RandomnessUnavailable)?; + let value = u64::from_le_bytes(bytes); + if value < unbiased_limit { + break value; + } + }; + password.push(characters[(value % range) as usize]); + } + Ok(SecretBytes::new(password.into_bytes())) +} + +fn replace_first_line(password: &SecretBytes, existing: &SecretBytes) -> SecretBytes { + let suffix = existing + .expose() + .iter() + .position(|byte| *byte == b'\n') + .map_or(&[][..], |index| &existing.expose()[index..]); + let mut replacement = Vec::with_capacity(password.expose().len() + suffix.len()); + replacement.extend_from_slice(password.expose()); + replacement.extend_from_slice(suffix); + SecretBytes::new(replacement) +} + +fn validate_length(length: usize) -> Result { + let length = NonZeroUsize::new(length).ok_or(GenerateError::InvalidLength)?; + if length.get() > MAX_PASSWORD_LENGTH { + return Err(GenerateError::InvalidLength); + } + Ok(length) +} + +fn validate_character_set(character_set: &str) -> Result, GenerateError> { + let characters = character_set.chars().collect::>(); + if characters.is_empty() { + return Err(GenerateError::EmptyCharacterSet); + } + let mut unique = BTreeSet::new(); + if characters + .iter() + .any(|character| character.is_control() || !unique.insert(*character)) + { + return Err(GenerateError::InvalidCharacterSet); + } + Ok(characters) +} + +#[derive(Debug)] +pub enum GenerateError { + Repository(RepositoryError), + Crypto(CryptoError), + Write(WriteError), + InvalidLength, + EmptyCharacterSet, + InvalidCharacterSet, + IncompatibleFlags, + RandomnessUnavailable, +} + +impl fmt::Display for GenerateError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Repository(error) => error.fmt(formatter), + Self::Crypto(error) => error.fmt(formatter), + Self::Write(error) => error.fmt(formatter), + Self::InvalidLength => formatter.write_str("password length is invalid"), + Self::EmptyCharacterSet => formatter.write_str("password character set is empty"), + Self::InvalidCharacterSet => formatter.write_str("password character set is invalid"), + Self::IncompatibleFlags => { + formatter.write_str("--force and --in-place cannot be combined") + } + Self::RandomnessUnavailable => { + formatter.write_str("operating-system randomness is unavailable") + } + } + } +} + +impl Error for GenerateError {} + +impl From for GenerateError { + fn from(error: RepositoryError) -> Self { + Self::Repository(error) + } +} + +impl From for GenerateError { + fn from(error: WriteError) -> Self { + Self::Write(error) + } +} + +impl From for GenerateError { + fn from(error: CryptoError) -> Self { + Self::Crypto(error) + } +} diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 4185f5b..f22ae31 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -8,6 +8,7 @@ pub mod command; pub mod config; pub mod crypto; +pub mod generate; pub mod read; pub mod recipient; pub mod repository; diff --git a/crates/storage/src/write.rs b/crates/storage/src/write.rs index 2996f6e..c261ee0 100644 --- a/crates/storage/src/write.rs +++ b/crates/storage/src/write.rs @@ -248,6 +248,48 @@ impl<'a> VaultWriter<'a> { }) } + #[allow(clippy::too_many_arguments)] + pub fn store_generated( + &self, + path: &EntryPath, + contents: SecretBytes, + force: bool, + overwrite: OverwriteDecision, + signing: Option<&SigningPolicy>, + committer: &mut impl EntryCommitter, + ) -> Result { + let original = match self.repository.read_entry(path) { + Ok(original) => Some(original), + Err(RepositoryError::NotFound { .. }) => None, + Err(error) => return Err(error.into()), + }; + if original.is_some() && !force && overwrite == OverwriteDecision::Decline { + return Err(WriteError::Cancelled); + } + let recipients = RecipientPolicyManager::new(self.repository, self.keys) + .resolve_for_entry(path, signing)?; + let ciphertext = self.keys.encrypt(contents, recipients.recipients())?; + self.repository.write_entry(path, &ciphertext)?; + let change = EntryCommit { + path: path.clone(), + action: EntryAction::Insert, + message: format!("Add generated password for {path}."), + }; + if let Err(error) = committer.commit(&change) { + if let Err(rollback) = self.restore(path, original.as_ref()) { + return Err(WriteError::RollbackFailed { + operation: error, + rollback, + }); + } + return Err(WriteError::Commit(error)); + } + Ok(WriteOutcome { + path: path.clone(), + action: EntryAction::Insert, + }) + } + #[allow(clippy::too_many_arguments)] pub fn finish_edit( &self, diff --git a/crates/storage/tests/password_generation.rs b/crates/storage/tests/password_generation.rs new file mode 100644 index 0000000..3535042 --- /dev/null +++ b/crates/storage/tests/password_generation.rs @@ -0,0 +1,380 @@ +#![forbid(unsafe_code)] + +mod support; + +use std::{collections::BTreeMap, num::NonZeroUsize}; + +use ironstorage::{ + command::{GenerateRequest, GeneratedPresentation}, + crypto::{KeyInfo, KeyStore, SecretProvider, SecretProviderError}, + generate::{ + GenerateError, GeneratedChannel, GeneratorConfig, MAX_PASSWORD_LENGTH, PasswordGenerator, + }, + repository::{EntryPath, Repository, SecretBytes}, + write::{EntryCommit, EntryCommitError, EntryCommitter, OverwriteDecision, WriteError}, +}; +use rand_chacha::{ + ChaCha20Rng, + rand_core::{CryptoRng, Error as RandError, RngCore, SeedableRng}, +}; +use support::compatibility::{FixtureSet, TestResult}; + +struct FixtureSecrets(BTreeMap>); + +impl FixtureSecrets { + fn all(fixture: &FixtureSet) -> Self { + Self( + fixture + .generated + .keys + .iter() + .map(|key| { + ( + key.primary_fingerprint.clone(), + key.passphrase.as_bytes().to_vec(), + ) + }) + .collect(), + ) + } +} + +impl SecretProvider for FixtureSecrets { + fn secret_for(&mut self, key: &KeyInfo) -> Result { + self.0 + .get(key.fingerprint().as_str()) + .cloned() + .map(SecretBytes::new) + .ok_or(SecretProviderError::Unavailable) + } +} + +#[derive(Default)] +struct Committer { + changes: Vec, + fail: bool, +} + +struct FailingRng; + +impl RngCore for FailingRng { + fn next_u32(&mut self) -> u32 { + 0 + } + + fn next_u64(&mut self) -> u64 { + 0 + } + + fn fill_bytes(&mut self, _destination: &mut [u8]) { + panic!("fallible generator must use try_fill_bytes") + } + + fn try_fill_bytes(&mut self, _destination: &mut [u8]) -> Result<(), RandError> { + Err(RandError::new("simulated unavailable randomness")) + } +} + +impl CryptoRng for FailingRng {} + +impl EntryCommitter for Committer { + fn commit(&mut self, change: &EntryCommit) -> Result<(), EntryCommitError> { + self.changes.push(change.clone()); + if self.fail { + Err(EntryCommitError::new("simulated")) + } else { + Ok(()) + } + } +} + +#[test] +fn deterministic_generation_uses_only_requested_characters_and_length() -> TestResult { + let fixture = FixtureSet::load()?; + let store = fixture.materialize_store("basic")?; + let repository = Repository::open(store.path())?; + let keys = KeyStore::load(fixture.path("keys"))?; + let service = PasswordGenerator::new(&repository, &keys, GeneratorConfig::new(12, "abc123")?); + let mut rng = ChaCha20Rng::from_seed([0x81; 32]); + let mut provider = FixtureSecrets::all(&fixture); + let mut committer = Committer::default(); + + let outcome = service.generate_with_rng( + &request( + "generated/custom", + Some(64), + false, + false, + false, + GeneratedPresentation::Terminal, + ), + OverwriteDecision::Allow, + None, + &mut provider, + &mut committer, + &mut rng, + )?; + assert_eq!(outcome.password().expose().len(), 64); + assert!( + outcome + .password() + .expose() + .iter() + .all(|byte| b"abc123".contains(byte)) + ); + assert_eq!(outcome.channel(), GeneratedChannel::Terminal); + assert!(!format!("{outcome:?}").contains(std::str::from_utf8(outcome.password().expose())?)); + assert_eq!( + committer.changes[0].message(), + "Add generated password for generated/custom." + ); + Ok(()) +} + +#[test] +fn default_no_symbols_and_presentation_actions_are_typed() -> TestResult { + let fixture = FixtureSet::load()?; + let store = fixture.materialize_store("basic")?; + let repository = Repository::open(store.path())?; + let keys = KeyStore::load(fixture.path("keys"))?; + let service = PasswordGenerator::new(&repository, &keys, GeneratorConfig::pass_defaults()); + let mut provider = FixtureSecrets::all(&fixture); + let mut committer = Committer::default(); + let mut rng = ChaCha20Rng::from_seed([0x82; 32]); + + for (path, presentation, channel) in [ + ( + "generated/clip", + GeneratedPresentation::Clipboard, + GeneratedChannel::Clipboard, + ), + ( + "generated/qr", + GeneratedPresentation::QrCode, + GeneratedChannel::QrCode, + ), + ] { + let outcome = service.generate_with_rng( + &request(path, None, true, false, false, presentation), + OverwriteDecision::Allow, + None, + &mut provider, + &mut committer, + &mut rng, + )?; + assert_eq!(outcome.password().expose().len(), 25); + assert!( + outcome + .password() + .expose() + .iter() + .all(u8::is_ascii_alphanumeric) + ); + assert_eq!(outcome.channel(), channel); + } + Ok(()) +} + +#[test] +fn in_place_replaces_only_first_line_and_preserves_suffix_bytes() -> TestResult { + let fixture = FixtureSet::load()?; + let store = fixture.materialize_store("basic")?; + let repository = Repository::open(store.path())?; + let keys = KeyStore::load(fixture.path("keys"))?; + let service = PasswordGenerator::new(&repository, &keys, GeneratorConfig::new(8, "XYZ")?); + let mut provider = FixtureSecrets::all(&fixture); + let mut committer = Committer::default(); + let mut rng = ChaCha20Rng::from_seed([0x83; 32]); + let original = fixture.read("expected/basic/email/personal.txt")?; + let suffix = &original[original + .iter() + .position(|byte| *byte == b'\n') + .expect("newline")..]; + + let outcome = service.generate_with_rng( + &request( + "email/personal", + Some(8), + false, + false, + true, + GeneratedPresentation::Terminal, + ), + OverwriteDecision::Decline, + None, + &mut provider, + &mut committer, + &mut rng, + )?; + let plaintext = keys.decrypt( + &repository.read_entry(&EntryPath::parse("email/personal")?)?, + &mut provider, + )?; + assert_eq!(&plaintext.expose()[8..], suffix); + assert_eq!(&plaintext.expose()[..8], outcome.password().expose()); + Ok(()) +} + +#[test] +fn validates_lengths_character_sets_and_incompatible_flags() -> TestResult { + assert!(matches!( + GeneratorConfig::new(0, "abc"), + Err(GenerateError::InvalidLength) + )); + assert!(matches!( + GeneratorConfig::new(MAX_PASSWORD_LENGTH + 1, "abc"), + Err(GenerateError::InvalidLength) + )); + assert!(matches!( + GeneratorConfig::new(10, ""), + Err(GenerateError::EmptyCharacterSet) + )); + assert!(matches!( + GeneratorConfig::new(10, "aab"), + Err(GenerateError::InvalidCharacterSet) + )); + assert!(matches!( + GeneratorConfig::new(10, "a\nb"), + Err(GenerateError::InvalidCharacterSet) + )); + + let fixture = FixtureSet::load()?; + let store = fixture.materialize_store("basic")?; + let repository = Repository::open(store.path())?; + let keys = KeyStore::load(fixture.path("keys"))?; + let service = PasswordGenerator::new(&repository, &keys, GeneratorConfig::pass_defaults()); + let mut provider = FixtureSecrets::all(&fixture); + let mut rng = ChaCha20Rng::from_seed([0x84; 32]); + assert!(matches!( + service.generate_with_rng( + &request( + "entry", + Some(8), + false, + true, + true, + GeneratedPresentation::Terminal + ), + OverwriteDecision::Allow, + None, + &mut provider, + &mut Committer::default(), + &mut rng, + ), + Err(GenerateError::IncompatibleFlags) + )); + assert!(matches!( + service.generate_with_rng( + &request( + "randomness-failure", + Some(8), + false, + false, + false, + GeneratedPresentation::Terminal, + ), + OverwriteDecision::Allow, + None, + &mut provider, + &mut Committer::default(), + &mut FailingRng, + ), + Err(GenerateError::RandomnessUnavailable) + )); + assert!(!store.path().join("randomness-failure.gpg").exists()); + Ok(()) +} + +#[test] +fn decline_missing_in_place_and_commit_failure_leave_entries_unchanged() -> TestResult { + let fixture = FixtureSet::load()?; + let store = fixture.materialize_store("basic")?; + let repository = Repository::open(store.path())?; + let keys = KeyStore::load(fixture.path("keys"))?; + let service = PasswordGenerator::new(&repository, &keys, GeneratorConfig::pass_defaults()); + let path = EntryPath::parse("email/personal")?; + let original = repository.read_entry(&path)?; + let mut provider = FixtureSecrets::all(&fixture); + let mut rng = ChaCha20Rng::from_seed([0x85; 32]); + + assert!(matches!( + service.generate_with_rng( + &request( + "email/personal", + Some(8), + false, + false, + false, + GeneratedPresentation::Terminal + ), + OverwriteDecision::Decline, + None, + &mut provider, + &mut Committer::default(), + &mut rng, + ), + Err(GenerateError::Write(WriteError::Cancelled)) + )); + assert_eq!(repository.read_entry(&path)?, original); + assert!(matches!( + service.generate_with_rng( + &request( + "missing", + Some(8), + false, + false, + true, + GeneratedPresentation::Terminal + ), + OverwriteDecision::Allow, + None, + &mut provider, + &mut Committer::default(), + &mut rng, + ), + Err(GenerateError::Repository(_)) + )); + + let mut failing = Committer { + fail: true, + ..Committer::default() + }; + assert!(matches!( + service.generate_with_rng( + &request( + "email/personal", + Some(8), + false, + true, + false, + GeneratedPresentation::Terminal + ), + OverwriteDecision::Allow, + None, + &mut provider, + &mut failing, + &mut rng, + ), + Err(GenerateError::Write(WriteError::Commit(_))) + )); + assert_eq!(repository.read_entry(&path)?, original); + Ok(()) +} + +fn request( + entry: &str, + length: Option, + no_symbols: bool, + force: bool, + in_place: bool, + presentation: GeneratedPresentation, +) -> GenerateRequest { + GenerateRequest { + entry: entry.to_owned(), + length: length.map(|length| NonZeroUsize::new(length).expect("test length is nonzero")), + no_symbols, + force, + in_place, + presentation, + } +} diff --git a/docs/password-generation.md b/docs/password-generation.md new file mode 100644 index 0000000..a63eec0 --- /dev/null +++ b/docs/password-generation.md @@ -0,0 +1,25 @@ +# Secure password generation + +`PasswordGenerator` owns `pass generate` semantics in `crates/storage`. +Production calls use `OsRng`; tests inject a deterministic cryptographic RNG. +Random bytes are converted to character indexes with fallible rejection +sampling, so arbitrary set sizes have no modulo bias and operating-system RNG +failure is reported before repository mutation. + +The default is 25 characters from printable ASCII punctuation and +alphanumerics. `--no-symbols` selects ASCII letters and digits. A +`GeneratorConfig` may supply another Unicode character set and default length. +Lengths must be 1 through 4096. Sets must be nonempty and contain no duplicate +or control characters; rejecting duplicates prevents accidental weighting. + +Normal generation uses the same overwrite decision and `--force` rules as +insert. `--in-place` requires an existing entry, decrypts it, replaces only the +bytes before its first newline, and preserves that newline and every following +byte exactly. The generated password is returned separately as redacted, +zeroizing data for typed terminal, clipboard, or QR presentation. + +The completed entry is encrypted for the nearest recipient policy and written +atomically. Embedded Git receives `Add generated password for ...` only after a +successful write. Decline, missing in-place targets, validation, randomness, +encryption, and commit failures leave the previous repository state intact; +commit failure uses the write-domain rollback path.