From 6eef5e9ffc6e4d1e25e33b82afdabe741f00afba Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Mon, 3 Jun 2024 17:12:47 +0200 Subject: [PATCH] Change sq key password to support the cert store and key store. - Change `sq key password` to support the cert store and key store. - See #205. --- Cargo.lock | 20 ++-- Cargo.toml | 2 +- src/cli/key.rs | 18 ++- src/commands/key/password.rs | 217 +++++++++++++++++++++++++---------- tests/sq-key-password.rs | 169 +++++++++++++++++---------- 5 files changed, 293 insertions(+), 133 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c2e6f151..21190f45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3178,9 +3178,9 @@ dependencies = [ [[package]] name = "sequoia-gpg-agent" -version = "0.4.0" +version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c929d572dee98c48d286cef43e2ade4201962f3454c015f52bf43b5a8e40d42" +checksum = "9b1df6f0a2de8dfdef7a4ea49d096237eb1a3081b3e235eb61255d654257a6de" dependencies = [ "anyhow", "chrono", @@ -3225,9 +3225,9 @@ dependencies = [ [[package]] name = "sequoia-keystore" -version = "0.4.1" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa77ac702f6be1489580eb092aa5acae36050db04fa5ae445238a84591e1ad7a" +checksum = "8556f1dd77d72fbe9c2dfcd2985eb9eaf87b7e6c8e0181a27be23e6c8778886f" dependencies = [ "anyhow", "capnp", @@ -3250,9 +3250,9 @@ dependencies = [ [[package]] name = "sequoia-keystore-backend" -version = "0.4.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ab69a90e3455e15aa0ff47d676e84bf1a085716691b72156badc50d0a01dab1" +checksum = "5e7a7a4b148f8ea2083f0fec111208ea43a1c1529fe29a2add58251aa0554ee0" dependencies = [ "anyhow", "async-trait", @@ -3268,9 +3268,9 @@ dependencies = [ [[package]] name = "sequoia-keystore-gpg-agent" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "454e8d580617e07d595b8df718d7fa3e26cdc58f35d1ad89f9fecc78ef0d55a7" +checksum = "0bc3da475399420b544a2749f75318503d12ef1a355ab6d63bdd48d5c242192f" dependencies = [ "anyhow", "async-trait", @@ -3287,9 +3287,9 @@ dependencies = [ [[package]] name = "sequoia-keystore-softkeys" -version = "0.4.1" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f9707371cae085b6e1cac9e17bf94a19efcdc04da4dba5cbda1cb8f8c0a655a" +checksum = "5bb53ee667b0e10a44bff1b2dbb7ee76ba5c765bf5f3456b1916ce47d44511f3" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 77fdceb0..d5dc5264 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ indicatif = "0.17" itertools = ">=0.10, <0.13" once_cell = "1.17" sequoia-cert-store = "0.5.3" -sequoia-keystore = { version = "0.4" } +sequoia-keystore = { version = "0.5" } sequoia-wot = { version = "0.11", default-features = false } tempfile = "3.1" thiserror = "1" diff --git a/src/cli/key.rs b/src/cli/key.rs index 58c26069..1804426c 100644 --- a/src/cli/key.rs +++ b/src/cli/key.rs @@ -421,22 +421,30 @@ $ sq key password --clear < juliet.encrypted_key.pgp \\ > juliet.decrypted_key.pgp ", )] +#[clap(group(ArgGroup::new("cert_input").args(&["cert_file", "cert"]).required(true)))] pub struct PasswordCommand { #[clap( long, - default_value_t = FileOrStdin::default(), - help = FileOrStdin::HELP_OPTIONAL, + help = "Change the password of the specified certificate", value_name = FileOrStdin::VALUE_NAME, )] - pub cert_file: FileOrStdin, + pub cert: Option, + #[clap( + long, + value_name = "CERT_FILE", + help = "Change the password of the specified certificate", + )] + pub cert_file: Option, + #[clap( - default_value_t = FileOrStdout::default(), help = FileOrStdout::HELP_OPTIONAL, long, short, value_name = FileOrStdout::VALUE_NAME, + conflicts_with = "cert", )] - pub output: FileOrStdout, + pub output: Option, + /// File containing password to decrypt key /// /// Note that the entire key file will be used as the password, including diff --git a/src/commands/key/password.rs b/src/commands/key/password.rs index 12346bc7..5a0f5a07 100644 --- a/src/commands/key/password.rs +++ b/src/commands/key/password.rs @@ -1,81 +1,182 @@ -use openpgp::parse::Parse; +use anyhow::Context; + +use sequoia_openpgp as openpgp; +use openpgp::crypto::Password; use openpgp::serialize::Serialize; -use openpgp::Cert; use openpgp::Packet; use openpgp::Result; -use sequoia_openpgp as openpgp; + +use sequoia_keystore as keystore; +use keystore::Protection; use crate::common; use crate::Sq; use crate::cli; use crate::decrypt_key; +use crate::cli::types::FileOrStdout; +use crate::common::password; pub fn password( sq: Sq, command: cli::key::PasswordCommand, ) -> Result<()> { - let input = command.cert_file.open()?; - let key = Cert::from_buffered_reader(input)?; + let mut new_password_ = None; + // Some(password) => new password + // None => clear password + let mut get_new_password = || -> Result> { + if new_password_.is_none() { + new_password_ = if command.clear { + Some(None) + } else if let Some(path) = command.new_password_file.as_ref() { + Some(Some(std::fs::read(path)?.into())) + } else { + Some(common::password::prompt_for_new("key")?) + }; + } - if !key.is_tsk() { - return Err(anyhow::anyhow!("Certificate has no secrets")); - } - - // First, decrypt all secrets. - let passwords = &mut Vec::new(); - for password in command.old_password_file { - passwords.push(std::fs::read(password)?.into()); - }; - let mut decrypted: Vec = vec![decrypt_key( - key.primary_key().key().clone().parts_into_secret()?, - passwords, - )? - .into()]; - for ka in key.keys().subkeys().secret() { - decrypted.push( - decrypt_key(ka.key().clone().parts_into_secret()?, passwords)? - .into(), - ); - } - let mut key = key.insert_packets(decrypted)?; - assert_eq!( - key.keys().secret().count(), - key.keys().unencrypted_secret().count() - ); - - let new_password = if command.clear { - None - } else if let Some(path) = command.new_password_file { - Some(std::fs::read(path)?.into()) - } else { - common::password::prompt_for_new("key")? + Ok(new_password_.clone().unwrap()) }; - if let Some(new) = new_password { - let mut encrypted: Vec = vec![key - .primary_key() - .key() - .clone() - .parts_into_secret()? - .encrypt_secret(&new)? - .into()]; - for ka in key.keys().subkeys().unencrypted_secret() { - encrypted.push( - ka.key() - .clone() - .parts_into_secret()? - .encrypt_secret(&new)? + if let Some(file) = command.cert_file { + assert!(command.cert.is_none()); + + let key = sq.lookup_one(file, None, true)?; + + // First, decrypt all secrets. + let passwords = &mut Vec::new(); + for password in command.old_password_file { + passwords.push(std::fs::read(password)?.into()); + }; + let mut decrypted: Vec = vec![ + decrypt_key( + key.primary_key().key().clone().parts_into_secret()?, + passwords, + )?.into(), + ]; + for ka in key.keys().subkeys().secret() { + decrypted.push( + decrypt_key(ka.key().clone().parts_into_secret()?, passwords)? .into(), ); } - key = key.insert_packets(encrypted)?; + let mut key = key.insert_packets(decrypted)?; + assert_eq!( + key.keys().secret().count(), + key.keys().unencrypted_secret().count() + ); + + if let Some(new) = get_new_password()? { + let mut encrypted: Vec = vec![ + key + .primary_key() + .key() + .clone() + .parts_into_secret()? + .encrypt_secret(&new)? + .into() + ]; + for ka in key.keys().subkeys().unencrypted_secret() { + encrypted.push( + ka.key() + .clone() + .parts_into_secret()? + .encrypt_secret(&new)? + .into(), + ); + } + key = key.insert_packets(encrypted)?; + } + + let output = command.output.unwrap_or_else(|| FileOrStdout::new(None)); + let mut output = output.for_secrets().create_safe(sq.force)?; + if command.binary { + key.as_tsk().serialize(&mut output)?; + } else { + key.as_tsk().armored().serialize(&mut output)?; + } + } else if let Some(kh) = command.cert { + assert!(command.output.is_none()); + + for password in command.old_password_file { + sq.cache_password(std::fs::read(password)?.into()); + } + + let cert = sq.lookup_one(kh, None, true)?; + let vc = cert.with_policy(sq.policy, sq.time)?; + + let uid = sq.best_userid(&cert, true); + + let ks = sq.key_store_or_else()?; + let mut ks = ks.lock().unwrap(); + + for ka in vc.keys() { + let keys = ks.find_key(ka.key_handle()) + .with_context(|| { + format!("Looking up {}", ka.fingerprint()) + })?; + + // XXX: What should we do if the key is present multiple + // times? + let mut key = keys.into_iter().next().expect("have at least one"); + + let provide_password + = if let Protection::Password(hint) = key.locked()? + { + let mut unlocked = false; + for p in sq.cached_passwords() { + if key.unlock(p).is_ok() { + unlocked = true; + break; + } + } + + if ! unlocked { + if let Some(hint) = hint { + eprintln!("{}", hint); + } + + loop { + let p = password::prompt_to_unlock(&format!( + "Please enter the password to decrypt \ + the key {}/{}, {}", + cert.keyid(), ka.keyid(), uid))?; + + match key.unlock(p.clone()) { + Ok(()) => { + sq.cache_password(p.clone()); + break; + } + Err(err) => { + eprintln!("Failed to unlock key: {}", err); + } + } + } + } + true + } else { + key.password_source()?.is_inline() + }; + + let password = if provide_password { + if let Some(password) = get_new_password()? { + Some(password) + } else { + // change_password interprets None as prompt for + // password, and "" as clear password. + Some("".into()) + } + } else { + None + }; + + key.change_password(password.as_ref()) + .with_context(|| { + format!("Changing {}'s password", key.fingerprint()) + })?; + } + } else { + panic!("clap ensures --cert-file or --cert"); } - let mut output = command.output.for_secrets().create_safe(sq.force)?; - if command.binary { - key.as_tsk().serialize(&mut output)?; - } else { - key.as_tsk().armored().serialize(&mut output)?; - } Ok(()) } diff --git a/tests/sq-key-password.rs b/tests/sq-key-password.rs index d36fb475..4f8c2e09 100644 --- a/tests/sq-key-password.rs +++ b/tests/sq-key-password.rs @@ -12,75 +12,126 @@ fn sq_key_password() -> Result<()> { let (tmpdir, cert_path, time) = sq_key_generate(None)?; let cert_path = cert_path.display().to_string(); let cert = Cert::from_file(&cert_path)?; + let cert_fpr = cert.fingerprint().to_string(); - let mut sq = Sq::at(time.into()); + for keystore in [false, true] { + eprintln!("Keystore: {}", keystore); - let orig_password = sq.base().join("orig_password.txt"); - std::fs::write(&orig_password, "t00 ez").unwrap(); - let new_password = sq.base().join("new_password.txt"); - std::fs::write(&new_password, "crazy passw0rd").unwrap(); + let mut sq = Sq::at(time.into()); - let msg_txt = sq.base().join("msg.txt"); - std::fs::write(&msg_txt, "hello world").unwrap(); + let orig_password = sq.base().join("orig_password.txt"); + std::fs::write(&orig_password, "t00 ez").unwrap(); + let new_password = sq.base().join("new_password.txt"); + std::fs::write(&new_password, "crazy passw0rd").unwrap(); - let msg_sig = sq.base().join("msg.sig"); + let msg_txt = sq.base().join("msg.txt"); + std::fs::write(&msg_txt, "hello world").unwrap(); - // Two days go by. - sq.tick(2 * 24 * 60 * 60); + let msg_sig = sq.base().join("msg.sig"); - // Sign a message. - let mut cmd = sq.command(); - cmd.args([ - "sign", &msg_txt.to_string_lossy(), - "--signer-file", &cert_path, - "--password-file", &orig_password.to_string_lossy(), - "--output", &msg_sig.to_string_lossy(), - ]); - sq.run(cmd, true); + // Two days go by. + sq.tick(2 * 24 * 60 * 60); - // Change the key's password. - let updated_path = &tmpdir.path().join("updated.pgp"); - let mut cmd = sq.command(); - cmd.args([ - "key", "password", - "--cert-file", &cert_path, - "--new-password-file", &new_password.to_string_lossy(), - "--output", &updated_path.to_string_lossy(), - ]); - sq.run(cmd, true); + if keystore { + sq.key_import(&cert_path); + } - // Sign a message. - let mut cmd = sq.command(); - cmd.args([ - "--force", - "sign", &msg_txt.to_string_lossy(), - "--signer-file", &updated_path.to_string_lossy(), - "--password-file", &new_password.to_string_lossy(), - "--output", &msg_sig.to_string_lossy(), - ]); - sq.run(cmd, true); + // Sign a message. + let mut cmd = sq.command(); + cmd.args([ + "sign", &msg_txt.to_string_lossy(), + "--password-file", &orig_password.to_string_lossy(), + "--output", &msg_sig.to_string_lossy(), + ]); + if keystore { + cmd.args([ + "--signer-key", &cert_fpr, + ]); + } else { + cmd.args([ + "--signer-file", &cert_path, + ]); + } + sq.run(cmd, true); - // Clear the key's password. - let updated2_path = &tmpdir.path().join("updated2.pgp"); - let mut cmd = sq.command(); - cmd.args([ - "key", "password", - "--cert-file", &updated_path.to_string_lossy(), - "--old-password-file", &new_password.to_string_lossy(), - "--clear", - "--output", &updated2_path.to_string_lossy(), - ]); - sq.run(cmd, true); + // Change the key's password. + eprintln!("Change the key's password."); + let updated_path = &tmpdir.path().join("updated.pgp"); + let mut cmd = sq.command(); + cmd.args([ + "key", "password", + "--new-password-file", &new_password.to_string_lossy(), + ]); + if keystore { + cmd.args([ + "--cert", &cert_fpr, + ]); + } else { + cmd.args([ + "--cert-file", &cert_path, + "--output", &updated_path.to_string_lossy(), + ]); + } + sq.run(cmd, true); - // Sign a message. - let mut cmd = sq.command(); - cmd.args([ - "--force", - "sign", &msg_txt.to_string_lossy(), - "--signer-file", &updated2_path.to_string_lossy(), - "--output", &msg_sig.to_string_lossy(), - ]); - sq.run(cmd, true); + // Sign a message. + let mut cmd = sq.command(); + cmd.args([ + "--force", + "sign", &msg_txt.to_string_lossy(), + "--password-file", &new_password.to_string_lossy(), + "--output", &msg_sig.to_string_lossy(), + ]); + if keystore { + cmd.args([ + "--signer-key", &cert_fpr, + ]); + } else { + cmd.args([ + "--signer-file", &updated_path.to_string_lossy(), + ]); + } + sq.run(cmd, true); + + // Clear the key's password. + eprintln!("Clear the key's password."); + let updated2_path = &tmpdir.path().join("updated2.pgp"); + let mut cmd = sq.command(); + cmd.args([ + "key", "password", + "--old-password-file", &new_password.to_string_lossy(), + "--clear", + ]); + if keystore { + cmd.args([ + "--cert", &cert_fpr, + ]); + } else { + cmd.args([ + "--cert-file", &updated_path.to_string_lossy(), + "--output", &updated2_path.to_string_lossy(), + ]); + } + sq.run(cmd, true); + + // Sign a message. + let mut cmd = sq.command(); + cmd.args([ + "--force", + "sign", &msg_txt.to_string_lossy(), + "--output", &msg_sig.to_string_lossy(), + ]); + if keystore { + cmd.args([ + "--signer-key", &cert_fpr, + ]); + } else { + cmd.args([ + "--signer-file", &updated2_path.to_string_lossy(), + ]); + } + sq.run(cmd, true); + } tmpdir.close()?;