Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

See if cached PIN signing is possible without requiring a PS/SC transaction #47

Open
ericchiang opened this issue Apr 26, 2020 · 16 comments

Comments

@ericchiang
Copy link
Collaborator

Based off my testing, releasing the transaction wipes the PIN.

@ericchiang ericchiang changed the title See if cached PIN signing without requiring a PS/SC transaction See if cached PIN signing is possible without requiring a PS/SC transaction Apr 26, 2020
@FiloSottile
Copy link

Alternatively, the same high-level use case would be served by having a "weak" lock on the token, by somehow letting other instances take over. This could be a PCSC option, a takeover method, a notification of other pending transactions, or even just a way to find out what process has an open transaction, letting the applications sort it out with signals.

FiloSottile added a commit to FiloSottile/yubikey-agent that referenced this issue Apr 26, 2020
When running yubikey-agent has an exclusive lock on the YubiKey which
applies to both PIV and PGP applets (presumably because they are
mediated by the same daemon) but not FIDO2. This is necessary in order
to keep the PIN cached, see go-piv/piv-go#47.

Handle SIGHUP by releasing that lock. Never was a signal name more apt.

This allows using "killall -HUP yubikey-agent" in a script or alias to
release the applet when needed.

$ ssh -T [email protected]
$ killall -HUP yubikey-agent; pass filippo.io
$ killall gpg-agent # couldn't find a command to make it drop its tx

yubikey-agent will seamlessly make a new transaction and ask for a PIN
the next time a signing operation is requested.
@ericchiang
Copy link
Collaborator Author

#51 now marks the smartcard context as exclusive so other applications that try to access the card will fail instead of hang.

Did a little looking into SCardGetStatusChange to see if we can detect when another application attempts to access the card. That might be a way to implement a "weak" lock.

https://ludovicrousseau.blogspot.com/2018/02/how-to-use-scardgetstatuschange.html

@tv42
Copy link
Contributor

tv42 commented Sep 4, 2020

Would it be possible to release the USB device, as a whole?

I have a Chromebook with multiple LXD containers ("crostini") where I would like to be able to SSH out from different containers using the same yubikey (and use the yubikey for encryption, too, outside of ssh). They're system containers, so each container runs its own systemd, and in this case, pcscd and yubikey-agent. As long as I make one stop using the USB device, another one can start using it; automatically releasing the USB device on idle would make that mostly happen without user action.

@tv42
Copy link
Contributor

tv42 commented Sep 4, 2020

Others working on pretty much the same problem (more in the context of letting go of the USB device than between clients of one pcscd): GoogleChromeLabs/chromeos_smart_card_connector#122

@ericchiang
Copy link
Collaborator Author

Releasing the smart card context can already be done by calling YubiKey.Close(), right? It's probably better for applications to define what "idle" is instead of this package.

https://pkg.go.dev/github.com/go-piv/piv-go/piv?tab=doc#YubiKey.Close

On a less related note, I'd be happy to add an option to this library to use a SCARD_SHARE_SHARED connection instead of SCARD_SHARE_EXCLUSIVE at the cost of cached PINs. But I think that'll only help multiple applications connecting to the same pcscd.

@markomitranic
Copy link

Any update on this front guys? :)

@ericchiang
Copy link
Collaborator Author

If anyone wants to add a client option to opt into SCARD_SHARE_SHARED I'm happy to review.

https://github.com/go-piv/piv-go/blob/v1.8.0/piv/piv.go#L135

@FStelzer
Copy link

as far as i understand even with shared access a new transaction will be needed and a new transaction will need a new pin to unlock. i'm not 100% sure about the pin part but every tool i have seen that gracefully handles concurrent card access is caching the card pin itself (tools like pivy-agent, pagentcac and i think p11-kit/opensc as well). or alternatively people will always have their unlocked keychain enter the pin for them on every operation.

i played around a bit with this in yubikey-agent which i'd like to use as a replacement for pivy-agent but since the pin passes through several variables/functions into piv-go it doesen't really help if i secure it correctly within yubikey-agent since it is scattered in memory anyway.
I think piv-go should allow for a pin caching option and also use something like https://github.com/awnumar/memguard to securely store the pin in memory (even when not caching it!)
I'd be happy to help with this if you are open to it.

@ericchiang
Copy link
Collaborator Author

@FStelzer, @rajnikant12345 started a PR here to allow shared connections: #100

There's an upstream proposal to allow key erasure in the Go runtime that might be interesting to you as well golang/go#21865

I don't think piv-go would take on a dependency like memguard. This package tries to be agnostic about how the PIN is stored and received from the user. If there's anything tweaks to the way we're receiving PINs that would help enable that kind of use case, happy to do that though.

https://pkg.go.dev/github.com/go-piv/piv-go/piv#KeyAuth

@FStelzer
Copy link

fair enough. i saw #100 and are looking forward to it.

i'll find out what could be improved in piv-go for a consumer like yubikey-agent to be able to safely use memguard for pin caching.

@delucca
Copy link

delucca commented Dec 20, 2021

Any updates on this?

@Endareth
Copy link

Endareth commented May 6, 2022

Given this is proving a difficult challenge to get past, any chance of adding an option to either timeout the transaction after a specific period of time (e.g. require PIN again after 15 minutes), or even not keeping the transaction open at all (always require PIN re-entry)?

@cedws
Copy link

cedws commented Oct 17, 2022

Looks like the PR for piv-go is stuck.

Would it make sense to build a broker that could be used by this project and others? Though it would be difficult to get projects to adopt it.

@FiloSottile
Copy link

I observed something unexpected on macOS: the PIN cache persists across not only transactions, but even sessions and processes. FiloSottile/yubikey-agent#4 (comment) If I use yubikey-agent, then fully kill it and restart it, it will not require the PIN (presumably because the card tells piv-go it's fine in ykLoginNeeded).

piv-go/piv/piv.go

Lines 245 to 249 in 1902689

func ykLoginNeeded(tx *scTx) bool {
cmd := apdu{instruction: insVerify, param2: 0x80}
_, err := tx.Transmit(cmd)
return err != nil
}

This suggests at least on macOS it's fine to release whatever is keeping the lock after every operation and just re-acquire it for the next operation.

@FiloSottile
Copy link

Based on some testing, experimenting, and reports, the cross-session PIN caching only works on macOS, for firmware version 5. (I also tested patching out the Serial call, as on firmware 4 that is known to drop the PIN cache by switching applets.)

This is unfortunate, because PIN cache sharing between yubikey-agent and age-plugin-yubikey is the UX we want, and it'd be nice to get it without the complexity of making age talk to yubikey-agent.

I wonder if this can be patched into pcscd at least, as there's no reason it should be macOS-specific.

@gdbinit
Copy link

gdbinit commented Feb 1, 2023

It's not a macOS-specific issue but Yubikey firmware version related. The PIN caching works as expected for firmware 5 and not for 4. Tested this on Linux with age-plugin-yubikey and it works fine with firmware 5 but not with 4.

I was going crazy today while adding a yubikey "plugin" to age code to avoid external plugins and it kept asking for the PIN even if the policy was set to once. I was pretty sure it was working as expected in previous tests until I saw all these issues and most probably my age-plugin-yubikey initial tests were made with firmware 5 and now was developing with a firmware 4. For a command line execute once tool like age I guess there isn't a fancy (secure) solution other than not using firmware 4 keys :-)
Or looking deeper into pcscd which might be a nightmare on macOS because impossible to get Apple to patch their version. But I wonder if CryptoToken framework has these issues (probably since they appear to be firmware 4 related, not just with the serial retrieval resetting the pin, which I can reproduce).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants