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
Fix OpenPGP driver to work correctly with YubiKey NEO #507
Conversation
/* Awful hack for composite DOs that have | ||
* a TLV with the DO's id encompassing the | ||
* entire blob. Example: Yubikey Neo */ | ||
if (tag == blob->id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scrolling through the standard I can't see that this encoding is valid, right?
If this is Yubikey specific, could you add a check for the card here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is happening here is that the original code seems to expect something like this:
Reading DO 0x65 (Cardholder Related Data):
0x5B (Name)
0x07 (7 bytes)
'Example'
0x5F 0x2D (Language)
0x02 (2 bytes)
'en'
...
In other words, the original code is expecting a plain concatenation of the simple DOs. What the YubiKey and other javacardopenpgp-derived code does is something like the following:
Reading DO 0x65 (Cardholder Related Data):
0x65
0x14 (The entire 0x65 DO has 20 bytes)
0x5B (Name, this is the payload of the 0x65 object)
0x07 (7 bytes)
'Example'
0x5F 0x2D (Language)
0x02 (2 bytes)
'en'
Other fields up to 20 bytes
So the added hack here simply unwraps the outermost layer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A side effect of this hack is that the code will now treat something like the following as valid:
Reading DO 0x6E (Application Related Data):
0x6E
0xXX (some length)
...
0x73 (Discretionary data objects)
0xXX (some length)
0x73 (Discretionary data objects tag nested twice)
0xXX (some length)
0xC0 (Extended capabilities)
...
where there are two 0x73 tags nested.
Referring back to your original comment, this hack isn't YubiKey specific, because YubiKey, javacardopenpgp, and other cards all could possibly return a constructed DO with an outer layer like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick reading of the GnuPG code (http://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=common/tlv.c;h=c6875640690c8410406d7b19afebcd543bb42c65;hb=refs/heads/STABLE-BRANCH-2-0#l39) seems to imply that it is willing to recurse up to 100 times when looking in a constructed DO. This would imply that the above side effect isn't too major because it seems GnuPG would accept that anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks for clearifying.
Does that mean that the PIV driver will never be used for the NEO after these changes? I'm certainly happy if there's a way for the OpenPGP driver to be used, but there needs to be a way for the PIV driver also to be used. Many NEO users who have the Yubico tools will have loaded their keys onto the Yubikey with the PIV layout. I don't know how the OpenPGP and PIV applet interact. Is it possible to use both at once, or do you instead have to choose which one you're using? Maybe for the NEO ATR, we need to read some data files from the card to see whether it's been initialised with the OpenPGP or PIV layout, rather than relying on the ATR alone. Given that Yubico's tools use the PIV mode, perhaps that should be the default? I'm biased of course, because that's just how my NEO is set up! |
Yes, I agree PIV should be the default. But you still can force the PGP in |
This patch happens to make OpenPGP now the default. I think this is because the "openpgp" entry appears before the "PIV-II" entry in internal_card_drivers in src/libopensc/ctx.c. You can still set either one as default by adding the appropriate card_atr block in opensc.conf. I don't know if it is possible to make both of them accessible from OpenSC at the same time (via e.g. separate slots). I didn't look deeply into OpenSC to see how difficult it would be to have two drivers active at the same time. Unfortunately, I don't think @NWilson's suggestion of "read[ing] some data files from the card" will work because it is possible to have different keys loaded into both the OpenPGP applet and the PIV applet (for a total of 7 keys). IMHO the support for the various applets on the Yubikey NEO is a huge mess overall (not the fault of OpenSC in particular). Just as an example, on Linux (Ubuntu at least), running "gpg --card-status" will lock out the YubiOATH authenticator. |
@rqou The locking issue - that's not the fault of OpenSC nor NEO, that's the choice of GnuPG developers - they mandate exclusive access to the token. So it is either GnuPG way or the highway, no (easy) compromises. |
Sorry, I wasn't trying to blame any particular project for the annoyances. I assume that cards such as the Yubikey NEO with its many independent applets are somewhat uncommon, leading to less-than-optimal usability. I don't actually have any good ideas on how to deal with this issue. |
Most NEO users are using the PIV applet, and if we detect (or default) that and use the PIV driver then we're happy. If you have initialised the OpenPGP data, but not initialised any of the data files the PIV applet uses, then we can detect that and use the OpenPGP driver. If you have loaded a combination of PIV and OpenPGP keys then there's really nothing that can be done automatically! |
IMHO the option would be that GnuPG relaxes its stance and allows to share tokens in use, but I've tried a few times to ask about such possibility and that's something that seems to be against the design philosophy of GnuPG. |
@NWilson I would not be so sure about the PIV vs PGP issue. Do you have hard numbers? X509 vs PGP is a divided world and I'd say there are equally diehard devotees. |
I do know that Windows (at least Win 8.1 and newer) has a built-in driver for PIV cards that seems to recognize the NEO (and obviously does not have a built-in driver for OpenPGP cards). |
I don't have any idea how many people use these applets, I'm completely guessing! I bought my NEO and just followed the Yubikey docs. Windows 7 also uses the PIV applet for the NEO out of the box. |
I don't particularly care whether OpenSC defaults to OpenPGP or PIV for the NEO. Should I include a change in etc/opensc.conf.in to default it to PIV? |
@rqou that would be a great solution, thanks! |
Some comments: The PIV really defines an application on the card. In the past, there was not distinction between With PKCS#11 the only way to select an application would be if the PKCS#11 code did something with multiple slots, Future versions of the Neo could appear as multiple USB devices, with OpenPGP on one and PIV on another... The Neo is the only PIV like device that I know of that allows the user to generate key and write to the card. The PIV was designed I thought that the Neo allowed the user to set the default for the device using the Yubikey-personalize program. You could have two opensc.conf files one that selected OpenPGP first, the other PIV first and then set the OPENSC_CONF env variable to point On 8/24/2015 4:52 AM, Robert Ou wrote:
Douglas E. Engert DEEngert@gmail.com |
On 25/08/15 00:03, Doug Engert wrote:
Is this wishful thinking or you have some insight? Because AFAIU this is
Strangely enough, there are no (recent and usable) PIV applet
Not any more, since: https://www.yubico.com/2014/07/yubikey-neo-updates/
Eventually something more user-friendly should be figured out to support Martin |
Wishful thinking, looking for a way that have one device serve multiple purposes. On 8/24/2015 10:30 PM, Martin Paljak wrote:
Douglas E. Engert DEEngert@gmail.com |
Hi, sorry for the late response On Tuesday, 4. August 2015 01:44:47 Frank Morgner wrote:
To the contrary: looking at the individual patches, I noticed that it even Please apply. PeterPeter Marschall |
@dengert , @rqou suggested to add an entry in Apart from that, card drivers that need to transfer APDUs (e.g. SELECT AID) should be ordered after the drivers that don't need to exchange any data, see https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/ctx.c#L106-L107 |
Disagree. A driver that test only for an ATR where it is known that the card and OpenSC already support other applications on the card must test that the application is on the card. #507 knowingly causes a problem with the NEO with a PIV application. "drivers that don't need to exchange any data" are making the assumption that the ATR defines the application. But there are cards such as the Neo where this is not true. A number of solutions: (2) Check for AIDs in common routine, on all cards. See #538. (3) card-openpgp.c check in the match card has the OpenPGP application (4a) Put PIV driver before OpenPGP driver because PIV driver does check if PIV application is on the card during match phase. (On line code moved in ctx.c) (4b) Put the Openpgp after the PIV driver (One line code moved in ctx.c 4b puts both drivers after "drivers that don't need to exchange any data") (6) Using the opensc.conf to set the ATR, or the list of drivers. I am most interested in PIV, so (4b) would keep the the PIV working if the rest of #507 is installed. (3) would address other cards that support a OpenPGP application without additional code changes. (2) could address other future card for other applications without additional code changes. (6) Using the opensc.conf to force a driver was added to allow some flexibility on the part of a user with a new card or reader that was almost supported by OpenSC until code changes could be made. There may be implications with the pkcs15-* emulation drivers getting mixed up. We have seen this with the CNS/CIE cards, A future issue, how could OpenSC present both applications on the Neo at the same time via PKCS#11? On 8/31/2015 6:23 AM, Frank Morgner wrote:
Douglas E. Engert DEEngert@gmail.com |
Yes, (3) should be implemented. Actually I have already done that here frankmorgner@e2b1d8e but I didn't have time to complete testing and fixing all hiccups. Handling multiple applications on a single card should be addressed in a separate issue. Currently, we only have the option of setting a default driver. I consider doing that via
In my view, #507 doesn't cause a bug in the piv driver, it only changes the default application for the neo. Re-ordering would be fixed by (6). If possible I'd like to avoid changes to the source code (4a/4b) for this behavior because the root of this problem is that OpenSC does not have any support for multi application cards. This should be addressed separately as stated above. |
The change addresses my concern because it in effect also does (4b) by the cahnge in ctx.c I have sent a note to Yubico developers, and opensc-devel that may get some comments from Yubico. |
Agreed, it would actually be interesting to hear what @Yubico would like to have as default application in OpenSC... |
We have a reply here http://sourceforge.net/p/opensc/mailman/message/34426171/ which does not give a definitive answer but opts for PIV as default application within OpenSC. @rqou please remove the neo's ATR in |
Thanks! From: Frank Morgner [notifications@github.com] We have a reply here http://sourceforge.net/p/opensc/mailman/message/34426171/ which does not give a definitive answer but opts for PIV as default application within OpenSC. @rqouhttps://github.com/rqou please remove the neo's ATR in card-openpgp.c, all other changes can stay as they are. This way the PIV applet is used by default, while card-openpgp.c is technically compatible with the neo's pgp applet. If you want you can add an entry in opensc.conf that forces the openpgp driver for the neo's ATR, but please make sure it's commented out by default. — |
A second opensc.conf, say for example: neo-openpgp-opensc.conf
and when starting a program than needs openpgp, set or export OPENSC_CONF=/path/to/neo-openpgp-opensc.conf On 9/2/2015 1:36 PM, Mouse wrote:
Douglas E. Engert DEEngert@gmail.com |
I like this recommendation. Unfortunately it does not work - perhaps you could tell me what I seem to be doing wrong: $ OPENSC_CONF=/Library/OpenSC/etc/neo-openpgp-opensc.conf openpgp-tool -v --raw In case more details would help: $ OPENSC_CONF=/Library/OpenSC/etc/neo-openpgp-opensc.conf openpgp-tool -vv -U On Sep 2, 2015, at 15:44 , Doug Engert <notifications@github.commailto:notifications@github.com> wrote: A second opensc.conf, say for example: neo-openpgp-opensc.conf card_atr 3b:fc:13:00:00:81:31:fe:15:59:75:62:69:6b:65:79:4e:45:4f:72:33:e1 { and when starting a program than needs openpgp, set or export OPENSC_CONF=/path/to/neo-openpgp-opensc.conf On 9/2/2015 1:36 PM, Mouse wrote:
Douglas E. Engert <DEEngert@gmail.commailto:DEEngert@gmail.com> — Uri Blumenthal |
You're doing everything right. Unfortunately, it is OpenSC's pgp driver that's written too narrow... It heavily relies on the card type that needs to be initialized (currently via ATR). Sorry for not checking earlier! I fear we will have to fall back to @dengert's option (4b). Could you check if everything works as expected when you cherry pick frankmorgner@e2b1d8e on top of your PR? (frankmorgner@e2b1d8e already shuffles the order card driver recognition and determines the pgp |
Iin the neo-openpgp-opensc.conf type = "9002"; This will set it a SC_CARD_TYPE_OPENPGP_V2. On 9/3/2015 1:09 AM, Mouse wrote:
Douglas E. Engert DEEngert@gmail.com |
Thank you - this proceeded a little further: $ OPENSC_CONF=/Library/OpenSC/etc/neo-openpgp-opensc.conf openpgp-tool -v -U Using reader with a card: Yubico Yubikey NEO OTP+U2F+CCID Connecting to card in reader Yubico Yubikey NEO OTP+U2F+CCID... Using card driver OpenPGP card. Account: uri Failed to select EF 3F00:0065:005B: File not found I'm open for more help/suggestions! ;) From: Doug Engert [notifications@github.com] In the neo-openpgp-opensc.conf type = "9002"; This will set it a SC_CARD_TYPE_OPENPGP_V2. On 9/3/2015 1:09 AM, Mouse wrote:
Douglas E. Engert DEEngert@gmail.com — |
From reading the above comments, you also need this pull request minus line 48: Looks like this ws added for Neo:
On 9/3/2015 1:57 PM, Mouse wrote:
Douglas E. Engert DEEngert@gmail.com |
fixes OpenSC#391 closes OpenSC#507
fixes OpenSC#391 closes OpenSC#507
Does the following things:
Tested only with a YubiKey (I don't have any other OpenPGP cards). Does not handle trying to use both the OpenPGP mode and the PIV mode (OpenPGP mode now takes over after adding the ATR).