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

IASECC: Add support for CPx cards #2217

Merged
merged 13 commits into from Mar 17, 2021
Merged

Conversation

vjardin
Copy link
Contributor

@vjardin vjardin commented Feb 1, 2021

The French CPx Healthcare cards are designed to support the IASECC
standard.

The French CPx Healthcare cards are designed to support the IASECC
standard.
Thanks to this commit, we get the full support of:
  - ./opensc-explore
    cd 0001
    asn1 2F00
  - ./pkcs11-tool -O
  - etc.
There are 2 applications: default one (contact mode) and the contactless
mode.
@vjardin
Copy link
Contributor Author

vjardin commented Feb 2, 2021

(record notes)

Thanks to this patch, we can get its SDO:

./iasecc-tool --list-sdos 1
Using reader with a card: Microchip SEC1110 [CCID Interface] (XXXXX) 00 00
Found SDO class 1, reference 1
	contact ACLs:	F3:00:00:13:FF:00
	tries maximum:	03
	tries remaining:	03
Found SDO class 1, reference 2
	contact ACLs:	F3:FF:00:FF:FF:00
	tries maximum:	0A
	tries remaining:	0A
	usage remaining:	00:64

or

./iasecc-tool --list-sdos 0x7B
Using reader with a card: Microchip SEC1110 [CCID Interface] (XXXX) 00 00
Found SE #2
	CRT #A4:	usage 08; algo 00; ref 01:00:...
Found SE #3
	CRT #A4:	usage 08; algo 00; ref 02:00:...
Found SE #5
	CRT #A4:	usage C0; algo 8C; ref 02:00:...
	CRT #B8:	usage 30; algo 8C; ref 02:00:...
	CRT #B4:	usage 30; algo 8C; ref 02:00:...
Found SE #6
	CRT #A4:	usage C0; algo 8C; ref 03:00:...
	CRT #B8:	usage 30; algo 8C; ref 03:00:...
	CRT #B4:	usage 30; algo 8C; ref 03:00:...

and its list of applications:

./iasecc-tool --list-applications
Using reader with a card: Microchip SEC1110 [CCID Interface] (XXXXX) 00 00
Application 'CPX IAS':
	AID: E828BD080F8025000001FF0010
	DDO: 4F0DE828BD080F8025000001FF0010

Application 'CPX IAS CL':
	AID: E828BD080F8025000001FF0020
	DDO: 4F0DE828BD080F8025000001FF0010

Serial number:

./opensc-tool --name --serial
Using reader with a card: Microchip SEC1110 [CCID Interface] (XXXXX) 00 00
00 00 01 99 WW XX YY 0F ........
IAS-ECC

Without this fix, we get:
./pkcs11-tool --module ../lib/onepin-opensc-pkcs11.so -M
Using slot 0 with a present token (0x0)
Supported mechanisms:
  SHA-1, digest
  SHA224, digest
  SHA256, digest
  SHA384, digest
  SHA512, digest
  MD5, digest
  RIPEMD160, digest
  GOSTR3411, digest

Once we include it, we get:
./pkcs11-tool --module ../lib/onepin-opensc-pkcs11.so -M
Using slot 0 with a present token (0x0)
Supported mechanisms:
  SHA-1, digest
  SHA224, digest
  SHA256, digest
  SHA384, digest
  SHA512, digest
  MD5, digest
  RIPEMD160, digest
  GOSTR3411, digest
  RSA-9796, keySize={1024,2048}, hw, decrypt, sign, verify
  RSA-PKCS, keySize={1024,2048}, hw, decrypt, sign, verify
  SHA1-RSA-PKCS, keySize={1024,2048}, sign, verify
  SHA256-RSA-PKCS, keySize={1024,2048}, sign, verify
  RSA-PKCS-KEY-PAIR-GEN, keySize={1024,2048}, generate_key_pair
src/libopensc/card-iasecc.c Outdated Show resolved Hide resolved
Comment on lines 671 to 678
else if (card->type == SC_CARD_TYPE_IASECC_CPX)
rv = iasecc_init_cpx(card);
else if (card->type == SC_CARD_TYPE_IASECC_CPXCL)
rv = iasecc_init_cpx(card);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this fillows the previous branches, but I would rather see it in one block such as follows so one does not have to investigate what is different in each of the blocks.

