From 50b3ed9a61161761f20d134b13c2f8ac07698ea5 Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Thu, 7 May 2020 14:16:08 +0200 Subject: openpgp: Use a builder to construct Verifier. - See #498. --- guide/src/chapter_01.md | 20 +++-- ipc/tests/gpg-agent.rs | 22 +++-- openpgp-ffi/src/parse/stream.rs | 6 +- openpgp/examples/generate-sign-verify.rs | 5 +- openpgp/src/parse/stream.rs | 148 ++++++++++++++++--------------- openpgp/src/policy.rs | 30 ++----- openpgp/src/serialize/stream.rs | 3 +- tool/src/commands/mod.rs | 3 +- 8 files changed, 121 insertions(+), 116 deletions(-) diff --git a/guide/src/chapter_01.md b/guide/src/chapter_01.md index 0aa5dbf9..3d2af662 100644 --- a/guide/src/chapter_01.md +++ b/guide/src/chapter_01.md @@ -16,7 +16,7 @@ extern crate sequoia_openpgp as openpgp; use openpgp::cert::prelude::*; use openpgp::serialize::stream::*; use openpgp::packet::prelude::*; -use openpgp::parse::stream::*; +use openpgp::parse::{Parse, stream::*}; use openpgp::policy::Policy; use openpgp::policy::StandardPolicy as P; @@ -92,7 +92,8 @@ fn main() { # }; # # // Now, create a verifier with a helper using the given Certs. -# let mut verifier = Verifier::from_bytes(policy, signed_message, helper, None)?; +# let mut verifier = VerifierBuilder::from_bytes(signed_message)? +# .with_policy(policy, None, helper)?; # # // Verify the data. # io::copy(&mut verifier, sink)?; @@ -164,7 +165,7 @@ create it: # use openpgp::cert::prelude::*; # use openpgp::serialize::stream::*; # use openpgp::packet::prelude::*; -# use openpgp::parse::stream::*; +# use openpgp::parse::{Parse, stream::*}; # use openpgp::policy::Policy; # use openpgp::policy::StandardPolicy as P; # @@ -240,7 +241,8 @@ fn generate() -> openpgp::Result { # }; # # // Now, create a verifier with a helper using the given Certs. -# let mut verifier = Verifier::from_bytes(policy, signed_message, helper, None)?; +# let mut verifier = VerifierBuilder::from_bytes(signed_message)? +# .with_policy(policy, None, helper)?; # # // Verify the data. # io::copy(&mut verifier, sink)?; @@ -312,7 +314,7 @@ implements [`io::Write`], and we simply write the plaintext to it. # use openpgp::cert::prelude::*; # use openpgp::serialize::stream::*; # use openpgp::packet::prelude::*; -# use openpgp::parse::stream::*; +# use openpgp::parse::{Parse, stream::*}; # use openpgp::policy::Policy; # use openpgp::policy::StandardPolicy as P; # @@ -388,7 +390,8 @@ fn sign(policy: &dyn Policy, # }; # # // Now, create a verifier with a helper using the given Certs. -# let mut verifier = Verifier::from_bytes(policy, signed_message, helper, None)?; +# let mut verifier = VerifierBuilder::from_bytes(signed_message)? +# .with_policy(policy, None, helper)?; # # // Verify the data. # io::copy(&mut verifier, sink)?; @@ -471,7 +474,7 @@ Verified data can be read from this using [`io::Read`]. # use openpgp::cert::prelude::*; # use openpgp::serialize::stream::*; # use openpgp::packet::prelude::*; -# use openpgp::parse::stream::*; +# use openpgp::parse::{Parse, stream::*}; # use openpgp::policy::Policy; # use openpgp::policy::StandardPolicy as P; # @@ -547,7 +550,8 @@ fn verify(policy: &dyn Policy, }; // Now, create a verifier with a helper using the given Certs. - let mut verifier = Verifier::from_bytes(policy, signed_message, helper, None)?; + let mut verifier = VerifierBuilder::from_bytes(signed_message)? + .with_policy(policy, None, helper)?; // Verify the data. io::copy(&mut verifier, sink)?; diff --git a/ipc/tests/gpg-agent.rs b/ipc/tests/gpg-agent.rs index 3048a136..ab3a6b4a 100644 --- a/ipc/tests/gpg-agent.rs +++ b/ipc/tests/gpg-agent.rs @@ -12,7 +12,7 @@ use crate::openpgp::types::{ SymmetricAlgorithm, }; use crate::openpgp::crypto::SessionKey; -use crate::openpgp::parse::stream::*; +use crate::openpgp::parse::{Parse, stream::*}; use crate::openpgp::serialize::{Serialize, stream::*}; use crate::openpgp::cert::prelude::*; use crate::openpgp::policy::Policy; @@ -27,7 +27,7 @@ macro_rules! make_context { Err(e) => { eprintln!("SKIP: Failed to create GnuPG context: {}\n\ SKIP: Is GnuPG installed?", e); - return; + return Ok(()); }, }; match ctx.start("gpg-agent") { @@ -35,7 +35,7 @@ macro_rules! make_context { Err(e) => { eprintln!("SKIP: Failed to create GnuPG context: {}\n\ SKIP: Is the GnuPG agent installed?", e); - return; + return Ok(()); }, } ctx @@ -43,23 +43,25 @@ macro_rules! make_context { } #[test] -fn nop() { +fn nop() -> openpgp::Result<()> { let ctx = make_context!(); let mut agent = Agent::connect(&ctx).wait().unwrap(); agent.send("NOP").unwrap(); let response = agent.wait().collect::>(); assert_eq!(response.len(), 1); assert!(response[0].is_ok()); + Ok(()) } #[test] -fn help() { +fn help() -> openpgp::Result<()> { let ctx = make_context!(); let mut agent = Agent::connect(&ctx).wait().unwrap(); agent.send("HELP").unwrap(); let response = agent.wait().collect::>(); assert!(response.len() > 3); assert!(response.iter().last().unwrap().is_ok()); + Ok(()) } const MESSAGE: &'static str = "дружба"; @@ -79,7 +81,7 @@ fn gpg_import(ctx: &Context, what: &[u8]) { } #[test] -fn sign() { +fn sign() -> openpgp::Result<()> { use self::CipherSuite::*; use openpgp::policy::StandardPolicy as P; @@ -131,8 +133,8 @@ fn sign() { let helper = Helper { cert: &cert }; // Now, create a verifier with a helper using the given Certs. - let mut verifier = - Verifier::from_bytes(p, &message, helper, None).unwrap(); + let mut verifier = VerifierBuilder::from_bytes(&message)? + .with_policy(p, None, helper)?; // Verify the data. let mut sink = Vec::new(); @@ -184,10 +186,11 @@ fn sign() { } } } + Ok(()) } #[test] -fn decrypt() { +fn decrypt() -> openpgp::Result<()> { use self::CipherSuite::*; use openpgp::policy::StandardPolicy as P; @@ -291,4 +294,5 @@ fn decrypt() { } } } + Ok(()) } diff --git a/openpgp-ffi/src/parse/stream.rs b/openpgp-ffi/src/parse/stream.rs index 227b75a4..a0a65df3 100644 --- a/openpgp-ffi/src/parse/stream.rs +++ b/openpgp-ffi/src/parse/stream.rs @@ -30,7 +30,7 @@ use self::openpgp::parse::stream::{ DecryptionHelper, Decryptor, VerificationHelper, - Verifier, + VerifierBuilder, }; use crate::Maybe; @@ -643,10 +643,12 @@ fn pgp_verifier_new<'a>(errp: Option<&mut *mut crate::error::Error>, time: time_t) -> Maybe { + ffi_make_fry_from_errp!(errp); let policy = policy.ref_raw().as_ref(); let helper = VHelper::new(inspect, get_certs, check, cookie); - Verifier::from_reader(policy, input.ref_mut_raw(), helper, maybe_time(time)) + ffi_try_or!(VerifierBuilder::from_reader(input.ref_mut_raw()), None) + .with_policy(policy, maybe_time(time), helper) .map(|r| io::ReaderKind::Generic(Box::new(r))) .move_into_raw(errp) } diff --git a/openpgp/examples/generate-sign-verify.rs b/openpgp/examples/generate-sign-verify.rs index ae4f174f..f638b075 100644 --- a/openpgp/examples/generate-sign-verify.rs +++ b/openpgp/examples/generate-sign-verify.rs @@ -5,7 +5,7 @@ use std::io::{self, Write}; extern crate sequoia_openpgp as openpgp; use crate::openpgp::cert::prelude::*; use crate::openpgp::serialize::stream::*; -use crate::openpgp::parse::stream::*; +use crate::openpgp::parse::{Parse, stream::*}; use crate::openpgp::policy::Policy; use crate::openpgp::policy::StandardPolicy as P; @@ -79,7 +79,8 @@ fn verify(p: &dyn Policy, sink: &mut dyn Write, }; // Now, create a verifier with a helper using the given Certs. - let mut verifier = Verifier::from_bytes(p, signed_message, helper, None)?; + let mut verifier = VerifierBuilder::from_bytes(signed_message)? + .with_policy(p, None, helper)?; // Verify the data. io::copy(&mut verifier, sink)?; diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs index f776b283..c5da1343 100644 --- a/openpgp/src/parse/stream.rs +++ b/openpgp/src/parse/stream.rs @@ -71,7 +71,7 @@ //! use std::io::Read; //! use sequoia_openpgp as openpgp; //! use openpgp::{KeyHandle, Cert, Result}; -//! use openpgp::parse::stream::*; +//! use openpgp::parse::{Parse, stream::*}; //! use openpgp::policy::StandardPolicy; //! //! let p = &StandardPolicy::new(); @@ -98,7 +98,8 @@ //! -----END PGP MESSAGE-----"; //! //! let h = Helper {}; -//! let mut v = Verifier::from_bytes(p, message, h, None)?; +//! let mut v = VerifierBuilder::from_bytes(&message[..])? +//! .with_policy(p, None, h)?; //! //! let mut content = Vec::new(); //! v.read_to_end(&mut content)?; @@ -761,7 +762,8 @@ impl DecryptionHelper for NoDecryptionHelper { /// "; /// /// let h = Helper {}; -/// let mut v = Verifier::from_bytes(p, message, h, None)?; +/// let mut v = VerifierBuilder::from_bytes(&message[..])? +/// .with_policy(p, None, h)?; /// /// let mut content = Vec::new(); /// v.read_to_end(&mut content)?; @@ -771,60 +773,76 @@ pub struct Verifier<'a, H: VerificationHelper> { decryptor: Decryptor<'a, NoDecryptionHelper>, } -impl<'a, H: VerificationHelper> Verifier<'a, H> { - /// Creates a `Verifier` from the given reader. - /// - /// Signature verifications are done relative to time `t`, or the - /// current time, if `t` is `None`. - pub fn from_reader(policy: &'a dyn Policy, reader: R, helper: H, t: T) - -> Result> - where R: io::Read + 'a, T: Into> +/// A builder for `Verifier`. +/// +/// This allows the customization of [`Verifier`], which can +/// be built using [`VerifierBuilder::with_policy`]. +/// +/// [`Verifier`]: struct.Verifier.html +/// [`VerifierBuilder::with_policy`]: struct.VerifierBuilder.html#method.with_policy +pub struct VerifierBuilder<'a> { + message: Box + 'a>, +} + +impl<'a> Parse<'a, VerifierBuilder<'a>> + for VerifierBuilder<'a> +{ + fn from_reader(reader: R) -> Result> + where R: io::Read + 'a, { - // Do not eagerly map `t` to the current time. - let t = t.into(); - Verifier::from_buffered_reader( - policy, - Box::new(buffered_reader::Generic::with_cookie(reader, None, - Default::default())), - helper, t) + VerifierBuilder::new(buffered_reader::Generic::with_cookie( + reader, None, Default::default())) } - /// Creates a `Verifier` from the given file. - /// - /// Signature verifications are done relative to time `t`, or the - /// current time, if `t` is `None`. - pub fn from_file(policy: &'a dyn Policy, path: P, helper: H, t: T) - -> Result> + fn from_file

