Skip to content

NIST Secure Messaging moved from PIV to separate sm/sm-nist.c #3098

Draft
dengert wants to merge 17 commits intoOpenSC:masterfrom
dengert:sm-nist
Draft

NIST Secure Messaging moved from PIV to separate sm/sm-nist.c #3098
dengert wants to merge 17 commits intoOpenSC:masterfrom
dengert:sm-nist

Conversation

@dengert
Copy link
Copy Markdown
Member

@dengert dengert commented Apr 4, 2024

With the introduction of Secure Messaging in PIV driver last year, some developers where questioning why the PIV SM was implemented in card-piv.c rather then using sm/sm-iso.c where it could be used by other card drivers. It was suggested that MyEID may have some interest in using it.

This proof of concept PR moves most of the PIV SM code to sm/sm-nist.c where, with some additional work, it could be used by other card drivers.

The concern at the time of writing the PIV SM code was that sm/sm-iso.c did not support the same SM as used by PIV SM.

  • sm/sm-iso.c applied SM to a single APDU. PIV SM applies SM to the data to be sent, then uses APDU chaining and Get Response to send and receiver the secured data and response using multiple APDUs. ISO-7816-4 10: " Secure messaging (SM) protects all or part of a C-RP, or a concatenation of consecutive data fields (payload fragmentation, see 5.3),..." i.e. chaining and get response.

  • CMAC is used by PIV SM and the MAC is not padded.

  • The padding tag byte is not used in PIV SM.

To address these issues, flags were added to iso_sm_ctx in sm/sm-iso.h and used by sm/sm-iso.c and several other places.

card-piv.c was modified to allow for building without any SM, with the current PIV SM implementation or to use the sm/sm-nist.c that uses the modified sm-iso.c. This was done as it was not clear which code could be moved to sm/sm-nist.c so there many #if defined.. statements.

If this POC code becomes a PR, there would be only two build options with NO SM, or use sm/sm-nist.c. (OpenSSL and EC support is required in any case.)

There is still a lot to do. For example, where should the pairing code handled. Is there code in card-piv.c that should be moved. No testing was done to have multiple applications trying to use the same card at same time.

configure.ac was modified for testing to force all the github actions it check the sm/nist.c code.

There is no known PIV applet that supports SM that could be run virtually for testing. As far as I know, only Jakub and I have cards from Idemia.

@frankmorgner, MyEID or other developers would this be useful to proceed with this code, or not?

@frankmorgner
Copy link
Copy Markdown
Member

This looks promising, thank you. but it will need some time for a review. Just some quick comments regarding your questions:

If we switch to the SM implementation in sm/sm-nist.c rather than card-piv.c, then it would be good to remove the duplicated code in card-piv.c

Looking at the implementation, sm-nist.c does include a padding content indicator and it padds data with 0x80..0x00. So padding is exactly what is defined in iso-sm.c. A better integration into the existing infrastructure would avoid the modifications of sm-ico.c.

From what you write, creating the protected short lenght C-APDUs with chainging is very much like creating the protected extended length C-APDU without chaining: In both cases the input data is encoded identitically, this single blob is encrypted identically and this single block is maced identically. Only when sending this one big SM APDU, you need to split up the apdu->data into multiple chunks with multiple sc_transmit_apdu calls rather. Same for the response APDU: once all GET RESPONSES are done, decoding/decrypting is identically. I think that part is easily integrated in iso-sm.c in the transmit phase - all steps before that can stay as they are.

I think it would be best to move the existing padding of the data to be MACed into sm-eac.c, because the padding content indicator seems to refer to the plain text data only. sm-ico.c should then only forward the unpadded data to be MACed via authenticate so that you can use it without problems via CMAC.

My approach with sm-eac.c was to include the crypto stuff that is required to establish an E2E channel to the card. That does not only include the encryption/decryption of APDUs, but also the stuff for establishing the session keys when verifying PIN or terminal keys. Having that in mind, I think sm-nist.c should keep sm_nist_start as entry point with the raw paring code and parsing or managing the pairing code should stay in card-piv.c

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Apr 10, 2024

Thanks for the review. I would also request any developers (MyEID?) who might want to use the NIST PIV SM in some other card driver also comment on their plans.

looks promising, thank you. but it will need some time for a review. Just some quick comments regarding your questions:

If we switch to the SM implementation in sm/sm-nist.c rather than card-piv.c, then it would be good to remove the duplicated code in card-piv.c

Yes that is the plan. I left it with both ways as I was trying to move some SM code to sm/sm-nist. if we move the code, the old version and the #if would be removed. By leaving the old code in, and changes are made to the old code, the changes can also be made to the code in sm-nist at a later time.

I really don't want to spend a lot of time on this unless it is going to be used by some other card driver.

Looking at the implementation, sm-nist.c does include a padding content indicator and it padds data with 0x80..0x00. So padding is exactly what is defined in iso-sm.c. A better integration into the existing infrastructure would avoid the modifications of sm-ico.c.

The changes to sm/sm-iso.c deal with not breaking up the data into multiple blobs, and then using chaining after encryption, MAC additions.

From what you write, creating the protected short lenght C-APDUs with chaining is very much like creating the protected extended length C-APDU without chaining: In both cases the input data is encoded identitically, this single blob is encrypted identically and this single block is maced identically. Only when sending this one big SM APDU, you need to split up the apdu->data into multiple chunks with multiple sc_transmit_apdu calls rather. Same for the response APDU: once all GET RESPONSES are done, decoding/decrypting is identically. I think that part is easily integrated in iso-sm.c in the transmit phase - all steps before that can stay as they are.

Yes, that what it does, by using the SC_APDU_FLAGS_SM_CHAINING which is in OpenSC 0.24.0 9933d62

I think it would be best to move the existing padding of the data to be MACed into sm-eac.c, because the padding content indicator seems to refer to the plain text data only. sm-ico.c should then only forward the unpadded data to be MACed via authenticate so that you can use it without problems via CMAC.

No way am if going to try modify sm-eac.c This exercise was more of a prof of concept. And as I said above, I don't want to spend anymore time on it unless there is some other card driver that would actual use sm-nist.

My approach with sm-eac.c was to include the crypto stuff that is required to establish an E2E channel to the card. That does not only include the encryption/decryption of APDUs, but also the stuff for establishing the session keys when verifying PIN or terminal keys. Having that in mind, I think sm-nist.c should keep sm_nist_start as entry point with the raw paring code and parsing or managing the pairing code should stay in card-piv.c

@frankmorgner
Copy link
Copy Markdown
Member

I've added a commit on top of you PR, which avoids padding the data-to-be-mac'ed including the changes to sm-eac.c and sm-iso.c so you don't have to.

@frankmorgner
Copy link
Copy Markdown
Member

Regarding the padding, I just realized that your code includes a sanity check whether the padding is OK and it doesn't add any padding. That sanity check may be removed, because sm-iso.c already does this im rm_padding(). Also the other initial comments should be resolved with your response.

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Apr 12, 2024

388929c works. I will look closer at what code in sm-nist could be removed and at squashing many of the commits.

If you want I can post an opensc-debug log.

