Pkcs11 engine causes crash in OpenSSL 1.1.0b in RSA_public_decrypt.

[nased0]

Hello! I have compiled libp11 in VS2015 for OpenSSL 1.1.0b by modifying OPENSSL_LIB in make.rules.mak . Unfortunately engine pkcs11.dll crashes OpenSSL 1.1, because it does not set RSA method "rsa_pub_dec" - rsa->meth->rsa_pub_dec is a NULL. pointer! In contrary, RSA method rsa_priv_dec points to to "pkcs11.dll ! pkcs11_rsa_priv_dec_method()". Please correct this error, I really need working pkcs11 engine for my application, and I'm fed up with memory leaks in OpenSSL 1.0.2!

[dengert]

@mouse07410 said: "so the list appears to me more about what pkcs11-tool can do rather than what the token supports directly"

Its not about what pkcs11-tool can do at all. Its about what the PKCS#11 module (opensc-pkcs11.so) can do in software and/or hardware. The list comes from the module and lists the mechanisms the module and token can do.

The module says it can do: RSA-X-509, keySize={1024,3072}, hw, decrypt, sign, verify Says it can do RSA RAW on the token. RSA-PKCS, keySize={1024,3072}, hw, decrypt, sign, verify Says it can do padding then RAW RSA.

The last "hw" may be misleading, as RSA-PKCS is two operations, and there is no way to say padding done in software and RSA RAW done in hardware. There is no padding only mechanism.

[mouse07410]

Its not about what pkcs11-tool can do at all. Its about what the PKCS#11 module (opensc-pkcs11.so) can do in software and/or hardware. The list comes from the module and lists the mechanisms the module and token can do.

Thank you! So that list is what this module can do with this token? And how the work is split between the module and the token, is not shown at all?

That explains a lot.

[nased0]

pkcs11-tool.exe --module enigmap11.dll -M Auto configuration failed 1096:error:260B6091:engine routines:DYNAMIC_LOAD:version incompatibility:.\crypto\engine\eng_dyn.c:487: 1096:error:260BC066:engine routines:INT_ENGINE_CONFIGURE:engine configuration error:.\crypto\engine\eng_cnf.c:204:section=pkcs11_section, name=dynamic_path, value=C:/DIAGOGOL/openssl/bin/pkcs11.dll 1096:error:0E07606D:configuration file routines:MODULE_RUN:module initialization error:.\crypto\conf\conf_mod.c:235:module=engines, value=engine_section, retcode=-1

It seems that the pkcs11-tool.exe has OpenSSL v.1.0.2 statically linked inside, that reads my openssl configuration, that points to engine pkcs11.dll compiled for OpenSSL v. 1.1. I will reinstall current engine compiled with mingw for OpenSSL v. 1.0.2j and try again.

[nased0]

pkcs11-tool.exe --module enigmap11.dll -M Using slot 0 with a present token (0x0) Supported mechanisms: RSA-PKCS, keySize={1024,2048}, hw, decrypt, sign RSA-PKCS-KEY-PAIR-GEN, keySize={1024,2048}, hw, generate_key_pair

P.S. There is no verify in RSA-PKCS and no RSA-X-509. I will post results for my cryptoCertum card tomorrow.

[dengert]

What that could be saying is the card only supports RSA when the card does the padding on the card. It does not expose the RSA RAW. I have an old GemSafe which shows with an older version of OpenSC: Supported mechanisms: SHA-1, digest SHA256, digest SHA384, digest SHA512, digest MD5, digest RIPEMD160, digest GOSTR3411, digest RSA-PKCS, keySize={512,1024}, hw, decrypt, sign, verify SHA1-RSA-PKCS, keySize={512,1024}, sign, verify MD5-RSA-PKCS, keySize={512,1024}, sign, verify RSA-PKCS-KEY-PAIR-GEN, keySize={512,1024}, generate_key_pair

(It does not work at all with github master, and should be considered as deprecated or deleted.)

[dwmw2]

Displaying my ignorance here... we're there not attacks with malformed padding which allow bits of the key to be extracted? Might it not make sense for some devices to refuse to play except when they do the padding themselves?

[nased0]

