Move ascii-armored stuff from Key to DcKey

This means all key conversions/serialisation/deserialisation can be
done with DcKey rather than Key.  Also migrate all key conversion
tests to DcKey rather than Key.

src/imex.rs

@@ -1,5 +1,6 @@
 //! # Import/export module
 
+use std::any::Any;
 use std::cmp::{max, min};
 
 use async_std::path::{Path, PathBuf};
@@ -16,7 +17,7 @@ use crate::dc_tools::*;
 use crate::e2ee;
 use crate::error::*;
 use crate::events::Event;
-use crate::key::{self, DcKey, Key, SignedSecretKey};
+use crate::key::{self, DcKey, Key, SignedPublicKey, SignedSecretKey};
 use crate::message::{Message, MsgId};
 use crate::mimeparser::SystemMessage;
 use crate::param::*;
@@ -181,7 +182,7 @@ pub async fn render_setup_file(context: &Context, passphrase: &str) -> Result<St
         passphrase.len() >= 2,
         "Passphrase must be at least 2 chars long."
     );
-    let private_key = Key::from(SignedSecretKey::load_self(context).await?);
+    let private_key = SignedSecretKey::load_self(context).await?;
     let ac_headers = match context.get_config_bool(Config::E2eeEnabled).await {
         false => None,
         true => Some(("Autocrypt-Prefer-Encrypt", "mutual")),
@@ -291,7 +292,9 @@ async fn set_self_key(
     prefer_encrypt_required: bool,
 ) -> Result<()> {
     // try hard to only modify key-state
-    let keys = Key::from_armored_string(armored, KeyType::Private)
+    let keys = SignedSecretKey::from_asc(armored)
+        .map(|(key, hdrs)| (Key::from(key), hdrs))
+        .ok()
         .and_then(|(k, h)| if k.verify() { Some((k, h)) } else { None })
         .and_then(|(k, h)| k.split_key().map(|pub_key| (k, pub_key, h)));
 
@@ -696,9 +699,9 @@ async fn export_self_keys(context: &Context, dir: impl AsRef<Path>) -> Result<()
             |row| {
                 let id = row.get(0)?;
                 let public_key_blob: Vec<u8> = row.get(1)?;
-                let public_key = Key::from_slice(&public_key_blob, KeyType::Public);
+                let public_key = SignedPublicKey::from_slice(&public_key_blob);
                 let private_key_blob: Vec<u8> = row.get(2)?;
-                let private_key = Key::from_slice(&private_key_blob, KeyType::Private);
+                let private_key = SignedSecretKey::from_slice(&private_key_blob);
                 let is_default: i32 = row.get(3)?;
 
                 Ok((id, public_key, private_key, is_default))
@@ -741,22 +744,32 @@ async fn export_self_keys(context: &Context, dir: impl AsRef<Path>) -> Result<()
 /*******************************************************************************
  * Classic key export
  ******************************************************************************/
-async fn export_key_to_asc_file(
+async fn export_key_to_asc_file<T>(
     context: &Context,
     dir: impl AsRef<Path>,
     id: Option<i64>,
-    key: &Key,
+    key: &T,
-) -> std::io::Result<()> {
+) -> std::io::Result<()>
+where
+    T: DcKey + Any,
+{
     let file_name = {
-        let kind = if key.is_public() { "public" } else { "private" };
+        let any_key = key as &dyn Any;
+        let kind = if any_key.downcast_ref::<SignedPublicKey>().is_some() {
+            "public"
+        } else if any_key.downcast_ref::<SignedPublicKey>().is_some() {
+            "private"
+        } else {
+            "unknown"
+        };
         let id = id.map_or("default".into(), |i| i.to_string());
-
         dir.as_ref().join(format!("{}-key-{}.asc", kind, &id))
     };
     info!(context, "Exporting key {}", file_name.display());
     dc_delete_file(context, &file_name).await;
 
-    let res = key.write_asc_to_file(&file_name, context).await;
+    let content = key.to_asc(None).into_bytes();
+    let res = dc_write_file(context, &file_name, &content).await;
     if res.is_err() {
         error!(context, "Cannot write key to {}", file_name.display());
     } else {
@@ -822,7 +835,7 @@ mod tests {
     #[async_std::test]
     async fn test_export_key_to_asc_file() {
         let context = dummy_context().await;
-        let key = Key::from(alice_keypair().public);
+        let key = alice_keypair().public;
         let blobdir = "$BLOBDIR";
         assert!(export_key_to_asc_file(&context.ctx, blobdir, None, &key)
             .await

src/key.rs

@@ -4,7 +4,6 @@ use std::collections::BTreeMap;
 use std::fmt;
 use std::io::Cursor;
 
-use async_std::path::Path;
 use async_trait::async_trait;
 use num_traits::FromPrimitive;
 use pgp::composed::Deserializable;
@@ -14,7 +13,7 @@ use pgp::types::{KeyTrait, SecretKeyTrait};
 use crate::config::Config;
 use crate::constants::*;
 use crate::context::Context;
-use crate::dc_tools::{dc_write_file, time, EmailAddress, InvalidEmailError};
+use crate::dc_tools::{time, EmailAddress, InvalidEmailError};
 use crate::sql;
 
 // Re-export key types
@@ -69,6 +68,15 @@ pub trait DcKey: Serialize + Deserializable + KeyTrait + Clone {
         Self::from_slice(&bytes)
     }
 
+    /// Create a key from an ASCII-armored string.
+    ///
+    /// Returns the key and a map of any headers which might have been set in
+    /// the ASCII-armored representation.
+    fn from_asc(data: &str) -> Result<(Self::KeyType, BTreeMap<String, String>)> {
+        let bytes = data.as_bytes();
+        Self::KeyType::from_armor_single(Cursor::new(bytes)).map_err(Error::Pgp)
+    }
+
     /// Load the users' default key from the database.
     async fn load_self(context: &Context) -> Result<Self::KeyType>;
 
@@ -88,6 +96,14 @@ pub trait DcKey: Serialize + Deserializable + KeyTrait + Clone {
         base64::encode(&DcKey::to_bytes(self))
     }
 
+    /// Serialise the key to ASCII-armored representation.
+    ///
+    /// Each header line must be terminated by `\r\n`.  Only allows setting one
+    /// header as a simplification since that's the only way it's used so far.
+    // Since .to_armored_string() are actual methods on SignedPublicKey and
+    // SignedSecretKey we can not generically implement this.
+    fn to_asc(&self, header: Option<(&str, &str)>) -> String;
+
     /// The fingerprint for the key.
     fn fingerprint(&self) -> Fingerprint {
         Fingerprint::new(KeyTrait::fingerprint(self))
@@ -121,6 +137,22 @@ impl DcKey for SignedPublicKey {
             Err(err) => Err(err.into()),
         }
     }
+
+    fn to_asc(&self, header: Option<(&str, &str)>) -> String {
+        // Not using .to_armored_string() to make clear *why* it is
+        // safe to ignore this error.
+        // Because we write to a Vec<u8> the io::Write impls never
+        // fail and we can hide this error.
+        let headers = header.map(|(key, value)| {
+            let mut m = BTreeMap::new();
+            m.insert(key.to_string(), value.to_string());
+            m
+        });
+        let mut buf = Vec::new();
+        self.to_armored_writer(&mut buf, headers.as_ref())
+            .unwrap_or_default();
+        std::string::String::from_utf8(buf).unwrap_or_default()
+    }
 }
 
 #[async_trait]
@@ -150,6 +182,22 @@ impl DcKey for SignedSecretKey {
             Err(err) => Err(err.into()),
         }
     }
+
+    fn to_asc(&self, header: Option<(&str, &str)>) -> String {
+        // Not using .to_armored_string() to make clear *why* it is
+        // safe to do these unwraps.
+        // Because we write to a Vec<u8> the io::Write impls never
+        // fail and we can hide this error.  The string is always ASCII.
+        let headers = header.map(|(key, value)| {
+            let mut m = BTreeMap::new();
+            m.insert(key.to_string(), value.to_string());
+            m
+        });
+        let mut buf = Vec::new();
+        self.to_armored_writer(&mut buf, headers.as_ref())
+            .unwrap_or_default();
+        std::string::String::from_utf8(buf).unwrap_or_default()
+    }
 }
 
 async fn generate_keypair(context: &Context) -> Result<KeyPair> {
@@ -264,58 +312,6 @@ impl<'a> std::convert::TryFrom<&'a Key> for &'a SignedPublicKey {
 }
 
 impl Key {
-    pub fn is_public(&self) -> bool {
-        match self {
-            Key::Public(_) => true,
-            Key::Secret(_) => false,
-        }
-    }
-
-    pub fn is_secret(&self) -> bool {
-        !self.is_public()
-    }
-
-    pub fn from_slice(bytes: &[u8], key_type: KeyType) -> Result<Self> {
-        if bytes.is_empty() {
-            return Err(Error::Empty);
-        }
-
-        let res = match key_type {
-            KeyType::Public => SignedPublicKey::from_bytes(Cursor::new(bytes))?.into(),
-            KeyType::Private => SignedSecretKey::from_bytes(Cursor::new(bytes))?.into(),
-        };
-
-        Ok(res)
-    }
-
-    pub fn from_armored_string(
-        data: &str,
-        key_type: KeyType,
-    ) -> Option<(Self, BTreeMap<String, String>)> {
-        let bytes = data.as_bytes();
-        let res: std::result::Result<(Key, _), _> = match key_type {
-            KeyType::Public => SignedPublicKey::from_armor_single(Cursor::new(bytes))
-                .map(|(k, h)| (Into::into(k), h)),
-            KeyType::Private => SignedSecretKey::from_armor_single(Cursor::new(bytes))
-                .map(|(k, h)| (Into::into(k), h)),
-        };
-
-        match res {
-            Ok(res) => Some(res),
-            Err(err) => {
-                eprintln!("Invalid key bytes: {:?}", err);
-                None
-            }
-        }
-    }
-
-    pub fn to_bytes(&self) -> Vec<u8> {
-        match self {
-            Key::Public(k) => Serialize::to_bytes(&k).unwrap_or_default(),
-            Key::Secret(k) => Serialize::to_bytes(&k).unwrap_or_default(),
-        }
-    }
-
     pub fn verify(&self) -> bool {
         match self {
             Key::Public(k) => k.verify().is_ok(),
@@ -323,42 +319,6 @@ impl Key {
         }
     }
 
-    pub fn to_armored_string(
-        &self,
-        headers: Option<&BTreeMap<String, String>>,
-    ) -> pgp::errors::Result<String> {
-        match self {
-            Key::Public(k) => k.to_armored_string(headers),
-            Key::Secret(k) => k.to_armored_string(headers),
-        }
-    }
-
-    /// Each header line must be terminated by `\r\n`
-    pub fn to_asc(&self, header: Option<(&str, &str)>) -> String {
-        let headers = header.map(|(key, value)| {
-            let mut m = BTreeMap::new();
-            m.insert(key.to_string(), value.to_string());
-            m
-        });
-
-        self.to_armored_string(headers.as_ref())
-            .expect("failed to serialize key")
-    }
-
-    pub async fn write_asc_to_file(
-        &self,
-        file: impl AsRef<Path>,
-        context: &Context,
-    ) -> std::io::Result<()> {
-        let file_content = self.to_asc(None).into_bytes();
-
-        let res = dc_write_file(context, &file, &file_content).await;
-        if res.is_err() {
-            error!(context, "Cannot write key to {}", file.as_ref().display());
-        }
-        res
-    }
-
     pub fn split_key(&self) -> Option<Key> {
         match self {
             Key::Public(_) => None,
@@ -552,9 +512,19 @@ mod tests {
         assert_eq!(fingerprint, "1234567890ABCDABCDEFABCDEF");
     }
 
+    #[test]
+    fn test_format_fingerprint() {
+        let fingerprint = dc_format_fingerprint("1234567890ABCDABCDEFABCDEF1234567890ABCD");
+
+        assert_eq!(
+            fingerprint,
+            "1234 5678 90AB CDAB CDEF\nABCD EF12 3456 7890 ABCD"
+        );
+    }
+
     #[test]
     fn test_from_armored_string() {
-        let (private_key, _) = Key::from_armored_string(
+        let (private_key, _) = SignedSecretKey::from_asc(
             "-----BEGIN PGP PRIVATE KEY BLOCK-----
 
 xcLYBF0fgz4BCADnRUV52V4xhSsU56ZaAn3+3oG86MZhXy4X8w14WZZDf0VJGeTh
@@ -612,58 +582,64 @@ i8pcjGO+IZffvyZJVRWfVooBJmWWbPB1pueo3tx8w3+fcuzpxz+RLFKaPyqXO+dD
 7yPJeQ==
 =KZk/
 -----END PGP PRIVATE KEY BLOCK-----",
-            KeyType::Private,
         )
-        .expect("failed to decode"); // NOTE: if you take out the ===GU1/ part, everything passes!
+        .expect("failed to decode");
-        let binary = private_key.to_bytes();
+        let binary = DcKey::to_bytes(&private_key);
-        Key::from_slice(&binary, KeyType::Private).expect("invalid private key");
+        SignedSecretKey::from_slice(&binary).expect("invalid private key");
     }
 
     #[test]
-    fn test_format_fingerprint() {
+    fn test_asc_roundtrip() {
-        let fingerprint = dc_format_fingerprint("1234567890ABCDABCDEFABCDEF1234567890ABCD");
+        let key = KEYPAIR.public.clone();
+        let asc = key.to_asc(Some(("spam", "ham")));
+        let (key2, hdrs) = SignedPublicKey::from_asc(&asc).unwrap();
+        assert_eq!(key, key2);
+        assert_eq!(hdrs.len(), 1);
+        assert_eq!(hdrs.get("spam"), Some(&String::from("ham")));
 
-        assert_eq!(
+        let key = KEYPAIR.secret.clone();
-            fingerprint,
+        let asc = key.to_asc(Some(("spam", "ham")));
-            "1234 5678 90AB CDAB CDEF\nABCD EF12 3456 7890 ABCD"
+        let (key2, hdrs) = SignedSecretKey::from_asc(&asc).unwrap();
-        );
+        assert_eq!(key, key2);
+        assert_eq!(hdrs.len(), 1);
+        assert_eq!(hdrs.get("spam"), Some(&String::from("ham")));
     }
 
     #[test]
     fn test_from_slice_roundtrip() {
-        let public_key = Key::from(KEYPAIR.public.clone());
+        let public_key = KEYPAIR.public.clone();
-        let private_key = Key::from(KEYPAIR.secret.clone());
+        let private_key = KEYPAIR.secret.clone();
 
-        let binary = public_key.to_bytes();
+        let binary = DcKey::to_bytes(&public_key);
-        let public_key2 = Key::from_slice(&binary, KeyType::Public).expect("invalid public key");
+        let public_key2 = SignedPublicKey::from_slice(&binary).expect("invalid public key");
         assert_eq!(public_key, public_key2);
 
-        let binary = private_key.to_bytes();
+        let binary = DcKey::to_bytes(&private_key);
-        let private_key2 = Key::from_slice(&binary, KeyType::Private).expect("invalid private key");
+        let private_key2 = SignedSecretKey::from_slice(&binary).expect("invalid private key");
         assert_eq!(private_key, private_key2);
     }
 
     #[test]
     fn test_from_slice_bad_data() {
         let mut bad_data: [u8; 4096] = [0; 4096];
-
         for i in 0..4096 {
             bad_data[i] = (i & 0xff) as u8;
         }
-
         for j in 0..(4096 / 40) {
-            let bad_key = Key::from_slice(
+            let slice = &bad_data[j..j + 4096 / 2 + j];
-                &bad_data[j..j + 4096 / 2 + j],
+            assert!(SignedPublicKey::from_slice(slice).is_err());
-                if 0 != j & 1 {
+            assert!(SignedSecretKey::from_slice(slice).is_err());
-                    KeyType::Public
-                } else {
-                    KeyType::Private
-                },
-            );
-            assert!(bad_key.is_err());
         }
     }
 
+    #[test]
+    fn test_base64_roundtrip() {
+        let key = KEYPAIR.public.clone();
+        let base64 = key.to_base64();
+        let key2 = SignedPublicKey::from_base64(&base64).unwrap();
+        assert_eq!(key, key2);
+    }
+
     #[async_std::test]
     async fn test_load_self_existing() {
         let alice = alice_keypair();
@@ -721,23 +697,6 @@ i8pcjGO+IZffvyZJVRWfVooBJmWWbPB1pueo3tx8w3+fcuzpxz+RLFKaPyqXO+dD
         assert_eq!(res0.unwrap(), res1.unwrap());
     }
 
-    #[test]
-    fn test_ascii_roundtrip() {
-        let public_key = Key::from(KEYPAIR.public.clone());
-        let private_key = Key::from(KEYPAIR.secret.clone());
-
-        let s = public_key.to_armored_string(None).unwrap();
-        let (public_key2, _) =
-            Key::from_armored_string(&s, KeyType::Public).expect("invalid public key");
-        assert_eq!(public_key, public_key2);
-
-        let s = private_key.to_armored_string(None).unwrap();
-        println!("{}", &s);
-        let (private_key2, _) =
-            Key::from_armored_string(&s, KeyType::Private).expect("invalid private key");
-        assert_eq!(private_key, private_key2);
-    }
-
     #[test]
     fn test_split_key() {
         let private_key = Key::from(KEYPAIR.secret.clone());