@frankmorgner frankmorgner marked this pull request as draft April 22, 2024 21:41
@dengert dengert force-pushed the sm-nist branch 2 times, most recently from 3cf9949 to 55a6fe9 Compare April 6, 2025 02:08
@dengert
Copy link
Copy Markdown
Member Author

dengert commented Apr 6, 2025

Aventra was kind enough to send me some, soon to be release, MyEID cards one of which is configured as a PIV card. The others are not yet configured. The cards using the PIV applet or the MyEID applet can use the NIST sp800-73-4 Secure messaging.

Changes added this week include changes to card-myeid.c to use SM. Aventra's vision is to use the PIV applet only if MyEID code is not available on sys systems. The cards are populated using the MyEID applet and additional commands can be used to setup mapping of the certificates and keys to be used from either applet.

The test card with both applets was not formatted for PKCS15 but the certificates can still read by opensc-explorer so it was possible to read the SM cert-signer and CVC certificate using ISO 7816-4 APDU commands, to do the SM authentication and use SM for some simple non PKCS15 commands such as read the serial number. (Since setting up the SM should be done early, having a fixed path to these files on the card should be included in their documentation.)

So there is still a lot to do including using pkcs15-tool to configure the card, and add commands in some tool to configure the PIV applet to use keys, objects and certificates created by pkcs15-tool.

Aventra used the OpenSC PIV SM code during development of the on card SM code.

This PR has makes it possible to easily use the SM from the card-myeid.c

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Apr 6, 2025

This PR also fixes some .github issues with "awk" not being available on some fedora systems. I would suggest that these be reviewed by others and committed be others.
./github/setup-fedora.sh

@ -4,6 +4,7 @@ set -ex -o xtrace
 
 # Generic dependencies
 DEPS="make /usr/bin/xsltproc docbook-style-xsl autoconf automake libtool bash-completion vim-common softhsm openssl diffutils openpace openpace-devel"
+DEPS="$DEPS awk"

@metsma
Copy link
Copy Markdown
Contributor

metsma commented Apr 7, 2025

This PR also fixes some .github issues with "awk" not being available on some fedora systems. I would suggest that these be reviewed by others and committed be others. ./github/setup-fedora.sh

@ -4,6 +4,7 @@ set -ex -o xtrace
 
 # Generic dependencies
 DEPS="make /usr/bin/xsltproc docbook-style-xsl autoconf automake libtool bash-completion vim-common softhsm openssl diffutils openpace openpace-devel"
+DEPS="$DEPS awk"

Same here:
https://github.com/OpenSC/OpenSC/pull/3382/files#diff-0dd2f5bb468e616df04d5b691b8edd4d235d57b917d200ded61013c04c281babR6

src/sm/sm-eac.c Outdated
eacsmctx = ctx->priv_data;

inbuf = BUF_MEM_create_init(data, datalen);
r = iso_add_80_pad(data, datalen, ctx->block_length, &p);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The idea not to include padding into the authenticate and verify_authentication callback is that padding is done before calling those functions. The reasoning is that with the padding content indicator from ISO-7816, padding to a block size is clearly handled at the ISO level rather than at the card driver level, where the actual MAC is calculated or verified.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The change was made because of your commit "avoid padding mac data" 2014-4-11
currently listed in my branch as: 2245a69 It also removed some code I though would be needed but it is not.

added iso_add_80_pad(const u8 *data, size_t datalen, size_t block_size, u8 **padded) to sm-iso.h and sm-iso.c. But in sm-eac.c it is used as `iso_add_80_pad(ctx, data, datalen, &p);`` which did not compile. So 37a5415 was added. If this is not correct what should it be?

As an example of OPENSC_DRIVER=myeid gdb --args ./opensc-tool --serial

opensc-debug-250407-myeid-sm-serial.txt

shows the myeid_init establishing the PIV SM then reading the serial number starting on line 403.

The reasoning is that with the padding content indicator from ISO-7816, padding to a block size is clearly handled at the ISO level rather than at the card driver level, where the actual MAC is calculated or verified.

The sm-nist is implemented to be called by the card driver and the sm-nist calls the sm-iso calls as needed.

src/sm/sm-iso.c Outdated
case SC_APDU_CASE_3_SHORT:
case SC_APDU_CASE_3_EXT:
if (apdu->ins & 1) {
if (ctx->padding_tag == 1) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the reasoning behind this change?

The current idea is that the indicator is always prepended if the instruction code is even, because odd instructions indicate a BER-TLV payload (ISO 7816-4):

In the interindustry class, bit b1 of INS indicates a data field format as follows.
 If bit b1 is set to 0 (even INS code), then no indication is provided.
 If bit b1 is set to 1 (odd INS code), payloads (if any) shall be encoded in BER-TLV (see 8.1).

Do you want to support BER-TLV content for an even INS, because NIST SP 800 73-4 always uses 0x87 with indicator and encrypted context?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you want to support BER-TLV content for an even INS, because NIST SP 800 73-4 always uses 0x87 with indicator and encrypted context?

I would say yes, but maybe this in not needed. It has been a year. I will have to run gdb to have a look, if this change is really need or not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, yes I remember (even though its been 11 years now ^^)... I thought that the padding in PIV was different, but it turns out that both padding mechanisms are the same. So it seems the commit from back then is not correct, because indeed the mac data is padded in all cases. compare, NIST.SP.800-73pt2-5 Fig. 2. PIV data integrity of command with 9303_p11_cons_en.pdf Figure 5. Computation of an SM command APDU for even INS Byte, fr example. So this old commit could be removed.

Copy link
Copy Markdown
Member Author

@dengert dengert Apr 7, 2025

Choose a reason for hiding this comment

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

The NIST PIV SM treats even and odd ins the same and needs the padding character.
The code in in the PR in effect skips over the } else if (apdu->ins & 1) { in line 358 so even and odd are treated like even.

NIST SP 800-73pt2-5:

Secure messaging specified in this section CAN be applied to the following commands:

• GET DATA   (this is odd 'CB' for PIV, but the MyEID uses 'CA' and SM works on the one test I have run with it)  
• VERIFY        (`20` even)
• CHANGE REFERENCE DATA
• GENERAL AUTHENTICATE"

Table 21. Secure messaging data objects
    Tag     Description 
    '87'     **Padding content indicator byte** followed by the encrypted data
    '8E'     Cryptographic checksum (MAC)
    '97'     Le
    '99'     Status word

Doing some further testing, setting in sm-nist.c ctx->padding_tag = 0; adding to sm-iso.c

sc_debug(card->ctx,  SC_LOG_DEBUG_VERBOSE, "prepend_padding_indicator: %d, padding_indicator %02X",
                         prepend_padding_indicator, ctx->padding_indicator);

and using pkcs11-tool --login --test

P:46338; T:0x140737346566144 16:24:11.810 [opensc-pkcs11] apdu.c:367:sc_single_transmit: CLA:0, INS:CB, P1:3F, P2:FF, data(5) 0x7fffffffc080
P:46338; T:0x140737346566144 16:24:11.810 [opensc-pkcs11] sm.c:133:sc_sm_single_transmit: called
P:46338; T:0x140737346566144 16:24:11.810 [opensc-pkcs11] sm.c:134:sc_sm_single_transmit: SM_MODE:200
P:46338; T:0x140737346566144 16:24:11.810 [opensc-pkcs11] sm-nist.c:2138:sm_nist_pre_transmit: returning with: 0 (Success)
P:46338; T:0x140737346566144 16:24:11.810 [opensc-pkcs11] sm-iso.c:383:sm_encrypt: 
Protected Le (plain) (1 byte):
00 .

P:46338; T:0x140737346566144 16:24:11.810 [opensc-pkcs11] sm-iso.c:224:format_data: 
Data to encrypt (16 bytes):
5C 03 5F C1 0C 80 00 00 00 00 00 00 00 00 00 00 \._.............

P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] sm-iso.c:230:format_data: 
Cryptogram (16 bytes):
D3 BC 8F 27 34 F5 E6 A9 51 64 7F CE 54 4C 13 AB ...'4...Qd..TL..

P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] sm-iso.c:232:format_data: prepend_padding_indicator: 0, padding_indicator 01
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] sm-iso.c:400:sm_encrypt: 
Padding-content indicator followed by cryptogram (plain) (16 bytes):
D3 BC 8F 27 34 F5 E6 A9 51 64 7F CE 54 4C 13 AB ...'4...Qd..TL..

P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Cryptogram'
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1833:asn1_encode_entry: type=4, tag=0x80000005, parm=0x555555606d40, len=16
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=18
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Padding-content indicator followed by cryptogram' (not present)
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=0
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Protected Le'
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1833:asn1_encode_entry: type=4, tag=0x80000017, parm=0x555555606d80, len=1
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=3
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Cryptographic Checksum' (not present)
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=0
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] sm-iso.c:458:sm_encrypt: 
Data to authenticate (37 bytes):
0C CB 3F FF 80 00 00 00 00 00 00 00 00 00 00 00 ..?.............
85 10 D3 BC 8F 27 34 F5 E6 A9 51 64 7F CE 54 4C .....'4...Qd..TL
13 AB 97 01 00     
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] sm-nist.c:1958:sm_nist_authenticate: called
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] sm-iso.c:467:sm_encrypt: 
Cryptographic Checksum (plain) (8 bytes):
3C E0 FD 86 E1 67 A7 29 <....g.)

P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Cryptogram'
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1833:asn1_encode_entry: type=4, tag=0x80000005, parm=0x555555606d40, len=16
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=18
P:46338; T:0x140737346566144 16:24:11.811 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Padding-content indicator followed by cryptogram' (not present)
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=0
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Protected Le'
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] asn1.c:1833:asn1_encode_entry: type=4, tag=0x80000017, parm=0x555555606d80, len=1
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=3
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] asn1.c:1828:asn1_encode_entry: encoding 'Cryptographic Checksum'
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] asn1.c:1833:asn1_encode_entry: type=4, tag=0x8000000e, parm=0x555555695500, len=8
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] asn1.c:2026:asn1_encode_entry: length of encoded item=10
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] sm-iso.c:518:sm_encrypt: 
ASN.1 encoded encrypted APDU data (31 bytes):
85 10 D3 BC 8F 27 34 F5 E6 A9 51 64 7F CE 54 4C .....'4...Qd..TL
13 AB 97 01 00 8E 08 3C E0 FD 86 E1 67 A7 29    .......<....g.)
P:46338; T:0x140737346566144 16:24:11.812 [opensc-pkcs11] reader-pcsc.c:245:pcsc_internal_transmit: called
P:46338; T:0x140737346566144 16:24:11.842 [opensc-pkcs11] reader-pcsc.c:337:pcsc_transmit: 
Incoming APDU (2 bytes):
69 88 i.

meaning the card did not accept the SM encoding.
(This shows another problem, the code turned off SM, and ran the rest without SM. which needs to be addressed.)

With `sctx->padding_tag = 1; the difference is in:

