ECDSA flags #2190

dengert · 2020-12-21T18:35:43Z

ECDSA Signatures with hashes

Fixes:#2181 (Partially)

This PR is based on discussion with @popovec in
https://github.com/OpenSC/OpenSC/issues/2181
and https://github.com/OpenSC/OpenSC/pull/2187
which was cherry-picked as 5e5300816c8

This has been tested with PIV, MyEID and Smartcard-HSM  with ECDSA keys.

The main fixes include :
 - Setting "flags" in card drivers
 - added code to sc_pkcs15-compute-signature for handle ECDSA with hashes
 - code in framework-pkcs15.c

Signatures made by pkcs11-tool -sign,  verify with openssl dgst
but pkcs11-tool --verify  does not work with ECDSA but does with RSA
I suspect it has to do with:
and some  then creating the wrong PKCS11 mechanisms

It should work with the epass2003 which does hashes in the driver.

Checklist

PKCS#11 module is tested

CKM_ECDSA and CKM_ECDSA_SHA1 cannot be registered in the same way. We need to use sc_pkcs11_register_sign_and_hash_mechanism () for CKM_ECDSA_SHA1. This fix also enables more ECDSA-SHAxxx mechanisms in framework-pkcs15.c Tested: MyEID 4.0.1 (secp256r1 with SHA1, SHA224, SHA256, SHA384, SHA512) CI tests (Travis + OsEID) for ECDSA-SHAxxx mechanisms are also enabled.

@popovec

This PR is based on discussion with @popovec in OpenSC#2181 and OpenSC#2187 which was cherry-picked as 5e53008 This has been tested with PIV, MyEID and Smartcard-HSM. with ECDSA keys. The main fixes include : - Setting "flags" in card drivers - added code to sc_pkcs15-compute-signature for handle ECDSA with hashes - code in framework-pkcs15.c Signatures made by pkcs11-tool -sigm verify with openssl but pkcs11-tool --verify does not work with ECDSA but does with RSA I suspect it has to do with: and some then creating the wrong PKCS11 mechanisms It should work with the epass2003 which does hashes in the driver.

popovec · 2020-12-21T21:01:34Z

Thanks, I looked at patch - looks good.

There is also SC_ALGORITHM_ECDSA_HASH_SHA1 in isoApplet ? Is this OK ?
src/libopensc/card-isoApplet.c: flags |= SC_ALGORITHM_ECDSA_HASH_SHA1;

Just one comment to ECDSA verify .. ECDSA verify operation is completely different from RSA.

RSA is running in symmetric way i.e. what is encrypted by public key this is decrypted by private key .. and in reverse.. if I do encrypt with private key = signature , then decrypt with public key return back the signature = verify.

ECDSA has no such symmetry, even private key is only one number, public key two numbers. Repeated signing of the same data will return different numbers (r, s) in each signature. This is because a different temporal key is used. From a mathematical point of view, ECDSA verification is a different operation than an ECDSA signature.

Maybe some of (OpenSC) supported cards offer PSO Verify digital signature (APDU 00 2a 00 a8), but I didn't notice that opensc had support for this operation.

It is likely that ECDSA verification support must be completely rewritten and left to openssl.

The --signature-format openssl in pkcs11-tool does the correct operation to convert the OpenSSL formated signature to rs for PKCS11 This commit modifies pkcs11/openssl.c to convert back to sequence for EVP_VerifyFinal Without this mod the signature file was passed unmodified to PKCS11, then to EVP_VerifyFinal but this violates PKCS11 standard. On branch ECDSA-flags Changes to be committed: modified: openssl.c

dengert · 2020-12-22T19:12:03Z

This last commit fixes the problem with opensc-pkcs11.so verify of ECDSA-SHA* signatures.

The pkcs11-tool verify --signature-format openssl now works as expected when reading a ECDSA signature file in the openssl sequence format, it is converted to the PKCS11 format of r,s. the pkcs11/openssl.c will now that the r,s and convert to openssl sequence for EVP_VerifyFinal.

popovec · 2020-12-24T10:26:33Z

I tested this patch with MyEID and OsEID card, all ECDSA-SHAxxx mechanism seems running ok (with all supported EC keys). Verification works as expected, except verify of RAW ECDSA...

echo "test" > testfile.txt
pkcs11-tool --pin 1111 --sign -m ECDSA --signature-format openssl --input-file testfile.txt --output-file testfile.txt.pkcs11.sig --id 02