pkcs11-tool.exe --module crypto3PKCS.dll -M Using slot 0 with a present token (0x1) Supported mechanisms: RSA-PKCS-KEY-PAIR-GEN, keySize={768,2048}, hw, generate_key_pair RSA-PKCS, keySize={768,2048}, hw, encrypt, decrypt, sign, verify, wrap, unwrap RSA-PKCS-OAEP, keySize={768,2048}, hw, encrypt, decrypt, wrap, unwrap RSA-X-509, keySize={768,2048}, hw, encrypt, decrypt, sign, verify, wrap, unwrap MD5-RSA-PKCS, keySize={768,2048}, hw, sign, verify SHA1-RSA-PKCS, keySize={768,2048}, hw, sign, verify RIPEMD160-RSA-PKCS, keySize={768,2048}, hw, sign, verify mechtype-70, keySize={768,2048}, hw, sign, verify SHA256-RSA-PKCS, keySize={768,2048}, hw, sign, verify SHA384-RSA-PKCS, keySize={768,2048}, hw, sign, verify SHA512-RSA-PKCS, keySize={768,2048}, hw, sign, verify ECDSA-KEY-PAIR-GEN, keySize={224,320}, hw, generate_key_pair, other flags=0x1900000 ECDSA, keySize={224,320}, hw, sign, verify, other flags=0x1900000 ECDSA-SHA1, keySize={224,320}, hw, sign, verify, other flags=0x1900000 ECDH1-DERIVE, keySize={224,320}, hw, derive, other flags=0x1900000 ECDH1-COFACTOR-DERIVE, keySize={224,320}, hw, derive, other flags=0x1900000 MD5, digest SHA-1, digest RIPEMD160, digest mechtype-597, digest SHA256, digest SHA384, digest SHA512, digest RC2-KEY-GEN, keySize={8,1024}, generate RC2-ECB, keySize={8,1024}, encrypt, decrypt RC2-CBC, keySize={8,1024}, encrypt, decrypt RC2-CBC-PAD, keySize={8,1024}, encrypt, decrypt RC4-KEY-GEN, keySize={8,2048}, generate RC4, keySize={8,2048}, encrypt, decrypt DES-KEY-GEN, generate DES-ECB, encrypt, decrypt DES-CBC, encrypt, decrypt DES-CBC-PAD, encrypt, decrypt DES2-KEY-GEN, generate DES3-KEY-GEN, generate DES3-ECB, encrypt, decrypt DES3-CBC, encrypt, decrypt DES3-CBC-PAD, encrypt, decrypt AES-KEY-GEN, keySize={128,256}, generate AES-ECB, keySize={128,256}, encrypt, decrypt AES-CBC, keySize={128,256}, encrypt, decrypt AES-CBC-PAD, keySize={128,256}, encrypt, decrypt

[mouse07410]

were there not attacks with malformed padding which allow bits of the key to be extracted?

Not of the key, but of the encrypted plaintext. That plaintext could be a "wrapped key", like when transporting a key from one TPM to another. Most smart cards don't do that anyway.

Might it not make sense for some devices to refuse to play except when they do the padding themselves?

It's exactly the opposite. Devices should refuse to do any padding themselves, and only perform encryption/decryption of key-size blocks, thus eliminating the padding oracle altogether.

This is decrypting Padding Oracle, so the original padding doesn't matter. Device processing the padding (and either reporting that the padding on the received ciphertext was bad, or merely taking longer time if the padding is good) is what makes Padding Oracle possible.

