From 1fa018acc90194e1cd1ddf2bd0c6706000a3bc9e Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Fri, 26 Apr 2019 14:34:36 +0200 Subject: openpgp: New TSK type. - With a1e226f8f1418de43e577fdaa1d087b68bbb09ae in place, we have a more general way to add components to a TPK. Retire the current `TSK` type and replace it with a thin shim that only allows serialization of secret keys. - Fixes #107. --- openpgp-ffi/examples/Makefile | 1 + openpgp-ffi/examples/generate-key.c | 37 +++++ openpgp-ffi/include/sequoia/openpgp.h | 61 +------- openpgp-ffi/src/tpk.rs | 12 +- openpgp-ffi/src/tsk.rs | 59 +------ openpgp/src/autocrypt.rs | 4 +- openpgp/src/lib.rs | 13 +- openpgp/src/tpk/mod.rs | 14 +- openpgp/src/tpk/tsk.rs | 280 ++++++++++++++++++++++++++++++++++ openpgp/src/tsk.rs | 3 + openpgp/tests/for-each-artifact.rs | 5 +- tool/src/commands/key.rs | 3 +- 12 files changed, 362 insertions(+), 130 deletions(-) create mode 100644 openpgp-ffi/examples/generate-key.c create mode 100644 openpgp/src/tpk/tsk.rs diff --git a/openpgp-ffi/examples/Makefile b/openpgp-ffi/examples/Makefile index 77e5aad9..f5dd0f8c 100644 --- a/openpgp-ffi/examples/Makefile +++ b/openpgp-ffi/examples/Makefile @@ -11,6 +11,7 @@ EXAMPLE_TARGET_DIR ?= $(CARGO_TARGET_DIR)/debug/c-examples/openpgp-ffi EXAMPLES = \ example reader parser encrypt-for armor \ decrypt-with \ + generate-key \ type-safety-demo use-after-free-demo immutable-reference-demo \ CFLAGS = -I../include -O0 -g -Wall -Werror diff --git a/openpgp-ffi/examples/generate-key.c b/openpgp-ffi/examples/generate-key.c new file mode 100644 index 00000000..268e5593 --- /dev/null +++ b/openpgp-ffi/examples/generate-key.c @@ -0,0 +1,37 @@ +#include +#include + +#include + +int +main () { + pgp_status_t rc; + + /* First, generate the key. */ + pgp_tpk_builder_t builder = pgp_tpk_builder_default (); + pgp_tpk_builder_set_cipher_suite (&builder, PGP_TPK_CIPHER_SUITE_CV25519); + + pgp_tpk_t tpk; + pgp_signature_t revocation; + pgp_tpk_builder_generate (NULL, builder, &tpk, &revocation); + assert (tpk); + assert (revocation); + pgp_signature_free (revocation); /* Free the generated revocation. */ + + /* Now, setup an armor writer for stdout. */ + pgp_writer_t sink = pgp_writer_from_fd (STDOUT_FILENO); + pgp_writer_t armor = pgp_armor_writer_new (NULL, sink, + PGP_ARMOR_KIND_SECRETKEY, + NULL, 0); + assert (armor); + + /* Finally, derive a TSK object, and serialize it. */ + pgp_tsk_t tsk = pgp_tpk_as_tsk (tpk); + rc = pgp_tsk_serialize (NULL, tsk, armor); + assert (rc == PGP_STATUS_SUCCESS); + + pgp_tsk_free (tsk); + pgp_writer_free (armor); + pgp_writer_free (sink); + pgp_tpk_free (tpk); +} diff --git a/openpgp-ffi/include/sequoia/openpgp.h b/openpgp-ffi/include/sequoia/openpgp.h index 0d2494bd..d2af360c 100644 --- a/openpgp-ffi/include/sequoia/openpgp.h +++ b/openpgp-ffi/include/sequoia/openpgp.h @@ -675,10 +675,13 @@ pgp_fingerprint_t pgp_tpk_fingerprint (const pgp_tpk_t tpk); /*/ -/// Cast the public key into a secret key that allows using the secret -/// parts of the containing keys. +/// Derive a [`TSK`] object from this key. +/// +/// This object writes out secret keys during serialization. +/// +/// [`TSK`]: tpk/struct.TSK.html /*/ -pgp_tsk_t pgp_tpk_into_tsk (pgp_tpk_t tpk); +pgp_tsk_t pgp_tpk_as_tsk (pgp_tpk_t tpk); /*/ /// Returns a reference to the TPK's primary key. @@ -854,63 +857,11 @@ pgp_tpk_t pgp_tpk_builder_generate(pgp_error_t *errp, pgp_tpk_builder_t tpkb, /* TSK */ -/*/ -/// Generates a new RSA 3072 bit key with UID `primary_uid`. -/*/ -pgp_status_t pgp_tsk_new (pgp_error_t *errp, char *primary_uid, - pgp_tsk_t *tpk, pgp_signature_t *revocation); - -/*/ -/// Returns the first TSK encountered in the reader. -/*/ -pgp_tsk_t pgp_tsk_from_reader (pgp_error_t *errp, - pgp_reader_t reader); - -/*/ -/// Returns the first TSK encountered in the file. -/*/ -pgp_tsk_t pgp_tsk_from_file (pgp_error_t *errp, - const char *filename); - -/*/ -/// Returns the first TSK found in `buf`. -/// -/// `buf` must be an OpenPGP-encoded TSK. -/*/ -pgp_tsk_t pgp_tsk_from_bytes (pgp_error_t *errp, - const uint8_t *b, size_t len); - /*/ /// Frees the TSK. /*/ void pgp_tsk_free (pgp_tsk_t tsk); -/*/ -/// Clones the TSK. -/*/ -pgp_tsk_t pgp_tsk_clone (pgp_tsk_t message); - -/*/ -/// Returns a human readable description of this object suitable for -/// debugging. -/*/ -char *pgp_tsk_debug (const pgp_tsk_t); - -/*/ -/// Compares TPKs. -/*/ -bool pgp_tsk_equal (const pgp_tsk_t a, const pgp_tsk_t b); - -/*/ -/// Returns a reference to the corresponding TPK. -/*/ -pgp_tpk_t pgp_tsk_tpk (pgp_tsk_t tsk); - -/*/ -/// Converts the TSK into a TPK. -/*/ -pgp_tpk_t pgp_tsk_into_tpk (pgp_tsk_t tsk); - /*/ /// Serializes the TSK. /*/ diff --git a/openpgp-ffi/src/tpk.rs b/openpgp-ffi/src/tpk.rs index a1ca0ebe..75baba6d 100644 --- a/openpgp-ffi/src/tpk.rs +++ b/openpgp-ffi/src/tpk.rs @@ -128,12 +128,14 @@ fn pgp_tpk_fingerprint(tpk: *const TPK) tpk.fingerprint().move_into_raw() } -/// Cast the public key into a secret key that allows using the secret -/// parts of the containing keys. +/// Derive a [`TSK`] object from this key. +/// +/// This object writes out secret keys during serialization. +/// +/// [`TSK`]: tpk/struct.TSK.html #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "system" -fn pgp_tpk_into_tsk(tpk: *mut TPK) - -> *mut TSK { - tpk.move_from_raw().into_tsk().move_into_raw() +fn pgp_tpk_as_tsk(tpk: *const TPK) -> *mut TSK<'static> { + tpk.ref_raw().as_tsk().move_into_raw() } /// Returns a reference to the TPK's primary key. diff --git a/openpgp-ffi/src/tsk.rs b/openpgp-ffi/src/tsk.rs index ef17c285..29e9e1c8 100644 --- a/openpgp-ffi/src/tsk.rs +++ b/openpgp-ffi/src/tsk.rs @@ -1,21 +1,10 @@ //! Transferable secret keys. //! -//! Wraps [`sequoia-openpgp::TSK`]. +//! Wraps [`sequoia-openpgp::tpk::TSK`]. //! -//! [`sequoia-openpgp::TSK`]: ../../sequoia_openpgp/struct.TSK.html - -use failure; -use libc::c_char; +//! [`sequoia-openpgp::tpk::TSK`]: ../../sequoia_openpgp/struct.TSK.html extern crate sequoia_openpgp as openpgp; -use super::packet::signature::Signature; - -use super::tpk::TPK; -use ::error::Status; -use RefRaw; -use MoveFromRaw; -use MoveIntoRaw; -use MoveResultIntoRaw; /// A transferable secret key (TSK). /// @@ -24,44 +13,8 @@ use MoveResultIntoRaw; /// /// [RFC 4880, section 11.2]: https://tools.ietf.org/html/rfc4880#section-11.2 /// -/// Wraps [`sequoia-openpgp::TSK`]. +/// Wraps [`sequoia-openpgp::tpk::TSK`]. /// -/// [`sequoia-openpgp::TSK`]: ../../sequoia_openpgp/enum.TSK.html -#[::ffi_wrapper_type(prefix = "pgp_", name = "tsk", - derive = "Clone, Debug, PartialEq, Parse, Serialize")] -pub struct TSK(openpgp::TSK); - -/// Generates a new RSA 3072 bit key with UID `primary_uid`. -#[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "system" -fn pgp_tsk_new(errp: Option<&mut *mut ::error::Error>, - primary_uid: *const c_char, - tsk_out: *mut *mut TSK, - revocation_out: *mut *mut Signature) - -> Status -{ - let tsk_out = ffi_param_ref_mut!(tsk_out); - let revocation_out = ffi_param_ref_mut!(revocation_out); - let primary_uid = ffi_param_cstr!(primary_uid).to_string_lossy(); - match openpgp::TSK::new(primary_uid) { - Ok((tsk, revocation)) => { - *tsk_out = tsk.move_into_raw(); - *revocation_out = revocation.move_into_raw(); - Status::Success - }, - Err(e) => Err::<(), failure::Error>(e).move_into_raw(errp), - } -} - -/// Returns a reference to the corresponding TPK. -#[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "system" -fn pgp_tsk_tpk(tsk: *const TSK) - -> *const TPK { - tsk.ref_raw().tpk().move_into_raw() -} - -/// Converts the TSK into a TPK. -#[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "system" -fn pgp_tsk_into_tpk(tsk: *mut TSK) - -> *mut TPK { - tsk.move_from_raw().into_tpk().move_into_raw() -} +/// [`sequoia-openpgp::tpk::TSK`]: ../../sequoia_openpgp/enum.TSK.html +#[::ffi_wrapper_type(prefix = "pgp_", name = "tsk", derive = "Serialize")] +pub struct TSK<'a>(openpgp::tpk::TSK<'a>); diff --git a/openpgp/src/autocrypt.rs b/openpgp/src/autocrypt.rs index 38567743..3353bbf7 100644 --- a/openpgp/src/autocrypt.rs +++ b/openpgp/src/autocrypt.rs @@ -486,9 +486,7 @@ impl AutocryptSetupMessage { &[ (&"Autocrypt-Prefer-Encrypt"[..], self.prefer_encrypt().unwrap_or(&"nopreference"[..])) ])?; - // XXX - let tsk = self.tpk.clone().into_tsk(); - tsk.serialize(&mut w)?; + self.tpk.as_tsk().serialize(&mut w)?; Ok(w.finalize()?) } diff --git a/openpgp/src/lib.rs b/openpgp/src/lib.rs index 584ae406..f9bf609a 100644 --- a/openpgp/src/lib.rs +++ b/openpgp/src/lib.rs @@ -126,9 +126,6 @@ use constants::{ mod fingerprint; mod keyid; - -mod tsk; -pub use tsk::TSK; #[cfg(test)] use std::path::PathBuf; @@ -394,6 +391,16 @@ pub struct PacketPile { /// /// [RFC 4880, section 11.1]: https://tools.ietf.org/html/rfc4880#section-11.1 /// +/// # Secret keys +/// +/// Any key in a `TPK` may have a secret key attached to it. To +/// protect secret keys from being leaked, secret keys are not written +/// out if a `TPK` is serialized. To also serialize the secret keys, +/// you need to use [`TPK::as_tsk()`] to get an object that writes +/// them out during serialization. +/// +/// [`TPK::as_tsk()`]: #method.as_tsk +/// /// # Example /// /// ```rust diff --git a/openpgp/src/tpk/mod.rs b/openpgp/src/tpk/mod.rs index 35ed123d..305aa8f8 100644 --- a/openpgp/src/tpk/mod.rs +++ b/openpgp/src/tpk/mod.rs @@ -31,7 +31,6 @@ use { TPK, KeyID, Fingerprint, - TSK, }; use parse::{Parse, PacketParserResult, PacketParser}; use serialize::{Serialize, SerializeInto, SerializeKey}; @@ -42,6 +41,8 @@ mod lexer; mod grammar; mod builder; mod bindings; +mod tsk; +pub use self::tsk::TSK; use self::lexer::Lexer; pub use self::lexer::Token; @@ -2823,10 +2824,13 @@ impl TPK { TPK::from_packet_pile(PacketPile::from(combined)) } - /// Cast the public key into a secret key that allows using the secret - /// parts of the containing keys. - pub fn into_tsk(self) -> TSK { - TSK::from_tpk(self) + /// Derive a [`TSK`] object from this key. + /// + /// This object writes out secret keys during serialization. + /// + /// [`TSK`]: tpk/struct.TSK.html + pub fn as_tsk<'a>(&'a self) -> TSK<'a> { + TSK::new(self) } /// Returns whether at least one of the keys includes a secret diff --git a/openpgp/src/tpk/tsk.rs b/openpgp/src/tpk/tsk.rs new file mode 100644 index 00000000..978affcb --- /dev/null +++ b/openpgp/src/tpk/tsk.rs @@ -0,0 +1,280 @@ +use std::io; + +use Result; +use TPK; +use packet::{Key, Tag}; +use serialize::{Serialize, SerializeKey}; + +/// A reference to a TPK that allows serialization of secret keys. +/// +/// To avoid accidental leakage `TPK::serialize()` skips secret keys. +/// To serialize `TPK`s with secret keys, use [`TPK::as_tsk()`] to +/// create a `TSK`, which is a shim on top of the `TPK`, and serialize +/// this. +/// +/// [`TPK::as_tsk()`]: ../struct.TPK.html#method.as_tsk +/// +/// # Example +/// ``` +/// # use sequoia_openpgp::{*, tpk::*, parse::Parse, serialize::Serialize}; +/// # f().unwrap(); +/// # fn f() -> Result<()> { +/// let (tpk, _) = TPKBuilder::default().generate()?; +/// assert!(tpk.is_tsk()); +/// +/// let mut buf = Vec::new(); +/// tpk.as_tsk().serialize(&mut buf)?; +/// +/// let tpk_ = TPK::from_bytes(&buf)?; +/// assert!(tpk_.is_tsk()); +/// assert_eq!(tpk, tpk_); +/// # Ok(()) } +pub struct TSK<'a> { + tpk: &'a TPK, + filter: Option bool>>, +} + +impl<'a> TSK<'a> { + /// Creates a new view for the given `TPK`. + pub(crate) fn new(tpk: &'a TPK) -> Self { + Self { + tpk: tpk, + filter: None, + } + } + + /// Filters which secret keys to export using the given predicate. + /// + /// Note that the given filter replaces any existing filter. + /// + /// # Example + /// ``` + /// # use sequoia_openpgp::{*, tpk::*, parse::Parse, serialize::Serialize}; + /// # f().unwrap(); + /// # fn f() -> Result<()> { + /// let (tpk, _) = TPKBuilder::default().add_signing_subkey().generate()?; + /// assert_eq!(tpk.keys_valid().secret(true).count(), 2); + /// + /// // Only write out the primary key's secret. + /// let mut buf = Vec::new(); + /// tpk.as_tsk().set_filter(|k| k == tpk.primary()).serialize(&mut buf)?; + /// + /// let tpk_ = TPK::from_bytes(&buf)?; + /// assert_eq!(tpk_.keys_valid().secret(true).count(), 1); + /// assert!(tpk_.primary().secret().is_some()); + /// # Ok(()) } + pub fn set_filter

