-
-
Notifications
You must be signed in to change notification settings - Fork 125
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
Generate two keys with different touch policies #22
Comments
I have a proof-of-concept of the second no-touch key in my fork. I implemented both key-generation and support for the ssh-agent. I don't mind if someone wants to take this idea and make a polished pull request. Feedback welcome.
|
@FiloSottile I know you're hesitant about adding more complexity to the setup but what do you think about the agent-side changes? Would it be ok to iterate all connected Yubikeys and all slots and return them all? That way the provisioning stays external but an unmodified agent could be used like this. The PIN policies could be listed in the comment on the agent |
It would be great to have that option. When running operations that touch ssh/git (composer, bundle, ...) multiple times it slows everything as you have to confirm each and every ssh connection. It's also very easy to mis-touch and input the 2FA string instead. And as @joneskoo mentioned, auto-fetching tools like lazygit, GH Desktop, ... are much less useful. |
If I try as-is without resetting the key, it fails with I'll have to test with another key as I'd rather not wipe this one and I don't have another one on hand here. |
@hlidotbe I tried to find an error and I can't see any way you'd get that error from my change. Maybe I'm missing something? Or, perhaps you have an older Yubikey and it started failing because of update of go-piv? Can you confirm that master works for you? I suspect you maybe hitting go-piv/piv-go#55 although I thought that was already fixed. In any case I noticed I hadn't pushed all code before, now it should work. But testing it now I notice there's possibly 300ms performance penalty from getting all slots which we can avoid by caching the signers. Perhaps we need to do something to not slow down the happy path. @FiloSottile would appreciate your comment, is it welcome to load certificates from all slots (or at least one extra slot for no-touch), and can we cache the signers (destroying cache when transaction closed)? It may affect setup, otherwise I don't see harm from caching. |
@hlidotbe Ok I was wrong. I suspect the error you see is because you haven't generated certificate on the other slot. I changed my PR to now load all 4 slots and cache the signers. Can you try again? And you can use https://github.com/joneskoo/yubikey-agent/blob/master/contrib/add-second-key/main.go to generate a key on the other slot; should not affect the first slot but it hasn't been tested at all - so assume it will destroy the PIV content. (It shouldn't.) |
@joneskoo second key generated correctly without issue. I did a quick test and it seems to work well. I'll run some more testing but so far so good, thanks! |
I very much want to +1 this. Doing anything I'd hope this would be accepted! |
I would also like to mention the same |
Could this use-case be solved by the setting touch policy to "cache for 15 seconds"? |
Honestly I don't want to enable touch confirm at all. As long as I have the key it should work, its not like its a biometric scanner. |
I opened a new cleaner PR looking for feedback. |
I'd like to help out with this. While #57 is a good proof of concept, I think the approach should be revised. One point is that we should definitely support (and even favor) using the 20 retired slots, which are pretty much free for arbitrary use, rather than (ab)use the 4 standard slots for this. Maybe it's also a good idea to filter certs based on the organization field, like age-plugin-yubikey does. This brings us to the performance concern, because 24 slots is going to slow things down a lot more than 4. The proper solution in my opinion is to listen for hotplug with SCardGetStatusChange (which will most likely require support in go-piv). This would allow enumerating and caching the certs on a key when it's plugged in/inserted, and at that point support for multiple yubikeys almost comes for free. Lastly for the setup script, my preference would be a simple wrapper over ykman's python API. I've already used the ykman CLI to generate P384 keys (questionable decision, but it works), but more importantly so that I could use an AES management key. |
It should be possible to support two different keys with different touch policy on Yubikey. This would allow having some more sensitive services require a touch, but also enable other services to still use the hardware key but no touch required, only PIN per session.
Primary use case for me now is Github; I have auto-fetch on my editor and it keeps prompting for touch. It is making way too many (unidentified, #8 ) prompts for touch. If I could use a second slot on the same key for no-touch use cases, it would not be a problem, as long as SSH doesn't trigger signing and therefore touch for the passive other key with touch-required.
The text was updated successfully, but these errors were encountered: