Consolidate sq revoke commands as sq key subcommands

- Move the `sq revoke certificate`, `sq revoke subkey` and `sq revoke
  userid` subcommands below the `sq key` namespace as `sq key revoke`,
  `sq key subkey revoke` and `sq key userid revoke` (respectively). This
  consolidates commands relevant to key management below `sq key`, which
  is in line with already existing subcommands (e.g. `sq key generate`,
  `sq key subkey add` or `sq key userid add`).
- Replace the use of a common `revoke()` with `CertificateRevocation`,
  `SubkeyRevocation` and `UserIDRevocation` to reduce complexity and
  allow for easier per target (i.e., certificate, subkey or userid)
  command modification.
- Allow specifying an output file using `--output`/ `-o` for all
  revocation subcommands (i.e., `sq key revoke`, `sq key subkey revoke`,
  `sq key userid revoke`). If unspecified, output goes to stdout as
  before.
- Add common test facilities to create a default certificate in a
  temporary directory.
- Add common test function to compare a set of notations with those in
  a `Signature`.
- Replace the integration tests which used to test a combined `sq
  revoke` subcommand with integration tests for `sq key subkey revoke`,
  `sq key userid revoke` and `sq key revoke` using direct and third
  party revocation.

Fixes #93

src/commands/key.rs

@@ -14,6 +14,8 @@ mod generate;
 use generate::generate;
 mod password;
 use password::password;
+mod revoke;
+use revoke::certificate_revoke;
 mod subkey;
 use subkey::subkey;
 mod userid;
@@ -25,6 +27,7 @@ pub fn dispatch(config: Config, command: sq_cli::key::Command) -> Result<()> {
         Generate(c) => generate(config, c)?,
         Password(c) => password(config, c)?,
         Userid(c) => userid(config, c)?,
+        Revoke(c) => certificate_revoke(config, c)?,
         Subkey(c) => subkey(config, c)?,
         ExtractCert(c) => extract_cert(config, c)?,
         Adopt(c) => adopt(config, c)?,

src/commands/key/revoke.rs

@@ -0,0 +1,203 @@
+use std::time::SystemTime;
+
+use anyhow::Context;
+
+use openpgp::armor::Kind;
+use openpgp::armor::Writer;
+use openpgp::cert::CertRevocationBuilder;
+use openpgp::packet::signature::subpacket::NotationData;
+use openpgp::policy::Policy;
+use openpgp::serialize::Serialize;
+use openpgp::types::ReasonForRevocation;
+use openpgp::Cert;
+use openpgp::Packet;
+use openpgp::Result;
+use sequoia_openpgp as openpgp;
+
+use crate::commands::cert_stub;
+use crate::common::get_secret_signer;
+use crate::common::read_cert;
+use crate::common::read_secret;
+use crate::common::RevocationOutput;
+use crate::parse_notations;
+use crate::sq_cli::key::RevokeCommand;
+use crate::sq_cli::types::FileOrStdout;
+use crate::Config;
+
+/// Handle the revocation of a certificate
+struct CertificateRevocation<'a> {
+    cert: Cert,
+    secret: Cert,
+    policy: &'a dyn Policy,
+    time: Option<SystemTime>,
+    revocation_packet: Packet,
+    first_party_issuer: bool,
+}
+
+impl<'a> CertificateRevocation<'a> {
+    /// Create a new CertificateRevocation
+    pub fn new(
+        cert: Cert,
+        secret: Option<Cert>,
+        policy: &'a dyn Policy,
+        time: Option<SystemTime>,
+        private_key_store: Option<&str>,
+        reason: ReasonForRevocation,
+        message: &str,
+        notations: &[(bool, NotationData)],
+    ) -> Result<Self> {
+        let (secret, mut signer) = get_secret_signer(
+            &cert,
+            policy,
+            secret.as_ref(),
+            private_key_store,
+            time,
+        )?;
+
+        let first_party_issuer = secret.fingerprint() == cert.fingerprint();
+
+        let revocation_packet = {
+            // Create a revocation for the certificate.
+            let mut rev = CertRevocationBuilder::new()
+                .set_reason_for_revocation(reason, message.as_bytes())?;
+            if let Some(time) = time {
+                rev = rev.set_signature_creation_time(time)?;
+            }
+            for (critical, notation) in notations {
+                rev = rev.add_notation(
+                    notation.name(),
+                    notation.value(),
+                    Some(notation.flags().clone()),
+                    *critical,
+                )?;
+            }
+            let rev = rev.build(&mut signer, &cert, None)?;
+            Packet::Signature(rev)
+        };
+
+        Ok(CertificateRevocation {
+            cert,
+            secret,
+            policy,
+            time,
+            revocation_packet,
+            first_party_issuer,
+        })
+    }
+}
+
+impl<'a> RevocationOutput for CertificateRevocation<'a> {
+    /// Write the revocation certificate to output
+    fn write(
+        &self,
+        output: FileOrStdout,
+        binary: bool,
+        force: bool,
+    ) -> Result<()> {
+        let mut output = output.create_safe(force)?;
+
+        let (stub, packets): (Cert, Vec<Packet>) = {
+            if self.first_party_issuer {
+                (self.cert.clone(), vec![self.revocation_packet.clone()])
+            } else {
+                let cert_stub = match cert_stub(
+                    self.cert.clone(),
+                    self.policy,
+                    self.time,
+                    None,
+                ) {
+                    Ok(stub) => stub,
+                    // We failed to create a stub.  Just use the original
+                    // certificate as is.
+                    Err(_) => self.cert.clone(),
+                };
+
+                (
+                    cert_stub.clone(),
+                    cert_stub
+                        .insert_packets(self.revocation_packet.clone())?
+                        .into_packets()
+                        .collect(),
+                )
+            }
+        };
+
+        if binary {
+            for packet in packets {
+                packet
+                    .serialize(&mut output)
+                    .context("serializing revocation certificate")?;
+            }
+        } else {
+            // Add some more helpful ASCII-armor comments.
+            let mut more: Vec<String> = vec![];
+
+            // First, the thing that is being revoked.
+            more.push("including a revocation for the certificate".to_string());
+
+            if !self.first_party_issuer {
+                // Then if it was issued by a third-party.
+                more.push("issued by".to_string());
+                more.push(self.secret.fingerprint().to_spaced_hex());
+                if let Ok(valid_cert) =
+                    &stub.with_policy(self.policy, self.time)
+                {
+                    if let Ok(uid) = valid_cert.primary_userid() {
+                        let uid = String::from_utf8_lossy(uid.value());
+                        // Truncate it, if it is too long.
+                        more.push(format!(
+                            "{:?}",
+                            uid.chars().take(70).collect::<String>()
+                        ));
+                    }
+                }
+            }
+
+            let headers = &stub.armor_headers();
+            let headers: Vec<(&str, &str)> = headers
+                .iter()
+                .map(|s| ("Comment", s.as_str()))
+                .chain(more.iter().map(|value| ("Comment", value.as_str())))
+                .collect();
+
+            let mut writer =
+                Writer::with_headers(&mut output, Kind::PublicKey, headers)?;
+            for packet in packets {
+                packet
+                    .serialize(&mut writer)
+                    .context("serializing revocation certificate")?;
+            }
+            writer.finalize()?;
+        }
+
+        Ok(())
+    }
+}
+
+/// Revoke a certificate
+pub fn certificate_revoke(
+    config: Config,
+    command: RevokeCommand,
+) -> Result<()> {
+    let cert = read_cert(command.input.as_deref())?;
+
+    let secret = read_secret(command.secret_key_file.as_deref())?;
+
+    let time = Some(config.time);
+
+    let notations = parse_notations(command.notation)?;
+
+    let revocation = CertificateRevocation::new(
+        cert,
+        secret,
+        &config.policy,
+        time,
+        command.private_key_store.as_deref(),
+        command.reason.into(),
+        &command.message,
+        &notations,
+    )?;
+    revocation.write(command.output, command.binary, config.force)?;
+
+    Ok(())
+}

src/commands/key/subkey.rs

@@ -1,22 +1,243 @@
+use std::time::SystemTime;
+
+use anyhow::Context;
+
+use anyhow::anyhow;
 use chrono::DateTime;
 use chrono::Utc;
 
+use openpgp::armor::Kind;
+use openpgp::armor::Writer;
+use openpgp::cert::amalgamation::ValidAmalgamation;
 use openpgp::cert::KeyBuilder;
+use openpgp::cert::SubkeyRevocationBuilder;
+use openpgp::packet::signature::subpacket::NotationData;
 use openpgp::parse::Parse;
+use openpgp::policy::Policy;
 use openpgp::serialize::Serialize;
 use openpgp::types::KeyFlags;
+use openpgp::types::ReasonForRevocation;
 use openpgp::Cert;
+use openpgp::KeyHandle;
+use openpgp::Packet;
 use openpgp::Result;
 use sequoia_openpgp as openpgp;
 
+use crate::commands::cert_stub;
+use crate::common::get_secret_signer;
+use crate::common::read_cert;
+use crate::common::read_secret;
+use crate::common::RevocationOutput;
+use crate::common::NULL_POLICY;
+use crate::parse_notations;
 use crate::sq_cli::key::EncryptPurpose;
-use crate::sq_cli::key::SubkeyCommand;
 use crate::sq_cli::key::SubkeyAddCommand;
+use crate::sq_cli::key::SubkeyCommand;
+use crate::sq_cli::key::SubkeyRevokeCommand;
+use crate::sq_cli::types::FileOrStdout;
 use crate::Config;
 
+/// Handle the revocation of a subkey
+struct SubkeyRevocation<'a> {
+    cert: Cert,
+    secret: Cert,
+    policy: &'a dyn Policy,
+    time: Option<SystemTime>,
+    revocation_packet: Packet,
+    first_party_issuer: bool,
+    subkey_packets: Vec<Packet>,
+    subkey_as_hex: String,
+}
+
+impl<'a> SubkeyRevocation<'a> {
+    /// Create a new SubkeyRevocation
+    pub fn new(
+        keyhandle: &KeyHandle,
+        cert: Cert,
+        secret: Option<Cert>,
+        policy: &'a dyn Policy,
+        time: Option<SystemTime>,
+        private_key_store: Option<&str>,
+        reason: ReasonForRevocation,
+        message: &str,
+        notations: &[(bool, NotationData)],
+    ) -> Result<Self> {
+        let (secret, mut signer) = get_secret_signer(
+            &cert,
+            policy,
+            secret.as_ref(),
+            private_key_store,
+            time,
+        )?;
+
+        let first_party_issuer = secret.fingerprint() == cert.fingerprint();
+
+        let mut subkey_packets = vec![];
+        let mut subkey_as_hex = String::new();
+        let mut subkey = None;
+
+        let revocation_packet = {
+            let valid_cert = cert.with_policy(NULL_POLICY, None)?;
+
+            for key in valid_cert.keys().subkeys() {
+                if keyhandle.aliases(KeyHandle::from(key.fingerprint())) {
+                    subkey_packets.push(Packet::from(key.key().clone()));
+                    subkey_packets
+                        .push(Packet::from(key.binding_signature().clone()));
+                    subkey_as_hex.push_str(&key.fingerprint().to_spaced_hex());
+                    subkey = Some(key);
+                    break;
+                }
+            }
+
+            if let Some(ref subkey) = subkey {
+                let mut rev = SubkeyRevocationBuilder::new()
+                    .set_reason_for_revocation(reason, message.as_bytes())?;
+                if let Some(time) = time {
+                    rev = rev.set_signature_creation_time(time)?;
+                }
+                for (critical, notation) in notations {
+                    rev = rev.add_notation(
+                        notation.name(),
+                        notation.value(),
+                        Some(notation.flags().clone()),
+                        *critical,
+                    )?;
+                }
+                let rev = rev.build(&mut signer, &cert, subkey.key(), None)?;
+                Packet::Signature(rev)
+            } else {
+                eprintln!(
+                    "Subkey {} not found.\nValid subkeys:",
+                    keyhandle.to_spaced_hex()
+                );
+                let mut have_valid = false;
+                for k in valid_cert.keys().subkeys() {
+                    have_valid = true;
+                    eprintln!(
+                        "  - {} {} [{:?}]",
+                        k.fingerprint().to_hex(),
+                        DateTime::<Utc>::from(k.creation_time()).date_naive(),
+                        k.key_flags().unwrap_or_else(KeyFlags::empty)
+                    );
+                }
+                if !have_valid {
+                    eprintln!("  - Certificate has no subkeys.");
+                }
+                return Err(anyhow!(
+                    "The certificate does not contain the specified subkey."
+                ));
+            }
+        };
+
+        Ok(SubkeyRevocation {
+            cert,
+            secret,
+            policy,
+            time,
+            revocation_packet,
+            first_party_issuer,
+            subkey_packets,
+            subkey_as_hex,
+        })
+    }
+}
+
+impl<'a> RevocationOutput for SubkeyRevocation<'a> {
+    /// Write the revocation certificate to output
+    fn write(
+        &self,
+        output: FileOrStdout,
+        binary: bool,
+        force: bool,
+    ) -> Result<()> {
+        let mut output = output.create_safe(force)?;
+
+        let (stub, packets): (Cert, Vec<Packet>) = {
+            let mut cert_stub = match cert_stub(
+                self.cert.clone(),
+                self.policy,
+                self.time,
+                None,
+            ) {
+                Ok(stub) => stub,
+                // We failed to create a stub.  Just use the original
+                // certificate as is.
+                Err(_) => self.cert.clone(),
+            };
+
+            if !self.subkey_packets.is_empty() {
+                cert_stub =
+                    cert_stub.insert_packets(self.subkey_packets.clone())?;
+            }
+
+            (
+                cert_stub.clone(),
+                cert_stub
+                    .insert_packets(self.revocation_packet.clone())?
+                    .into_packets()
+                    .collect(),
+            )
+        };
+
+        if binary {
+            for packet in packets {
+                packet
+                    .serialize(&mut output)
+                    .context("serializing revocation certificate")?;
+            }
+        } else {
+            // Add some more helpful ASCII-armor comments.
+            let mut more: Vec<String> = vec![];
+
+            // First, the thing that is being revoked.
+            more.push(
+                "including a revocation to revoke the subkey".to_string(),
+            );
+            more.push(self.subkey_as_hex.clone());
+
+            if !self.first_party_issuer {
+                // Then if it was issued by a third-party.
+                more.push("issued by".to_string());
+                more.push(self.secret.fingerprint().to_spaced_hex());
+                if let Ok(valid_cert) =
+                    &stub.with_policy(self.policy, self.time)
+                {
+                    if let Ok(uid) = valid_cert.primary_userid() {
+                        let uid = String::from_utf8_lossy(uid.value());
+                        // Truncate it, if it is too long.
+                        more.push(format!(
+                            "{:?}",
+                            uid.chars().take(70).collect::<String>()
+                        ));
+                    }
+                }
+            }
+
+            let headers = &stub.armor_headers();
+            let headers: Vec<(&str, &str)> = headers
+                .iter()
+                .map(|s| ("Comment", s.as_str()))
+                .chain(more.iter().map(|value| ("Comment", value.as_str())))
+                .collect();
+
+            let mut writer =
+                Writer::with_headers(&mut output, Kind::PublicKey, headers)?;
+            for packet in packets {
+                packet
+                    .serialize(&mut writer)
+                    .context("serializing revocation certificate")?;
+            }
+            writer.finalize()?;
+        }
+        Ok(())
+    }
+}
+
 pub fn subkey(config: Config, command: SubkeyCommand) -> Result<()> {
     match command {
         SubkeyCommand::Add(c) => subkey_add(config, c)?,
+        SubkeyCommand::Revoke(c) => subkey_revoke(config, c)?,
     }
 
     Ok(())
@@ -66,3 +287,43 @@ fn subkey_add(
     }
     Ok(())
 }
+
+/// Revoke a Subkey of an existing primary key
+///
+/// ## Errors
+///
+/// Returns an error if parsing of the [`KeyHandle`] fails, if reading of the
+/// [`Cert`] fails, if retrieval of [`NotationData`] fails or if the eventual
+/// revocation fails.
+pub fn subkey_revoke(
+    config: Config,
+    command: SubkeyRevokeCommand,
+) -> Result<()> {
+    let cert = read_cert(command.input.as_deref())?;
+
+    let secret = read_secret(command.secret_key_file.as_deref())?;
+
+    let time = Some(config.time);
+
+    let notations = parse_notations(command.notation)?;
+
+    let keyhandle: KeyHandle = command.subkey.parse().context(format!(
+        "Parsing {:?} as an OpenPGP fingerprint or Key ID",
+        command.subkey
+    ))?;
+
+    let revocation = SubkeyRevocation::new(
+        &keyhandle,
+        cert,
+        secret,
+        &config.policy,
+        time,
+        command.private_key_store.as_deref(),
+        command.reason.into(),
+        &command.message,
+        &notations,
+    )?;
+    revocation.write(command.output, command.binary, config.force)?;
+
+    Ok(())
+}

