Simplify differences between CardOS 5 versions and unbreak 5.3 signatures

[Jakuje]

This is an alternative approach to the #1079, which resolves the same problem (and I somehow forgot to fill the PR earlier), but also unbreaks the signatures too as stated in #1055.

On the other hand, it "breaks" back the mechanisms advertised by CardOS cards by adding RSA_X_509 to the list which, in fact, is not supported by the card operations.

I would appreciate ideas how to improve this situation sooner or later. Currently, this fact is making the tests in pkcs11-tool failing, because the RSA_X_509 signatures do not work.

[frankmorgner]

• 👍 for making the driver code simpler and making CardOS 5.0 support improvement #1079 obsolete
• But I suspect that the problem with RSA_X_509 could break some real world applications...

Since I'm not a user of this card, I'm somewhat undecided...

[frankmorgner]

@debrouxl could you have a look?

[Jakuje]

@frankmorgner that is why I am asking somebody with better overview about the project architecture. I believe we will see more cards that would not allow using raw signatures.

I can try to add back SC_ALGORITHM_RSA_PAD_PKCS1 to the flags and add the PKCS#1 padding "manually" in the cardos_compute_signature(), which should result in the correct flags and hopefully correct signature. Lets give it a try.

[Jakuje]

To clarify what is the CardOS driver trying to do with RSA-X-509 (RSA_SIG internal mechanism):

message[256 B] ->
| OpenSC -> removes PCKCS#1.5 padding, digest info -> message [~30 B] -> card [adds a padding back] |
-> signature [256 B]

The signature can be verified using OpenSSL as a data with valid PKCS#1.5 padding which was added by the card. This works fine for short messages but clearly fails for more than ~100B (that can not be padded? Tested with pkcs11-tool by adding variable padding of length from 155 bytes).

Interesting thing is that if I use the current master, the resulting signature of RSA-PKCS (using RSA_PURE_SIG) are what we would expect from RSA-X-509 (~30 B).

message[~30 B] ->
| OpenSC -> card |
-> signature [~30 B]

But I am not able to verify that signature as a raw using OpenSSL. With this PR and short-enough message I am able to get verifiable signature with some mess around.

So I still do not have any better idea how this can be resolved. Ideas welcomed. I can provide the full logs for specific inputs if it could help. Otherwise I would consider this solution as a "good enough".

[debrouxl]

I've tested these two commits applied on top of current master.
With my CardOS 5.0 card, the sets of commands from https://gist.github.com/Jakuje/5a993d2b2d8a9cac35203599e49e6831 show that signature and validation work, but decryption fails with CKR_GENERAL_ERROR:
"
Using slot 0 with a present token (0x0)
Using decrypt algorithm RSA-PKCS
error: PKCS11 function C_Decrypt failed: rv = CKR_GENERAL_ERROR (0x5)
Aborting.
"
"
Using slot 0 with a present token (0x0)
Using decrypt algorithm RSA-X-509
error: PKCS11 function C_Decrypt failed: rv = CKR_GENERAL_ERROR (0x5)
Aborting.
"

Likewise for pkcs11-tool -tl, as expected.

[Jakuje]

Thank you for testing.
That is unfortunately "expected". The decryption methods are not implemented in the CardOS driver so it is using generic ISO methods that do not work here.

I have somewhere a patch that tries to resolve this (by adding a pkcs1 padding back), but we can never know what were the random bytes in padding (which was stripped by the card) so rather than trying to return wrong decrypted message, I would rather say it is not supported (as it never was to my understanding).

[frankmorgner]

What about applying #1079 as quick fix for 0.17.0 and after the release we can merge this PR (#1080)? This should give us more time for testing with real world applications (or even more time for coming up with a better solution).

[debrouxl]

