plugins: Change recipient-v1 state machine phase 2 to be bidirectional

The previous iteration of the recipient-v1 state machine assumed that
user interaction would never be required during encryption. This is
almost certainly true for asymmetric recipients, but is not the case
for symmetric recipients (e.g. the symmetric key might be stored on a
hardware token that requires a PIN).

The recipient-v1 state machine now uses a bi-directional second phase,
matching the identity-v1 state machine. It defines the same commands
for interacting with users.

age-core/src/plugin.rs

@@ -289,6 +289,29 @@ impl<'a, R: Read, W: Write> BidirSend<'a, R, W> {
             )),
         }
     }
+
+    /// Send an entire stanza.
+    pub fn send_stanza(
+        &mut self,
+        command: &str,
+        metadata: &[&str],
+        stanza: &Stanza,
+    ) -> Result<Stanza, ()> {
+        for grease in self.0.grease_gun() {
+            self.0.send(&grease.tag, &grease.args, &grease.body)?;
+            self.0.receive()?;
+        }
+        self.0.send_stanza(command, metadata, stanza)?;
+        let s = self.0.receive()?;
+        match s.tag.as_ref() {
+            RESPONSE_OK => Ok(Ok(s)),
+            RESPONSE_FAIL => Ok(Err(())),
+            tag => Err(io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("unexpected response: {}", tag),
+            )),
+        }
+    }
 }
 
 /// The possible replies to a bidirectional command.

age-plugin/examples/age-plugin-unencrypted.rs

@@ -1,9 +1,9 @@
 use age_core::format::{FileKey, Stanza};
 use age_plugin::{
-    identity::{self, Callbacks, IdentityPluginV1},
+    identity::{self, IdentityPluginV1},
     print_new_identity,
     recipient::{self, RecipientPluginV1},
-    run_state_machine,
+    run_state_machine, Callbacks,
 };
 use gumdrop::Options;
 use secrecy::ExposeSecret;
@@ -42,13 +42,19 @@ impl RecipientPluginV1 for RecipientPlugin {
         }
     }
 
-    fn wrap_file_key(&mut self, file_key: &FileKey) -> Result<Vec<Stanza>, Vec<recipient::Error>> {
+    fn wrap_file_key(
+        &mut self,
+        file_key: &FileKey,
+        mut callbacks: impl Callbacks<recipient::Error>,
+    ) -> io::Result<Result<Vec<Stanza>, Vec<recipient::Error>>> {
         // A real plugin would wrap the file key here.
-        Ok(vec![Stanza {
+        let _ = callbacks
+            .message("This plugin doesn't have any recipient-specific logic. It's unencrypted!")?;
+        Ok(Ok(vec![Stanza {
             tag: RECIPIENT_TAG.to_owned(),
             args: vec!["does".to_owned(), "nothing".to_owned()],
             body: file_key.expose_secret().to_vec(),
-        }])
+        }]))
     }
 }
 