src/commands/key/userid.rs

@@ -1,33 +1,243 @@
+use std::str::from_utf8;
 use std::time::SystemTime;
 
 use anyhow::Context;
 
+use anyhow::anyhow;
 use itertools::Itertools;
 
+use openpgp::armor::Kind;
+use openpgp::armor::Writer;
 use openpgp::cert::amalgamation::ValidAmalgamation;
-use openpgp::packet::UserID;
+use openpgp::cert::UserIDRevocationBuilder;
+use openpgp::packet::signature::subpacket::NotationData;
 use openpgp::packet::signature::subpacket::SubpacketTag;
 use openpgp::packet::signature::SignatureBuilder;
+use openpgp::packet::UserID;
 use openpgp::parse::Parse;
 use openpgp::policy::HashAlgoSecurity;
 use openpgp::policy::Policy;
 use openpgp::serialize::Serialize;
+use openpgp::types::ReasonForRevocation;
 use openpgp::types::SignatureType;
 use openpgp::Cert;
 use openpgp::Packet;
 use openpgp::Result;
 use sequoia_openpgp as openpgp;
 
+use crate::commands::cert_stub;
 use crate::commands::get_primary_keys;
+use crate::common::get_secret_signer;
+use crate::common::read_cert;
+use crate::common::read_secret;
+use crate::common::RevocationOutput;
+use crate::common::NULL_POLICY;
+use crate::parse_notations;
 use crate::sq_cli;
+use crate::sq_cli::key::UseridRevokeCommand;
+use crate::sq_cli::types::FileOrStdout;
 use crate::Config;
 
+/// Handle the revocation of a User ID
+struct UserIDRevocation<'a> {
+    cert: Cert,
+    secret: Cert,
+    policy: &'a dyn Policy,
+    time: Option<SystemTime>,
+    revocation_packet: Packet,
+    first_party_issuer: bool,
+    userid: String,
+}
+
+impl<'a> UserIDRevocation<'a> {
+    /// Create a new UserIDRevocation
+    pub fn new(
+        userid: String,
+        force: bool,
+        cert: Cert,
+        secret: Option<Cert>,
+        policy: &'a dyn Policy,
+        time: Option<SystemTime>,
+        private_key_store: Option<&str>,
+        reason: ReasonForRevocation,
+        message: &str,
+        notations: &[(bool, NotationData)],
+    ) -> Result<Self> {
+        let (secret, mut signer) = get_secret_signer(
+            &cert,
+            policy,
+            secret.as_ref(),
+            private_key_store,
+            time,
+        )?;
+
+        let first_party_issuer = secret.fingerprint() == cert.fingerprint();
+
+        let revocation_packet = {
+            // Create a revocation for a User ID.
+
+            // Unless force is specified, we require the User ID to
+            // have a valid self signature under the Null policy.  We
+            // use the Null policy and not the standard policy,
+            // because it is still useful to revoke a User ID whose
+            // self signature is no longer valid.  For instance, the
+            // binding signature may use SHA-1.
+            if !force {
+                let valid_cert = cert.with_policy(NULL_POLICY, None)?;
+                let present = valid_cert
+                    .userids()
+                    .any(|u| u.value() == userid.as_bytes());
+
+                if !present {
+                    eprintln!(
+                        "User ID, cert: Cert, secret: Option<Cert>: '{}' not found.\nValid User IDs:",
+                        userid
+                    );
+                    let mut have_valid = false;
+                    for ua in valid_cert.userids() {
+                        if let Ok(u) = from_utf8(ua.userid().value()) {
+                            have_valid = true;
+                            eprintln!("  - {}", u);
+                        }
+                    }
+                    if !have_valid {
+                        eprintln!("  - Certificate has no valid User IDs.");
+                    }
+                    return Err(anyhow!(
+                        "The certificate does not contain the specified User \
+                        ID.  To create a revocation certificate for that User \
+                        ID anyways, specify '--force'"
+                    ));
+                }
+            }
+
+            let mut rev = UserIDRevocationBuilder::new()
+                .set_reason_for_revocation(reason, message.as_bytes())?;
+            if let Some(time) = time {
+                rev = rev.set_signature_creation_time(time)?;
+            }
+            for (critical, notation) in notations {
+                rev = rev.add_notation(
+                    notation.name(),
+                    notation.value(),
+                    Some(notation.flags().clone()),
+                    *critical,
+                )?;
+            }
+            let rev = rev.build(
+                &mut signer,
+                &cert,
+                &UserID::from(userid.as_str()),
+                None,
+            )?;
+            Packet::Signature(rev)
+        };
+
+        Ok(UserIDRevocation {
+            cert,
+            secret,
+            policy,
+            time,
+            revocation_packet,
+            first_party_issuer,
+            userid,
+        })
+    }
+}
+
+impl<'a> RevocationOutput for UserIDRevocation<'a> {
+    /// Write the revocation certificate to output
+    fn write(
+        &self,
+        output: FileOrStdout,
+        binary: bool,
+        force: bool,
+    ) -> Result<()> {
+        let mut output = output.create_safe(force)?;
+
+        let (stub, packets): (Cert, Vec<Packet>) = {
+            let cert_stub = match cert_stub(
+                self.cert.clone(),
+                self.policy,
+                self.time,
+                Some(&UserID::from(self.userid.clone())),
+            ) {
+                Ok(stub) => stub,
+                // We failed to create a stub.  Just use the original
+                // certificate as is.
+                Err(_) => self.cert.clone(),
+            };
+
+            (
+                cert_stub.clone(),
+                cert_stub
+                    .insert_packets(self.revocation_packet.clone())?
+                    .into_packets()
+                    .collect(),
+            )
+        };
+
+        if binary {
+            for packet in packets {
+                packet
+                    .serialize(&mut output)
+                    .context("serializing revocation certificate")?;
+            }
+        } else {
+            // Add some more helpful ASCII-armor comments.
+            let mut more: Vec<String> = vec![];
+
+            // First, the thing that is being revoked.
+            more.push(
+                "including a revocation to revoke the User ID".to_string(),
+            );
+            more.push(format!("{:?}", self.userid));
+
+            if !self.first_party_issuer {
+                // Then if it was issued by a third-party.
+                more.push("issued by".to_string());
+                more.push(self.secret.fingerprint().to_spaced_hex());
+                if let Ok(valid_cert) =
+                    &stub.with_policy(self.policy, self.time)
+                {
+                    if let Ok(uid) = valid_cert.primary_userid() {
+                        let uid = String::from_utf8_lossy(uid.value());
+                        // Truncate it, if it is too long.
+                        more.push(format!(
+                            "{:?}",
+                            uid.chars().take(70).collect::<String>()
+                        ));
+                    }
+                }
+            }
+
+            let headers = &stub.armor_headers();
+            let headers: Vec<(&str, &str)> = headers
+                .iter()
+                .map(|s| ("Comment", s.as_str()))
+                .chain(more.iter().map(|value| ("Comment", value.as_str())))
+                .collect();
+
+            let mut writer =
+                Writer::with_headers(&mut output, Kind::PublicKey, headers)?;
+            for packet in packets {
+                packet
+                    .serialize(&mut writer)
+                    .context("serializing revocation certificate")?;
+            }
+            writer.finalize()?;
+        }
+        Ok(())
+    }
+}
+
 pub fn userid(
     config: Config,
     command: sq_cli::key::UseridCommand,
 ) -> Result<()> {
     match command {
         sq_cli::key::UseridCommand::Add(c) => userid_add(config, c)?,
+        sq_cli::key::UseridCommand::Revoke(c) => userid_revoke(config, c)?,
         sq_cli::key::UseridCommand::Strip(c) => userid_strip(config, c)?,
     }
 
@@ -244,3 +454,39 @@ signatures on other User IDs to make the key valid again.",
     }
     Ok(())
 }
+
+/// Revoke a UserID of an existing primary key
+///
+/// ## Errors
+///
+/// Returns an error if reading of the [`Cert`] fails, if retrieval of
+/// [`NotationData`] fails or if the eventual revocation fails.
+pub fn userid_revoke(
+    config: Config,
+    command: UseridRevokeCommand,
+) -> Result<()> {
+    let cert = read_cert(command.input.as_deref())?;
+
+    let secret = read_secret(command.secret_key_file.as_deref())?;
+
+    let time = Some(config.time);
+
+    let notations = parse_notations(command.notation)?;
+
+    let revocation = UserIDRevocation::new(
+        command.userid,
+        config.force,
+        cert,
+        secret,
+        &config.policy,
+        time,
+        command.private_key_store.as_deref(),
+        command.reason.into(),
+        &command.message,
+        &notations,
+    )?;
+
+    revocation.write(command.output, command.binary, config.force)?;
+
+    Ok(())
+}

src/commands/mod.rs

@@ -53,7 +53,6 @@ pub mod decrypt;
 pub use self::decrypt::decrypt;
 pub mod sign;
 pub use self::sign::sign;
-pub mod revoke;
 pub mod dump;
 pub use self::dump::dump;
 mod inspect;

src/commands/revoke.rs

