PIV and PIV-Want-To-Be Issues #816
Conversation
|
Thank you! Will try with #797 and report here. In the meanwhile, two comments:
I personally do not consider this NIST violation to be an issue, and do not want it "fixed", i.e., do not want the code to enforce the "no-PIN-over-NFC" rule.
I agree that it's bad, but some tokens just do not support RSA keys larger than 2048 bits, and ECC keys over curve larger than P256. I don't think NIST requires compliant PIV tokens to be able to support RSA-3072 and/or ECC over P384 (though I for one would love if my NEO would be able to do so, and maintain its NFC capabilities). |
|
In regards to your question on #624 about CAC. It they still fail, try changing line 3042: I would like to see a debug log if CAC fails without the above change. |
|
Hmm... Patch to
But this patch applied cleanly against OpenSC master. So is its intention to co-exist with #797, or replace it? I confess my confusion. |
--2016-07-06 12:36:29-- https://github.com/OpenSC/OpenSC/pull/816.patch 816.patch [ <=> ] 20.58K --.-KB/s in 0.03s Last-modified header missing -- time-stamps turned off. $ patch -p1 < 816.patch Douglas E. Engert [email protected] |
Douglas E. Engert [email protected] |
Douglas E. Engert [email protected] |
|
|
||
| /* TODO 800-73-3 does not define a logout, 800-73-4 does */ | ||
|
|
||
| SC_FUNC_RETURN(card->ctx, SC_LOG_DEBUG_NORMAL, r); |
There was a problem hiding this comment.
If the card doesn't implement a logout function, you should return SC_ERROR_NOT_SUPPORTED instead of ignoring the explicit logout. If a logout according to 800-73-3 (whatever this may be) is not possible you could use sc_reset instead.
|
@mouse07410, mkdir test |
|
@frankmorgner, Tat is the output of: it was started and then when the prompt for the pin is displayed, the following command is run in another window to cause interference. /usr/bin/opensc-tool -s "00 A4 04 00 0F D2 33 00 00 00 45 73 74 45 49 44 20 76 33 35" This does a select file (select AID) as issued by the card-mcrd.c from mcrd_match_card. Then the PIN is entered. In the opensc-debuglog only the pkcs11-tool output is shown. The other command was run and when it completed, the PIN was entered. This in the time period between 14:42:14.112 and 14:42:23.293 C_Login is called, pcsc responds that the card has been reset by the other process, and sc_pin_cmd is called 14:42:23.293 line 1708. Line 1727 is in the report of the issue in the code you asked about testing for 6D 00. When run the interference program is run with disconnect_action=leave; the same 6D 00 is returned, |
|
First of all, it's great that you're solving the PIV (wanna be) quirks inside the card driver. Also, I think it's best to solve the PIN related stuff inside A simple workaround for the problem with the reset could be to re-initialize the card when the reset was reported (i.e. somewhere around https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/card.c#L405). Without any big changes we could do the following here:
Thought this possibly introduces some superfluous communication, it should definitely set the card into a usable state after the reset. |
|
Re-initializing the card after a reset should even work for all cards, and doesn't rely on a special call back implemented in the card driver. |
|
I disagree. Interference of our process by some other process comes in many forms. A reset by another process is just one type of interference which PCSC can detect and report. (I can send you an opensc-debug log were there was no reset done, but the AID was reset.) Other types of interference include a different AID was selected then the one our process was using. A Secure message session was interrupted and our process need to reestablish it. (This may already be handled.) A logout was done and a verify needs to be done again. Other cards may have more issues. Re-initializing the card can be very time consuming and aggravating for the user, as many cards read every cert on re-initialization and any pin caching would also be lost. Re-initializing can loose session objects. For example PKCS#11 ECDH key derivation is a two step operation. PKCS#11 key derivation returns a secret key object. The PKCS#11 application does not have to use it right away, giving time for interference from some other process. If the secret key is stored on the card, interference may cause it to be lost, if it is stored in memory interference will not cause a problem, but a card re-initialization will lose it even if in memory or on the card (PIV key derivation returns the secret key in one operation from the card and the key gets stored in memory until PKCS#11 requests it.) So I still think the card driver should get involved, it has routines for every other type of operation, in the sc_card_operations structure. If you want some default routine in iso7816.c that is OK, but the card driver should be able to override it. Interference from other processes cannot occur while our process holds the PCSC lock, i.e. from SCardBeginTransaction to SCardEndTransaction. Or SCardConnect to SCardDisconnect if SCARD_SHARE_EXCLUSIVE or SCARD_SHARE_DIRECT are used. So a good place for a card driver routine to check and fix if possible it is when the PCSC lock is obtained. the _check_state routines never went that far, but they cold have. |
|
I agree that there are many other ways an other process may cause trouble. However, session objects are most likely no problem since they are typically lost by a card after a reset. It is true that in theory it is possible to put a lot of thought into the process of how to recover interference. Doing this at the card driver level will most likely not be enough, because for some things need a global view to protect against all possible loss of information. In the end, I think, protecting against everything is not very practical. This would introduce a lot of unused code which will most likely not be of much use. I suggested to protect against a detectable reset, because it's easily done and it's likely. If you are paranoid, you could extend the |
|
Actually, while it's fun making alternatives for working code, I'm asking myself how much effort it is really worth. A simplified #624 did the job for me satisfactory enough. My criteria:
Anyway... #797 + #816 (applied an hour ago to the OpenSC master) failed to work: Here's the log: And here's the log of failed sequence of Keychain Access unlock->lock->unlock etc. At the end of this log the token cannot be unlocked by Keychain Access any more - it doesn't even prompt for the PIN. |
|
Looking at opesc-797-2-pss2.txt ... Your NEO has version 1.0.4 Mine have 0.1.2 Can you add this so lines 3027-2032 look like: Also change line 3182 Or 3183 if the above line was added): |
Added, built, and will report.
Around line 2032 - what does that flag mean? What's its effect? (And why is it there?) |
|
Still fails. Sequence was (to interpret what's in the log):
Here's a simpler test:
opensc-797-2-keychain2.txt
|
Douglas E. Engert [email protected] |
OK, thanks! |
|
I have an updated card-piv.c that that is built upon #797 and #816. #797 changes how the pin_cmd returned the login state and the tries_left. There are some other changes in the commit: @frankmorgner it is not clear how to make a PR of all of this. This list of commits can be found here: @mouse07410 can you try this? df62b35 |
|
@dengert to create a branch I am not sure which other changes you mean, but they may be cherry picked as well. |
|
Right now I seem unable to download or pull a single commit (unlike a PR), and it's a bit too large to just go in and do it manually. Perhaps you want to do what @frankmorgner suggested and create a new branch? Or just push it as a part of #816? Update I've made a manual edit, but would still prefer something that doesn't depend as much on my carefulness. |
|
@dengert and @frankmorgner I have been able to try this commit df62b35 only with NEO. But at this time the results are very promising. All of my tests passed successfully (so far :). I still need to try it with CAC (and see if Gemalto Dual reader with its extra performance toll and NFC throws any monkey wrench) - but so far the results seem to be even better than with the original #624. Good work, if I may say so. Update Here's the log, in case you need to see exactly what worked, and how. First I unlocked (and maybe locked again?) the token with Keychain, then I ran a couple of OpenSC/OpenSSL CLI tests, then sent signed email with Apple Mail, then did another OpenSSL test, then sent another signed email, then decrypted chain of inbound encrypted emails (this part is important!), then did a couple of tests with Keychain parallel access that also worked well. We also want to state explicitly what extra objects NEO must have installed/written. This is NOT a big deal, I just want it recorded somewhere, so whoever needs to use NEO PIV before the firmware is fixed, can get it working quickly and easily. |
Mac OS X El Capitan 10.11.5.
It is my understanding that How can I check that (and do I need to)? |
Not all PIV cards follow the NIST 800-73-3 standard. This commit is designed to address some of the issues. OpenSC developers don't have access to all the different versions of devices or access to release notes for the devices to see when a bug was introduced and when it is fixed. To make OpenSC code changes easier, the code is divided into four sections: (1) Identify the card/token as best possible by looking at the "Historical bytes" in the ATR. For the Yubico devices read their version number and log it via sc_debug. (2) Define the card_issues CI_* defines in card-piv.c. There are 8 of them at the moment. See below. (3) based on the card->type and possibly Yubico version set the priv->card_issues flags that apply to current card or device. (4) Implement in the code changes needed for each issue. Other issues can be added. As more info is obtained (3) can be updated using the version number as needed. The card issues are: CI_VERIFY_630X - VERIFY "tries left" returns 630X rather then 63CX CI_VERIFY_LC0_FAIL - VERIFY Lc=0 never returns 90 00 if PIN not needed. Will also test after first PIN verify if protected object can be used instead CI_CANT_USE_GETDATA_FOR_STATE - No object to test verification in place of VERIFY Lc=0 CI_LEAKS_FILE_NOT_FOUND - GET DATA of empty object returns 6A 82 even if PIN not verified CI_OTHER_AID_LOSE_STATE - Other drivers match routines may reset our security state and lose AID CI_NFC_EXPOSE_TOO_MUCH - PIN, crypto and objects exposed over NFS in violation of 800-73-3 CI_NO_RSA2048 - does not have RSA 2048 CI_NO_EC384 - does not have EC 384 The piv_card_match and piv_init interactions were cleaned up. Changes to be committed: modified: card-piv.c modified: cards.h
PR 797 changed the way pin_cmd returned the verification state of the card and the tries_left. This commit updates card-piv.c and PR 816 to follow PR 797 Changes to be committed: modified: card-piv.c
A sleep(1) is added after SCARD_W_CARD_RESET as done in other parts of reader-pcsc.c Extra debugging messages are output. SCard routines return "LONG" which may be different then "long" on some systems were "LONG" is 32 bits and "long" is 64 bits. Make sure printf format of 0x%08lx has a matching "long" input variable. Changes to be committed: modified: reader-pcsc.c
Changes to be committed: modified: src/libopensc/reader-pcsc.c
dengert
force-pushed
the
not-all-pivs-are-pivs
branch
from
1a96317
to
98fa1eb
Compare
|
Rebased on master see #823. |
|
@dengert it looks like the current #816 rebased against the current master (see #823) works fine with NEO (I kept Update: I forgot to include the log. It's probably unnecessary, since there are no complaints - but just in case that you might want to see the detailed/debugging output of the NEO interactions. I want to remind again @frankmorgner and @viktorTarasov that the current master broke PIV cards (because of the merged #797), and merging #816 is necessary to restore PIV functionality. |
|
@frankmorgner. @viktorTarasov, can you merge #816? ( I could do it, but prefer not to get in you way.) |
|
@dengert, I've just done a few gentle tests with CAC (current master with your current #816), and it appears to work:
Here's the log: Maybe you should "get in their way" and merge #816 yourself. After all, master remains broken (for PIV) - and it doesn't look like people care too much, obviously being busy with other important things. |
|
@viktorTarasov, @frankmorgner, |
|
@dengert @viktorTarasov A security issue was just merged; my comment above regarding the logout function is still relevant:
For example, the minidriver will automatically do a reset if the logout is not implemented (return code |
|
@viktorTarasov, you squished 2 of the 4 commits for improved logging and merged it. The other two commits still need to be merged to get the piv cards working with #797. |
|
@viktorTarasov, |
|
@viktorTarasov @frankmorgner, I take back what I said. It looks like you did merge #816. Thanks. |
Douglas E. Engert [email protected] |
What's the relation between this "auto-reset" and |
Douglas E. Engert [email protected] |
|
There's a strange problem with Gemalto Prox Dual card reader and MacBook Air. I was practically unable to use the NEO token (PIV) in that reader. Smartcard logon did not work. Token recognition was screwed up. And it again displayed token name as "PIV-II" instead of the CN of the first available certificate. NEO took 25-35 seconds to be recognized - against 2-4 seconds it used to take before this merge. Here's the log of the NEO insertion - we didn't even get to the unlock yet. Here's the same as above but with a successful (albeit taking 3-4 seconds!) unlock: This is also of interest: with the corresponding log: Here's the same thing when I unplugged Gemalto reader and inserted NEO directly into USB slot (and now everything appears kosher): @dengert I know my tokens work over NFC, and I use this type of reader for that (though usually on MacBook Pro). Yesterday I updated my OS from 10.11.5 to 10.11.6. And now I moved from my master to OpenSC master (probably I shouldn't've done that, but I was stupid). Do you see any of these as factors capable of causing the above? |
http:https://ludovicrousseau.blogspot.com/2010/05/how-to-know-pin-sizes-supported-by.html Using reader with a card: Gemalto Prox Dual USB PC Link Reader(2) Using reader with a card: Yubico Yubikey NEO OTP+U2F+CCID Douglas E. Engert [email protected] |
Thank you - submitted #833. I'm sure you're right, and this has to do with PCSC or reader-pcsc.c. MacOS - maybe, but I doubt it because it works on MacBook Pro with the same software (including OS and reader). Maybe performance requirements of the reader can't be satisfied by the I/O of MacBook Air...? I don't know... But I'd like it to work... |
Fix a number of PIV NEO card issues with using pin_cmd This is designed to get pin_cmd to return verification state even when the care does not follow the ISO and NIST specs.
It also addresses other isses with compliance with NIST standards.
It is designed to be usable with #797 and does much of what #624 would have done, but only for PIV cards.
Many of the fixes are are not related to #797.
This can be committed before #797 and any tokend changes to need pin_cmd to work.
@mouse07410 Can you see if this solves many of you issues when using #797?
Not all PIV cards follow the NIST 800-73-3 standard. This commit is designed to address some
of the issues. OpenSC developers don't have access to all the different versions of devices
or access to release notes for the devices to see when a bug was introduced and when it is fixed.
To make OpenSC code changes easier, the code is divided into four sections:
(1) Identify the card/token as best possible by looking at the "Historical bytes" in the ATR.
For the Yubico devices read their version number and log it via sc_debug.
(2) Define the card_issues CI_* defines in card-piv.c. There are 8 of them at the moment.
See below.
(3) based on the card->type and possibly Yubico version set the priv->card_issues flags that
apply to current card or device.
(4) Implement in the code changes needed for each issue.
Other issues can be added. As more info is obtained (3) can be updated using the version
number as needed.
The card issues are:
CI_VERIFY_630X - VERIFY "tries left" returns 630X rather then 63CX
CI_VERIFY_LC0_FAIL - VERIFY Lc=0 never returns 90 00 if PIN not needed. Will also test after
first PIN verify if protected object can be used instead
CI_CANT_USE_GETDATA_FOR_STATE - No object to test verification in place of VERIFY Lc=0
CI_LEAKS_FILE_NOT_FOUND - GET DATA of empty object returns 6A 82 even if PIN not verified
CI_OTHER_AID_LOSE_STATE - Other drivers match routines may reset our security state and lose AID
CI_NFC_EXPOSE_TOO_MUCH - PIN, crypto and objects exposed over NFS in violation of 800-73-3
CI_NO_RSA2048 - does not have RSA 2048
CI_NO_EC384 - does not have EC 384
The piv_card_match and piv_init interactions were cleaned up.
Changes to be committed:
modified: card-piv.c
modified: cards.h