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
Workaround Yubikey bug with SELECT DATA #2969
base: master
Are you sure you want to change the base?
Conversation
There is a known issue with Yubikey tokens, that they're requiring another length byte in APDU for SELECT DATA: Yubico/yubikey-manager#403 Let's workaround this, by checking return value of SELECT DATA, and if error indicates that parameters are wrong, let's try Yubikey-specific ones.
Thanks for the PR! Isn't there a way to detect yubikey before we will get the error to send the right APDU right away? |
Very good question - I tried to find the way. It seems that in yubikey-manager, they use Yubikey-specific command to detect firmware version (https://github.com/Yubico/yubikey-manager/blob/main/yubikit/openpgp.py#L1018), which, I thought, is not the best approach for OpenSC. I cannot find any generic way to determine vendor, unfortunately. |
Using a Yubikey with OpenPGP applet to get ATR and version:
Paste the ATR into https://smartcard-atr.apdu.fr/ With PIV applet can also get version number sending |
Well. We already have a yubico_version as part of the PIV private structure: OpenSC/src/libopensc/card-piv.c Line 424 in 23f3df9
Would using that to detect yubikey work? |
I believe it would for OpenPGP applet. Just a different command. Yubikey PIV applet uses |
Hmm, is this ATR standard command? I did not want to use vendor-specific commands to detect version etc, to not cause issues with other tokens. |
I would say it should be safe to assume the token is yubico if it is in the ATR historic bytes. |
I was thinking, that there is already ATR matching, by pgp_atrs, and there is even flags field - so, I can add flag there, to allow Yubikey-specific commands/workarounds. But, I cannot find where these flags are read or passed. I'll write PoC, and will push as a separate commit into this PR. |
https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/card-openpgp.c#L305-L315 will match a card by ATR, or in lines https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/card-openpgp.c#L320-L325 will match a card if it has a OpenPGP applet. In either case, The ATR Historical bytes can be found at: The ATR is for the card. A Yubikey card can have both a openpgp applet and a piv applet. (OpenSC can only use one at a time. card-piv.c checks the Historical bytes here: It the sets a card type. But with OpenPGP the card type is based on version of OpenPGP applet. You could just set a flag saying card is a Yubikey and later call a routing like |
Please, check new commit - I added new flag, used it to select proper pgp_select_data..., and in pgp_select_data_yubikey I'm checking for Yubikey version, to see if it should try Yubikey-specific ATR. It works on both of my Yubikey tokens, selecting proper data, but I cannot test it on other vendors, unfortunately. |
The point of reading the Yubikey version, is that Yubikey as fixed problem in 5.4.3 and OpenSC could then handle old and new cards as needed. You could simplify the code by saving the version in https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/card-openpgp.h#L171-L190 Then test in You would not need SC_CARD_FLAG_YUBIKEY_SELECT or special |
It is not a card type, it's a flag
Why to test for historical bytes 2nd time, if there is already historical bytes check via pgp_match_card? select_data seems to be available only in OpenPGP v3, so, there is no point to check for Yubikey version for Yubikey 4 and earlier, right? It seems that historical bytes for Yubikey 5 are different from Yubikey 4, so, it won't help much, to have historical bytes check, to detect version of firmware on all Yubikeys |
So, yeah, in Yubikey 5, last bytes of ATR are 59:75:62:69:4b:65:79 instead of 59:75:62:69:6b:65:79:34 - so, YubiKey (with big The whole historical bytes for Yubikey5 looks like this 80:73:c0:21:c0:57:59:75:62:69:4b:65:79 - I'm not sure, if I should compare with this whole line? It might have some settings in it... |
Then add a flag that is only used in OpenPGP driver in If you did want to have a new card type for OpenPGP card driver, add it in https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/cards.h#L107
Because if the ATRs is not matched, the code then tests for OpenPGP applet at
You could almost copy this code to test for Yubikey cards: Also look at https://smartcard-atr.apdu.fr/ and paste in the ATR to see what is in it. What cards do you have? |
@ya-isakov While looking at But of the 3 Yubikey devices listed in I would suggest the code be modified here to parse further for Yubikey and to then also read the Yubikey version number to handle the bug you are trying to address. |
@ya-isakov will you have a second look at the suggestions above? |
@frankmorgner Sorry, I don't have time now to do it as suggested. |
There is a known issue with Yubikey tokens, that they're requiring another length byte in APDU for SELECT DATA:
Yubico/yubikey-manager#403
Let's workaround this, by checking return value of SELECT DATA, and if error indicates that parameters are wrong, let's try Yubikey-specific ones.
Checklist