openpgp: Only impl Serialize for objects that are normally exported.

  - Add two new traits: `Marshal` and `MarshalInto`.

  - Implement them instead of `Serialize` and `SerializeInto`.

  - Only implement `Serialize` and `SerializeInto` for data structures
    that are normally exported.

  - This should prevent users from accidentally serializing a bare
    signature (`Signature`) when they meant to serialize a signature
    packet (`Packet`), for instance.

  - Fixes #368.

27 files changed, 464 insertions, 208 deletions

diff --git a/openpgp/examples/notarize.rs b/openpgp/examples/notarize.rs
index 9c8605ea..b6c71dab 100644
--- a/openpgp/examples/notarize.rs
+++ b/openpgp/examples/notarize.rs
@@ -9,7 +9,7 @@ use crate::openpgp::{
     armor,
     Packet,
     parse::{Parse, PacketParserResult},
-    serialize::Serialize,
+    serialize::Marshal,
 };
 use crate::openpgp::serialize::stream::{Message, LiteralWriter, Signer};
 use crate::openpgp::policy::StandardPolicy as P;
diff --git a/openpgp/examples/statistics.rs b/openpgp/examples/statistics.rs
index c990fb58..1dfebe38 100644
--- a/openpgp/examples/statistics.rs
+++ b/openpgp/examples/statistics.rs
@@ -15,7 +15,7 @@ use crate::openpgp::types::*;
 use crate::openpgp::packet::{user_attribute, header::BodyLength, Tag};
 use crate::openpgp::packet::signature::subpacket::SubpacketTag;
 use crate::openpgp::parse::{Parse, PacketParserResult, PacketParser};