The "main" publication was Efficient Padding Oracle Attacks Against Cryptographic Hardware. There are several web sites that discussed it in depth (https://blog.cryptographyengineering.com/2012/06/21/bad-couple-of-years-for-cryptographic.html, http://secgroup.dais.unive.it/wp-content/uploads/2012/11/Practical-Padding-Oracle-Attacks-on-RSA.html, http://crypto.stackexchange.com/questions/3775/how-practical-are-side-channel-attacks-and-how-much-of-a-concern-are-they).

This attack is:

• Against encryption only (it does not translate to signatures), and only when PKCS v1.5 padding is used (OAEP is secure, unless you also allow PKCS v1.5).
• Aimed at Key Wrapping (and does not make sense against anything else, because if you have the ciphertext, the card and the PIN - you can just tell the card to decrypt whatever it is, and be done with it).
• Applicable only when the card tells you whether the padding was correct or not (here the "padding oracle"), thus for the card to be resistant it merely should return the decryption results and leave the padding checks to the calling software.

This attack (and similar) can not compromise RSA private keys on the card.

If you have different attack(s) in mind, please share here.

[dwmw2]

That sounds like what I was vaguely remembering; thanks.

[mouse07410]

I'm not using OpenSSL-1.1.0b, so cannot test the current state. What's the verdict - does the current master run OK with tokens on 1.1.0b? If so, should this issue be closed? If not - what's the remaining problem(s)?

[nased0]

I suspect that the line 214 in eng_front.c !ENGINE_set_RSA(e, PKCS11_get_rsa_method()) || causes memory leaks in my app when engine is linked with OpenSSL 1.0.2j or 1.1.0b It is unacceptable for me, so I comment this line before compiling the engine. I will test it again tomorrow using SoftwareVerify Memory Validator and the current master of the OpenSSL 1.1 and cURL.

This line caused crashes because both current cURL 7.50.3, released on 14th of September 2016 and my program were setting pkcs11 as the default engine. It was corrected recently in (not published officially) cURL's git master and my app. I'd like to note that this harmful line is not necessary for engine pkcs11 operations, only to increase functional reference counter in OpenSSL, so the faulty openssl(1) program does not crash, at the price of memory leaks. But pkcs11 engine cannot do all required RSA operations, hence the crash. And even if it could, the PKCS#11 card that actually does the RSA operations can be removed from the reader! P.S. dwmw2 wrote: Maybe we should provide an MD5 implementation to work around this issue, not an RSA method.

But the line 214 still remains unchanged, so I doubt that the engine pkcs11 behavior has improved with OpenSSL 1.1.0b or cURL 7.50.3. Edit: confirmed - current libp11-master's pkcs11.dll crashes both in OpenSSL 1.1.0b and cURL 7.50.3, like in my first posts in this thread. Current master of OpenSSL 1.1 crashes too when I use: openssl s_client -engine pkcs11 -msg -tls1_1 -CAfile cck.pem -connect 172.25.223.7:443 or openssl s_client -msg -cert its1_crt.pem -key its1_key.pem -CAfile cck.pem -no_tls1_2 -connect 172.25.223.7:443

[nased0]

And I think that it is not your fault that Encard's enigmap11.dll CRYPTOKI module cannot do what it declares is able to do (RSA-PKCS padding and signing of block of 83 bytes).

[nased0]

I have tested pkcs11.dll using my app under Memory Validator, OpenSSL 1.1.0b, current cURL master and there are significant memory leaks (increasing with every OpenSSL communication session). (https://cloud.githubusercontent.com/assets/13164219/19928576/456f5036-a0ff-11e6-8f63-afa21088c22c.gif) P.S. I would prefer some harmless operation instead of RSA, like MD5, to allow to use command openssl s_client

[nased0]

Hello again. I have discovered, that when i define OPENSSL_NO_RSA in eng_front.c openssl s_client -engine pkcs11 -msg -tls1_1 -CAfile cck.pem -connect 172.25.223.7:443 works fine, but the following command, that works with unmodified engine, starts to crash:

openssl req -engine pkcs11 -new -key slot_0-id_d7f4b99792cc4dd708e408d3eb91b566e0a06c02 \ -keyform engine -x509 -out req.pem -text -days 365 -subj "/CN=Adam Smith"

So it is not a viable solution for OpenSSL 1.1 to define OPENSSL_NO_RSA. IMO you need to define functions pkcs11_public_decrypt and pkcs11_public_encrypt in p11_rsa.c

P.S. The following commands fail when I'm using OpenSSL 1.1 and unmodified or modified engine:

openssl x509 -engine pkcs11 -signkey slot_0-id_d7f4b99792cc4dd708e408d3eb91b566e0a06c02 \ -keyform engine -in req.pem -out test.pem

engine "pkcs11" set. x509: Cannot open input file slot_0-id_d7f4b99792cc4dd708e408d3eb91b566e0a06c02, No such file or directory x509: Use -help for summary.

OpenSSL x509 -engine pkcs11 -signkey "pkcs11:type=private;pin-value=1234" -keyform engine -in req.pem -out cert.pem

engine "pkcs11" set. x509: Cannot open input file pkcs11:type=private;pin-value=1234, No such file or directory x509: Use -help for summary.

This command works with unmodified engine, but crashes when OPENSSL_NO_RSA is set: OpenSSL req -engine pkcs11 -new -key "pkcs11:type=private;pin-value=1234" -keyform engine -out req.pem -text -x509 -subj "/CN=Adam Smith"

P.S. 2 The above commands work fine with OpenSSL 1.0.2 unmodified engine (as well as command: openssl s_client).

[mtrojnar]

@nased0 I think https://github.com/OpenSC/libp11/commit/f160e2fce640a577597b90fd9c647fc268b00867 and https://github.com/OpenSC/libp11/commit/1029978e00142f13a158a750def4eb279a4a868c should fix the null pointer dereference of rsa->meth->rsa_pub_dec. Can you confirm that the original issue is fixed in the current master branch? Otherwise, please submit a stack backtrace.

The thread seems to also discuss several other unrelated topics. Please open separate issues for each of those topics. Otherwise, it is really hard to track them.

[nased0]

Sorry, my test was faulty, because I tested debug openssl 1.1.0c with release pkcs11. It crashes on s_client exactly like in first post, because function pointer rsa->meth->rsa_pub_dec is null.

libcrypto-1_1.dll!RSA_public_decrypt(int flen, const unsigned char from, unsigned char to, rsa_st rsa, int padding) Line 49 C libcrypto-1_1.dll!int_rsa_verify(int type, const unsigned char m, unsigned int m_len, unsigned char rm, unsigned int prm_len, const unsigned char sigbuf, unsigned int siglen, rsa_st rsa) Line 143 C libcrypto-1_1.dll!RSA_verify(int type, const unsigned char m, unsigned int m_len, const unsigned char sigbuf, unsigned int siglen, rsa_st rsa) Line 247 C libcrypto-1_1.dll!pkey_rsa_verify(evp_pkey_ctx_st ctx, const unsigned char sig, unsigned int siglen, const unsigned char tbs, unsigned int tbslen) Line 230 C libcrypto-1_1.dll!EVP_PKEY_verify(evp_pkey_ctx_st ctx, const unsigned char sig, unsigned int siglen, const unsigned char tbs, unsigned int tbslen) Line 97 C libcrypto-1_1.dll!EVP_DigestVerifyFinal(evp_md_ctx_st ctx, const unsigned char sig, unsigned int siglen) Line 168 C libcrypto-1_1.dll!ASN1_item_verify(const ASN1_ITEM_st it, X509_algor_st a, asn1_string_st signature, void asn, evp_pkey_st pkey) Line 173 C libcrypto-1_1.dll!X509_verify(x509_st a, evp_pkey_st r) Line 26 C libcrypto-1_1.dll!internal_verify(x509_store_ctx_st ctx) Line 1719 C libcrypto-1_1.dll!verify_chain(x509_store_ctx_st ctx) Line 233 C libcrypto-1_1.dll!X509_verify_cert(x509_store_ctx_st ctx) Line 293 C libssl-1_1.dll!ssl_verify_cert_chain(ssl_st s, stack_st_X509 sk) Line 439 C libssl-1_1.dll!tls_process_server_certificate(ssl_st s, PACKET pkt) Line 1226 C libssl-1_1.dll!ossl_statem_client_process_message(ssl_st s, PACKET pkt) Line 624 C libssl-1_1.dll!read_state_machine(ssl_st s) Line 589 C libssl-1_1.dll!state_machine(ssl_st s, int server) Line 385 C libssl-1_1.dll!ossl_statem_connect(ssl_st s) Line 170 C libssl-1_1.dll!ssl3_write_bytes(sslst s, int type, const void buf, int len) Line 377 C libssl-1_1.dll!ssl3_write(ssl_st s, const void buf, int len) Line 3822 C libssl-1_1.dll!SSL_write(ssl_st s, const void buf, int num) Line 1605 C openssl.exe!s_client_main(int argc, char argv) Line 2226 C openssl.exe!do_cmd(lhash_st_FUNCTION prog, int argc, char argv) Line 471 C openssl.exe!main(int argc, char * argv) Line 177 C

[nased0]

But command OpenSSL req -engine pkcs11 -new -key "pkcs11:type=private;pin-value=1234" -keyform engine -out req.pem -text -x509 -subj "/CN=Adam Smith" works correctly

[nased0]

IMO you need to define functions pkcs11_public_decrypt and pkcs11_public_encrypt in p11_rsa.c to ensure that both s_client and req works in openssl 1.1.0c.

[nased0]

@mtrojnar Please re-read my previous comments, I was in a hurry and I made a mistake. When you revert https://github.com/OpenSC/libp11/commit/1029978e00142f13a158a750def4eb279a4a868c command s_client works, but req crashes at instruction cpriv = PRIVCTX(KEY2CTX(key)); in function check_key_fork(PKCS11_KEY *key)

pkcs11.dll!check_key_fork(PKCS11_key_st key) Line 150 C pkcs11.dll!PKCS11_private_encrypt(int flen, const unsigned char from, unsigned char to, PKCS11_key_st key, int padding) Line 432 C pkcs11.dll!pkcs11_rsa_priv_enc_method(int flen, const unsigned char from, unsigned char to, rsa_st rsa, int padding) Line 354 C libcrypto-1_1.dll!RSA_private_encrypt(int flen, const unsigned char from, unsigned char to, rsa_st rsa, int padding) Line 37 C libcrypto-1_1.dll!RSA_sign(int type, const unsigned char m, unsigned int m_len, unsigned char sigret, unsigned int siglen, rsa_st rsa) Line 103 C libcrypto-1_1.dll!pkey_rsa_sign(evp_pkey_ctx_st ctx, unsigned char sig, unsigned int siglen, const unsigned char tbs, unsigned int tbslen) Line 147 C libcrypto-1_1.dll!EVP_PKEY_sign(evp_pkey_ctx_st ctx, unsigned char sig, unsigned int siglen, const unsigned char tbs, unsigned int tbslen) Line 64 C libcrypto-1_1.dll!EVP_DigestSignFinal(evp_md_ctx_st ctx, unsigned char sigret, unsigned int siglen) Line 123 C libcrypto-1_1.dll!ASN1_item_sign_ctx(const ASN1_ITEM_st it, X509_algor_st algor1, X509_algor_st algor2, asn1_string_st signature, void asn, evp_md_ctx_st ctx) Line 209 C libcrypto-1_1.dll!X509_sign_ctx(x509_st x, evp_md_ctx_st ctx) Line 53 C openssl.exe!do_X509_sign(x509_st x, evp_pkey_st pkey, const evp_md_st md, stack_st_OPENSSL_STRING sigopts) Line 1481 C openssl.exe!req_main(int argc, char argv) Line 647 C openssl.exe!do_cmd(lhash_st_FUNCTION prog, int argc, char argv) Line 471 C openssl.exe!main(int argc, char argv) Line 177 C

[mtrojnar]

IMO you need to define functions pkcs11_public_decrypt and pkcs11_public_encrypt in p11_rsa.c to ensure that both s_client and req works in openssl 1.1.0c.

The code you couldn't find is:

static RSA_METHOD *RSA_meth_new(const char *name, int flags)
{
    RSA_METHOD *meth = OPENSSL_malloc(sizeof(RSA_METHOD));

    if (meth == NULL)
        return NULL;
    memcpy(meth, RSA_get_default_method(), sizeof(RSA_METHOD));
    meth->name = OPENSSL_strdup(name);
    meth->flags = flags;
    return meth;
}

Public key methods are copied from the default OpenSSL values returned by RSA_get_default_method(). Then we only overwrite the private key methods, because PKCS#11 provides its own implementation for them:

RSA_METHOD *PKCS11_get_rsa_method(void)
{
    static RSA_METHOD *ops = NULL;

    if (ops == NULL) {
        alloc_rsa_ex_index();
        ops = RSA_meth_new("libp11 RSA method", 0);
        if (ops == NULL)
            return NULL;
        RSA_meth_set_priv_enc(ops, pkcs11_rsa_priv_enc_method);
        RSA_meth_set_priv_dec(ops, pkcs11_rsa_priv_dec_method);
        RSA_meth_set_finish(ops, pkcs11_rsa_free_method);
    }
    return ops;
}

[mtrojnar]

I managed to reproduce the crash with OpenSSL 1.0.2i-dev. It works fine with OpenSSL 1.1.1.

$ openssl req -engine pkcs11 -new -key 'pkcs11:token=My token 1;object=server;pin-value=1234' -keyform engine -x509 -out req.pem -text -days 365 -subj "/CN=Adam Smith"
p11-kit: invalid config filename, will be ignored in the future: /home/mtrojnar/.config/pkcs11/modules/softhsm2
engine "pkcs11" set.
 3937 file=buf_str.c, line=92, number=5, address=0227F120
 3939 file=dso_lib.c, line=106, number=72, address=022B2010
 2686 file=lhash.c, line=191, number=24, address=02242870
 3946 file=ameth_lib.c, line=289, number=208, address=022B2980
 3950 file=buf_str.c, line=92, number=18, address=0227FE60
 3942 file=dso_dlfcn.c, line=368, number=11, address=0227F600
 3936 file=stack.c, line=164, number=32, address=022B1EC0
 3932 file=eng_dyn.c, line=210, number=88, address=02230290
Segmentation fault (core dumped)

[mtrojnar]

Correction: the crash was caused by the buggy memory leak debugger in an OpenSSL debug build. It works fine on production OpenSSL 1.0.2k-dev.

[nased0]

To reproduce this crash in req in openssl 1.1.0c you need to define OPENSSL_NO_RSA in src/eng_front.c. or comment line 215: // !ENGINE_set_RSA(e, PKCS11_get_rsa_method()) ||

About s_client command on openssl 1.1.0c: please ensure that rsa->meth->rsa_pub_dec and rsa->meth->rsa_pub_enc are not null, then it will not crash. These function pointers should point to default openssl implementation or new pkcs11' s CRYPTOKI implementation. It is funny that commenting line 215 makes s_client command work.

[nased0]

@mtrojnar You should rather improve the code that crashes the memory leak debugger than be happy that it works in the release build. There must be other reason for this crash than buggy debugger :)

[mtrojnar]

To reproduce this crash in req in openssl 1.1.0c you need to define OPENSSL_NO_RSA in src/eng_front.c.

The RSA functionality is not supposed to work when the library was build with OPENSSL_NO_RSA.

[mtrojnar]

The OpenSSL internal memory leak debugger code also crashes with stunnel. This bug is not specific to libp11, and it should be fixed at its source (in OpenSSL).

[nased0]

Ok, thanks for explanations, but we haven't resolved s_client command crash in openssl 1.1.0c, that is main subject of this thread.

[mtrojnar]

I got confused with your initial confirmation that it was fixed. I'll fix it.

[mtrojnar]

The most difficult part of this fix was digging out relevant posts from this really long thread...

[nased0]

I will test this patch tomorrow. I noticed large memory leaks after applying @dengert patch (by using SoftwareVerify Memory Validator and my curl based application).

[mtrojnar]

How exactly do you define "large"? How many bytes per private key operation?

My guess is that it only leaks some structures initially allocated during engine initialization. I other words this leak has no practical impact whatsoever.

[nased0]

I will test it tomorrow, because I left the card and USB reader at work. I'm against memory leaks that increase with every curl openssl communication session that involves pkcs11 engine initialization and deinitialization.

[dengert]

Prior to OpenSSL-1.1, rsa methods were static. There was no RSA_METH*_free

OpenSSL-1.1 has a RSA_METH_free. Does it need to be called?

[mouse07410]

The RSA functionality is not supposed to work when the library was build with OPENSSL_NO_RSA.

👍 :-) :-)

OpenSSL-1.1 has a RSA_METH_free. Does it need to be called?

IMHO it's worth to put this call in (#ifdefing it for 1.1, of course). If it gets pushed in, I can test it on the current 1.1-stable (when I get to work).

[nased0]

@mtrojnar I have tested your patch and it is OK. Commands s_client and req work correctly. There are no new memory leaks, except this one I corrected in https://github.com/OpenSC/libp11/pull/138 When this patch is applied, there are no memory leaks at all.