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

Revert 1d5c81e85 "Add some more logic to decide if login is required" #2769

Closed
dengert opened this issue May 2, 2023 · 2 comments
Closed

Comments

@dengert
Copy link
Member

dengert commented May 2, 2023

Problem Description

The commit 1d5c81e affects every card and changes the logic such that slot->token_info.flags |= CKF_LOGIN_REQUIRED; is never set.

The commit test for "SC_PKCS15_TOKEN_LOGIN_REQUIRED" is only used in OpenSC in this commit and defined in pkcs15.h as #define SC_PKCS15_TOKEN_LOGIN_REQUIRED 0x02 /* Don't use */ with comment "Don't use" since 2010-10-05.

pkcs15.c:sc_pkcs15_parse_tokeninfo will read the flags for true pkcs15 cards:
sc_format_asn1_entry(asn1_toki_attrs + 5, &ti->flags, &flags_len, 0); (But they may not have SC_PKCS15_TOKEN_LOGIN_REQUIRED. set correctly.)

None of the emulated cards set SC_PKCS15_TOKEN_LOGIN_REQUIRED.

To see drivers that update some tokeninfo fields: grep "tokeninfo->" src/libopensc/pkcs15-*.

And drivers that update the flags:

$ grep "tokeninfo->flags" src/libopensc/pkcs15-*
pkcs15-cardos.c:			if (p15card->tokeninfo && p15card->tokeninfo->flags & SC_PKCS15_TOKEN_EID_COMPLIANT) {
pkcs15-esteid2018.c:	p15card->tokeninfo->flags = SC_PKCS15_TOKEN_READONLY;
pkcs15-esteid.c:	p15card->tokeninfo->flags = SC_PKCS15_TOKEN_PRN_GENERATION
pkcs15-nqApplet.c:		p15card->tokeninfo->flags = SC_PKCS15_TOKEN_READONLY;
pkcs15-oberthur.c:		p15card->tokeninfo->flags |= SC_PKCS15_TOKEN_PRN_GENERATION;
pkcs15-openpgp.c:	p15card->tokeninfo->flags = SC_PKCS15_TOKEN_PRN_GENERATION | SC_PKCS15_TOKEN_EID_COMPLIANT;
pkcs15-pteid.c:	p15card->tokeninfo->flags |= SC_PKCS15_TOKEN_PRN_GENERATION
pkcs15-skeid.c:	p15card->tokeninfo->flags = SC_PKCS15_TOKEN_PRN_GENERATION | SC_PKCS15_TOKEN_READONLY;
pkcs15-starcos-esign.c:		p15card->tokeninfo->flags = SC_PKCS15_TOKEN_READONLY;

With cards that have some objects (keys, data or certificates) that require a PIN and some that don't have been handled in past by in PKCS11 by searching for objects before doing C_Login and if not found then do C_Login and search again. (PIV is one case where Card auth key does not require a PIN).

Not having CKF_LOGIN_REQUIRED causes applications to assume there are no PINS and never attempt to login.

(If a specific card has a problem (, changes should be handled by pkcs15 -*.c or card-*.c or additional flags added which are only set by the card drivers.)

This was found while testing new OpenSSL provider latchset/pkcs11-provider#234 (comment)

I would also suggest that the other commits committed at the same time be reviewed closer.

Proposed Resolution

Revert the commit for now.

Add some other flag that set by card drivers so framework-pkcs15.c will not set CKF_LOGIN_REQUIRED.

OR

Look at setting SC_PKCS15_TOKEN_LOGIN_REQUIRED set in every card including the 15 emulated cards accept the Slovenian eID that can turn it off in its pkcs15-eoi.c

Steps to reproduce

With the commit ./pkcs11-tool -T with your favorite card will not have login required
Using older versions of OpenSC will have login required

Logs

@dengert
Copy link
Member Author

dengert commented May 2, 2023

Rather the reverting, this patch (tested with only a Yubikey 5 PIV) could be used:

SC_PKCS15_CARD_DONT_SET_CKF_LOGIN_REQUIRED.diff.txt

@frankmorgner
Copy link
Member

Done with 7b782d5

I think OpenSC's mapping to and handling of CKF_LOGIN_REQUIRED is correct. The desired behavior for eOID seems to be due to some PKCS#11 handling issue on the application side. I've suggested an eOID specific workaround and some more details in this discussion

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

No branches or pull requests

2 participants