Suggested change
else if (card->type == SC_CARD_TYPE_IASECC_CPX)
rv = iasecc_init_cpx(card);
else if (card->type == SC_CARD_TYPE_IASECC_CPXCL)
rv = iasecc_init_cpx(card);
else if (card->type == SC_CARD_TYPE_IASECC_CPX ||
card->type == SC_CARD_TYPE_IASECC_CPXCL)
rv = iasecc_init_cpx(card);

Maybe even better would be the use of switch/case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that a switch/case would be a better design, but I do not want to "change" the design logic. It was already a if/else chain. Once this pull request will be merged, I can provide an update that replaces this if/else chain with a proper switch/case.

@Jakuje
Copy link
Member

Jakuje commented Feb 3, 2021

Thank you for the contribution. I put couple of minor comments inline. Does the pkcs11-tool --test work with your configured mechanisms? Does listing of the objects work with pkcs11-tool -O?

@vjardin
Copy link
Contributor Author

vjardin commented Feb 3, 2021

@Jakuje enclosed some outputs per your request:

./pkcs11-tool -O
Using slot 0 with a present token (0x0)
Data object 2966466816
  label:          'CPX_DIR'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966451920
  label:          'CPX_ATR'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966455072
  label:          'CPX_TECH_1'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966462480
  label:          'CPX_TECH_2'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966458800
  label:          'CPX_ID_TECH'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966459472
  label:          'CPX_ID_CARD'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966446240
  label:          'CPX_DATA'
  application:    'CPX'
  app_id:         <empty>
  flags:           modifiable
Data object 2966467184
  label:          'CPX_NAME_PS'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966457680
  label:          'CPX_LANG_PS'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Data object 2966453120
  label:          'CPX_INFO_PS'
  application:    'CPX'
  app_id:         <empty>
  flags:          <empty>
Profile object 2966451040
  profile_id:          '3'

regarding the self tests:

./pkcs11-tool --test
Using slot 0 with a present token (0x0)
C_SeedRandom() and C_GenerateRandom():
  seeding (C_SeedRandom) not supported
  seems to be OK
Digests:
  all 4 digest functions seem to work
  MD5: OK
  SHA-1: OK
error: PKCS11 function C_DigestInit failed: rv = CKR_GENERAL_ERROR (0x5)
Aborting.

which is an expected behavior for the current support.

@Jakuje
Copy link
Member

Jakuje commented Feb 3, 2021

The above commands show that you have only data objects on the card so then what is the point registering the RSA mechanisms if it does not have any keys? Or is it WIP to add support for the keys?

I am not sure what is the purpose of that card and what do you want to use it for with OpenSC.

@vjardin
Copy link
Contributor Author

vjardin commented Feb 3, 2021

The above commands show that you have only data objects on the card so then what is the point registering the RSA mechanisms if it does not have any keys? Or is it WIP to add support for the keys?

@Jakuje right, it is WIP.

I am not sure what is the purpose of that card and what do you want to use it for with OpenSC.

I need OpenSC to support a widely used IASECC card in France by about 1.4M+ users (healthcare users). Till now, it was using a framework that has never been upstreamed.

@frankmorgner
Copy link
Member

I think to make this more valuable, you need to support some private key and PIN operation to make this meaningful for some users.

I'm not happy with the changes to ef-atr.c, but let's discuss this in #2220...

@vjardin vjardin force-pushed the iasecc_cpx branch 2 times, most recently from 1d702ba to 2c9aab2 Compare February 6, 2021 09:44
@vjardin
Copy link
Contributor Author

vjardin commented Feb 6, 2021

you need to check if rv > 1

done b7b5cde#diff-d9ab7bee953b722791ab5aa28ac145b0149fec6cbd3082ce3f5b5c92b44bf953R171

thanks for pointing this issue.

Copy link
Member

@frankmorgner frankmorgner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK in general, please let me know if you were able to test some private key operation.

src/tools/opensc-explorer.c Show resolved Hide resolved
@vjardin
Copy link
Contributor Author

vjardin commented Feb 8, 2021

Looks OK in general, please let me know if you were able to test some private key operation.

I can perform properly a pkcs11-tool -O --login --ping ABCD but regarding some deep pkcs11 tests, I'd like to provide them thru a new serie of pull request. From the current analysis, for instance, if I compare OpenSC with the PKCS11 driver provided with the drivers, the C_CreateObject() callback should be unset (NULL).

