net: Fix KeyServer::{get,search}.

  - Change both functions to return a vector of Result<Cert>.  First,
    the keyserver may return more than one cert for both user ID
    searches as well as searches by key handle.  Second, we may
    understand not all certs returned by the server, and that is okay,
    and we shouldn't let the whole query fail because of that.

  - Previously, the functions made an attempt at validating the
    results.  However, that is flawed.  First, in the case of
    retrieval by key handle, the test was brittle and easily
    circumvented by a key server.  Second, the server may have good
    reason to return an additional cert, even if it doesn't have the
    key handle that the user asked for.  For example, it may know that
    a cert is superseded and return the new one too, as a courtesy.
    draft-shaw-openpgp-hkp-00 doesn't forbid that.

  - In the case of search by email address, the server may know the
    association between the queried email address and the cert, even
    if said association is not recorded in the cert itself.

  - Remove the brittle checks, return all certs returned by the
    server, add a warning to the documentation of KeyServer::get.

3 files changed, 18 insertions, 54 deletions

diff --git a/net/examples/tor-hkp-get.rs b/net/examples/tor-hkp-get.rs
index 959918d2..17ac3894 100644
--- a/net/examples/tor-hkp-get.rs
+++ b/net/examples/tor-hkp-get.rs
@@ -40,6 +40,8 @@ async fn main() -> Result<()> {
         client)?;
 
     // Finally, get the requested certificate.
-    keyserver.get(handle).await?.armored().serialize(&mut io::stdout())?;
+    for cert in keyserver.get(handle).await? {
+        cert?.armored().serialize(&mut io::stdout())?;
+    }
     Ok(())
 }
diff --git a/net/src/lib.rs b/net/src/lib.rs
index eb42b246..fc804904 100644
--- a/net/src/lib.rs
+++ b/net/src/lib.rs
@@ -43,8 +43,6 @@ pub use reqwest;
 
 use percent_encoding::{percent_encode, AsciiSet, CONTROLS};
 
-use std::io::Cursor;
-
 use reqwest::{
     StatusCode,
     Url,
@@ -52,7 +50,6 @@ use reqwest::{
 
 use sequoia_openpgp::{
     self as openpgp,
-    armor,
     cert::{Cert, CertParser},
     KeyHandle,
     packet::UserID,
@@ -130,8 +127,13 @@ impl KeyServer {
     }
 
     /// Retrieves the certificate with the given handle.
+    ///
+    /// # Warning
+    ///
+    /// Returned certificates must be mistrusted, and be carefully
+    /// interpreted under a policy and trust model.
     pub async fn get<H: Into<KeyHandle>>(&self, handle: H)
-                                         -> Result<Cert>
+                                         -> Result<Vec<Result<Cert>>>
     {
         let handle = handle.into();
         let url = self.request_url.join(
@@ -141,33 +143,8 @@ impl KeyServer {
         match res.status() {
             StatusCode::OK => {
                 let body = res.bytes().await?;
-                let r = armor::Reader::from_reader(
-                    Cursor::new(body),
-                    armor::ReaderMode::Tolerant(Some(armor::Kind::PublicKey)),
-                );
-                let cert = Cert::from_reader(r)?;
-                // XXX: This test is dodgy.  Passing it doesn't really
-                // mean anything.  A malicious keyserver can attach
-                // the key with the queried keyid to any certificate
-                // they control.  Querying for signing-capable sukeys
-                // are safe because they require a primary key binding
-                // signature which the server cannot produce.
-                // However, if the public key algorithm is also
-                // capable of encryption (I'm looking at you, RSA),
-                // then the server can simply turn it into an
-                // encryption subkey.
-                //
-                // Returned certificates must be mistrusted, and be
-                // carefully interpreted under a policy and trust
-                // model.  This test doesn't provide any real
-                // protection, and maybe it is better to remove it.
-                // That would also help with returning multiple certs,
-                // see above.
-                if cert.keys().any(|ka| ka.key_handle().aliases(&handle)) {
-                    Ok(cert)
-                } else {
-                    Err(Error::MismatchedKeyHandle(handle, cert).into())
-                }
+                let certs = CertParser::from_bytes(&body)?.collect();
+                Ok(certs)
             }
             StatusCode::NOT_FOUND => Err(Error::NotFound.into()),
             n => Err(Error::HttpStatus(n).into()),
@@ -180,18 +157,15 @@ impl KeyServer {
     /// conventions for userids, or it does not contain a email
     /// address, an error is returned.
     ///
-    ///   [`UserID`]: https://docs.sequoia-pgp.org/sequoia_openpgp/packet/struct.UserID.html
-    ///
-    /// Any certificates returned by the server that do not contain
-    /// the email address queried for are silently discarded.
+    ///   [`UserID`]: sequoia_openpgp::packet::UserID
     ///
     /// # Warning
     ///
     /// Returned certificates must be mistrusted, and be carefully
     /// interpreted under a policy and trust model.
     #[allow(clippy::blocks_in_if_conditions)]
-    pub async fn search<U: Into<UserID>>(&mut self, userid: U)
-                                         -> Result<Vec<Cert>>
+    pub async fn search<U: Into<UserID>>(&self, userid: U)
+                                         -> Result<Vec<Result<Cert>>>
     {
         let userid = userid.into();
         let email = userid.email2().and_then(|addr| addr.ok_or_else(||
@@ -203,20 +177,7 @@ impl KeyServer {
         let res = self.client.get(url).send().await?;
         match res.status() {
             StatusCode::OK => {
-                let body = res.bytes().await?;
-                let mut certs = Vec::new();
-                for certo in CertParser::from_bytes(&body)? {
-                    let cert = certo?;
-                    if cert.userids().any(|uid| {
-                        uid.email2().ok()
-                            .and_then(|addro| addro)
-                            .map(|addr| addr == email)
-                            .unwrap_or(false)
-                    }) {
-                        certs.push(cert);
-                    }
-                }
-                Ok(certs)
+                Ok(CertParser::from_bytes(&res.bytes().await?)?.collect())
             },
             StatusCode::NOT_FOUND => Err(Error::NotFound.into()),
             n => Err(Error::HttpStatus(n).into()),
diff --git a/net/tests/hkp.rs b/net/tests/hkp.rs
index 4db859e2..5133c280 100644
--- a/net/tests/hkp.rs
+++ b/net/tests/hkp.rs
@@ -124,9 +124,10 @@ async fn get() -> anyhow::Result<()> {
 
     let keyserver = KeyServer::new(&format!("hkp://{}", addr))?;
     let keyid: KeyID = ID.parse()?;
-    let key = keyserver.get(keyid).await?;
+    let keys = keyserver.get(keyid).await?;
+    assert_eq!(keys.len(), 1);
 
-    assert_eq!(key.fingerprint(),
+    assert_eq!(keys[0].as_ref().unwrap().fingerprint(),
                FP.parse().unwrap());
     Ok(())
 }