@@ -1,440 +0,0 @@
-use anyhow::Context as _;
-use std::path::Path;
-use std::time::SystemTime;
-
-use sequoia_openpgp as openpgp;
-use openpgp::armor;
-use openpgp::cert::prelude::*;
-use openpgp::KeyHandle;
-use openpgp::Packet;
-use openpgp::packet::signature::subpacket::NotationData;
-use openpgp::packet::UserID;
-use openpgp::parse::Parse;
-use openpgp::policy::NullPolicy;
-use openpgp::Result;
-use openpgp::serialize::Serialize;
-use openpgp::types::KeyFlags;
-use openpgp::types::ReasonForRevocation;
-use crate::sq_cli::types::FileOrStdin;
-use crate::sq_cli::types::FileOrStdout;
-use crate::{
-    commands::cert_stub,
-    Config,
-    load_certs,
-    parse_notations,
-};
-
-const NP: &NullPolicy = &NullPolicy::new();
-
-enum RevocationTarget {
-    Certificate,
-    Subkey(KeyHandle),
-    UserID(String),
-}
-
-impl RevocationTarget {
-    fn is_certificate(&self) -> bool {
-        matches!(self, RevocationTarget::Certificate)
-    }
-
-    fn userid(&self) -> Option<&str> {
-        if let RevocationTarget::UserID(userid) = self {
-            Some(userid)
-        } else {
-            None
-        }
-    }
-}
-
-use crate::sq_cli::revoke;
-
-pub fn dispatch(config: Config, c: revoke::Command) -> Result<()> {
-
-    match c.subcommand {
-        revoke::Subcommands::Certificate(c) => revoke_certificate(config, c),
-        revoke::Subcommands::Subkey(c) => revoke_subkey(config, c),
-        revoke::Subcommands::Userid(c) => revoke_userid(config, c),
-    }
-}
-
-pub fn revoke_certificate(config: Config, c: revoke::CertificateCommand) -> Result<()> {
-    let revocation_target = RevocationTarget::Certificate;
-
-    let cert = read_cert(c.input.as_deref())?;
-
-    let secret = read_secret(c.secret_key_file.as_deref())?;
-
-    let time = Some(config.time);
-
-    let notations = parse_notations(c.notation)?;
-
-    revoke(
-        config,
-        c.private_key_store.as_deref(),
-        cert,
-        revocation_target,
-        secret,
-        c.binary,
-        time,
-        c.reason.into(),
-        &c.message,
-        &notations
-    )?;
-
-    Ok(())
-}
-
-pub fn revoke_subkey(config: Config, c: revoke::SubkeyCommand) -> Result<()> {
-    let revocation_target = {
-        let kh: KeyHandle = c.subkey
-            .parse()
-            .context(
-                format!("Parsing {:?} as an OpenPGP fingerprint or Key ID",
-                        c.subkey))?;
-
-        RevocationTarget::Subkey(kh)
-    };
-
-    let cert = read_cert(c.input.as_deref())?;
-
-    let secret = read_secret(c.secret_key_file.as_deref())?;
-
-    let time = Some(config.time);
-
-    let notations = parse_notations(c.notation)?;
-
-    revoke(
-        config,
-        c.private_key_store.as_deref(),
-        cert,
-        revocation_target,
-        secret,
-        c.binary,
-        time,
-        c.reason.into(),
-        &c.message,
-        &notations
-    )?;
-
-    Ok(())
-}
-
-pub fn revoke_userid(config: Config, c: revoke::UseridCommand) -> Result<()> {
-    let revocation_target = RevocationTarget::UserID(c.userid);
-
-    let cert = read_cert(c.input.as_deref())?;
-
-    let secret = read_secret(c.secret_key_file.as_deref())?;
-
-    let time = Some(config.time);
-
-    let notations = parse_notations(c.notation)?;
-
-    revoke(
-        config,
-        c.private_key_store.as_deref(),
-        cert,
-        revocation_target,
-        secret,
-        c.binary,
-        time,
-        c.reason.into(),
-        &c.message,
-        &notations
-    )?;
-
-    Ok(())
-}
-
-/// Parse the cert from input and ensure it is only one cert.
-fn read_cert(input: Option<&Path>) -> Result<Cert> {
-    let input = FileOrStdin::from(input).open()?;
-
-    let cert = CertParser::from_reader(input)?.collect::<Vec<_>>();
-    let cert = match cert.len() {
-        0 => Err(anyhow::anyhow!("No certificates provided."))?,
-        1 => cert.into_iter().next().expect("have one")?,
-        _ => Err(
-            anyhow::anyhow!("Multiple certificates provided."))?,
-    };
-    Ok(cert)
-}
-
-/// Parse the secret key and ensure it is at most one.
-fn read_secret(skf: Option<&Path>) -> Result<Option<Cert>> {
-    let secret = load_certs(skf.into_iter())?;
-    if secret.len() > 1 {
-        Err(anyhow::anyhow!("Multiple secret keys provided."))?;
-    }
-    let secret = secret.into_iter().next();
-    Ok(secret)
-}
-
-
-fn revoke(config: Config,
-          private_key_store: Option<&str>,
-          cert: openpgp::Cert,
-          revocation_target: RevocationTarget,
-          secret: Option<openpgp::Cert>,
-          binary: bool,
-          time: Option<SystemTime>,
-          reason: ReasonForRevocation,
-          message: &str,
-          notations: &[(bool, NotationData)])
-    -> Result<()>
-{
-    let output_type = FileOrStdout::default();
-    let mut output = output_type.create_safe(config.force)?;
-
-    let (secret, mut signer) = if let Some(secret) = secret.as_ref() {
-        if let Ok(keys) = super::get_certification_keys(
-            &[ secret ], &config.policy, private_key_store, time, None)
-        {
-            assert_eq!(keys.len(), 1);
-            (secret, keys.into_iter().next().expect("have one"))
-        } else {
-            if let Some(time) = time {
-                return Err(anyhow::anyhow!("\
-No certification key found: the key specified with --revocation-file \
-does not contain a certification key with secret key material.  \
-Perhaps this is because no certification keys are valid at the time \
-you specified ({})",
-                    chrono::DateTime::<chrono::offset::Utc>::from(time)));
-            } else {
-                return Err(anyhow::anyhow!("\
-No certification key found: the key specified with --revocation-file \
-does not contain a certification key with secret key material"));
-            }
-        }
-    } else {
-        if let Ok(keys) = super::get_certification_keys(
-            &[ &cert ], &config.policy, private_key_store, time, None)
-        {
-            assert_eq!(keys.len(), 1);
-            (&cert, keys.into_iter().next().expect("have one"))
-        } else {
-            if let Some(time) = time {
-                return Err(anyhow::anyhow!("\
-No certification key found: --revocation-file not provided and the
-certificate to revoke does not contain a certification key with secret
-key material.  Perhaps this is because no certification keys are valid at
-the time you specified ({})",
-                    chrono::DateTime::<chrono::offset::Utc>::from(time)));
-            } else {
-                return Err(anyhow::anyhow!("\
-No certification key found: --revocation-file not provided and the
-certificate to revoke does not contain a certification key with secret
-key material"));
-            }
-        }
-    };
-
-    let first_party = secret.fingerprint() == cert.fingerprint();
-    let mut subkey = None;
-
-    let rev: Packet = match revocation_target {
-        RevocationTarget::UserID(ref userid) => {
-            // Create a revocation for a User ID.
-
-            // Unless force is specified, we require the User ID to
-            // have a valid self signature under the Null policy.  We
-            // use the Null policy and not the standard policy,
-            // because it is still useful to revoke a User ID whose
-            // self signature is no longer valid.  For instance, the
-            // binding signature may use SHA-1.
-            if ! config.force {
-                let vc = cert.with_policy(NP, None)?;
-                let present = vc.userids().any(|u| {
-                    u.value() == userid.as_bytes()
-                });
-
-                if ! present {
-                    eprintln!("User ID: '{}' not found.\nValid User IDs:",
-                              userid);
-                    let mut have_valid = false;
-                    for ua in vc.userids() {
-                        if let Ok(u) = std::str::from_utf8(ua.userid().value()) {
-                            have_valid = true;
-                            eprintln!("  - {}", u);
-                        }
-                    }
-                    if ! have_valid {
-                        eprintln!("  - Certificate has no valid User IDs.");
-                    }
-                    return Err(anyhow::anyhow!("\
-The certificate does not contain the specified User ID.  To create
-a revocation certificate for that User ID anyways, specify '--force'"));
-                }
-            }
-
-            let mut rev = UserIDRevocationBuilder::new()
-                .set_reason_for_revocation(reason, message.as_bytes())?;
-            if let Some(time) = time {
-                rev = rev.set_signature_creation_time(time)?;
-            }
-            for (critical, notation) in notations {
-                rev = rev.add_notation(notation.name(),
-                                       notation.value(),
-                                       Some(notation.flags().clone()),
-                                       *critical)?;
-            }
-            let rev = rev.build(
-                &mut signer, &cert, &UserID::from(userid.as_str()), None)?;
-            Packet::Signature(rev)
-        }
-        RevocationTarget::Subkey(ref subkey_fpr) => {
-            let vc = cert.with_policy(NP, None)?;
-
-            for k in vc.keys().subkeys() {
-                if subkey_fpr.aliases(KeyHandle::from(k.fingerprint())) {
-                    subkey = Some(k);
-                    break;
-                }
-            }
-
-            if let Some(ref subkey) = subkey {
-                let mut rev = SubkeyRevocationBuilder::new()
-                    .set_reason_for_revocation(reason, message.as_bytes())?;
-                if let Some(time) = time {
-                    rev = rev.set_signature_creation_time(time)?;
-                }
-                for (critical, notation) in notations {
-                    rev = rev.add_notation(notation.name(),
-                                           notation.value(),
-                                           Some(notation.flags().clone()),
-                                           *critical)?;
-                }
-                let rev = rev.build(
-                    &mut signer, &cert, subkey.key(), None)?;
-                Packet::Signature(rev)
-            } else {
-                eprintln!("Subkey {} not found.\nValid subkeys:",
-                          subkey_fpr.to_spaced_hex());
-                let mut have_valid = false;
-                for k in vc.keys().subkeys() {
-                    have_valid = true;
-                    eprintln!("  - {} {} [{:?}]",
-                              k.fingerprint().to_hex(),
-                              chrono::DateTime::<chrono::offset::Utc>
-                                  ::from(k.creation_time())
-                                  .date_naive(),
-                              k.key_flags().unwrap_or_else(KeyFlags::empty));
-                }
-                if ! have_valid {
-                    eprintln!("  - Certificate has no subkeys.");
-                }
-                return Err(anyhow::anyhow!("\
-The certificate does not contain the specified subkey."));
-            }
-        }
-        RevocationTarget::Certificate => {
-            // Create a revocation for the certificate.
-            let mut rev = CertRevocationBuilder::new()
-                .set_reason_for_revocation(reason, message.as_bytes())?;
-            if let Some(time) = time {
-                rev = rev.set_signature_creation_time(time)?;
-            }
-            for (critical, notation) in notations {
-                rev = rev.add_notation(notation.name(),
-                                       notation.value(),
-                                       Some(notation.flags().clone()),
-                                       *critical)?;
-            }
-            let rev = rev.build(&mut signer, &cert, None)?;
-            Packet::Signature(rev)
-        }
-    };
-
-    let mut stub = None;
-    let packets: Vec<Packet> = if first_party && revocation_target.is_certificate() {
-        vec![ rev ]
-    } else {
-        let mut s = match cert_stub(
-            cert.clone(), &config.policy, time,
-            revocation_target.userid().map(UserID::from).as_ref())
-        {
-            Ok(stub) => stub,
-            // We failed to create a stub.  Just use the original
-            // certificate as is.
-            Err(_) => cert.clone(),
-        };
-
-        if let Some(ref subkey) = subkey {
-            s = s.insert_packets([
-                Packet::from(subkey.key().clone()),
-                Packet::from(subkey.binding_signature().clone())
-            ])?;
-        }
-
-        stub = Some(s.clone());
-
-        s.insert_packets(rev)?
-            .into_packets()
-            .collect()
-    };
-
-    if binary {
-        for p in packets {
-            p.serialize(&mut output)
-                .context("serializing revocation certificate")?;
-        }
-    } else {
-        let cert = stub.as_ref().unwrap_or(&cert);
-
-        // Add some more helpful ASCII-armor comments.
-        let mut more: Vec<String> = Vec::new();
-
-        // First, the thing that is being revoked.
-        match revocation_target {
-            RevocationTarget::Certificate => {
-                more.push(
-                    "including a revocation for the certificate".to_string());
-            }
-            RevocationTarget::Subkey(_) => {
-                more.push(
-                    "including a revocation to revoke the subkey".to_string());
-                more.push(subkey.unwrap().fingerprint().to_spaced_hex());
-            }
-            RevocationTarget::UserID(raw) => {
-                more.push(
-                    "including a revocation to revoke the User ID".to_string());
-                more.push(format!("{:?}", raw));
-            }
-        }
-
-        if ! first_party {
-            // Then if it was issued by a third-party.
-            more.push("issued by".to_string());
-            more.push(secret.fingerprint().to_spaced_hex());
-            if let Ok(vc) = cert.with_policy(&config.policy, time) {
-                if let Ok(uid) = vc.primary_userid() {
-                    let uid = String::from_utf8_lossy(uid.value());
-                    // Truncate it, if it is too long.
-                    more.push(
-                        format!("{:?}",
-                                uid.chars().take(70).collect::<String>()));
-                }
-            }
-        }
-
-        let headers = cert.armor_headers();
-        let headers: Vec<(&str, &str)> = headers
-            .iter()
-            .map(|s| ("Comment", s.as_str()))
-            .chain(
-                more
-                    .iter()
-                    .map(|value| ("Comment", value.as_str())))
-            .collect();
-
-        let mut writer = armor::Writer::with_headers(
-            &mut output, armor::Kind::PublicKey, headers)?;
-        for p in packets {
-            p.serialize(&mut writer)
-                .context("serializing revocation certificate")?;
-        }
-        writer.finalize()?;
-    }
-
-    Ok(())
-}

src/common.rs

@@ -0,0 +1,42 @@
+use std::path::Path;
+
+use anyhow::anyhow;
+use anyhow::Result;
+
+use openpgp::cert::CertParser;
+use openpgp::parse::Parse;
+use openpgp::policy::NullPolicy;
+use openpgp::Cert;
+use sequoia_openpgp as openpgp;
+
+use crate::load_certs;
+use crate::sq_cli::types::FileOrStdin;
+
+mod revoke;
+pub use revoke::get_secret_signer;
+pub use revoke::RevocationOutput;
+
+pub const NULL_POLICY: &NullPolicy = &NullPolicy::new();
+
+/// Parse the cert from input and ensure it is only one cert.
+pub fn read_cert(input: Option<&Path>) -> Result<Cert> {
+    let input = FileOrStdin::from(input).open()?;
+
+    let cert = CertParser::from_reader(input)?.collect::<Vec<_>>();
+    let cert = match cert.len() {
+        0 => Err(anyhow!("No certificates provided."))?,
+        1 => cert.into_iter().next().expect("have one")?,
+        _ => Err(anyhow!("Multiple certificates provided."))?,
+    };
+    Ok(cert)
+}
+
+/// Parse the secret key and ensure it is at most one.
+pub fn read_secret(skf: Option<&Path>) -> Result<Option<Cert>> {
+    let secret = load_certs(skf.into_iter())?;
+    if secret.len() > 1 {
+        Err(anyhow!("Multiple secret keys provided."))?;
+    }
+    let secret = secret.into_iter().next();
+    Ok(secret)
+}

src/common/revoke.rs

@@ -0,0 +1,103 @@
+use std::time::SystemTime;
+
+use anyhow::anyhow;
+use anyhow::Result;
+
+use chrono::offset::Utc;
+use chrono::DateTime;
+
+use openpgp::crypto::Signer;
+use openpgp::policy::Policy;
+use openpgp::Cert;
+use sequoia_openpgp as openpgp;
+
+use crate::commands::get_certification_keys;
+use crate::sq_cli::types::FileOrStdout;
+
+/// A trait for unifying the approach of writing a revocation to an output
+pub trait RevocationOutput {
+    fn write(
+        &self,
+        output: FileOrStdout,
+        binary: bool,
+        force: bool,
+    ) -> Result<()>;
+}
+
+/// Get secret Cert and Signer from an optional secret Cert or a Cert
+///
+/// Returns a secret Cert and the corresponding Signer, derived from `secret`,
+/// if `secret` is `Some`, else attempts to derive it from `cert`.
+///
+/// ## Errors
+///
+/// - Returns an `Error` if `secret` is `Some`, but no suitable certification key
+/// can be derived from it.
+/// - Returns an `Error` if `secret` is `None` and no suitable certification key
+/// can be derived from `cert`.
+pub fn get_secret_signer<'a>(
+    cert: &'a Cert,
+    policy: &dyn Policy,
+    secret: Option<&'a Cert>,
+    private_key_store: Option<&str>,
+    time: Option<SystemTime>,
+) -> Result<(Cert, Box<dyn Signer + Send + Sync>)> {
+    if let Some(secret) = secret {
+        if let Ok(keys) = get_certification_keys(
+            &[secret.clone()],
+            policy,
+            private_key_store,
+            time,
+            None,
+        ) {
+            assert_eq!(keys.len(), 1);
+            Ok((secret.clone(), keys.into_iter().next().expect("have one")))
+        } else {
+            if let Some(time) = time {
+                return Err(anyhow!(
+                    "\
+No certification key found: the key specified with --revocation-file \
+does not contain a certification key with secret key material.  \
+Perhaps this is because no certification keys are valid at the time \
+you specified ({})",
+                    DateTime::<Utc>::from(time)
+                ));
+            } else {
+                return Err(anyhow!(
+                    "\
+No certification key found: the key specified with --revocation-file \
+does not contain a certification key with secret key material"
+                ));
+            }
+        }
+    } else {
+        if let Ok(keys) = get_certification_keys(
+            &[cert],
+            policy,
+            private_key_store,
+            time,
+            None,
+        ) {
+            assert_eq!(keys.len(), 1);
+            Ok((cert.clone(), keys.into_iter().next().expect("have one")))
+        } else {
+            if let Some(time) = time {
+                return Err(anyhow!(
+                    "\
+No certification key found: --revocation-file not provided and the
+certificate to revoke does not contain a certification key with secret
+key material.  Perhaps this is because no certification keys are valid at
+the time you specified ({})",
+                    DateTime::<Utc>::from(time)
+                ));
+            } else {
+                return Err(anyhow!(
+                    "\
+No certification key found: --revocation-file not provided and the
+certificate to revoke does not contain a certification key with secret
+key material"
+                ));
+            }
+        }
+    }
+}

src/sq.rs

@@ -59,6 +59,7 @@ use clap::FromArgMatches;
 #[macro_use] mod macros;
 #[macro_use] mod log;
 
+mod common;
 
 mod sq_cli;
 use sq_cli::packet;
@@ -1328,10 +1329,6 @@ fn main() -> Result<()> {
             commands::key::dispatch(config, command)?
         }
 
-        SqSubcommands::Revoke(command) => {
-            commands::revoke::dispatch(config, command)?
-        }
-
         SqSubcommands::Wkd(command) => {
             commands::net::dispatch_wkd(config, command)?
         }

src/sq_cli/key.rs

@@ -2,7 +2,9 @@ use std::path::PathBuf;
 
 use clap::{ValueEnum, ArgGroup, Args, Parser, Subcommand};
 
-use sequoia_openpgp::cert::CipherSuite as SqCipherSuite;
+use sequoia_openpgp as openpgp;
+use openpgp::cert::CipherSuite as SqCipherSuite;
+use openpgp::types::ReasonForRevocation as OpenPGPRevocationReason;
 
 use crate::sq_cli::types::ClapData;
 use crate::sq_cli::types::FileOrStdin;
@@ -12,6 +14,41 @@ use crate::sq_cli::types::Time;
 use crate::sq_cli::KEY_VALIDITY_DURATION;
 use crate::sq_cli::KEY_VALIDITY_IN_YEARS;
 
+/// The revocation reason for a certificate or subkey
+#[derive(ValueEnum, Clone, Debug)]
+pub enum RevocationReason {
+    Compromised,
+    Superseded,
+    Retired,
+    Unspecified
+}
+
+impl From<RevocationReason> for OpenPGPRevocationReason {
+    fn from(rr: RevocationReason) -> Self {
+        match rr {
+            RevocationReason::Compromised => OpenPGPRevocationReason::KeyCompromised,
+            RevocationReason::Superseded => OpenPGPRevocationReason::KeySuperseded,
+            RevocationReason::Retired => OpenPGPRevocationReason::KeyRetired,
+            RevocationReason::Unspecified => OpenPGPRevocationReason::Unspecified,
+        }
+    }
+}
+
+/// The revocation reason for a UserID
+#[derive(ValueEnum, Clone, Debug)]
+pub enum UseridRevocationReason {
+    Retired,
+    Unspecified
+}
+
+impl From<UseridRevocationReason> for OpenPGPRevocationReason {
+    fn from(rr: UseridRevocationReason) -> Self {
+        match rr {
+            UseridRevocationReason::Retired => OpenPGPRevocationReason::UIDRetired,
+            UseridRevocationReason::Unspecified => OpenPGPRevocationReason::Unspecified,
+        }
+    }
+}
 #[derive(Parser, Debug)]
 #[clap(
     name = "key",
@@ -39,6 +76,7 @@ pub struct Command {
 pub enum Subcommands {
     Generate(GenerateCommand),
     Password(PasswordCommand),
+    Revoke(RevokeCommand),
     #[clap(subcommand)]
     Userid(UseridCommand),
     #[clap(subcommand)]
@@ -279,6 +317,143 @@ pub struct PasswordCommand {
     pub binary: bool,
 }
 
+#[derive(Debug, Args)]
+#[clap(
+    about = "Revoke a certificate",
+    long_about =
+"Revokes a certificate
+
+Creates a revocation certificate for the certificate.
+
+If \"--revocation-file\" is provided, then that key is used to create
+the signature.  If that key is different from the certificate being
+revoked, this creates a third-party revocation.  This is normally only
+useful if the owner of the certificate designated the key to be a
+designated revoker.
+
+If \"--revocation-file\" is not provided, then the certificate must
+include a certification-capable key.
+
+\"sq key revoke\" respects the reference time set by the top-level \
+\"--time\" argument.  When set, it uses the specified time instead of \
+the current time, when determining what keys are valid, and it sets \
+the revocation certificate's creation time to the reference time \
+instead of the current time.
+",
+)]
+pub struct RevokeCommand {
+    #[clap(
+        value_name = "FILE",
+        long = "certificate-file",
+        alias = "cert-file",
+        help = "The certificate to revoke",
+        long_help =
+"Reads the certificate to revoke from FILE or stdin, if omitted.  It is \
+an error for the file to contain more than one certificate.",
+    )]
+    pub input: Option<PathBuf>,
+
+    #[clap(
+        long = "revocation-file",
+        value_name = "KEY_FILE",
+        help = "Signs the revocation certificate using the key in KEY_FILE",
+        long_help =
+"Signs the revocation certificate using the key in KEY_FILE.  If the key is \
+different from the certificate, this creates a third-party revocation.  If \
+this option is not provided, and the certificate includes secret key material, \
+then that key is used to sign the revocation certificate.",
+    )]
+    pub secret_key_file: Option<PathBuf>,
+
+    #[clap(
+        long = "private-key-store",
+        value_name = "KEY_STORE",
+        help = "Provides parameters for private key store",
+    )]
+    pub private_key_store: Option<String>,
+
+    #[clap(
+        value_name = "REASON",
+        required = true,
+        help = "The reason for the revocation",
+        long_help =
+"The reason for the revocation.  This must be either: compromised,
+superseded, retired, or unspecified:
+
+  - compromised means that the secret key material may have been
+    compromised.  Prefer this value if you suspect that the secret
+    key has been leaked.
+
+  - superseded means that the owner of the certificate has replaced
+    it with a new certificate.  Prefer \"compromised\" if the secret
+    key material has been compromised even if the certificate is also
+    being replaced!  You should include the fingerprint of the new
+    certificate in the message.
+
+  - retired means that this certificate should not be used anymore,
+    and there is no replacement.  This is appropriate when someone
+    leaves an organisation.  Prefer \"compromised\" if the secret key
+    material has been compromised even if the certificate is also
+    being retired!  You should include how to contact the owner, or
+    who to contact instead in the message.
+
+  - unspecified means that none of the three other three reasons
+    apply.  OpenPGP implementations conservatively treat this type
+    of revocation similar to a compromised key.
+
+If the reason happened in the past, you should specify that using the
+--time argument.  This allows OpenPGP implementations to more
+accurately reason about objects whose validity depends on the validity
+of the certificate.",
+    value_enum,
+    )]
+    pub reason: RevocationReason,
+
+    #[clap(
+        value_name = "MESSAGE",
+        help = "A short, explanatory text",
+        long_help =
+"A short, explanatory text that is shown to a viewer of the revocation \
+certificate.  It explains why the certificate has been revoked.  For \
+instance, if Alice has created a new key, she would generate a \
+'superseded' revocation certificate for her old key, and might include \
+the message \"I've created a new certificate, FINGERPRINT, please use \
+that in the future.\"",
+    )]
+    pub message: String,
+
+    #[clap(
+        long,
+        value_names = &["NAME", "VALUE"],
+        number_of_values = 2,
+        help = "Adds a notation to the certification.",
+        long_help = "Adds a notation to the certification.  \
+            A user-defined notation's name must be of the form \
+            \"name@a.domain.you.control.org\". If the notation's name starts \
+            with a !, then the notation is marked as being critical.  If a \
+            consumer of a signature doesn't understand a critical notation, \
+            then it will ignore the signature.  The notation is marked as \
+            being human readable."
+    )]
+    pub notation: Vec<String>,
+
+    #[clap(
+        default_value_t = FileOrStdout::default(),
+        help = FileOrStdout::HELP,
+        long,
+        short,
+        value_name = FileOrStdout::VALUE_NAME,
+    )]
+    pub output: FileOrStdout,
+
+    #[clap(
+        short = 'B',
+        long,
+        help = "Emits binary data",
+    )]
+    pub binary: bool,
+}
+
 #[derive(Debug, Args)]
 #[clap(
     name = "extract-cert",