@@ -83,7 +89,7 @@ impl IdentityPluginV1 for IdentityPlugin {
     fn unwrap_file_keys(
         &mut self,
         files: Vec<Vec<Stanza>>,
-        mut callbacks: impl Callbacks,
+        mut callbacks: impl Callbacks<identity::Error>,
     ) -> io::Result<HashMap<usize, Result<FileKey, Vec<identity::Error>>>> {
         let mut file_keys = HashMap::with_capacity(files.len());
         for (file_index, stanzas) in files.into_iter().enumerate() {

age-plugin/src/identity.rs

@@ -8,30 +8,11 @@ use secrecy::{ExposeSecret, SecretString};
 use std::collections::HashMap;
 use std::io;
 
+use crate::Callbacks;
+
 const ADD_IDENTITY: &str = "add-identity";
 const RECIPIENT_STANZA: &str = "recipient-stanza";
 
-/// The interface that age plugins can use to interact with an age implementation.
-pub trait Callbacks {
-    /// Shows a message to the user.
-    ///
-    /// This can be used to prompt the user to take some physical action, such as
-    /// inserting a hardware key.
-    fn message(&mut self, message: &str) -> plugin::Result<(), ()>;
-
-    /// Requests a secret value from the user, such as a passphrase.
-    ///
-    /// `message` will be displayed to the user, providing context for the request.
-    fn request_secret(&mut self, message: &str) -> plugin::Result<SecretString, ()>;
-
-    /// Sends an error.
-    ///
-    /// Note: This API may be removed in a subsequent API refactor, after we've figured
-    /// out how errors should be handled overall, and how to distinguish between hard and
-    /// soft errors.
-    fn error(&mut self, error: Error) -> plugin::Result<(), ()>;
-}
-
 /// The interface that age implementations will use to interact with an age plugin.
 pub trait IdentityPluginV1 {
     /// Stores an identity that the user would like to use for decrypting age files.
@@ -66,14 +47,14 @@ pub trait IdentityPluginV1 {
     fn unwrap_file_keys(
         &mut self,
         files: Vec<Vec<Stanza>>,
-        callbacks: impl Callbacks,
+        callbacks: impl Callbacks<Error>,
     ) -> io::Result<HashMap<usize, Result<FileKey, Vec<Error>>>>;
 }
 
 /// The interface that age plugins can use to interact with an age implementation.
 struct BidirCallbacks<'a, 'b, R: io::Read, W: io::Write>(&'b mut BidirSend<'a, R, W>);
 
-impl<'a, 'b, R: io::Read, W: io::Write> Callbacks for BidirCallbacks<'a, 'b, R, W> {
+impl<'a, 'b, R: io::Read, W: io::Write> Callbacks<Error> for BidirCallbacks<'a, 'b, R, W> {
     /// Shows a message to the user.
     ///
     /// This can be used to prompt the user to take some physical action, such as

age-plugin/src/lib.rs

@@ -71,10 +71,10 @@
 //! ```
 //! use age_core::format::{FileKey, Stanza};
 //! use age_plugin::{
-//!     identity::{self, Callbacks, IdentityPluginV1},
+//!     identity::{self, IdentityPluginV1},
 //!     print_new_identity,
 //!     recipient::{self, RecipientPluginV1},
-//!     run_state_machine,
+//!     Callbacks, run_state_machine,
 //! };
 //! use gumdrop::Options;
 //! use std::collections::HashMap;
@@ -93,7 +93,8 @@
 //!     fn wrap_file_key(
 //!         &mut self,
 //!         file_key: &FileKey,
-//!     ) -> Result<Vec<Stanza>, Vec<recipient::Error>> {
+//!         mut callbacks: impl Callbacks<recipient::Error>,
+//!     ) -> io::Result<Result<Vec<Stanza>, Vec<recipient::Error>>> {
 //!         todo!()
 //!     }
 //! }
@@ -111,7 +112,7 @@
 //!     fn unwrap_file_keys(
 //!         &mut self,
 //!         files: Vec<Vec<Stanza>>,
-//!         mut callbacks: impl Callbacks,
+//!         mut callbacks: impl Callbacks<identity::Error>,
 //!     ) -> io::Result<HashMap<usize, Result<FileKey, Vec<identity::Error>>>> {
 //!         todo!()
 //!     }
@@ -151,6 +152,7 @@
 #![deny(intra_doc_link_resolution_failure)]
 #![deny(missing_docs)]
 
+use secrecy::SecretString;
 use std::io;
 
 pub mod identity;
@@ -209,3 +211,24 @@ pub fn run_state_machine<R: recipient::RecipientPluginV1, I: identity::IdentityP
         )),
     }
 }
+
+/// The interface that age plugins can use to interact with an age implementation.
+pub trait Callbacks<E> {
+    /// Shows a message to the user.
+    ///
+    /// This can be used to prompt the user to take some physical action, such as
+    /// inserting a hardware key.
+    fn message(&mut self, message: &str) -> age_core::plugin::Result<(), ()>;
+
+    /// Requests a secret value from the user, such as a passphrase.
+    ///
+    /// `message` will be displayed to the user, providing context for the request.
+    fn request_secret(&mut self, message: &str) -> age_core::plugin::Result<SecretString, ()>;
+
+    /// Sends an error.
+    ///
+    /// Note: This API may be removed in a subsequent API refactor, after we've figured
+    /// out how errors should be handled overall, and how to distinguish between hard and
+    /// soft errors.
+    fn error(&mut self, error: E) -> age_core::plugin::Result<(), ()>;
+}

age-plugin/src/recipient.rs

@@ -2,11 +2,14 @@
 
 use age_core::{
     format::{FileKey, Stanza, FILE_KEY_BYTES},
-    plugin::{Connection, UnidirSend},
+    plugin::{self, BidirSend, Connection},
 };
+use secrecy::SecretString;
 use std::convert::TryInto;
 use std::io;
 
+use crate::Callbacks;
+
 const ADD_RECIPIENT: &str = "add-recipient";
 const WRAP_FILE_KEY: &str = "wrap-file-key";
 const RECIPIENT_STANZA: &str = "recipient-stanza";
@@ -28,7 +31,47 @@ pub trait RecipientPluginV1 {
     ///
     /// Returns either one stanza per recipient, or any errors if one or more recipients
     /// could not be wrapped to.
-    fn wrap_file_key(&mut self, file_key: &FileKey) -> Result<Vec<Stanza>, Vec<Error>>;
+    ///
+    /// `callbacks` can be used to interact with the user, to have them take some physical
+    /// action or request a secret value.
+    fn wrap_file_key(
+        &mut self,
+        file_key: &FileKey,
+        callbacks: impl Callbacks<Error>,
+    ) -> io::Result<Result<Vec<Stanza>, Vec<Error>>>;
+}
+
+/// The interface that age plugins can use to interact with an age implementation.
+struct BidirCallbacks<'a, 'b, R: io::Read, W: io::Write>(&'b mut BidirSend<'a, R, W>);
+
+impl<'a, 'b, R: io::Read, W: io::Write> Callbacks<Error> for BidirCallbacks<'a, 'b, R, W> {
+    /// Shows a message to the user.
+    ///
+    /// This can be used to prompt the user to take some physical action, such as
+    /// inserting a hardware key.
+    fn message(&mut self, message: &str) -> plugin::Result<(), ()> {
+        self.0
+            .send("msg", &[], message.as_bytes())
+            .map(|res| res.map(|_| ()))
+    }
+
+    /// Requests a secret value from the user, such as a passphrase.
+    ///
+    /// `message` will be displayed to the user, providing context for the request.
+    fn request_secret(&mut self, message: &str) -> plugin::Result<SecretString, ()> {
+        self.0
+            .send("request-secret", &[], message.as_bytes())
+            .and_then(|res| match res {
+                Ok(s) => String::from_utf8(s.body)
+                    .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "secret is not UTF-8"))
+                    .map(|s| Ok(SecretString::new(s))),
+                Err(()) => Ok(Err(())),
+            })
+    }
+
+    fn error(&mut self, error: Error) -> plugin::Result<(), ()> {
+        error.send(self.0).map(|()| Ok(()))
+    }
 }
 
 /// The kinds of errors that can occur within the recipient plugin state machine.
