Align sq key subkey expire and sq key subkey revoke.

- Make the latter take a named --key argument that can be given
    multiple times.

  - Fixes #329.
This commit is contained in:
Justus Winter 2024-09-16 15:01:24 +02:00
parent 719f282544
commit 2c4ecbb41e
No known key found for this signature in database
GPG Key ID: 686F55B4AB2B3386
3 changed files with 84 additions and 26 deletions

View File

@ -534,6 +534,7 @@ pub struct ExpireCommand {
#[clap( #[clap(
long, long,
value_name = "FINGERPRINT|KEYID",
help = "Change the expiration of this subkey", help = "Change the expiration of this subkey",
required = true, required = true,
)] )]
@ -585,13 +586,27 @@ const SUBKEY_REVOKE_EXAMPLES: Actions = Actions {
"alice-secret.pgp", "alice-secret.pgp",
], ],
}), }),
Action::Example(Example { Action::Example(Example {
comment: "\ comment: "\
Revoke Alice's signing subkey.", Revoke Alice's signing subkey.",
command: &[ command: &[
"sq", "key", "subkey", "revoke", "sq", "key", "subkey", "revoke",
"--cert", "EB28F26E2739A4870ECC47726F0073F60FD0CBF0", "--cert", "EB28F26E2739A4870ECC47726F0073F60FD0CBF0",
"42020B87D51877E5AF8D272124F3955B0B8DECC8", "--key", "42020B87D51877E5AF8D272124F3955B0B8DECC8",
"retired",
"Subkey rotation.",
],
}),
Action::Example(Example {
comment: "\
Revoke Alice's signing subkey and encryption subkeys.",
command: &[
"sq", "key", "subkey", "revoke",
"--cert", "EB28F26E2739A4870ECC47726F0073F60FD0CBF0",
"--key", "42020B87D51877E5AF8D272124F3955B0B8DECC8",
"--key", "74DCDEAF17D9B995679EB52BA6E65EA2C8497728",
"retired", "retired",
"Subkey rotation.", "Subkey rotation.",
], ],
@ -675,10 +690,12 @@ for the file to contain more than one certificate.",
pub revoker_file: Option<FileOrStdin>, pub revoker_file: Option<FileOrStdin>,
#[clap( #[clap(
long = "key",
value_name = "FINGERPRINT|KEYID", value_name = "FINGERPRINT|KEYID",
help = "The subkey to revoke", help = "Revoke this subkey",
required = true,
)] )]
pub subkey: KeyHandle, pub keys: Vec<KeyHandle>,
#[clap( #[clap(
value_name = "REASON", value_name = "REASON",

View File