@@ -337,6 +512,7 @@ Add User IDs to, or strip User IDs from a key.
 )]
 pub enum UseridCommand {
     Add(UseridAddCommand),
+    Revoke(UseridRevokeCommand),
     Strip(UseridStripCommand),
 }
 
@@ -406,6 +582,139 @@ pub struct UseridAddCommand {
     pub binary: bool,
 }
 
+#[derive(Debug, Args)]
+#[clap(
+    about = "Revoke a User ID",
+    long_about =
+"Revokes a User ID
+
+Creates a revocation certificate for a User ID.
+
+If \"--revocation-key\" is provided, then that key is used to create \
+the signature.  If that key is different from the certificate being \
+revoked, this creates a third-party revocation.  This is normally only \
+useful if the owner of the certificate designated the key to be a \
+designated revoker.
+
+If \"--revocation-key\" is not provided, then the certificate must \
+include a certification-capable key.
+
+\"sq key userid revoke\" respects the reference time set by the top-level \
+\"--time\" argument.  When set, it uses the specified time instead of \
+the current time, when determining what keys are valid, and it sets \
+the revocation certificate's creation time to the reference time \
+instead of the current time.
+",)]
+pub struct UseridRevokeCommand {
+    #[clap(
+        value_name = "CERT_FILE",
+        long = "certificate-file",
+        alias = "cert-file",
+        help = "The certificate containing the User ID to revoke",
+        long_help =
+"Reads the certificate to revoke from CERT_FILE or stdin, \
+if omitted.  It is an error for the file to contain more than one \
+certificate."
+    )]
+    pub input: Option<PathBuf>,
+
+    #[clap(
+        long = "revocation-file",
+        value_name = "KEY_FILE",
+        help = "Signs the revocation certificate using the key in KEY_FILE",
+        long_help =
+"Signs the revocation certificate using the key in KEY_FILE.  If the key is \
+different from the certificate, this creates a third-party revocation.  If \
+this option is not provided, and the certificate includes secret key material, \
+then that key is used to sign the revocation certificate.",
+    )]
+    pub secret_key_file: Option<PathBuf>,
+
+    #[clap(
+        long = "private-key-store",
+        value_name = "KEY_STORE",
+        help = "Provides parameters for private key store",
+    )]
+    pub private_key_store: Option<String>,
+
+    #[clap(
+        value_name = "USERID",
+        help = "The User ID to revoke",
+        long_help =
+"The User ID to revoke.  By default, this must exactly match a \
+self-signed User ID.  Use --force to generate a revocation certificate \
+for a User ID, which is not self signed."
+    )]
+    pub userid: String,
+
+    #[clap(
+        value_enum,
+        value_name = "REASON",
+        help = "The reason for the revocation",
+        long_help =
+"The reason for the revocation.  This must be either: retired, or \
+unspecified:
+
+  - retired means that this User ID is no longer valid.  This is
+    appropriate when someone leaves an organisation, and the
+    organisation does not have their secret key material.  For
+    instance, if someone was part of Debian and retires, they would
+    use this to indicate that a Debian-specific User ID is no longer
+    valid.
+
+  - unspecified means that a different reason applies.
+
+If the reason happened in the past, you should specify that using the \
+--time argument.  This allows OpenPGP implementations to more \
+accurately reason about objects whose validity depends on the validity \
+of a User ID."
+    )]
+    pub reason: UseridRevocationReason,
+
+    #[clap(
+        value_name = "MESSAGE",
+        help = "A short, explanatory text",
+        long_help =
+"A short, explanatory text that is shown to a viewer of the revocation \
+certificate.  It explains why the certificate has been revoked.  For \
+instance, if Alice has created a new key, she would generate a \
+'superseded' revocation certificate for her old key, and might include \
+the message \"I've created a new certificate, FINGERPRINT, please use \
+that in the future.\"",
+    )]
+    pub message: String,
+
+    #[clap(
+        long,
+        value_names = &["NAME", "VALUE"],
+        number_of_values = 2,
+        help = "Adds a notation to the certification.",
+        long_help = "Adds a notation to the certification.  \
+            A user-defined notation's name must be of the form \
+            \"name@a.domain.you.control.org\". If the notation's name starts \
+            with a !, then the notation is marked as being critical.  If a \
+            consumer of a signature doesn't understand a critical notation, \
+            then it will ignore the signature.  The notation is marked as \
+            being human readable."
+    )]
+    pub notation: Vec<String>,
+
+    #[clap(
+        default_value_t = FileOrStdout::default(),
+        help = FileOrStdout::HELP,
+        long,
+        short,
+        value_name = FileOrStdout::VALUE_NAME,
+    )]
+    pub output: FileOrStdout,
+
+    #[clap(
+        short = 'B',
+        long,
+        help = "Emits binary data",
+    )]
+    pub binary: bool,
+}
 
 #[derive(Debug, Args)]
 #[clap(
@@ -420,7 +729,7 @@ to its local copy of that certificate.  Systems that have obtained
 a copy of your certificate with the User ID that you are trying to
 strip will not drop that User ID from their copy.)
 
-In most cases, you will want to use the 'sq revoke userid' operation
+In most cases, you will want to use the 'sq key userid revoke' operation
 instead.  That issues a revocation for a User ID, which can be used to mark
 the User ID as invalidated.
 
@@ -619,6 +928,7 @@ Add new subkeys to an existing key.
 #[non_exhaustive]
 pub enum SubkeyCommand {
     Add(SubkeyAddCommand),
+    Revoke(SubkeyRevokeCommand),
 }
 
 #[derive(Debug, Args)]
@@ -750,3 +1060,151 @@ pub struct SubkeyAddCommand {
     )]
     pub can_encrypt: Option<EncryptPurpose>,
 }
+
+#[derive(Debug, Args)]
+#[clap(
+    about = "Revoke a subkey",
+    long_about =
+"Revokes a subkey
+
+Creates a revocation certificate for a subkey.
+
+If \"--revocation-file\" is provided, then that key is used to \
+create the signature.  If that key is different from the certificate \
+being revoked, this creates a third-party revocation.  This is \
+normally only useful if the owner of the certificate designated the \
+key to be a designated revoker.
+
+If \"--revocation-file\" is not provided, then the certificate \
+must include a certification-capable key.
+
+\"sq key subkey revoke\" respects the reference time set by the top-level \
+\"--time\" argument.  When set, it uses the specified time instead of \
+the current time, when determining what keys are valid, and it sets \
+the revocation certificate's creation time to the reference time \
+instead of the current time.
+",
+)]
+pub struct SubkeyRevokeCommand {
+    #[clap(
+        value_name = "FILE",
+        long = "certificate-file",
+        alias = "cert-file",
+        help = "The certificate containing the subkey to revoke",
+        long_help =
+"Reads the certificate containing the subkey to revoke from FILE or stdin, \
+if omitted.  It is an error for the file to contain more than one \
+certificate."
+    )]
+    pub input: Option<PathBuf>,
+
+    #[clap(
+        long = "revocation-file",
+        value_name = "KEY_FILE",
+        help = "Signs the revocation certificate using the key in KEY_FILE",
+        long_help =
+
+"Signs the revocation certificate using the key in KEY_FILE.  If the key \
+is different from the certificate, this creates a third-party revocation.  \
+If this option is not provided, and the certificate includes secret key \
+material, then that key is used to sign the revocation certificate.",
+    )]
+    pub secret_key_file: Option<PathBuf>,
+
+    #[clap(
+        long = "private-key-store",
+        value_name = "KEY_STORE",
+        help = "Provides parameters for private key store",
+    )]
+    pub private_key_store: Option<String>,
+
+    #[clap(
+        value_name = "SUBKEY",
+        help = "The subkey to revoke",
+        long_help =
+"The subkey to revoke.  This must either be the subkey's Key ID or its \
+fingerprint.",
+    )]
+    pub subkey: String,
+
+    #[clap(
+        value_name = "REASON",
+        required = true,
+        help = "The reason for the revocation",
+        long_help =
+"The reason for the revocation.  This must be either: compromised, \
+superseded, retired, or unspecified:
+
+  - compromised means that the secret key material may have been
+    compromised.  Prefer this value if you suspect that the secret
+    key has been leaked.
+
+  - superseded means that the owner of the certificate has replaced
+    it with a new certificate.  Prefer \"compromised\" if the secret
+    key material has been compromised even if the certificate is
+    also being replaced!  You should include the fingerprint of the
+    new certificate in the message.
+
+  - retired means that this certificate should not be used anymore,
+    and there is no replacement.  This is appropriate when someone
+    leaves an organisation.  Prefer \"compromised\" if the secret key
+    material has been compromised even if the certificate is also
+    being retired!  You should include how to contact the owner, or
+    who to contact instead in the message.
+
+  - unspecified means that none of the three other three reasons
+    apply.  OpenPGP implementations conservatively treat this type
+    of revocation similar to a compromised key.
+
+If the reason happened in the past, you should specify that using the \
+--time argument.  This allows OpenPGP implementations to more \
+accurately reason about objects whose validity depends on the validity \
+of the certificate.",
+    value_enum,
+    )]
+    pub reason: RevocationReason,
+
+    #[clap(
+        value_name = "MESSAGE",
+        help = "A short, explanatory text",
+        long_help =
+"A short, explanatory text that is shown to a viewer of the revocation \
+certificate.  It explains why the subkey has been revoked.  For \
+instance, if Alice has created a new key, she would generate a \
+'superseded' revocation certificate for her old key, and might include \
+the message \"I've created a new subkey, please refresh the certificate."
+    )]
+    pub message: String,
+
+    #[clap(
+        long,
+        value_names = &["NAME", "VALUE"],
+        number_of_values = 2,
+        help = "Adds a notation to the certification.",
+        long_help = "Adds a notation to the certification.  \
+            A user-defined notation's name must be of the form \
+            \"name@a.domain.you.control.org\". If the notation's name starts \
+            with a !, then the notation is marked as being critical.  If a \
+            consumer of a signature doesn't understand a critical notation, \
+            then it will ignore the signature.  The notation is marked as \
+            being human readable."
+    )]
+    pub notation: Vec<String>,
+
+    #[clap(
+        default_value_t = FileOrStdout::default(),
+        help = FileOrStdout::HELP,
+        long,
+        short,
+        value_name = FileOrStdout::VALUE_NAME,
+    )]
+    pub output: FileOrStdout,
+
+    #[clap(
+        short = 'B',
+        long,
+        help = "Emits binary data",
+    )]
+    pub binary: bool,
+}
+

src/sq_cli/mod.rs

@@ -25,7 +25,6 @@ pub mod keyserver;
 pub mod link;
 mod output_versions;
 pub mod packet;
-pub mod revoke;
 mod sign;
 mod verify;
 pub mod wkd;
@@ -251,7 +250,5 @@ pub enum SqSubcommands {
     Inspect(inspect::Command),
     Packet(packet::Command),
 
-    Revoke(revoke::Command),
-
     OutputVersions(output_versions::Command),
 }

src/sq_cli/revoke.rs

