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

Fixed PIN authentication bypass #2806

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Conversation

frankmorgner
Copy link
Member

@frankmorgner frankmorgner commented Jun 21, 2023

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
  • Documentation is added or updated
  • New files have a LGPL 2.1 license statement
  • PKCS#11 module is tested
  • Windows minidriver is tested
  • macOS tokend is tested

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.
@frankmorgner
Copy link
Member Author

@hhonkanen , you've introduced this initially for convenience with PIN pad readers, could you check whether this impacts your use cases?

@frankmorgner
Copy link
Member Author

frankmorgner commented Jun 21, 2023

To reproduce the problem, just do pkcs11-tool --login and then use the unlocked token with an empty PIN, e.g. in Firefox for client authentication. Even though Firefox requests the PIN, an empty PIN will be accepted. I used a Yubikey for testing, which currently doesn't support logout in OpenSC.

@hhonkanen
Copy link
Contributor

@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.

@dengert
Copy link
Member

dengert commented Jun 21, 2023

Also note that several Yubikey PIV tokens have problems with login state. Grep for:
/* card_issues - bugs in PIV implementations requires special handling */ in card-piv.c.
The grep for CI_VERIFY_LC0_FAIL to see what versions of yubikey have problem with testing login state.
Other problems with these cards is selecting the wrong AID can reset the login state.

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)

@dengert
Copy link
Member

dengert commented Jun 21, 2023

@hhonkanen do you disconnect = leave; in opensc.conf? That was designed to allow card to stay logged in so multiple processes could use the card. even without pin caching. In my tests I used "leave" and pin caching off and certificate caching off.

@Jakuje
Copy link
Member

Jakuje commented Jun 21, 2023

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

  • when I enter empty PIN in chromium, I do not see corresponding PKCS#11 call from NSS/chromium
  • Just using pkcs11-tool with --login does not issue C_Login for zero-length input (which is sane, but something that we should not depend on).
  • If I modify the pkcs11-tool to send the zero-length pins, I am getting CKR_PIN_LEN_RANGE (makes sense -- there are lenght limits for PINs but did not check if before this change or after).
  • I believe the calling application would have to send the empty Login and ignore its results which is already two NO-NO. But even after modifying the pkcs11 too to do so I am still getting the objects listing without private keys and the attempts to sign leads to the "private key not found" errors so I assume there must be some missing piece to make this working with the pkcs11 (if even possible).

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).

@hhonkanen
Copy link
Contributor

@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?

@Jakuje
Copy link
Member

Jakuje commented Jun 22, 2023

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.

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.

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.

AFAIK the zero-length pin is provided by the software every time when there is a pinpad reader.

Could anybody test doing several PKCS#11 operations (like C_Sign / C_Decrypt / C_Wrap / C_UnWrap) using 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.

@hhonkanen
Copy link
Contributor

AFAIK the zero-length pin is provided by the software every time when there is a pinpad reader.

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.

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.

Here is a test program:
https://github.com/hhonkanen/WrapTest

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.

@frankmorgner
Copy link
Member Author

frankmorgner commented Jun 22, 2023

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.

disconnect_action=reset could solve the issue sometimes, but that is neither the default configuration, nor does it cover all of the problems with the PIN bypass. Consequently, we should not rely on this for solving the PIN issue.

Yes, you could check for pinlen==0 in the application (which was my first try of solving frankmorgner/OpenSCToken#50 and which is what pkcs11-tool does), but as stated above, Firefox does not prevent the use of C_Login with an empty PIN and the same is true for macOS logon/screen saver (via CTK).

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.

@dengert
Copy link
Member

dengert commented Jun 24, 2023

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.

I believe that is correct.

PKCS11-base-V3.0-os says:

3198 CKF_PROTECTED_AUTHENTICATION_PATH flag in its CK_TOKEN_INFO being set, then that means
3199 that there is some way for a user to be authenticated to the token without having to send a PIN through
3200 the Cryptoki library. One such possibility is that the user enters a PIN on a PIN pad on the token itself, or
3201 on the slot device. Or the user might not even use a PIN—authentication could be achieved by some
3202 fingerprint-reading device, for example. To log into a token with a protected authentication path, the pPin
3203 parameter to C_Login should be NULL_PTR. When C_Login returns, whatever authentication method 
3204 supported by the token will have been performed; a return value of CKR_OK means that the user was 
3205 successfully authenticated, and a return value of CKR_PIN_INCORRECT means that the user was 
3206 denied access. 

Some thing to check:
OpenSC is not setting CKF_PROTECTED_AUTHENTICATION_PATH when PIN PAD reader is detected.

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 opensc.conf option to accommodate a bad application?

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.)

Copy link
Member

@Jakuje Jakuje left a 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.

@frankmorgner frankmorgner merged commit 868f76f into OpenSC:master Jun 29, 2023
30 of 32 checks passed
@xhanulik xhanulik mentioned this pull request Jul 3, 2023
@martinpaljak
Copy link
Member

martinpaljak commented Jul 5, 2023

OpenSC is not setting CKF_PROTECTED_AUTHENTICATION_PATH when PIN PAD reader is detected.

I think it is?

slot->token_info.flags |= CKF_PROTECTED_AUTHENTICATION_PATH;

The application may not check for CKF_PROTECTED_AUTHENTICATION_PATH and issue prompt for a PIN.

Yes

If its an application issue do we need some opensc.conf option to accommodate a bad application?

Yes

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?

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.

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.

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)

@dengert
Copy link
Member

dengert commented Oct 19, 2023

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 iso7816.c or other program can call sc_build_pin with a zero length pin.

Variable i in initialized to 0. depending on type of padding it looks like i could endup as zero. sc_build_pin returns i = 0.

iso7816_build_pin_apdu in case: SC_PIN_CMD_VERIFY calls r = sc_build_pin and sets len = r; i.e. zero and breaks.

        sc_format_apdu(card, apdu, cse, ins, p1, data->pin_reference);
        apdu->lc = len;
        apdu->datalen = len;
        apdu->data = buf;
        apdu->resplen = 0;

will endup with APDU with Lc=0 with cse=SC_APDU_CASE_3_SHORT
The only difference in this APDU vs a SC_PIN_CMD_GET_INFO type is the cse=

iso7816_pin_cmd will endup calling r = sc_transmit_apdu(card, apdu);

reader-pcsc.c never tests the cse. so it looks like a request of zero len pin is not a verify but check the login stat apdu. which returns 90 00 if the login state of card is still valid.

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?

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

Successfully merging this pull request may close these issues.

None yet

5 participants