-
Notifications
You must be signed in to change notification settings - Fork 711
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
Inform card driver of C_Login(CKU_CONTEXT_SPECIFIC) #1072
Conversation
I have updated this pull request by adding a sc_pkcs15_verify_pin_context_specific routine. This is called from pkcs11/framework-pkcs15.c Here is an annotated opensc-debug.log using Thunderbird 52.1.1 64 bit on Ubuntu-16.4 Here is a list of the comments and line numbers that all start with %%%% 5254:%%%% A C_GetSessionInfo is started before the C_Login to login to the card |
04bd719
to
9f73e39
Compare
The branch has been squashed into a single commit. I would really like to see this PR in 0.17.0 or 0.17.0-rc2 if there is one.
I first submitted CKA_ALWAYS_AUTHENTICATE mods to Mozilla in 2011 starting with this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=357025#c41 And then got OpenSC to also support CKA_ALWAYS_AUTHENTICATE, Thunderbird was working to sign E-Mail. This took additional code in OpenSC card-piv.c to make sure it had cached in memory the all the certificates from the card to avoid having to read a certificate between C_Login(CKU_CONTEXT_SPECIFIC) and C_Sign to accommodate any other place where Thunderbird was following PKCS#11 but causing teh "PIN Always" rule to be violated. The Mozilla bug was finally resolved in 2012: https://bugzilla.mozilla.org/show_bug.cgi?id=357025#c55 I would like to get Thunderbird running again and the PR does that by not issuing the VERIFY Lc=0 that is part of the SC_PIN_CMD_GET_INFO. But only when the code is trying to satisfy a "PIN Always" rule enforced by the card. Turing off the SC_CARD_CAP_ISO7816_PIN_INFO as you have suggested cause other problems that were fixed by the SC_PIN_CMD_GET_INFO. I would not like to see that happen either. A lot of people spent a lot of time (11 years) with Thunderbird I would like to get this working again with Thunderbird and any other application that may have started failing since SC_CARD_CAP_ISO7816_PIN_INFO code was introduced. |
src/libopensc/card-piv.c
Outdated
@@ -3274,6 +3280,16 @@ piv_pin_cmd(sc_card_t *card, struct sc_pin_cmd_data *data, int *tries_left) | |||
data->pin1.tries_left = priv->tries_left; | |||
if (tries_left) | |||
*tries_left = priv->tries_left; | |||
/* | |||
* if a context specific login was done, don't try and test | |||
* login state as this will violate PIV "always authnticate" |
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.
There is a typo "authnticate" -> "authenticate"
Except the typo in the previous comment, the solution looks good to me and having it in 0.17.0 would be good. I tested with NSS and the PIV test card and it is solving the problem. |
@Jakuje I will fix the comment. It should say "PIN Always" rule, which is what NIST calls it. |
To clarify, I was not testing with Thunderbird, but with testsuite written using NSS trying the signatures using all of the keys and all of the mechanisms supported (I am not the author of this so I can not publish it). |
9f73e39
to
07454b9
Compare
Comment fixed and squashed. |
As stated above, I'd like to avoid pulling the PKCS#11 context into the PKCS#15 or the driver level. I've found a problem with my patch above. @dengert could you try again with https://github.com/frankmorgner/OpenSC/tree/piv1? |
Add new routine sc_pkcs15_verify_pin_context_specific. It is intended to be used by pkcs11/framework-pkcs15.c or other PKCS#15 routines to tell card driver when a C_login(CKU_CONTEXT_SPECIFIC) login has started. A card driver can then use and reset this flag as needed or ignore it. Many PKCS#11 applications, Thunderbird for example, call C_GetSessionInfo between C_login(CKU_CONTEXT_SPECIFIC) and C_Sign. This is allowed by PKCS#11, but may cause problems if the card enforces some "PIN Always" rule which requires no commands between a VERIFY and a crypto command to use the private key. A "PIN Always" rule is used by the NIST 800-73 PIV card. OpenSC also tries to query the card to determine its login state when C_GetSessinInfo is called. A VERIFY PIN command without a PIN is is a common way to test the login state of a card. Currently, card-piv.c is the only card that needs the commit. card-piv.c will not send VERIFY Lc=0 command to the card while sc_pkcs15_context_specific is set and after a crypto operation card-piv.c will reset sc_pkcs15_context_specific. While sc_pkcs15_context_specific is set card-piv.c return the previous login state. Thus following the "PIN Always" rule and still providing the application with a login state as best it can. Date: Sun Jun 18 17:59:29 2017 -0500 Changes to be committed: modified: src/libopensc/card-piv.c modified: src/libopensc/libopensc.exports modified: src/libopensc/opensc.h modified: src/libopensc/pkcs15-pin.c modified: src/libopensc/pkcs15.h modified: src/pkcs11/framework-pkcs15.c
@frankmorgner, @viktorTarasov , @LudovicRousseau, @martinpaljak, @Jakuje Frank and I are at a disagreement on how to solve the NSS issue #1071 I am looking for you input on how OpenSC should proceed. Frank's patch in effect turns off the the functionality of the SC_PIN_CMD_GET_INFO to test the login state of the card after the first SC_PIN_CMD_GET_INFO command has set SC_PIN_STATE_LOGGED_IN. From then on it it always returns the same state. My patch does something similar, but only returns the previous result if the SC_PIN_CMD_GET_INFO is called between a C_Login(CKU_CONTEXT_SPECIFIC) and and the C_Sign. But OpenSC must tell the card driver C_Login(CKU_CONTEXT_SPECIFIC) has been called. Frank said: " I'd like to avoid pulling the PKCS#11 context into the PKCS#15 or the driver level." But my patch (both previous of current) does not pull the PKCS#11 context into the PKCS#15 or the driver level. framework-pkcs11.c calls the new sc_pkcs15_verify_pin_context_specific(). sc_pkcs15_verify_pin_context_specific() then card->sc_card_context_specific_login = 1;. Note most if not all sc_pkcs15 routines access I contend that the card driver needs to know when the caller is in a context_specific login to preserve the functionality of the SC_PIN_CMD_GET_INFO for long running process like e-mail, browsers or system login processes. Other processes or applications may reset the logins state. That is why these processes call C_GetSessionInfo to see if the login state of the card has changed or card reset or removed. Both Frank's patch and my patch work in the simple case of signing one or two emails using Thunderbird when there is no interference from other applications. This is an improvement in that TB can sign mail. The following may be a new bug, and I will look closer at it. It does not appear to be related to either Frank's or my mods. While trying with both Frank's and my patch, after sending one or two signed emails, then running opensc-tool --serial in another window, then trying to send another signed email, I get a popup from Thunderbird saying:" Send of the message failed. Unable to sign message..." |
I agree with Frank, that we should not pollute the p15 layer with stuff from the layers above. However, that is also true for the session PIN mechanism used by the minidriver. Why don't we create a generic mechanism to pass additional login information all the way down into the card driver (e.g. as part of sc_pin_cmd_data) ? Most drivers can ignore the additional information silently, but the piv driver can use the 'CKU_CONTEXT_SPECIFIC' information to disable subsequent card commands. I can imaging other authentication mechanisms (like biometric match or public key authentication) that would benefit from such a mechanism. OpenSC is already full of quick hacks that make it very difficult to oversee all the side effects in the code. At the end nobody remembers why a special method or flags was introduced. |
Well, as PKCS#15 does not have the term "context specific pin" I kind of fall towards the comment from Frank, that it is a bit of a "pollution". But then again, there is a bunch of things related to Windows in pkcs15 layer, so it doesn't really matter, practicality beats purity sometimes, if carefully evaluated. Obviously it is also a good example of different specifications (PKCS#11 vs PIV APDU interface) not fitting together in a multi-card, multi-app environment. But, the fact that there are other calls that result in APDU traffic, should in ideal world be dealt with by the card specific driver, if possible. Maybe it is not spelled out that clearly which internal API-s are intended to be using stateful/cached information and which are safe to generate card traffic? Maybe it is a bug for NSS instead, to not call the session/token related methods? Regarding the reconnect/reset: |
With further testing, the reconnect/reset problem appears to be some timing issue. Running I have been using Ubuntu 16.04 with pcscd 1.8.14-1ubuntu.16.04. Could be the card or reader has not completed the reset, before the first command after the reconnect is issued. causing 0x8010000f error. But if opensc.conf has If someone wants to write what @CardContact described, that would be OK with me. I have spent way to much time on this and the best I can tell @frankmorgner is not going to add my mod to 0.17.0 anyway. So @frankmorgner do what every you want for 0.17.0 |
So if I understood correctly, then #1084 equally solves the problem for a PIV key with "Always Authenticate". And it is functionally equivalent with this PR. And it solves the problem with minimal effort (two lines of code in a single file). If you agree, @dengert I'll merge #1084 for 0.17.0. Yes, there have been many hacks in the past, but for the future we should try avoiding these mistakes instead of adding new hacks. Hence, regarding the extended Correctly handling card resets is not a new problem. From a security point of view, resetting the login state (i.e. the card) is the right way to do, I think. However, we need a better handling of a resetted card. We have |
In regard to #1084 vs #1072. In #1084 after the verify of the PIN succeeds, all further requests for I regards to an "extended sc_pin_cmd_data" yes, I would like to see that so the functionality of #1072 could be implemented in a future release. In my testing I used: in opensc.conf Here is an opensc-debug.log of #1084 of thurnderbird sending 3 signed emails. Between the With the "card reset problem" this is the first time I have seen a 0x8010000f failure. I don't know if this is a reader, card or pcsc problem. But it appears to be timing problem. This can wait till after 0.17.0. |
OK, great. I created https://github.com/OpenSC/OpenSC/projects/3 to track the issue with resets. |
A fix for issue #1071
framework-pkcs15.c sets card->sc_card_context_specific_login
if C_Login(CKU_CONTEXT_SPECIFIC) is done.
Card drivers can ignore or use and reset this information.
card-piv.c will use this to not send VERIFY Lc=0 command to the card till
a crypto operation is done. NIST 800 SIGN key is "Always Authenticate"
and enforced on card which requires VERIFY PIN immediately before
the crypto operation using this key.
Thunderbird and other applications that call C_GetSessionInfo between
C_Login and C_Sign were causing OpenSC to test status of the card i
using VERIFY Lc=0 which card treated this as a violation of the
"Always Authenticate".
Changes to be committed:
modified: card-piv.c
modified: opensc.h
modified: ../pkcs11/framework-pkcs15.c