@@ -1,467 +0,0 @@
-use std::path::PathBuf;
-
-use clap::Parser;
-use clap::{ValueEnum, Args, Subcommand};
-
-use sequoia_openpgp as openpgp;
-
-#[derive(Parser, Debug)]
-#[clap(
-    name = "revoke",
-    about = "Generates revocation certificates",
-    long_about = "Generates revocation certificates.
-
-A revocation certificate indicates that a certificate, a subkey, a
-User ID, or a signature should not be used anymore.
-
-A revocation certificate includes two fields, a type and a
-human-readable explanation, which allows the issuer to indicate why
-the revocation certificate was issued.  It is important to set the
-type field accurately as this allows an OpenPGP implementation to
-better reason about artifacts whose validity relies on the revoked
-object.  For instance, if a certificate is retired, it is reasonable
-to consider signatures that it made prior to its retirement as still
-being valid.  However, if a certificate's secret key material is
-compromised, any signatures that it made should be considered
-potentially forged, as they could have been made by an attacker and
-backdated.
-
-As the intent of a revocation certificate is to stop others from using
-a certificate, it is necessary to distribute the revocation
-certificate.  One effective way to do this is to upload the revocation
-certificate to a keyserver.
-",
-    after_help =
-"EXAMPLES:
-
-# Revoke a certificate.
-$ sq revoke certificate --time 20220101 --cert-file juliet.pgp \\
-  compromised \"My parents went through my things, and found my backup.\"
-
-# Revoke a User ID.
-$ sq revoke userid --time 20220101 --cert-file juliet.pgp \\
-  \"Juliet <juliet@capuleti.it>\" retired \"I've left the family.\"
-",
-    subcommand_required = true,
-    arg_required_else_help = true,
-)]
-pub struct Command {
-    #[clap(subcommand)]
-    pub subcommand: Subcommands,
-}
-
-#[derive(Debug, Subcommand)]
-pub enum Subcommands {
-    Certificate(CertificateCommand),
-    Subkey(SubkeyCommand),
-    Userid(UseridCommand),
-}
-
-#[derive(Debug, Args)]
-#[clap(
-    about = "Revoke a certificate",
-    long_about =
-"Revokes a certificate
-
-Creates a revocation certificate for the certificate.
-
-If \"--revocation-file\" is provided, then that key is used to create
-the signature.  If that key is different from the certificate being
-revoked, this creates a third-party revocation.  This is normally only
-useful if the owner of the certificate designated the key to be a
-designated revoker.
-
-If \"--revocation-file\" is not provided, then the certificate must
-include a certification-capable key.
-
-\"sq revoke\" respects the reference time set by the top-level \
-\"--time\" argument.  When set, it uses the specified time instead of \
-the current time, when determining what keys are valid, and it sets \
-the revocation certificate's creation time to the reference time \
-instead of the current time.
-",
-)]
-pub struct CertificateCommand {
-    #[clap(
-        value_name = "FILE",
-        long = "certificate-file",
-        alias = "cert-file",
-        help = "The certificate to revoke",
-        long_help =
-"Reads the certificate to revoke from FILE or stdin, if omitted.  It is \
-an error for the file to contain more than one certificate.",
-    )]
-    pub input: Option<PathBuf>,
-    #[clap(
-        long = "revocation-file",
-        value_name = "KEY_FILE",
-        help = "Signs the revocation certificate using the key in KEY_FILE",
-        long_help =
-"Signs the revocation certificate using the key in KEY_FILE.  If the key is \
-different from the certificate, this creates a third-party revocation.  If \
-this option is not provided, and the certificate includes secret key material, \
-then that key is used to sign the revocation certificate.",
-    )]
-    pub secret_key_file: Option<PathBuf>,
-    #[clap(
-        long = "private-key-store",
-        value_name = "KEY_STORE",
-        help = "Provides parameters for private key store",
-    )]
-    pub private_key_store: Option<String>,
-
-    #[clap(
-        value_name = "REASON",
-        required = true,
-        help = "The reason for the revocation",
-        long_help =
-"The reason for the revocation.  This must be either: compromised,
-superseded, retired, or unspecified:
-
-  - compromised means that the secret key material may have been
-    compromised.  Prefer this value if you suspect that the secret
-    key has been leaked.
-
-  - superseded means that the owner of the certificate has replaced
-    it with a new certificate.  Prefer \"compromised\" if the secret
-    key material has been compromised even if the certificate is also
-    being replaced!  You should include the fingerprint of the new
-    certificate in the message.
-
-  - retired means that this certificate should not be used anymore,
-    and there is no replacement.  This is appropriate when someone
-    leaves an organisation.  Prefer \"compromised\" if the secret key
-    material has been compromised even if the certificate is also
-    being retired!  You should include how to contact the owner, or
-    who to contact instead in the message.
-
-  - unspecified means that none of the three other three reasons
-    apply.  OpenPGP implementations conservatively treat this type
-    of revocation similar to a compromised key.
-
-If the reason happened in the past, you should specify that using the
---time argument.  This allows OpenPGP implementations to more
-accurately reason about objects whose validity depends on the validity
-of the certificate.",
-    value_enum,
-    )]
-    pub reason: RevocationReason,
-
-    #[clap(
-        value_name = "MESSAGE",
-        help = "A short, explanatory text",
-        long_help =
-"A short, explanatory text that is shown to a viewer of the revocation \
-certificate.  It explains why the certificate has been revoked.  For \
-instance, if Alice has created a new key, she would generate a \
-'superseded' revocation certificate for her old key, and might include \
-the message \"I've created a new certificate, FINGERPRINT, please use \
-that in the future.\"",
-    )]
-    pub message: String,
-    #[clap(
-        long,
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Adds a notation to the certification.",
-        long_help = "Adds a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            \"name@a.domain.you.control.org\". If the notation's name starts \
-            with a !, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
-    #[clap(
-        short = 'B',
-        long,
-        help = "Emits binary data",
-    )]
-    pub binary: bool,
-}
-
-#[derive(ValueEnum, Clone, Debug)]
-pub enum RevocationReason {
-    Compromised,
-    Superseded,
-    Retired,
-    Unspecified
-}
-
-use openpgp::types::ReasonForRevocation as OpenPGPRevocationReason;
-impl From<RevocationReason> for OpenPGPRevocationReason {
-    fn from(rr: RevocationReason) -> Self {
-        match rr {
-            RevocationReason::Compromised => OpenPGPRevocationReason::KeyCompromised,
-            RevocationReason::Superseded => OpenPGPRevocationReason::KeySuperseded,
-            RevocationReason::Retired => OpenPGPRevocationReason::KeyRetired,
-            RevocationReason::Unspecified => OpenPGPRevocationReason::Unspecified,
-        }
-    }
-}
-
-#[derive(Debug, Args)]
-#[clap(
-    about = "Revoke a subkey",
-    long_about =
-"Revokes a subkey
-
-Creates a revocation certificate for a subkey.
-
-If \"--revocation-file\" is provided, then that key is used to \
-create the signature.  If that key is different from the certificate \
-being revoked, this creates a third-party revocation.  This is \
-normally only useful if the owner of the certificate designated the \
-key to be a designated revoker.
-
-If \"--revocation-file\" is not provided, then the certificate \
-must include a certification-capable key.
-
-\"sq revoke subkey\" respects the reference time set by the top-level \
-\"--time\" argument.  When set, it uses the specified time instead of \
-the current time, when determining what keys are valid, and it sets \
-the revocation certificate's creation time to the reference time \
-instead of the current time.
-",
-)]
-pub struct SubkeyCommand {
-    #[clap(
-        value_name = "FILE",
-        long = "certificate-file",
-        alias = "cert-file",
-        help = "The certificate containing the subkey to revoke",
-        long_help =
-"Reads the certificate containing the subkey to revoke from FILE or stdin, \
-if omitted.  It is an error for the file to contain more than one \
-certificate."
-    )]
-    pub input: Option<PathBuf>,
-    #[clap(
-        long = "revocation-file",
-        value_name = "KEY_FILE",
-        help = "Signs the revocation certificate using the key in KEY_FILE",
-        long_help =
-
-"Signs the revocation certificate using the key in KEY_FILE.  If the key \
-is different from the certificate, this creates a third-party revocation.  \
-If this option is not provided, and the certificate includes secret key \
-material, then that key is used to sign the revocation certificate.",
-    )]
-    pub secret_key_file: Option<PathBuf>,
-    #[clap(
-        long = "private-key-store",
-        value_name = "KEY_STORE",
-        help = "Provides parameters for private key store",
-    )]
-    pub private_key_store: Option<String>,
-    #[clap(
-        value_name = "SUBKEY",
-        help = "The subkey to revoke",
-        long_help =
-"The subkey to revoke.  This must either be the subkey's Key ID or its \
-fingerprint.",
-    )]
-    pub subkey: String,
-
-    #[clap(
-        value_name = "REASON",
-        required = true,
-        help = "The reason for the revocation",
-        long_help =
-"The reason for the revocation.  This must be either: compromised, \
-superseded, retired, or unspecified:
-
-  - compromised means that the secret key material may have been
-    compromised.  Prefer this value if you suspect that the secret
-    key has been leaked.
-
-  - superseded means that the owner of the certificate has replaced
-    it with a new certificate.  Prefer \"compromised\" if the secret
-    key material has been compromised even if the certificate is
-    also being replaced!  You should include the fingerprint of the
-    new certificate in the message.
-
-  - retired means that this certificate should not be used anymore,
-    and there is no replacement.  This is appropriate when someone
-    leaves an organisation.  Prefer \"compromised\" if the secret key
-    material has been compromised even if the certificate is also
-    being retired!  You should include how to contact the owner, or
-    who to contact instead in the message.
-
-  - unspecified means that none of the three other three reasons
-    apply.  OpenPGP implementations conservatively treat this type
-    of revocation similar to a compromised key.
-
-If the reason happened in the past, you should specify that using the \
---time argument.  This allows OpenPGP implementations to more \
-accurately reason about objects whose validity depends on the validity \
-of the certificate.",
-    value_enum,
-    )]
-    pub reason: RevocationReason,
-    #[clap(
-        value_name = "MESSAGE",
-        help = "A short, explanatory text",
-        long_help =
-"A short, explanatory text that is shown to a viewer of the revocation \
-certificate.  It explains why the subkey has been revoked.  For \
-instance, if Alice has created a new key, she would generate a \
-'superseded' revocation certificate for her old key, and might include \
-the message \"I've created a new subkey, please refresh the certificate."
-    )]
-    pub message: String,
-    #[clap(
-        long,
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Adds a notation to the certification.",
-        long_help = "Adds a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            \"name@a.domain.you.control.org\". If the notation's name starts \
-            with a !, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
-    #[clap(
-        short = 'B',
-        long,
-        help = "Emits binary data",
-    )]
-    pub binary: bool,
-}
-
-#[derive(Debug, Args)]
-#[clap(
-    about = "Revoke a User ID",
-    long_about =
-"Revokes a User ID
-
-Creates a revocation certificate for a User ID.
-
-If \"--revocation-key\" is provided, then that key is used to create \
-the signature.  If that key is different from the certificate being \
-revoked, this creates a third-party revocation.  This is normally only \
-useful if the owner of the certificate designated the key to be a \
-designated revoker.
-
-If \"--revocation-key\" is not provided, then the certificate must \
-include a certification-capable key.
-
-\"sq revoke userid\" respects the reference time set by the top-level \
-\"--time\" argument.  When set, it uses the specified time instead of \
-the current time, when determining what keys are valid, and it sets \
-the revocation certificate's creation time to the reference time \
-instead of the current time.
-",)]
-pub struct UseridCommand {
-    #[clap(
-        value_name = "CERT_FILE",
-        long = "certificate-file",
-        alias = "cert-file",
-        help = "The certificate containing the User ID to revoke",
-        long_help =
-"Reads the certificate to revoke from CERT_FILE or stdin, \
-if omitted.  It is an error for the file to contain more than one \
-certificate."
-    )]
-    pub input: Option<PathBuf>,
-    #[clap(
-        long = "revocation-file",
-        value_name = "KEY_FILE",
-        help = "Signs the revocation certificate using the key in KEY_FILE",
-        long_help =
-"Signs the revocation certificate using the key in KEY_FILE.  If the key is \
-different from the certificate, this creates a third-party revocation.  If \
-this option is not provided, and the certificate includes secret key material, \
-then that key is used to sign the revocation certificate.",
-    )]
-    pub secret_key_file: Option<PathBuf>,
-    #[clap(
-        long = "private-key-store",
-        value_name = "KEY_STORE",
-        help = "Provides parameters for private key store",
-    )]
-    pub private_key_store: Option<String>,
-    #[clap(
-        value_name = "USERID",
-        help = "The User ID to revoke",
-        long_help =
-"The User ID to revoke.  By default, this must exactly match a \
-self-signed User ID.  Use --force to generate a revocation certificate \
-for a User ID, which is not self signed."
-    )]
-    pub userid: String,
-    #[clap(
-        value_enum,
-        value_name = "REASON",
-        help = "The reason for the revocation",
-        long_help =
-"The reason for the revocation.  This must be either: retired, or \
-unspecified:
-
-  - retired means that this User ID is no longer valid.  This is
-    appropriate when someone leaves an organisation, and the
-    organisation does not have their secret key material.  For
-    instance, if someone was part of Debian and retires, they would
-    use this to indicate that a Debian-specific User ID is no longer
-    valid.
-
-  - unspecified means that a different reason applies.
-
-If the reason happened in the past, you should specify that using the \
---time argument.  This allows OpenPGP implementations to more \
-accurately reason about objects whose validity depends on the validity \
-of a User ID."
-    )]
-    pub reason: UseridRevocationReason,
-    #[clap(
-        value_name = "MESSAGE",
-        help = "A short, explanatory text",
-        long_help =
-"A short, explanatory text that is shown to a viewer of the revocation \
-certificate.  It explains why the certificate has been revoked.  For \
-instance, if Alice has created a new key, she would generate a \
-'superseded' revocation certificate for her old key, and might include \
-the message \"I've created a new certificate, FINGERPRINT, please use \
-that in the future.\"",
-    )]
-    pub message: String,
-    #[clap(
-        long,
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Adds a notation to the certification.",
-        long_help = "Adds a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            \"name@a.domain.you.control.org\". If the notation's name starts \
-            with a !, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
-    #[clap(
-        short = 'B',
-        long,
-        help = "Emits binary data",
-    )]
-    pub binary: bool,
-}
-
-#[derive(ValueEnum, Clone, Debug)]
-pub enum UseridRevocationReason {
-    Retired,
-    Unspecified
-}
-
-impl From<UseridRevocationReason> for OpenPGPRevocationReason {
-    fn from(rr: UseridRevocationReason) -> Self {
-        match rr {
-            UseridRevocationReason::Retired => OpenPGPRevocationReason::UIDRetired,
-            UseridRevocationReason::Unspecified => OpenPGPRevocationReason::Unspecified,
-        }
-    }
-}

tests/common/mod.rs

@@ -0,0 +1,122 @@
+use std::path::PathBuf;
+
+use anyhow::anyhow;
+use assert_cmd::Command;
+
+use chrono::DateTime;
+use chrono::Duration;
+use chrono::Utc;
+
+use openpgp::packet::Signature;
+use openpgp::parse::Parse;
+use openpgp::policy::StandardPolicy;
+use openpgp::Cert;
+use openpgp::Result;
+use sequoia_openpgp as openpgp;
+
+use tempfile::TempDir;
+
+pub const STANDARD_POLICY: &StandardPolicy = &StandardPolicy::new();
+
+/// Generate a new key in a temporary directory and return its TempDir,
+/// PathBuf and creation time in a Result
+pub fn sq_key_generate(
+    userids: Option<&[&str]>,
+) -> Result<(TempDir, PathBuf, DateTime<Utc>)> {
+    let tmpdir = TempDir::new().unwrap();
+    let path = tmpdir.path().join("key.pgp");
+    let mut time = Utc::now();
+    // Round it down to a whole second to match the resolution of
+    // OpenPGP's timestamp.
+    time = time - Duration::nanoseconds(time.timestamp_subsec_nanos() as i64);
+    let userids = if let Some(userids) = userids {
+        userids
+    } else {
+        &["alice <alice@example.org>"]
+    };
+
+    let mut cmd = Command::cargo_bin("sq")?;
+    cmd.args([
+        "--no-cert-store",
+        "key",
+        "generate",
+        "--time",
+        &time.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
+        "--expiry",
+        "never",
+        "--output",
+        &*path.to_string_lossy(),
+    ]);
+    for userid in userids {
+        cmd.args(["--userid", userid]);
+    }
+    cmd.assert().success();
+
+    let original_cert = Cert::from_file(&path)?;
+    let original_valid_cert =
+        original_cert.with_policy(STANDARD_POLICY, None)?;
+    assert_eq!(
+        original_valid_cert
+            .keys()
+            .filter(|x| x.for_authentication())
+            .count(),
+        1
+    );
+    assert_eq!(
+        original_valid_cert
+            .keys()
+            .filter(|x| x.for_certification())
+            .count(),
+        1
+    );
+    assert_eq!(
+        original_valid_cert
+            .keys()
+            .filter(|x| x.for_signing())
+            .count(),
+        1
+    );
+    assert_eq!(
+        original_valid_cert
+            .keys()
+            .filter(|x| x.for_storage_encryption())
+            .count(),
+        1
+    );
+    assert_eq!(
+        original_valid_cert
+            .keys()
+            .filter(|x| x.for_transport_encryption())
+            .count(),
+        1
+    );
+
+    Ok((tmpdir, path, time))
+}
+
+/// Ensure notations can be found in a Signature
+///
+/// ## Errors
+///
+/// Returns an error if a notation can not be found in the Signature
+pub fn compare_notations(
+    signature: &Signature,
+    notations: Option<&[(&str, &str); 2]>,
+) -> Result<()> {
+    if let Some(notations) = notations {
+        let found_notations: Vec<(&str, String)> = signature
+            .notation_data()
+            .map(|n| (n.name(), String::from_utf8_lossy(n.value()).into()))
+            .collect();
+
+        for (key, value) in notations {
+            if !found_notations.contains(&(key, String::from(*value))) {
+                return Err(anyhow!(format!(
+                    "Expected notation \"{}: {}\" in {:?}",
+                    key, value, found_notations
+                )));
+            }
+        }
+    }
+    Ok(())
+}

tests/sq-key-revoke.rs

