Merge pull request #100 from str4d/pin-caching

Enable PIN caching

CHANGELOG.md

@@ -9,6 +9,16 @@ to 0.3.0 are beta releases.
 ## [Unreleased]
 ### Changed
 - MSRV is now 1.60.0.
+- The YubiKey PIV PIN and touch caches are now preserved across processes in
+  most cases. See [README.md](README.md#agent-support) for exceptions. This has
+  several usability effects:
+  - If a YubiKey's PIN is cached by an agent like `yubikey-agent`, and then
+    `age-plugin-yubikey` is run (either directly or as a plugin), the agent
+    won't request a PIN entry on its next use.
+  - If a YubiKey's PIN was requested by either a previous invocation of
+    `age-plugin-yubikey` or an agent like `yubikey-agent`, subsequent calls to
+    `age-plugin-yubikey` won't request a PIN entry to decrypt a file with an
+    identity that has a PIN policy of `once`.
 
 ## [0.3.2] - 2023-01-01
 ### Changed

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",

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" }

README.md

@@ -115,13 +115,24 @@ age client as normal (e.g. `rage -d -i yubikey-identity.txt`).
 ### Agent support
 
 `age-plugin-yubikey` does not provide or interact with an agent for decryption.
-As age plugin binaries have short lifetimes (they only run while the age client
-is running), this means that YubiKey identities configured with a PIN policy of
-`once` will actually prompt for the PIN on every decryption.
+It does however preserve the PIN cache by not soft-resetting the YubiKey after a
+decryption or read-only operation, which enables YubiKey identities configured
+with a PIN policy of `once` to not prompt for the PIN on every decryption.
 
-A decryption agent will most likely be implemented as a separate age plugin that
-interacts with [`yubikey-agent`](https://github.com/FiloSottile/yubikey-agent),
-enabling YubiKeys to be used simultaneously with age and SSH.
+The session that corresponds to the `once` policy can be ended in several ways,
+not all of which are necessarily intuitive:
+
+- Unplugging the YubiKey (the obvious way).
+- Using a different applet (e.g. FIDO2). This causes the PIV applet to be closed
+  which clears its state.
+- Generating a new age identity via `age-plugin-yubikey --generate` or the CLI
+  interface. This is to avoid leaving the YubiKey authenticated with the
+  management key.
+
+If the current PIN UX proves to be insufficient, a decryption agent will most
+likely be implemented as a separate age plugin that interacts with
+[`yubikey-agent`](https://github.com/FiloSottile/yubikey-agent), enabling
+YubiKeys to be used simultaneously with age and SSH.
 
 ### Manual setup and technical details
 

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";
@@ -575,13 +592,13 @@ impl Connection {
                     metadata => metadata,
                 };
         }
-        if let Some(PinPolicy::Never) = self.cached_metadata.as_ref().and_then(|m| m.pin_policy) {
-            return Ok(Ok(()));
+        match self.cached_metadata.as_ref().and_then(|m| m.pin_policy) {
+            Some(PinPolicy::Never) => return Ok(Ok(())),
+            Some(PinPolicy::Once) if self.yubikey.verify_pin(&[]).is_ok() => return Ok(Ok(())),
+            _ => (),
         }
 
         // The policy requires a PIN, so request it.
-        // Note that we can't distinguish between PinPolicy::Once and PinPolicy::Always
-        // because this plugin is ephemeral, so we always request the PIN.
         let enter_pin_msg = fl!(
             "plugin-enter-pin",
             yubikey_serial = self.yubikey.serial().to_string(),
@@ -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)]

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(());
                 }
             }

src/plugin.rs

@@ -249,6 +249,8 @@ impl IdentityPluginV1 for IdentityPlugin {
                     }
                 }
             }
+
+            conn.disconnect_without_reset();
         }
         Ok(file_keys)
     }