Currently, with OpenSC, it is always:

(gdb) p *card->framework
$4 = {bind = 0x7ffff5649c00 <pkcs15_bind>, unbind = 0x7ffff5649ad0 <pkcs15_unbind>, create_tokens = 0x7ffff564e290 <pkcs15_create_tokens>, release_token = 0x7ffff56431e0 <pkcs15_release_token>, 
  login = 0x7ffff5648500 <pkcs15_login>, logout = 0x7ffff5643740 <pkcs15_logout>, change_pin = 0x7ffff56433e0 <pkcs15_change_pin>, init_token = 0x7ffff5647480 <pkcs15_initialize>, 
  init_pin = 0x7ffff56470e0 <pkcs15_init_pin>, create_object = 0x7ffff5648180 <pkcs15_create_object>, gen_keypair = 0x7ffff5644ca0 <pkcs15_gen_keypair>, get_random = 0x7ffff5644ba0 <pkcs15_get_random>}

while any calls to C_CreateObject() should return CKR_FUNCTION_NOT_SUPPORTED, at least, based on my current understanding. So, I feel that it is a new set of topics about "to do" or "not to do" thru another thread.

@dengert
Copy link
Member

dengert commented Feb 8, 2021

card->framework is used by all cards. There is also the pkcs15init director that deals with making changes to a card. There are 3 iasecc files there. You may also need to make changes to pkcs15-iasecc.c if there is still some issue with create_object for your card.

@vjardin
Copy link
Contributor Author

vjardin commented Feb 8, 2021

I need to understand the logics that are under the hood of the pkcs15-iasecc in order to get the proper Oasis C_xyz() callbacks to be set and unset ; for instance C_CreateObject() should be unset. The more I dig, the more I feel it should be part of another pull request once we can close this current pull request.

@dengert
Copy link
Member

dengert commented Feb 9, 2021

PKCS11 is an API.
PKCS15 is a description of a file system.
OpenSC and framework-pkcs15.c takes PKCS11 calls and and and uses PKCS15 routines to access the files on a token.

For cards that have a full PKCS15 filesystem, pkcs15-<card>.c does not do much at all. But for cards that do not or have only partial or improperly implemented PKCS15, pkcs15-<card>.c takes care of issues. Or
emulate a PKCS15 file system (or subset).

Many pkcs15-<card>.c has a sc_pkcs15emu_<card>_init_ex() and/or a sc_pkcs15emu_<card>_init().
in your case sc_pkcs15emu_<card>_init calls sc_pkcs15_bind_internal then overwrites
p15card->ops.parse_df = _iasecc_parse_df; in the sc_pkcs15_operations structure.

OpenSC pkcs15 routines call the card driver to access the card, via the the routines listed in

card-<card>.c routines implement ISO7816 routines to access the token and the applet(s) on the card.
struct sc_card_driver *iso_drv = sc_get_iso7816_driver();
Then can provide card specific function. In your card-iasecc.c driver see the sc_get_drive() that overwrites 8 of these.

I don't think you need to worry about trying to create objects for two reasons:

  1. Your card should be protecting itself from hacking and only allow write to the card with some special pin or authentication with a 3DES or AES key known only to the card issuer.
  2. PKCS11 allows session objects that may be on the token, or in memory, so only try an block a write in the card driver if you must, by modified your sc_get_drive() to reject the operation.

@vjardin vjardin force-pushed the iasecc_cpx branch 3 times, most recently from b8ae984 to d66104b Compare February 13, 2021 15:57
flags = SC_ALGORITHM_RSA_PAD_PKCS1;
flags |= SC_ALGORITHM_RSA_RAW;

/* No signature with contactless mode */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you do here does not reflect this comment. What you do allows raw RSA and RSA with PKCS1 padding. If you do not want to allow any signatures in contactless mode, you should not set flags at all and not call _sc_card_add_rsa_alg.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, currently, I am focusing on the Contact mode. The contactless mode is supposed to be close to the contactless mode, but with a different AID and no PIN code. For the time being, I'll focus on getting a full support of the contact mode.

So let's focus on this scope:
3867bd3

