From 9418921dab67f7ba01f6ffce2a10f5e3ca1c4557 Mon Sep 17 00:00:00 2001
From: Jack Grigg <thestr4d@gmail.com>
Date: Mon, 2 Jan 2023 16:11:38 +0000
Subject: [PATCH] Disconnect without resetting YubiKeys if it is safe to do so

This enables the PIN caches to be preserved across age-plugin-yubikey
processes, allowing PIN policies of "once" to become meaningful.
---
 Cargo.lock    |  3 +--
 Cargo.toml    |  3 +++
 src/key.rs    | 24 ++++++++++++++++++++++++
 src/main.rs   | 20 ++++++++++++++++++--
 src/plugin.rs |  2 ++
 5 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 0f4d257..093a517 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2122,8 +2122,7 @@ dependencies = [
 [[package]]
 name = "yubikey"
 version = "0.7.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "10e6fa9476951a9b93d9a31aa5554b5bbac7aafdc5b23e663eb3f9b635c86053"
+source = "git+https://github.com/iqlusioninc/yubikey.rs.git?rev=1d33ea174791f699dfc0e6a06648aa3d9e066144#1d33ea174791f699dfc0e6a06648aa3d9e066144"
 dependencies = [
  "base16ct",
  "chrono",
diff --git a/Cargo.toml b/Cargo.toml
index dbc0943..7a67735 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -53,3 +53,6 @@ sysinfo = "0.27"
 [dev-dependencies]
 flate2 = "1"
 man = "0.3"
+
+[patch.crates-io]
+yubikey = { git = "https://github.com/iqlusioninc/yubikey.rs.git", rev = "1d33ea174791f699dfc0e6a06648aa3d9e066144" }
diff --git a/src/key.rs b/src/key.rs
index eeba777..32636eb 100644
--- a/src/key.rs
+++ b/src/key.rs
@@ -236,6 +236,23 @@ pub(crate) fn open(serial: Option<Serial>) -> Result<YubiKey, Error> {
     Ok(yubikey)
 }
 
+/// Disconnect from the YubiKey without resetting it.
+///
+/// This can be used to preserve the YubiKey's PIN and touch caches. There are two cases
+/// where we want to do this:
+///
+/// - We connected to this YubiKey in a read-only context, so we have not made any changes
+///   to the YubiKey's state. However, we might have asked an agent to release the YubiKey
+///   in `key::open_connection`, and we want to allow any state it may have left behind
+///   (such as cached PINs or touches) to persist beyond our execution, for usability.
+/// - We opened this connection in a decryption context, so the only changes to the
+///   YubiKey's state were to potentially cache the PIN and/or touch (depending on the
+///   policies of the slot). We want to allow these to persist beyond our execution, for
+///   usability.
+pub(crate) fn disconnect_without_reset(yubikey: YubiKey) {
+    let _ = yubikey.disconnect(pcsc::Disposition::LeaveCard);
+}
+
 pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
     const DEFAULT_PIN: &str = "123456";
     const DEFAULT_PUK: &str = "12345678";
@@ -674,6 +691,13 @@ impl Connection {
             Err(_) => Err(()),
         }
     }
+
+    /// Close this connection without resetting the YubiKey.
+    ///
+    /// This can be used to preserve the YubiKey's PIN and touch caches.
+    pub(crate) fn disconnect_without_reset(self) {
+        disconnect_without_reset(self.yubikey);
+    }
 }
 
 #[cfg(test)]
diff --git a/src/main.rs b/src/main.rs
index 0bf07bd..f6cd93d 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -181,6 +181,13 @@ fn generate(flags: PluginFlags) -> Result<(), Error> {
 
     util::print_identity(stub, recipient, metadata);
 
+    // We have written to the YubiKey, which means we've authenticated with the management
+    // key. Out of an abundance of caution, we let the YubiKey be reset on disconnect,
+    // which will clear its PIN and touch caches. This has as small negative UX effect,
+    // but identity generation is a relatively infrequent occurrence, and users are more
+    // likely to see their cached PINs reset due to switching applets (e.g. from PIV to
+    // FIDO2).
+
     Ok(())
 }
 
@@ -200,6 +207,8 @@ fn print_single(
 
     printer(stub, recipient, metadata);
 
+    key::disconnect_without_reset(yubikey);
+
     Ok(())
 }
 
@@ -233,6 +242,8 @@ fn print_multiple(
             println!();
         }
         println!();
+
+        key::disconnect_without_reset(yubikey);
     }
     if printed > 1 {
         eprintln!("{}", fl!("printed-multiple", kind = kind, count = printed));
@@ -360,11 +371,13 @@ fn main() -> Result<(), Error> {
             .iter()
             .map(|reader| {
                 key::open_connection(reader).map(|yk| {
-                    fl!(
+                    let name = fl!(
                         "cli-setup-yk-name",
                         yubikey_name = reader.name(),
                         yubikey_serial = yk.serial().to_string(),
-                    )
+                    );
+                    key::disconnect_without_reset(yk);
+                    name
                 })
             })
             .collect::<Result<Vec<_>, _>>()?;
@@ -457,8 +470,10 @@ fn main() -> Result<(), Error> {
                         util::Metadata::extract(&mut yubikey, slot, key.certificate(), true)
                             .unwrap();
 
+                    key::disconnect_without_reset(yubikey);
                     ((stub, recipient, metadata), false)
                 } else {
+                    key::disconnect_without_reset(yubikey);
                     return Ok(());
                 }
             } else {
@@ -540,6 +555,7 @@ fn main() -> Result<(), Error> {
                         true,
                     )
                 } else {
+                    key::disconnect_without_reset(yubikey);
                     return Ok(());
                 }
             }
diff --git a/src/plugin.rs b/src/plugin.rs
index e29edf4..c0cd18b 100644
--- a/src/plugin.rs
+++ b/src/plugin.rs
@@ -249,6 +249,8 @@ impl IdentityPluginV1 for IdentityPlugin {
                     }
                 }
             }
+
+            conn.disconnect_without_reset();
         }
         Ok(file_keys)
     }