@ -6,7 +6,7 @@ use chrono::Utc;
use sequoia_openpgp as openpgp; use sequoia_openpgp as openpgp;
use openpgp::cert::KeyBuilder; use openpgp::cert::KeyBuilder;
use openpgp::cert::SubkeyRevocationBuilder; use openpgp::cert::SubkeyRevocationBuilder;
use openpgp::packet::{Key, key}; use openpgp::packet::{Key, Signature, key};
use openpgp::packet::signature::subpacket::NotationData; use openpgp::packet::signature::subpacket::NotationData;
use openpgp::parse::Parse; use openpgp::parse::Parse;
use openpgp::serialize::Serialize; use openpgp::serialize::Serialize;
@ -116,27 +116,26 @@ fn subkey_expire(sq: Sq, command: ExpireCommand)
struct SubkeyRevocation { struct SubkeyRevocation {
cert: Cert, cert: Cert,
revoker: Cert, revoker: Cert,
revocation_packet: Packet, revocations: Vec<(Key<key::PublicParts, key::SubordinateRole>, Signature)>,
subkey: Key<key::PublicParts, key::SubordinateRole>,
} }
impl SubkeyRevocation { impl SubkeyRevocation {
/// Create a new SubkeyRevocation /// Create a new SubkeyRevocation
pub fn new( pub fn new(
sq: &Sq, sq: &Sq,
keyhandle: &KeyHandle, keyhandles: &[KeyHandle],
cert: Cert, cert: Cert,
revoker: Option<Cert>, revoker: Option<Cert>,
reason: ReasonForRevocation, reason: ReasonForRevocation,
message: &str, message: &str,
notations: &[(bool, NotationData)], notations: &[(bool, NotationData)],
) -> Result<Self> { ) -> Result<Self> {
let valid_cert = cert.with_policy(NULL_POLICY, None)?;
let (revoker, mut signer) let (revoker, mut signer)
= get_secret_signer(sq, &cert, revoker.as_ref())?; = get_secret_signer(sq, &cert, revoker.as_ref())?;
let (subkey, revocation_packet) = { let mut revocations = Vec::new();
let valid_cert = cert.with_policy(NULL_POLICY, None)?; for keyhandle in keyhandles {
let keys = valid_cert.keys().subkeys() let keys = valid_cert.keys().subkeys()
.key_handle(keyhandle.clone()) .key_handle(keyhandle.clone())
.map(|skb| skb.key().clone()) .map(|skb| skb.key().clone())
@ -156,7 +155,7 @@ impl SubkeyRevocation {
)?; )?;
} }
let rev = rev.build(&mut signer, &cert, &subkey, None)?; let rev = rev.build(&mut signer, &cert, &subkey, None)?;
(subkey.into(), Packet::Signature(rev)) revocations.push((subkey, rev));
} else if keys.len() > 1 { } else if keys.len() > 1 {
wprintln!("Key ID {} does not uniquely identify a subkey, \ wprintln!("Key ID {} does not uniquely identify a subkey, \
please use a fingerprint instead.\nValid subkeys:", please use a fingerprint instead.\nValid subkeys:",
@ -198,26 +197,31 @@ impl SubkeyRevocation {
Ok(SubkeyRevocation { Ok(SubkeyRevocation {
cert, cert,
revoker, revoker,
revocation_packet, revocations,
subkey,
}) })
} }
} }
impl RevocationOutput for SubkeyRevocation { impl RevocationOutput for SubkeyRevocation {
fn cert(&self) -> Result<Cert> { fn cert(&self) -> Result<Cert> {
let cert = Cert::from_packets(vec![ Cert::from_packets(
self.cert.primary_key().key().clone().into(), std::iter::once(
self.subkey.clone().into(), Packet::from(self.cert.primary_key().key().clone()))
self.revocation_packet.clone(), .chain(self.revocations.iter().flat_map(
].into_iter())?; |(k, s)| [k.clone().into(), s.clone().into()].into_iter()))
)
Ok(cert)
} }
fn comment(&self) -> String { fn comment(&self) -> String {
format!("Includes a revocation certificate to revoke the subkey {}", if self.revocations.len() == 1 {
self.subkey.fingerprint()) format!("Includes a revocation certificate to revoke the subkey {}",
self.revocations[0].0.fingerprint())
} else {
let fingerprints: Vec<_> = self.revocations.iter()
.map(|k| k.0.fingerprint().to_string()).collect();
format!("Includes revocation certificates to revoke the subkeys {}",
fingerprints.join(", "))
}
} }
fn revoker(&self) -> &Cert { fn revoker(&self) -> &Cert {
@ -352,7 +356,7 @@ pub fn subkey_revoke(
let revocation = SubkeyRevocation::new( let revocation = SubkeyRevocation::new(
&sq, &sq,
&command.subkey, &command.keys,
cert, cert,
revoker, revoker,
command.reason.into(), command.reason.into(),

View File

@ -251,7 +251,7 @@ fn sq_key_subkey_revoke() -> Result<()> {
"key", "key",
"subkey", "subkey",
"revoke", "revoke",
&subkey_fingerprint.to_string(), "--key", &subkey_fingerprint.to_string(),
reason_str, reason_str,
message, message,
]); ]);
@ -276,7 +276,8 @@ fn sq_key_subkey_revoke() -> Result<()> {
} }
let output = cmd.output()?; let output = cmd.output()?;
if !output.status.success() { if !output.status.success() {
panic!("sq exited with non-zero status code: {:?}", output.stderr); panic!("sq exited with non-zero status code: {}",
String::from_utf8_lossy(&output.stderr));
} }
// whether we found a revocation signature // whether we found a revocation signature
@ -347,6 +348,42 @@ fn sq_key_subkey_revoke() -> Result<()> {
Ok(()) Ok(())
} }
#[test]
fn sq_key_subkey_revoke_multiple() -> Result<()> {
let sq = Sq::new();
let (cert, cert_path, _rev_path)
= sq.key_generate(&["--email", "alice@example.org"], &[]);
assert!(cert.keys().subkeys().count() > 0);
sq.key_import(cert_path);
let mut cmd = sq.command();
cmd.args([
"key", "subkey", "revoke",
"--cert", &cert.fingerprint().to_string(),
]);
for subkey in cert.keys().subkeys() {
cmd.args(["--key", &subkey.key().fingerprint().to_string(),]);
}
cmd.args(["retired", "rotation"]);
sq.run(cmd, Some(true));
let revoked = sq.cert_export(cert.key_handle());
assert_eq!(cert.keys().subkeys().count(), revoked.keys().subkeys().count());
let vrevoked = revoked.with_policy(STANDARD_POLICY, sq.now())?;
for subkey in vrevoked.keys().subkeys() {
use sequoia_openpgp::cert::amalgamation::ValidAmalgamation;
assert!(matches!(subkey.revocation_status(),
RevocationStatus::Revoked(_)));
}
Ok(())
}
#[test] #[test]
fn sq_key_subkey_revoke_thirdparty() -> Result<()> { fn sq_key_subkey_revoke_thirdparty() -> Result<()> {
let sq = Sq::new(); let sq = Sq::new();
@ -467,7 +504,7 @@ fn sq_key_subkey_revoke_thirdparty() -> Result<()> {
"key", "key",
"subkey", "subkey",
"revoke", "revoke",
&subkey_fingerprint.to_string(), "--key", &subkey_fingerprint.to_string(),
reason_str, reason_str,
message, message,
]); ]);