P:50526; T:0x140737346566144 16:31:26.151 [opensc-pkcs11] sm-iso.c:224:format_data: 
Data to encrypt (16 bytes):
5C 03 5F C1 0C 80 00 00 00 00 00 00 00 00 00 00 \._.............

P:50526; T:0x140737346566144 16:31:26.151 [opensc-pkcs11] sm-iso.c:230:format_data: 
Cryptogram (16 bytes):
BE 53 89 F8 D1 4D 1F 86 1F 62 79 4C FE 06 43 64 .S...M...byL..Cd

P:50526; T:0x140737346566144 16:31:26.151 [opensc-pkcs11] sm-iso.c:232:format_data: prepend_padding_indicator: 1, padding_indicator 01
P:50526; T:0x140737346566144 16:31:26.151 [opensc-pkcs11] sm-iso.c:400:sm_encrypt: 
Padding-content indicator followed by cryptogram (plain) (17 bytes):
01 BE 53 89 F8 D1 4D 1F 86 1F 62 79 4C FE 06 43 ..S...M...byL..C
64  

So the code needs to add the padding byte. The choice of the name ctx->padding_tag could be changed,
and the if statement code be reworked.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see, thanks for the clarification. I think it would be most flexible if you could add a general flag to struct iso_sm_ctx, which handles these kinds of exceptions. Something like ISO_SM_FLAG_ALWAYS_PADDING_INDICATOR would be fine. However, it should be needed to be set explicitly, because I still believe the BER-TLV ruling above is the correct reading. At least SC-HSM is also adhering to it and will require no content indicator in odd instructions.

src/sm/sm-iso.h Outdated
/** @brief Padding-content indicator byte (ISO 7816-4 Table 30) */
u8 padding_indicator;
/** @brief if 1 use tag 87 */
u8 padding_tag;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This value should not be needed, because it is covered by padding_indicator. The possible, ISO defined values are as follows:
grafik

PIV SM is using 01 (SM_ISO_PADDING) and is well covered in iso-sm.c

src/sm/sm-iso.c Outdated
sm_apdu->data = sm_data;
sm_apdu->datalen = sm_data_len;
sm_apdu->lc = sm_data_len;
if (ctx->use_sm_chaining && sm_apdu->datalen > 255) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we really need a dedicated flag use_sm_chaining? If you added sm_apdu->flags |= SC_APDU_FLAGS_CHAINING, then sc_transmit_apdu() would pick this up and split the APDU into short length and send all of it subsequently, right? So just returning a complete long APDU from the ISO layer with chaining enabled would be enough to let the transmit code handle the sending. At first glance, it looks like the chaining flag could be added in the pre_transmit hook for the PIV card.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we need this. The ISO model is to process every APDU in a chain operations, where PIV does the mac and encryption on whatever data need to be sent then breaks it up whine chaining. Same for response.
I can also send a debug log of PIV doing this.

Copy link
Copy Markdown
Member

@frankmorgner frankmorgner Apr 7, 2025

Choose a reason for hiding this comment

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

The ISO model is to process every APDU in a chain operations

Why do you think so? AFAICT, sm-iso.c always builds a single SM APDU from a signle non encrypted APDU. Currently, the cards I have tested are all supporting extended length, which is then also used for the encryption - no need for chaining. In case of PIV, the same encoding and encryption routine can be used with the only difference the the SM APDU is not sent as one single (extended) APDU, but possibly chopped into multiple short APDUs. From what I read, this is exactly what the code in apdu.c does, when the chaining flag is set.

src/sm/sm-iso.h Outdated
/** @brief do_not_split_apdu into multiple apdus */
u8 use_sm_chaining;
/** @brief get response is always in clear */
u8 get_response_in_clear;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

