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

Inform card driver of C_Login(CKU_CONTEXT_SPECIFIC) #1072

Closed
wants to merge 1 commit into from

Conversation

dengert
Copy link
Member

@dengert dengert commented Jun 18, 2017

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

@dengert
Copy link
Member Author

dengert commented Jun 23, 2017

I have updated this pull request by adding a sc_pkcs15_verify_pin_context_specific routine. This is called from pkcs11/framework-pkcs15.c
It can be squash'ed if accepted.

Here is an annotated opensc-debug.log using Thunderbird 52.1.1 64 bit on Ubuntu-16.4
context_specific.opensc-debug-20170623-0721.log.txt

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
5270:%%%% From previous C_GetSessionInfo and VERIFY Lc=0 commands we know user has 3 pin attempts left, and is not logged into card
5282:%%%% A VERIFY Lc=0 is sent
5289:%%%% VERIFY Lc=0 returns user is not logged in and has 3 tries left.
5305:%%%% C_Login to login to card starts
5355:%%%% C_Login to login in to card completes.
5357:%%%% C_GetSessionInfo starts, the first after the login
5371:%%%% card-piv.c starts it test to get login state from card
5382:%%%% A VERIFY Lc=0 is sent to card
5389:%%%% Card returns 90 00 saying user is logged in
5403:%%%% C_GetSessionInfo completes and the applications starts looking for objects.
7177:%%%% C_GetSessionInfo completes
7179:%%%% C_Login(CKU_CONTEXT_SPECIFIC) login starts
7184:%%%% pkcs11/framework-pkcs15.c detects it is a context specific login
7188:%%%% New routine, sc_pkcs15_verify_pin_context_specific is called. It will set sc_card_context_specific_login = 1
7211:%%%% the VERIFY of the pin is sent, to satisfy a "PIN Always" rule.
7237:%%%% Application calls C_GetSessionInfo after of C_Login(CKU_CONTEXT_SPECIFIC) and before C_SIGN
7251:%%%% sc_card_context_specific_login == 1 so the verify Lc=0 is not done, and the previous state is returned
7262:%%%% C_GetSessionInfo returns
7515:%%%% C_Sign(Final) was called (but no log message) to do a signature.
7638:%%%% The crypto sign operation has ended, and the sc_card_context_specific_login is reset to 0
7653:%%%% C_Sign returns 256 byte signature and CKR_OK
7713:%%%% VERIFY Lc=0 commands are now being done again because we are no longer concerned with "PIN Always" and user is still logged in.
doug@XUbuntu-16:/afs/anl.gov/appl/OpenSC-dev/build/opensc-git-my/amd64_linux26-1.1$

@dengert dengert force-pushed the context_specific branch 2 times, most recently from 04bd719 to 9f73e39 Compare June 28, 2017 21:57
@dengert
Copy link
Member Author

dengert commented Jun 29, 2017

The branch has been squashed into a single commit.

@frankmorgner, @Jakuje

I would really like to see this PR in 0.17.0 or 0.17.0-rc2 if there is one.

  • It adds one routine that is only called if C_Login(CKU_CONTEXT_SPECIFIC) is called.

  • it adds only one field to the sc_card structure, and a card driver can ignore it.

  • It solves a problem with the PIV and Thunderbird.

  • Thunderbird's uses of G_GetSessionInfo meets the PKCS#11 specifications, in that the call to C_Login(CKU_CONTEXT_SPECIFIC) is done after the C_SignInit. PKCS#11 does not say anything about what can be between the C_Login(CKU_CONTEXT_SPECIFIC) and the C_Sign.

  • The NIST-800-73 "PIN Always" rule was not designed to meet PKCS#11 standards.

  • So what the PR is doing is trying to satisfy both the G_GetSessionInfo request to get the card status when needed and the card's "PIN Always" rule for selected keys.

  • C_Login(CKU_CONTEXT_SPECIFIC) should only be called if a private key has CKA_ALWAYS_AUTHENTICATE=true. This is only done for a handful of cards, if they have PKCS#15 user_consent set.

  • This PR is in the spirit of trying to accommodate PKCS#11 applications which may or may not follow the strict specifications with cards that may have other requirements that do not match up well with PKCS#11.

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.

@@ -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"
Copy link
Member

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"

@Jakuje
Copy link
Member

Jakuje commented Jun 29, 2017

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.

@dengert
Copy link
Member Author

dengert commented Jun 29, 2017

