From ebef0cf9eed674a07ee32339935e5436036a4fad Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Wed, 11 Dec 2024 16:22:09 +0100 Subject: [PATCH] Use cert designators for `sq cert list`. - This aligns it with `sq key list` and `sq pki link list`. - Fixes #446. --- NEWS | 5 +++ src/cli/cert/list.rs | 45 +++++++++++++------- src/cli/types/userid_designator.rs | 3 -- src/commands/cert.rs | 67 +++++++++++++++++++++--------- src/commands/download.rs | 1 + src/commands/pki.rs | 6 +-- src/common/pki/authenticate.rs | 9 ++++ tests/integration/common.rs | 19 +++++++++ tests/integration/sq_cert_list.rs | 8 ++-- tests/integration/sq_pki.rs | 30 ++++++++++--- 10 files changed, 144 insertions(+), 49 deletions(-) diff --git a/NEWS b/NEWS index 6d2649fb..e28e4764 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,11 @@ #+TITLE: sequoia-sq NEWS – history of user-visible changes #+STARTUP: content hidestars +* Changes in 1.0.0 +** Notable changes + - `sq cert list` now takes cert designators, like `--cert-email` + instead of `--email`. + * Changes in 0.41.0 ** New functionality - `sq encrypt --for-self` now adds the certs configured under diff --git a/src/cli/cert/list.rs b/src/cli/cert/list.rs index 14f93572..deeda969 100644 --- a/src/cli/cert/list.rs +++ b/src/cli/cert/list.rs @@ -8,8 +8,7 @@ use crate::cli::pki::CertificationNetworkArg; use crate::cli::pki::GossipArg; use crate::cli::pki::RequiredTrustAmountArg; use crate::cli::pki::ShowPathsArg; -use crate::cli::types::UserIDDesignators; -use crate::cli::types::userid_designator; +use crate::cli::types::cert_designator::*; const EXAMPLES: Actions = Actions { actions: &[ @@ -29,13 +28,15 @@ const EXAMPLES: Actions = Actions { Action::example().comment( "List all authenticated bindings for User IDs containing a specific email address." ).command (&[ - "sq", "cert", "list", "--email", "alice@example.org", + "sq", "cert", "list", + "--cert-email=alice@example.org", ]).build(), Action::example().comment( "List all paths to certificates containing a specific email address." ).command (&[ - "sq", "cert", "list", "--gossip", "--show-paths", "--email", "alice@example.org", + "sq", "cert", "list", "--gossip", "--show-paths", + "--cert-email=alice@example.org", ]).build(), ] }; @@ -62,18 +63,24 @@ test_examples!(sq_cert_list, EXAMPLES); )] pub struct Command { #[command(flatten)] - pub userid: UserIDDesignators< - userid_designator::AnyUserIDEmailArgs, - userid_designator::OptionalValueNoLinting>, + pub certs: CertDesignators, - /// A pattern to select the bindings to authenticate. - /// - /// The pattern is treated as a UTF-8 encoded string and a - /// case insensitive substring search (using the current - /// locale) is performed against each User ID. If a User ID - /// is not valid UTF-8, the binding is ignored. #[clap( - conflicts_with_all = &[ "userid", "email" ] + value_name = "FINGERPRINT|KEYID|PATTERN", + help = "A pattern to filter the displayed certificates", + long_help = "\ +A pattern to filter the displayed certificates. + +If the pattern appears to be a fingerprint or key ID, it is treated as \ +if it were passed to `--cert`, and matches on the certificate's \ +fingerprint. Otherwise, it is treated as if it were passed via \ +`--cert-grep`, and matches on user IDs. +", + conflicts_with_all = &["cert", "cert-userid", "cert-email", + "cert-domain", "cert-grep"], )] pub pattern: Option, @@ -89,3 +96,13 @@ pub struct Command { #[command(flatten)] pub trust_amount: RequiredTrustAmountArg, } + +/// Documentation for the cert designators for the cert list. +pub struct ListCertDoc {} + +impl AdditionalDocs for ListCertDoc { + fn help(_: &'static str, help: &'static str) -> clap::builder::StyledStr { + debug_assert!(help.starts_with("Use certificates")); + help.replace("Use certificates", "List certs").into() + } +} diff --git a/src/cli/types/userid_designator.rs b/src/cli/types/userid_designator.rs index e5daec6f..a692b3cd 100644 --- a/src/cli/types/userid_designator.rs +++ b/src/cli/types/userid_designator.rs @@ -118,9 +118,6 @@ pub type AllMatchesNonSelfSigned = typenum::U8; pub type OneValueNoLinting = >::Output; -pub type OptionalValueNoLinting - = >::Output; - pub type AllMatchesNonSelfSignedNoLinting = >::Output; diff --git a/src/commands/cert.rs b/src/commands/cert.rs index f845ab63..47ceae87 100644 --- a/src/commands/cert.rs +++ b/src/commands/cert.rs @@ -1,12 +1,16 @@ //! Operations on certs. use sequoia_openpgp as openpgp; -use openpgp::KeyHandle; +use openpgp::{ + Fingerprint, + KeyHandle, +}; use crate::{ Sq, Result, cli::cert::{Command, list, Subcommands}, + cli::types::cert_designator, common::pki::authenticate, }; @@ -25,28 +29,53 @@ pub fn dispatch(sq: Sq, command: Command) -> Result<()> // List all authenticated bindings. Subcommands::List(list::Command { - userid, pattern, gossip, certification_network, trust_amount, + mut certs, pattern, gossip, certification_network, trust_amount, show_paths, }) => { - let userid = userid.designators.into_iter().next(); + let pattern = if let Some(pattern) = pattern { + let mut d = None; + if let Ok(kh) = pattern.parse::() { + if matches!(kh, KeyHandle::Fingerprint(Fingerprint::Invalid(_))) { + let hex = pattern.chars() + .map(|c| { + if c == ' ' { 0 } else { 1 } + }) + .sum::(); - if let Some(kh) = pattern.as_ref() - .and_then(|p| p.parse::().ok()) - { - let cert = sq.resolve_cert(&kh.into(), 0)?.0; - authenticate( - &mut std::io::stdout(), - &sq, false, None, - *gossip, *certification_network, *trust_amount, - userid.as_ref(), Some(&cert), *show_paths) + if hex >= 16 { + weprintln!("Warning: {} looks like a fingerprint or key ID, \ + but its invalid. Treating it as a text pattern.", + pattern); + } + } else { + d = Some(cert_designator::CertDesignator::Cert(kh)); + } + }; + + if let Some(d) = d { + certs.push(d); + None + } else { + certs.push( + cert_designator::CertDesignator::Grep(pattern.clone())); + Some(pattern) + } } else { - authenticate( - &mut std::io::stdout(), - &sq, pattern.is_none(), pattern, - *gossip, *certification_network, *trust_amount, - userid.as_ref(), None, *show_paths) - } - } + None + }; + + let certs = sq.resolve_certs_or_fail( + &certs, trust_amount.map(|t| t.amount()) + .unwrap_or(sequoia_wot::FULLY_TRUSTED))?; + + authenticate( + &mut std::io::stdout(), + &sq, certs.is_empty(), pattern, + *gossip, *certification_network, *trust_amount, + None, None, + (! certs.is_empty()).then_some(certs), + *show_paths) + }, Subcommands::Lint(command) => lint::lint(sq, command), diff --git a/src/commands/download.rs b/src/commands/download.rs index dd7ad058..abca0c86 100644 --- a/src/commands/download.rs +++ b/src/commands/download.rs @@ -403,6 +403,7 @@ pub fn dispatch(sq: Sq, c: download::Command) Some(TrustAmount::Full), // trust amount None, // user ID Some(&cert), + None, true, // show paths ).is_ok(); diff --git a/src/commands/pki.rs b/src/commands/pki.rs index 6abdbf4c..a5c209be 100644 --- a/src/commands/pki.rs +++ b/src/commands/pki.rs @@ -34,7 +34,7 @@ pub fn dispatch(sq: Sq, cli: cli::pki::Command, matches: &ArgMatches) &mut std::io::stdout(), &sq, false, None, *gossip, *certification_network, *trust_amount, - Some(&userid), Some(&cert), *show_paths, + Some(&userid), Some(&cert), None, *show_paths, )? } @@ -51,7 +51,7 @@ pub fn dispatch(sq: Sq, cli: cli::pki::Command, matches: &ArgMatches) &mut std::io::stdout(), &sq, false, None, *gossip, *certification_network, *trust_amount, - Some(&userid), None, *show_paths)?; + Some(&userid), None, None, *show_paths)?; } // Find and list all authenticated bindings for a given @@ -66,7 +66,7 @@ pub fn dispatch(sq: Sq, cli: cli::pki::Command, matches: &ArgMatches) &mut std::io::stdout(), &sq, false, None, *gossip, *certification_network, *trust_amount, - None, Some(&cert), *show_paths)?; + None, Some(&cert), None, *show_paths)?; } // Authenticates a given path. diff --git a/src/common/pki/authenticate.rs b/src/common/pki/authenticate.rs index 8ed362ca..856eca1b 100644 --- a/src/common/pki/authenticate.rs +++ b/src/common/pki/authenticate.rs @@ -76,6 +76,7 @@ pub fn authenticate<'store, 'rstore>( trust_amount: Option>, userid_designator: Option<&userid_designator::UserIDDesignator>, certificate: Option<&Cert>, + certs: Option>, show_paths: bool, ) -> Result<()> where 'store: 'rstore, @@ -169,6 +170,14 @@ pub fn authenticate<'store, 'rstore>( .into_iter() .map(|fpr| (fpr, UserID::from(userid))) .collect(); + } else if let Some(certs) = certs { + // List all certs. + t!("Authenticating given certs"); + bindings = certs.iter().flat_map(|cert| { + let fp = cert.fingerprint(); + let userids = n.certified_userids_of(&fp); + userids.into_iter().map(move |uid| (fp.clone(), uid)) + }).collect(); } else { // No User ID, no Fingerprint. // List everything. diff --git a/tests/integration/common.rs b/tests/integration/common.rs index 75f92972..54d2e9ca 100644 --- a/tests/integration/common.rs +++ b/tests/integration/common.rs @@ -285,6 +285,25 @@ impl UserIDArg<'_> { cmd.arg("--email-or-add").arg(email), }; } + + /// Add the argument to a `Command` as a cert designator. + /// + /// Note: only a subset of user ID arguments are expressible as + /// cert designators. + pub fn as_cert_designator(&self, cmd: &mut Command) { + match self { + UserIDArg::UserID(userid) => + cmd.arg("--cert-userid").arg(userid), + UserIDArg::Email(email) => + cmd.arg("--cert-email").arg(email), + UserIDArg::Name(name) => + unreachable!(), + UserIDArg::AddUserID(userid) => + unreachable!(), + UserIDArg::AddEmail(email) => + unreachable!(), + }; + } } impl std::fmt::Display for UserIDArg<'_> { diff --git a/tests/integration/sq_cert_list.rs b/tests/integration/sq_cert_list.rs index 7e90ba32..f6899992 100644 --- a/tests/integration/sq_cert_list.rs +++ b/tests/integration/sq_cert_list.rs @@ -18,11 +18,11 @@ fn list() { // By user ID. sq.cert_list(&[userid]); - sq.cert_list(&["--userid", userid]); + sq.cert_list(&["--cert-userid", userid]); // By email. sq.cert_list(&[email]); - sq.cert_list(&["--email", email]); + sq.cert_list(&["--cert-email", email]); // By name. sq.cert_list(&[name]); @@ -34,10 +34,10 @@ fn list() { sq.cert_list(&["ExAmPlE.Or"]); // When we use --userid, then we don't do substring matching. - assert!(sq.cert_list_maybe(&["--userid", &userid[1..]]).is_err()); + assert!(sq.cert_list_maybe(&["--cert-userid", &userid[1..]]).is_err()); // When we use --email, then we don't do substring matching. - assert!(sq.cert_list_maybe(&["--email", &email[1..]]).is_err()); + assert!(sq.cert_list_maybe(&["--cert-email", &email[1..]]).is_err()); } #[test] diff --git a/tests/integration/sq_pki.rs b/tests/integration/sq_pki.rs index 9d61a150..fcdf38f4 100644 --- a/tests/integration/sq_pki.rs +++ b/tests/integration/sq_pki.rs @@ -121,13 +121,31 @@ where cmd.arg(&target.to_string()); } if let Some(userid) = userid { - userid.as_arg(&mut cmd); + if command == "list" { + userid.as_cert_designator(&mut cmd); + } else { + userid.as_arg(&mut cmd); + } } for arg in args { cmd.arg(arg); } cmd.args(&["--amount", &format!("{}", amount)]); + eprintln!("Running: {}", + std::iter::once(cmd.get_program()) + .chain(cmd.get_args()) + .map(|arg| { + let arg = arg.to_string_lossy(); + if arg.contains(" ") { + format!("{:?}", arg) + } else { + arg.into_owned() + } + }) + .collect::>() + .join(" ")); + if success { let assertion = cmd.assert(); let assertion = assertion.success(); @@ -140,8 +158,8 @@ where assert_eq!( occurrences, *expected_occurrences, - "Failed to find: '{}' {} times\n\ - in output:\n\ + "Failed to find: '{}' {} times \ + in output:\n\n\ {}", s, expected_occurrences, String::from_utf8_lossy(haystack), @@ -160,8 +178,8 @@ where assert_eq!( occurrences, *expected_occurrences, - "Failed to find: '{}' {} times\n\ - in output:\n\ + "Failed to find: '{}' {} times \ + in output:\n\n\ {}", s, expected_occurrences, String::from_utf8_lossy(haystack), @@ -1316,7 +1334,7 @@ fn list_pattern() -> Result<()> { .map(|(userid, target)| { (1, format!("- {} {}", HR_OK, userid).to_string()) }) - .chain(vec![(bindings.len(), HR_OK.to_string())].into_iter()) + .chain(vec![(3, HR_OK.to_string())].into_iter()) .collect::>(); test(