similar as with the chaining, we already have the management of GET RESPONSE built into sc_transmit(). I believe if you would not set SC_APDU_FLAGS_NO_GET_RESP on the SM apdu, then sc_transmit() would do what you want, i.e. call sc_get_response() without applying SM. As sc_get_response() delegates this command to the driver call back get_response, you're in full control on how this is executed. In PIV, the default implementation of the iso driver is used. I suggest you use the pre_transmit hook to write a private variable for disabling SM on GET RESPONSE, and then use the post_transmit hook to reset this variable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sm-nist.c has sm_nist_pre_transmit and sm_nist_post_transmit routines and can selectively apply SM on some commands, all or at none. the default is if contactless, apply on all, if contact, GET_DATA is done in clear, with pin, and crypto with SM.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What I wanted to say is that the handling you want should already be present in apdu.c and I believe it is not needed to duplicate this code for SM APDUs

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Apr 10, 2025

@frankmorgner
What I have been doing for the last few days, is going back to the unmodified sm-iso.h sm-iso.c and sm-eac.c.
and adding switches as needed. (sm-eac.c should never have been modified.)

I have adding code based on switches that default to 0 so they should not be a problem for existing code in sm-iso
There are so 4 switches. see sm-iso.h

diff --git a/src/sm/sm-iso.h b/src/sm/sm-iso.h
index 88a277869..b0d6382ba 100644
--- a/src/sm/sm-iso.h
+++ b/src/sm/sm-iso.h
@@ -162,6 +162,14 @@ struct iso_sm_ctx {
 
        /** @brief Padding-content indicator byte (ISO 7816-4 Table 30) */
        u8 padding_indicator;
+       /** @brief If 1 always include padding_indicator even for tag 87 */
+       u8 always_add_padding_indicator;
+       /** @brief do not add padding on data to be mac'ed */
+       u8 skip_mac_padding;
+       /** @brief Do sm_encrypt of all the data. If needed use command chaining to send chunks to card.
+        * Card will then return response using get_response commands in multiple chunks if needed.
+        * Then do the sm decrypt and verify once on the full response. */
+       u8 sm_encrypt_once_then_chaining;
        /** @brief Pad to this block length */
        size_t block_length;

NIST SP 800-73pt2-5 "Fig. 3. Single command under secure messaging" is for short commands.
Fig. 4. "Chained command under secure messaging"
And also says: "Command chaining will be needed if the secure messaging data field to be transported is larger than 255 bytes."

Note it requires the use of short APDUs, There is no mention of extended APDUs although some cards support it as an extension.

The difference between sm-iso.c and sm-nist.c is that iso would use apdu.c "divide et impera: transmit APDU in chunks with Lc <= max_send_size" before doing the sm_encrypt routines where as NIST wants the the sm_nist_encrypt done before command channing break up the encrypted data into chained APDUs.

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Apr 10, 2025

@frankmorgner
Over the last few day, removed all changes to sm-iso.c, sm-iso.h and sm-eac.c, and added new switches in sm-iso.h that default to 0 and added code in sm-iso.c for the 4 switches. sm-eac.cshould not have been changed at all.)

The last one was sm_encrypt_once_then_chaining as ISO use the apdu.c "divide et impera: transmit APDU in chunks with Lc <= max_send_size" before doing encryption amd mac for each chunk. Where as NIST does the encryption and CMAC to the whole data then uses command chaining to transmit the encrypted and MACed data using chained APDUs.

NIST SP 800-73pt2-5 "4.2.4. Command With PIV Secure Messaging" says: "Command chaining will be needed if the secure messaging data field to be transported is larger than 255 bytes." and figure 3 an figure 4 show the difference. NIST does not mention extended APDUs, although some cards may support it.

@dengert dengert force-pushed the sm-nist branch 2 times, most recently from f937fd9 to a127fe6 Compare April 10, 2025 15:12
}
_sc_card_add_rsa_alg(card, 2048, flags, 0);

if (card_caps.card_supported_features & MYEID_CARD_CAP_RSA) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

where can this MYEID_CARD_CAP_RSA be switched on?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

"The Aventra MyEID PKI Smart Card Reference manual Ver. 3.0.6" says "Date:10.3.2016 Version:2.1.0 Description:
Added GET DATA: MyEID CARD CAPABILITIES"
APDU 00 CA 01 AA 0C there is a version 1 and version 2 (since MyEID 5, the tests cards are 5 but not certified and have version 4.9.1)
The card returns 02 00 6D 10 00 00 00 01 00 02 09 00 90 00
02 is version
006D says: Specifies capabilities supported in this version of MyEID.
bit flags, bytes in big endian order:
bit 0 (0001h): RSA algorithm
bit 1 (0002h): DES/3DES algorithm
bit 2 (0004h): AES algorithm
bit 3 (0008h): ECDSA and ECDH algorithms
bit 4 (0010h): GridPIN
bit 5 (0020h): PIV emulation
bit 6 (0040h): Supports Windows Smart Card Framework ID.
bits 6-15: RFU

1000 max RSA 4096
0000 no DES3
0100 max AES 256
02 09 max EC 521
00 Security certifications looks like this is added in version 2 of the structure
The 4.9.1 test cards are not certified yet

@frankmorgner
Copy link
Copy Markdown
Member

There are still some basic questions above. What's the status of this topic, how are you going to proceed?

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Jun 4, 2025

There are still some basic questions above. What's the status of this topic, how are you going to proceed?

Yes. Aventra sent me 5 MyEID version 4.9.1 cards in April along with a PDF "Aventra MyEID PKI Smart Card Reference manual
Ver. 3.0.6" which will cover version 5.0 cards. These cards in PIV mode can use sp800-73-4 and above PIV Secure Messaging and Aventra has been testing it with the current OpenSC where it is built in to card-piv.c.

The MyEID code in OpenSC can be used to provision the card with 4 privkey, pubkey and certificates. Additional APDUs can be used to enable PIV mode, and may the keys and certificates.
But the current OpenSC code in pkcs15init or pkcs15 functions do not know how to create or handle CVC certificates which is required for PIV SM.

I have been working on using additional scripts to create a PIV CVC certificate, sign it with a subCA key and then create files for the CVC certificate and write it to the card using opensc-explorer. The SubCA certificate is written using pkcs15-init lie any other certificate.

As part of the Reference manual is how to put and get data objects (PIV Initialization Parameters) that map the pkcs15 file paths to PIV key and certificate objects. The data object (Secure Messaging Parameters) lists the Algorithm ids 0x27 and/or 0x2E that can be used, the path the CVCs for these Algorithms, and the path to the SubCA (FID of the Secure Messaging Certificate Signer certificate.) These are written and or read using opensc-explorer.

Just recently I have been able to use this PR, some additional changes not yet committed and these scripts to use the the PIV SM code with the MyEID card-myeid.c. Using OPENSC_DRIVER=PIV-II ./pkcs11-tool --test --login or OPENSC_DRIVER=myeid ./pkcs11-tool --test --login shows that the SM is connected, and some of the APDUs are encrypted. The sm_nist_pre_transmit checks the APDU instruction, and only handles the PIV APDUs (Odd get and put data, 0x87 verify and others), where as the myeid driver uses the even get and put data, as well as other ISO7816 crypto instructions. The verify are the same in both and the verify commands are protected by the PIV SM in both drivers.

