Rework the keys module.

  - Add a new field 'primary' for the primary key.

  - Improve error handling.

  - Actually do a little canonicalization.

  - Improve and document ffi glue.

4 files changed, 147 insertions, 53 deletions

diff --git a/src/ffi.rs b/src/ffi.rs
index f34ce4fd..735b1186 100644
--- a/src/ffi.rs
+++ b/src/ffi.rs
@@ -9,7 +9,6 @@ use std::str;
 use keys::TPK;
 use net::KeyServer;
 use openpgp::types::KeyId;
-use openpgp;
 use self::libc::{uint8_t, uint64_t, c_char, size_t};
 use self::native_tls::Certificate;
 use super::{Config, Context};
@@ -157,30 +156,25 @@ pub extern "system" fn sq_keyid_free(keyid: *mut KeyId) {
 }
 
 
-/* keys.  */
+/* sequoia::keys.  */
+
+/// Returns the first TPK found in `buf`.
+///
+/// `buf` must be an OpenPGP encoded message.
 #[no_mangle]
 pub extern "system" fn sq_tpk_from_bytes(b: *const uint8_t, len: size_t) -> *mut TPK {
     assert!(!b.is_null());
-    let bytes = unsafe {
+    let buf = unsafe {
         slice::from_raw_parts(b, len as usize)
     };
-    let m = openpgp::Message::from_bytes(bytes);
-
-    if let Some(tpk) = m.ok().and_then(|m| TPK::from_message(m)) {
+    if let Ok(tpk) = TPK::from_bytes(buf) {
         Box::into_raw(Box::new(tpk))
     } else {
         ptr::null_mut()
     }
 }
 
-#[no_mangle]
-pub extern "system" fn sq_tpk_dump(tpk: *mut TPK) {
-    assert!(!tpk.is_null());
-    unsafe {
-        println!("{:?}", *tpk);
-    }
-}
-
+/// Frees the TPK.
 #[no_mangle]
 pub extern "system" fn sq_tpk_free(tpk: *mut TPK) {
     if tpk.is_null() {
@@ -191,6 +185,18 @@ pub extern "system" fn sq_tpk_free(tpk: *mut TPK) {
     }
 }
 
+/// Dumps the TPK.
+#[no_mangle]
+pub extern "system" fn sq_tpk_dump(tpk: *mut TPK) {
+    assert!(!tpk.is_null());
+    unsafe {
+        println!("{:?}", *tpk);
+    }
+}
+
+
+/* sequoia::net.  */
+
 /// Returns a handle for the given URI.
 ///
 /// `uri` is a UTF-8 encoded value of a keyserver URI,
diff --git a/src/keys.rs b/src/keys.rs
index 8c4faac2..ec31ac9a 100644
--- a/src/keys.rs
+++ b/src/keys.rs
@@ -1,10 +1,18 @@
+use io;
+
 use super::openpgp;
 use super::openpgp::Packet;
 
-/// This represents a transferable public key (see [RFC 4880, section
-/// 11.1](https://tools.ietf.org/html/rfc4880#section-11.1)).
+/// A transferable public key (TPK).
+///
+/// A TPK (see [RFC 4880, section 11.1]) can be used to verify
+/// signatures and encrypt data.  It can be stored in a keystore and
+/// uploaded to keyservers.
+///
+/// [RFC 4880, section 11.1]: https://tools.ietf.org/html/rfc4880#section-11.1
 #[derive(Debug)]
 pub struct TPK {
+    primary: openpgp::Key,
     userids: Vec<UserIDBinding>,
     subkeys: Vec<SubkeyBinding>,
 }
@@ -31,17 +39,19 @@ enum States {
 }
 
 impl TPK {
-    /// Return the first transferable public key found in `m`.
-    pub fn from_message(m: openpgp::Message) -> Option<Self> {
+    /// Returns the first TPK found in `m`.
+    pub fn from_message(m: openpgp::Message) -> Result<Self> {
         let mut state = States::Start;
-        let mut tpk = TPK { userids: vec![], subkeys: vec![] };
+        let mut primary = None;
+        let mut userids = vec![];
+        let mut subkeys = vec![];
         for p in m.into_iter() {
             state = match state {
                 States::Start => {
                     /* Find the first public key packet.  */
                     match p {
                         Packet::PublicKey(pk) => {
-                            tpk.subkeys.push(SubkeyBinding{subkey: pk, signatures: vec![]});
+                            primary = Some(pk);
                             States::TPK
                         },
                         _ => States::Start,
@@ -69,11 +79,11 @@ impl TPK {
                             States::End
                         },
                         Packet::UserID(uid) => {
-                            tpk.userids.push(u);
+                            userids.push(u);
                             States::UserID(UserIDBinding{userid: uid, signatures: vec![]})
                         },
                         Packet::PublicSubkey(key) => {
-                            tpk.userids.push(u);
+                            userids.push(u);
                             States::Subkey(SubkeyBinding{subkey: key, signatures: vec![]})
                         },
                         Packet::Signature(sig) => {
@@ -90,11 +100,11 @@ impl TPK {
                             States::End
                         },
                         Packet::UserID(uid) => {
-                            tpk.subkeys.push(s);
+                            subkeys.push(s);
                             States::UserID(UserIDBinding{userid: uid, signatures: vec![]})
                         },
                         Packet::PublicSubkey(key) => {
-                            tpk.subkeys.push(s);
+                            subkeys.push(s);
                             States::Subkey(SubkeyBinding{subkey: key, signatures: vec![]})
                         },
                         Packet::Signature(sig) => {
@@ -108,46 +118,65 @@ impl TPK {
             };
         }
 
+        let mut tpk = if let Some(p) = primary {
+            TPK{primary: p, userids: userids, subkeys: subkeys}
+        } else {
+            return Err(Error::NoKeyFound);
+        };
+
         match state {
             States::UserID(u) => {
                 tpk.userids.push(u);
-                Some(tpk)
+                Ok(tpk)
             },
             States::Subkey(s) => {
                 tpk.subkeys.push(s);
-                Some(tpk)
+                Ok(tpk)
             },
-            States::End => Some(tpk),
-            _ => None,
+            States::End => Ok(tpk),
+            _ => Err(Error::NoKeyFound),
         }.and_then(|tpk| tpk.canonicalize())
     }
 
-    fn canonicalize(self) -> Option<Self> {
+    /// Returns the first TPK found in `buf`.
+    ///
+    /// `buf` must be an OpenPGP encoded message.
+    pub fn from_bytes(buf: &[u8]) -> Result<Self> {
+        openpgp::Message::from_bytes(buf)
+            .map_err(|e| Error::IoError(e))
+            .and_then(Self::from_message)
+    }
+
+    fn canonicalize(mut self) -> Result<Self> {
         // Sanity checks.
 
         // - One or more User ID packets.
         if self.userids.len() == 0 {
-            return None;
+            return Err(Error::NoUserId);
         }
 
-        // - After each Subkey packet, one Signature packet.
-        for subkey in self.subkeys.iter().skip(1) {
-            if subkey.signatures.len() == 0 {
-                return None;
-            }
-        }
+        // Drop user ids.
+        self.userids.retain(|userid| {
+            // XXX Check binding signature.
+            userid.signatures.len() > 0
+        });
+
+        // Drop invalid subkeys.
+        self.subkeys.retain(|subkey| {
+            // XXX Check binding signature.
+            subkey.signatures.len() > 0
+        });
 
-        // XXX Do some canonicalization.
+        // XXX Do some more canonicalization.
 
-        Some(self)
+        Ok(self)
     }
 
     /// Serialize the transferable public key into an OpenPGP message.
     pub fn to_message(self) -> openpgp::Message {
         let mut p : Vec<openpgp::Packet> = Vec::new();
-        let mut subkeys = self.subkeys;
 
-        p.push(openpgp::Packet::PublicKey(subkeys.remove(0).subkey));
+        p.push(openpgp::Packet::PublicKey(self.primary));
 
         for u in self.userids.into_iter() {
             p.push(openpgp::Packet::UserID(u.userid));
@@ -156,6 +185,7 @@ impl TPK {
             }
         }
 
+        let subkeys = self.subkeys;
         for k in subkeys.into_iter() {
             p.push(openpgp::Packet::PublicSubkey(k.subkey));
             for s in k.signatures.into_iter() {
@@ -167,10 +197,28 @@ impl TPK {
     }
 }
 
+type Result<T> = ::std::result::Result<T, Error>;
+
+/// Errors returned from the key routines.
+#[derive(Debug)]
+pub enum Error {
+    /// No key found in OpenPGP message.
+    NoKeyFound,
+    /// No user id found.
+    NoUserId,
+    /// An `io::Error` occured.
+    IoError(io::Error),
+}
+
+impl From<io::Error> for Error {
+    fn from(error: io::Error) -> Self {
+        Error::IoError(error)
+    }
+}
+
 #[cfg(test)]
 mod test {
-    use super::TPK;
-    use super::openpgp;
+    use super::{Error, TPK, openpgp};
 
     macro_rules! bytes {
         ( $x:expr ) => { include_bytes!(concat!("../tests/data/keys/", $x)) };
@@ -179,16 +227,22 @@ mod test {
     #[test]
     fn broken() {
         let m = openpgp::Message::from_bytes(bytes!("testy-broken-no-pk.pgp")).unwrap();
-        let tpk = TPK::from_message(m);
-        assert!(tpk.is_none());
+        if let Err(Error::NoKeyFound) = TPK::from_message(m) {
+            /* Pass.  */
+        } else {
+            panic!("Expected error, got none.");
+        }
 
         let m = openpgp::Message::from_bytes(bytes!("testy-broken-no-uid.pgp")).unwrap();
-        let tpk = TPK::from_message(m);
-        assert!(tpk.is_none());
+        if let Err(Error::NoUserId) = TPK::from_message(m) {
+            /* Pass.  */
+        } else {
+            panic!("Expected error, got none.");
+        }
 
         let m = openpgp::Message::from_bytes(bytes!("testy-broken-no-sig-on-subkey.pgp")).unwrap();
-        let tpk = TPK::from_message(m);
-        assert!(tpk.is_none());
+        let tpk = TPK::from_message(m).unwrap();
+        assert_eq!(tpk.subkeys.len(), 0);
     }
 
     #[test]
@@ -201,7 +255,7 @@ mod test {
         assert_eq!(tpk.userids.len(), 1, "number of userids");
         // XXX .value is private
         //assert_eq!(tpk.userids[0].userid.value, "Testy McTestface <testy@example.org>");
-        assert_eq!(tpk.subkeys.len(), 2, "number of subkeys");
+        assert_eq!(tpk.subkeys.len(), 1, "number of subkeys");
 
         // XXX Messages cannot be compared.
         assert_eq!(format!("{:?}", tpk.to_message()), orig_dbg);
@@ -211,7 +265,7 @@ mod test {
         let tpk = TPK::from_message(m).unwrap();
 
         assert_eq!(tpk.userids.len(), 1, "number of userids");
-        assert_eq!(tpk.subkeys.len(), 1, "number of subkeys");
+        assert_eq!(tpk.subkeys.len(), 0, "number of subkeys");
         // XXX Messages cannot be compared.
         assert_eq!(format!("{:?}", tpk.to_message()), orig_dbg);
     }
diff --git a/src/net/mod.rs b/src/net/mod.rs
index c8cf597a..672b4422 100644
--- a/src/net/mod.rs
+++ b/src/net/mod.rs
@@ -41,7 +41,7 @@ use std::io::{Cursor, Read};
 use std::io;
 
 use super::Context;
-use super::keys::TPK;
+use super::keys::{self, TPK};
 use super::openpgp::types::KeyId;
 use super::openpgp::{self, armor};
 
@@ -160,7 +160,7 @@ impl KeyServer {
         };
 
         let m = openpgp::Message::from_bytes(&key?)?;
-        TPK::from_message(m).ok_or(Error::MalformedResponse)
+        TPK::from_message(m).map_err(|e| Error::KeysError(e))
     }
 
     /// Sends the given key to the server.
@@ -235,6 +235,8 @@ pub enum Error {
     MalformedResponse,
     /// A communication partner violated the protocol.
     ProtocolViolation,
+    /// There was an error parsing the key.
+    KeysError(keys::Error),
     /// Encountered an unexpected low-level http status.
     HttpStatus(hyper::StatusCode),
     /// An `io::Error` occured.
@@ -247,6 +249,12 @@ pub enum Error {
     TlsError(native_tls::Error),
 }
 
+impl From<keys::Error> for Error {
+    fn from(e: keys::Error) -> Self {
+        Error::KeysError(e)
+    }
+}
+
 impl From<hyper::StatusCode> for Error {
     fn from(status: hyper::StatusCode) -> Self {
         Error::HttpStatus(status)
diff --git a/src/sequoia.h b/src/sequoia.h
index 09ce1ab3..6201ade7 100644
--- a/src/sequoia.h
+++ b/src/sequoia.h
@@ -110,11 +110,37 @@ struct sq_keyid *sq_keyid_from_hex (const char *id);
 /*/
 void sq_keyid_free (struct sq_keyid *keyid);
 
+
+/* sequoia::keys.  */
+
+/*/
+/// A transferable public key (TPK).
+///
+/// A TPK (see [RFC 4880, section 11.1]) can be used to verify
+/// signatures and encrypt data.  It can be stored in a keystore and
+/// uploaded to keyservers.
+///
+/// [RFC 4880, section 11.1]: https://tools.ietf.org/html/rfc4880#section-11.1
+/*/
 struct sq_tpk;
+
+/*/
+/// Returns the first TPK found in `buf`.
+///
+/// `buf` must be an OpenPGP encoded message.
+/*/
 struct sq_tpk *sq_tpk_from_bytes (const char *b, size_t len);
-void sq_tpk_dump (const struct sq_tpk *tpk);
+
+/*/
+/// Frees the TPK.
+/*/
 void sq_tpk_free (struct sq_tpk *tpk);
 
+/*/
+/// Dumps the TPK.
+/*/
+void sq_tpk_dump (const struct sq_tpk *tpk);
+
 
 /* sequoia::net.  */