src/libopensc/card-iasecc.c Outdated Show resolved Hide resolved
src/libopensc/card-iasecc.c Outdated Show resolved Hide resolved
src/libopensc/ef-atr.c Outdated Show resolved Hide resolved
The previous commit was over simplified. According to the known
mechanism, we should have the following scope:

./pkcs11-tool --module ../lib/onepin-opensc-pkcs11.so -M
Using slot 0 with a present token (0x0)
Supported mechanisms:
  SHA-1, digest
  SHA224, digest
  SHA256, digest
  SHA384, digest
  SHA512, digest
  MD5, digest
  RIPEMD160, digest
  GOSTR3411, digest
  RSA-X-509, keySize={512,2048}, hw, decrypt, sign, verify
  RSA-PKCS, keySize={512,2048}, hw, decrypt, sign, verify
  SHA1-RSA-PKCS, keySize={512,2048}, sign, verify
  SHA256-RSA-PKCS, keySize={512,2048}, sign, verify
  RSA-PKCS-PSS, keySize={512,2048}, hw, sign, verify
  SHA1-RSA-PKCS-PSS, keySize={512,2048}, sign, verify
  SHA256-RSA-PKCS-PSS, keySize={512,2048}, sign, verify

do not use the default flags yet:
  _sc_card_add_rsa_alg(card, 1024, IASECC_CARD_DEFAULT_FLAGS, 0x10001);
  _sc_card_add_rsa_alg(card, 2048, IASECC_CARD_DEFAULT_FLAGS, 0x10001);
  _sc_card_add_rsa_alg(card, 512, IASECC_CARD_DEFAULT_FLAGS, 0x10001);

Contactless specific behaviour shall be added later on.
Log the send/recv data extracted from the EF.ATR (2F01).
2F01 is:
./opensc-explorer -r 0
OpenSC [3F00]> cat 2F01
00000000: 80 43 01 B8 46 04 04 B0 EC C1 47 03 94 01 80 4F .C..F.....G....O
00000010: 08 80 25 00 00 01 FF 01 00 E0 10 02 02 01 04 02 ..%.............
00000020: 02 01 04 02 02 01 00 02 02 01 00 78 08 06 06 2B ...........x...+
00000030: 81 22 F8 78 02 82 02 90 00                      .".x.....

so the ASN1 decoder gets confused because it assumes that two bytes are
needed before getting the first tag 43/ISO7816_TAG_II_CARD_SERVICE.
In order to avoid such confusion, whenever the content of the EF.ATR/2F01 starts
with ISO7816_II_CATEGORY_TLV, we skip the first byte in order to parse
the ASN1 payload.

Fix: issue OpenSC#2220
@frankmorgner
Copy link
Member

I don't like adding more (undocumented) flags globally, because that makes the code even harder to read. In my opinion, having a relaxed parsing as default is just good enough.

Actually, instead of introducing new flags, new error codes, we could add a warning to the log that the ASN.1 encoding was incorrect given a strict interpretation of the standard and just continue parsing. Obviously, card manufacturers are making a lot of such errors, so we must cope with this type of problem. Yes, we may trade one error for another, as Doug said, but either way, the user observes an error and will most likely create a log. The latter will then show the exact trace of the error including a possible warning for badly encoded ASN.1...

Let's wait for @Jakuje 's comment...

@vjardin
Copy link
Contributor Author

vjardin commented Mar 10, 2021

You will need to change decode_bit_string to return SC_ERROR_ASN1_BAD_BITSTRING if the "strict" code found one of the bad encodings 03 01 08 or 03 02 08 00 Any other bad encoding is something we don't know about so return SC_ERROR_INVALID_ASN1_OBJECT.

Right, I forgot to add it. I can add this logic and return SC_ERROR_ASN1_BAD_BITSTRING using a deeper check into this section according to your suggestion:

        /* The formatting is only enforced by SHALL keyword so we should accept
         * by default also non-strict values. */
        if (strict) {
                /* 8.6.2.3 If the bitstring is empty, there shall be no
                 * subsequent octets,and the initial octet shall be zero. */
                if (inlen == 1 && *in != 0)
                        return SC_ERROR_INVALID_ASN1_OBJECT;
                /* ITU-T Rec. X.690 8.6.2.2: The number shall be in the range zero to seven. */
                if ((*in & ~0x07) != 0)
                        return SC_ERROR_INVALID_ASN1_OBJECT;
        }

