Change sq cert lint to support the cert store and key store.

- See #205.
This commit is contained in:
Neal H. Walfield 2024-05-28 14:33:27 +02:00
parent 5c1cf92f9b
commit ab0e2a446c
No known key found for this signature in database
GPG Key ID: 6863C9AD5B4D22D3
4 changed files with 764 additions and 605 deletions

2
NEWS
View File

@ -32,6 +32,8 @@
- Change `sq cert lint` to not read from stdin by default.
- In `sq cert lint`, change the certificate file parameter from a
position parameter to a named parameter, `--cert-file`.
- `sq cert lint` can now use the certificate store and the
keystore.
* Changes in 0.36.0
- Missing
* Changes in 0.35.0

View File

@ -1,6 +1,10 @@
//! Command-line parser for `sq cert lint`.
use clap::Args;
use clap::ArgGroup;
use sequoia_openpgp as openpgp;
use openpgp::KeyHandle;
use crate::cli::types::ClapData;
use crate::cli::types::FileOrStdin;
@ -67,6 +71,7 @@ $ sq cert lint --list-keys keyring.pgp \\
| while read FPR; do something; done
"
)]
#[clap(group(ArgGroup::new("cert_input").args(&["cert_file", "cert"]).required(true)))]
pub struct Command {
/// Quiet; does not output any diagnostics.
#[arg(short, long)]
@ -94,20 +99,29 @@ pub struct Command {
#[clap(
long,
help = FileOrStdin::HELP_OPTIONAL,
value_name = FileOrStdin::VALUE_NAME,
required = true,
value_name = "CERT_FILE",
help = "Lint the certificates in the specified file",
)]
pub cert_file: Vec<FileOrStdin>,
#[clap(
long,
value_name = "FINGERPRINT|KEYID",
help = "Lint the specified certificate",
conflicts_with = "cert_file",
)]
pub cert: Vec<KeyHandle>,
#[clap(
default_value_t = FileOrStdout::default(),
help = FileOrStdout::HELP_OPTIONAL,
long,
short,
value_name = FileOrStdout::VALUE_NAME,
help = "Write to the specified FILE. If not specified, and the \
certificate was read from the certificate store, imports the \
modified certificate into the cert store. If not specified, \
and the certificate was read from a file, writes the modified \
certificate to stdout.",
)]
pub output: FileOrStdout,
pub output: Option<FileOrStdout>,
#[clap(
short = 'B',
long = "binary",

View File

