Prevent crash by not loading engine if there is multiple libcrypto.so

[vt-alt]

If libcrypto.so.1 loads engine compiled for libcrypto.so.3 crash is imminent. Avoid the crash by not loading engine at all if there is two libcrypto.so. There cannot be two libcrypto.so.1 or two libcrypto.so.3, so this workaround seems good.

Link: https://github.com/openssl/openssl/issues/17102 Signed-off-by: Vitaly Chikunov [email protected]

[yanovich]

#384 should solve this one as well. If the engine is built against a different version than the executable, there will be unresolved symbols after load.

[vt-alt]

Idea is interesting. Is this solution tested? Can you show load errors?

[yanovich]

node-17.2.0 (openssl-3.0), node-16.13.1 (openssl-1.1.1l) and system openssl-1.1.1l from ubuntu-21.10 can all use gost-engine-3 with my patch normally. node-17.2.0 (openssl-3.0) fails to load gost-engine-1.1 with my patch for v1.1.1:

$ node -v 
OpenSSL configuration error:
C0935BB83D7F0000:error:12800067:DSO support routines:dlfcn_load:could not load the shared library:../deps/openssl/openssl/crypto/dso/dso_dlfcn.c:118:filename(/usr/lib/x86_64-linux-gnu/engines-1.1/gost.so): /usr/lib/x86_64-linux-gnu/engines-1.1/gost.so: undefined symbol: EVP_PKEY_base_id
C0935BB83D7F0000:error:12800067:DSO support routines:DSO_load:could not load the shared library:../deps/openssl/openssl/crypto/dso/dso_lib.c:162:
C0935BB83D7F0000:error:13000084:engine routines:dynamic_load:dso not found:../deps/openssl/openssl/crypto/engine/eng_dyn.c:422:
C0935BB83D7F0000:error:13000066:engine routines:int_engine_configure:engine configuration error:../deps/openssl/openssl/crypto/engine/eng_cnf.c:139:section=gost_section, name=dynamic_path, value=/usr/lib/x86_64-linux-gnu/engines-1.1/gost.so
C0935BB83D7F0000:error:0700006D:configuration file routines:module_run:module initialization error:../deps/openssl/openssl/crypto/conf/conf_mod.c:243:module=engines, value=engine_section retcode=-1

[vt-alt]

Thanks! While this is a cool idea in itself, this method is not reliable. It's by accident changes between 1.1.1 and 3.0 have difference in linking symbols causing dynamic linking error. But, there could more subtle ABI change (for example if some struct format changes) which will not cause linking error, but will still produce (even harder to debug) crashes/bugs.

AFAIK, the only reliable ABI change marker is SOVERSION change.

[vt-alt]

While this is a cool idea in itself, this method is not reliable. It's by accident changes between 1.1.1 and 3.0 have difference in linking symbols causing dynamic linking error.

Ah, I am missed that function names in libcrypto are versioned too. Then it should be quire reliable indeed!

$ nm -D /lib64/libcrypto.so.1.1|grep -i engine|tail
0000000000158770 T ENGINE_unregister_RAND@@OPENSSL_1_1_0
0000000000158880 T ENGINE_unregister_RSA@@OPENSSL_1_1_0
00000000001564c0 T ENGINE_up_ref@@OPENSSL_1_1_0
0000000000154ed0 T ERR_load_ENGINE_strings@@OPENSSL_1_1_0
0000000000173a30 T EVP_PKEY_get0_engine@@OPENSSL_1_1_1c
0000000000173980 T EVP_PKEY_set1_engine@@OPENSSL_1_1_0g
00000000001e5dd0 T OSSL_STORE_LOADER_get0_engine@@OPENSSL_1_1_1
00000000001a7160 T RAND_set_rand_engine@@OPENSSL_1_1_0
00000000001b0140 T RSA_get0_engine@@OPENSSL_1_1_0
00000000001e7090 T TS_CONF_set_default_engine@@OPENSSL_1_1_0

[yanovich]

Unfortunately, there is no way to solve broken ABI problem without upstream (libcrypto) cooperation. If gost.so is linked as a plugin, there is no version in unresolved symbols for an obvious reason:

build$ nm -D bin/gost.so.1.1 | grep ' U ' | tail 
                 U strcmp@GLIBC_2.2.5
                 U strlen@GLIBC_2.2.5
                 U strtol@GLIBC_2.2.5
                 U X509_ALGOR_get0
                 U X509_ALGOR_set0
                 U X509_PUBKEY_get
                 U X509_PUBKEY_get0_param
                 U X509_PUBKEY_it
                 U X509_PUBKEY_set
                 U X509_PUBKEY_set0_param

If openssl can prohibit interface changes in libcrypto ABI (that is only additions and removals are allowed, and if you need a(b, c) to become a(b, c, d), you remove a(b, c) and add a2(b, c, d)), this will work reliably.

[beldmit]

If I remember correctly, versioning happens on a distro level.

[yanovich]

It is certainly a project policy, Base guidelines are by libtool versioning. But in this case a stricter variant is needed, like something along graphql guidelines for interface (add, delete, but never change).