keyfork mnemonic generate: feedback improvements

* Touch policy is now set to `on` by default (not fixed, as that's
  irreversible).
* The value passed to `--encrypt-to-self` is the actual encrypted
  output.
* The `cert_output` passed to `--encrypt-to-self` by default is the
  fingerprint of the certificate.
* The OpenPGP provisioner can now be used without identifiers, if the
  correct amount of smartcards are actively plugged into the current
  system.
* The OpenPGP provisioner, when run without `--encrypt-to-self`, will
  output the OpenPGP certificate for the smartcard.

crates/keyfork/src/cli/mnemonic.rs

@@ -87,6 +87,7 @@ impl From<&SeedSize> for usize {
         }
     }
 }
+
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum MnemonicSeedSourceParseError {
     #[error("Expected one of system, playing, tarot, dice")]
@@ -143,6 +144,18 @@ impl MnemonicSeedSource {
     }
 }
 
+/// An error occurred while performing an operation.
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+    /// An error occurred when interacting iwth a file.
+    #[error("Error while performing IO operation on: {1}")]
+    IOContext(#[source] std::io::Error, PathBuf),
+}
+
+fn context_stub<'a>(path: &'a Path) -> impl Fn(std::io::Error) -> Error + 'a {
+    |e| Error::IOContext(e, path.to_path_buf())
+}
+
 #[derive(Subcommand, Clone, Debug)]
 pub enum MnemonicSubcommands {
     /// Generate a mnemonic using a given entropy source.
@@ -206,12 +219,12 @@ pub enum MnemonicSubcommands {
         /// `--provision openpgp-card` or another relevant provisioner, to ensure the newly
         /// generated mnemonic would be decryptable by some form of provisioned hardware.
         ///
-        /// When given arguments in the format `--encrypt-to-self output.asc,output=encrypted.asc`,
+        /// When given arguments in the format `--encrypt-to-self encrypted.asc,output=cert.asc`,
-        /// the output of the OpenPGP certificate will be written to `output.asc`, while the output
+        /// the output of the OpenPGP certificate will be written to `cert.asc`, while the output
         /// of the encryption will be written to `encrypted.asc`. Otherwise, the
-        /// default behavior is to write the encrypted mnemonic to `output.enc.asc`. If either
+        /// default behavior is to write the certificate to a file named after the certificate's
-        /// output file already exists, it will not be overwritten, and the command will exit
+        /// fingerprint. If either output file already exists, it will not be overwritten, and the
-        /// unsuccessfully.
+        /// command will exit unsuccessfully.
         ///
         /// Additionally, when given the `account=` option (which must match the `account=` option
         /// of the relevant provisioner), the given account will be used instead of the default
@@ -296,7 +309,7 @@ fn do_encrypt_to(
     literal_message.write_all(b"\n")?;
     literal_message.finalize()?;
 
-    let mut file = File::create_new(&output_file)?;
+    let mut file = File::create_new(&output_file).map_err(context_stub(&output_file))?;
     if is_armored {
         let mut writer = Writer::new(file, Kind::Message)?;
         writer.write_all(&output)?;
@@ -313,11 +326,6 @@ fn do_encrypt_to_self(
     path: &Path,
     options: &StringMap,
 ) -> Result<(), Box<dyn std::error::Error>> {
-    let output_file = determine_valid_output_path(path, "enc", options.get("output"));
-
-    let is_armored =
-        options.get("armor").is_some_and(|a| a == "true") || is_extension_armored(&output_file);
-
     let account = options
         .get("account")
         .map(|account| u32::from_str(account))
@@ -350,23 +358,28 @@ fn do_encrypt_to_self(
         &userid.unwrap_or(UserID::from("Keyfork-Generated Key")),
     )?;
 
-    let mut file = File::create_new(path)?;
+    let cert_path = match options.get("output") {
-    if is_armored {
+        Some(path) => PathBuf::from(path),
+        None => {
+            let path = PathBuf::from(cert.fingerprint().to_string()).with_extension("asc");
+            eprintln!(
+                "Writing OpenPGP certificate to default path: {path}",
+                path = path.display()
+            );
+            path
+        }
+    };
+
+    let file = File::create_new(&cert_path).map_err(context_stub(&cert_path))?;
     let mut writer = Writer::new(file, Kind::PublicKey)?;
     cert.serialize(&mut writer)?;
     writer.finalize()?;
-    } else {
-        cert.serialize(&mut file)?;
-    }
 
     // a sneaky bit of DRY
     do_encrypt_to(
         mnemonic,
-        path,
+        &cert_path,
-        &StringMap::from([(
+        &StringMap::from([(String::from("output"), path.to_string_lossy().to_string())]),
-            String::from("output"),
-            output_file.to_string_lossy().to_string(),
-        )]),
     )?;
 
     Ok(())
@@ -421,7 +434,7 @@ fn do_shard(
     let mut output = vec![];
     openpgp.shard_and_encrypt(threshold, max, mnemonic.as_bytes(), &certs[..], &mut output)?;
 
-    let mut file = File::create_new(&output_file)?;
+    let mut file = File::create_new(&output_file).map_err(context_stub(&output_file))?;
     if is_armored {
         file.write_all(&output)?;
     } else {
@@ -466,7 +479,7 @@ fn do_shard_to(
         &mut output,
     )?;
 
-    let mut file = File::create_new(&output_file)?;
+    let mut file = File::create_new(&output_file).map_err(context_stub(&output_file))?;
     if is_armored {
         file.write_all(&output)?;
     } else {
@@ -482,10 +495,6 @@ fn do_shard_to(
     Ok(())
 }
 
-#[derive(thiserror::Error, Debug)]
-#[error("missing key: {0}")]
-struct MissingKey(&'static str);
-
 fn do_provision(
     mnemonic: &keyfork_mnemonic::Mnemonic,
     provisioner: &provision::Provisioner,
@@ -499,16 +508,27 @@ fn do_provision(
         .unwrap_or(0);
     let identifier = options
         .remove("identifier")
-        .ok_or(MissingKey("identifier"))?
+        .map(|s| s.split('.').map(String::from).collect::<Vec<_>>())
-        .split(',')
+        .map(Result::<_, Box<dyn std::error::Error>>::Ok)
-        .map(String::from)
+        .unwrap_or_else(|| {
-        .collect::<Vec<_>>();
+            Ok(provisioner
+                .discover()?
+                .into_iter()
+                .map(|(identifier, _)| identifier)
+                .collect())
+        })?;
     let count = options
         .remove("count")
         .map(|count| usize::from_str(&count))
         .transpose()?
         .unwrap_or(identifier.len());
 
+    assert_eq!(
+        count,
+        identifier.len(),
+        "amount of identifiers discovered or provided did not match provisioner count"
+    );
+
     for (_, identifier) in (0..count).zip(identifier.into_iter()) {
         let provisioner_config = config::Provisioner {
             account,
@@ -562,7 +582,20 @@ impl MnemonicSubcommands {
                 }
 
                 if let Some(provisioner) = provision {
-                    do_provision(&mnemonic, &provisioner.inner, &provisioner.values)?;
+                    // NOTE: If we have encrypt_to_self, we likely also have the certificate
+                    // already generated. Therefore, we can skip generating it in the provisioner.
+                    // However, if we don't have encrypt_to_self, we might not have the
+                    // certificate, therefore the provisioner - by default - generates the public
+                    // key output.
+                    //
+                    // We use the atypical `_skip_cert_output` field here to denote an automatic
+                    // marking to skip the cert output. However, the `cert_output` field will take
+                    // priority, since it can only be manually set by the user.
+                    let mut values = provisioner.values.clone();
+                    if encrypt_to_self.is_some() {
+                        values.insert(String::from("_skip_cert_output"), String::from("1"));
+                    }
+                    do_provision(&mnemonic, &provisioner.inner, &values)?;
                 }
 
                 if let Some(shard_to) = shard_to {

crates/keyfork/src/cli/provision/openpgp.rs

@@ -3,7 +3,12 @@ use crate::{config, openpgp_card::factory_reset_current_card};
 
 use card_backend_pcsc::PcscBackend;
 use keyfork_derive_openpgp::{
-    openpgp::{packet::UserID, types::KeyFlags},
+    openpgp::{
+        armor::{Kind, Writer},
+        packet::UserID,
+        serialize::Serialize,
+        types::KeyFlags,
+    },
     XPrv,
 };
 use keyfork_prompt::{
@@ -11,6 +16,7 @@ use keyfork_prompt::{
     validators::{SecurePinValidator, Validator},
 };
 use openpgp_card_sequoia::{state::Open, Card};
+use std::path::PathBuf;
 
 #[derive(Clone, Debug)]
 pub struct OpenPGPCard;
@@ -70,11 +76,6 @@ impl ProvisionExec for OpenPGPCard {
             &admin_pin_validator,
         )?;
 
-        let mut has_provisioned = false;
-
-        for backend in PcscBackend::cards(None)? {
-            let backend = backend?;
-
         let subkeys = vec![
             KeyFlags::empty().set_certification(),
             KeyFlags::empty().set_signing(),
@@ -90,8 +91,42 @@ impl ProvisionExec for OpenPGPCard {
         let userid = UserID::from("Keyfork-Provisioned Key");
         let cert = keyfork_derive_openpgp::derive(xprv.clone(), &subkeys, &userid)?;
 
+        // cert_output is never automatically set, but _skip_cert_output is, so we bypass the
+        // automatically generated _skip_cert_output
+        if !provisioner
+            .metadata
+            .as_ref()
+            .is_some_and(|m| m.contains_key("_skip_cert_output") && !m.contains_key("cert_output"))
+        {
+            let cert_output = match provisioner
+                .metadata
+                .as_ref()
+                .and_then(|m| m.get("cert_output"))
+            {
+                Some(cert_output) => PathBuf::from(cert_output),
+                None => {
+                    let path = PathBuf::from(cert.fingerprint().to_string()).with_extension("asc");
+                    eprintln!(
+                        "Writing OpenPGP certificate to: {path}",
+                        path = path.display()
+                    );
+                    path
+                }
+            };
+
+            let cert_output_file = std::fs::File::create_new(cert_output)?;
+            let mut writer = Writer::new(cert_output_file, Kind::PublicKey)?;
+            cert.serialize(&mut writer)?;
+            writer.finalize()?;
+        }
+
+        let mut has_provisioned = false;
+
+        for backend in PcscBackend::cards(None)? {
+            let backend = backend?;
+
             let result = factory_reset_current_card(
-                &mut |identifier| { identifier == provisioner.identifier },
+                &mut |identifier| identifier == provisioner.identifier,
                 user_pin.trim(),
                 admin_pin.trim(),
                 &cert,

crates/keyfork/src/openpgp_card.rs

@@ -1,5 +1,5 @@
 use card_backend_pcsc::PcscBackend;
-use openpgp_card_sequoia::{state::Open, types::KeyType, Card};
+use openpgp_card_sequoia::{state::Open, types::KeyType, Card, types::TouchPolicy};
 use keyfork_derive_openpgp::openpgp::{Cert, policy::Policy};
 
 /// Factory reset the current card so long as it does not match the last-used backend.
@@ -42,8 +42,11 @@ pub fn factory_reset_current_card(
     transaction.factory_reset()?;
     let mut admin = transaction.to_admin_card("12345678")?;
     admin.upload_key(signing_key, KeyType::Signing, None)?;
+    admin.set_touch_policy(KeyType::Signing, TouchPolicy::On)?;
     admin.upload_key(decryption_key, KeyType::Decryption, None)?;
+    admin.set_touch_policy(KeyType::Decryption, TouchPolicy::On)?;
     admin.upload_key(authentication_key, KeyType::Authentication, None)?;
+    admin.set_touch_policy(KeyType::Authentication, TouchPolicy::On)?;
     transaction.change_user_pin("123456", user_pin)?;
     transaction.change_admin_pin("12345678", admin_pin)?;
     Ok(true)