so, we would get this warning as you suggested:

                if (r == SC_ERROR_ASN1_BAD_BITSTRING && entry->flags & SC_ASN1_BAD_BITSTRING_ALLOWED) {
                        sc_debug(ctx, SC_LOG_DEBUG_ASN1, "Warning: SC_ERROR_ASN1_BAD_BITSTRING Allowed");
                        r = *len;
                }

but it will become quite complex because it requires many changes into the code in order to avoid the "Unknown error"s:

 [opensc-pkcs11] reader-pcsc.c:736:pcsc_unlock: called
 [opensc-pkcs11] pkcs15.c:2452:sc_pkcs15_read_file: returning with: 0 (Success)
 [opensc-pkcs11] asn1.c:1703:asn1_decode: called, left=174, depth 0
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'x509Certificate', tag 0x20000010
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry: decoding 'x509Certificate', raw data:30340C1B436572746966696361742064...
 [opensc-pkcs11] asn1.c:1703:asn1_decode:  called, left=82, depth 1
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'commonObjectAttributes', tag 0x20000010
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry:  decoding 'commonObjectAttributes', raw data:0C1B4365727469666963617420646520...
 [opensc-pkcs11] asn1.c:1703:asn1_decode:   called, left=52, depth 2
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'label', tag 0xc, OPTIONAL
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry:   decoding 'label', raw data:43657274696669636174206465205369...
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'flags', tag 0x3, OPTIONAL
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry:   decoding 'flags', raw data:0800
 [opensc-pkcs11] asn1.c:1571:asn1_decode_entry: Warning: SC_ERROR_ASN1_BAD_BITSTRING Allowed
 [opensc-pkcs11] asn1.c:1686:asn1_decode_entry: decoding of ASN.1 object 'flags' failed: Unknown error
 [opensc-pkcs11] asn1.c:1686:asn1_decode_entry: decoding of ASN.1 object 'commonObjectAttributes' failed: Unknown error
 [opensc-pkcs11] asn1.c:1686:asn1_decode_entry: decoding of ASN.1 object 'x509Certificate' failed: Unknown error
 [opensc-pkcs11] pkcs15-cert.c:502:sc_pkcs15_decode_cdf_entry: Certificate path 'e828bd080f8025000001ff0010::'
 [opensc-pkcs11] asn1.c:1703:asn1_decode: called, left=174, depth 0
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'x509Certificate', tag 0x20000010
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry: decoding 'x509Certificate', raw data:30340C1B436572746966696361742064...
 [opensc-pkcs11] asn1.c:1703:asn1_decode:  called, left=82, depth 1
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'commonObjectAttributes', tag 0x20000010
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry:  decoding 'commonObjectAttributes', raw data:0C1B4365727469666963617420646520...
 [opensc-pkcs11] asn1.c:1703:asn1_decode:   called, left=52, depth 2
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'label', tag 0xc, OPTIONAL
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry:   decoding 'label', raw data:43657274696669636174206465205369...
 [opensc-pkcs11] asn1.c:1727:asn1_decode: Looking for 'flags', tag 0x3, OPTIONAL
 [opensc-pkcs11] asn1.c:1505:asn1_decode_entry:   decoding 'flags', raw data:0800
 [opensc-pkcs11] asn1.c:1571:asn1_decode_entry: Warning: SC_ERROR_ASN1_BAD_BITSTRING Allowed
 [opensc-pkcs11] asn1.c:1686:asn1_decode_entry: decoding of ASN.1 object 'flags' failed: Unknown error
 [opensc-pkcs11] asn1.c:1686:asn1_decode_entry: decoding of ASN.1 object 'commonObjectAttributes' failed: Unknown error
 [opensc-pkcs11] asn1.c:1686:asn1_decode_entry: decoding of ASN.1 object 'x509Certificate' failed: Unknown error

but as commented by @frankmorgner into #2217 (comment) ; let's agree first on the target. Either ways are fine for me, I fully understand @frankmorgner 's comments, but I do not have enough background on OpenSC's community in order to comment which option is best.

@frankmorgner
Copy link
Member

Maintaining software is difficult and implementation has always multiple ways to solve a problem... Thanks for your patience!

@dengert
Copy link
Member

dengert commented Mar 10, 2021

