Prevent loading engine twice

[max619]

Currently second call to engine_load fails which leads to xcerts not being filled.
Faced that during debugging of #419.

[mtrojnar]

@max619 Is there a specific reason you're loading the engine twice?

Please include the exact command you're executing, along with its full output and any errors, in your PR description. Avoid making us reverse-engineer the symptoms based on your attempted fix.

Kindly consider reading How to Report Bugs Effectively.

[max619]

Ok,

I'm running the command:

osslsigncode sign -pass '***' -pkcs11module '...libykcs11.so.2.7.1' -pkcs11cert 'pkcs11:...;type=cert' -key 'pkcs11:...;type=private' -ac 'ssl.com_rsa_r3.pem' -t 'http://timestamp.digicert.com' -h sha256 -in 'in.exe' -out 'out.exe'

The issues is that certificate passed in -ac 'ssl.com_rsa_r3.pem' param is not being added to an executable neither being loaded. The cause of it is due to failing of a second call to load_engine method.

The first time call succeeds here and "loads" key and cert from smart card:

-    } else if (options->p11engine && !engine_load(options)) {

Then second call happens here:

+        /* try to find and load libp11 'pkcs11' or 'dynamic' engine */
          if (!engine_load(options)) {
             goto out;

Which fails, and then goto out; is executed which bypasses loading of options->certs and options->xcerts

    /* Load additional (cross) certificates ('-ac' option) */
    load_objects_from_store(options->xcertfile, options->pass, NULL, options->xcerts, NULL);

    /* Load the certificate chain ('-certs' option) */
    load_objects_from_store(options->certfile, options->pass, NULL, options->certs, NULL);
    ...
out:
    return (options->pkey && sk_X509_num(options->certs) > 0) ? 1 : 0;

However, as far as options->pkey and options->certs were loaded in the load_engine function from the using pkcs11 provider, the function exits with code 1 and signing continue to work, but not all certificates are silently being added to an executable.

[olszomal]

Thanks for the report.

The code path:

-    } else if (options->p11engine && !engine_load(options)) {

will only be entered if the-engine option is explicitly used. Based on your command line:

osslsigncode sign -pass '***' -pkcs11module '...libykcs11.so.2.7.1' -pkcs11cert 'pkcs11:...;type=cert' -key 'pkcs11:...;type=private' -ac 'ssl.com_rsa_r3.pem' -t 'http://timestamp.digicert.com' -h sha256 -in 'in.exe' -out 'out.exe'

you're not using the -engine option, so that condition should not apply. However, if the PKCS#11 engine is being automatically initialized via your OpenSSL configuration file (openssl.cnf), it could explain the observed behavior.

Could you please check your openssl.cnf for an implicit engine load? Specifically, look for something like this:

[openssl_init]
engines = engine_section

[engine_section]
pkcs11 = pkcs11_section

[pkcs11_section]
engine_id = pkcs11
MODULE_PATH = /usr/lib64/opensc-pkcs11.so
init = 1

If such a section is present, OpenSSL will load and initialize the engine automatically at startup with the -engine option leading to potentially unexpected behavior.

In that case, signing the file will fail with the following error:

OPENSSL_CONF=./engine.cnf \
osslsigncode sign \
-pkcs11module /usr/lib64/opensc-pkcs11.so \
-engine /usr/lib64/pkcs11.so \
-pkcs11cert 'pkcs11:object=certificate;type=cert' \
-key 'pkcs11:object=key;type=private' -pass 1234 \
-in unsigned.exe -out signed.exe \
-ac CAcross.pem

Failed to set 'dynamic' engine
Failed to read key or certificates
8080705CFB7F0000:error:1300006D:engine routines:dynamic_load:init failed:crypto/engine/eng_dyn.c:510:
Failed

This happens because the engine is already initialized externally, and osslsigncode tries to initialize it again internally, leading to a conflict.

If signing completes successfully, it means the additional (cross) certificates specified using -ac have been included in the signature, so make sure to check the signature carefully.

Let us know what you find.

[max619]

@olszomal That's actually my bad, i was using the -engine option.

That was the command:

./osslsigncode sign -pkcs11engine ../../libp11-0.4.13/src/.libs/pkcs11.so -pkcs11cert "pkcs11:..;type=cert" -key "pkcs11:...;type=private"  -pkcs11module ../../yubico-piv-tool-yubico-piv-tool-2.7.1/build/ykcs11/libykcs11.so -h sha256 -in ~/in.exe -out ./out.exe

And that's the output:

Engine "pkcs11" set.
Workaround for OpenSSL 3.0.13 30 Jan 2024 enabled
Enter PKCS#11 token PIN for YubiKey PIV #***:
Unable to load provider: pkcs11prov
Failed to set 'dynamic' engine
Succeeded

openssl.conf file is default one. There is no pkcs11 or engine related entries.

[olszomal]

Please check if my changes are sufficient. It would be great if you could also test how it works with the master branch of libp11.

[max619]

Changes are sufficient, thx. Tested with libp11 at master branch - works fine.