@@ -62,7 +105,7 @@ impl Error {
         }
     }
 
-    fn send<R: io::Read, W: io::Write>(self, phase: &mut UnidirSend<R, W>) -> io::Result<()> {
+    fn send<R: io::Read, W: io::Write>(self, phase: &mut BidirSend<R, W>) -> io::Result<()> {
         let index = match self {
             Error::Recipient { index, .. } => Some(index.to_string()),
             Error::Internal { .. } => None,
@@ -73,7 +116,11 @@ impl Error {
             None => vec![self.kind()],
         };
 
-        phase.send("error", &metadata, self.message().as_bytes())
+        phase
+            .send("error", &metadata, self.message().as_bytes())?
+            .unwrap();
+
+        Ok(())
     }
 }
 
@@ -122,7 +169,7 @@ pub(crate) fn run_v1<P: RecipientPluginV1>(mut plugin: P) -> io::Result<()> {
     };
 
     // Phase 2: wrap the file keys or return errors
-    conn.unidir_send(|mut phase| {
+    conn.bidir_send(|mut phase| {
         let (recipients, file_keys) = match (recipients, file_keys) {
             (Ok(recipients), Ok(file_keys)) => (recipients, file_keys),
             (Err(errors1), Err(errors2)) => {
@@ -148,8 +195,8 @@ pub(crate) fn run_v1<P: RecipientPluginV1>(mut plugin: P) -> io::Result<()> {
         } else {
             match file_keys
                 .into_iter()
-                .map(|file_key| plugin.wrap_file_key(&file_key))
+                .map(|file_key| plugin.wrap_file_key(&file_key, BidirCallbacks(&mut phase)))
-                .collect::<Result<Vec<_>, _>>()
+                .collect::<Result<Result<Vec<_>, _>, _>>()?
             {
                 Ok(files) => {
                     for (file_index, stanzas) in files.into_iter().enumerate() {
@@ -159,11 +206,9 @@ pub(crate) fn run_v1<P: RecipientPluginV1>(mut plugin: P) -> io::Result<()> {
                         assert_eq!(stanzas.len(), recipients.len());
 
                         for stanza in stanzas {
-                            phase.send_stanza(
+                            phase
-                                RECIPIENT_STANZA,
+                                .send_stanza(RECIPIENT_STANZA, &[&file_index.to_string()], &stanza)?
-                                &[&file_index.to_string()],
+                                .unwrap();
-                                &stanza,
-                            )?;
                         }
                     }
                 }

age/src/plugin.rs

@@ -157,19 +157,24 @@ impl Plugin {
 ///
 /// This struct implements [`Recipient`], enabling the plugin to encrypt a file to the
 /// entire set of recipients.
-pub struct RecipientPluginV1 {
+pub struct RecipientPluginV1<C: Callbacks> {
     plugin: Plugin,
     recipients: Vec<Recipient>,
+    callbacks: C,
 }
 
-impl RecipientPluginV1 {
+impl<C: Callbacks> RecipientPluginV1<C> {
     /// Creates an age plugin from a plugin name and a list of recipients.
     ///
     /// The list of recipients will be filtered by the plugin name; recipients that don't
     /// match will be ignored.
     ///
     /// Returns an error if the plugin's binary cannot be found in `$PATH`.
-    pub fn new(plugin_name: &str, recipients: &[Recipient]) -> Result<Self, EncryptError> {
+    pub fn new(
+        plugin_name: &str,
+        recipients: &[Recipient],
+        callbacks: C,
+    ) -> Result<Self, EncryptError> {
         Plugin::new(plugin_name)
             .map_err(|binary_name| EncryptError::MissingPlugin { binary_name })
             .map(|plugin| RecipientPluginV1 {
@@ -179,11 +184,12 @@ impl RecipientPluginV1 {
                     .filter(|r| r.name == plugin_name)
                     .cloned()
                     .collect(),
+                callbacks,
             })
     }
 }
 
-impl crate::Recipient for RecipientPluginV1 {
+impl<C: Callbacks> crate::Recipient for RecipientPluginV1<C> {
     fn wrap_file_key(&self, file_key: &FileKey) -> Result<Vec<Stanza>, EncryptError> {
         // Open connection
         let mut conn = self.plugin.connect(RECIPIENT_V1)?;
@@ -197,53 +203,73 @@ impl crate::Recipient for RecipientPluginV1 {
         })?;
 
         // Phase 2: collect either stanzas or errors
-        let (stanzas, mut errors) = {
+        let mut stanzas = vec![];
-            let (stanzas, errors) = conn.unidir_receive(
+        let mut errors = vec![];
-                (CMD_RECIPIENT_STANZA, |mut s| {
+        if let Err(e) = conn.bidir_receive(
-                    if s.args.len() >= 2 {
+            &[CMD_MSG, CMD_REQUEST_SECRET, CMD_RECIPIENT_STANZA, CMD_ERROR],
+            |mut command, reply| match command.tag.as_str() {
+                CMD_MSG => {
+                    self.callbacks
+                        .prompt(&String::from_utf8_lossy(&command.body));
+                    reply.ok(None)
+                }
+                CMD_REQUEST_SECRET => {
+                    if let Some(secret) = self
+                        .callbacks
+                        .request_passphrase(&String::from_utf8_lossy(&command.body))
+                    {
+                        reply.ok(Some(secret.expose_secret().as_bytes()))
+                    } else {
+                        reply.fail()
+                    }
+                }
+                CMD_RECIPIENT_STANZA => {
+                    if command.args.len() >= 2 {
                         // We only requested one file key be wrapped.
-                        if s.args.remove(0) == "0" {
+                        if command.args.remove(0) == "0" {
-                            s.tag = s.args.remove(0);
+                            command.tag = command.args.remove(0);
-                            Ok(s)
+                            stanzas.push(command);
                         } else {
-                            Err(PluginError::Other {
+                            errors.push(PluginError::Other {
                                 kind: "internal".to_owned(),
                                 metadata: vec![],
                                 message: "plugin wrapped file key to a file we didn't provide"
                                     .to_owned(),
-                            })
+                            });
                         }
                     } else {
-                        Err(PluginError::Other {
+                        errors.push(PluginError::Other {
                             kind: "internal".to_owned(),
                             metadata: vec![],
                             message: format!(
                                 "{} command must have at least two metadata arguments",
                                 CMD_RECIPIENT_STANZA
                             ),
-                        })
+                        });
                     }
-                }),
+                    reply.ok(None)
-                (CMD_ERROR, |s| {
+                }
-                    // Here, errors are are okay!
+                CMD_ERROR => {
-                    if s.args.len() == 2 && s.args[0] == "recipient" {
+                    if command.args.len() == 2 && command.args[0] == "recipient" {
-                        let index: usize = s.args[1].parse().unwrap();
+                        let index: usize = command.args[1].parse().unwrap();
-                        Ok(PluginError::Recipient {
+                        errors.push(PluginError::Recipient {
                             binary_name: binary_name(&self.recipients[index].name),
                             recipient: self.recipients[index].recipient.clone(),
-                            message: String::from_utf8_lossy(&s.body).to_string(),
+                            message: String::from_utf8_lossy(&command.body).to_string(),
-                        })
+                        });
                     } else {
-                        Ok(PluginError::from(s))
+                        errors.push(PluginError::from(command));
                     }
-                }),
+                    reply.ok(None)
-            )?;
+                }
-            (stanzas, errors.expect("All Ok"))
+                _ => unreachable!(),
+            },
+        ) {
+            return Err(e.into());
         };
-        match (stanzas, errors.is_empty()) {
+        match (stanzas.is_empty(), errors.is_empty()) {
-            (Ok(stanzas), true) if !stanzas.is_empty() => Ok(stanzas),
+            (false, true) => Ok(stanzas),
-            (Ok(stanzas), b) => {
+            (a, b) => {
-                let a = stanzas.is_empty();
                 if a & b {
                     errors.push(PluginError::Other {
                         kind: "internal".to_owned(),
@@ -259,9 +285,6 @@ impl crate::Recipient for RecipientPluginV1 {
                 }
                 Err(EncryptError::Plugin(errors))
             }
-            (Err(errs), _) => Err(EncryptError::Plugin(
-                errors.into_iter().chain(errs.into_iter()).collect(),
-            )),
         }
     }
 }

rage/src/bin/rage/main.rs

@@ -2,7 +2,9 @@
 
 use age::{
     armor::{ArmoredReader, ArmoredWriter, Format},
-    cli_common::{file_io, read_identities, read_or_generate_passphrase, read_secret, Passphrase},
+    cli_common::{
+        file_io, read_identities, read_or_generate_passphrase, read_secret, Passphrase, UiCallbacks,
+    },
     plugin, Recipient,
 };
 use gumdrop::{Options, ParsingStyle};
@@ -123,6 +125,7 @@ fn read_recipients(
         recipients.push(Box::new(plugin::RecipientPluginV1::new(
             plugin_name,
             &plugin_recipients,
+            UiCallbacks,
         )?))
     }