pkcs11-tool --verify -m ECDSA --signature-format openssl --input-file testfile.txt --signature-file testfile.txt.pkcs11.sig --id 02
Using signature algorithm ECDSA
error: PKCS11 function C_VerifyFinal failed: rv = CKR_ARGUMENTS_BAD (0x7)
Aborting.

I added some code into openssl.c (patch below) .. now ECDSA verify is working. It would be good to integrate this code into your patch.

diff --git a/src/pkcs11/openssl.c b/src/pkcs11/openssl.c
index 51d6e102..6f0c58fe 100644
--- a/src/pkcs11/openssl.c
+++ b/src/pkcs11/openssl.c
@@ -518,6 +518,44 @@ CK_RV sc_pkcs11_verify_data(const unsigned char *pubkey, unsigned int pubkey_len
                        sc_log(context, "EVP_VerifyFinal() returned %d\n", res);
                        return CKR_GENERAL_ERROR;
                }
+       } else if (md == NULL && mech->mechanism == CKM_ECDSA){
+               size_t signat_len_tmp;
+               unsigned char *signat_tmp = NULL;
+               EVP_PKEY_CTX *ctx;
+
+               sc_log(context, "Trying to verify using EVP");
+
+               if (NULL == EVP_PKEY_get0_EC_KEY(pkey)) {
+                       sc_log(context, "Not correct key type\n");
+                       EVP_PKEY_free(pkey);
+                       return CKR_KEY_TYPE_INCONSISTENT;
+               }
+                if (sc_asn1_sig_value_rs_to_sequence(NULL, signat,
+                             signat_len, &signat_tmp, &signat_len_tmp)){
+                             sc_log(context,"Failed to convert signature to ASN.1 sequence format");
+                             EVP_PKEY_free(pkey);
+                             return CKR_KEY_TYPE_INCONSISTENT;
+               }
+               ctx = EVP_PKEY_CTX_new(pkey, NULL);
+               if (!ctx){
+                       EVP_PKEY_free(pkey);
+                       return CKR_HOST_MEMORY;
+               }
+               if(!EVP_PKEY_verify_init(ctx)){
+                       EVP_PKEY_free(pkey);
+                       return CKR_SIGNATURE_INVALID;
+               }
+               res = EVP_PKEY_verify(ctx, signat_tmp, signat_len_tmp, data, data_len);
+               EVP_PKEY_free(pkey);
+               if (res == 1)
+                       return CKR_OK;
+               else if (res == 0) {
+                       sc_log(context, "EVP_PKEY_verify: Signature invalid");
+                       return CKR_SIGNATURE_INVALID;
+               } else {
+                       sc_log(context, "EVP_PKEY_verify returned %d\n", res);
+                       return CKR_GENERAL_ERROR;
+               }
        } else {
                RSA *rsa;
                unsigned char *rsa_out = NULL, pad;

dengert · 2020-12-24T13:45:00Z

Yesterday opendnssec/SoftHSMv2#590 fixed a bug for EVP_md_null() OpenSC has not used this in the past, but I would bet we could use it for ECDSA and even RSA.
Note the comment in openssl.c:

487                 /* This does not really use the data argument, but the data
488                  * are already collected in the md_ctx
489                  */

Are you willing to look at this? If not, your mod should work.

popovec · 2020-12-24T15:19:22Z

Thanks for the information. I'll look into it.

Openssl offers more options for signature verification. src/pkcs11/openssl.c already uses EVP_PKEY_verify() in CK_RV gostr3410_verify_data() function. Before EVP_PKEY_verify() call, the EVP_PKEY_CTX_set_signature_md() function can be inserted - here specific hash function can be attached. Example code: https://www.openssl.org/docs/manmaster/man3/EVP_PKEY_verify_init.html

popovec · 2020-12-27T09:08:55Z

I tested the solution using EVP_DigestVerifyInit .. (Openssl from debian version 1.1.1d-0+deb10u4):

	if (!EVP_DigestVerifyInit(ctx, NULL, NULL, NULL, pkey))
	if (!EVP_DigestVerifyInit(ctx, NULL, EVP_md_null(), NULL, pkey))

for EVP_md_null() - EVP_DigestVerifyInit returns error
for EVP_sha256() - EVP_DigestVerifyInit returns OK, but of course verification fails
for NULL - EVP_DigestVerifyInit returns OK, verification fails

The function EVP_PKEY_CTX_set_signature_md(ctx,...) seems have a similar problem. I can attach EVP_sha256() but for EVP_md_null() this function return an error. If this function is not called then ECDSA verification is working.

For completeness, if anyone would like to test this (of course, this is just a part of the code that should be used with the above patch):

if 1
                EVP_PKEY_CTX *ctx;
                ctx = EVP_PKEY_CTX_new(pkey, NULL);
                if (!ctx){
                        EVP_PKEY_free(pkey);
                        return CKR_HOST_MEMORY;
                }
                if(!EVP_PKEY_verify_init(ctx)){
                        EVP_PKEY_CTX_free(ctx);
                        EVP_PKEY_free(pkey);
                        return CKR_FUNCTION_FAILED;
                }
                if (EVP_PKEY_CTX_set_signature_md(ctx, EVP_md_null) <= 0) {
                        EVP_PKEY_CTX_free(ctx);
                        EVP_PKEY_free(pkey);
                        return CKR_FUNCTION_FAILED;
                }
                res = EVP_PKEY_verify(ctx, signat_tmp, signat_len_tmp, data, data_len);
                EVP_PKEY_CTX_free(ctx);
#else
                EVP_MD_CTX* ctx;
                ctx = EVP_MD_CTX_new();
                if (!ctx){
                        EVP_PKEY_free(pkey);
                        return CKR_HOST_MEMORY;
                }
                if (1 != EVP_DigestVerifyInit(ctx, NULL, EVP_md_null(), NULL, pkey)) {
                        EVP_MD_CTX_free(ctx);
                        EVP_PKEY_free(pkey);
                        return CKR_FUNCTION_FAILED;
                }
                res = EVP_DigestVerify(ctx, signat_tmp, signat_len_tmp, data, data_len);
                EVP_MD_CTX_free(ctx);
#endif

dengert · 2020-12-28T19:46:09Z

Sounds like your mod is better then trying to use EVP_md_null.

popovec · 2020-12-29T09:48:06Z

Details from openssl:

The EVP_DigestVerifyInit() function internally calls the EVP_PKEY_CTX_set_signature_md() function. The EVP_md_null digest is then attached to ECDSA verification via pkey_ec_ctrl () function.

Here relevant part (openssl from debian sources 1.1.1i):

    case EVP_PKEY_CTRL_MD:
        if (EVP_MD_type((const EVP_MD *)p2) != NID_sha1 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_ecdsa_with_SHA1 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha224 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha256 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha384 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha512 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha3_224 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha3_256 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha3_384 &&
            EVP_MD_type((const EVP_MD *)p2) != NID_sha3_512) {
            ECerr(EC_F_PKEY_EC_CTRL, EC_R_INVALID_DIGEST_TYPE);
            return 0;
        }

It seems to me, that it is not possible to use EVP_md_null for ECDSA verification.

mouse07410 · 2020-12-29T20:33:08Z

Does it make sense from security point of view to allow raw ECDSA?

dengert · 2020-12-30T01:39:10Z

PKCS#11 defines the CKM_ECDSA so OpenSC supports it. But that does not answer your question. The answer may be ECDSA is not used for encryption. For signature verification the verifier knows the data to be signed and thus the hash. Or knows or brut force tries to guess the short data. But if found they have not learned anything. The r,s is used to prove the signer knows the private key not the hash.

…

On Tue, Dec 29, 2020, 2:33 PM Mouse ***@***.***> wrote: Does it make sense from security point of view to allow raw ECDSA? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2190 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGTIMPTN4NZ2F3MCJCBQGDSXI4JDANCNFSM4VESSDVA> .

popovec

if (EVP_PKEY_get0_EC_KEY(pkey)) - if here NULL is returned, key is inconsistent witch the ECDSA operation. I suggest to return CKR_KEY_TYPE_INCONSISTENT in this case, and not to call EVP_VerifyFinal()

dengert · 2020-12-30T11:20:00Z

Are you referring to line 493? if (EVP_PKEY_get0_EC_KEY(pkey)) The pkey at this point could also be RSA. This tests for EC vs RSA.

popovec · 2020-12-30T11:44:51Z

Are you referring to line 493? if (EVP_PKEY_get0_EC_KEY(pkey)) The pkey at this point could also be RSA. This tests for EC vs RSA.

This test is fine, if the key is not an EC key, we cannot proceed .. line 507:
EVP_VerifyFinal(md_ctx, signat, signat_len, pkey)

I think it would be better to change line 507:
return CKR_KEY_TYPE_INCONSISTENT;

johughes99 · 2020-12-30T11:48:22Z

There is no real difference between the security of raw ECDSA (ie CKM_ECDSA) and those using "strong" hashing" such as CKM_ECDSA_SHA256. In recent years the weakness in ECDSA implementations has been because of the poor selection of the random number (i.e. playstation 3 attack) or side channel attacks due to attacks on scalar arithmetic (see Minerva attack).

I think there are use cases for raw ECDSA where small amounts of data are exchanged for signing/verification.

To minimize attacks on the random number aspects "deterministic" ECDSA can be used (see RFC 6979)- where a random number number is not used. Although I'm not sure a PKCS#11 mechanism has been defined for this - even in 3.0

dengert · 2020-12-30T12:51:12Z

Look at original code:

if (md_ctx) {
res = EVP_VerifyFinal(md_ctx, signat, signat_len, pkey);

This works for RSA.

The mod inserted an EC only version:
if (EVP_PKEY_get0_EC_KEY(pkey)) {
convert r,s to openssl format
EVP_VerifyFinal(md_ctx, signat_tmp, signat_len_tmp, pkey);
} else
original code for RSA:
res = EVP_VerifyFinal(md_ctx, signat, signat_len, pkey);

popovec · 2020-12-30T14:48:31Z

Look at original code:

if (md_ctx) {
res = EVP_VerifyFinal(md_ctx, signat, signat_len, pkey);

This works for RSA.

Oh yes, you're right. Currently, only the public RSA or public EC key can be found here. If it's not an EC key, we can really complete the operation with the RSA key.

src/pkcs11/framework-pkcs15.c

On branch ECDSA-flags Changes to be committed: modified: framework-pkcs15.c

popovec and others added 2 commits December 21, 2020 11:47

dengert mentioned this pull request Dec 29, 2020

fix for issue #589: specify EVP_md_null() as hashing algorithm for EdDSA sig and vfy opendnssec/SoftHSMv2#590

Closed

popovec reviewed Dec 30, 2020

View reviewed changes

popovec mentioned this pull request Jan 5, 2021

ECDSA-SHA1 - unable to sign file/wrong signature #2181

Closed

Jakuje reviewed Jan 5, 2021

View reviewed changes

src/pkcs11/framework-pkcs15.c Outdated Show resolved Hide resolved

unused code removed

b7b10c0

On branch ECDSA-flags Changes to be committed: modified: framework-pkcs15.c

frankmorgner approved these changes Jan 6, 2021

View reviewed changes

frankmorgner mentioned this pull request Jan 22, 2021

ECDSA-SHA1: Apply SHA1 to input data before PSO compute signature. #2187

Closed

1 task

frankmorgner merged commit 0b0deae into OpenSC:master Jan 24, 2021

Jakuje mentioned this pull request Jan 25, 2021

EC flags for EC mechanisms #2209

Merged

3 tasks

Jakuje mentioned this pull request Mar 1, 2021

Towards new release 0.22.0 #2247

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ECDSA flags #2190

ECDSA flags #2190

dengert commented Dec 21, 2020

popovec commented Dec 21, 2020

dengert commented Dec 22, 2020

popovec commented Dec 24, 2020

dengert commented Dec 24, 2020

popovec commented Dec 24, 2020

popovec commented Dec 27, 2020

dengert commented Dec 28, 2020

popovec commented Dec 29, 2020

mouse07410 commented Dec 29, 2020

dengert commented Dec 30, 2020 via email

popovec left a comment

dengert commented Dec 30, 2020

popovec commented Dec 30, 2020

johughes99 commented Dec 30, 2020

dengert commented Dec 30, 2020

popovec commented Dec 30, 2020

ECDSA flags #2190

ECDSA flags #2190

Conversation

dengert commented Dec 21, 2020

Checklist

popovec commented Dec 21, 2020

dengert commented Dec 22, 2020

popovec commented Dec 24, 2020

dengert commented Dec 24, 2020

popovec commented Dec 24, 2020

popovec commented Dec 27, 2020

dengert commented Dec 28, 2020

popovec commented Dec 29, 2020

mouse07410 commented Dec 29, 2020

dengert commented Dec 30, 2020 via email

popovec left a comment

Choose a reason for hiding this comment

dengert commented Dec 30, 2020

popovec commented Dec 30, 2020

johughes99 commented Dec 30, 2020

dengert commented Dec 30, 2020

popovec commented Dec 30, 2020