(mut self, predicate: P) -> Self + where P: 'a + Fn(&'a Key) -> bool + { + self.filter = Some(Box::new(predicate)); + self + } +} + +impl<'a> Serialize for TSK<'a> { + fn serialize(&self, o: &mut W) -> Result<()> { + // Serializes public or secret key depending on the filter. + let serialize_key = |o: &mut W, key: &'a Key, tag_public, tag_secret| { + key.serialize(o, + if self.filter.as_ref().map( + |f| f(key)).unwrap_or(true) + { + tag_secret + } else { + tag_public + }) + }; + serialize_key(o, &self.tpk.primary, Tag::PublicKey, Tag::SecretKey)?; + + for s in self.tpk.primary_selfsigs.iter() { + s.serialize(o)?; + } + for s in self.tpk.primary_self_revocations.iter() { + s.serialize(o)?; + } + for s in self.tpk.primary_certifications.iter() { + s.serialize(o)?; + } + for s in self.tpk.primary_other_revocations.iter() { + s.serialize(o)?; + } + + for u in self.tpk.userids() { + u.userid().serialize(o)?; + for s in u.self_revocations() { + s.serialize(o)?; + } + for s in u.selfsigs() { + s.serialize(o)?; + } + for s in u.other_revocations() { + s.serialize(o)?; + } + for s in u.certifications() { + s.serialize(o)?; + } + } + + for u in self.tpk.user_attributes() { + u.user_attribute().serialize(o)?; + for s in u.self_revocations() { + s.serialize(o)?; + } + for s in u.selfsigs() { + s.serialize(o)?; + } + for s in u.other_revocations() { + s.serialize(o)?; + } + for s in u.certifications() { + s.serialize(o)?; + } + } + + for k in self.tpk.subkeys() { + serialize_key(o, k.subkey(), Tag::PublicSubkey, Tag::SecretSubkey)?; + for s in k.self_revocations() { + s.serialize(o)?; + } + for s in k.selfsigs() { + s.serialize(o)?; + } + for s in k.other_revocations() { + s.serialize(o)?; + } + for s in k.certifications() { + s.serialize(o)?; + } + } + + for u in self.tpk.unknowns.iter() { + u.unknown.serialize(o)?; + + for s in u.sigs.iter() { + s.serialize(o)?; + } + } + + for s in self.tpk.bad.iter() { + s.serialize(o)?; + } + + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + use parse::Parse; + use serialize::Serialize; + + fn test_tpk(name: &str) -> Result { + let path = format!("tests/data/keys/{}.pgp", name); + TPK::from_file(path) + } + + fn test_tsk(name: &str) -> Result { + let path = format!("tests/data/keys/{}-private.pgp", name); + TPK::from_file(path) + } + + const PUBLIC_TESTS: &[&str] = &[ + "dennis-simon-anton", + "dsa2048-elgamal3072", + "emmelie-dorothea-dina-samantha-awina-ed25519", + "erika-corinna-daniela-simone-antonia-nistp256", + "erika-corinna-daniela-simone-antonia-nistp384", + "erika-corinna-daniela-simone-antonia-nistp521", + "testy-new", + "testy", + "neal", + "dkg-sigs-out-of-order", + ]; + const SECRET_TESTS: &[&str] = &[ + "dennis-simon-anton", + "dsa2048-elgamal3072", + "emmelie-dorothea-dina-samantha-awina-ed25519", + "erika-corinna-daniela-simone-antonia-nistp256", + "erika-corinna-daniela-simone-antonia-nistp384", + "erika-corinna-daniela-simone-antonia-nistp521", + "testy-new", + "testy-nistp256", + "testy-nistp384", + "testy-nistp521", + "testy", + ]; + + /// Demonstrates that public keys and all components are + /// serialized. + #[test] + fn roundtrip_tpk() { + for test in PUBLIC_TESTS { + let tpk = match test_tpk(dbg!(test)) { + Ok(t) => t, + Err(_) => continue, + }; + assert!(! tpk.is_tsk()); + + let mut buf = Vec::new(); + tpk.as_tsk().serialize(&mut buf).unwrap(); + let tpk_ = TPK::from_bytes(&buf).unwrap(); + + assert_eq!(tpk, tpk_, "roundtripping {}.pgp failed", test); + } + } + + /// Demonstrates that secret keys and all components are + /// serialized. + #[test] + fn roundtrip_tsk() { + for test in SECRET_TESTS { + let tpk = test_tsk(test).unwrap(); + assert!(tpk.is_tsk()); + + let mut buf = Vec::new(); + tpk.as_tsk().serialize(&mut buf).unwrap(); + let tpk_ = TPK::from_bytes(&buf).unwrap(); + + assert_eq!(tpk, tpk_, "roundtripping {}-private.pgp failed", test); + + // This time, use a trivial filter. + let mut buf = Vec::new(); + tpk.as_tsk().set_filter(|_| true).serialize(&mut buf).unwrap(); + let tpk_ = TPK::from_bytes(&buf).unwrap(); + + assert_eq!(tpk, tpk_, "roundtripping {}-private.pgp failed", test); + } + } + + /// Demonstrates that TSK::serialize() with the right filter + /// reduces to TPK::serialize(). + #[test] + fn reduce_to_tpk_serialize() { + for test in SECRET_TESTS { + let tpk = test_tsk(test).unwrap(); + assert!(tpk.is_tsk()); + + // First, use TPK::serialize(). + let mut buf_tpk = Vec::new(); + tpk.serialize(&mut buf_tpk).unwrap(); + + // When serializing using TSK::serialize, filter out all + // secret keys. + let mut buf_tsk = Vec::new(); + tpk.as_tsk().set_filter(|_| false).serialize(&mut buf_tsk).unwrap(); + + // Check for equality. + let tpk_ = TPK::from_bytes(&buf_tpk).unwrap(); + let tsk_ = TPK::from_bytes(&buf_tsk).unwrap(); + assert_eq!(tpk_, tsk_, + "reducing failed on {}-private.pgp: not TPK::eq", + test); + + // Check for identinty. + assert_eq!(buf_tpk, buf_tsk, + "reducing failed on {}-private.pgp: serialized identity", + test); + } + } +} diff --git a/openpgp/src/tsk.rs b/openpgp/src/tsk.rs index 676e3c30..49c4da27 100644 --- a/openpgp/src/tsk.rs +++ b/openpgp/src/tsk.rs @@ -1,3 +1,6 @@ +//! This file is no longer used, but serves as reference for +//! high-level operations that can be done on TPKs. + use std::borrow::Cow; use std::io; use std::ops::{Deref, DerefMut}; diff --git a/openpgp/tests/for-each-artifact.rs b/openpgp/tests/for-each-artifact.rs index d1744dd5..8979ccaa 100644 --- a/openpgp/tests/for-each-artifact.rs +++ b/openpgp/tests/for-each-artifact.rs @@ -34,10 +34,7 @@ mod for_each_artifact { }; let mut v = Vec::new(); - // Temporarily convert to TSK to serialize secrets. - let p = p.into_tsk(); - p.serialize(&mut v)?; - let p = p.into_tpk(); + p.as_tsk().serialize(&mut v)?; let q = openpgp::TPK::from_bytes(&v)?; assert_eq!(p, q, "roundtripping {:?} failed", src); Ok(()) diff --git a/tool/src/commands/key.rs b/tool/src/commands/key.rs index 91b2d941..fbbfcb5e 100644 --- a/tool/src/commands/key.rs +++ b/tool/src/commands/key.rs @@ -84,7 +84,6 @@ pub fn generate(m: &ArgMatches, force: bool) -> failure::Fallible<()> { // Generate the key let (tpk, rev) = builder.generate()?; - let tsk = tpk.into_tsk(); // Export if m.is_present("export") { @@ -114,7 +113,7 @@ pub fn generate(m: &ArgMatches, force: bool) -> failure::Fallible<()> { { let w = create_or_stdout(Some(&key_path), force)?; let mut w = Writer::new(w, Kind::SecretKey, &[])?; - tsk.serialize(&mut w)?; + tpk.as_tsk().serialize(&mut w)?; } // write out rev cert -- cgit v1.2.3