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

Asking for user PIN when already logged in. #2903

Closed
larssilven opened this issue Oct 17, 2023 · 26 comments · Fixed by #2916
Closed

Asking for user PIN when already logged in. #2903

larssilven opened this issue Oct 17, 2023 · 26 comments · Fixed by #2916

Comments

@larssilven
Copy link
Contributor

larssilven commented Oct 17, 2023

Problem Description

0.24.0-rc1 asks for for PIN all the time regardless if the user is logged in or not. According to PKCS#11 PIN must only be provided to the token after C_Login.

I am using a PIN pad reader so I recognizes all logins.

Version 0.23.0 is working as expected. master branch and 0.24.0-rc1 has this problem.

Proposed Resolution

If this new behavior is wanted for some application it must be enabled by the configuration.
Default must be to have it working according to PKCS#11 (as in version 0.23.0).

Steps to reproduce

Just run any application that has "private" objects and you will have to provide PIN all the time.

Logs

Logs from a test that show what is happening. The name of the file tells opensc log level (_llevel) and version of opensc (_version.log).
Here are the files that have been produced

PIN is provided at these lines in pksc11_llevel_0.24.0-rc1.log (yes it was the same lines for both levels):
60 89 234 308 383 693 905 1237 1836 2319 2392 2423

In pkcs11_l8_0.23.0.log PIN is just provided 2 times each time after C_Login: 60 1237

In the opensc logs you can see when PIN was provided (all lines with "sc_pin_cmd"). It was done 32 times for 0.24.0-rc1. So some pkcs#11 functions generates several PIN authentications.

C_Login was just called 2 times so it should just have been 2 PIN authentications.

When not using PIN pad everything is working as expected if PIN caching is enabled. PIN is only provided by the application 2 times by using C_Login:

But if PIN caching is not enabled CKR_GENERAL_ERROR is returned:

@dengert
Copy link
Member

dengert commented Oct 17, 2023

To see pin operations in OpenSC debug log requires debug = 8 or greater. This is done to not expose a user's pin in the log. But since you are using a pin pad reader The pin is sent from reader to card and OpenSC software never sees the pin.

P:743636; T:0x140378281910272 19:50:11.077 [opensc-pkcs11] sec.c:203:sc_pin_cmd: called
P:743636; T:0x140378281910272 19:51:44.419 [opensc-pkcs11] sec.c:259:sc_pin_cmd: returning with: 0 (Success)

Shows it took 93 seconds to login to the card after first C_Login.

Can you get a debug log with debug = 8?
Can you get a debug log with debug = 8 using 0.23.0 too?

@Jakuje
Copy link
Member

Jakuje commented Oct 18, 2023

Is there some other application that might be accessing the PKCS#11 token in the meantime?

On the first sight, this sounds like related to 56d1abe but this was already part of 0.23.0 and the logout is called also only twice.

The other guess would be file caching, which was enabled in 0.24 and which interfered the card enrollment in the past. This should have been solved with #2798, but there might be still some corner case. The log says the card is MyEID 4.0.18, but I think we implemented this functionality only around the 4.5 version (not sure what are the differences here though).

Can you try with file caching disabled by setting use_file_caching=no; in framework pkcs15 block of opensc.conf?

@popovec @hhonkanen any thoughts about the older myeid cards and features supported?

@popovec
Copy link
Member

popovec commented Oct 18, 2023

I'll look into it, I don't think it has anything to do with the logout function already implemented in 0.23 and it has nothing to do with the file cache either.

Recently, however, I noticed repeated PIN requests when using MyEID with openvpn. But I have to check the version of opensc used in this solution. I have a suspicion that this could also happen to me when I upgraded opensc to a version from GIT. It certainly didn't happen with 0.23.

MyEID can be configured so that it automatically performs a logout after a security operation.
(man pkcs15-init, --user-consent)

@larssilven
Copy link
Contributor Author

