OpenSSL 3: Enabling pkcs11 engine makes EC operations fail

[vuori]

I built the libp11 and openssl master branches last night, and it seems that the presence of the pkcs11 engine causes problems with various operations on EC keys. Note that the issue occurs if the engine is just present in the configuration, even if it's not being used.

Steps to reproduce:

1. Copy the default openssl.cnf from OpenSSL sources, insert relevant engine_section and pkcs11_section bits from README with dynamic_path pointing to freshly-built pkcs11.so.
2. Try to generate an EC key: openssl ecparam -name brainpoolP256r1 -genkey -out t1.key. Note that it fails with error 80A26D89627F0000:error:078C0102:common libcrypto routines:get_string_internal:passed a null parameter:crypto/params.c:1258:. Comment out pkcs11 = pkcs11_section, try again and the key is generated.
3. Re-enable the engine and try to generate a self-signed cert: openssl req -subj /CN=ectest -new -x509 -key t1.key -out t1.crt. Error 80B2C6CB5A7F0000:error:03000093:digital envelope routines:default_check:command not supported:crypto/evp/ctrl_params_translate.c:329: occurs. Disabling the engine allows creating the cert.
4. Commands that shouldn't ever need the engine such as openssl cms -encrypt t1.crt where t1.crt is an EC cert also fail with a similar error: 801234265C7F0000:error:03000093:digital envelope routines:evp_pkey_ctx_ctrl_int:command not supported:crypto/evp/pmeth_lib.c:1321:

Though the messages are different, this may be related to issue #455. RSA operations work fine, including ones that use a smartcard through the pkcs11 engine.

[ch-f]

This seems to be a duplicate of #455

[dengert]

Yes looks #455 is the same problem.
With OpenSSL 3.0.5 it appears that ./ssl/engine has been added as ./ssl/engine-3 and libp11 will install the libpkcs11.so as a symlink to pkcs11.so

So can both of you try modifying the instructions found in README.md to use the engine-3 directory rather then the engine directory.
i.e. should have dynamic_path = /usr/lib/ssl/engines-3/libpkcs11.so

@ch-f test of "openssl req -x509 -new" works for me.

[dengert]

Please disregard previous comment, I an still looking at it.

[dengert]

I can reproduce the problem.
Running gdb --args ./openssl req -x509 --new show set_keygen_ctx in req.c:1500 showskeygen_engine==NULL when called.
but later:

