ipc: Rework Agent::decrypt using async fn.

  - Previously, the code used an explicit state machine because it
    predated the async fn support in rustc.

  - This also fixes a bug where server and client lose sync if
    PKDECRYPT returns an error.

2 files changed, 145 insertions, 197 deletions

diff --git a/ipc/src/gnupg.rs b/ipc/src/gnupg.rs
index 677a82fc..300890bf 100644
--- a/ipc/src/gnupg.rs
+++ b/ipc/src/gnupg.rs
@@ -8,7 +8,7 @@ use std::ffi::OsStr;
 use std::ops::{Deref, DerefMut};
 use std::path::{Path, PathBuf};
 
-use futures::{Future, Stream};
+use futures::{Future, Stream, StreamExt};
 
 use std::task::{Poll, self};
 use std::pin::Pin;
@@ -357,7 +357,64 @@ impl Agent {
                           ciphertext: &'a crypto::mpi::Ciphertext)
         -> Result<crypto::SessionKey>
     {
-        DecryptionRequest::new(self, key, ciphertext).await
+        for option in Self::options() {
+            self.send_simple(option).await?;
+        }
+
+        let grip = Keygrip::of(key.public.mpis())?;
+        self.send_simple(format!("SETKEY {}", grip)).await?;
+        self.send_simple(format!("SETKEYDESC {}",
+                                  assuan::escape(&key.password_prompt))).await?;
+        self.send("PKDECRYPT")?;
+        while let Some(r) = self.next().await {
+            match r? {
+                assuan::Response::Inquire { ref keyword, .. }
+                if keyword == "CIPHERTEXT" =>
+                    (), // What we expect.
+                assuan::Response::Comment { .. }
+                | assuan::Response::Status { .. } =>
+                    (), // Ignore.
+                assuan::Response::Error { ref message, .. } =>
+                    return assuan::operation_failed(self, message).await,
+                r =>
+                    return assuan::protocol_error(&r),
+            }
+        }
+
+        let mut buf = Vec::new();
+        Sexp::try_from(ciphertext)?.serialize(&mut buf)?;
+        self.data(&buf)?;
+        let mut padding = true;
+        let mut data = Vec::new();
+        while let Some(r) = self.next().await {
+            match r? {
+                assuan::Response::Ok { .. }
+                | assuan::Response::Comment { .. } =>
+                    (), // Ignore.
+                assuan::Response::Status { ref keyword, ref message } =>
+                    if keyword == "PADDING" {
+                        padding = message != "0";
+                    },
+                assuan::Response::Error { ref message, .. } =>
+                    return assuan::operation_failed(self, message).await,
+                assuan::Response::Data { ref partial } =>
+                    data.extend_from_slice(partial),
+                r =>
+                    return assuan::protocol_error(&r),
+            }
+        }
+
+        // Get rid of the safety-0.
+        //
+        // gpg-agent seems to add a trailing 0, supposedly for good
+        // measure.
+        if data.iter().last() == Some(&0) {
+            let l = data.len();
+            data.truncate(l - 1);
+        }
+
+        Sexp::from_bytes(&data)?.finish_decryption(
+            &key.public, ciphertext, padding)
     }
 
     /// Computes options that we want to communicate.
@@ -589,190 +646,6 @@ impl<'a, 'b, 'c> Future for SigningRequest<'a, 'b, 'c>
     }
 }
 