Needless to say there is still a lot to be done:

  • implement PKCS15 cvCertificates not just X.509 certificates
  • stop using bash and opensc-explorer scripts and move to either pkcs15init or a myeid-tool or piv-tool to create CVC certificates and load to the card.
  • look at how the Aventra minidriver initializes a card and what paths it uses, vs the OpenSC myeid.profile. It is not clear if Aventra initialized cards will work with OpenSC pkcs15. The one demo card they sent the worked with card-piv.c but just had iso7816 file paths, but no PKCS15 meta data.
  • Convince Aventra they need a few more PIV objects on the card not just the 4 sets of keys, pubkeys and certificates.

@hhonkanen Thanks for the cards and documentation.

@dengert dengert force-pushed the sm-nist branch 2 times, most recently from 601f174 to 706cd2b Compare June 4, 2025 16:42
@dengert
Copy link
Copy Markdown
Member Author

dengert commented Jun 6, 2025

@frankmorgner
You asked: There are still some basic questions above. What's the status of this topic, how are you going to proceed?

You also mentioned EAC setting a flag, but the only flag, I see, related to APDU in EAC code is ISO_COMMAND_CHAINING which sets apdu.class. Because only ISO_COMMAND_CHAINING is set, apdu.c treat it like a seperate single APDU.
All other uses of chaining in OpenSC is set using apdu.flags with SC_APDU_FLAGS_CHAINING which lets apdu.c
use chaining and if needed does:

/* divide et impera: transmit APDU in chunks with Lc <= max_send_size
 * bytes using command chaining */

This will call sc_transmit with each chunk to be SM encode if needed. That is not what PIV SM wants. The SC_APDU_FLAGS_SM_CHAINING is set so "divide et impera" will be done, but not do SM on each chunk because the PIV SM code has already encode the whole data block.

In my view, these two ways of getting around SM encoding each APDU are independent and there is no conflict, because SC_APDU_FLAGS_SM_CHAINING is only set when using PIV SM.


Now that it has been shown that the MyEID cards can use the sm-nist, I want to simplify the PR.

The current PR allows building with or with or without PIV SM or building with the PIV SM code in sm/sm-nist.c.

I would like to have the PR to be either no PIV SM, or PIV SM using sm/sm-nist.

The current PR also contains changed to MyEID code to use sm/sm-nist. I would like to back that our of this PR and make that a separate PR, as it is not complete and is just an addon.

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Jun 8, 2025

I have pushed a new version of this PR. This removes some 1860 lines of code that were included to allow building PIV SM using either the version totally in card-piv.c or building with most of the PIV SM code in sm/sm-nist.c Now that the sm/nist.c code appears to work as expected, there is no need to have 2 versions in card-piv.c

I have also not included any of the MyEID SM code, which will depend on the PR. I have already shown that the MyEID changes can use the sm/nist. There is more work to be done, including support for non X.509 certificates in OpenSC pkcs15. Pkcs15 specs defines other types of certificates, including CVC certificates. Much of the testing was done using opensc-explorer to issue specific MyEID apdus, that should be done in pkcs15init or some additional MyEID tool.

configure.ac was modified to enable building of sm-nist and having card-piv.c use it. These could be removed, if it would always be built when ENABLE_SM is set. In some test, building without ENABLE_SM has shown some card drivers fail to compile, so right now the capability to build without ENABLE_SM does not work. And fixing those drivers should be addressed by other PRs. And maybe some CI test to build without SM.

@frankmorgner So far I do not see any conflicts with EAC's use of ENVELOPE and the flags set and used only by this PR.

This last run points out some formatting issues which I hope to address this week.

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Jun 9, 2025

@frankmorgner If there is still some question about the SC_APDU_FLAGS_SM_CHAINING flag, maybe a better name would be SC_APDU_FLAGS_SM_ALREADY_ENCODED which tells apdu.c to not call SM to encode it again.

It prevent apdu.c while dividing up the data into chunks and encoding each chunk again. It is not the SC_APDU_FLAGS_CHAINING flag, which tells apdu.c to break up the data into chucks.

Copy link
Copy Markdown
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.

While I don't think that SC_APDU_FLAGS_SM_CHAINING and the new flags in iso_sm_ctx are strictly necessary, I would be OK with having those until as intermediate solution. (I agree, that those add ons should not interfere with the existing SM code)

CVC code should be unified as that's defined in ISO 7816-8, but that's part of #2702

There are still tons of TODOs and I've also commented on some parts of the code that seems like it has not been tested at all. Are there some test vectors that could be used for PIV in order to look at this more closely, maybe in an automated fashion? For EAC, for example, there is a worked example, which breaks down every operation from authentication to SM...

$(top_builddir)/src/ui/libstrings.la \
$(top_builddir)/src/sm/libsmeac.la \
$(top_builddir)/src/sm/libsmnist.la \
$(top_builddir)/src/sm/libsmiso.la \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makefile.mak also needs to be adapted

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixied.


priv->sm_params.csID = priv->csID;

r = sm_nist_start(card, &priv->sm_params);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

errors here seem to be ignored

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The PIV SM is an optional feature part of VCI, that is required to do crypto over a contactless connection. The card enforces the use of SM for verify and crypto over the contactless connection, but the card can also do SM over a contact connection and can do it for GET DATA and other APDUs too which are not considered sensitive. NIST sp800-73-4-2016 Part 2 "A.6 Authentication of the PIV Cardholder Over the Virtual Contact Interface" and other sections.

The lines following the above (line 4103) test for r < 0 and treat it as an error, if NIST_SM_FLAGS_ALWAYS was set from ENV or opensc.conf USE_PIV_SM = always ; see getenv lines around line 750. The default to to use SM for verify and crypto for contactless if the card supports. So the test above treats the use of SM as an error only if the user says always and the card supports SM. The other option is "never" where the use says don't use it in which the card will not expose sensitive data.

So in the default or never cases will continue to run without SM, but the card will enforce it use over the contactless interface and fail on a APDU that requires SM.

It my testing I use the USE_PIV_SM = always ; as it forces the SM over contact interface.

The USE_PIV_SM could be changed to USE_NIST_SM, or each driver could have a version like USE_MYEID_SM
and added to opensc.conf.example.in

Copy link
Copy Markdown
Member

@frankmorgner frankmorgner Aug 4, 2025

Choose a reason for hiding this comment

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

Thanks for the explanation. I missed the check of r after this else block.

Please add a warning in the log that failed establishment of SM channel is not treated as error if r < 0 and priv->sm_params.flags & NIST_SM_FLAGS_ALWAYS == 0. r < 0 is likely to still indicate a problem that needs to be solved.

Additionally, static code analysis will complain that r gets set without being used. If you emit a warning, the code checker will be silenced.

/* Step H10 Create AES session Keys */
/* kdf in is 4byte counter || Z || otherinfo 800-56A 5.8.1 */

kdf_inlen = 4 + Zlen + cs->otherinfolen;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

otherinfolen is not set anywhere, i.e. it will be 0

Copy link
Copy Markdown
Member Author

@dengert dengert Jul 2, 2025

