PIV - Improved Card Matching for Dual CAC/PIV and PIVKEY cards. #1549

dengert · 2018-12-05T20:53:12Z

Not all PIV applets are the same. Different versions of NIST 800-73 and improperly implemented
or not implemented required features of NIST 800-73 cases problems. Have a look at the card_issues
listed in card-piv.c. The PIV driver has tried to detect the differences based on clues found in
the ATR historical bytes and vendor version numbers for some cards.

At the same time it has tried to support the possibility there are multiple applets
on a card that the user may want to use at the same time from different applications.
This has lead to some detection problems with Dual CAC/PIV cards. The same cards
sold by the vendor may have only a PIV applet that may not be the same PIV applet that
is on the Dual PIV/CAC cards.

http://www.cac.mil/Portals/53/Documents/CAC-utilziation-and-variation-matrix-v2.03-20May2016.doc
defines a number of official CAC cards in active service. A table of the ATRs for these is now used
to detect these cards. The PIV version of the CCC is also read to see if any CAC PKI objects
are defined in the CCC, indicating it is a Dual CAC/PIV, even if the ATR is not listed.

A more conservative approach to try and handle multiple applets on a card is used. Based
on issues with the implementation of the PIV applet this may not be possible to do.
So for many cards no additional detection will be done at the start of every transaction,
and the login state can not be detected correctly.

ATRs for PIVKEY are also in the match table, as these cards have a lot of issues.

Other PIV cards in the future or not yet tested may not be covered properly by this patch.
Extra debugging was added with "PIV_MATCH" to help with these other cards.
With "debug = 7;", grep PIV_MATCH opensc-debug.log can be used to see how a card
type and card_issues are derived.

On branch piv-improved-matching
Changes to be committed:
modified: card-piv.c
modified: cards.h

Tested using NIST demo cards, Oberthur cards that have a CCC that lists CAC in CCC F0,
Oberthur PIV card with a CCC copied from a Dual CAC/PIV card, Yubico NEO, Yubkey 4, PIVKEY C980, T600 and T800. These last two are "Feitian eJAVA Tokens" with the Taglio PIVKEY PIV applet.

Checklist

PKCS#11 module is tested

frankmorgner

Your code doesn't build due to compiler warnings.

src/libopensc/card-piv.c

frankmorgner · 2018-12-05T23:07:31Z

The whole PIV situation looks like a big mess. How does it come that we are taking so much care when Apple, for example, is fine with such a simple PIV driver? Are we doing something wrong, are we overly picky regarding features and implementation?

dengert · 2018-12-06T00:53:28Z

We are overly picky with:

Trying to support multiple applets on the same card
Trying to allow different applications to access different applets on the same PIV card Yubico with PIV and OpenPGP and PIV and CAC
Trying to avoid caching the PIN, i.e. keep the login state
Trying to also support PIN PAD readers
Having OpenSC driver matching routines for other cards sending APDUs to the PIV cards causing problems with login state
Trying to deal with all the different PIV applet implementations with bugs. There are a few in github...

Jakuje

Few comments to the coding style, otherwise it looks good. I will give it a try as soon as possible.

src/libopensc/card-piv.c

Jakuje · 2018-12-06T15:22:03Z

src/libopensc/card-piv.c

+	int r = 0;
+	u8 * rbuf = NULL;
+	size_t rbuflen = 0;
+	/* piv_private_data_t * priv = PIV_DATA(card); */


Unused code can be removed.

src/libopensc/card-piv.c

Jakuje · 2018-12-06T15:58:09Z

@dengert Thank you for the work through the specification to identify the correct cards. I added few comments to the code that need some more attention but after fixing them, I report success:

[root@rhel8-2 piv2]# pkcs11-tool --test --login
Using slot 0 with a present token (0x0)
Logging in to "FLUORINE.JANE.F.2001441054".
Please enter User PIN:
C_SeedRandom() and C_GenerateRandom():
  seeding (C_SeedRandom) not supported
  ERR: C_GenerateRandom failed: CKR_DATA_INVALID (0x20)
