agherzan / yubikey-full-disk-encryption

Use YubiKey to unlock a LUKS partition
Apache License 2.0
795 stars 50 forks source link

Add support for OnlyKey HMAC SHA1 #56

Closed onlykey closed 3 years ago

onlykey commented 4 years ago

Hi,

We recently implemented HMAC SHA1 in OnlyKey, it is fully compatible with Yubikey's HMAC SHA1 challenge and response. We have integrated support with KeePassXC https://github.com/keepassxreboot/keepassxc/pull/3352 and are looking to integrate with other projects as well. The only change required is to allow OnlyKey's USB VID/PID to be used in addition to the already allowed Yubikey USB VID/PIDs. Would you be willing to add support for OnlyKey?

Thanks!

agherzan commented 4 years ago

I don't have a problem with that. The only issue is that someone will need to maintain that support. TIL about OnlyKey so I don't have any hardware.

onlykey commented 4 years ago

@agherzan We can support any issues that come up they can be posted here - https://github.com/trustcrypto/OnlyKey-Firmware

I don't expect there would be too many issues though as HMAC SHA1 feature should work the same whether an OnlyKey or Yubikey is used.

We can also send you hardware to test, send email to t@crp.to with mailing address.

Vincent43 commented 4 years ago

@onlykey We don't use libykpers library. We provide only shell script that calls:

  1. ykinfo -<slot_number>
  2. ykchalresp -<slot_number> <challenge>

Will it work out-of-the-box with onlykey? If it's not possible to work with onlykey from those commands then there is nothing we can do about it.

Aidan-Chelig commented 3 years ago

I actually bought my OnlyKey to do this but i didn't research and was under the impression i simply needed a u2f key. If i was going to go about adding this feature would you recommend a firmware change, a modified ykchalresp binary or some other solution?

Vincent43 commented 3 years ago

I think you need modified ykinfo and ykchalresp binaries. If your key supports FIDO2 standard then you may consider trying if https://github.com/mjec/khefin works for you as alternative solution.

Vincent43 commented 3 years ago

Closing this as it's not actionable here. This needs feature request in https://github.com/Yubico/yubikey-personalization first. If things change feel free to re-open.

onlykey commented 3 years ago

@Vincent43 We did add the feature request in yubikey-personalization. ykpers 1.20 adds the feature where you can open any vendor/product key by calling yk_open_key_vid_pid instead of yk_open_first_key(). Is it possible for you to use that?

EDIT: Just reread this issue, since this is just a script that calls ykchalresp binaries it would be the binaries that would have to change to support OnlyKey. This is not difficult to do if anyone is interested, its just adding OnlyKey USB VID/PID here: https://github.com/Yubico/yubikey-personalization/blob/d4c1d57033a45ea51189e7818954cf62508576de/ykcore/ykcore.c#L108 To support both Yubikey and OnlyKey there can be a check first to see if there are any Yubikeys to open and if not open OnlyKey that is what was done here - https://github.com/keepassxreboot/keepassxc/pull/3352

Vincent43 commented 3 years ago

If/When ykinfo/ykchalresp gets support for OnlyKey we can look at supporting it here (this may need help testing from someone who actually has this key). I don't know if there are still people who are interested in OnlyKey support in this project so this is your call if pushing another feature request to yubikey-personalization is worth your time.

agherzan commented 3 years ago

I totally agree with @Vincent43 . We need someone to maintain this support. Someone who uses onlykey as a daily driver.

Vincent43 commented 3 years ago

Actually adding support for OnlyKey (when possible) may be one time change. I just meant we need someone to confirm it works at time we add it so we won't fool ourself.

Ideally it would be implemented in yubikey-personalization in a way to be fully transparent for apps like ours so when we call ykchalresp -2 <challenge> it will automatically use OnlyKey when this is what's connected to system. In such case we won't need to do anything and users will just need to wait for yubikey-personalization update. I think this should be the goal for new yubikey-personalization feature request.

onlykey commented 3 years ago

@Vincent43 Yubico is very likely not going to implement support for other security keys in their own binaries. This would be the kind of thing where a custome ykchalresp binary would have to be compiled for OnlyKey support (simple change) and that custom binary be used.

Vincent43 commented 3 years ago

I thought they are open for leveraging their tools for other keys if they accepted supporting them in library. If it's not the case then indeed users are required to get custom patched binary that will work out of the box with this project so for us the case is closed.

My1 commented 3 years ago

honestly kinda sad to be entirely honest. I mean Yubico kinda added support for third party tokens by supplying extra IDs in their lib

Vincent43 commented 3 years ago

Adding support in libs doesn't help for projects that can't use them. Having third party keys support in libs but not in cli tools within the same project is certainly inconsistent.

My1 commented 3 years ago

wait seriously? kinda crazy, lol

ccchan234 commented 3 years ago

so sad, after reading this it seems EVEN onlykey are warm-hearted to really provide an open source, more powerful/functional USB security key, yubico is the longer time player here, and others may or may not join the free ride of yubico. sometimes it's possible, sometimes it's not.