-
Notifications
You must be signed in to change notification settings - Fork 759
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
Fixed PIN authentication bypass #2806
Conversation
If two processes are accessing a token, then one process may leave the card usable with an authenticated PIN so that a key may sign/decrypt any data. This is especially the case if the token does not support a way of resetting the authentication status (logout). We have some tracking of the authentication status in software via PKCS#11, Minidriver (os-wise) and CryptoTokenKit, which is why a PIN-prompt will appear even though the card may technically be unlocked as described in the above example. However, before this change, an empty PIN was not verified (likely yielding an error during PIN-verification), but it was just checked whether the PIN is authenticated. This defeats the purpose of the PIN verification, because an empty PIN is not the correct one. Especially during OS Logon, we don't want that kind of shortcut, but we want the user to verify the correct PIN (even though the token was left unattended and authentication at the computer). This essentially reverts commit e6f7373.
@hhonkanen , you've introduced this initially for convenience with PIN pad readers, could you check whether this impacts your use cases? |
To reproduce the problem, just do |
@frankmorgner The use case where I needed to introduce e6f7373 to get it working was such, that the system using the card via PKCS#11 needed to do several crypto-operations, after authenticating the PIN with a pin pad reader. If I remember correctly, only one process was involved. The PIN had to be authenticated only once. With a normal reader, enabling pin caching would allow this, but with a pin pad reader, the pin cannot be cached. Before e6f7373 the user would be prompted for PIN for each operation, which would have been inconvenient and unacceptable from usability point of view. Even across several processes, it may be desired behavior to keep the PIN validated to log-on to several applications or web sites with single pin entry. You can always mark a specific key "user-consent" to ensure that the associated PIN is required for each use, if needed. I agree it is a problem if an empty PIN from user input ends up here, and passes as validated. My commit was supposed to enable OpenSC to only internally check whether the PIN has been already authenticated. This should be in some way distinguished from user providing an empty PIN. Unfortunately I think with this PR the problems I encountered with pinpad readers might come back. |
Also note that several Yubikey PIV tokens have problems with login state. Grep for: So the code is guessing if the login state for these devices. PIV logout was added in NIST 800-73-4. PIV-4-extensions i.e. #2053 the PIV PR for SM ( open for almost 3 year now) adds support for logout: https://github.com/dengert/OpenSC/blob/PIV-4-extensions/src/libopensc/card-piv.c#L6133-L6139 (I am still on vacation) |
@hhonkanen do you |
It looks like I managed accidentally to reproduce this issue with my old yubikey, pkcs11 tool and chromium, but it is not that deterministic as I would like so I was not able to get a PKCS11 trace or something useful. From short observation
I think it will require some more investigation. Tracking the login state only in the software is also not ideal as then we move the responsibility from hardware to software. Using proper logout procedure should be preferred (even though it might not be always safe as the processes might crash before this is called). |
@dengert thanks for reminding me about disconnect = leave setting. I do not know if it would help in the pinpad use case. It's been a while since e6f7373, so other things which affect it may have changed in-between. I don't have a pinpad reader with me now, and cannot get one from anywhere soon, as I'll not be working at the office before August. I remember the problem was, that OpenSC decided it needs to authenticate the PIN again withing a single PKCS#11 session, and prompted user for PIN on the pinpad between PKCS#11 operations, and I could fix it this way. I have a feeling it may still be necessary to check the pin authentication state, if it is not checked somewhere else. But we should in some way distinguish this from providing a zero-length PIN externally. Could anybody test doing several PKCS#11 operations (like C_Sign / C_Decrypt / C_Wrap / C_UnWrap) using a pinpad reader? |
AFAIK that would be the application decision to issue the C_Login() itself. Maybe prompted by something in opensc, but that should not be the case unless there was "always_authenticate" attribute or some more complex auth scheme (did not look how the 4 pins were used in the original issue.
AFAIK the zero-length pin is provided by the software every time when there is a pinpad reader.
I have some HP pinpad reader (and MyEID cards) I can test, but I would probably need some more information on the configuration and steps you used. |
I just remember that I got into sc_pkcs15_verify_pin with zero length PIN between operations, without calling C_Login again in the calling software.
Here is a test program: To run it, you have to prepare a MyEID card with an RSA key. By default, it logins with PIN with value "1111" and so-pin with value "12345678". You can modify it to use pinpad. It's been a while since I have run it last time, some more preparations might be needed. The test program also verifies that always_authenticate works for a Generic Secret key being wrapped in the end, but I think it is not related to this issue. |
All cards supporting the query for PIN_INFO should be affected, i.e CAC1, PIV, Coolkey, SC-HSM, gemsafeV1, nqApplet, StarCOS, IASECC, CardOS, MyEid, IDPrime, skeid, CAC, OpenPGP.
Yes, you could check for In PKCS#11 we are terminating session with sc_logout, which means the above cards are not affected if (a) the card driver implements logout and (b) the process authenticating the card has terminated. That's why I suggested to use pkcs11-tool and a PIV card for reproducing the issue. |
I believe that is correct. PKCS11-base-V3.0-os says:
Some thing to check: The application may not check for CKF_PROTECTED_AUTHENTICATION_PATH and issue prompt for a PIN. In the above cases user may know there is a pin pad reader and does not know what else to do but send zero length PIN. OpenSC may in some places assume zero length pin means: PIN PAD reader is usable or no PIN is needed i.e. use existing login state or a zero length is valid for the device. If its an application issue do we need some Do we say if CKF_PROTECTED_AUTHENTICATION_PATH is set and C_Login is called with a zero length PIN we treat it as if C_Login was called with NULL_PTR? If CKF_PROTECTED_AUTHENTICATION_PATH is not set, then a zero length pin is and error? The Verify APDU with a zero length pin may be handled by card is strange ways. AFAIK most card enforce some minimal length for a PIN. (I like the last two.) |
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.
I am ok with this change, but I still need to get back to the investigating the issue with the pinpad reader. I will not get back to that earlier than next week but this change should not be blocked on this. If we will need to figure out some other solution for pinpad readers, it can land in separate issue/pr.
I think it is? OpenSC/src/pkcs11/framework-pkcs15.c Line 1145 in f1993dc
Yes
Yes
The zero length PIN is (was) for the cases where the misbehaving application calls C_Login after a PIN prompt even if CKF_PROTECTED_AUTHENTICATION_PATH is set and should be called with NULL.
There are fields available for PIN length information in PKCS#15. Internally some cards support ISO VERIFY with 0 length to get remaining tries, a feature used by some pinpad readers. But I find both questionable approaches (as cards that do NOT support it, will either barf or decrease try counter) |
The following is speculation on how the verify zero len pin could look like it worked. It depends on the card card driver and state of the card. PIN PAD readers follow a different path. Before #2806 without pin pad reader, it looks Variable
will endup with APDU with Lc=0 with cse=SC_APDU_CASE_3_SHORT iso7816_pin_cmd will endup calling
What should happen in a verify request should never be sent with zero len pin. I have not tried this with debugger. What card was being used when the failure occurred? |
If two processes are accessing a token, then one process may leave the card usable with an authenticated PIN so that a key may sign/decrypt any data. This is especially the case if the token does not support a way of resetting the authentication status (logout).
We have some tracking of the authentication status in software via PKCS#11, Minidriver (OS-wise) and CryptoTokenKit (OS-wise), which is why a PIN-prompt will appear even though the card may technically be unlocked as described in the above example. However, before this change, an empty PIN was not verified (likely yielding an error during PIN-verification), but it was just checked whether the PIN is authenticated. This defeats the purpose of the PIN verification, because an empty PIN is not the correct one. Especially during OS Logon, we don't want that kind of shortcut, but we want the user to verify the correct PIN (even though the token was left unattended and authentication at the computer).
This essentially reverts commit e6f7373.
Checklist