@@ -0,0 +1,362 @@
+use assert_cmd::Command;
+
+use chrono::Duration;
+use openpgp::parse::Parse;
+use openpgp::types::ReasonForRevocation;
+use openpgp::types::RevocationStatus;
+use openpgp::types::SignatureType;
+use openpgp::Cert;
+use openpgp::Packet;
+use openpgp::PacketPile;
+use openpgp::Result;
+use sequoia_openpgp as openpgp;
+
+mod common;
+use common::compare_notations;
+use common::sq_key_generate;
+use common::STANDARD_POLICY;
+
+#[test]
+fn sq_key_revoke() -> Result<()> {
+    let (tmpdir, path, time) = sq_key_generate(None)?;
+
+    let cert = Cert::from_file(&path)?;
+    let valid_cert = cert.with_policy(STANDARD_POLICY, Some(time.into()))?;
+    let fingerprint = &valid_cert.clone().fingerprint();
+
+    let message = "message";
+
+    // revoke for various reasons, with or without notations added, or with
+    // a revocation whose reference time is one hour after the creation of the
+    // certificate
+    for (reason, reason_str, notations, revocation_time) in [
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            None,
+            None,
+        ),
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            None,
+            Some(time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::KeyRetired, "retired", None, None),
+        (
+            ReasonForRevocation::KeyRetired,
+            "retired",
+            None,
+            Some(time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeyRetired,
+            "retired",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::KeySuperseded, "superseded", None, None),
+        (
+            ReasonForRevocation::KeySuperseded,
+            "superseded",
+            None,
+            Some(time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeySuperseded,
+            "superseded",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::Unspecified, "unspecified", None, None),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            None,
+            Some(time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+    ] {
+        let revocation = &path.parent().unwrap().join(format!(
+            "revocation_{}_{}_{}.rev",
+            reason_str,
+            if notations.is_some() {
+                "notations"
+            } else {
+                "no_notations"
+            },
+            if revocation_time.is_some() {
+                "time"
+            } else {
+                "no_time"
+            }
+        ));
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "revoke",
+            "--output",
+            &revocation.to_string_lossy(),
+            "--certificate-file",
+            &path.to_string_lossy(),
+            reason_str,
+            message,
+        ]);
+        if let Some(notations) = notations {
+            for (k, v) in notations {
+                cmd.args(["--notation", k, v]);
+            }
+        }
+        if let Some(time) = revocation_time {
+            cmd.args([
+                "--time",
+                &time.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
+            ]);
+        }
+        let output = cmd.output()?;
+        if !output.status.success() {
+            panic!(
+                "sq exited with non-zero status code: {}",
+                String::from_utf8(output.stderr)?
+            );
+        }
+
+        // We should get just a single signature packet.
+        let packet_pile = PacketPile::from_file(&revocation)?;
+
+        assert_eq!(
+            packet_pile.children().count(),
+            1,
+            "expected a single packet"
+        );
+
+        if let Some(Packet::Signature(sig)) = packet_pile.path_ref(&[0]) {
+            // the issuer is the certificate owner
+            assert_eq!(
+                sig.get_issuers().into_iter().next(),
+                Some(fingerprint.into())
+            );
+
+            let cert = Cert::from_file(&path)?;
+            let revoked_cert = cert.insert_packets(sig.clone()).unwrap();
+            let status = revoked_cert
+                .with_policy(STANDARD_POLICY, revocation_time.map(Into::into))
+                .unwrap()
+                .revocation_status();
+
+            println!("{:?}", sig);
+            println!("{:?}", status);
+            // Verify the revocation.
+            assert!(matches!(status, RevocationStatus::Revoked(_)));
+
+            // it is a key revocation
+            assert_eq!(sig.typ(), SignatureType::KeyRevocation);
+
+            // our reason for revocation and message matches
+            assert_eq!(
+                sig.reason_for_revocation(),
+                Some((reason, message.as_bytes()))
+            );
+
+            // the notations of the revocation match the ones
+            // we passed in
+            compare_notations(sig, notations)?;
+        } else {
+            panic!("Expected a signature, got: {:?}", packet_pile);
+        }
+    }
+
+    tmpdir.close()?;
+
+    Ok(())
+}
+
+#[test]
+fn sq_key_revoke_thirdparty() -> Result<()> {
+    let (tmpdir, path, _) = sq_key_generate(None)?;
+    let cert = Cert::from_file(&path)?;
+
+    let (thirdparty_tmpdir, thirdparty_path, thirdparty_time) =
+        sq_key_generate(Some(&["bob <bob@example.org"]))?;
+    let thirdparty_cert = Cert::from_file(&thirdparty_path)?;
+    let thirdparty_valid_cert = thirdparty_cert
+        .with_policy(STANDARD_POLICY, Some(thirdparty_time.into()))?;
+    let thirdparty_fingerprint = &thirdparty_valid_cert.clone().fingerprint();
+
+    let message = "message";
+
+    // revoke for various reasons, with or without notations added, or with
+    // a revocation whose reference time is one hour after the creation of the
+    // certificate
+    for (reason, reason_str, notations, revocation_time) in [
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            None,
+            None,
+        ),
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::KeyRetired, "retired", None, None),
+        (
+            ReasonForRevocation::KeyRetired,
+            "retired",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeyRetired,
+            "retired",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::KeySuperseded, "superseded", None, None),
+        (
+            ReasonForRevocation::KeySuperseded,
+            "superseded",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeySuperseded,
+            "superseded",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::Unspecified, "unspecified", None, None),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+    ] {
+        let revocation = &path.parent().unwrap().join(format!(
+            "revocation_{}_{}_{}.rev",
+            reason_str,
+            if notations.is_some() {
+                "notations"
+            } else {
+                "no_notations"
+            },
+            if revocation_time.is_some() {
+                "time"
+            } else {
+                "no_time"
+            }
+        ));
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "revoke",
+            "--output",
+            &revocation.to_string_lossy(),
+            "--certificate-file",
+            &path.to_string_lossy(),
+            "--revocation-file",
+            &thirdparty_path.to_string_lossy(),
+            reason_str,
+            message,
+        ]);
+        if let Some(notations) = notations {
+            for (k, v) in notations {
+                cmd.args(["--notation", k, v]);
+            }
+        }
+        if let Some(time) = revocation_time {
+            cmd.args([
+                "--time",
+                &time.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
+            ]);
+        }
+        let output = cmd.output()?;
+        if !output.status.success() {
+            panic!(
+                "sq exited with non-zero status code: {}",
+                String::from_utf8(output.stderr)?
+            );
+        }
+
+        // read revocation cert
+        let revocation_cert = Cert::from_file(&revocation)?;
+        let revocation_valid_cert = revocation_cert
+            .with_policy(STANDARD_POLICY, revocation_time.map(Into::into))?;
+
+        // evaluate revocation status
+        let status = revocation_valid_cert.revocation_status();
+        if let RevocationStatus::CouldBe(sigs) = status {
+            // there is only one signature packet
+            assert_eq!(sigs.len(), 1);
+            let sig = sigs.into_iter().next().unwrap();
+
+            // it is a key revocation
+            assert_eq!(sig.typ(), SignatureType::KeyRevocation);
+
+            // the issuer is a thirdparty revoker
+            assert_eq!(
+                sig.get_issuers().into_iter().next().as_ref(),
+                Some(&thirdparty_fingerprint.clone().into())
+            );
+
+            // the revocation can be verified
+            if sig
+                .clone()
+                .verify_primary_key_revocation(
+                    &thirdparty_cert.primary_key(),
+                    &cert.primary_key(),
+                )
+                .is_err()
+            {
+                panic!("revocation is not valid")
+            }
+
+            // our reason for revocation and message matches
+            assert_eq!(
+                sig.reason_for_revocation(),
+                Some((reason, message.as_bytes()))
+            );
+
+            // the notations of the revocation match the ones
+            // we passed in
+            compare_notations(sig, notations)?;
+        } else {
+            panic!("there are no signatures in {:?}", status);
+        }
+    }
+
+    tmpdir.close()?;
+    thirdparty_tmpdir.close()?;
+
+    Ok(())
+}

tests/sq-key-subkey.rs

@@ -1,173 +1,508 @@
 use assert_cmd::Command;
 
+use chrono::Duration;
+use openpgp::packet::Key;
 use openpgp::parse::Parse;
-use openpgp::policy::StandardPolicy;
+use openpgp::types::ReasonForRevocation;
+use openpgp::types::RevocationStatus;
+use openpgp::types::SignatureType;
 use openpgp::Cert;
 use openpgp::Result;
 use sequoia_openpgp as openpgp;
 
-mod integration {
+mod common;
-    use super::*;
+use common::compare_notations;
+use common::sq_key_generate;
+use common::STANDARD_POLICY;
 
-    use std::path::PathBuf;
+#[test]
-    use tempfile::TempDir;
+fn sq_key_subkey_generate_authentication_subkey() -> Result<()> {
+    let (tmpdir, path, _) = sq_key_generate(None).unwrap();
+    let output = path.parent().unwrap().join("new_key.pgp");
 
-    const P: &StandardPolicy = &StandardPolicy::new();
+    let mut cmd = Command::cargo_bin("sq")?;
+    cmd.args([
+        "--no-cert-store",
+        "key",
+        "subkey",
+        "add",
+        "--output",
+        &output.to_string_lossy(),
+        "--can-authenticate",
+        &path.to_string_lossy(),
+    ]);
+    cmd.assert().success();
 
-    /// Generate a new key in a temporary directory and return its TempDir,
+    let cert = Cert::from_file(&output)?;
-    /// PathBuf and creation times in a Result
+    let valid_cert = cert.with_policy(STANDARD_POLICY, None)?;
-    fn sq_key_generate() -> Result<(TempDir, PathBuf, String, u64)> {
-        let tmpdir = TempDir::new().unwrap();
-        let path = tmpdir.path().join("key.pgp");
-        let timestamp = "20220120T163236+0100";
-        let seconds = 1642692756;
 
-        let mut cmd = Command::cargo_bin("sq")?;
+    assert_eq!(
-        cmd.args([
+        valid_cert.keys().filter(|x| x.for_authentication()).count(),
-            "--no-cert-store",
+        2
-            "key",
+    );
-            "generate",
+    tmpdir.close()?;
-            "--time",
+    Ok(())
-            timestamp,
+}
-            "--expiry",
+
-            "never",
+#[test]
-            "--output",
+fn sq_key_subkey_generate_encryption_subkey() -> Result<()> {
-            &*path.to_string_lossy(),
+    let (tmpdir, path, _) = sq_key_generate(None).unwrap();
-        ]);
+    let output = path.parent().unwrap().join("new_key.pgp");
-        cmd.assert().success();
+
-
+    let mut cmd = Command::cargo_bin("sq")?;
-        let original_cert = Cert::from_file(&path)?;
+    cmd.args([
-        let original_valid_cert = original_cert.with_policy(P, None)?;
+        "--no-cert-store",
-        assert_eq!(
+        "key",
-            original_valid_cert
+        "subkey",
-                .keys()
+        "add",
-                .filter(|x| x.for_authentication())
+        "--output",
-                .count(),
+        &output.to_string_lossy(),
-            1
+        "--can-encrypt=universal",
-        );
+        &path.to_string_lossy(),
-        assert_eq!(
+    ]);
-            original_valid_cert
+    cmd.assert().success();
-                .keys()
+
-                .filter(|x| x.for_certification())
+    let cert = Cert::from_file(&output)?;
-                .count(),
+    let valid_cert = cert.with_policy(STANDARD_POLICY, None)?;
-            1
+
-        );
+    assert_eq!(
-        assert_eq!(
+        valid_cert
-            original_valid_cert
+            .keys()
-                .keys()
+            .filter(|x| x.for_storage_encryption())
-                .filter(|x| x.for_signing())
+            .count(),
-                .count(),
+        2
-            1
+    );
-        );
+    assert_eq!(
-        assert_eq!(
+        valid_cert
-            original_valid_cert
+            .keys()
-                .keys()
+            .filter(|x| x.for_transport_encryption())
-                .filter(|x| x.for_storage_encryption())
+            .count(),
-                .count(),
+        2
-            1
+    );
-        );
+    tmpdir.close()?;
-        assert_eq!(
+    Ok(())
-            original_valid_cert
+}
-                .keys()
+
-                .filter(|x| x.for_transport_encryption())
+#[test]
-                .count(),
+fn sq_key_subkey_generate_signing_subkey() -> Result<()> {
-            1
+    let (tmpdir, path, _) = sq_key_generate(None).unwrap();
-        );
+    let output = path.parent().unwrap().join("new_key.pgp");
-
+
-        Ok((tmpdir, path, timestamp.to_string(), seconds))
+    let mut cmd = Command::cargo_bin("sq")?;
-    }
+    cmd.args([
-
+        "--no-cert-store",
-    #[test]
+        "key",
-    fn sq_key_subkey_generate_authentication_subkey() -> Result<()> {
+        "subkey",
-        let (tmpdir, path, _, _) = sq_key_generate().unwrap();
+        "add",
-        let output = path.parent().unwrap().join("new_key.pgp");
+        "--output",
-
+        &output.to_string_lossy(),
-        let mut cmd = Command::cargo_bin("sq")?;
+        "--can-sign",
-        cmd.args([
+        &path.to_string_lossy(),
-            "--no-cert-store",
+    ]);
-            "key",
+    cmd.assert().success();
-            "subkey",
+
-            "add",
+    let cert = Cert::from_file(&output)?;
-            "--output",
+    let valid_cert = cert.with_policy(STANDARD_POLICY, None)?;
-            &output.to_string_lossy(),
+
-            "--can-authenticate",
+    assert_eq!(valid_cert.keys().filter(|x| x.for_signing()).count(), 2);
-            &path.to_string_lossy(),
+    tmpdir.close()?;
-        ]);
+    Ok(())
-        cmd.assert().success();
+}
-
+
-        let cert = Cert::from_file(&output)?;
+#[test]
-        let valid_cert = cert.with_policy(P, None)?;
+fn sq_key_subkey_revoke() -> Result<()> {
-
+    let (tmpdir, path, time) = sq_key_generate(None)?;
-        assert_eq!(
+
-            valid_cert.keys().filter(|x| x.for_authentication()).count(),
+    let cert = Cert::from_file(&path)?;
-            2
+    let valid_cert = cert.with_policy(STANDARD_POLICY, Some(time.into()))?;
-        );
+    let fingerprint = valid_cert.clone().fingerprint();
-        tmpdir.close()?;
+    let subkey: Key<_, _> = valid_cert
-        Ok(())
+        .with_policy(STANDARD_POLICY, Some(time.into()))
-    }
+        .unwrap()
-
+        .keys()
-    #[test]
+        .subkeys()
-    fn sq_key_subkey_generate_encryption_subkey() -> Result<()> {
+        .nth(0)
-        let (tmpdir, path, _, _) = sq_key_generate().unwrap();
+        .unwrap()
-        let output = path.parent().unwrap().join("new_key.pgp");
+        .key()
-
+        .clone();
-        let mut cmd = Command::cargo_bin("sq")?;
+    let subkey_fingerprint = subkey.fingerprint();
-        cmd.args([
+    let message = "message";
-            "--no-cert-store",
+
-            "key",
+    // revoke for various reasons, with or without notations added, or with
-            "subkey",
+    // a revocation whose reference time is one hour after the creation of the
-            "add",
+    // certificate
-            "--output",
+    for (reason, reason_str, notations, revocation_time) in [
-            &output.to_string_lossy(),
+        (
-            "--can-encrypt=universal",
+            ReasonForRevocation::KeyCompromised,
-            &path.to_string_lossy(),
+            "compromised",
-        ]);
+            None,
-        cmd.assert().success();
+            None,
-
+        ),
-        let cert = Cert::from_file(&output)?;
+        (
-        let valid_cert = cert.with_policy(P, None)?;
+            ReasonForRevocation::KeyCompromised,
-
+            "compromised",
-        assert_eq!(
+            None,
-            valid_cert
+            Some(time + Duration::hours(1)),
-                .keys()
+        ),
-                .filter(|x| x.for_storage_encryption())
+        (
-                .count(),
+            ReasonForRevocation::KeyCompromised,
-            2
+            "compromised",
-        );
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
-        assert_eq!(
+            None,
-            valid_cert
+        ),
-                .keys()
+        (ReasonForRevocation::KeyRetired, "retired", None, None),
-                .filter(|x| x.for_transport_encryption())
+        (
-                .count(),
+            ReasonForRevocation::KeyRetired,
-            2
+            "retired",
-        );
+            None,
-        tmpdir.close()?;
+            Some(time + Duration::hours(1)),
-        Ok(())
+        ),
-    }
+        (
-
+            ReasonForRevocation::KeyRetired,
-    #[test]
+            "retired",
-    fn sq_key_subkey_generate_signing_subkey() -> Result<()> {
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
-        let (tmpdir, path, _, _) = sq_key_generate().unwrap();
+            None,
-        let output = path.parent().unwrap().join("new_key.pgp");
+        ),
-
+        (ReasonForRevocation::KeySuperseded, "superseded", None, None),
-        let mut cmd = Command::cargo_bin("sq")?;
+        (
-        cmd.args([
+            ReasonForRevocation::KeySuperseded,
-            "--no-cert-store",
+            "superseded",
-            "key",
+            None,
-            "subkey",
+            Some(time + Duration::hours(1)),
-            "add",
+        ),
-            "--output",
+        (
-            &output.to_string_lossy(),
+            ReasonForRevocation::KeySuperseded,
-            "--can-sign",
+            "superseded",
-            &path.to_string_lossy(),
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
-        ]);
+            None,
-        cmd.assert().success();
+        ),
-
+        (ReasonForRevocation::Unspecified, "unspecified", None, None),
-        let cert = Cert::from_file(&output)?;
+        (
-        let valid_cert = cert.with_policy(P, None)?;
+            ReasonForRevocation::Unspecified,
-
+            "unspecified",
-        assert_eq!(valid_cert.keys().filter(|x| x.for_signing()).count(), 2);
+            None,
-        tmpdir.close()?;
+            Some(time + Duration::hours(1)),
-        Ok(())
+        ),
-    }
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+    ] {
+        let revocation = &path.parent().unwrap().join(format!(
+            "revocation_{}_{}_{}.rev",
+            reason_str,
+            if notations.is_some() {
+                "notations"
+            } else {
+                "no_notations"
+            },
+            if revocation_time.is_some() {
+                "time"
+            } else {
+                "no_time"
+            }
+        ));
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "subkey",
+            "revoke",
+            "--output",
+            &revocation.to_string_lossy(),
+            "--certificate-file",
+            &path.to_string_lossy(),
+            &subkey_fingerprint.to_string(),
+            reason_str,
+            message,
+        ]);
+        if let Some(notations) = notations {
+            for (k, v) in notations {
+                cmd.args(["--notation", k, v]);
+            }
+        }
+        if let Some(time) = revocation_time {
+            cmd.args([
+                "--time",
+                &time.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
+            ]);
+        }
+        let output = cmd.output()?;
+        if !output.status.success() {
+            panic!("sq exited with non-zero status code: {:?}", output.stderr);
+        }
+
+        // whether we found a revocation signature
+        let mut found_revoked = false;
+
+        // read revocation cert
+        let cert = Cert::from_file(&revocation)?;
+        let valid_cert =
+            cert.with_policy(STANDARD_POLICY, revocation_time.map(Into::into))?;
+        valid_cert
+            .with_policy(STANDARD_POLICY, revocation_time.map(Into::into))
+            .unwrap()
+            .keys()
+            .subkeys()
+            .for_each(|x| {
+                if x.fingerprint() == subkey_fingerprint {
+                    let status = x.revocation_status(
+                        STANDARD_POLICY,
+                        revocation_time.map(Into::into),
+                    );
+
+                    // the subkey is revoked
+                    assert!(matches!(status, RevocationStatus::Revoked(_)));
+
+                    if let RevocationStatus::Revoked(sigs) = status {
+                        // there is only one signature packet
+                        assert_eq!(sigs.len(), 1);
+                        let sig = sigs.into_iter().next().unwrap();
+
+                        // it is a subkey revocation
+                        assert_eq!(sig.typ(), SignatureType::SubkeyRevocation);
+
+                        // the issuer is the certificate owner
+                        assert_eq!(
+                            sig.get_issuers().into_iter().next().as_ref(),
+                            Some(&fingerprint.clone().into())
+                        );
+
+                        // our reason for revocation and message matches
+                        assert_eq!(
+                            sig.reason_for_revocation(),
+                            Some((reason, message.as_bytes()))
+                        );
+
+                        // the notations of the revocation match the ones
+                        // we passed in
+                        assert!(compare_notations(sig, notations).is_ok());
+
+                        found_revoked = true;
+                    }
+                }
+            });
+
+        if !found_revoked {
+            panic!("the revoked subkey is not found in the revocation cert");
+        }
+    }
+
+    tmpdir.close()?;
+
+    Ok(())
+}
+
+#[test]
+fn sq_key_subkey_revoke_thirdparty() -> Result<()> {
+    let (tmpdir, path, time) = sq_key_generate(None)?;
+    let (thirdparty_tmpdir, thirdparty_path, thirdparty_time) =
+        sq_key_generate(Some(&["bob <bob@example.org"]))?;
+
+    let cert = Cert::from_file(&path)?;
+    let valid_cert = cert.with_policy(STANDARD_POLICY, Some(time.into()))?;
+    let subkey: Key<_, _> = valid_cert
+        .with_policy(STANDARD_POLICY, Some(time.into()))
+        .unwrap()
+        .keys()
+        .subkeys()
+        .nth(0)
+        .unwrap()
+        .key()
+        .clone();
+    let subkey_fingerprint = subkey.fingerprint();
+
+    let thirdparty_cert = Cert::from_file(&thirdparty_path)?;
+    let thirdparty_valid_cert = thirdparty_cert
+        .with_policy(STANDARD_POLICY, Some(thirdparty_time.into()))?;
+    let thirdparty_fingerprint = thirdparty_valid_cert.clone().fingerprint();
+
+    let message = "message";
+
+    // revoke for various reasons, with or without notations added, or with
+    // a revocation whose reference time is one hour after the creation of the
+    // certificate
+    for (reason, reason_str, notations, revocation_time) in [
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            None,
+            None,
+        ),
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeyCompromised,
+            "compromised",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::KeyRetired, "retired", None, None),
+        (
+            ReasonForRevocation::KeyRetired,
+            "retired",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeyRetired,
+            "retired",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::KeySuperseded, "superseded", None, None),
+        (
+            ReasonForRevocation::KeySuperseded,
+            "superseded",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::KeySuperseded,
+            "superseded",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::Unspecified, "unspecified", None, None),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+    ] {
+        let revocation = &path.parent().unwrap().join(format!(
+            "revocation_{}_{}_{}.rev",
+            reason_str,
+            if notations.is_some() {
+                "notations"
+            } else {
+                "no_notations"
+            },
+            if revocation_time.is_some() {
+                "time"
+            } else {
+                "no_time"
+            }
+        ));
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "subkey",
+            "revoke",
+            "--output",
+            &revocation.to_string_lossy(),
+            "--certificate-file",
+            &path.to_string_lossy(),
+            "--revocation-file",
+            &thirdparty_path.to_string_lossy(),
+            &subkey_fingerprint.to_string(),
+            reason_str,
+            message,
+        ]);
+        if let Some(notations) = notations {
+            for (k, v) in notations {
+                cmd.args(["--notation", k, v]);
+            }
+        }
+        if let Some(time) = revocation_time {
+            cmd.args([
+                "--time",
+                &time.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
+            ]);
+        }
+        let output = cmd.output()?;
+        if !output.status.success() {
+            panic!("sq exited with non-zero status code: {:?}", output.stderr);
+        }
+
+        // whether we found a revocation signature
+        let mut found_revoked = false;
+
+        // read revocation cert
+        let cert = Cert::from_file(&revocation)?;
+        let valid_cert =
+            cert.with_policy(STANDARD_POLICY, revocation_time.map(Into::into))?;
+
+        assert_eq!(valid_cert.userids().count(), 1);
+        valid_cert
+            .with_policy(STANDARD_POLICY, revocation_time.map(Into::into))
+            .unwrap()
+            .keys()
+            .subkeys()
+            .for_each(|x| {
+                if x.fingerprint() == subkey_fingerprint {
+                    if let RevocationStatus::CouldBe(sigs) = x
+                        .revocation_status(
+                            STANDARD_POLICY,
+                            revocation_time.map(Into::into),
+                        )
+                    {
+                        // there is only one signature packet
+                        assert_eq!(sigs.len(), 1);
+                        let sig = sigs.into_iter().next().unwrap();
+
+                        // it is a subkey revocation
+                        assert_eq!(sig.typ(), SignatureType::SubkeyRevocation);
+
+                        // the issuer is a thirdparty revoker
+                        assert_eq!(
+                            sig.get_issuers().into_iter().next().as_ref(),
+                            Some(&thirdparty_fingerprint.clone().into())
+                        );
+
+                        // the revocation can be verified
+                        if sig
+                            .clone()
+                            .verify_subkey_revocation(
+                                &thirdparty_cert.primary_key(),
+                                &cert.primary_key(),
+                                &subkey,
+                            )
+                            .is_err()
+                        {
+                            panic!("revocation is not valid")
+                        }
+
+                        // our reason for revocation and message matches
+                        assert_eq!(
+                            sig.reason_for_revocation(),
+                            Some((reason, message.as_bytes()))
+                        );
+
+                        // the notations of the revocation match the ones
+                        // we passed in
+                        assert!(compare_notations(sig, notations).is_ok());
+
+                        found_revoked = true;
+                    } else {
+                        panic!("there are no signatures in {:?}", x);
+                    }
+                }
+            });
+
+        if !found_revoked {
+            panic!("the revoked subkey is not found in the revocation cert");
+        }
+    }
+
+    tmpdir.close()?;
+    thirdparty_tmpdir.close()?;
+
+    Ok(())
 }