-use crate::openpgp::serialize::SerializeInto;
+use crate::openpgp::serialize::MarshalInto;
 
 fn main() {
     let args: Vec<String> = env::args().collect();
diff --git a/openpgp/src/armor.rs b/openpgp/src/armor.rs
index cb106e03..6f11958b 100644
--- a/openpgp/src/armor.rs
+++ b/openpgp/src/armor.rs
@@ -38,7 +38,7 @@ use quickcheck::{Arbitrary, Gen};
 use crate::vec_truncate;
 use crate::packet::prelude::*;
 use crate::packet::header::{BodyLength, CTBNew, CTBOld};
-use crate::serialize::SerializeInto;
+use crate::serialize::MarshalInto;
 
 /// The encoded output stream must be represented in lines of no more
 /// than 76 characters each (see (see [RFC 4880, section
diff --git a/openpgp/src/crypto/hash.rs b/openpgp/src/crypto/hash.rs
index d0ff1c3d..47220da8 100644
--- a/openpgp/src/crypto/hash.rs
+++ b/openpgp/src/crypto/hash.rs
@@ -245,7 +245,7 @@ impl<P, R> Hash for Key4<P, R>
 {
     /// Update the Hash with a hash of the key.
     fn hash(&self, hash: &mut Context) {
-        use crate::serialize::SerializeInto;
+        use crate::serialize::MarshalInto;
 
         // We hash 8 bytes plus the MPIs.  But, the len doesn't
         // include the tag (1 byte) or the length (2 bytes).
@@ -303,7 +303,8 @@ impl Hash for Signature4 {
 impl Hash for signature::Builder {
     /// Adds the `Signature` to the provided hash context.
     fn hash(&self, hash: &mut Context) {
-        use crate::serialize::SerializeInto;
+        use crate::serialize::MarshalInto;
+
         // XXX: Annoyingly, we have no proper way of handling errors
         // here.
         let hashed_area = self.hashed_area().to_vec()
diff --git a/openpgp/src/crypto/mpis.rs b/openpgp/src/crypto/mpis.rs
index ace023d4..51374016 100644
--- a/openpgp/src/crypto/mpis.rs
+++ b/openpgp/src/crypto/mpis.rs
@@ -14,7 +14,7 @@ use crate::types::{
 };
 use crate::crypto::hash::{self, Hash};
 use crate::crypto::mem::{secure_cmp, Protected};
-use crate::serialize::Serialize;
+use crate::serialize::Marshal;
 
 use crate::Error;
 use crate::Result;
@@ -896,7 +896,6 @@ impl Arbitrary for Signature {
 mod tests {
     use super::*;
     use crate::parse::Parse;
-    use crate::serialize::Serialize;
 
     quickcheck! {
         fn mpi_roundtrip(mpi: MPI) -> bool {
@@ -910,7 +909,6 @@ mod tests {
         fn pk_roundtrip(pk: PublicKey) -> bool {
             use std::io::Cursor;
             use crate::PublicKeyAlgorithm::*;
-            use crate::serialize::Serialize;
 
             let buf = Vec::<u8>::default();
             let mut cur = Cursor::new(buf);
@@ -969,7 +967,6 @@ mod tests {
         fn sk_roundtrip(sk: SecretKeyMaterial) -> bool {
             use std::io::Cursor;
             use crate::PublicKeyAlgorithm::*;
-            use crate::serialize::Serialize;
 
             let buf = Vec::<u8>::default();
             let mut cur = Cursor::new(buf);
@@ -1009,7 +1006,6 @@ mod tests {
         fn ct_roundtrip(ct: Ciphertext) -> bool {
             use std::io::Cursor;
             use crate::PublicKeyAlgorithm::*;
-            use crate::serialize::Serialize;
 
             let buf = Vec::<u8>::default();
             let mut cur = Cursor::new(buf);
@@ -1040,7 +1036,6 @@ mod tests {
         fn signature_roundtrip(sig: Signature) -> bool {
             use std::io::Cursor;
             use crate::PublicKeyAlgorithm::*;
-            use crate::serialize::Serialize;
 
             let buf = Vec::<u8>::default();
             let mut cur = Cursor::new(buf);
diff --git a/openpgp/src/crypto/s2k.rs b/openpgp/src/crypto/s2k.rs
index fc29a7e1..d738a37d 100644
--- a/openpgp/src/crypto/s2k.rs
+++ b/openpgp/src/crypto/s2k.rs
@@ -290,7 +290,6 @@ mod tests {
     use crate::SymmetricAlgorithm;
     use crate::Packet;
     use crate::parse::{Parse, PacketParser};
-    use crate::serialize::Serialize;
 
     #[test]
     fn s2k_parser_test() {
@@ -423,7 +422,8 @@ mod tests {
 
     quickcheck! {
         fn s2k_roundtrip(s2k: S2K) -> bool {
-            use crate::serialize::SerializeInto;
+            use crate::serialize::Marshal;
+            use crate::serialize::MarshalInto;
 
             eprintln!("in {:?}", s2k);
             use std::io::Cursor;
diff --git a/openpgp/src/packet/key/mod.rs b/openpgp/src/packet/key/mod.rs
index 1cf16e08..cd184b64 100644
--- a/openpgp/src/packet/key/mod.rs
+++ b/openpgp/src/packet/key/mod.rs
@@ -1530,7 +1530,7 @@ pub struct Unencrypted {
 
 impl From<mpis::SecretKeyMaterial> for Unencrypted {
     fn from(mpis: mpis::SecretKeyMaterial) -> Self {
-        use crate::serialize::Serialize;
+        use crate::serialize::Marshal;
         let mut plaintext = Vec::new();
         // We need to store the type.
         plaintext.push(
diff --git a/openpgp/src/packet/literal.rs b/openpgp/src/packet/literal.rs
index 708a37a4..74123b76 100644
--- a/openpgp/src/packet/literal.rs
+++ b/openpgp/src/packet/literal.rs
@@ -200,7 +200,7 @@ impl Arbitrary for Literal {
 mod tests {
     use super::*;
     use crate::parse::Parse;
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
 
     quickcheck! {
         fn roundtrip(p: Literal) -> bool {
diff --git a/openpgp/src/packet/marker.rs b/openpgp/src/packet/marker.rs
index 512cda24..9e1e6718 100644
--- a/openpgp/src/packet/marker.rs
+++ b/openpgp/src/packet/marker.rs
@@ -55,7 +55,7 @@ impl Arbitrary for Marker {
 mod tests {
     use super::*;
     use crate::parse::Parse;
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
 
     #[test]
     fn roundtrip() {
diff --git a/openpgp/src/packet/one_pass_sig.rs b/openpgp/src/packet/one_pass_sig.rs
index debb2460..0fc72286 100644
--- a/openpgp/src/packet/one_pass_sig.rs
+++ b/openpgp/src/packet/one_pass_sig.rs
@@ -197,7 +197,7 @@ impl Arbitrary for OnePassSig3 {
 mod tests {
     use super::*;
     use crate::parse::Parse;
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
 
     quickcheck! {
         fn roundtrip(p: OnePassSig3) -> bool {
diff --git a/openpgp/src/packet/pkesk.rs b/openpgp/src/packet/pkesk.rs
index 4ab19c8d..ed09bddf 100644
--- a/openpgp/src/packet/pkesk.rs
+++ b/openpgp/src/packet/pkesk.rs
@@ -207,7 +207,7 @@ mod tests {
     use crate::PacketPile;
     use crate::Packet;
     use crate::parse::Parse;
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
 
     quickcheck! {
         fn roundtrip(p: PKESK3) -> bool {
diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs
index b45c74bc..5dfcfa7e 100644
--- a/openpgp/src/packet/signature/subpacket.rs
+++ b/openpgp/src/packet/signature/subpacket.rs
@@ -77,7 +77,7 @@ use crate::{
     Fingerprint,
     KeyID,
     SignatureType,
-    serialize::SerializeInto,
+    serialize::MarshalInto,
 };
 use crate::types::{
     AEADAlgorithm,
diff --git a/openpgp/src/packet/skesk.rs b/openpgp/src/packet/skesk.rs
index dfe9844e..33055c61 100644
--- a/openpgp/src/packet/skesk.rs
+++ b/openpgp/src/packet/skesk.rs
@@ -425,7 +425,7 @@ mod test {
     use super::*;
     use crate::PacketPile;
     use crate::parse::Parse;
-    use crate::serialize::{Serialize, SerializeInto};
+    use crate::serialize::{Marshal, MarshalInto};
 
     quickcheck! {
         fn roundtrip(p: SKESK) -> bool {
diff --git a/openpgp/src/packet/trust.rs b/openpgp/src/packet/trust.rs
index e4431701..35b45597 100644
--- a/openpgp/src/packet/trust.rs
+++ b/openpgp/src/packet/trust.rs
@@ -76,7 +76,7 @@ impl Arbitrary for Trust {
 mod tests {
     use super::*;
     use crate::parse::Parse;
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
 
     quickcheck! {
         fn roundtrip(p: Trust) -> bool {
diff --git a/openpgp/src/packet/user_attribute.rs b/openpgp/src/packet/user_attribute.rs
index cfbe608f..11ea2b0c 100644
--- a/openpgp/src/packet/user_attribute.rs
+++ b/openpgp/src/packet/user_attribute.rs
@@ -20,8 +20,8 @@ use crate::packet::{
     header::BodyLength,
 };
 use crate::Packet;
-use crate::serialize::Serialize;
-use crate::serialize::SerializeInto;
+use crate::serialize::Marshal;
+use crate::serialize::MarshalInto;
 
 /// Holds a UserAttribute packet.
 ///
@@ -270,7 +270,6 @@ impl Arbitrary for Image {
 mod tests {
     use super::*;
     use crate::parse::Parse;
-    use crate::serialize::SerializeInto;
 
     quickcheck! {
         fn roundtrip(p: UserAttribute) -> bool {
diff --git a/openpgp/src/packet/userid/mod.rs b/openpgp/src/packet/userid/mod.rs
index 4751b981..abc04b9e 100644
--- a/openpgp/src/packet/userid/mod.rs
+++ b/openpgp/src/packet/userid/mod.rs
@@ -832,7 +832,7 @@ impl Arbitrary for UserID {
 mod tests {
     use super::*;
     use crate::parse::Parse;
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
 
     quickcheck! {
         fn roundtrip(p: UserID) -> bool {
diff --git a/openpgp/src/parse/mpis.rs b/openpgp/src/parse/mpis.rs
index 0dc6de4b..efc4c893 100644
--- a/openpgp/src/parse/mpis.rs
+++ b/openpgp/src/parse/mpis.rs
@@ -155,7 +155,7 @@ impl mpis::SecretKeyMaterial {
     pub fn parse_chksumd<T: Read>(algo: PublicKeyAlgorithm, cur: T)
                                   -> Result<Self> {
         use std::io::Cursor;
-        use crate::serialize::Serialize;
+        use crate::serialize::Marshal;
 
         // read mpis
         let bio = buffered_reader::Generic::with_cookie(
@@ -483,7 +483,7 @@ impl mpis::Signature {
 fn mpis_parse_test() {
     use super::Parse;
     use crate::PublicKeyAlgorithm::*;
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
 
     // Dummy RSA public key.
     {
diff --git a/openpgp/src/parse/parse.rs b/openpgp/src/parse/parse.rs
index c7e8338b..7d899fcc 100644
--- a/openpgp/src/parse/parse.rs
+++ b/openpgp/src/parse/parse.rs
@@ -1172,7 +1172,7 @@ impl_parse_generic_packet!(Signature);
 
 #[test]
 fn signature_parser_test () {
-    use crate::serialize::SerializeInto;
+    use crate::serialize::MarshalInto;
     let data = crate::tests::message("sig.gpg");
 
     {
@@ -1790,7 +1790,7 @@ impl Key4<key::UnspecifiedParts, key::UnspecifiedRole>
     /// secret subkey packet.
     fn parse<'a, T: 'a + BufferedReader<Cookie>>(mut php: PacketHeaderParser<T>) -> Result<PacketParser<'a>> {
         use std::io::Cursor;
-        use crate::serialize::Serialize;
+        use crate::serialize::Marshal;
 
         make_php_try!(php);
         let tag = php.header.ctb().tag();
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index 4b057a22..af64d709 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -44,7 +44,7 @@ use crate::{
     packet::Signature,
     cert::prelude::*,
     crypto::SessionKey,
-    serialize::Serialize,
+    serialize::Marshal,
     policy::Policy,
 };
 use crate::parse::{
diff --git a/openpgp/src/serialize/cert.rs b/openpgp/src/serialize/cert.rs
index 689fcfcf..1c47da44 100644
--- a/openpgp/src/serialize/cert.rs
+++ b/openpgp/src/serialize/cert.rs
@@ -2,19 +2,11 @@ use crate::Result;
 use crate::cert::prelude::*;
 use crate::packet::{key, Signature, Tag};
 use crate::serialize::{
-    PacketRef, Serialize, SerializeInto,
+    PacketRef,
+    Marshal, MarshalInto,
     generic_serialize_into, generic_export_into,
 };
 
-impl Serialize for Cert {
-    fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
-        self.serialize_common(o, false)
-    }
-
-    fn export(&self, o: &mut dyn std::io::Write) -> Result<()> {
-        self.serialize_common(o, true)
-    }
-}
 
 impl Cert {
     /// Serializes or exports the Cert.
@@ -170,7 +162,21 @@ impl Cert {
     }
 }
 
-impl SerializeInto for Cert {
+impl crate::serialize::Serialize for Cert {}
+
+impl Marshal for Cert {
+    fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
+        self.serialize_common(o, false)
+    }
+
+    fn export(&self, o: &mut dyn std::io::Write) -> Result<()> {
+        self.serialize_common(o, true)
+    }
+}
+
+impl crate::serialize::SerializeInto for Cert {}
+
+impl MarshalInto for Cert {
     fn serialized_len(&self) -> usize {
         let mut l = 0;
         let primary = self.primary_key();
@@ -521,7 +527,9 @@ impl<'a> TSK<'a> {
     }
 }
 
-impl<'a> Serialize for TSK<'a> {
+impl<'a> crate::serialize::Serialize for TSK<'a> {}
+
+impl<'a> Marshal for TSK<'a> {
     fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
         self.serialize_common(o, false)
     }
@@ -531,7 +539,9 @@ impl<'a> Serialize for TSK<'a> {
     }
 }
 
-impl<'a> SerializeInto for TSK<'a> {
+impl<'a> crate::serialize::SerializeInto for TSK<'a> {}
+
+impl<'a> MarshalInto for TSK<'a> {
     fn serialized_len(&self) -> usize {
         let mut l = 0;
 
@@ -664,7 +674,6 @@ mod test {
     use super::*;
     use crate::vec_truncate;
     use crate::parse::Parse;
-    use crate::serialize::Serialize;
     use crate::packet::key;
     use crate::policy::StandardPolicy as P;
 
diff --git a/openpgp/src/serialize/cert_armored.rs b/openpgp/src/serialize/cert_armored.rs
index 5baaa8b6..32146cec 100644
--- a/openpgp/src/serialize/cert_armored.rs
+++ b/openpgp/src/serialize/cert_armored.rs
@@ -7,7 +7,8 @@ use crate::cert::{Cert, amalgamation::ValidAmalgamation};
 use crate::Result;
 use crate::types::RevocationStatus;
 use crate::serialize::{
-    Serialize, SerializeInto, generic_serialize_into, generic_export_into,
+    Marshal, MarshalInto,
+    generic_serialize_into, generic_export_into,
 };
 use crate::policy::StandardPolicy as P;
 
@@ -83,7 +84,9 @@ impl Cert {
     /// assert!(armored.contains("Mr. Pink ☮☮☮"));
     /// # Ok(()) }
     /// ```
-    pub fn armored<'a>(&'a self) -> impl Serialize + SerializeInto + 'a {
+    pub fn armored<'a>(&'a self)
+        -> impl crate::serialize::Serialize + crate::serialize::SerializeInto + 'a
+    {
         Encoder::new(self)
     }
 }
@@ -124,7 +127,9 @@ impl<'a> Encoder<'a> {
     }
 }
 
-impl<'a> Serialize for Encoder<'a> {
+impl<'a> crate::serialize::Serialize for Encoder<'a> {}
+
+impl<'a> Marshal for Encoder<'a> {
     fn serialize(&self, o: &mut dyn io::Write) -> Result<()> {
         self.serialize_common(o, false)
     }
@@ -134,7 +139,9 @@ impl<'a> Serialize for Encoder<'a> {
     }
 }
 
-impl<'a> SerializeInto for Encoder<'a> {
+impl<'a> crate::serialize::SerializeInto for Encoder<'a> {}
+
+impl<'a> MarshalInto for Encoder<'a> {
     fn serialized_len(&self) -> usize {
         let h = self.cert.armor_headers();
         let headers_len =
diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs
index da10d7d9..97fe667f 100644
--- a/openpgp/src/serialize/mod.rs
+++ b/openpgp/src/serialize/mod.rs
@@ -49,7 +49,107 @@ use crate::types::{
 const TRACE : bool = false;
 
 /// Serializes OpenPGP data structures.
-pub trait Serialize {
+///
+/// This trait provides the same interface as the `Marshal` trait (in
+/// fact, it is just a wrapper around that trait), but only data
+/// structures that it makes sense to export implement it.
+///
+/// Having a separate trait for data structures that it makes sense to
+/// export avoids an easy-to-make and hard-to-debug bug: inadvertently
+/// exporting an OpenPGP data structure without any framing
+/// information.
+///
+/// This bug is easy to make, because Rust infers types, which means
+/// that it is often not clear from the immediate context exactly what
+/// is being serialized.  This bug is hard to debug, because errors
+/// parsing data that has been incorrectly exported, are removed from
+/// the serialization code.
+///
+/// The following example shows how to correctly export a revocation
+/// certificate.  It should make clear how easy it is to forget to
+/// convert a bare signature into an OpenPGP packet before serializing
+/// it:
+///
+/// ```
+/// # extern crate sequoia_openpgp as openpgp;
+/// # use openpgp::Result;
+/// use openpgp::cert::prelude::*;
+/// use openpgp::Packet;
+/// use openpgp::serialize::Serialize;
+///
+/// # fn main() { f().unwrap(); }
+/// # fn f() -> Result<()> {
+/// let (_cert, rev) =
+///     CertBuilder::general_purpose(None, Some("alice@example.org"))
+///     .generate()?;
+/// let rev : Packet = rev.into();
+/// # let output = &mut Vec::new();
+/// rev.serialize(output)?;
+/// # Ok(())
+/// # }
+/// ```
+///
+/// Note: if you `use` both `Serialize` and `Marshal`, then, because
+/// they both have the same methods, and all data structures that
+/// implement `Serialize` also implement `Marshal`, you will have to
+/// use the Universal Function Call Syntax (UFCS) to call the methods
+/// on those objects, for example:
+///
+/// ```
+/// # extern crate sequoia_openpgp as openpgp;
+/// # use openpgp::Result;
+/// # use openpgp::cert::prelude::*;
+/// # use openpgp::Packet;
+/// # use openpgp::serialize::Serialize;
+/// #
+/// # fn main() { f().unwrap(); }
+/// # fn f() -> Result<()> {
+/// # let (_cert, rev) =
+/// #     CertBuilder::general_purpose(None, Some("alice@example.org"))
+/// #     .generate()?;
+/// # let rev : Packet = rev.into();
+/// # let output = &mut Vec::new();
+/// Serialize::serialize(&rev, output)?;
+/// # Ok(())
+/// # }
+/// ```
+///
+/// If you really needed `Marshal`, we strongly recommend importing it
+/// in as small a scope as possible to avoid this, and to avoid
+/// accidentally exporting data without the required framing.
+pub trait Serialize : Marshal {
+    /// Writes a serialized version of the object to `o`.
+    fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
+        Marshal::serialize(self, o)
+    }
+
+    /// Exports a serialized version of the object to `o`.
+    ///
+    /// This is similar to [`serialize(..)`], with these exceptions:
+