Yubico / yubikey-personalization

YubiKey Personalization cross-platform library and tool
https://developers.yubico.com/yubikey-personalization/
BSD 2-Clause "Simplified" License
300 stars 82 forks source link

Signify that yubikey is waiting for a HMAC touch #150

Closed cyrinux closed 4 years ago

cyrinux commented 4 years ago

I was chatting with @maximbaz about trying to implement HMAC support for yubikey-touch-detector, to fix this issue https://github.com/maximbaz/yubikey-touch-detector/issues/6. We search around the other possibility to implement this, and we realized that the best solution would be to implement support for this in yubikey-personalization, just like it was already implemented for pam-u2f - see this commit https://github.com/Yubico/pam-u2f/commit/0cdc53737d77f76abc37344e704e60f582e13761 .

To follow the exact same approach, I would like that libykpers get patched to open a file on yubikey waiting for a touch and get the file close after yubikey get touched.

The default value of the file, to do like pam-u2f would be /var/run/user/$UID/hmac-authpending

What do you think about?

Would you like to implement this yourself, or would you accept a such PR ?

Regards

klali commented 4 years ago

Where would you intend this to go?

I'm unsure if it's a good idea to add it to the library.

cyrinux commented 4 years ago

Hi @klali ,

For example, in my case HMAC is used for keepass, with https://github.com/brush701/keechallenge plugin. That one required libykpers. I'm not familar with the code yet, I was hoping you would be able to give me some hints where to add this. :)

maximbaz commented 4 years ago

@klali all we want is to be able to detect when yubikey starts and stops waiting for a touch, and of course the knowledge of these actions lies within libraries such as this one and pam-u2f; in case of U2F it was extremely helpful that we could merge such an signal directly in the library and rely on it in yubikey-touch-detector tool, but of course other suggestions how we could achieve the same are very welcome!

Touching a file was simply the smallest change we could come up with, and it is enough for us to detect when the file is touched.

klali commented 4 years ago

Yes.. I think it would be tricky to add in the library in a portable way that makes sense (quite unexpected to have your library create files for you). It's already possible to call the library in a way where it'll tell you that it will block if you call it and allow it to block (look at the ykchalresp binary with and without -N). Maybe this should be the advertised way of how to call this to allow the caller more information about what's happening.

maximbaz commented 4 years ago

Maybe this should be the advertised way of how to call this to allow the caller more information about what's happening.

This won't realistically help us as we would then need to request every single application that uses the library to expose the info about what is happening to a tool such as yubikey-touch-detector. Having such an indicator produced by the library on contrast would allow us to support each and every scenario right away 🙂

I think it would be tricky to add in the library in a portable way that makes sense (quite unexpected to have your library create files for you).

What do you think of the approach that was made in pam-u2f? It's not like we are creating a ton of big files that could harm users, just a single empty file in a runtime directory that no user will notice, but it will help a lot to solve a real problem 🙂

cyrinux commented 4 years ago

Yes.. I think it would be tricky to add in the library in a portable way that makes sense (quite unexpected to have your library create files for you). It's already possible to call the library in a way where it'll tell you that it will block if you call it and allow it to block (look at the ykchalresp binary with and without -N). Maybe this should be the advertised way of how to call this to allow the caller more information about what's happening.

About the -N flag, i got this issue https://github.com/Yubico/yubikey-personalization/issues/129

klali commented 4 years ago

I think the approach in pam-u2f is ok for that project, even though pam builds libraries it is more of an application that can make assumptions and know things about it's environment. I don't like the idea of a library opening files like that when not asked to. The equivalent so to say would be to add this to pam-yubico, but as you state that wouldn't solve it for other applications that link with libykpers.

maximbaz commented 4 years ago

Do you perhaps have any other suggestions how libykpers could signify when yubikey starts and stops waiting for a touch? As said we aren't really married to that specific approach of opening files, the only ask is to do this in a way that would support this for all applications that link with this library.

We also tried to analyze different system events that happen while yubikey is waiting for a touch (such as monitoring udev events), but couldn't easily identify anything, that's why decided to open this issue and ask for some ideas 🙂

maximbaz commented 4 years ago

This is no longer needed and can be closed, we figured out a way to detect when YubiKey is waiting for a touch because of HMAC operation: when YubiKey begins to wait for a touch, one of the /dev/hidraw* devices disappears, and when it stops waiting for a touch it reappears.