tests/sq-key-userid.rs

@@ -0,0 +1,338 @@
+use assert_cmd::Command;
+
+use chrono::Duration;
+
+use openpgp::packet::UserID;
+use openpgp::parse::Parse;
+use openpgp::types::ReasonForRevocation;
+use openpgp::types::RevocationStatus;
+use openpgp::types::SignatureType;
+use openpgp::Cert;
+use openpgp::Result;
+use sequoia_openpgp as openpgp;
+
+mod common;
+use common::compare_notations;
+use common::sq_key_generate;
+use common::STANDARD_POLICY;
+
+#[test]
+fn sq_key_userid_revoke() -> Result<()> {
+    let userids = &["alice <alice@example.org>", "alice <alice@other.org>"];
+    // revoke the last userid
+    let userid_revoke = userids.last().unwrap();
+    let (tmpdir, path, time) = sq_key_generate(Some(userids))?;
+
+    let cert = Cert::from_file(&path)?;
+    let valid_cert = cert.with_policy(STANDARD_POLICY, Some(time.into()))?;
+    let fingerprint = valid_cert.clone().fingerprint();
+
+    let message = "message";
+
+    // revoke for various reasons, with or without notations added, or with
+    // a revocation whose reference time is one hour after the creation of the
+    // certificate
+    for (reason, reason_str, notations, revocation_time) in [
+        (ReasonForRevocation::UIDRetired, "retired", None, None),
+        (
+            ReasonForRevocation::UIDRetired,
+            "retired",
+            None,
+            Some(time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::UIDRetired,
+            "retired",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::Unspecified, "unspecified", None, None),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            None,
+            Some(time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+    ] {
+        let revocation = &path.parent().unwrap().join(format!(
+            "revocation_{}_{}_{}.rev",
+            reason_str,
+            if notations.is_some() {
+                "notations"
+            } else {
+                "no_notations"
+            },
+            if revocation_time.is_some() {
+                "time"
+            } else {
+                "no_time"
+            }
+        ));
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "userid",
+            "revoke",
+            "--output",
+            &revocation.to_string_lossy(),
+            "--certificate-file",
+            &path.to_string_lossy(),
+            userid_revoke,
+            reason_str,
+            message,
+        ]);
+        if let Some(notations) = notations {
+            for (k, v) in notations {
+                cmd.args(["--notation", k, v]);
+            }
+        }
+        if let Some(time) = revocation_time {
+            cmd.args([
+                "--time",
+                &time.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
+            ]);
+        }
+        let output = cmd.output()?;
+        if !output.status.success() {
+            panic!(
+                "sq exited with non-zero status code: {}",
+                String::from_utf8(output.stderr)?
+            );
+        }
+
+        // whether we found a revocation signature
+        let mut found_revoked = false;
+
+        // read revocation cert
+        let cert = Cert::from_file(&revocation)?;
+        let valid_cert =
+            cert.with_policy(STANDARD_POLICY, revocation_time.map(Into::into))?;
+
+        // Make sure the certificate stub only contains the
+        // revoked User ID (the rest should be striped).
+        assert_eq!(valid_cert.userids().count(), 1);
+
+        valid_cert.userids().for_each(|x| {
+            if x.value() == userid_revoke.as_bytes() {
+                if let RevocationStatus::Revoked(sigs) = x.revocation_status(
+                    STANDARD_POLICY,
+                    revocation_time.map(Into::into),
+                ) {
+                    // there is only one signature packet
+                    assert_eq!(sigs.len(), 1);
+                    let sig = sigs.into_iter().next().unwrap();
+
+                    // it is a certification revocation
+                    assert_eq!(
+                        sig.typ(),
+                        SignatureType::CertificationRevocation
+                    );
+
+                    // the issuer is the certificate owner
+                    assert_eq!(
+                        sig.get_issuers().into_iter().next().as_ref(),
+                        Some(&fingerprint.clone().into())
+                    );
+
+                    // our reason for revocation and message matches
+                    assert_eq!(
+                        sig.reason_for_revocation(),
+                        Some((reason, message.as_bytes()))
+                    );
+
+                    // the notations of the revocation match the ones
+                    // we passed in
+                    assert!(compare_notations(sig, notations).is_ok());
+
+                    found_revoked = true;
+                }
+            }
+        });
+
+        if !found_revoked {
+            panic!("the revoked userid is not found in the revocation cert");
+        }
+    }
+
+    tmpdir.close()?;
+
+    Ok(())
+}
+
+#[test]
+fn sq_key_userid_revoke_thirdparty() -> Result<()> {
+    let userids = &["alice <alice@example.org>", "alice <alice@other.org>"];
+    // revoke the last userid
+    let userid_revoke = userids.last().unwrap();
+    let (tmpdir, path, _) = sq_key_generate(Some(userids))?;
+
+    let (thirdparty_tmpdir, thirdparty_path, thirdparty_time) =
+        sq_key_generate(Some(&["bob <bob@example.org"]))?;
+    let thirdparty_cert = Cert::from_file(&thirdparty_path)?;
+    let thirdparty_valid_cert = thirdparty_cert
+        .with_policy(STANDARD_POLICY, Some(thirdparty_time.into()))?;
+    let thirdparty_fingerprint = thirdparty_valid_cert.clone().fingerprint();
+
+    let message = "message";
+
+    // revoke for various reasons, with or without notations added, or with
+    // a revocation whose reference time is one hour after the creation of the
+    // certificate
+    for (reason, reason_str, notations, revocation_time) in [
+        (ReasonForRevocation::UIDRetired, "retired", None, None),
+        (
+            ReasonForRevocation::UIDRetired,
+            "retired",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::UIDRetired,
+            "retired",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+        (ReasonForRevocation::Unspecified, "unspecified", None, None),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            None,
+            Some(thirdparty_time + Duration::hours(1)),
+        ),
+        (
+            ReasonForRevocation::Unspecified,
+            "unspecified",
+            Some(&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")]),
+            None,
+        ),
+    ] {
+        let revocation = &path.parent().unwrap().join(format!(
+            "revocation_{}_{}_{}.rev",
+            reason_str,
+            if notations.is_some() {
+                "notations"
+            } else {
+                "no_notations"
+            },
+            if revocation_time.is_some() {
+                "time"
+            } else {
+                "no_time"
+            }
+        ));
+
+        let mut cmd = Command::cargo_bin("sq")?;
+        cmd.args([
+            "--no-cert-store",
+            "key",
+            "userid",
+            "revoke",
+            "--output",
+            &revocation.to_string_lossy(),
+            "--certificate-file",
+            &path.to_string_lossy(),
+            "--revocation-file",
+            &thirdparty_path.to_string_lossy(),
+            userid_revoke,
+            reason_str,
+            message,
+        ]);
+        if let Some(notations) = notations {
+            for (k, v) in notations {
+                cmd.args(["--notation", k, v]);
+            }
+        }
+        if let Some(time) = revocation_time {
+            cmd.args([
+                "--time",
+                &time.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
+            ]);
+        }
+        let output = cmd.output()?;
+        if !output.status.success() {
+            panic!(
+                "sq exited with non-zero status code: {}",
+                String::from_utf8(output.stderr)?
+            );
+        }
+
+        // whether we found a revocation signature
+        let mut found_revoked = false;
+
+        // read revocation cert
+        let revocation_cert = Cert::from_file(&revocation)?;
+        let revocation_valid_cert = revocation_cert
+            .with_policy(STANDARD_POLICY, revocation_time.map(Into::into))?;
+
+        // Make sure the certificate stub only contains the
+        // revoked User ID (the rest should be stripped).
+        assert_eq!(revocation_valid_cert.userids().count(), 1);
+
+        revocation_valid_cert.userids().for_each(|x| {
+            if x.value() == userid_revoke.as_bytes() {
+                if let RevocationStatus::CouldBe(sigs) = x.revocation_status(
+                    STANDARD_POLICY,
+                    revocation_time.map(Into::into),
+                ) {
+                    // there is only one signature packet
+                    assert_eq!(sigs.len(), 1);
+                    let sig = sigs.into_iter().next().unwrap();
+
+                    // it is a certification revocation
+                    assert_eq!(
+                        sig.typ(),
+                        SignatureType::CertificationRevocation
+                    );
+
+                    // the issuer is a thirdparty revoker
+                    assert_eq!(
+                        sig.get_issuers().into_iter().next().as_ref(),
+                        Some(&thirdparty_fingerprint.clone().into())
+                    );
+
+                    // the revocation can be verified
+                    if sig
+                        .clone()
+                        .verify_userid_revocation(
+                            &thirdparty_cert.primary_key(),
+                            &revocation_cert.primary_key(),
+                            &UserID::from(*userid_revoke),
+                        )
+                        .is_err()
+                    {
+                        panic!("revocation is not valid")
+                    }
+
+                    // our reason for revocation and message matches
+                    assert_eq!(
+                        sig.reason_for_revocation(),
+                        Some((reason, message.as_bytes()))
+                    );
+
+                    // the notations of the revocation match the ones
+                    // we passed in
+                    assert!(compare_notations(sig, notations).is_ok());
+
+                    found_revoked = true;
+                }
+            }
+        });
+
+        if !found_revoked {
+            panic!("the revoked userid is not found in the revocation cert");
+        }
+    }
+
+    tmpdir.close()?;
+    thirdparty_tmpdir.close()?;
+
+    Ok(())
+}

tests/sq-revoke.rs

@@ -1,770 +0,0 @@
-use std::fs::File;
-use std::io::Write;
-
-use anyhow::Context;
-use assert_cmd::Command;
-use chrono::prelude::*;
-use chrono::Duration;
-use tempfile::TempDir;
-
-use sequoia_openpgp as openpgp;
-use openpgp::Result;
-use openpgp::cert::prelude::*;
-use openpgp::parse::Parse;
-use openpgp::Packet;
-use openpgp::packet::Key;
-use openpgp::packet::UserID;
-use openpgp::PacketPile;
-use openpgp::policy::StandardPolicy;
-use openpgp::serialize::Serialize;
-use openpgp::types::SignatureType;
-use openpgp::types::ReasonForRevocation;
-use openpgp::types::RevocationStatus;
-
-const TRACE: bool = false;
-
-mod integration {
-    use super::*;
-
-    const P: &StandardPolicy = &StandardPolicy::new();
-
-    const ALICE: &str = "<alice@example.org>";
-
-    #[derive(PartialEq, Eq)]
-    enum Subcommand {
-        Certificate,
-        UserID(Vec<String>),
-        Subkey,
-    }
-
-    impl Subcommand {
-        fn userids(&self) -> &[String] {
-            if let Subcommand::UserID(ref userids) = self {
-                assert!(userids.len() > 0);
-                userids
-            } else {
-                &[]
-            }
-        }
-    }
-
-    // If subkey is not None, then a subkey will be revoked.
-    //
-    // Otherwise, USERIDS is a vector of User IDs.  If it is empty,
-    // then a default User ID will be used and the *certificate* will
-    // be revoked.  If it contains at least one entry, then each entry
-    // will be added as a User ID, and the last User ID will be
-    // revoked.
-    fn t(subcommand: &Subcommand,
-         reason: ReasonForRevocation,
-         reason_message: &str,
-         stdin: bool,
-         third_party: bool,
-         notations: &[(&str, &str)],
-         time: Option<DateTime<Utc>>) -> Result<()>
-    {
-        // Round it down to a whole second to match the resolution of
-        // OpenPGP's timestamp.
-        let time = time.map(|t| {
-            t - Duration::nanoseconds(t.timestamp_subsec_nanos() as i64)
-        });
-
-        let gen = |userids: &[&str]| {
-            let mut builder = CertBuilder::new()
-                .add_signing_subkey()
-                .set_creation_time(
-                    time.map(|t| (t - Duration::hours(1)).into()));
-            for &u in userids {
-                builder = builder.add_userid(u);
-            }
-            builder.generate().map(|(key, _rev)| key)
-        };
-
-        let mut userid: Option<&str> = None;
-        let mut userids: Vec<&str> = vec![ ALICE ];
-
-        if let Subcommand::UserID(_) = subcommand {
-            userids = subcommand.userids().iter()
-                .map(|u| u.as_str()).collect();
-            userid = userids.last().map(|u| *u)
-        }
-
-        // We're going to revoke alice's certificate or a User ID.  If
-        // we're doing it via a third-party revocation, then bob is
-        // the revoker.  Otherwise, it's alice.
-        let alice = gen(&userids)?;
-        let bob = gen(&[ "<revoker@some.org>" ])?;
-
-        let mut cert = Vec::new();
-        alice.serialize(&mut cert)?;
-
-        let mut revoker = Vec::new();
-        if third_party {
-            bob.as_tsk().serialize(&mut revoker)?;
-        } else {
-            alice.as_tsk().serialize(&mut revoker)?;
-        }
-
-        let subkey: Key<_, _> = alice.with_policy(P, None).unwrap()
-            .keys().subkeys().nth(0).unwrap().key().clone();
-
-        // Build up the command line.
-        let mut cmd = Command::cargo_bin("sq")?;
-        cmd.arg("--no-cert-store");
-        cmd.arg("revoke");
-        if let Some(userid) = userid {
-            cmd.args([
-                "userid", userid,
-                match reason {
-                    ReasonForRevocation::UIDRetired => "retired",
-                    ReasonForRevocation::Unspecified => "unspecified",
-                    _ => panic!("Invalid reason: {}", reason),
-                },
-                reason_message
-            ]);
-        } else {
-            match subcommand {
-                Subcommand::Certificate => {
-                    cmd.arg("certificate");
-                }
-                Subcommand::Subkey => {
-                    cmd.args(&["subkey", &subkey.fingerprint().to_string()]);
-                }
-                Subcommand::UserID(_) => unreachable!(),
-            }
-            cmd.args([
-                match reason {
-                    ReasonForRevocation::KeyCompromised => "compromised",
-                    ReasonForRevocation::KeyRetired => "retired",
-                    ReasonForRevocation::KeySuperseded => "superseded",
-                    ReasonForRevocation::Unspecified => "unspecified",
-                    _ => panic!("Invalid reason: {}", reason),
-                },
-                reason_message
-            ]);
-        }
-
-        let _tmp_dir = match (third_party, stdin) {
-            (true, true) => {
-                // cat cert | sq revoke --revocation-file third-party
-                let dir = TempDir::new()?;
-
-                cmd.write_stdin(cert);
-
-                let revoker_pgp = dir.path().join("revoker.pgp");
-                let mut file = File::create(&revoker_pgp)?;
-                file.write_all(&revoker)?;
-
-                cmd.args([
-                    "--revocation-file",
-                    &*revoker_pgp.to_string_lossy()
-                ]);
-
-                Some(dir)
-            },
-            (true, false) => { // third_party && ! stdin
-                // sq revoke --cert-file cert --revocation-file third-party
-                let dir = TempDir::new()?;
-
-                let cert_pgp = dir.path().join("cert.pgp");
-                let mut file = File::create(&cert_pgp)?;
-                file.write_all(&cert)?;
-
-                cmd.args([
-                    "--cert-file",
-                    &*cert_pgp.to_string_lossy()
-                ]);
-
-                let revoker_pgp = dir.path().join("revoker.pgp");
-                let mut file = File::create(&revoker_pgp)?;
-                file.write_all(&revoker)?;
-
-                cmd.args([
-                    "--revocation-file",
-                    &*revoker_pgp.to_string_lossy()
-                ]);
-
-                Some(dir)
-            },
-            (false, true) => { // ! third_party && stdin
-                // cat key | sq revoke
-                cmd.write_stdin(revoker);
-
-                None
-            },
-            (false, false) => { // ! third_party && ! stdin
-                // sq revoke --cert-file key
-                let dir = TempDir::new()?;
-
-                let key_pgp = dir.path().join("key.pgp");
-                let mut file = File::create(&key_pgp)?;
-                file.write_all(&revoker)?;
-
-                cmd.args([
-                    "--cert-file",
-                    &*key_pgp.to_string_lossy()
-                ]);
-
-                Some(dir)
-            },
-        };
-
-        // Time.
-        if let Some(t) = time {
-            cmd.args([
-                "--time",
-                &t.format("%Y-%m-%dT%H:%M:%SZ").to_string()],
-            );
-        }
-
-        // Notations.
-        for (k, v) in notations {
-            cmd.args(["--notation", k, v]);
-        }
-
-        if TRACE {
-            eprintln!("Running: {:?}", cmd);
-        }
-        let assertion = cmd.assert().try_success()?;
-        let stdout = String::from_utf8_lossy(&assertion.get_output().stdout);
-
-        // Pretty print 'sq revoke''s output for debugging purposes.
-        if TRACE {
-            let mut cmd = Command::cargo_bin("sq")?;
-            cmd.args([ "--no-cert-store", "inspect" ]);
-            cmd.write_stdin(stdout.as_bytes());
-            let assertion = cmd.assert().try_success()?;
-            eprintln!("Result:\n{}",
-                      String::from_utf8_lossy(&assertion.get_output().stdout));
-        }
-
-        {
-            let vc = alice.with_policy(P, time.map(Into::into)).unwrap();
-            assert!(matches!(vc.revocation_status(),
-                             RevocationStatus::NotAsFarAsWeKnow));
-
-            if let Some(userid) = userid {
-                let mut found = false;
-                for u in vc.userids() {
-                    if u.value() == userid.as_bytes() {
-                        assert!(matches!(u.revocation_status(),
-                                         RevocationStatus::NotAsFarAsWeKnow));
-
-                        found = true;
-                        break;
-                    }
-                }
-                assert!(found, "User ID {} not found on certificate", userid);
-            }
-        }
-
-        // Get the revocation certificate.
-        let sig = if ! third_party && subcommand == &Subcommand::Certificate {
-            // We should get just a single signature packet.
-            let pp = PacketPile::from_bytes(&*stdout)?;
-
-            assert_eq!(pp.children().count(), 1,
-                       "expected a single packet");
-
-            if let Some(Packet::Signature(sig)) = pp.path_ref(&[0]) {
-                // Alice issued the revocation.
-                assert_eq!(sig.get_issuers().into_iter().next(),
-                           Some(alice.fingerprint().into()));
-
-                let alice2 = alice.insert_packets(sig.clone()).unwrap();
-
-                // Verify the revocation.
-                assert!(matches!(
-                    alice2.with_policy(P, time.map(Into::into)).unwrap()
-                        .revocation_status(),
-                    RevocationStatus::Revoked(_)));
-
-                sig.clone()
-            } else {
-                panic!("Expected a signature, got: {:?}", pp);
-            }
-        } else {
-            // We should get a certificate stub.
-            let result = Cert::from_bytes(&*stdout)?;
-
-            let vc = result.with_policy(P, time.map(Into::into))?;
-
-            // Make sure the certificate stub only contains the
-            // revoked User ID (the rest should be striped).
-            assert_eq!(vc.userids().count(), 1);
-
-            // Get the revocation status of the revoked object.
-            let status = match subcommand {
-                Subcommand::Certificate => vc.revocation_status(),
-                Subcommand::Subkey => {
-                    let mut status = None;
-                    for k in vc.keys() {
-                        if k.fingerprint() == subkey.fingerprint() {
-                            status = Some(k.revocation_status());
-                            break;
-                        }
-                    }
-                    if let Some(status) = status {
-                        status
-                    } else {
-                        panic!("Revoked subkey {} not found on certificate",
-                               subkey.fingerprint());
-                    }
-                }
-                Subcommand::UserID(_) => {
-                    let userid = userid.unwrap();
-                    let mut status = None;
-                    for u in vc.userids() {
-                        if u.value() == userid.as_bytes() {
-                            status = Some(u.revocation_status());
-                            break;
-                        }
-                    }
-                    if let Some(status) = status {
-                        status
-                    } else {
-                        panic!("Revoked user ID {} not found on certificate",
-                               userid);
-                    }
-                }
-            };
-
-            // Make sure the revocation status is sane.
-            if third_party {
-                if let RevocationStatus::CouldBe(sigs) = status {
-                    assert_eq!(sigs.len(), 1);
-                    let sig = sigs.into_iter().next().unwrap();
-
-                    // Bob issued the revocation.
-                    assert_eq!(sig.get_issuers().into_iter().next(),
-                               Some(bob.fingerprint().into()));
-
-                    // Verify the revocation.
-                    match subcommand {
-                        Subcommand::Certificate => {
-                            sig.clone()
-                                .verify_primary_key_revocation(
-                                    &bob.primary_key(),
-                                    &alice.primary_key())
-                                .context("revocation is not valid")?;
-                        }
-                        Subcommand::Subkey => {
-                            sig.clone()
-                                .verify_subkey_revocation(
-                                    &bob.primary_key(),
-                                    &alice.primary_key(),
-                                    &subkey)
-                                .context("revocation is not valid")?;
-                        }
-                        Subcommand::UserID(_) => {
-                            sig.clone()
-                                .verify_userid_revocation(
-                                    &bob.primary_key(),
-                                    &alice.primary_key(),
-                                    &UserID::from(userid.unwrap()))
-                                .context("revocation is not valid")?;
-                        }
-                    }
-
-                    sig.clone()
-                } else {
-                    panic!("Unexpected revocation status: {:?}", status);
-                }
-            } else {
-                if let RevocationStatus::Revoked(sigs) = status {
-                    assert_eq!(sigs.len(), 1);
-                    let sig = sigs.into_iter().next().unwrap();
-
-                    // Alice issued the revocation.
-                    assert_eq!(sig.get_issuers().into_iter().next(),
-                               Some(alice.fingerprint().into()));
-
-                    // Since it is a self-siganture, sig has already
-                    // been validated.
-
-                    sig.clone()
-                } else {
-                    panic!("Unexpected revocation status: {:?}", status);
-                }
-            }
-        };
-
-        // Revocation reason.
-        match subcommand {
-            Subcommand::Certificate =>
-                assert_eq!(sig.typ(), SignatureType::KeyRevocation),
-            Subcommand::Subkey =>
-                assert_eq!(sig.typ(), SignatureType::SubkeyRevocation),
-            Subcommand::UserID(_) =>
-                assert_eq!(sig.typ(), SignatureType::CertificationRevocation),
-        }
-        assert_eq!(sig.reason_for_revocation(),
-                   Some((reason, reason_message.as_bytes())));
-
-        // Time.
-        if let Some(t) = time {
-            assert_eq!(Some(t.into()), sig.signature_creation_time());
-        }
-
-        // Notations.
-        let got: Vec<(&str, String)> = sig.notation_data()
-            .map(|n| {
-                (n.name(),
-                 String::from_utf8_lossy(n.value()).into())
-            })
-            .collect();
-
-        for (n, v) in notations {
-            assert!(got.contains(&(n, String::from(*v))),
-                    "notations: {:?}\nexpected: {}: {}",
-                    notations, n, v);
-        }
-
-        Ok(())
-    }
-
-    fn dispatch(subcommand: Subcommand,
-                reasons: &[ReasonForRevocation],
-                msgs: &[&str],
-                stdin: &[bool],
-                third_party: &[bool],
-                notations: &[&[(&str, &str)]],
-                time: &[Option<DateTime<Utc>>]) -> Result<()>
-    {
-        for third_party in third_party {
-            for time in time {
-                for stdin in stdin {
-                    for notations in notations {
-                        for reason in reasons {
-                            for msg in msgs {
-                                eprintln!("\n\
-                                           third party: {}\n\
-                                           time: {:?}\n\
-                                           stdin: {}\n\
-                                           notations: {:?}\n\
-                                           reason: {:?}\n\
-                                           message: {:?}",
-                                          third_party, time, stdin, notations,
-                                          reason, msg);
-                                t(&subcommand,
-                                  *reason, *msg,
-                                  *stdin, *third_party, *notations,
-                                  *time)?;
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
-        Ok(())
-    }
-
-    const CERT_REASONS: &[ ReasonForRevocation ] = &[
-        ReasonForRevocation::KeyCompromised,
-        ReasonForRevocation::KeyRetired,
-        ReasonForRevocation::KeySuperseded,
-        ReasonForRevocation::Unspecified
-    ];
-    const USERID_REASONS: &[ ReasonForRevocation ] = &[
-        ReasonForRevocation::UIDRetired,
-        ReasonForRevocation::Unspecified
-    ];
-    const MSGS: &[&str] = &[
-        "oh NO!",
-        "Löwe 老\n虎 Léopard"
-    ];
-    const NOTATIONS: &[ &[ (&str, &str) ] ] = &[
-        &[],
-        &[("a", "b")],
-        &[("a", "b"), ("hallo@sequoia-pgp.org", "VALUE")]
-    ];
-
-    // User ID revocation tests.
-    #[test]
-    fn sq_revoke_cert_stdin() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Certificate,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[true],
-            // third_party
-            &[false],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_cert() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Certificate,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[false],
-            // third_party
-            &[false],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_cert_third_party_stdin() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Certificate,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[true],
-            // third_party
-            &[true],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_cert_third_party() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Certificate,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[false],
-            // third_party
-            &[true],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-
-    // Subkey revocation tests.
-    //
-    // We manually unroll to get some parallelism.  Otherwise, the
-    // tests take way too long.
-    #[test]
-    fn sq_revoke_subkey_stdin() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Subkey,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[true],
-            // third_party
-            &[false],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_subkey() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Subkey,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[false],
-            // third_party
-            &[false],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_subkey_third_party_stdin() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Subkey,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[true],
-            // third_party
-            &[true],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_subkey_third_party() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::Subkey,
-            CERT_REASONS,
-            MSGS,
-            // stdin
-            &[false],
-            // third_party
-            &[true],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    // User ID revocation tests.
-    //
-    // We manually unroll to get some parallelism.  Otherwise, the
-    // tests take way too long.
-    #[test]
-    fn sq_revoke_userid_stdin() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::UserID(vec![ ALICE.into() ]),
-            USERID_REASONS,
-            MSGS,
-            // stdin
-            &[true],
-            // third_party
-            &[false],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_userid() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::UserID(vec![ ALICE.into() ]),
-            USERID_REASONS,
-            MSGS,
-            // stdin
-            &[false],
-            // third_party
-            &[false],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_userid_third_party_stdin() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::UserID(vec![ ALICE.into() ]),
-            USERID_REASONS,
-            MSGS,
-            // stdin
-            &[true],
-            // third_party
-            &[true],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-    #[test]
-    fn sq_revoke_userid_third_party() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::UserID(vec![ ALICE.into() ]),
-            USERID_REASONS,
-            MSGS,
-            // stdin
-            &[false],
-            // third_party
-            &[true],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-
-
-    #[test]
-    fn sq_revoke_one_of_three_userids() -> Result<()> {
-        let now = Utc::now();
-
-        dispatch(
-            Subcommand::UserID(vec![
-                "<alice@example.org".into(),
-                "<alice@some.org>".into(),
-                "<alice@other.org>".into(),
-            ]),
-            USERID_REASONS,
-            MSGS,
-            // stdin
-            &[false],
-            // third_party
-            &[false],
-            NOTATIONS,
-            // time
-            &[
-                None,
-                Some(now),
-                Some(now - Duration::hours(1))
-            ])
-    }
-}