The starting comment has now been update with more request after the request from @dengert

@frankmorgner frankmorgner added this to In progress in OpenSC 0.24.0 Oct 18, 2023
@dengert
Copy link
Member

dengert commented Oct 18, 2023

@popovec It could by 868f76f which essentially reverts commit e6f7373

@mouse07410
Copy link
Contributor

Yes to @dengert .
In addition, I think at least some of the fault belongs to the change that makes the software enforce ALWAYS_AUTHENTICATE instead of letting the card inform the middleware whether the requested operation requires new login.

My fork places the trust on the card: it the card requires a new login - software prompts for the PIN, otherwise it doesn't.

@dengert
Copy link
Member

dengert commented Oct 18, 2023

e6f7373 tested for if (pinlen == 0) { It should then test if a PIN _PAD reader. If not it should return error as a zero length pin is not valid. If PIN_PAD reader, it should test for the SC_PIN_STATE_LOGGED_IN as it did before 868f76f

@frankmorgner and I disagree on how to handle multiple processes each running their own PKCS11 module.

@frankmorgner has taken a more restrictive approach which is force login per process and logout after last PKCS11 session is closed. This issue is very similar.

I take the approach if the user logs in to the card on one process, that login state should remain as long as user is logged in to OS. User is expected to remove card when they are done. Logout or lock screen need to be addressed which looks like RedHat and PCSC-lite have addressed by using PolKit. Ubuntu appears to not build PCSC-lite with PolKit.

@larssilven
Copy link
Contributor Author

larssilven commented Oct 18, 2023

@Jakuje : No difference with use_file_caching=false; in framework pkcs15 { .

@Jakuje
Copy link
Member

Jakuje commented Oct 18, 2023

@popovec It could by 868f76f which essentially reverts commit e6f7373

Good point. This is indeed the place where the code goes in different direction in the both debug logs.
This means we finally figured out why this code was in there :) So it looks like the #2806 will need some more work to get done right.

But I think this is still related only to the enrollment phase. If I see right, the first diff comes from myeid_create_key() through sc_pkcs15init_create_file() to sc_pkcs15init_authenticate() which figures out ACL for file creation needs a PIN and it ends up asking for that on pinpad instead of checking the login state with the card. This was already partially discussed in the #2806.

I would be interested in the behavior with normal reader without pinpad. There is no way to prompt the user for the PIN interactively from OpenSC internals so what does it do? Just skips the login and work ok? @larssilven can you check (and provide debug logs if you have some throw-away pins to share)? if you do not have non-pinpad reader, you can try with enable_pinpad=false in reader_driver pcsc block of opensc.conf.

So there are ways out:

  • make sure the pinpad is handled in iso7816_pin_cmd(). I think this should do, but I do not have the cards at me now so I can not test anything.
  • return the 868f76f but make sure to handle the pinpad somehow specially and handle the PIV even more specially.

On a side note with 0.24.0 I see that for every file read, the MyEID driver calls the myeid_get_info() which basically reads other file intead. I think it will be always faster than reading potentially large files, but I am thinking if this could be cached between the calls we know that do not modify the state, what do you think @popovec ? I will probably open a separate issue for this.

@larssilven
Copy link
Contributor Author

@popovec ,
When not using PIN pad everything is working as expected. PIN is only provided by the application 2 times by using C_Login:

@popovec
Copy link
Member

popovec commented Oct 19, 2023

On a side note with 0.24.0 I see that for every file read, the MyEID driver calls the myeid_get_info() which basically reads other file intead. I think it will be always faster than reading potentially large files, but I am thinking if this could be cached between the calls we know that do not modify the state, what do you think @popovec ? I will probably open a separate issue for this.

MyEID uses the myeid_get_info() function to determine if the cache is usable. If using myeid_get_info() it finds the same "change counter" value on the card as it is recorded in the cache, it is obvious that the file does not have to be read from the card, but it is enough to use the file from the cache. Technically, this is a GET DATA operation and this should not change the login status of the card. Apparently, this is not even happening, as even turning off the file cache will not fix this issue.

It also failed to repeat the problem I observed with openvpn with OpenSC version 0.24-rc1.. Everything works as I expected.

Since the problem occurs only with an external PIN pad, I would look for this place. I'll try to reproduce it somehow, but I don't have a reader with an external PIN pad at hand at the moment.

@frankmorgner
Copy link
Member

The CCID Emulator is capable of emulating a reader with PIN pad, but it may be a little complex to set up...

@larssilven
Copy link
Contributor Author

@popovec ,
You could disable PIN caching. Then you will get a CKR_GENERAL_ERROR first time that PIN is needed after C_Login:

@Jakuje
Copy link
Member

Jakuje commented Oct 19, 2023

I thought the pin cache is disabled by default. My bad.
I see that without the pinpad the cached pin is passed to the card on the places you talked about in the description where you would be prompted for the pin so this is not limited to the pinpad readers, but really to the 868f76f which needs to come back, but should probably be skipped on the explicit C_Login() attempts requested by the application/user.

@popovec
Copy link
Member

popovec commented Oct 19, 2023

The problem with CKR_GENERAL_ERROR is caused by some logical error during pin verification. It seems that we really have a problem if the PIN length is 0, and this occurs when the pin cache is disabled.

OPENSC_DEBUG=255 pkcs11-tool -k --key-type RSA:1024 -l --pin 11111111

P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:4043:sc_pkcs15init_authenticate: verify acl(method:1,reference:1)
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:3855:sc_pkcs15init_verify_secret: called
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:3859:sc_pkcs15init_verify_secret: get and verify PIN('PIN',type:0x1,reference:0x1)
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:2327:sc_pkcs15init_get_pin_reference: called
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:2336:sc_pkcs15init_get_pin_reference: found 2 auth objects; looking for AUTH object(auth_method:1,referen
ce:1)
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:2342:sc_pkcs15init_get_pin_reference: check PIN(Security Officer PIN,auth_method:1,type:1,reference:3,fla
gs:B0)
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:2342:sc_pkcs15init_get_pin_reference: check PIN(,auth_method:1,type:1,reference:1,flags:30)
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:2347:sc_pkcs15init_get_pin_reference: returning with: 1
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:3879:sc_pkcs15init_verify_secret: found PIN reference 1
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:3904:sc_pkcs15init_verify_secret: found PIN object ''
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:3908:sc_pkcs15init_verify_secret: PIN object ''; pin_obj->content.len:0
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-pin.c:304:sc_pkcs15_verify_pin: called
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-pin.c:313:sc_pkcs15_verify_pin: returning with: -1304 (Invalid PIN length)
P:676123; T:0x140203908130880 10:06:25.765 [opensc-pkcs11] pkcs15-lib.c:3962:sc_pkcs15init_verify_secret: Cannot validate pkcs15 PIN: -1304 (Invalid PIN length)
P:676123; T:0x140203908130880 10:06:25.766 [opensc-pkcs11] pkcs15-lib.c:4049:sc_pkcs15init_authenticate: returning with: -1304 (Invalid PIN length)

@hhonkanen
Copy link
Contributor

@Jakuje: The key wrapping/unwrapping features were developed with MyEID 4.0.18 and 4.0.19 and should work with these versions. These versions were not widely distributed, 4.5.5 replaced them soon.

This is a familiar issue which I resolved in e6f7373 when developing these features, but has appeared again with later changes and has been discussed in #2806. In e6f7373 my intention was to check the authentication state from card, when ending up to sc_pkcs15_verify_pin in situations the PIN has already been verified. I have understood a reason for later changes was that explicitly calling C_Login with a zero length PIN also leads here, and in this case it does not behave correctly.

My thoughts on maintaining the login state:

  • it is often desired behavior that different processes can share the authentication state, and allow authenticating to several applications with a singe PIN entry.
  • if this is not desired, a key can be marked with CKA_ALWAYS_AUTHENTICATE / user consent to invalidate the associated PIN, or an application can use SCARD_SHARE_EXCLUSIVE in PS/SC / pcsclite level and reset the card in the end of the session. This can be configured with opensc.conf options I think.
  • in this particular issue, I think there is just on process and the problem is not about sharing authentication state between processes. It looks more like the logic with pin caching disabled and pin pads is broken.

Could this issue be fixed by intercepting C_Login calls with zero length PIN or NULL pin without pinpad in higher level and returning the check introduced in e6f7373?

@larssilven
Copy link
Contributor Author

larssilven commented Oct 19, 2023

I did this on my clone of the master branch:
git rebase --onto=868f76fb31255fd3fdacfc3e476452efeb61c3e7~1 868f76fb31255fd3fdacfc3e476452efeb61c3e7 master
And now it works as it should:

  • If providing PIN to C_Login the reader is not asking for PIN at all and no fault later.
  • If nullptr is provided to C_Login then the reader asks for PIN but just after each C_Login and no faults later.

You can clone it from: [email protected]:larssilven/OpenSC.git
I'm using use_file_caching = false and use_pin_caching = false

@Jakuje
Copy link
Member

Jakuje commented Oct 23, 2023

I did this on my clone of the master branch: git rebase --onto=868f76fb31255fd3fdacfc3e476452efeb61c3e7~1 868f76fb31255fd3fdacfc3e476452efeb61c3e7 master.

Rebasing with commit removal back in the history is confusing. I would propose rather revert the commit 868f76fb31255fd3fdacfc3e476452efeb61c3e7 on top of the master. This way the branches are diverged and hard to compare.

And now it works as it should:

* If providing PIN to C_Login the reader is not asking for PIN at all and no fault later.

* If nullptr is provided to C_Login then the reader asks for PIN but just after each C_Login and no faults later.

You can clone it from: [email protected]:larssilven/OpenSC.git I'm using use_file_caching = false and use_pin_caching = false

Thanks for verifying the 868f76f is indeed the culprit as we assumed. It will need to go back in some form.

I think the file caching nor pin caching should have any effects here (but feel free to double-check).

@Jakuje
Copy link
Member

Jakuje commented Oct 24, 2023

I am wondering if we should not change this actually somewhere inside of the src/pkcs15init/pkcs15-lib.c's sc_pkcs15init_verify_secret(), which is, I think the path the myeid card is using to verify the ACLs even before going into the sc_pkcs15_verify_pin(), where all the other changes are being proposed and which affects much more code paths than we would like. My proposal would be the following change to address this particular issue:

https://github.com/Jakuje/OpenSC/commits/bypass-pkcs15init

@larssilven can you try with this branch that it does what it is supposed to do? I have pinpad reader, but I learned that the MyEID card I have at work died so I will have to dig up some more tomorrow.

@hhonkanen
Copy link
Contributor

hhonkanen commented Oct 24, 2023

Haven't had time to debug it, but I was thinking of checking for zero length PIN in pkcs15_login function in framework_pkcs15.c, before calling sc_pkcs15_verify_pin(), and return the check introduced in e6f7373 to sc_pkcs15_verify_pin. In pkcs15_login, we would return an error if C_Login is called with an empty PIN, but when we end up to sc_pkcs15_verify_pin when accessing a private object, it would check the authentication state and avoid unnecessary PIN verifications.

@Jakuje your suggestion might do the job to resolve the issue as well. I'm just not sure, do we get to sc_pkcs15_verify_pin with pinlen=0 from other code paths than sc_pkcs15init_verify_secret and pkcs15_login (which my suggestion would handle).

@dengert
Copy link
Member

dengert commented Oct 24, 2023

PKCS11 C_Login with pinlen = 0 is used for more then PIN PAD readers. https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/pkcs15-pin.c#L275-L279 SC_CARD_CAP_PROTECTED_AUTHENTICATION_PATH is used in pkcs15-sc-hsm.c and card-sc-hsm.c.

@frankmorgner
Copy link
Member

@hhonkanen , see https://github.com/OpenSC/OpenSC/wiki/CVE-2023-40660 for what is wrong with e6f7373

In PKCS#11, we have slot->login_user which we can leverage to avoid unneeded reauthentication. If the user has authenticated before in the running process, then, I agree, she is free to use the card if it is authenticated to avoid a pin pad prompt. And I think we are already doing this independent from all changes discussed here:

if (slot->login_user >= 0) {
if ((CK_USER_TYPE) slot->login_user == userType)
rv = CKR_USER_ALREADY_LOGGED_IN;
else
rv = CKR_USER_ANOTHER_ALREADY_LOGGED_IN;
goto out;
}

Thanks @Jakuje , for analyzing the given log, which reveals that sc_pkcs15init_verify_secret is causing the repeated PIN prompting. Indeed, Jakuje@5ea4bdf, solves this exclusively in the pkcs15layer since, luckily, sc_pkcs15init_verify_secret is not used by framework-pkcs15init.c. However, I have some fear that handling this in the pkcs15init library may eventually trickle through to PKCS#11 at some point. Ideally, pkcs15-init (the executable) should keep some global state on who has authenticated previously, similar to slot->login_user in the case of the PKCS#11 context. On the other hand, we need to sort this out quickly to finally publish the release, so I would be fine with your proposed workaround.

@frankmorgner
Copy link
Member

SC_CARD_CAP_PROTECTED_AUTHENTICATION_PATH is basically a PIN pad on the card itself (sc-hsm even supports an integrated finger print reader). Handling from the PC side should be identical to a pin pad reader, though.

@Jakuje
Copy link
Member

Jakuje commented Oct 24, 2023

Note, that the reproducer from #2903 (comment) does not go through the pkcs15-init nor pkcs15-tool, but the code is called from the pkcs11 module by pkcs11-tool directly to generate keys so state tracking in other tools would not help. But it looks like this issue is with any object that might have ACLs even for reading, but @larssilven did not provide the myeid configuration he used nor commands, but from the pkcs11 trace I think just the keygen should do.

I have finally working MyEID card (@hhonkanen your shipping service is awesomely fast!), and I see my fix does not actually work, but I hope the concept and direction I wanted to point is clear. I updated the branch with tested code with MyEID card:

https://github.com/Jakuje/OpenSC/commits/bypass-pkcs15init

@larssilven
Copy link
Contributor Author

@Jakuje : I used this script to initialize the card:

/#!/bin/bash

failing() {
	echo "Command failed. Card not initialized."
	exit 1
}

sopin=12345
sopuk=23456789
pin=4321
puk=54321
#set -x
echo ${sopin}| pkcs15-init -E || failing
pkcs15-init -C --so-pin "${sopin}" --pin "" --so-puk "${sopuk}" --puk "" || failing
pkcs15-init -P -a 1 -l "${label}" --pin "${pin}" --puk "${puk}" --so-pin "notUsed" || failing
pkcs15-init -F || failing

echo "Smart card initialized."

Then I just run a test application that produced the logs. Note that it not just C_GenerateKeyPair that asks for PIN. In the problem description above each line number in the PKCS#11 log where PIN must be provided is listed.

I have not yet tested the fix that @Jakuje presented 2 days ago. Is it still of any value to test it? Please let me know if I should test anything else.

@Jakuje
Copy link
Member

Jakuje commented Oct 26, 2023

Thank you. Testing the changes would be still useful. They are now in #2916. I do not think there was any other proposal that could fix this issue in the meantime, except for the discussion in the above PR.

OpenSC 0.24.0 automation moved this from In progress to Done Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

7 participants