Breakpoint 2, set_keygen_ctx (gstr=0x0, pkeytype=0x7fffffffdac0, pkeylen=0x7fffffffdad8, keygen_engine=0x0) at ../openssl/apps/req.c:1608
1608	    if (param != NULL) {
(gdb) p keygen_engine
$38 = (ENGINE *) 0x0
(gdb) n
1623	        if (keygen_engine != NULL) {
(gdb) p keygen_engine
$39 = (ENGINE *) 0x0
(gdb) n
1630	            gctx = EVP_PKEY_CTX_new_from_name(app_get0_libctx(),
(gdb) n
1635	    if (gctx == NULL) {
(gdb) p *gctx->engine
$40 = {id = 0x7ffff79e123a "pkcs11", name = 0x7ffff79e1241 "pkcs11 engine", rsa_meth = 0x55555568e9e0, dsa_meth = 0x0, dh_meth = 0x0, 
  ec_meth = 0x55555568eb00, rand_meth = 0x0, ciphers = 0x0, digests = 0x0, pkey_meths = 0x7ffff79dd970 <PKCS11_pkey_meths>, pkey_asn1_meths = 0x0, 
  destroy = 0x7ffff79d2746 <engine_destroy>, init = 0x7ffff79d27ad <engine_init>, finish = 0x7ffff79d27e9 <engine_finish>, 
  ctrl = 0x7ffff79d2914 <engine_ctrl>, load_privkey = 0x7ffff79d2886 <load_privkey>, load_pubkey = 0x7ffff79d2832 <load_pubkey>, 
  load_ssl_client_cert = 0x0, cmd_defns = 0x7ffff79e6c20 <engine_cmd_defns>, flags = 0, struct_ref = 4, funct_ref = 3, ex_data = {ctx = 0x0, 
    sk = 0x555555687fe0}, prev = 0x555555687b90, next = 0x0, prev_dyn = 0x0, next_dyn = 0x0, dynamic_id = 0x7ffff79d2b8c <bind_engine>}

EVP_PKEY_CTX_new_from_name files in the pkcs11 engine in the gctx. It looks correct loaded.

https://www.openssl.org/docs/man3.0/man7/migration_guide.html warns of problems with engines vs providers. It looks like OpenSSL is assuming it can use the pkcs11 engine for all its operations, where in past versions it would be specific.

[ch-f]

Thanks, here this patch is fixing the issue.

[vuori]

Thanks, fixed with the patch for me.

[ulrichb]

@mtrojnar Why is this ticket closed? On master there's still the revert and therefore the issue is not fixed atm. (at least in my local repro).

BTW: The change in https://github.com/dengert/libp11/tree/deffer_init_crypto3 fixes the original problem in my local repro.

[ulrichb]

@mtrojnar Oh stop, I was confused by the second revert.

But I have still errors with the current master because of a following commit.

Namely feb22a6: Here I again get "SSL handshake" errors (as before @dengert's fixes).

Note that the previous commit 5ab3c0d (including @dengert's fixes) still works fine.

[mtrojnar]

And what exactly do you mean by "works fine"? Is it "works in my particular configuration", "passes the implemented CI tests on GitHub Actions", or something else? Feel free to submit a fix as a PR.

If you think that our testing suite is incomplete then please add an additional test as a PR.

[ulrichb]

@mtrojnar With "works fine / fails", I meant my local repro with a not yet published SignPath PKCS#11 module.

But I managed to reproduce the same also via the (SoftHSM) tests and created PR #465.

[ulrichb]

Should we re-open this issue as soon as the test in #465 is not green?

[mtrojnar]

The test in #465 tests a feature that is fundamentally broken (does not invoke the engine) in OpenSSL 1.1.1, and we don't want to support with OpenSSL 3.0 (there is no practical reason to generate a private key on a device and then download it to the library).
See 335cf75 for details.

[ulrichb]

@mtrojnar

Hmmm ... Yes, this PR didn't test a use-case via the PKCS#11 interface. But it still proved that the ECC initialization gets mangled by commit feb22a6.

After reverting the mentioned commit both this test and my internal repro (where I have interrupted TLS handshakes with ECDHE cipher suites) run again (with OpenSSL 3). Therefore I though this test should be enough to regression test this issue.

BTW: The PR tests also what this issue was originally about (running openssl ecparam -genkey).

Atm. I don't know which other test I should provide to have a contained sample. In my case the PKCS#11 runs an HTTPS request to a service with an ECDHE cipher suite. Writing such a test would be veeery involved.

[mtrojnar]

BTW: The PR tests also what this issue was originally about (running openssl ecparam -genkey).

I see. As openssl ecparam -genkey didn't work in OpenSSL 1.1.1 (silently executing all the computations in OpenSSL), and it's not supposed to work in OpenSSL 3.0, there was no regression here.

[ulrichb]

Okay, I now understand why using openssl ecparam -genkey with the -engine argument is not a valid test for OpenSSL 3.

Still I have issues with my ECDHE HTTPS connections (inside of a libp11 called PKCS#11 module) probably because of feb22a6. Do you have an idea how to regression test this without using openssl ecparam -genkey?

[mtrojnar]

IMHO HTTPS is irrelevant. All we need is TLS. So, maybe something like:

echo "test" | openssl s_server -engine pkcs11 &
sleep 1 # give s_server a second to bind the port
openssl s_client
wait

The goal is to make sure that OpenSSL does not use the engine for ECDHE in TLS, right?

[ulrichb]

D'accord that it's just about TLS not HTTPS.

It's about an TLS connection (with an ECDHE cipher suite) within a PKCS#11 call, specifically C_Sign and therefore after the engine has been initialized. So, I'm not sure if openssl s_server -engine is enough to trigger this. But I'll give it a shot.

Probably it's enough to trigger any ECC operation after the engine has been loaded, but unfortunately at the moment, I have no idea how to provide a repro sample without using openssl ecparam -engine ... :(

[mtrojnar]

So, I'm not sure if openssl s_server -engine is enough to trigger this.

I'm not giving you the solution, only an idea to investigate. Unless you hire me, don't expect me to do all the work for you.

[dengert]

I have a concern over the call to bind_helper_methods(engine); from engine_ctrl before a call to ctx_engine_ctrl. ctx_engine_ctrl is called for many reasons: https://github.com/OpenSC/libp11/blob/master/src/eng_back.c#L983-L1016

@ulrichb has been complaining about this commit too.

The call to bind_helper_methods(engine); at this location is overkill, it may have fixed one problem. What was the call that was failing?
This call should be moved to places that indicate that the engine is about to be called to do some crypto operation.

[dengert]

Let me point out that feb22a6 in effect reverses all of #457 Because it is called while the engine is being loaded. Why was feb22a6 added?

I and OpenSSL had talked about OpenSSL mixing up engines and providers. This is how that can happens.

In OpenSSL ./apps all the apps call setup_engine() but this is now a macro in ./include/apps.h:

ENGINE *setup_engine_methods(const char *id, unsigned int methods, int debug);
# define setup_engine(e, debug) setup_engine_methods(e, (unsigned int)-1, debug)

setup_engine_methods was introduced in OpenSSL #538404d2186: from openssl/openssl#4277 Note it was added then withdrawn then added. Many comments. Sounds like the same problem we have with libp11.

gitk shows:

Author: David von Oheimb <David.von.Oheimb@siemens.com>  2017-08-28 12:14:47
Committer: Dr. David von Oheimb <David.von.Oheimb@siemens.com>  2020-05-15 13:24:11
Parent: 8c10e1b660be1286439e15c9a955461f25b53616 (Clean up macro definitions of openssl_fdset() in apps.h and sockets.h)
Child:  a51f225d0d6a9ea5b25a07091a67bb3c737ffe31 (Add "md-nits" make target)
Branches: list, master, remotes/upstream/master, remotes/upstream/openssl-3.0, tag-openssl-3.0.0, tag-openssl-3.0.1, tag_openssl-3.0.5
Follows: openssl-3.0.0-alpha2
Precedes: openssl-3.0.0-alpha3

    Add 'methods' parameter to setup_engine() in apps.c for individual method defaults``

The "methods" is set to -1, to setup all methods supported by the engine.
using as test gdb --args /opt/ossl-3.0.5/bin/openssl ecparam -name brainpoolP256r1 -genkey -out t1.key

with breakpoint ./crypto/engine/tb_eckey.c ENGINE_set_default_EC(ENGINE *e) (on 3rd time with pkcs11 engine):
Note: rsa_meth, ec_meth and pkey_meths are all set to point into libp11

gdb --args /opt/ossl-3.0.5/bin/openssl ecparam -name brainpoolP256r1 -genkey -out t1.key

Breakpoint 1, ENGINE_register_EC (e=0x55555568e9f0)
    at ../openssl/crypto/engine/tb_eckey.c:29
29	{
(gdb) p *e
$5 = {id = 0x7ffff79e123a "pkcs11", name = 0x7ffff79e1241 "pkcs11 engine", 
  rsa_meth = 0x55555568f650, dsa_meth = 0x0, dh_meth = 0x0, 
  ec_meth = 0x55555568f7b0, rand_meth = 0x0, ciphers = 0x0, digests = 0x0, 
  pkey_meths = 0x7ffff79dd82a <PKCS11_pkey_meths>, pkey_asn1_meths = 0x0, 
  destroy = 0x7ffff79d2826 <engine_destroy>, 
  init = 0x7ffff79d288d <engine_init>, 
  finish = 0x7ffff79d28c9 <engine_finish>, 
  ctrl = 0x7ffff79d2a0c <engine_ctrl>, 
  load_privkey = 0x7ffff79d2972 <load_privkey>, 
  load_pubkey = 0x7ffff79d2912 <load_pubkey>, load_ssl_client_cert = 0x0, 
  cmd_defns = 0x7ffff79e6c20 <engine_cmd_defns>, flags = 0, struct_ref = 3, 
  funct_ref = 1, ex_data = {ctx = 0x0, sk = 0x55555568eb80}, 
  prev = 0x555555687b90, next = 0x0, prev_dyn = 0x0, next_dyn = 0x0, 
  dynamic_id = 0x7ffff79d2ca2 <bind_engine>}
(gdb) where
#0  ENGINE_register_EC (e=0x55555568e9f0)
    at ../openssl/crypto/engine/tb_eckey.c:29
#1  0x00007ffff7bdfe52 in ENGINE_register_complete (e=0x55555568e9f0)
    at ../openssl/crypto/engine/eng_fat.c:105
#2  0x00007ffff7bdfeb1 in ENGINE_register_all_complete ()
    at ../openssl/crypto/engine/eng_fat.c:119
#3  0x00007ffff7c4be2e in OPENSSL_init_crypto (opts=30284, settings=0x0)
    at ../openssl/crypto/init.c:645
#4  0x00007ffff7f2da8b in OPENSSL_init_ssl (opts=30284, settings=0x0)
    at ../openssl/ssl/ssl_init.c:115
#5  0x00005555555c9694 in apps_startup () at ../openssl/apps/openssl.c:66
#6  0x00005555555c97a9 in main (argc=7, argv=0x7fffffffde98)
    at ../openssl/apps/openssl.c:265

With a build without feb22a6:
Note: rsa_meth, ec_meth and pkey_meth are all 0x0

Breakpoint 1, ENGINE_register_EC (e=0x55555568e9f0)
    at ../openssl/crypto/engine/tb_eckey.c:29
29	{
(gdb) where
#0  ENGINE_register_EC (e=0x55555568e9f0)
    at ../openssl/crypto/engine/tb_eckey.c:29
#1  0x00007ffff7bdfe52 in ENGINE_register_complete (e=0x55555568e9f0)
    at ../openssl/crypto/engine/eng_fat.c:105
#2  0x00007ffff7bdfeb1 in ENGINE_register_all_complete ()
    at ../openssl/crypto/engine/eng_fat.c:119
#3  0x00007ffff7c4be2e in OPENSSL_init_crypto (opts=30284, settings=0x0)
    at ../openssl/crypto/init.c:645
#4  0x00007ffff7f2da8b in OPENSSL_init_ssl (opts=30284, settings=0x0)
    at ../openssl/ssl/ssl_init.c:115
#5  0x00005555555c9694 in apps_startup () at ../openssl/apps/openssl.c:66
#6  0x00005555555c97a9 in main (argc=7, argv=0x7fffffffde98)
    at ../openssl/apps/openssl.c:265
(gdb) p *e
$2 = {id = 0x7ffff79e123a "pkcs11", name = 0x7ffff79e1241 "pkcs11 engine", 
  rsa_meth = 0x0, dsa_meth = 0x0, dh_meth = 0x0, ec_meth = 0x0, 
  rand_meth = 0x0, ciphers = 0x0, digests = 0x0, pkey_meths = 0x0, 
  pkey_asn1_meths = 0x0, destroy = 0x7ffff79d2826 <engine_destroy>, 
  init = 0x7ffff79d288d <engine_init>, 
  finish = 0x7ffff79d28c9 <engine_finish>, 
  ctrl = 0x7ffff79d2a0c <engine_ctrl>, 
  load_privkey = 0x7ffff79d2972 <load_privkey>, 
  load_pubkey = 0x7ffff79d2912 <load_pubkey>, load_ssl_client_cert = 0x0, 
  cmd_defns = 0x7ffff79e6c20 <engine_cmd_defns>, flags = 0, struct_ref = 3, 
  funct_ref = 1, ex_data = {ctx = 0x0, sk = 0x55555568eb80}, 
  prev = 0x555555687b90, next = 0x0, prev_dyn = 0x0, next_dyn = 0x0, 
  dynamic_id = 0x7ffff79d2c96 <bind_engine>}

In other words, the pkca11 engine is ignored when looking for defaults for rsa, ec or pkey. But when a key is explicitly loaded by the engine, these methods are set to be used as needed. But feb22a6 caused the methods to be added too early and should be reverted.

[ulrichb]

FYI: The issue I mentioned above, #456 (comment), where TLS calls w/ EC ciphers failed in combination w/ libp11 is fixed until OpenSSL >= 3.0.9. Probably because of the fix of openssl/openssl#20161.