Reselection of DF after failure in `sc_pkcs15_decipher` function about opensc HOT 5 CLOSED

With 0494e46 in pkcs15-framwork.c sc_pkcs15_decrypt is called twice if first fails and not using PKCS#1 v1.5.

So why limit the test to padding to PKCS#1 v1.5?

It looks like ca08e97 from 2011 was trying to: "Limit the number of cases when applicated re-selection of application DF to strict minimum." It calls sc_pkcs15_decrypt a second time if it failed for any reason trying to avoid having to call reselect_app_df. I don't know what cards are depending on this feature.

The PIV card does not depend on this, as it uses the sc_card_operations card_reader_lock_obtained callback to
select the PIV AID when the first lock is obtained after SCardBeginTransaction locks the reader. Nine other card drivers also use this callback.

So the real fix maybe removing most if not all of ca08e97 and making sure all card drivers use the card_reader_lock_obtained to reselect_app_df if needed. (A select AID is a form of select app df).

Part of 0494e46

@@ -4609,27 +4611,53 @@ pkcs15_prkey_decrypt(struct sc_pkcs11_session *session, void *obj,
 	rv = sc_pkcs15_decipher(fw_data->p15_card, prkey->prv_p15obj, flags,
 			pEncryptedData, ulEncryptedDataLen, decrypted, sizeof(decrypted), pMechanism);
 
-	if (rv < 0 && !sc_pkcs11_conf.lock_login && !prkey_has_path)
+	/* skip for PKCS#1 v1.5 padding prevent side channel attack */
+	if (!(flags & SC_ALGORITHM_RSA_PAD_PKCS1) &&
+			rv < 0 && !sc_pkcs11_conf.lock_login && !prkey_has_path)
 		if (reselect_app_df(fw_data->p15_card) == SC_SUCCESS)
 			rv = sc_pkcs15_decipher(fw_data->p15_card, prkey->prv_p15obj, flags,
 					pEncryptedData, ulEncryptedDataLen, decrypted, sizeof(decrypted), pMechanism);

from opensc.

The PIV card does not depend on this, as it uses the sc_card_operations card_reader_lock_obtained callback to
select the PIV AID when the first lock is obtained after SCardBeginTransaction locks the reader. Nine other card drivers also use this callback.

Correct. I think we were moving most of the card drivers to use the card_reader_lock_obtained, but it was much later than this commit was created so this might not be needed at all anymore

So the real fix maybe removing most if not all of ca08e97 and making sure all card drivers use the card_reader_lock_obtained to reselect_app_df if needed. (A select AID is a form of select app df).

Yes, that was also my thinking that we should be able to remove this part (not only because it clearly does not play well with the always authenticate keys in PIV).

Does anyone have the backup of the old issue tracker? The issue IDs in this old commit, I think, are from the opensc-project.org and not from github. Lets try our luck @viktorTarasov ?

from opensc.

I agree that always reselecting the applet in case of an error destroys the correct error code. However, card_reader_lock_obtained is currently only used in the scope of the card driver and typically checks whether the card application associated with the card driver is selected. Native OS card drivers (e.g. cardos) are offering DF selection via AID and would all have to implement card_reader_lock_obtained including the state tracking in XXX_select_file on what AID is selected.

In the past, we have removed the use of struct sc_card_cache, because it was causing many issues when concurrently running applications were used. And thinking about this, I believe this could now propably be fixed by setting the cache to some unknown state in card_reader_lock_obtained, meaning that we cannot use a shortcut in path selection and we may have to recover the current DF in case of an error (not only in case of private key operation, but also in case of, for example, EF selection). However, I believe that tracking all this introduces a lot complexity that will be used very rarely.

In my opinion, our best option would be to remove the heuristic introduced with ca08e97, because a) we cannot fix this at the PKCS#11 level and b) use_key() uses sc_lock() to bind the private key selection together with the actual usage of the key so that no other application can interfere. I think sc_pkcs15_encrypt/decrypt_sym() are the only cryptographic operations that do not use use_key() in the end and we may have to review the locking here (it looks like they have a misplaced logging command about sc_lock without actually locking the card...). In short, yes remove it, but please in the next release.

from opensc.

Sounds like a plan. I would suggest that @xhanulik start with the decrypt, as reselect is causing problems with the "prevent side channel attack"

from opensc.

Sure, I will take look into it.

from opensc.