From d5c4c5032681f9876fd3a62ca9165c2ad5a2d945 Mon Sep 17 00:00:00 2001
From: "Neal H. Walfield" <neal@sequoia-pgp.org>
Date: Fri, 22 Nov 2024 16:55:23 +0100
Subject: [PATCH] Make sq key password change the password of weakly bound
 keys.

  - Change `sq key password` to also change the password of keys that
    are weakly bound.  Users are likely to be more surprised when a
    password is not changed.
---
 src/commands/key/password.rs         | 13 +++++++++-
 tests/integration/sq_key_password.rs | 36 ++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/src/commands/key/password.rs b/src/commands/key/password.rs
index f88b29ed..2b57a29a 100644
--- a/src/commands/key/password.rs
+++ b/src/commands/key/password.rs
@@ -8,6 +8,7 @@ use openpgp::Cert;
 use crate::Result;
 use crate::Sq;
 use crate::cli;
+use crate::common::NULL_POLICY;
 use crate::common::key::get_keys;
 use crate::common::key::password;
 
@@ -17,13 +18,23 @@ pub fn dispatch(sq: Sq, command: cli::key::password::Command)
     let (cert, cert_source)
         = sq.resolve_cert(&command.cert, sequoia_wot::FULLY_TRUSTED)?;
 
-    let vc = Cert::with_policy(&cert, sq.policy, sq.time)
+    // We require the certificate be valid under the standard policy.
+    Cert::with_policy(&cert, sq.policy, sq.time)
         .with_context(|| {
             format!("The certificate {} is not valid under the \
                      current policy.",
                     cert.fingerprint())
         })?;
 
+    // But we change the password for all keys with plausible
+    // bindings.
+    let vc = Cert::with_policy(&cert, NULL_POLICY, sq.time)
+        .with_context(|| {
+            format!("The certificate {} is not valid under the \
+                     null policy.",
+                    cert.fingerprint())
+        })?;
+
     let kas = vc.keys().collect::<Vec<_>>();
     let kas = kas.iter().collect::<Vec<_>>();
 
diff --git a/tests/integration/sq_key_password.rs b/tests/integration/sq_key_password.rs
index d9df358c..4cc54c4a 100644
--- a/tests/integration/sq_key_password.rs
+++ b/tests/integration/sq_key_password.rs
@@ -205,6 +205,42 @@ fn hard_revoked_subkey() {
     }
 }
 
+#[test]
+fn sha1_subkey() {
+    // Make sure we can change the password of keys that are bound
+    // using SHA-1.
+
+    let sq = Sq::new();
+
+    let new_password = sq.scratch_file("new-password.txt");
+    std::fs::write(&new_password, "crazy passw0rd").unwrap();
+
+    let cert_path = sq.test_data()
+        .join("keys")
+        .join("sha1-subkey-priv.pgp");
+
+    let cert = Cert::from_file(&cert_path).expect("can read");
+    let vc = cert.with_policy(STANDARD_POLICY, sq.now())
+        .expect("valid cert");
+
+    // Make sure the subkey key is there and really uses SHA-1.
+    let valid_subkeys: Vec<_> = vc.keys().subkeys()
+        .map(|ka| ka.fingerprint())
+        .collect();
+    let all_subkeys: Vec<_> = cert.keys().subkeys()
+        .map(|ka| ka.fingerprint())
+        .collect();
+
+    assert_eq!(valid_subkeys.len(), 0);
+    assert_eq!(all_subkeys.len(), 1);
+
+    let updated = sq.key_password(
+        cert_path, None, Some(new_password.as_path()), None);
+    for ka in updated.keys() {
+        assert!(! ka.has_unencrypted_secret());
+    }
+}
+
 #[test]
 fn subkey_without_secret_key_material() {
     // Make sure we can change the password of keys where some of the