Lets take a step back here.
@vjardin is trying to use French Healthcare cards, where PKCS#15 commonObjectAttributes of the Flags does not follow ASN.1 standards imposed by 2 recent patches to asn1.c in enforce the bit string standards. He found this problem using opensc-explorer. There may be other problems like this too.

A solution to the opensc-explorer where is is parsing the ASN.1 and would like to skip errors and continue is one thing. But fixing up bad ASN.1 is another, as information may be lost or reported incorrectly.

OpenSC needs to work with existing cards with the bad ASN.1 which is loaded via pkcs15.c. There are no hooks, for a card driver or a pkcs15-card.c to ask to "fix" the error while pkcs15 routines loads all the pkcs15 data.

We know that the 2 enforcement patches may cause existing cards to stop working if these are included in next release.
To avoid or mitigate problems, we should also have a way to turn off these enforcement patches from opensc.conf and let users have a way to fallback if needed, while we fix the problem in following release.

That said, if we know there is an ASN.1 problem in a specific card in a specific ASN.1 encoding and maybe only in a specify object, we would need a way to let card driver or pkcs15 driver fix the problem.

But OpenSC is not setup to allow a card driver to control the reading of PKCS#15 files with bad ASN.1. Decode routines don't ever get passed sc_context_t, or sc_card_t. The strict/lax flag would work if it can be set by a card driver for the specific card. i.e. with PKCS#11 lax should be set for only that one card.

So in other words, it looks to difficult to do the SC_ERROR_ASN1_BAD_BITSTRING.

@mouse07410
Copy link
Contributor

I think it was a bad idea from the beginning to try to enforce the "correct" ASN.1. All it accomplishes is breaking what did or could work.

OpenSC is not a Quality Control tool or test suite for manufacturers to debug their implementations. It's a tool for "normal" users to work with their imperfect cards.