Choose a reason for hiding this comment

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

The two cipher suites, for 27 and 2E are constants. cs points at one one of these suites.
otherlen is defined as 61 or 69 on these. Look for the line "/*Table 14 and other constants */" in sm-nist.c
and in sp800-73-4 part 2, section "4.1.6 Key Derivation" which says: "and OtherInfo being constructed using the concatenation format as show below."

for (i = 0; i < cs->o0len; i++)
*p++ = cs->o0_char; /* 0x09 or 0x0d */

*p++ = sizeof(IDsh);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

everything you write from here on has not been allocated

Copy link
Copy Markdown
Member Author

@dengert dengert Jul 2, 2025

Choose a reason for hiding this comment

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

All the lengths are fixed and listed in the cipher suites. The alloc is done at line 957 as part of this:

        /* Step H10 Create AES session Keys */
	/* kdf in is 4byte counter || Z || otherinfo  800-56A 5.8.1 */

	kdf_inlen = 4 + Zlen + cs->otherinfolen;
	kdf_in = malloc(kdf_inlen);

The o0len=0x04 and o0_char=x09 are from 4.1.6 Key Derivation as the first 5 bytes
CS2 0x04 || 0x09 || 0x09 || 0x09 || 0x09
CS7 0x04 || 0x0D || 0x0D || 0x0D || 0x0D

and in line 1000 a test is done that the correct number of bytes as copied"

        if (p != kdf_in + kdf_inlen) {
		r = SC_ERROR_INTERNAL;
		goto err;
	}

src/sm/sm-nist.c Outdated
piv_clear_cvc_content(&priv->sm_in_cvc);
piv_clear_cvc_content(&priv->sm_cvc);
free(priv);
/* TODO IS this needed? ctx->priv_data = NULL; */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But I would say no, based on:

@dengert
Copy link
Copy Markdown
Member Author

dengert commented Jul 1, 2025

@frankmorgner @hhonkanen
In regards to "Are there some test vectors that could be used for PIV in order to look at this more closely, maybe in an automated fashion?" from: #3098 (review)

I do not know of any open source PIV applets that support the NIST PIV SM. So the only testing would have to be done with real cards. And the only cards that do are require vendor utilities to provision the cards, something I have not wanted to even try and use. The SM also requires CVC keys and certs and a trusted cert signer certificate to be loaded on the card.

But MyEID with the "Aventra MyEID PKI Smart Card Reference manual Version - 3.0.6" (provided by the vendor) does support provisioning a version 4.9.1 card as a MyEID and accessing the card as a PIV card. This requires some mapping of PIV objects to the iso7816 files used by MyEID. So I have started down that path to provision some of the cards they have sent me as PIV cards.

The OpenSC MyEID card was based on version 3 cards and they have added dual PIV (without SM) in version 4? cards, and only the 4.9.1 (i.e. bata cards for version 5.0) support SM for PIV. The PIV support was added to the version 2.30 manual on 25.3.2019

So there are at least two git branches involved:

  • Branch sm-nist this PR NIST Secure Messaging moved from PIV to separate sm/sm-nist.c  #3098 just deals with getting shared parts of the the SM code out of card-piv.c and into src/sm/sm-nist.
  • branch myeid-4.9.1 (not a PR yet) deals with updating the OpenSC myeid code to support the provisioning of the cards with the new features for MyEID cards.

Additional changes are needed to support non X.509 certificates in PKCS11 and PKCS15, as part of the provisioning it to write a CVC certificate (defined as a der blob in PKCS15 but not defined in PKCS11.) which are in the works.

And additional changes are are not yet committed card-myeid.c is to support sm-nist from the card-myeid.c

So far I can provision a card with a CVC using some ugly scripts to create the CVC and pkcs15-init to write the CVC to the card, and setup the PIV mapping and get the PIV applet on the card to do SM. pkcs15 routines and pkcs11 routines can handle the non X.509 certificates. as a cert blob.

I also have code to git the card-myeid.c to also use sm/sm-nist.c for some operations using the same keys and certificates.
This will take some changes especially as a MyID card will now have two applets that will have all the same issued as PIV cards with more than one applet, such as CAC or OpenPGP.

