Cache PIN in Linux keyring #168
Conversation
|
Pushed another commit which sets the PIN while creating the key and sets the timeout after that. Unfortunately, Btw cc @str4d, opinions? :) |
Ma27
force-pushed
the
linux-pin-cache
branch
2 times, most recently
from
f743ccd
to
5c2d642
Compare
There was a problem hiding this comment.
I'm sympathetic to the UX issue, and I am not entirely against this change, but I would want to see documentation about how the Linux keyring behaves when used in this way (and if that behaviour is not consistent across all Linux OSs, an examination of those differences) before this could be merged.
PinPolicy::Once means slightly different things to different audiences:
- To users, it means "once per time that the YubiKey is plugged in". The UX expectation is that the user plugs in their YubiKey, types in the PIN once, and then they don't need to do so again before they unplug their YubiKey. Unplugging and plugging in again requires another PIN entry.
- To the YubiKey, it means "once per time that the PIV applet is opened on the YubiKey"; switching to the GPG or CTAP applets cause the PIV applet to be closed and the cached PIN dropped.
The discrepancy between these two meanings causes problems for the users' mental model, and I can see the benefit of using a temporary PIN cache to paper over the user's other apps accessing different YubiKey applets (e.g. for SSH authentication or GPG signing). However, we then need a way to ensure that the keyring cache is cleared when the YubiKey unplugs - and that can occur after age-plugin-yubikey has finished running! I don't know what kind of flexibility the Linux keyring offers, but if all we have access to is a timeout, then I don't think this PR is suitable as-is. We'd instead need some way for age-plugin-yubikey to spawn an independent child process that manages that keyring caching, and ensures eviction at the right time.
| if let Ok(ring) = maybe_ring { | ||
| if let Ok(key) = ring.search(&description) { | ||
| if let Ok(cached_pin) = key.read_to_vec() { | ||
| if self.yubikey.verify_pin(&cached_pin).is_ok() { |
There was a problem hiding this comment.
Any PIN caching MUST respect the configured policy for the slot. If the slot uses PinPolicy::Always, then we must request the PIN directly from the user, and never cache it. What that means in practice is that the only time this cache should be used is PinPolicy::Once.
| ring.add_key(&description, &pin.expose_secret().as_bytes()) | ||
| .map_err(|e| format!("add_key({description}, pin): {e}")) | ||
| .and_then(|key| { | ||
| key.set_timeout(300).map_err(|e| format!("set_timeout(300): {e}"))?; |
There was a problem hiding this comment.
- How reliable is this timeout?
- Given that you are not setting a timeout at the point where the PIN is added to the keyring (I presume due to limitations with the
linux-keyutilsAPI), this means that if the keyring addition succeeds and then the timeout-setting fails, the PIN is now (IIUC) permanently stored in the keyring (which is not acceptable behaviour).
|
Hi! FIrst of all, thanks a lot for your input!
👍 will be added to the list above. Not sure if you've seen the back-and-forth here, after the last iteration I'm not sure if UserSession or Session is what I want, so more research will be necessary anyways.
This is a good point. One thing I did so far was to invalidate the keyring when I lock my machine and I wanted to see if I can do it reasonably with udev as well. Not sure about the capabilities of the keyring itself, but perhaps some kind of udev rule that will be installed with this package may be an option as well? I'm using a distro-provided package though, not sure how this behaves with
Agreed 👍
@lheckemann heh, and I were discussing exactly that! I reversed that however after finding out that you'll lose the timeout when |
Co-authored-by: Linus Heckemann <[email protected]>
In fact our little trick with writing garbage into the keyring and setting the timeout afterwards doesn't seem to work. I noticed rather late because I'm running this change with a higher timeout locally. This is because the `key_update` in Linux resets `expiry` in this case[1]. Originally I also tried to write into `@us` because that's shared between `systemctl --user` and my Sway session, but it seems as if every key created in there has the systemd user manager as possessor which means that I can't do anything from the Sway session. This doesn't seem to be changeable because newly created keys always have 0x3f010000 as mask[2]. [1] https://github.com/torvalds/linux/blob/v6.8/security/keys/key.c#L1083 [2] https://github.com/torvalds/linux/blob/v6.8/security/keys/key.c#L905-L915
Ma27
force-pushed
the
linux-pin-cache
branch
from
5c2d642
to
f125261
Compare
Unresolved questions / TODO:
PinPolicy::Onceaarch64-darwin).I'm happy to update the docs accordingly when we agreed on whether this is desirable.
cc @lheckemann