I say - make parsing "loose" or tolerant by default, maybe add an explicit flag for strict parsing (though who would care to use it, and when - I've no idea).

@dengert
Copy link
Member

dengert commented Mar 10, 2021

The one thing the about enforcing the "correct" ASN.1 has shown that this card has invalid ASN.1 for the commonObjectAttributes Flag bit string.

So far @vjardin has shown he can get opensc-explorer to parse the invalid bitstring, but the value of the bits is not what was intended. He still has to show he can get it to work with the incorrect bits, or how to set the bits some other way.

@vjardin what does ./pkcs11-tool -O --login show?
Can you try these commands with and without --verify-pin: pkcs15-tool -D', pkcs15-tool -c, pkcs15-tool -kandpkcs15-tool --list-public-keys private` object flag is one that appears to not be set correctly, and would show up only when PIN was verified.

@mouse07410
Copy link
Contributor

The one thing the about enforcing the "correct" ASN.1 has shown that this card has invalid ASN.1 for the commonObjectAttributes Flag bit string.

Great. And how did it help us? Will we tell the vendor to fix the format?

@dengert
Copy link
Member

dengert commented Mar 10, 2021

I hope so. And we know enough to look at other bit stings on these cards that may also be invalid.
But not clear if the French Healthcare card issuers will listen.

@mouse07410
Copy link
Contributor

Yubico listened, and their PIV applet is one of the closest to the standard...

Would French government supplier listen? I rather doubt - but let's see.

@vjardin
Copy link
Contributor Author

vjardin commented Mar 10, 2021

@vjardin what does ./pkcs11-tool -O --login show?
Can you try these commands with and without --verify-pin: pkcs15-tool -D', pkcs15-tool -c, pkcs15-tool -k`

see the enclosed logs
pkcs15-tool-c+verify-pin.log.gz
pkcs15-tool-D.log.gz
pkcs15-tool-D+verify-pin.log.gz
pkcs15-tool-k.log.gz
pkcs15-tool-k+verify-pin.log.gz
pkcs15-tool-c.log.gz

@vjardin
Copy link
Contributor Author

vjardin commented Mar 10, 2021

Yubico listened, and their PIV applet is one of the closest to the standard...

?! What does it mean ? Does Yubico supports the IASECC standard ???

Would French government supplier listen? I rather doubt - but let's see.

No way, they are not going to change 1.4M of cards which are deployed for many years. They are going to stay as-is for many years.

@vjardin
Copy link
Contributor Author

vjardin commented Mar 10, 2021

I hope so. And we know enough to look at other bit stings on these cards that may also be invalid.

It seems that there are only 2 issues, for the 2 flags of these 2 files.

But not clear if the French Healthcare card issuers will listen.

Even if they "listen", it is too late the million of cards are already shipped for many years. I prefer that we live with a proper workaround that fits all the cases.

@mouse07410
Copy link
Contributor

mouse07410 commented Mar 11, 2021

Does Yubico supports the IASECC standard ???

No, of course not.

It means that Yuhikeys had problems in their PIV implementation, we pointed that out to them, and Yubico improved their implementation greatly.

Even if they "listen", it is too late the million of cards are already shipped for many years. I prefer that we live with a proper workaround that fits all the cases.

Completely agree.

Not to mention that the probability of "they listen" is very near to zero.

@vjardin
Copy link
Contributor Author

vjardin commented Mar 12, 2021

Please what's next ? How to tackle it ?

@dengert
Copy link
Member

dengert commented Mar 13, 2021

What's next? Not sure.
OpenSC has been developed by card vendors, people interested in using (or need to use) their work or government issued cards on non-windows systems, linux distros wanting to support many smartcards, open source developers wanting user controllable security, delelopers wanting to develeope a better applet, programmers interested in a challenge and I may have missed some other too. None of these people have more then a handful of cards for testing.

French CPx Healthcare cards are not of much interest outside of France. It looks like a pkcs15 type card, but we have found it does not encode a bit string properly. @Jakuje in #2217 (comment) said it looks like the card has only objects and no keys or certificates. That could be true but it looks like the bit string issue caused pkcs15 routines to not find the keys or certificates. There may be other ANS.1 or invalid PKCS15 structures that will also cause problems.

As with other government issues cards, they most likely did not publish the code or documents about the cards. Without these you may have to resort to PC/SC or USB tracing of APDUs. You may want to look at #2244 where I was talking to @the-docta about how to do some traces, and do simple command line issuing of APDUs.

There is no general consensus right now on how to handle the ASN1 bit string bug. Either drop the commits the enforce it, or make same major changes in the asn1 parsing code that will effect every card.

So if you wish to pursue this, I would suggest for your testing, revert the two commits and see how far you can get. If you can get the card working that is a case to solve the bit string problem.

If this card is as simple as I would suspect, with 1 or 2 certificates and private keys on the card and RSA only, it might be possible write a new driver, that emulates PKCS15 and avoids any unnecessary parsing of bas ASN1 data.

Personally I fall into the "interested is using their work/government issued card on linux" and "programmers interested in a challenge". But with no one else is interested in CPx Healthcare card, I don't want to spend much time on it.

So find some documentation on the card, buy (or find) copy of ISO-7816-4,ISO-7816-8, PKCS15 (or ISO-7816-15) and get some PC/SC traces of a card on windows.

@frankmorgner
Copy link
Member

please leave this PR as is for now.

I suggest to remove a0ca9ef from this PR and merge it. Always having a non-strict parsing is in line with what we're currently using with sc_asn1_decode_integer.

When we're done with this PR, we should discuss elsewhere whether we want to continue adding these type of flags for ASN.1 parsing...

There are two flavours of CPX cards:
  - contact mode,
  - contactless mode
Few years ago, the commit 0362844
did squash the 3F00nnnn path to nnnn. For instance, 3F002F00
becomes 2F00. It is an issue such as:
  00000200 [139681798813440] APDU: 00 A4 09 04 02 2F 00
  00029790 [139681798813440] SW: 6A 82

Fix: issue OpenSC#2231
@vjardin
Copy link
Contributor Author

vjardin commented Mar 16, 2021

please leave this PR as is for now.

I suggest to remove a0ca9ef from this PR and merge it. Always having a non-strict parsing is in line with what we're currently using with sc_asn1_decode_integer.

done. Thanks for the advices.

Copy link
Member

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for a delay. Catching up with the emails after last week. I think this PR is fine to merge

@frankmorgner frankmorgner merged commit fc0df4e into OpenSC:master Mar 17, 2021
Release 0.22.0 automation moved this from In progress to Done Mar 17, 2021
@vjardin vjardin deleted the iasecc_cpx branch March 17, 2021 16:02
@vjardin
Copy link
Contributor Author

vjardin commented Mar 17, 2021

thank you.

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 this pull request may close these issues.

None yet

5 participants