-struct DecryptionRequest<'a, 'b, 'c>
-{
-    c: &'a mut assuan::Client,
-    key: &'b KeyPair,
-    ciphertext: &'c crypto::mpi::Ciphertext,
-    options: Vec<String>,
-    state: DecryptionRequestState,
-}
-
-impl<'a, 'b, 'c> DecryptionRequest<'a, 'b, 'c>
-{
-    fn new(c: &'a mut assuan::Client,
-           key: &'b KeyPair,
-           ciphertext: &'c crypto::mpi::Ciphertext)
-           -> Self {
-        Self {
-            c,
-            key,
-            ciphertext,
-            options: Agent::options(),
-            state: DecryptionRequestState::Start,
-        }
-    }
-}
-
-#[derive(Debug)]
-enum DecryptionRequestState {
-    Start,
-    Options,
-    SetKey,
-    SetKeyDesc,
-    PkDecrypt,
-    Inquire(Vec<u8>, bool), // Buffer and padding.
-}
-
-impl<'a, 'b, 'c> Future for DecryptionRequest<'a, 'b, 'c>
-{
-    type Output = Result<crypto::SessionKey>;
-
-    fn poll(mut self: Pin<&mut Self>, cx: &mut task::Context<'_>) -> Poll<Self::Output> {
-        use self::DecryptionRequestState::*;
-
-        // The compiler is not smart enough to figure out disjoint borrows
-        // through Pin via DerefMut (which wholly borrows `self`), so unwrap it
-        let Self { c, state, key, ciphertext, options } = self.deref_mut();
-        let mut client = Pin::new(c);
-
-        loop {
-            match state {
-                Start => {
-                    if options.is_empty() {
-                        let grip = Keygrip::of(key.public.mpis())?;
-                        client.send(format!("SETKEY {}", grip))?;
-                        *state = SetKey;
-                    } else {
-                        let opts = options.pop().unwrap();
-                        client.send(opts)?;
-                        *state = Options;
-                    }
-                },
-
-                Options => match client.as_mut().poll_next(cx)? {
-                    Poll::Ready(Some(r)) => match r {
-                        assuan::Response::Ok { .. }
-                        | assuan::Response::Comment { .. }
-                        | assuan::Response::Status { .. } =>
-                            (), // Ignore.
-                        assuan::Response::Error { ref message, .. } =>
-                            return Poll::Ready(operation_failed(message)),
-                        _ =>
-                            return Poll::Ready(protocol_error(&r)),
-                    },
-                    Poll::Ready(None) => {
-                        if let Some(option) = options.pop() {
-                            client.send(option)?;
-                        } else {
-                            let grip = Keygrip::of(key.public.mpis())?;
-                            client.send(format!("SETKEY {}", grip))?;
-                            *state = SetKey;
-                        }
-                    },
-                    Poll::Pending => return Poll::Pending,
-                },
-
-                SetKey => match client.as_mut().poll_next(cx)? {
-                    Poll::Ready(Some(r)) => match r {
-                        assuan::Response::Ok { .. }
-                        | assuan::Response::Comment { .. }
-                        | assuan::Response::Status { .. } =>
-                            (), // Ignore.
-                        assuan::Response::Error { ref message, .. } =>
-                            return Poll::Ready(operation_failed(message)),
-                        _ =>
-                            return Poll::Ready(protocol_error(&r)),
-                    },
-                    Poll::Ready(None) => {
-                        client.send(
-                            format!("SETKEYDESC {}",
-                                    assuan::escape(&key.password_prompt)))?;
-                        *state = SetKeyDesc;
-                    },
-                    Poll::Pending => return Poll::Pending,
-                },
-
-                SetKeyDesc => match client.as_mut().poll_next(cx)? {
-                    Poll::Ready(Some(r)) => match r {
-                        assuan::Response::Ok { .. }
-                        | assuan::Response::Comment { .. }
-                        | assuan::Response::Status { .. } =>
-                            (), // Ignore.
-                        assuan::Response::Error { ref message, .. } =>
-                            return Poll::Ready(operation_failed(message)),
-                        _ =>
-                            return Poll::Ready(protocol_error(&r)),
-                    },
-                    Poll::Ready(None) => {
-                        client.send("PKDECRYPT")?;
-                        *state = PkDecrypt;
-                    },
-                    Poll::Pending => return Poll::Pending,
-                },
-
-                PkDecrypt => match client.as_mut().poll_next(cx)? {
-                    Poll::Ready(Some(r)) => match r {
-                        assuan::Response::Inquire { ref keyword, .. }
-                          if keyword == "CIPHERTEXT" =>
-                            (), // What we expect.
-                        assuan::Response::Comment { .. }
-                        | assuan::Response::Status { .. } =>
-                            (), // Ignore.
-                        assuan::Response::Error { ref message, .. } =>
-                            return Poll::Ready(operation_failed(message)),
-                        _ =>
-                            return Poll::Ready(protocol_error(&r)),
-                    },
-                    Poll::Ready(None) => {
-                        let mut buf = Vec::new();
-                        Sexp::try_from(*ciphertext)?
-                            .serialize(&mut buf)?;
-                        client.data(&buf)?;
-                        *state = Inquire(Vec::new(), true);
-                    },
-                    Poll::Pending => return Poll::Pending,
-                },
-
-
-                Inquire(ref mut data, ref mut padding) => match client.as_mut().poll_next(cx)? {
-                    Poll::Ready(Some(r)) => match r {
-                        assuan::Response::Ok { .. }
-                        | assuan::Response::Comment { .. } =>
-                            (), // Ignore.
-                        assuan::Response::Status { ref keyword, ref message } =>
-                            if keyword == "PADDING" {
-                                *padding = message != "0";
-                            },
-                        assuan::Response::Error { ref message, .. } =>
-                            return Poll::Ready(operation_failed(message)),
-                        assuan::Response::Data { ref partial } =>
-                            data.extend_from_slice(partial),
-                        _ =>
-                            return Poll::Ready(protocol_error(&r)),
-                    },
-                    Poll::Ready(None) => {
-                        // Get rid of the safety-0.
-                        //
-                        // gpg-agent seems to add a trailing 0,
-                        // supposedly for good measure.
-                        if data.iter().last() == Some(&0) {
-                            let l = data.len();
-                            data.truncate(l - 1);
-                        }
-
-                        return Poll::Ready(
-                            Sexp::from_bytes(&data)?.finish_decryption(
-                            &key.public, ciphertext, *padding)
-                        );
-                    },
-                    Poll::Pending => return Poll::Pending,
-                },
-            }
-        }
-    }
-}
-
 /// A cryptographic key pair.
 ///
 /// A `KeyPair` is a combination of public and secret key.  This
diff --git a/ipc/tests/gpg-agent.rs b/ipc/tests/gpg-agent.rs
index 898b86c8..a244ba44 100644
--- a/ipc/tests/gpg-agent.rs
+++ b/ipc/tests/gpg-agent.rs
@@ -6,11 +6,18 @@ use anyhow::Context as _;
 use futures::StreamExt;
 
 use sequoia_openpgp as openpgp;
-use crate::openpgp::types::{
-    HashAlgorithm,
-    SymmetricAlgorithm,
+use crate::openpgp::{
+    packet::{
+        Any,
+        PKESK,
+    },
+    PacketPile,
+    types::{
+        HashAlgorithm,
+        SymmetricAlgorithm,
+    },
 };
-use crate::openpgp::crypto::SessionKey;
+use crate::openpgp::crypto::{SessionKey, Decryptor};
 use crate::openpgp::parse::{Parse, stream::*};
 use crate::openpgp::serialize::{Serialize, stream::*};
 use crate::openpgp::cert::prelude::*;
@@ -213,9 +220,16 @@ fn decrypt() -> openpgp::Result<()> {
     use openpgp::policy::StandardPolicy as P;
 
     let p = &P::new();
-    let ctx = make_context!();
+
+    // Make a cert for a second recipient.
+    let (other, _) = CertBuilder::new()
+        .add_userid("other-recipient@example.org")
+        .add_transport_encryption_subkey()
+        .generate()?;
 
     for cs in &[RSA2k, Cv25519, P521] {
+        let ctx = make_context!();
+
         let (cert, _) = CertBuilder::new()
             .set_cipher_suite(*cs)
             .add_userid("someone@example.org")
@@ -226,12 +240,18 @@ fn decrypt() -> openpgp::Result<()> {
         cert.as_tsk().serialize(&mut buf).unwrap();
         gpg_import(&ctx, &buf)?;
 
+        // Import the second recipient.
+        let mut buf = Vec::new();
+        other.serialize(&mut buf).unwrap();
+        gpg_import(&ctx, &buf)?;
+
         let mut message = Vec::new();
         {
             let recipients =
-                cert.keys().with_policy(p, None).alive().revoked(false)
-                .for_transport_encryption()
-                .map(|ka| ka.key())
+                [&cert, &other].iter().flat_map(
+                    |c| c.keys().with_policy(p, None).alive().revoked(false)
+                        .for_transport_encryption()
+                        .map(|ka| ka.key()))
                 .collect::<Vec<_>>();
 
             // Start streaming an OpenPGP message.
@@ -254,9 +274,41 @@ fn decrypt() -> openpgp::Result<()> {
             literal_writer.finalize().unwrap();
         }
 
+        // First, test Agent::decrypt.  Using this function we can try
+        // multiple decryption requests on the same connection.
+        let rt = tokio::runtime::Runtime::new()?;
+        let mut agent = rt.block_on(Agent::connect(&ctx))?;
+        let pp = PacketPile::from_bytes(&message)?;
+        let pkesk_0: &PKESK =
+            pp.path_ref(&[0]).unwrap().downcast_ref().unwrap();
+        let pkesk_1: &PKESK =
+            pp.path_ref(&[1]).unwrap().downcast_ref().unwrap();
+
+        // We only gave the cert to GnuPG, the agent doesn't have the
+        // secret.
+        let keypair = KeyPair::new(
+            &ctx,
+            other.keys().with_policy(p, None)
+                .for_storage_encryption().for_transport_encryption()
+                .take(1).next().unwrap().key())?;
+        assert!(rt.block_on(agent.decrypt(&keypair, pkesk_1.esk()))
+                .is_err());
+
+        // Now try "our" key.
+        let keypair = KeyPair::new(
+            &ctx,
+            cert.keys().with_policy(p, None)
+                .for_storage_encryption().for_transport_encryption()
+                .take(1).next().unwrap().key())?;
+        assert!(rt.block_on(agent.decrypt(&keypair, pkesk_0.esk()))
+                .is_ok());
+
+        // Close connection.
+        drop(agent);
+
         // Make a helper that that feeds the recipient's secret key to the
         // decryptor.
-        let helper = Helper { policy: p, ctx: &ctx, cert: &cert, };
+        let helper = Helper { policy: p, ctx: &ctx, cert: &cert, other: &other};
 
         // Now, create a decryptor with a helper using the given Certs.
         let mut decryptor = DecryptorBuilder::from_bytes(&message).unwrap()
@@ -271,6 +323,7 @@ fn decrypt() -> openpgp::Result<()> {
             policy: &'a dyn Policy,
             ctx: &'a Context,
             cert: &'a openpgp::Cert,
+            other: &'a openpgp::Cert,
         }
 
         impl<'a> VerificationHelper for Helper<'a> {
@@ -296,6 +349,20 @@ fn decrypt() -> openpgp::Result<()> {
                           -> openpgp::Result<Option<openpgp::Fingerprint>>
                 where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
             {
+                // We only gave the cert to GnuPG, the agent doesn't
+                // have the secret.
+                let mut keypair = KeyPair::new(
+                    self.ctx,
+                    self.other.keys().with_policy(self.policy, None)
+                        .for_storage_encryption().for_transport_encryption()
+                        .take(1).next().unwrap().key())
+                    .unwrap();
+
+                for pkesk in pkesks {
+                    assert!(pkesk.decrypt(&mut keypair, sym_algo).is_none());
+                }
+
+                // Now use "our" key.
                 let mut keypair = KeyPair::new(
                     self.ctx,
                     self.cert.keys().with_policy(self.policy, None)
@@ -303,8 +370,16 @@ fn decrypt() -> openpgp::Result<()> {
                         .take(1).next().unwrap().key())
                     .unwrap();
 
-                pkesks[0].decrypt(&mut keypair, sym_algo)
-                    .map(|(algo, session_key)| decrypt(algo, &session_key));
+                for pkesk in pkesks {
+                    if *pkesk.recipient() != keypair.public().keyid() {
+                        continue;
+                    }
+
+                    let (algo, session_key) =
+                        pkesk.decrypt(&mut keypair, sym_algo)
+                        .expect("decryption must succeed");
+                    assert!(decrypt(algo, &session_key));
+                }
 
                 // XXX: In production code, return the Fingerprint of the
                 // recipient's Cert here