Use the null policy when creating revocation certificates.
- When creating a revocation certificate using, e.g., `sq key revoke`, use the null policy. - Even if a certificate is not valid according to the standard policy, it can still be useful to revoke it. - Fixes #250.
This commit is contained in:
parent
fa835e234f
commit
0e5c58ef05
@ -10,8 +10,9 @@ use openpgp::Result;
|
||||
use crate::Sq;
|
||||
use crate::cli::key::RevokeCommand;
|
||||
use crate::cli::types::FileOrStdout;
|
||||
use crate::common::RevocationOutput;
|
||||
use crate::common::get_secret_signer;
|
||||
use crate::common::NULL_POLICY;
|
||||
use crate::common::RevocationOutput;
|
||||
use crate::parse_notations;
|
||||
|
||||
/// Handle the revocation of a certificate
|
||||
@ -98,7 +99,8 @@ pub fn certificate_revoke(
|
||||
let br = file.open()?;
|
||||
Cert::from_buffered_reader(br)?
|
||||
} else if let Some(kh) = command.cert {
|
||||
sq.lookup_one(&kh, None, true)?
|
||||
sq.lookup_one_with_policy(&kh, None, true,
|
||||
NULL_POLICY, sq.time)?
|
||||
} else {
|
||||
panic!("clap enforces --cert or --cert-file");
|
||||
};
|
||||
@ -107,7 +109,8 @@ pub fn certificate_revoke(
|
||||
let br = file.open()?;
|
||||
Some(Cert::from_buffered_reader(br)?)
|
||||
} else if let Some(kh) = command.revoker {
|
||||
Some(sq.lookup_one(&kh, None, true)?)
|
||||
Some(sq.lookup_one_with_policy(&kh, None, true,
|
||||
NULL_POLICY, sq.time)?)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
@ -123,6 +123,7 @@ pub fn get_secret_signer<'a>(
|
||||
let flags = Some(&[
|
||||
GetKeysOptions::AllowRevoked,
|
||||
GetKeysOptions::AllowNotAlive,
|
||||
GetKeysOptions::NullPolicy,
|
||||
][..]);
|
||||
|
||||
if let Some(secret) = secret {
|
||||
|
18
src/sq.rs
18
src/sq.rs
@ -24,6 +24,7 @@ use openpgp::packet::prelude::*;
|
||||
use openpgp::parse::Parse;
|
||||
use openpgp::policy::Policy;
|
||||
use openpgp::policy::StandardPolicy as P;
|
||||
use openpgp::policy::NullPolicy;
|
||||
use openpgp::types::KeyFlags;
|
||||
use openpgp::types::RevocationStatus;
|
||||
|
||||
@ -50,6 +51,8 @@ use crate::output::hint::Hint;
|
||||
use crate::PreferredUserID;
|
||||
use crate::print_error_chain;
|
||||
|
||||
static NULL_POLICY: NullPolicy = NullPolicy::new();
|
||||
|
||||
/// Flags for Sq::get_keys and related functions.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum GetKeysOptions {
|
||||
@ -57,6 +60,8 @@ pub enum GetKeysOptions {
|
||||
AllowNotAlive,
|
||||
/// Don't ignore keys that are not revoke.
|
||||
AllowRevoked,
|
||||
/// Use the NULL Policy.
|
||||
NullPolicy,
|
||||
}
|
||||
|
||||
/// Flag for Sq::get_keys and related function.
|
||||
@ -1005,9 +1010,7 @@ impl<'store: 'rstore, 'rstore> Sq<'store, 'rstore> {
|
||||
|
||||
// Second, apply the null policy.
|
||||
if primary_uid.is_none() {
|
||||
const NULL: openpgp::policy::NullPolicy =
|
||||
openpgp::policy::NullPolicy::new();
|
||||
if let Ok(vcert) = cert.with_policy(&NULL, self.time) {
|
||||
if let Ok(vcert) = cert.with_policy(&NULL_POLICY, self.time) {
|
||||
if let Ok(primary) = vcert.primary_userid() {
|
||||
primary_uid = Some(primary.userid());
|
||||
}
|
||||
@ -1295,6 +1298,13 @@ impl<'store: 'rstore, 'rstore> Sq<'store, 'rstore> {
|
||||
let options = options.unwrap_or(&[][..]);
|
||||
let allow_not_alive = options.contains(&GetKeysOptions::AllowNotAlive);
|
||||
let allow_revoked = options.contains(&GetKeysOptions::AllowRevoked);
|
||||
let null_policy = options.contains(&GetKeysOptions::NullPolicy);
|
||||
|
||||
let policy = if null_policy {
|
||||
&NULL_POLICY as &dyn Policy
|
||||
} else {
|
||||
self.policy as &dyn Policy
|
||||
};
|
||||
|
||||
let mut keys: Vec<(Box<dyn crypto::Signer + Send + Sync>,
|
||||
Option<Password>)>
|
||||
@ -1302,7 +1312,7 @@ impl<'store: 'rstore, 'rstore> Sq<'store, 'rstore> {
|
||||
|
||||
'next_cert: for cert in certs {
|
||||
let cert = cert.borrow();
|
||||
let vc = match cert.with_policy(self.policy, self.time) {
|
||||
let vc = match cert.with_policy(policy, self.time) {
|
||||
Ok(vc) => vc,
|
||||
Err(err) => {
|
||||
return Err(
|
||||
|
@ -19,6 +19,7 @@ use chrono::Utc;
|
||||
|
||||
use openpgp::packet::Signature;
|
||||
use openpgp::parse::Parse;
|
||||
use openpgp::policy::NullPolicy;
|
||||
use openpgp::policy::StandardPolicy;
|
||||
use openpgp::Cert;
|
||||
use openpgp::cert::CertParser;
|
||||
@ -30,6 +31,11 @@ use sequoia_openpgp as openpgp;
|
||||
use tempfile::TempDir;
|
||||
|
||||
pub const STANDARD_POLICY: &StandardPolicy = &StandardPolicy::new();
|
||||
pub const NULL_POLICY: &NullPolicy = &NullPolicy::new();
|
||||
|
||||
pub fn artifact(filename: &str) -> PathBuf {
|
||||
PathBuf::from("tests/data").join(filename)
|
||||
}
|
||||
|
||||
// Returns the power set excluding the empty set.
|
||||
pub fn power_set<T: Clone>(set: &[T]) -> Vec<Vec<T>> {
|
||||
|
BIN
tests/data/keys/only-sha1-priv.pgp
Normal file
BIN
tests/data/keys/only-sha1-priv.pgp
Normal file
Binary file not shown.
BIN
tests/data/keys/only-sha1-pub.pgp
Normal file
BIN
tests/data/keys/only-sha1-pub.pgp
Normal file
Binary file not shown.
@ -9,9 +9,11 @@ use openpgp::Result;
|
||||
use sequoia_openpgp as openpgp;
|
||||
|
||||
mod common;
|
||||
use common::artifact;
|
||||
use common::compare_notations;
|
||||
use common::FileOrKeyHandle;
|
||||
use common::Sq;
|
||||
use common::NULL_POLICY;
|
||||
use common::STANDARD_POLICY;
|
||||
|
||||
#[test]
|
||||
@ -20,7 +22,7 @@ fn sq_key_revoke() -> Result<()> {
|
||||
|
||||
let time = sq.now();
|
||||
|
||||
let (cert, cert_path, _cert_rev)
|
||||
let (_cert, cert_path, _cert_rev)
|
||||
= sq.key_generate(&[], &["alice"]);
|
||||
|
||||
let message = "message";
|
||||
@ -28,7 +30,7 @@ fn sq_key_revoke() -> Result<()> {
|
||||
// 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 [
|
||||
for ((reason, reason_str, notations, revocation_time), cert_path) in [
|
||||
(
|
||||
ReasonForRevocation::KeyCompromised,
|
||||
"compromised",
|
||||
@ -74,11 +76,21 @@ fn sq_key_revoke() -> Result<()> {
|
||||
&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")][..],
|
||||
None,
|
||||
),
|
||||
] {
|
||||
].into_iter().flat_map(|test| {
|
||||
[
|
||||
// A normal key.
|
||||
(test, cert_path.clone()),
|
||||
// A key that uses SHA-1.
|
||||
(test, artifact("keys/only-sha1-priv.pgp")),
|
||||
]
|
||||
})
|
||||
{
|
||||
eprintln!("==========================");
|
||||
eprintln!("reason: {}, message: {}, notations: {:?}, time: {:?}",
|
||||
reason, reason_str, notations, revocation_time);
|
||||
|
||||
let cert = Cert::from_file(&cert_path).expect("valid cert");
|
||||
|
||||
for keystore in [false, true].into_iter() {
|
||||
eprintln!("--------------------------");
|
||||
eprintln!("keystore: {}", keystore);
|
||||
@ -166,22 +178,18 @@ fn sq_key_revoke_thirdparty() -> Result<()> {
|
||||
|
||||
let time = sq.now();
|
||||
|
||||
let (cert, cert_path, _cert_rev)
|
||||
let (_cert, cert_path, _cert_rev)
|
||||
= sq.key_generate(&[], &["alice"]);
|
||||
|
||||
let (thirdparty_cert, thirdparty_path, _cert_rev)
|
||||
let (_thirdparty_cert, thirdparty_path, _cert_rev)
|
||||
= sq.key_generate(&[], &["bob <bob@example.org>"]);
|
||||
|
||||
let thirdparty_valid_cert = thirdparty_cert
|
||||
.with_policy(STANDARD_POLICY, Some(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 [
|
||||
for ((reason, reason_str, notations, revocation_time), cert_path, thirdparty_path) in [
|
||||
(
|
||||
ReasonForRevocation::KeyCompromised,
|
||||
"compromised",
|
||||
@ -239,7 +247,31 @@ fn sq_key_revoke_thirdparty() -> Result<()> {
|
||||
&[("foo", "bar"), ("hallo@sequoia-pgp.org", "VALUE")][..],
|
||||
None,
|
||||
),
|
||||
] {
|
||||
].into_iter().flat_map(|test| {
|
||||
[
|
||||
// Two valid keys.
|
||||
(test,
|
||||
cert_path.clone(),
|
||||
thirdparty_path.clone()),
|
||||
// The revokee is invalid (SHA-1).
|
||||
(test,
|
||||
artifact("keys/only-sha1-priv.pgp"),
|
||||
thirdparty_path.clone()),
|
||||
// The revoker is invalid (SHA-1).
|
||||
(test,
|
||||
cert_path.clone(),
|
||||
artifact("keys/only-sha1-priv.pgp")),
|
||||
]
|
||||
}) {
|
||||
let cert = Cert::from_file(&cert_path).expect("valid cert");
|
||||
let thirdparty_cert
|
||||
= Cert::from_file(&thirdparty_path).expect("valid cert");
|
||||
|
||||
let thirdparty_valid_cert = thirdparty_cert
|
||||
.with_policy(NULL_POLICY, Some(time.into()))?;
|
||||
let thirdparty_fingerprint
|
||||
= &thirdparty_valid_cert.clone().fingerprint();
|
||||
|
||||
for keystore in [false, true].into_iter() {
|
||||
let revocation = sq.scratch_file(Some(&format!(
|
||||
"revocation_{}_{}_{}.rev",
|
||||
@ -280,7 +312,6 @@ fn sq_key_revoke_thirdparty() -> Result<()> {
|
||||
notations,
|
||||
Some(revocation.as_path()));
|
||||
|
||||
let revocation_cert = Cert::from_file(&revocation)?;
|
||||
assert!(! revocation_cert.is_tsk());
|
||||
|
||||
// evaluate revocation status
|
||||
|
Loading…
Reference in New Issue
Block a user