From a8a59d496b87aefdc9e489893874223a132a27b7 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Mon, 17 Aug 2026 17:00:31 +0200 Subject: [PATCH] Allow existing GPG keys with cloned stores --- apple/Sources/App/IronStorageApp.swift | 27 +-- crates/storage/src/mobile_key_transfer.rs | 59 ++++++- crates/storage/src/mobile_onboarding.rs | 206 +++++++++++++++++++--- 3 files changed, 252 insertions(+), 40 deletions(-) diff --git a/apple/Sources/App/IronStorageApp.swift b/apple/Sources/App/IronStorageApp.swift index 4008637..b72d221 100644 --- a/apple/Sources/App/IronStorageApp.swift +++ b/apple/Sources/App/IronStorageApp.swift @@ -1927,7 +1927,7 @@ private final class KeyImportScannerViewController: UIViewController, : "Import this public key?" let alert = UIAlertController( title: localSetup - ? "Create Local Store with This Key?" + ? "Use This GPG Key?" : (privateKey ? "Import Private GPG Key?" : "Import Public GPG Key?"), message: "\(warning)\n\n\(key.title)\n\(key.detail)", preferredStyle: .alert @@ -1952,6 +1952,15 @@ private final class KeyImportScannerViewController: UIViewController, ) }) } + if localSetup && privateKey { + alert.addAction(UIAlertAction(title: "Use with Existing Store", style: .default) { + [weak self, weak alert] _ in + self?.finishImport( + passphrase: alert?.textFields?.first?.text, + makeDefault: false + ) + }) + } if privateKey { alert.addAction(UIAlertAction( title: localSetup ? "Create Local Store" : "Import as Default", @@ -6290,7 +6299,7 @@ private final class SetupChoiceViewController: UITableViewController { case (0, 1): content.image = UIImage(systemName: "qrcode.viewfinder") content.text = "Import Local GPG Key" - content.secondaryText = "Scan a private IronStorage key transfer" + content.secondaryText = "Use a private key with a new or existing store" default: content.image = UIImage(systemName: "network") content.text = "Connect Existing Store" @@ -6796,15 +6805,11 @@ private final class OnboardingViewController: UITableViewController { case let .success(.completed(outcome)): tokenField.text = nil let alert = UIAlertController(title: outcome.title, message: outcome.detail, preferredStyle: .alert) - alert.addAction(UIAlertAction(title: "Done", style: .default) { [weak self] _ in - if self?.attachLocalStore == true { - NotificationCenter.default.post( - name: .ironStorageConfigurationDidChange, - object: nil - ) - } else { - self?.navigationController?.popViewController(animated: true) - } + alert.addAction(UIAlertAction(title: "Done", style: .default) { _ in + NotificationCenter.default.post( + name: .ironStorageConfigurationDidChange, + object: nil + ) }) present(alert, animated: true) case let .failure(failure): diff --git a/crates/storage/src/mobile_key_transfer.rs b/crates/storage/src/mobile_key_transfer.rs index 7be07c9..594d8cb 100644 --- a/crates/storage/src/mobile_key_transfer.rs +++ b/crates/storage/src/mobile_key_transfer.rs @@ -145,7 +145,7 @@ pub struct MobileKeyTransferService { impl MobileKeyTransferService { pub fn load() -> Result { let config = Config::load(None)?; - let keys = KeyStore::load(config.key_material())?; + let keys = load_keys_or_empty(config.key_material())?; Ok(Self { config, keys }) } @@ -316,16 +316,24 @@ impl MobileKeyTransferImport { } if matches!(self.destination, MobileKeyTransferDestination::LocalSetup) { - if key.kind != MobileKeyTransferKind::Private || !make_default { + if key.kind != MobileKeyTransferKind::Private { return Err(MobileKeyTransferError::PublicDefault); } - let outcome = crate::mobile_onboarding::MobileOnboardingOperation::default() - .setup_local_imported( + let operation = crate::mobile_onboarding::MobileOnboardingOperation::default(); + let outcome = if make_default { + operation.setup_local_imported( SecretBytes::new(armor.expose().to_vec()), passphrase, overwrite, ) - .map_err(MobileKeyTransferError::LocalSetup)?; + } else { + operation.stage_local_imported( + SecretBytes::new(armor.expose().to_vec()), + passphrase, + overwrite, + ) + } + .map_err(MobileKeyTransferError::LocalSetup)?; return Ok(MobileKeyTransferOutcome { title: outcome.title().to_owned(), detail: outcome.detail().to_owned(), @@ -334,7 +342,7 @@ impl MobileKeyTransferImport { let MobileKeyTransferDestination::Configured(config) = &self.destination else { unreachable!("local setup returned above") }; - let existing = KeyStore::load(config.key_material())?; + let existing = load_keys_or_empty(config.key_material())?; if let Some(found) = existing .infos() .find(|info| info.fingerprint().as_str() == key.fingerprint) @@ -389,6 +397,13 @@ impl MobileKeyTransferImport { } } +fn load_keys_or_empty(path: &Path) -> Result { + match KeyStore::load(path) { + Err(CryptoError::NoKeyMaterial) => Ok(KeyStore::new()), + result => result, + } +} + #[derive(Clone, Debug, Eq, PartialEq)] pub enum MobileKeyTransferError { Configuration, @@ -640,7 +655,7 @@ fn set_private_permissions(_temporary: &TempFile<'_>) -> Result<(), MobileKeyTra mod tests { use super::{ MobileKeyTransferError, MobileKeyTransferImport, MobileKeyTransferKind, - MobileKeyTransferService, encode_multipart, + MobileKeyTransferService, encode_multipart, load_keys_or_empty, }; use crate::{config::Config, crypto::KeyStore, repository::SecretBytes}; use tempfile::tempdir; @@ -800,6 +815,36 @@ mod tests { Ok(()) } + #[test] + fn cloned_store_with_empty_key_directory_can_import_its_private_key() + -> Result<(), Box> { + let fixture = configured_key("placeholder", b"")?; + std::fs::remove_file(fixture.key_path.join("placeholder"))?; + let service = MobileKeyTransferService { + keys: load_keys_or_empty(&fixture.key_path)?, + config: fixture.config.clone(), + }; + assert!(service.keys().is_empty()); + + let mut importer = service.importer(); + importer.add_frame(SecretBytes::new(ALICE_SECRET.to_vec()))?; + importer.import( + Some(SecretBytes::new(b"fixture-alice-passphrase".to_vec())), + true, + false, + )?; + + let keys = KeyStore::load(&fixture.key_path)?; + assert!(keys.resolve(ALICE_FINGERPRINT).is_ok()); + assert_eq!( + Config::load(Some(fixture.config.source()))? + .default_key() + .as_str(), + ALICE_FINGERPRINT + ); + Ok(()) + } + struct Fixture { _root: tempfile::TempDir, config: Config, diff --git a/crates/storage/src/mobile_onboarding.rs b/crates/storage/src/mobile_onboarding.rs index 2956ad3..8989c05 100644 --- a/crates/storage/src/mobile_onboarding.rs +++ b/crates/storage/src/mobile_onboarding.rs @@ -179,7 +179,7 @@ impl MobileOnboardingOperation { .map_err(MobileOnboardingError::from_git)?; fs::create_dir_all(&paths.keys).map_err(|_| MobileOnboardingError::configuration())?; set_private_directory(&paths.keys).map_err(|_| MobileOnboardingError::configuration())?; - let default_key = default_key(&repository)?; + let default_key = default_key(&repository, &paths.keys)?; store_application_token(request)?; Config::create_mobile_clone( paths.config, @@ -262,21 +262,23 @@ impl MobileOnboardingOperation { passphrase: Option, replace_existing_key: bool, ) -> Result { - validate_transfer_armor(armor.expose()) - .map_err(|_| MobileOnboardingError::invalid_gpg_key())?; - let mut keys = KeyStore::new(); - let infos = keys - .import(armor.expose()) - .map_err(MobileOnboardingError::from_crypto)?; - let key = infos - .first() - .filter(|_| infos.len() == 1) - .ok_or_else(MobileOnboardingError::invalid_gpg_key)?; - validate_local_key(&keys, key, passphrase.as_ref())?; - self.setup_local_key( + let fingerprint = validate_imported_local_key(&armor, passphrase.as_ref())?; + self.setup_local_key(armor, passphrase, &fingerprint, replace_existing_key) + } + + pub fn stage_local_imported( + &self, + armor: SecretBytes, + passphrase: Option, + replace_existing_key: bool, + ) -> Result { + let fingerprint = validate_imported_local_key(&armor, passphrase.as_ref())?; + let paths = MobileOnboardingPaths::system()?; + self.stage_local_key_at( + paths, armor, - passphrase, - key.fingerprint().as_str(), + passphrase.as_ref(), + &fingerprint, replace_existing_key, ) } @@ -368,6 +370,50 @@ impl MobileOnboardingOperation { .to_owned(), }) } + + fn stage_local_key_at( + &self, + paths: MobileOnboardingPaths, + armor: SecretBytes, + passphrase: Option<&SecretBytes>, + fingerprint: &str, + replace_existing_key: bool, + ) -> Result { + if Config::load(Some(&paths.config)).is_ok() || paths.config.exists() { + return Err(MobileOnboardingError::already_configured()); + } + paths.prepare_root()?; + require_empty_directory(&paths.keys, replace_existing_key)?; + + let staging = paths.staging_path(); + fs::create_dir(&staging).map_err(|_| MobileOnboardingError::configuration())?; + set_private_directory(&staging).map_err(|_| MobileOnboardingError::configuration())?; + let result = (|| { + let staged_keys = staging.join("keys"); + fs::create_dir(&staged_keys).map_err(|_| MobileOnboardingError::configuration())?; + set_private_directory(&staged_keys) + .map_err(|_| MobileOnboardingError::configuration())?; + persist_armored_key(&staged_keys, fingerprint, true, armor.expose(), false) + .map_err(|_| MobileOnboardingError::configuration())?; + let keys = KeyStore::load(&staged_keys).map_err(MobileOnboardingError::from_crypto)?; + let key = keys + .infos() + .find(|key| key.fingerprint().as_str() == fingerprint) + .ok_or_else(MobileOnboardingError::invalid_gpg_key)?; + validate_local_key(&keys, &key, passphrase)?; + paths.install_staged_keys(&staging) + })(); + if staging.exists() { + let _ = fs::remove_dir_all(&staging); + } + result?; + Ok(MobileOnboardingOutcome { + title: "GPG Key Ready".to_owned(), + detail: + "The private key is ready. Connect Existing Store to clone its password repository." + .to_owned(), + }) + } } pub struct MobileOnboardingRequest { @@ -528,7 +574,7 @@ impl MobileOnboardingError { Self { kind: MobileOnboardingErrorKind::ExistingKey, title: "GPG Key Already Exists", - detail: "Creating this local store would replace existing local key material. Confirm replacement to continue." + detail: "This operation would replace existing local key material. Confirm replacement to continue." .to_owned(), } } @@ -738,6 +784,22 @@ impl MobileOnboardingPaths { Ok(()) } + fn install_staged_keys(&self, staging: &Path) -> Result<(), MobileOnboardingError> { + let previous = staging.join("previous-keys"); + let had_keys = self.keys.exists(); + if had_keys { + fs::rename(&self.keys, &previous) + .map_err(|_| MobileOnboardingError::configuration())?; + } + if fs::rename(staging.join("keys"), &self.keys).is_err() { + if had_keys { + let _ = fs::rename(previous, &self.keys); + } + return Err(MobileOnboardingError::configuration()); + } + Ok(()) + } + fn restore_staged(&self, staging: &Path) -> std::io::Result<()> { let failed_vault = staging.join("failed-vault"); let failed_keys = staging.join("failed-keys"); @@ -800,6 +862,24 @@ fn validate_local_key( .map_err(|_| MobileOnboardingError::invalid_gpg_key()) } +fn validate_imported_local_key( + armor: &SecretBytes, + passphrase: Option<&SecretBytes>, +) -> Result { + validate_transfer_armor(armor.expose()) + .map_err(|_| MobileOnboardingError::invalid_gpg_key())?; + let mut keys = KeyStore::new(); + let infos = keys + .import(armor.expose()) + .map_err(MobileOnboardingError::from_crypto)?; + let key = infos + .first() + .filter(|_| infos.len() == 1) + .ok_or_else(MobileOnboardingError::invalid_gpg_key)?; + validate_local_key(&keys, key, passphrase)?; + Ok(key.fingerprint().as_str().to_owned()) +} + struct NoSetupSecrets; impl SecretProvider for NoSetupSecrets { @@ -868,7 +948,10 @@ fn hex_prefix(bytes: &[u8], length: usize) -> String { .collect() } -fn default_key(repository: &Repository) -> Result { +fn default_key( + repository: &Repository, + key_material: &Path, +) -> Result { let root = DirectoryPath::parse("").map_err(|_| MobileOnboardingError::inaccessible_repository())?; let contents = repository @@ -877,11 +960,27 @@ fn default_key(repository: &Repository) -> Result .ok_or_else(MobileOnboardingError::inaccessible_repository)?; let text = std::str::from_utf8(&contents) .map_err(|_| MobileOnboardingError::inaccessible_repository())?; - text.lines() + let identities = text + .lines() .map(|line| line.split('#').next().unwrap_or_default().trim()) - .find(|identity| !identity.is_empty()) - .filter(|identity| identity.len() <= 512 && !identity.chars().any(char::is_control)) - .map(str::to_owned) + .filter(|identity| { + !identity.is_empty() && identity.len() <= 512 && !identity.chars().any(char::is_control) + }) + .collect::>(); + if let Ok(keys) = KeyStore::load(key_material) { + for identity in &identities { + if let Ok(handle) = keys.resolve(identity) + && keys + .infos() + .any(|key| key.fingerprint() == handle.fingerprint() && key.has_secret()) + { + return Ok(handle.fingerprint().to_string()); + } + } + } + identities + .first() + .map(|identity| (*identity).to_owned()) .ok_or_else(MobileOnboardingError::inaccessible_repository) } @@ -957,7 +1056,7 @@ mod tests { use super::{ MobileOnboardingErrorKind, MobileOnboardingOperation, MobileOnboardingPaths, - MobileOnboardingPhase, MobileOnboardingRequest, + MobileOnboardingPhase, MobileOnboardingRequest, default_key, }; #[test] @@ -1058,6 +1157,69 @@ mod tests { ); } + #[test] + fn imported_key_can_be_staged_before_cloning_without_creating_a_repository() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let root = temporary.path().join("ironstorage"); + let paths = MobileOnboardingPaths { + config: root.join("config.toml"), + vault: root.join("vault"), + keys: root.join("keys"), + root, + }; + let passphrase = SecretBytes::new(b"existing key passphrase".to_vec()); + let generated = + KeyStore::generate("Existing Store Key ", &passphrase) + .expect("generate key"); + let fingerprint = generated.info().fingerprint().as_str().to_owned(); + + MobileOnboardingOperation::default() + .stage_local_key_at( + paths, + generated.into_armor(), + Some(&passphrase), + &fingerprint, + false, + ) + .expect("stage imported key"); + + assert!(!temporary.path().join("ironstorage/config.toml").exists()); + assert!(!temporary.path().join("ironstorage/vault").exists()); + let keys = + KeyStore::load(temporary.path().join("ironstorage/keys")).expect("staged key material"); + assert!(keys.resolve(&fingerprint).is_ok()); + } + + #[test] + fn existing_store_selects_the_staged_private_recipient_as_default() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let vault = temporary.path().join("vault"); + let keys_path = temporary.path().join("keys"); + fs::create_dir(&vault).expect("vault"); + fs::create_dir(&keys_path).expect("keys"); + let passphrase = SecretBytes::new(b"existing key passphrase".to_vec()); + let generated = + KeyStore::generate("Existing Store Key ", &passphrase) + .expect("generate key"); + let fingerprint = generated.info().fingerprint().as_str().to_owned(); + fs::write( + vault.join(".gpg-id"), + format!("UNAVAILABLE-RECIPIENT\n{fingerprint}\n"), + ) + .expect("recipient policy"); + fs::write( + keys_path.join("imported-secret.asc"), + generated.into_armor().expose(), + ) + .expect("key material"); + let repository = crate::repository::Repository::open(&vault).expect("repository"); + + assert_eq!( + default_key(&repository, &keys_path).expect("default key"), + fingerprint + ); + } + #[test] fn local_setup_requires_confirmation_before_replacing_key_material() { let temporary = tempfile::tempdir().expect("temporary directory");