(path: P) -> Result> where P: AsRef, - T: Into> { - // Do not eagerly map `t` to the current time. - let t = t.into(); - Verifier::from_buffered_reader( - policy, - Box::new(buffered_reader::File::with_cookie(path, - Default::default())?), - helper, t) + VerifierBuilder::new(buffered_reader::File::with_cookie( + path, Default::default())?) + } + + fn from_bytes(data: &'a D) -> Result> + where D: AsRef<[u8]> + ?Sized, + { + VerifierBuilder::new(buffered_reader::Memory::with_cookie( + data.as_ref(), Default::default())) + } +} + +impl<'a> VerifierBuilder<'a> { + fn new(signatures: B) -> Result + where B: buffered_reader::BufferedReader + 'a + { + Ok(VerifierBuilder { + message: Box::new(signatures), + }) } - /// Creates a `Verifier` from the given buffer. + /// Creates the `Verifier`. /// - /// Signature verifications are done relative to time `t`, or the - /// current time, if `t` is `None`. - pub fn from_bytes(policy: &'a dyn Policy, - bytes: &'a [u8], helper: H, t: T) - -> Result> - where T: Into> + /// Signature verifications are done under the given `policy` and + /// relative to time `time`, or the current time, if `time` is + /// `None`. `helper` is the [`VerificationHelper`] to use. + /// + /// [`VerificationHelper`]: trait.VerificationHelper.html + pub fn with_policy(self, policy: &'a dyn Policy, time: T, helper: H) + -> Result> + where H: VerificationHelper, + T: Into>, { // Do not eagerly map `t` to the current time. - let t = t.into(); - Verifier::from_buffered_reader( - policy, - Box::new(buffered_reader::Memory::with_cookie(bytes, - Default::default())), - helper, t) + let t = time.into(); + Ok(Verifier { + decryptor: Decryptor::from_buffered_reader( + policy, + self.message, + NoDecryptionHelper { v: helper, }, + t, Mode::Verify)?, + }) } +} +impl<'a, H: VerificationHelper> Verifier<'a, H> { /// Returns a reference to the helper. pub fn helper_ref(&self) -> &H { &self.decryptor.helper_ref().v @@ -849,23 +867,6 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { pub fn message_processed(&self) -> bool { self.decryptor.message_processed() } - - /// Creates the `Verifier`, and buffers the data up to `BUFFER_SIZE`. - /// - /// Signature verifications are done relative to time `t`, or the - /// current time, if `t` is `None`. - pub(crate) fn from_buffered_reader(policy: &'a dyn Policy, - bio: Box + 'a>, - helper: H, time: T) - -> Result> - where T: Into> - { - Ok(Verifier { - decryptor: Decryptor::from_buffered_reader( - policy, bio, NoDecryptionHelper { v: helper, }, time, - Mode::Verify)?, - }) - } } impl<'a, H: VerificationHelper> io::Read for Verifier<'a, H> { @@ -1981,7 +1982,7 @@ mod test { } #[test] - fn verifier() { + fn verifier() -> Result<()> { let p = P::new(); let keys = [ @@ -2005,8 +2006,8 @@ mod test { // Test Verifier. let h = VHelper::new(0, 0, 0, 0, keys.clone()); let mut v = - match Verifier::from_bytes(&p, crate::tests::file(f), h, - crate::frozen_time()) { + match VerifierBuilder::from_bytes(crate::tests::file(f))? + .with_policy(&p, crate::frozen_time(), h) { Ok(v) => v, Err(e) => if r.error > 0 || r.unknown > 0 { // Expected error. No point in trying to read @@ -2058,12 +2059,13 @@ mod test { assert_eq!(reference.len(), content.len()); assert_eq!(reference, &content[..]); } + Ok(()) } /// Tests the order of signatures given to /// VerificationHelper::check(). #[test] - fn verifier_levels() { + fn verifier_levels() -> Result<()> { let p = P::new(); struct VHelper(()); @@ -2112,9 +2114,9 @@ mod test { } // Test verifier. - let v = Verifier::from_bytes( - &p, crate::tests::message("signed-1-notarized-by-ed25519.pgp"), - VHelper(()), crate::frozen_time()).unwrap(); + let v = VerifierBuilder::from_bytes( + crate::tests::message("signed-1-notarized-by-ed25519.pgp"))? + .with_policy(&p, crate::frozen_time(), VHelper(()))?; assert!(v.message_processed()); // Test decryptor. @@ -2122,6 +2124,7 @@ mod test { &p, crate::tests::message("signed-1-notarized-by-ed25519.pgp"), VHelper(()), crate::frozen_time()).unwrap(); assert!(v.message_processed()); + Ok(()) } #[test] @@ -2176,7 +2179,7 @@ mod test { } #[test] - fn verify_long_message() { + fn verify_long_message() -> Result<()> { use std::io::Write; use crate::serialize::stream::{LiteralWriter, Signer, Message}; @@ -2205,7 +2208,7 @@ mod test { // Test Verifier. let h = VHelper::new(0, 0, 0, 0, vec![cert.clone()]); - let mut v = Verifier::from_bytes(p, &buf, h, None).unwrap(); + let mut v = VerifierBuilder::from_bytes(&buf)?.with_policy(p, None, h)?; assert!(!v.message_processed()); assert!(v.helper_ref().good == 0); @@ -2228,7 +2231,7 @@ mod test { // Try the same, but this time we let .check() fail. let h = VHelper::new(0, 0, /* makes check() fail: */ 1, 0, vec![cert.clone()]); - let mut v = Verifier::from_bytes(p, &buf, h, None).unwrap(); + let mut v = VerifierBuilder::from_bytes(&buf)?.with_policy(p, None, h)?; assert!(!v.message_processed()); assert!(v.helper_ref().good == 0); @@ -2296,5 +2299,6 @@ mod test { assert!(v.helper_ref().bad == 1); assert!(v.helper_ref().unknown == 0); assert!(v.helper_ref().error == 0); + Ok(()) } } diff --git a/openpgp/src/policy.rs b/openpgp/src/policy.rs index 6aaedb64..f5be5ce9 100644 --- a/openpgp/src/policy.rs +++ b/openpgp/src/policy.rs @@ -982,7 +982,7 @@ mod test { use crate::parse::stream::MessageLayer; use crate::parse::stream::MessageStructure; use crate::parse::stream::VerificationHelper; - use crate::parse::stream::Verifier; + use crate::parse::stream::VerifierBuilder; use crate::policy::StandardPolicy as P; use crate::types::Curve; use crate::types::KeyFlags; @@ -1151,7 +1151,7 @@ mod test { #[test] - fn binary_signature() { + fn binary_signature() -> Result<()> { #[derive(PartialEq, Debug)] struct VHelper { good: usize, @@ -1255,12 +1255,8 @@ mod test { // Standard policy => ok. let h = VHelper::new(keys.clone()); - let mut v = - match Verifier::from_bytes(standard, crate::tests::file(data), h, - crate::frozen_time()) { - Ok(v) => v, - Err(e) => panic!("{}", e), - }; + let mut v = VerifierBuilder::from_bytes(crate::tests::file(data))? + .with_policy(standard, crate::frozen_time(), h)?; assert!(v.message_processed()); assert_eq!(v.helper_ref().good, 1); assert_eq!(v.helper_ref().errors, 0); @@ -1273,12 +1269,8 @@ mod test { // Kill the subkey. let h = VHelper::new(keys.clone()); - let mut v = match Verifier::from_bytes(no_subkey_signatures, - crate::tests::file(data), h, - crate::frozen_time()) { - Ok(v) => v, - Err(e) => panic!("{}", e), - }; + let mut v = VerifierBuilder::from_bytes(crate::tests::file(data))? + .with_policy(no_subkey_signatures, crate::frozen_time(), h)?; assert!(v.message_processed()); assert_eq!(v.helper_ref().good, 0); assert_eq!(v.helper_ref().errors, 1); @@ -1291,13 +1283,8 @@ mod test { // Kill the data signature. let h = VHelper::new(keys.clone()); - let mut v = - match Verifier::from_bytes(no_binary_signatures, - crate::tests::file(data), h, - crate::frozen_time()) { - Ok(v) => v, - Err(e) => panic!("{}", e), - }; + let mut v = VerifierBuilder::from_bytes(crate::tests::file(data))? + .with_policy(no_binary_signatures, crate::frozen_time(), h)?; assert!(v.message_processed()); assert_eq!(v.helper_ref().good, 0); assert_eq!(v.helper_ref().errors, 1); @@ -1364,6 +1351,7 @@ mod test { v.read_to_end(&mut content).unwrap(); assert_eq!(reference.len(), content.len()); assert_eq!(reference, &content[..]); + Ok(()) } #[test] diff --git a/openpgp/src/serialize/stream.rs b/openpgp/src/serialize/stream.rs index ba21f764..b0fccf93 100644 --- a/openpgp/src/serialize/stream.rs +++ b/openpgp/src/serialize/stream.rs @@ -713,7 +713,8 @@ impl<'a> Signer<'a> { /// } /// } /// - /// let mut verifier = Verifier::from_bytes(p, &sink, Helper(&cert), None)?; + /// let mut verifier = VerifierBuilder::from_bytes(&sink)? + /// .with_policy(p, None, Helper(&cert))?; /// /// let mut message = String::new(); /// verifier.read_to_string(&mut message)?; diff --git a/tool/src/commands/mod.rs b/tool/src/commands/mod.rs index 1639c087..e7851f70 100644 --- a/tool/src/commands/mod.rs +++ b/tool/src/commands/mod.rs @@ -390,7 +390,8 @@ pub fn verify(ctx: &Context, policy: &dyn Policy, v.verify_reader(input)?; v.into_helper() } else { - let mut v = Verifier::from_reader(policy, input, helper, None)?; + let mut v = VerifierBuilder::from_reader(input)? + .with_policy(policy, None, helper)?; io::copy(&mut v, output)?; v.into_helper() }; -- cgit v1.2.3