@ -1,17 +1,20 @@
use std::cmp::Ordering;
use std::io::IsTerminal;
use std::io:: Write;
use std::process::exit;
use std::time::SystemTime;
use std::sync::Arc;
use std::io::IsTerminal;
use openpgp::crypto::Password;
use termcolor::{WriteColor, StandardStream, ColorChoice, ColorSpec, Color};
use sequoia_openpgp as openpgp;
use anyhow::Context;
use sequoia_openpgp as openpgp;
use openpgp::Result;
use openpgp::armor;
use openpgp::cert::prelude::*;
use openpgp::crypto::Password;
use openpgp::crypto::Signer;
use openpgp::parse::Parse;
use openpgp::packet::prelude::*;
use openpgp::policy::Policy;
@ -23,29 +26,33 @@ use openpgp::types::RevocationStatus;
use openpgp::types::SignatureType;
use openpgp::types::SymmetricAlgorithm;
use sequoia_cert_store as cert_store;
use cert_store::LazyCert;
use cert_store::StoreUpdate;
use crate::{
Sq,
decrypt_key,
cli::cert::lint::Command,
commands::FileOrStdout,
};
fn update_cert_revocation(cert: &Cert, rev: &Signature,
fn update_cert_revocation(sq: &Sq,
cert: &Cert, rev: &Signature,
passwords: &mut Vec<Password>,
reference_time: &SystemTime)
-> Result<Signature>
{
assert_eq!(rev.typ(), SignatureType::KeyRevocation);
let pk = cert.primary_key().key();
let ka = cert.primary_key();
let pk = ka.key();
// Derive a signer.
let mut signer =
decrypt_key(
pk.clone().parts_into_secret()?,
passwords)?
.into_keypair()?;
let (mut signer, password) = sq.get_signer(&ka, Some(&passwords))?;
if let Some(password) = password {
passwords.push(password);
}
let sig = SignatureBuilder::from(rev.clone())
.set_signature_creation_time(reference_time.clone())?
@ -64,19 +71,20 @@ const GOOD_HASHES: &[ HashAlgorithm ] = &[
// Update the binding signature for ua.
//
// ua is using a weak policy.
fn update_user_id_binding(ua: &ValidUserIDAmalgamation,
fn update_user_id_binding(sq: &Sq,
ua: &ValidUserIDAmalgamation,
passwords: &mut Vec<Password>,
reference_time: &SystemTime)
-> Result<Signature>
{
let pk = ua.cert().primary_key().key();
let ka = ua.cert().primary_key();
let pk = ka.key();
// Derive a signer.
let mut signer =
decrypt_key(
pk.clone().parts_into_secret()?,
passwords)?
.into_keypair()?;
let (mut signer, password) = sq.get_signer(&ka, Some(&passwords))?;
if let Some(password) = password {
passwords.push(password);
}
let sym = &[
SymmetricAlgorithm::AES128,
@ -134,20 +142,21 @@ fn update_user_id_binding(ua: &ValidUserIDAmalgamation,
// Update the subkey binding signature for ka.
//
// ka is using a weak policy.
fn update_subkey_binding<P>(ka: &ValidSubordinateKeyAmalgamation<P>,
fn update_subkey_binding<P>(sq: &Sq,
ka: &ValidSubordinateKeyAmalgamation<P>,
passwords: &mut Vec<Password>,
reference_time: &SystemTime)
-> Result<Signature>
where P: key::KeyParts + Clone
{
let pk = ka.cert().primary_key().key();
let primary = ka.cert().primary_key();
let pk = primary.key();
// Derive a signer.
let mut signer =
decrypt_key(
pk.clone().parts_into_secret()?,
passwords)?
.into_keypair()?;
let (mut signer, password) = sq.get_signer(&primary, Some(&passwords))?;
if let Some(password) = password {
passwords.push(password);
}
// Update the signature.
let sig = ka.binding_signature();
@ -159,17 +168,16 @@ fn update_subkey_binding<P>(ka: &ValidSubordinateKeyAmalgamation<P>,
if let Some(backsig) = sig.embedded_signatures()
.filter(|backsig| {
(*backsig).clone().verify_primary_key_binding(
&ka.cert().cert().primary_key(),
pk,
ka.key()).is_ok()
})
.nth(0)
{
// Derive a signer.
let mut subkey_signer
= decrypt_key(
ka.key().clone().parts_into_secret()?,
passwords)?
.into_keypair()?;
let (mut subkey_signer, password) = sq.get_signer(&ka, Some(&passwords))?;
if let Some(password) = password {
passwords.push(password);
}
let backsig = SignatureBuilder::from(backsig.clone())
.set_signature_creation_time(reference_time.clone())?
@ -248,22 +256,42 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
let mut passwords = Vec::new();
let mut out = args.output.create_pgp_safe(
let mut out = if args.output.is_some() || ! args.cert_file.is_empty() {
let output = if let Some(output) = args.output {
output
} else {
FileOrStdout::new(None)
};
Some(output.create_pgp_safe(
sq.force, args.binary,
if args.export_secret_keys {
armor::Kind::SecretKey
} else {
armor::Kind::PublicKey
})?;
})?)
} else {
None
};
'next_input: for input in args.cert_file {
let mut input_reader = input.open()?;
'next_input: for input in args.cert.into_iter().map(Ok)
.chain(args.cert_file.into_iter().map(Err))
{
let certs = match input {
Ok(kh) => {
let cert = sq.lookup_one(&kh, None, true)?;
vec![cert]
}
Err(input) => {
let filename = match input.inner() {
Some(path) => path.display().to_string(),
None => "/dev/stdin".to_string(),
};
let mut input_reader = input.open()?;
let mut certs = Vec::new();
let certp = CertParser::from_buffered_reader(&mut input_reader)?;
'next_cert: for (certi, certo) in certp.enumerate() {
for (certi, certo) in certp.enumerate() {
match certo {
Err(err) => {
if ! args.quiet {
@ -278,7 +306,14 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
bad_input = true;
continue 'next_input;
}
Ok(cert) => {
Ok(cert) => certs.push(cert)
}
}
certs
}
};
'next_cert: for cert in certs.into_iter() {
// Whether we found at least one issue.
let mut found_issue = false;
@ -310,11 +345,21 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
() => {{
if updates.len() > 0 {
let cert = cert.insert_packets(updates)?;
if let Some(mut out) = out.as_mut() {
if args.export_secret_keys {
cert.as_tsk().serialize(&mut out)?;
} else {
cert.serialize(&mut out)?;
}
} else {
let fpr = cert.fingerprint();
let cert_store = sq.cert_store_or_else()?;
cert_store.update(Arc::new(LazyCert::from(cert)))
.with_context(|| {
format!("Error importing the {} into cert store",
fpr)
})?;
}
}
continue 'next_cert;
}}
@ -419,7 +464,7 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
rev.digest_prefix()[1]);
if args.fix {
match update_cert_revocation(
&cert, rev, &mut passwords,
&sq, &cert, rev, &mut passwords,
&reference_time)
{
Ok(sig) => {
@ -543,7 +588,7 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
}
if args.fix {
match update_user_id_binding(
&ua, &mut passwords, &reference_time)
&sq, &ua, &mut passwords, &reference_time)
{
Ok(sig) => {
updates.push(sig);
@ -596,7 +641,7 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
}
if args.fix {
match update_subkey_binding(
&ka, &mut passwords,
&sq, &ka, &mut passwords,
&reference_time)
{
Ok(sig) => updates.push(sig),
@ -685,7 +730,7 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
}
if args.fix {
match update_subkey_binding(
&ka, &mut passwords, &reference_time)
&sq, &ka, &mut passwords, &reference_time)
{
Ok(sig) => updates.push(sig),
Err(err) => {
@ -726,10 +771,10 @@ pub fn lint(sq: Sq, mut args: Command) -> Result<()> {
next_cert!();
}
}
}
}
if let Some(out) = out {
out.finalize()?;
}
let pl = |n, singular, plural| { if n == 1 { singular } else { plural } };
macro_rules! err {

View File

@ -5,6 +5,8 @@ mod integration {
use assert_cmd::Command;
use predicates::prelude::*;
use tempfile::TempDir;
use sequoia_openpgp as openpgp;
use openpgp::Cert;
@ -70,16 +72,73 @@ mod integration {
}
for suffix in suffixes.iter() {
for keystore in [false, true] {
let home = TempDir::new().unwrap();
let home = home.path().display().to_string();
// Lint it.
let filename = &format!("{}-{}.pgp", base, suffix);
eprintln!("Linting {}", filename);
sq()
let cert = Cert::from_file(dir.join(filename))
.expect(&format!("Can parse {}", filename));
if keystore {
// When using the keystore, we need to import the key.
if suffix == &"pub" {
eprintln!("Import certificate from {}", filename);
let mut cmd = sq();
cmd
.current_dir(&dir)
.arg("--no-cert-store")
.arg("--no-key-store")
.args([
"--home", &home,
"cert",
"import",
&filename,
]);
let output = cmd.output().expect("can sq cert import");
if !output.status.success() {
panic!(
"sq exited with non-zero status code: {}",
String::from_utf8_lossy(&output.stderr)
);
}
} else {
eprintln!("Import key from {}", filename);
let mut cmd = sq();
cmd
.current_dir(&dir)
.args([
"--home", &home,
"key",
"import",
&filename,
]);
let output = cmd.output().expect("can sq key import");
if !output.status.success() {
panic!(
"sq exited with non-zero status code: {}",
String::from_utf8_lossy(&output.stderr)
);
}
}
}
let mut cmd = sq();
cmd
.current_dir(&dir)
.arg("--home").arg(&home)
.arg("cert").arg("lint")
.arg("--time").arg(FROZEN_TIME)
.arg("--cert-file").arg(filename)
.arg("--time").arg(FROZEN_TIME);
if keystore {
cmd.arg("--cert").arg(&cert.fingerprint().to_string());
} else {
cmd.arg("--cert-file").arg(filename);
}
cmd
.assert()
.code(if required_fixes > 0 { 2 } else { 0 });
@ -106,17 +165,26 @@ mod integration {
} else {
expected_fixes
};
eprintln!("{} expected fixes, {} required fixes",
expected_fixes, required_fixes);
let mut cmd = sq();
let mut cmd = cmd.current_dir(&dir)
.args(&[
"--no-cert-store",
"--no-key-store",
"--home", &home,
"cert", "lint",
"--time", FROZEN_TIME,
"--fix",
"--cert-file", &format!("{}-{}.pgp", base, suffix)
]);
if keystore {
cmd.args([
"--cert", &cert.fingerprint().to_string(),
]);
} else {
cmd.args([
"--cert-file", &format!("{}-{}.pgp", base, suffix),
]);
}
for p in passwords.iter() {
cmd = cmd.arg("-p").arg(p)
}
@ -128,17 +196,22 @@ mod integration {
// If there are no fixes, nothing is printed.
output == b""
} else {
// We got a certificate on stdout. Pass it
// through the linter.
Command::cargo_bin("sq").unwrap()
// Pass the result through the linter.
let mut cmd = Command::cargo_bin("sq").unwrap();
cmd
.current_dir(&dir)
.arg("--no-cert-store")
.arg("--no-key-store")
.arg("--home").arg(&home)
.arg("cert").arg("lint")
.arg("--time").arg(FROZEN_TIME)
.arg("--cert-file").arg("-")
.write_stdin(output)
.assert()
.arg("--time").arg(FROZEN_TIME);
if keystore {
cmd.arg("--cert")
.arg(&cert.fingerprint().to_string());
} else {
cmd.arg("--cert-file").arg("-")
.write_stdin(output);
}
cmd.assert()
.code(
if expected_fixes == required_fixes {
// Everything should have been fixed.
@ -150,8 +223,8 @@ mod integration {
// Check that the number of new signatures equals
// the number of expected new signatures.
let orig_sigs: isize =
Cert::from_file(dir.clone().join(filename)).unwrap()
let orig_sigs: isize = cert
.clone()
.into_packets2()
.map(|p| {
if let Packet::Signature(_) = p {
@ -162,7 +235,31 @@ mod integration {
})
.sum();
let fixed_sigs: isize = Cert::from_bytes(output)
let updated_cert = if keystore {
let mut cmd = sq();
let cmd = cmd.current_dir(&dir)
.args(&[
"--home", &home,
"cert", "export",
"--cert", &cert.fingerprint().to_string(),
]);
let output = cmd.output()
.expect(&format!("Can run sq cert export"));
if !output.status.success() {
panic!(
"sq exited with non-zero status code: {}",
String::from_utf8_lossy(&output.stderr)
);
}
Cert::from_bytes(&output.stdout)
} else {
// When not using the keystore, `sq
// cert lint --fix` emits the fixed
// certificate on stdout.
Cert::from_bytes(output)
};
let fixed_sigs: isize = updated_cert
.map(|cert| {
cert.into_packets2()
.map(|p| {
@ -196,6 +293,7 @@ mod integration {
}));
}
}
}
#[test]
fn known_good() {