@Jakuje
Thanks for testing. You said it works for you. Can you say what version for Thunderbird did you use and on what platform?

I will fix the comment. It should say "PIN Always" rule, which is what NIST calls it.

@Jakuje
Copy link
Member

Jakuje commented Jun 29, 2017

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).
The NSS used is 3.30.2. Without this commit it was failing to authenticate (as explained in the mozilla bug referenced earlier. With this commit it properly prompts for the pin and successfully completes the signatures.

@dengert
Copy link
Member Author

dengert commented Jun 29, 2017

Comment fixed and squashed.

@frankmorgner
Copy link
Member

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

dengert commented Jul 1, 2017

@frankmorgner, @viktorTarasov , @LudovicRousseau, @martinpaljak, @Jakuje

Frank and I are at a disagreement on how to solve the NSS issue #1071
Frank is proposing https://github.com/frankmorgner/OpenSC/tree/piv1
I am proposing this PR #1072

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 sc_card_t with code like sc_context_t *ctx = p15card->card->ctx; or sc_lock(card); If you want a I can create a sc_set_context_specific(card) and call that from sc_pkcs15_verify_pin_context_specific().

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..."
Looking at the opensc-debug.log. After opensc-tool does a SCardDisconnect Opensc-pkcs11 from Thunderbird does a SCardReconnect, finds SCARD_W_RESET_CARD then every attempt to do any other SCard command gets SCARD_E_PROTO_MISMATCH The protocol should have been T=1, so not clear what is going on.

@CardContact
Copy link
Member

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.

@martinpaljak
Copy link
Member

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:
The multi-app scenario, with apps that keep state (for PKCS#11) and a module that tries it (OpenSC) and cards that eventually establish truth, is a hairy one. To be honest, I regret a bit the use of SCardReconnect as my current experience with another piece of c++ code (the Pc/SC part of the app behind https://github.com/web-eid/web-eid.js) suggests that 99% of the time it would be easier and more reliable to establish a new fresh handle with SCardConnect and invalidate anything that is cached "higher up" if needed.

@dengert
Copy link
Member Author

dengert commented Jul 2, 2017

With further testing, the reconnect/reset problem appears to be some timing issue. Running Thunderbird -g to start gdb, it appears to not have the reset problem. Could be a thread issue.

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 disconnect_action = leave; things work fairly well as long as an interfering application does not do a card reset. (I have always favored using disconnect_action = leave, easier for the user.)

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

frankmorgner added a commit to frankmorgner/OpenSC that referenced this pull request Jul 2, 2017
@frankmorgner
Copy link
Member

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 sc_pin_cmd_data, I'm open for suggestions if it solves an actual problem and make things clearer from a developer's point of view. (If anyone has some spare time, why not evaluate all those little hacks if they are still needed or if it can be solved in a simpler way?)

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 card_reader_lock_obtained and recently there has been #850. All in all, that still needs some work. (Yet an other pain point someone could look at with a little more spare time...)

@dengert
Copy link
Member Author

dengert commented Jul 3, 2017

In regard to #1084 vs #1072. In #1084 after the verify of the PIN succeeds, all further requests for SC_PIN_CMD_GET_INFO will return the previous information. With #1072 a SC_PIN_CMD_GET_INFO request will only return the previous information between the verify and crypto operation when the verify was a context_specific operation. So without interference from other processes #1084 works as the user stays logged in. I do not consider this functionally equivalent. But is better then no change at all for 0.17.0

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 disconnect_action = leave; This avoids the card reset issues and allows the user to not have to type in the pin as often. I consider this is the way to run OpenSC in an environment where the user is running multiple concurrent applications.

Here is an opensc-debug.log of #1084 of thurnderbird sending 3 signed emails. Between the
second and third, in another window, opensc-tool --serial was run. You will note that the last verify Lc=0 is sent in line 5498. The first verify to login to the card is in line 5538, with the three other verify commands caused by C_Login(CKU_CONTEXT_SPECIFIC) at lines 6691, 10131 and 15306.
fm.piv.opensc-debug.log.txt

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.

@frankmorgner
Copy link
Member

OK, great. I created https://github.com/OpenSC/OpenSC/projects/3 to track the issue with resets.

frankmorgner added a commit that referenced this pull request Jul 4, 2017
frankmorgner added a commit to frankmorgner/OpenSC that referenced this pull request Jul 4, 2017
@dengert dengert deleted the context_specific branch May 29, 2018 16:26
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