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

Use ykman instead of yubikey-personalization #101

Open
EliasHolzmann opened this issue Jul 16, 2023 · 12 comments
Open

Use ykman instead of yubikey-personalization #101

EliasHolzmann opened this issue Jul 16, 2023 · 12 comments

Comments

@EliasHolzmann
Copy link

Currently, this project uses the YubiKey Personalization Tool which is no longer under active development. It would be nice for yubikey-full-disk-encryption to instead use YubiKey Manager which has more features. I would be especially interested in this because the YubiKey Personalization Tool does not seem to support NFC, which I need.

Would you be willing to merge a pull request that implements this? I'm currently a bit busy, but I might be able to get this done in a few weeks.

@agherzan
Copy link
Owner

Yes, that is something we can/should do. ykman provides the top calculate/chalresp that could be used similarly. The only issue is that calculate takes a hex argument, so we will need to do the conversion (od or something). I would also vote for maintaining backward compatibility especially since everything works as expected with yubikey-personalization as of now.

@EliasHolzmann
Copy link
Author

The only issue is that calculate takes a hex argument, so we will need to do the conversion (od or something).

I'm probably being a bit obtuse right now, but didn't ykchalresp require the challenge in hex as well, and we get it in hex from sha256sum anyway? Replacing printf %s "$YKFDE_CHALLENGE" | ykchalresp -"$YKFDE_CHALLENGE_SLOT" -i- with ykman otp calculate 2 "$(printf %s "$YKFDE_CHALLENGE")" seems to work correctly for me at least (though I couldn't verify that both results are the same as I don't own a security token with USB).

I would also vote for maintaining backward compatibility especially since everything works as expected with yubikey-personalization as of now.

Agreed, that would be nice. Although that begs the question: Which of the two yubikey packages should be a dependency, and which should be an optional dependency? Or should we specify both as optdepends?

@Vincent43
Copy link
Collaborator

Vincent43 commented Jul 18, 2023

I'm probably being a bit obtuse right now, but didn't ykchalresp require the challenge in hex as well, and we get it in hex from sha256sum anyway?

No, ykchalresp can take anything up to 64 byte length, it doesn't need to be hex. I used sha256 only because this way it's always a valid challenge while user typed one longer than 64 byte won't be valid.

In my testing the responses for the same challenge are different though which makes this a no-go as it breaks all current users:

❯ ykman otp calculate 2 61e748be34d998d8c5a13db63c2b50c4f129d2828a46bae182e6ddec5ff1ff8a
Touch your YubiKey...
a47aa802aedcf28c6f0391d74028da887f006e8d
❯ ykchalresp -2 61e748be34d998d8c5a13db63c2b50c4f129d2828a46bae182e6ddec5ff1ff8a
3613fa5ed3f61a3f187d9db0fbdfc5012af5e2b9

@Vincent43
Copy link
Collaborator

BTW: there is https://github.com/Frederick888/ykchalresp-nfc which we support to some degree.

@agherzan
Copy link
Owner

That output looks strange. What could produce that difference?

@Vincent43
Copy link
Collaborator

Vincent43 commented Jul 20, 2023

I found the fact ykman encodes challenge as hex is the answer. If I call ykchalresp in hex mode then the output matches:

❯ ykchalresp -2 -x 61e748be34d998d8c5a13db63c2b50c4f129d2828a46bae182e6ddec5ff1ff8a
a47aa802aedcf28c6f0391d74028da887f006e8d

In default mode those tools aren't compatible with each other. I think backward compatibility is crucial so ykman support could be only introduced as opt-in.

@agherzan
Copy link
Owner

I agree - and thanks for looking into it. We would need to make it backwards compatible.

@Vincent43
Copy link
Collaborator

Note that in Arch ykman (yubikey-manager) package depends on yubikey-personalization so all users always need them both installed.

@agherzan
Copy link
Owner

I don't see why that dependency is defined as such. These specific tools don't have a dependency per se.

@EliasHolzmann
Copy link
Author

I don't see why that dependency is defined as such. These specific tools don't have a dependency per se.

I've looked into that – it was needed until a while ago, now the dependency is unused and will probably be removed sooner or later: https://bugs.archlinux.org/task/73290

@NgoHuy
Copy link

NgoHuy commented Dec 8, 2023

I added commit #103 please test it.

@NgoHuy
Copy link

NgoHuy commented Dec 8, 2023

My patch keeps old challenge working and therefor, it can replace yk-personalization.

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

4 participants