fix: ensure keys are always valid (#66)

- always verify keys
- ensure serialized blobs stay allocated until written to sqlite

src/dc_apeerstate.rs

@@ -436,47 +436,44 @@ pub unsafe fn dc_apeerstate_save_to_db(
         sqlite3_bind_int64(stmt, 2, peerstate.last_seen_autocrypt as sqlite3_int64);
         sqlite3_bind_int64(stmt, 3, peerstate.prefer_encrypt as sqlite3_int64);
 
-        if let Some(ref key) = peerstate.public_key {
+        let pub_bytes = peerstate.public_key.as_ref().map(|k| k.to_bytes());
-            let b = key.to_bytes();
+        let gossip_bytes = peerstate.gossip_key.as_ref().map(|k| k.to_bytes());
-            sqlite3_bind_blob(
+        let ver_bytes = peerstate.verified_key.as_ref().map(|k| k.to_bytes());
-                stmt,
+
-                4,
+        sqlite3_bind_blob(
-                b.as_ptr() as *const _,
+            stmt,
-                b.len() as libc::c_int,
+            4,
-                None,
+            pub_bytes
-            );
+                .as_ref()
-        } else {
+                .map(|b| b.as_ptr())
-            sqlite3_bind_blob(stmt, 4, std::ptr::null(), 0, None);
+                .unwrap_or_else(|| std::ptr::null()) as *const _,
-        }
+            pub_bytes.as_ref().map(|b| b.len()).unwrap_or_else(|| 0) as libc::c_int,
+            None,
+        );
 
         sqlite3_bind_int64(stmt, 5, peerstate.gossip_timestamp as sqlite3_int64);
-        if let Some(ref key) = peerstate.gossip_key {
+        sqlite3_bind_blob(
-            let b = key.to_bytes();
+            stmt,
-            sqlite3_bind_blob(
+            6,
-                stmt,
+            gossip_bytes
-                6,
+                .as_ref()
-                b.as_ptr() as *const _,
+                .map(|b| b.as_ptr())
-                b.len() as libc::c_int,
+                .unwrap_or_else(|| std::ptr::null()) as *const _,
-                None,
+            gossip_bytes.as_ref().map(|b| b.len()).unwrap_or_else(|| 0) as libc::c_int,
-            );
+            None,
-        } else {
+        );
-            sqlite3_bind_blob(stmt, 6, std::ptr::null(), 0, None);
-        }
-
         sqlite3_bind_text(stmt, 7, peerstate.public_key_fingerprint, -1, None);
         sqlite3_bind_text(stmt, 8, peerstate.gossip_key_fingerprint, -1, None);
-        if let Some(ref key) = peerstate.verified_key {
+        sqlite3_bind_blob(
-            let b = key.to_bytes();
+            stmt,
-            sqlite3_bind_blob(
+            9,
-                stmt,
+            ver_bytes
-                9,
+                .as_ref()
-                b.as_ptr() as *const _,
+                .map(|b| b.as_ptr())
-                b.len() as libc::c_int,
+                .unwrap_or_else(|| std::ptr::null()) as *const _,
-                None,
+            ver_bytes.as_ref().map(|b| b.len()).unwrap_or_else(|| 0) as libc::c_int,
-            );
+            None,
-        } else {
+        );
-            sqlite3_bind_blob(stmt, 9, std::ptr::null(), 0, None);
-        }
 
         sqlite3_bind_text(stmt, 10, peerstate.verified_key_fingerprint, -1, None);
         sqlite3_bind_text(stmt, 11, peerstate.addr, -1, None);

src/dc_imex.rs

@@ -531,20 +531,20 @@ unsafe fn set_self_key(
                 b"DELETE FROM keypairs WHERE public_key=? OR private_key=?;\x00" as *const u8
                     as *const libc::c_char,
             );
-            let bytes = public_key.to_bytes();
+            let pub_bytes = public_key.to_bytes();
             sqlite3_bind_blob(
                 stmt,
                 1,
-                bytes.as_ptr() as *const _,
+                pub_bytes.as_ptr() as *const _,
-                bytes.len() as libc::c_int,
+                pub_bytes.len() as libc::c_int,
                 None,
             );
-            let bytes = private_key.to_bytes();
+            let priv_bytes = private_key.to_bytes();
             sqlite3_bind_blob(
                 stmt,
                 2,
-                bytes.as_ptr() as *const _,
+                priv_bytes.as_ptr() as *const _,
-                bytes.len() as libc::c_int,
+                priv_bytes.len() as libc::c_int,
                 None,
             );
             sqlite3_step(stmt);

src/dc_key.rs

@@ -91,13 +91,24 @@ impl Key {
     }
 
     pub fn from_slice(bytes: &[u8], key_type: KeyType) -> Option<Self> {
-        match key_type {
+        let res: Result<Key, _> = match key_type {
-            KeyType::Public => SignedPublicKey::from_bytes(Cursor::new(bytes))
+            KeyType::Public => SignedPublicKey::from_bytes(Cursor::new(bytes)).map(Into::into),
-                .map(Into::into)
+            KeyType::Private => SignedSecretKey::from_bytes(Cursor::new(bytes)).map(Into::into),
-                .ok(),
+        };
-            KeyType::Private => SignedSecretKey::from_bytes(Cursor::new(bytes))
+
-                .map(Into::into)
+        match res {
-                .ok(),
+            Ok(key) => {
+                if key.verify() {
+                    Some(key)
+                } else {
+                    eprintln!("Invalid key: {:?}", key);
+                    None
+                }
+            }
+            Err(err) => {
+                println!("Invalid key bytes: {:?}\n{}", err, hex::encode(bytes));
+                None
+            }
         }
     }
 
@@ -204,6 +215,13 @@ impl Key {
         }
     }
 
+    pub fn verify(&self) -> bool {
+        match self {
+            Key::Public(k) => k.verify().is_ok(),
+            Key::Secret(k) => k.verify().is_ok(),
+        }
+    }
+
     pub fn to_base64(&self, break_every: usize) -> String {
         let buf = self.to_bytes();
 
@@ -440,4 +458,18 @@ mod tests {
             "1234 5678 90AB CDAB CDEF\nABCD EF12 3456 7890 ABCD"
         );
     }
+
+    #[test]
+    fn test_from_slice_roundtrip() {
+        let (public_key, private_key) =
+            crate::dc_pgp::dc_pgp_create_keypair(CString::new("hello").unwrap().as_ptr()).unwrap();
+
+        let binary = public_key.to_bytes();
+        let public_key2 = Key::from_slice(&binary, KeyType::Public).expect("invalid public key");
+        assert_eq!(public_key, public_key2);
+
+        let binary = private_key.to_bytes();
+        let private_key2 = Key::from_slice(&binary, KeyType::Private).expect("invalid private key");
+        assert_eq!(private_key, private_key2);
+    }
 }