Digests:
  all 4 digest functions seem to work
  MD5: OK
  SHA-1: OK
  RIPEMD160: OK
Signatures (currently only for RSA)
  testing key 0 (PIV AUTH key)
  all 4 signature functions seem to work
  testing signature mechanisms:
    RSA-X-509: OK
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
    SHA256-RSA-PKCS: OK
  testing key 1 (2048 bits, label=SIGN key) with 1 signature mechanism
Logging in to "FLUORINE.JANE.F.2001441054".
Please enter context specific PIN:
    RSA-X-509: OK
  testing key 2 (2048 bits, label=KEY MAN key) with 1 signature mechanism -- can't be used to sign/verify, skipping
Verify (currently only for RSA)
  testing key 0 (PIV AUTH key)
    RSA-X-509: OK
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
  testing key 1 (SIGN key) with 1 mechanism
Logging in to "FLUORINE.JANE.F.2001441054".
Please enter context specific PIN:
    RSA-X-509: OK
  testing key 2 (KEY MAN key) with 1 mechanism -- can't be used to sign/verify, skipping
Unwrap: not implemented
Decryption (currently only for RSA)
  testing key 0 (PIV AUTH key)
    RSA-X-509: OK
    RSA-PKCS: OK
  testing key 1 (SIGN key)
    RSA-X-509: Logging in to "FLUORINE.JANE.F.2001441054".
Please enter context specific PIN:
OK
    RSA-PKCS: Logging in to "FLUORINE.JANE.F.2001441054".
Please enter context specific PIN:
OK
  testing key 2 (KEY MAN key)
    RSA-X-509: OK
    RSA-PKCS: OK
1 errors

Only issue is with the C_GenerateRandom, which still reports errors. Could this be also blacklisted as nonworking for the dual cards?

dengert · 2018-12-06T22:04:23Z