With my CardOS 5.0 card, applying #1079 on top of #1003 (commit 8f33305) turns: " $ pkcs11-tool Using slot 0 with a present token (0x0) Logging in to "PIN (PKCS#15 Atos User Card + OT". Please enter User PIN: 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 RIPEMD160: OK Signatures (currently only for RSA) testing key 0 (<REDACTED 1>) error: PKCS11 function C_SignFinal failed: rv = CKR_DATA_INVALID (0x20) Aborting. " into: " $ pkcs11-tool -tl Using slot 0 with a present token (0x0) Logging in to "PIN (PKCS#15 Atos User Card + OT". Please enter User PIN: 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 RIPEMD160: OK Signatures (currently only for RSA) testing key 0 (<REDACTED 1>) all 4 signature functions seem to work testing signature mechanisms: RSA-X-509: OK RSA-PKCS: OK SHA1-RSA-PKCS: OK MD5-RSA-PKCS: OK RIPEMD160-RSA-PKCS: OK SHA256-RSA-PKCS: OK testing key 1 (2048 bits, label=<REDACTED 2>) with 1 signature mechanism RSA-X-509: OK Verify (currently only for RSA) testing key 0 (<REDACTED 1>) RSA-X-509: OK RSA-PKCS: OK SHA1-RSA-PKCS: OK MD5-RSA-PKCS: OK RIPEMD160-RSA-PKCS: OK testing key 1 (<REDACTED 2>) with 1 mechanism RSA-X-509: OK Unwrap: not implemented Decryption (currently only for RSA) testing key 0 (<REDACTED 1>) RSA-X-509: resulting cleartext doesn't match input Original: 61 62 63 64 65 66 67 68 69 00 Decrypted: <snip many bytes> 00 61 62 63 64 65 66 67 68 69 00 RSA-PKCS: OK testing key 1 (<REDACTED 2>) RSA-X-509: resulting cleartext doesn't match input Original: 61 62 63 64 65 66 67 68 69 00 Decrypted: <snip many bytes> 00 61 62 63 64 65 66 67 68 69 00 RSA-PKCS: OK 2 errors " and the sign / verify commands from https://gist.github.com/Jakuje/5a993d2b2d8a9cac35203599e49e6831 work with both algorithms, whereas they don't before the change. So the fact of using apdu.p1 = 0x41 on CardOS 5.0 cards as well as on CardOS 5.3 cards, as suggested by Jakuje in #1003, still improves CardOS 5.0 support a lot, as I wrote at the time :) _However_... applying #1079 alone on top of current master does *not* produce a working CardOS 5.0 support (when testing several days ago, I probably didn't actually test the one-liner against current master...): " $ pkcs11-tool -tl Using slot 0 with a present token (0x0) Logging in to "PIN (PKCS#15 Atos User Card + OT". Please enter User PIN: 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 RIPEMD160: OK Signatures (currently only for RSA) testing key 0 (<REDACTED 1>) error: PKCS11 function C_SignFinal failed: rv = CKR_GENERAL_ERROR (0x5) Aborting. " and the commands from the gist linked above fail with the same CKR_GENERAL_ERROR (0x5). I performed a bisection, here's the log (#1079 one-liner applied manually at every step): git bisect start # good: [8f33305] Make CardOS 5.3 working with OpenSC (#1003) git bisect good 8f33305 # bad: [7e28c1b] [cac] Correctly select APDU CASE to unbreak get_challenge git bisect bad 7e28c1b # bad: [eb19691] added compatibility with WiX 3.11 git bisect bad eb19691 # bad: [f5aa3f5] build fix for libressl 2.5.3 git bisect bad f5aa3f5 # bad: [fcc8ea5] reader-pcsc: removed cardmod driver git bisect bad fcc8ea5 # good: [1ca09b8] pkcs11-tool: Do not use unitialized data when C_GetTokenInfo() failed git bisect good 1ca09b8 # good: [e6f7373] Added a check to sc_pkcs15_verify_pin to find out if the access condition is already open on card. This check is performed only if this function is called with empty data. This change fixes a problem with pinpad readers, when PIN cache is disabled and prevents unnecessary PIN queries. git bisect good e6f7373 # good: [8cf68bc] Improved creation of key files so that the correct security attributes are set and keys can be created under specific PINs. Previously keys were always created under PIN 1. Changed description of myeid_create_key function. git bisect good 8cf68bc # first bad commit: [fcc8ea5] reader-pcsc: removed cardmod driver Applying #1080 to current master clearly improves things (both `pkcs11-tool -tl` and the commands from the gist), as I wrote above... but isn't there _another_ problem somehow caused by the commit pointed by the bisection ?

[Jakuje]

The fix in #1079 does not resolve the problem completely. There are also bad flags set up during initialization. They were already present in the #1003, but are fixed in this PR. Applying this PR is that minimal fix that I would like to see in 0.17.0.

The pkcs11-tool --test is failing because of the advertised RSA_X_509 mechanism, which is in fact not supported (for generic data sizes). I am able to make it pass if I modify the data to have proper PKCS#1.5 padding and the message shorter than 100 bytes (see following patch). Then it fails little bit further:

diff --git a/src/tools/pkcs11-tool.c b/src/tools/pkcs11-tool.c
--- a/src/tools/pkcs11-tool.c
+++ b/src/tools/pkcs11-tool.c
@@ -4312,6 +4312,10 @@ static int test_signature(CK_SESSION_HANDLE sess)
 	if (firstMechType == CKM_RSA_X_509) {
 		/* make sure our data is smaller than the modulus */
 		data[0] = 0x00;
+		data[1] = 0x01;
+		for (i = 2; i < 154; i++)
+			data[i] = 0xff;
+		data[i] = 0x00;
 	}
 
 	ck_mech.mechanism = firstMechType;

[frankmorgner]

OK, then we'll accept this PR. Thanks for giving the details.

The change in pkcs11-tool would only mask the real problem (advertising RSA_X_509 though not supporting it), so I'd rather not do this change.

[frankmorgner]

On the PKCS#15 layer, the card could initialize p15card->tokeninfo->supported_algos ,
which allows to be more explicit (see comments here #508 (comment))

[Jakuje]

Thank you for the pointer. I read through the comments (specially the linked one), but still I am not sure what is your proposed solution and how clean clean would it be.
Modifying the initialized private key structures that carries the list of supported algorithms with all we know sounds for me very cumbersome. But if there will not be any other possibility, I will try to do that.