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

[dengert]

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]

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]

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]

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]

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]

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.