/* If SM was active, test if SM connection is still valid to card using VERIFY 00 20 00 XX
* If reply is 90 00 or 63 Cx Our SM connection must still be valid to PIV applet.
* and we get tries left too.
* if not select aid reauthenticate as other process may be using SM too
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If a different process uses SM as well, then using your internal SM context will yield an SM error as the card thinks the keys have changed. So you can simply use iso7816_select_aid (select AID in with your internal SM) and check if SM is still active.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The use of the 00 20 00 xx vs the select_aid is a toss up. if the 00 20 00 xx is used via SM it also tests the login state
which in the PIV case allows the login state to be set by one process and used by another. But the OpenSC software still prompt user of each process to enter the PIN.

One of the tests I do run is to start two or more pkcs11-tool --test --login --pin xxxx from a script, which will
cause process interference. and have done so using OPENSC_DRIVER=PIV-II for one and OPENSC_DRIVER-myeid for another We should consider this for other card drivers too.

The PIV card has some features not seen on other cards.

  1. The Sign key requires a pin verify before a crypto operation with no other APDUs between them. Card enforced.
  2. Within a SM session, some APDUs can be protected and other done in the clear, for example reading certificates.
  3. When using a card which supports SM, over a contactless interface, the card requires SM to be used for verify and crypto.
  4. The card may have multiple applets and PIV applet will reset login state (and I think any SM session) when switching to another applet on the card.
  5. There must be a dozen different versions of PIV applets, some approved by NIST, other not. Some violate the standard in some way.

The reader_lock_obtained was originally added to handle (1.) then enhanced to handle (4.) And with the PIV SM code in card-piv.c reader_lock_obtained was modified to handle the SM code too. reader_lock_obtained puts the card driver in control when a sc_lock causes a PCSC SCardBegin to be obtained. The sc_lock maybe from internal OpenSC routines such as SM code or even from the driver.

There are 43 card drivers. 9 card drivers call sc_lock at least once. PIV does about 12 times. 12 drivers have a reader_lock_obtained. That means most card drivers never addressed any of the problems with multiple process
but relying on internal OpenSC routines to retry failed APDUs.

One of the things I noticed early on (years ago) of there are more then one process using the same card, is interference especially during the card driver init routine. card-piv.c addresses this be locking while it is reading all the objects it might need in piv_init.

To be able to handle (1.) (without pin caching or with a pin pad reader), requires a lock to be obtained when the user enters the PIN then make sure the applet is selected, and SM if being used is setup before the verify is sent to the card, followed immediately by the crypto command.

Now if card-myeid.c was to use PIV SM, it does not call sc_lock at all relying on OpenSC routines to do the sc_lock. And it looks like it does several multiple APDU operations that should be done in the same SCardBeginTransaction. These include selecting a file, then reading the data. And another is set security environment followed by crypto operations. This needs some more investigation.

So I prefer the card driver be in charge of the SM.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One of the things I noticed early on (years ago) of there are more then one process using the same card, is interference especially during the card driver init routine. card-piv.c addresses this be locking while it is reading all the objects it might need in piv_init.

The internal object cache of card-piv.c doesn't yield any cache hits, meaning once an object was read, it will not be read again. Things seem to have changed, the internal object cache is not needed anymore.

My request here is to keep the code simple: Please evaluate the PIN status, where it is actually needed. You will not catch all possible errors in card_reader_lock_obtained. The mind model of your code is not handable anymore if you introduce more and more complexity. In case of that key which requires the PIN to be validated immediately before, I guess the best place is to fix this right before the signing operation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the comments.

The internal object cache of card-piv.c doesn't yield any cache hits, meaning once an object was read, it will not be read again.

It does not count cache hits. It either knows object can not be present, (See History object or based on version of card or known to not be supported on some card). Or when requested to read an object, if it was read or is not present on card.

With a PIV card the only way to know if a key and cert exist is to read the cert "object" which contains the certificate (which may be gzipped) and one of the objects may have an SM intermediate CVC. The SPKI from the cert has the key type and parameters. The History object does have information of how many retired keys are on the card and how many of these keys have certs on the card and a URL to get the off card certs. The cert objects are read during card-pv.c init_piv, before pkcs15-piv.c is bound. There is no PKCS15 CDF on the card.

A PKCS11 applications can read any of the objects on the card. Some of the objects 9some require a pin) can be large like fingerprints and iris image and not not used in any standard application so most of these are not read in init-piv
PKCS11 can read certificates, pubkeys and info in privkeys all of which are obtained from the cert objects.

Objects other than the cert objects, History, Discovery select AID response are not cached unless requested.

Things seem to have changed, the internal object cache is not needed anymore.

I don't see what has changed.

card_reader_lock_obtained was designed to catch interference issues from other processes by checking the state of the card when the sc_lock first obtains exclusive access to the card., i.e. is correct applet selected, and if login state has been lost. This would then work with multi applet cards where each process is using a different applet, and where both use the PIV applet.

When SM is used, this goes one step further. A process could setup SM, which does not touch the login state and PIV allows some get_data commands to be done in the clear. So another process accessing the card could continue to use the card without SM while the other with SM uses SM from time to time Both using sc_lock to obtain exclusive access when needed. (The card only requires SM to do crypto over a contactless interface. But using SM with contact interfaces has some security improvements, so the OpenSC driver by default uses SM for crypto and verify. )
But if both setup SM, this causes the card to drop the first and setup a new SM for the second. When the first tries to use its SM, it will fail.

As you point out there might be a better way to restart the SM from the SM routines.

BUT in all these cases PIV applet developers, do not always follow the standard, either by error or deliberately.
For example login state where a select AID to the same applet will reset the login state. PIV specs say that should not happen when the PIV applet is selected while the PIV applet is the active applet, but the login state to the PIV applet should be reset if a non PIV applet is selected. Complicated by fact PIV can use Global PIN vs PIV applet PIN. (the Discovery object says which is allowed and which is preferred.)
Another is Yubikey allows crypto and verify over contactless interface, and does not have SM.

So the card_reader_lock_obtained tries to handle these cases.

The point of moving the PIV SM out of the card-piv:

  • You wanted the code to use more of the SM code in the sm directory.
  • Allow other card drivers to use the PIV SM which this PR moves to sm/sm_nist.h and sm/sm_nist.c

As you might have noticed, https://github.com/dengert/OpenSC/tree/myeid-4.9.1 is built upon this PR: https://github.com/dengert/OpenSC/tree/sm-nist

@hhonkanen at Aventra has sent me 5 MYED cards that support both a MYEID applet using PKCS15 and a PIV applet that supports the NIST PIV SM on their card. This then makes the MYEID card a multi applet card, where both the PIV and the MYEID card can use PIV NIST SM. And the MYEID card driver in OpenSC does not do anything to protect against interference. (But the intent of Aventra has been the PIV applet would only be used in situations where no MYEID card drivers are available.) They used the OpenSC PIV driver to test their SM card implementation. I used Idemia ID-One PIV 2.4.1 demo cards Idemia sent me to write the OpenSC PIV SM driver.

* from reader_lock_obtained maybe caused
* by interference of other process that started its own
* SM session. In this case do not close and let reader_lock_obtained
* continue.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you elaborate on what happened here? I would not expect multiple processes to use different SM keys with the same OpenSC context (i.e. sm_ctx).

Copy link
Copy Markdown
Member Author

@dengert dengert Jul 2, 2025

Choose a reason for hiding this comment

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

Each process sets up its own keys, an ephemeral EC key is used as part of the SM initialization, and the application and card each have counters of number of transactions. Each process has has its own sm_ctx. So if process tries to use SM it will cause an error for the other process.

The comment "In this case do not close and let reader_lock_obtained continue" is the way the SM is telling the "reader_lock_obtain" to reopen the SM before continuing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is horribly unmaintanable spaghetti code. It it scattered across multiple files and undocumented flags. Imagine some day you cannot continue to work on the PIV driver anymore. Who do you think will have the time to build up the mental model to understand what you're doing here?

The calls to sm_close and sm_open, that I have experienced were always in sequential order, there never was some interference. The same would go for card_reader_lock_obtained: If you need to reestablish SM, first call sm_close, then call sm_open. Done, no interference!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Imagine some day you cannot continue to work on the PIV driver anymore. Who do you think will have the time to build up the mental model to understand what you're doing here.

That day could be sooner than you think. Some card vendor who wants to use NIST SM in their cards. See
#3098 (comment) and https://github.com/dengert/OpenSC/tree/myeid-4.9.1

The calls to sm_close and sm_open, that I have experienced were always in sequential order, there never was some interference. The same would go for card_reader_lock_obtained: If you need to reestablish SM, first call sm_close, then call sm_open. Done, no interference!

Will have to look closer. The intent was restart SM on the next sc_lock that starts a transaction, not at the end of a failing APDU that would endup doing sc_unlock which could allow for more interference.

OpenSC should have tests for cards to test for interference. For example: 2 or more pkcs11-tool -test processes started at the same time with a card with more than one RSA key. I see many places in OpenSC where this could be a problem. Not having select file in same transaction as the read binary (MYEID testing) and possibly MSE and crypto in different transactions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have analyzed the code a bit and I now understand what @dengert wants to achieve with it. We have a somewhat similar mechanism in our proprietary minidriver to silently re-establish SM without disturbing the user in case it gets dropped out. The card can drop the SM session, if another process creates a separate SM session. The card may also drop the SM session, if it doesn't allow plain APDUs in between.

The new MyEID can maintain only one SM session at a time. It can do SM in two modes: The non-exclusive mode allows other processes to send plain APDUs without affecting the SM session's state. In the exclusive mode, sending a plain APDU drops the authentication state of the SM session, to prevent another process being able to gain access for using private keys when the required PIN was authenticated in the SM session.

I understand the decision to handle the situation within reader_lock_obtained and sm_nist_close. This way, we don't need to handle the situation in many different places. To my understanding sm_nist_close gets called by the if the VERIFY command sent through the SM channel fails, like after sending any APDU through the channel. We can use the flags to let sm_nist_close to return without calling iso_sm_close in this specific situation. I don't know if this is the optimal solution - in a higher level language like C++ or C# it would perhaps be easier to make the code more readable but we are working with C here. I thought of other ways to handle this situation, but didn't really find a better way which would take all aspects (e.g. the existing structure of OpenSC, and avoiding large modifications into it) into account.

I don't see any serious problems in the code. While it involves flags in several files, I think they are in logical places and their meaning is described in code comments. Perhaps the mechanism could be elaborated further in comments. I mean, by describing more clearly in sm_nist_close how the calls lead here when the SM validity check is done in the card driver, and adding more comments to piv_card_reader_lock_obtained as well. I would support accepting this PR (regarding this piv_card_reader_obtained/nist_sm_close mechanism, haven't reviewed other discussed issues thorougly), if some more comments are added, and unless someone suggests a better, concrete solution.

@dengert dengert force-pushed the sm-nist branch 2 times, most recently from 8be497a to 712ad95 Compare July 3, 2025 22:04
dengert added 17 commits August 9, 2025 09:09
Add CSE and FLAGS to APDU debug message Very useful when debugging SM code.

Pass code formating test that inserts blanks in wrong places.
	data(%"SC_FORMAT_LEN_SIZE_T"u) %p
is replaced with
	datalen:%"SC_FORMAT_LEN_SIZE_T"u data:%p

 Changes to be committed:
	modified:   src/libopensc/apdu.c
The PIV SM code is moved to sm-nist.c so it could be used by other card drivers.

sm-nist.c uses sm-iso.c code to simplify the process as sugested.
This required some changes to sm-iso.h and sm-iso.c as NIST sp800-73-4
encodes and adds CMAC for the entire transaction then send it to the card
using command chaining of a single PUT_DATA command. Original sm-iso.c
would break up a payload and encode and MAC each part and sent as seperate APDUs.

The response data handled in similar fashion.

 Changes to be committed:
	modified:   src/libopensc/Makefile.am
	modified:   src/libopensc/card-piv.c
	modified:   src/sm/Makefile.am
	modified:   src/sm/sm-iso.c
	modified:   src/sm/sm-iso.h
	new file:   src/sm/sm-nist.c
	new file:   src/sm/sm-nist.h
ENABLE_SM_NIST (enabled by default) is used to build sm/sm-nist.c
based on NIST sp800-73-4 Secure Messaging which was moved
from card-piv.c to so it can be used by other card drivers.

ENABLE_PIV_SM (enabled by default) is used within card-piv.c
to use the sm-nist code if ENABLE_SM_NIST is defined.

If sm-nist is used by other card drivers, data needed such as the
SM EC key reference and certificate with the public key and
CVC certificates will need to be provided by the
card driver. The card driver will also determine which APDUs will
be protected by SM.
sm_nist_params_t is used to pass parameters and shared data
to sm_nist_start. This allows a card driver to provide
certificates and flags outside of the strict PIV requirements
so other card drivers can use the NIST SM as defined
in NIST sp800-73-4 but store the certificates X509
or CVC as PKCS15 files.

sm_nist_params_cleanup can be used by a card driver to cleanup
the sm_nist_params_t which can be part of a card driver's private data.

 On branch sm-nist
 Changes to be committed:
	modified:   src/sm/sm-nist.c
	modified:   src/sm/sm-nist.h
 Changes to be committed:
	modified:   src/libopensc/card-piv.c
 On branch sm-nist
 Changes to be committed:
	modified:   src/libopensc/card-piv.c

 Changes to be committed:
	modified:   src/libopensc/card-piv.c
Thse are the changes need to get sm-nist.c to use sm-iso routines with 4 added
modifications changes controlled by flags listed in sm-iso.h The default values
are all 0 so sm-iso should continue to work.

The major changes was to get sm-iso to support short APDUs using command chaining.

Still to be done in error handling.

 On branch sm-nist
 Changes to be committed:
	modified:   sm-iso.c
	modified:   sm-iso.h
	modified:   sm-nist.c
Simplify  the process  to select applet and reopen SM
after obtaining a PCSC lock, to only select the PIV applet and
reopen any SM session.

Piv_init already runs under a single PCSC Transaction. So this
basiclly leaves crypto operations.

This is done to allow  multiple processes to use multiple
applets  on the same card, and when SM is used, each
process uses its one SM sessions.

 On branch sm-nist
 Changes to be committed:
	modified:   libopensc/card-piv.c
 On branch sm-nist
 Changes to be committed:
	modified:   sm/sm-nist.c
	modified:   sm/sm-nist.h
Many of the functions, defines, structures and variables have been renamed
as part of separating the NIST sp800-73-4 Secure Messaging out of card-piv.c
so it can be used by other card drivers.

Names with nist* or NIST_*are in sm-nist.h and sm-nist.c and used by card drivers.
Names with piv* or PIV_* are specific to card-piv.c and implement
specific SM and non SM features of PIV cards.

Note the names used for the pairing code and VCI  have not been changed yet.

 On branch sm-nist
 Changes to be committed:
	modified:   libopensc/card-piv.c
	modified:   sm/sm-nist.c
	modified:   sm/sm-nist.h
The flag can be used by reader_lock_obtained when Sm should still
be active.

	modified:   src/sm/sm-nist.c
	modified:   src/sm/sm-nist.h
 Changes to be committed:
	modified:   sm/sm-nist.c

 On branch sm-nist
 Changes to be committed:
	modified:   src/sm/sm-nist.c
 On branch sm-nist
 Changes to be committed:
	modified:   sm/sm-nist.c
 On branch sm-nist
 Changes to be committed:
	modified:   sm/sm-nist.h
The flag is used by reader_lock_obtained when SM should still
be active. It uses a  SM encoded VERIFY pin command to get tries left.

The command will only responded correctly when:
-  The selected applet has not changed
-  The card and application SM have same shared secret
-  and the Mac counters are the same

If some other process has accessed the card and set up its own
SM  or selected a different applet the shared secret would be
different as the application uses an ephemeral EC key and the
card generates a random nonce. Sm and Login state are reset
in PIV card if a different applet is selected.

PIV allows for some non SM protected operation to be done
to a card even when SM is active such as get data and and verify.
This is the original way PIV cards worked before sp800-74-4.
So this still allows for multiple processes to use the same
the card when the first process has verified the pin.

If SM was not  not active, fall back to the SELECT AID
and to a VERIFY for login state.

Note: the SM encoded VERIFY also returns the login state.

This code has been tested by running  `pkcs11-tool --test --login --pin xxxxx`
in teo processes both using SM  and watching  the inter actions
which include having to resestablish SM authenticaion.

 On branch sm-nist
 Changes to be committed:
	modified:   src/libopensc/card-piv.c
Makefile.mak in 5 dirs - build libsmnist on windows

 Changes to be committed:
	modified:   src/libopensc/Makefile.mak
	modified:   src/minidriver/Makefile.mak
	modified:   src/pkcs11/Makefile.mak
	modified:   src/sm/Makefile.mak
	modified:   src/smm/Makefile.mak
…_content

nist-decode_cvc and nist_clear_cvc_content were renamed and exported via libopensc.exports
nist_cvc_t was moved to sm-nist.h

This allows them to be callable from outside of sm-nist to parse a CVC certificate.
To be added later is sm_nist_encode_cvc

 On branch myeid-4.9.1
 Changes to be committed:
	modified:   sm/sm-nist.c
	modified:   sm/sm-nist.h
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.

5 participants