I could add the CI_NO_RANDOM to the card_issues for all the CAC cards. This PR does turn off ``SC_CARD_CAP_RNG`.

Note that PKCS#11 says C_SeedRandom can return CKR_RANDOM_SEED_NOT_SUPPORTED and CKR_RANDOM_NO_RNG and C_GenerateRandom can return CKR_RANDOM_NO_RNG. But framework-pkcs15.c does not return either, and sc_to_cryptoki_error_conversion does not have way to convert a SC_ERROR_ code to either. OpenSC has SC_CARD_CAP_RNG which cold be used to set CKR_RANDOM_NO_RNG

So to do this right needs some work in OpenSC internals and pkcs11-tool to address this correctly.

dengert · 2018-12-07T01:40:37Z

CI_NO_RANDOM added for Dual PIV/CAC cards. The PIV "9B" admin key was used to get a challenge. Some cards do not have one.

Jakuje

I will verify the functionality on Monday or Tuesday, but generally it looks good to me.

src/libopensc/card-piv.c

Jakuje · 2018-12-08T21:10:32Z

src/libopensc/card-piv.c

+		if (r < 0)
+			goto err;
+		switch (tag) {
+			case PIV_CCC_TAG_F0:


nit: Switch and case can have the same indent to avoid too much indentation. I would recommend it in this case.

frankmorgner

If the minor change requests are resolved then this PR looks OK.

frankmorgner · 2018-12-09T22:29:28Z

src/libopensc/card-piv.c

 		 * to test for active AID.
 		 */
 		int i7e = piv_find_discovery(card);

-		if (i7e != 0 && i7e !=  SC_ERROR_FILE_NOT_FOUND) {
+		sc_debug(card->ctx,SC_LOG_DEBUG_MATCH, "PIV_MATCH card->type:%d i7e:%d CI:%08x r:%d\n", card->type, i7e, priv->card_issues, r);
+		if (i7e != 0) {


SC_SUCCESS instead of 0?

Fixed use if (i7e < 0) { as is done in other places when testing return of piv_find_discovery(card)

src/libopensc/card-piv.c

Not all PIV applets are the same. Different versions of NIST 800-73 and improperly implemented or not implemented required features of NIST 800-73 cases problems. Have a look at the card_issues listed in card-piv.c. The PIV driver has tried to detect the differences based on clues found in the ATR historical bytes and vendor version numbers for some cards. At the same time it has tried to support the possibility there are multiple applets on a card that the user may want to use at the same time from different applications. This has lead to some detection problems with Dual CAC/PIV cards. The same cards sold by the vendor may have only a PIV applet that may not be the same PIV applet that is on the Dual PIV/CAC cards. http://www.cac.mil/Portals/53/Documents/CAC-utilziation-and-variation-matrix-v2.03-20May2016.doc defines a number of official CAC cards in active service. A table of the ATRs for these is now used to detect these cards. The PIV version of the CCC is also read to see if any CAC PKI objects are defined in the CCC, indicating it is a Dual CAC/PIV, even if the ATR is not listed. A more conservative approach to try and handle multiple applets on a card is used. Based on issues with the implementation of the PIV applet this may not be possible to do. So for many cards no additional detection will be done at the start of every transaction, and the login state can not be detected correctly. ATRs for PIVKEY are also in the match table, as these cards have a log of issues. Other PIV cards in the future or not yet tested may not be covered properly by this patch. Extra debugging was added with "PIV_MATCH" to help with these other cards. With "debug = 7;", `grep PIV_MATCH opensc-debug.log` can be used to see how a card type and card_issues are derived. On branch piv-improved-matching Changes to be committed: modified: card-piv.c modified: cards.h

Random data from PIV card is obtained using GENERAL AUTHENTICATE command for a request of a Challenge from the card. "00 87 00 9B 04 7C 02 81 00" Usually 8 bytes are returned. NIST 800-73-3_PART2, "A.1 Authentication of the PIV Card Application Administrator" "Table 11. Authentication of PIV Card Application Administrator" shows an example of how to do this. Some cards (one I have: 3b:7d:96:00:00:80:31:80:65:b0:83:11:17:d6:83:00:90:00) will not allow 2 of these commands in a row. (Maybe assuming command is only used as in Table 11 and is expecting the second command.) Code was added to card-piv.c so if "6A 80" is returned, try the command one more time. For any other GENERAL AUTHENTICATE failure, SC_ERROR_NOT_SUPPORTED is returned. piv_get_challenge may be called within a loop from sc_get_challenge if more random data is needed thus causing the the 2 commands to sent in a row. On branch piv-improved-matching Changes to be committed: modified: card-piv.c

Jakuje

Few minor comments, but otherwise I think it is ready for merging.

src/libopensc/card-piv.c

Jakuje · 2018-12-12T12:04:52Z

src/libopensc/card-piv.c

+	end = body + bodylen;
+
+	for(; (body < end); body += len) {
+


nit: trailing whitespace

src/libopensc/card-piv.c

On branch piv-improved-matching Changes to be committed: modified: card-piv.c

Changes to be committed: modified: card-piv.c

dengert · 2018-12-19T14:27:11Z

I believe this PR is ready to commit, and all comments have been addressed.
The failure of c319fa3 is do to "There are newer queued builds for this pull request, failing early" because I forced pushed another commit before the AppVeyor completed all of its tests.

If you want I can rebase into one commit.

They are against code that was forced merged, and can not be found. The issues were addressed by the merge.

dengert · 2018-12-26T22:40:10Z

@frankmorgner, I am dismissing your comment: "Your code doesn't build due to compiler warnings." This was caused by Appveyor because I forced merged a new set of commits. "All checks have passed" and "This branch has no conflicts with the base branch"

Can we get this merged in soon. @Jakuje has approved it and I would like to get in into master so other can easily try it too.

Thanks.

frankmorgner requested changes Dec 5, 2018

View reviewed changes

src/libopensc/card-piv.c Show resolved Hide resolved

src/libopensc/card-piv.c Outdated Show resolved Hide resolved

Jakuje reviewed Dec 6, 2018

View reviewed changes

src/libopensc/card-piv.c Outdated Show resolved Hide resolved

frankmorgner requested changes Dec 6, 2018

View reviewed changes

src/libopensc/card-piv.c Show resolved Hide resolved

Jakuje reviewed Dec 6, 2018

View reviewed changes

src/libopensc/card-piv.c Outdated Show resolved Hide resolved

Jakuje approved these changes Dec 8, 2018

View reviewed changes

frankmorgner previously requested changes Dec 9, 2018

View reviewed changes

dengert force-pushed the piv-improved-matching branch from 226dc0b to c030412 Compare December 10, 2018 21:15

dengert added 2 commits December 10, 2018 15:18

dengert force-pushed the piv-improved-matching branch from c030412 to 974ffd8 Compare December 10, 2018 21:18

dengert mentioned this pull request Dec 11, 2018

OpenSSL handshake failure when using hardware module #1547

Closed

Jakuje reviewed Dec 12, 2018

View reviewed changes

dengert added 3 commits December 12, 2018 11:33

PIV - Fix blanks and use returned value

c319fa3

On branch piv-improved-matching Changes to be committed: modified: card-piv.c

PIV - test i7e

86663e4

On branch piv-improved-matching Changes to be committed: modified: card-piv.c

PIV - Some VA cards have LCS = 0F Terminated

a932a89

Changes to be committed: modified: card-piv.c

frankmorgner merged commit 075cac4 into OpenSC:master Dec 27, 2018

Jakuje mentioned this pull request Mar 6, 2019

OpenSSH: Sign and Send Public Key Failed #1460

Closed

Jakuje mentioned this pull request Jun 4, 2020

Test of PKCS # 11 yields CKR_USER_NOT_LOGGED_IN (0x101) #1531

Closed

frankmorgner mentioned this pull request Apr 16, 2024

Avoid non SELECT(AID) commands for probing #3108

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PIV - Improved Card Matching for Dual CAC/PIV and PIVKEY cards. #1549

PIV - Improved Card Matching for Dual CAC/PIV and PIVKEY cards. #1549

dengert commented Dec 5, 2018

frankmorgner left a comment

frankmorgner commented Dec 5, 2018

dengert commented Dec 6, 2018

Jakuje left a comment

Jakuje Dec 6, 2018

dengert Dec 8, 2018

Jakuje commented Dec 6, 2018

dengert commented Dec 6, 2018

dengert commented Dec 7, 2018

Jakuje left a comment

Jakuje Dec 8, 2018

frankmorgner left a comment

frankmorgner Dec 9, 2018

dengert Dec 12, 2018

Jakuje left a comment

Jakuje Dec 12, 2018

dengert Dec 12, 2018

dengert commented Dec 19, 2018

dengert commented Dec 26, 2018

PIV - Improved Card Matching for Dual CAC/PIV and PIVKEY cards. #1549

PIV - Improved Card Matching for Dual CAC/PIV and PIVKEY cards. #1549

Conversation

dengert commented Dec 5, 2018

Checklist

frankmorgner left a comment

Choose a reason for hiding this comment

frankmorgner commented Dec 5, 2018

dengert commented Dec 6, 2018

Jakuje left a comment

Choose a reason for hiding this comment

Jakuje Dec 6, 2018

Choose a reason for hiding this comment

dengert Dec 8, 2018

Choose a reason for hiding this comment

Jakuje commented Dec 6, 2018

dengert commented Dec 6, 2018

dengert commented Dec 7, 2018

Jakuje left a comment

Choose a reason for hiding this comment

Jakuje Dec 8, 2018

Choose a reason for hiding this comment

frankmorgner left a comment

Choose a reason for hiding this comment

frankmorgner Dec 9, 2018

Choose a reason for hiding this comment

dengert Dec 12, 2018

Choose a reason for hiding this comment

Jakuje left a comment

Choose a reason for hiding this comment

Jakuje Dec 12, 2018

Choose a reason for hiding this comment

dengert Dec 12, 2018

Choose a reason for hiding this comment

dengert commented Dec 19, 2018

dengert commented Dec 26, 2018