[Touch-packages] [Bug 1951943] Re: Engine crashes when loading the configuration more than once
Hi, might be missing something here, but if OpenSSL integrated their fix across all versions, what is holding Jammy from getting a new package with this fix? Is it just an issue of time for testing it? Or is it something that is missing? -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to openssl in Ubuntu. https://bugs.launchpad.net/bugs/1951943 Title: Engine crashes when loading the configuration more than once Status in openssl package in Ubuntu: Fix Committed Status in openssl source package in Bionic: Fix Released Status in openssl source package in Focal: Fix Released Status in openssl source package in Hirsute: Fix Released Status in openssl source package in Impish: Fix Released Status in openssl source package in Jammy: Fix Committed Bug description: [Impact] * Engine crashes when loading the configuration more than once * Upstream started to avoid loading engines twice by using dynamic ids to track the loaded engines correctly * OpenSSL 3 https://github.com/openssl/openssl/commit/81c11349c2a0e945aa3dfc6bd81c957363dd2011 (bugfix) https://github.com/openssl/openssl/commit/38e2957249c90317a26a080c7e7eb186dd5b6598 (test case) * OpenSSL 1.1.1 backports: https://github.com/openssl/openssl/commit/9b06ebb1edfddffea083ba36090af7eb7cad207b (bugfix) https://github.com/openssl/openssl/pull/17083 (test case) [Test Plan] * https://github.com/openssl/openssl/issues/17023 lists multiple ways how one can trigger the issue at hand, but also test case implements this issue too by explicitly attempting to load an engine multiple times and checking that it is operational. The test a is run during the build as part of the upstream regression test suite, for the shared library build (as static build does not support engines), so you'll see one pass and one skip in the log. [Where problems could occur] * Separately we have started to fix userspace packages that needlessly load configuration files multiple times, which used to trigger this issue. The codepaths changed are with engine use, how they are loaded/unloaded/used. It is possible that this fix will make some engines to start working and be used resulting in new behaviour. But also exposing bugs in the engines that previously were installed & configured but not actually used. [Other Info] * Previous bug reports about this issues are: https://bugs.launchpad.net/ubuntu/+source/wget/+bug/1921518 https://bugs.launchpad.net/ubuntu/+source/curl/+bug/1940528 To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1951943/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1951943] Re: Engine crashes when loading the configuration more than once
Tested libssl1.1_1.1.1f-1ubuntu2.10_arm64.deb on an arm64 setup with older wget installed (1.20.3-1ubuntu1), and PKA engine (1.3) configured with debug prints. OpenSSL indeed loaded the PKA engine only once, causing wget to work as expected even without wget's patch against loading the engines only once - https://bugs.launchpad.net/ubuntu/+source/wget/+bug/1921518. I approve that this libssl package successfully addresses the engine issue that it aims to resolve. -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to openssl in Ubuntu. https://bugs.launchpad.net/bugs/1951943 Title: Engine crashes when loading the configuration more than once Status in openssl package in Ubuntu: Confirmed Status in openssl source package in Bionic: Fix Committed Status in openssl source package in Focal: Fix Committed Status in openssl source package in Hirsute: Fix Committed Status in openssl source package in Impish: Fix Committed Status in openssl source package in Jammy: Confirmed Bug description: [Impact] * Engine crashes when loading the configuration more than once * Upstream started to avoid loading engines twice by using dynamic ids to track the loaded engines correctly * OpenSSL 3 https://github.com/openssl/openssl/commit/81c11349c2a0e945aa3dfc6bd81c957363dd2011 (bugfix) https://github.com/openssl/openssl/commit/38e2957249c90317a26a080c7e7eb186dd5b6598 (test case) * OpenSSL 1.1.1 backports: https://github.com/openssl/openssl/commit/9b06ebb1edfddffea083ba36090af7eb7cad207b (bugfix) https://github.com/openssl/openssl/pull/17083 (test case) [Test Plan] * https://github.com/openssl/openssl/issues/17023 lists multiple ways how one can trigger the issue at hand, but also test case implements this issue too by explicitly attempting to load an engine multiple times and checking that it is operational. The test a is run during the build as part of the upstream regression test suite, for the shared library build (as static build does not support engines), so you'll see one pass and one skip in the log. [Where problems could occur] * Separately we have started to fix userspace packages that needlessly load configuration files multiple times, which used to trigger this issue. The codepaths changed are with engine use, how they are loaded/unloaded/used. It is possible that this fix will make some engines to start working and be used resulting in new behaviour. But also exposing bugs in the engines that previously were installed & configured but not actually used. [Other Info] * Previous bug reports about this issues are: https://bugs.launchpad.net/ubuntu/+source/wget/+bug/1921518 https://bugs.launchpad.net/ubuntu/+source/curl/+bug/1940528 To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1951943/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1921518] Re: OpenSSL "double free" error
The wget package that was tested and approved on our setup (using PKA 1.3 engine) is the one you declared above - 1.20.3-1ubuntu2. The tests were basic functionality tests for wget, including debugging to verify that the engine is loaded exactly once. Same for curl (exactly the same procedure). -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to wget in Ubuntu. https://bugs.launchpad.net/bugs/1921518 Title: OpenSSL "double free" error Status in wget package in Ubuntu: Fix Released Status in wget source package in Focal: Fix Committed Bug description: [Impact] openssl config file is being loaded twice, causing engines to be loaded twice if specified therein, causing double free errors and other strange behavior. [Test plan] Run the command of the package being tested in gdb -ex "break CONF_modules_load_file" -ex "run" --args and make sure it only breaks one. Regression test: In default Ubuntu configuration, either no openssl configuration is provided, or it contains no settings that affect wget. This code path changes how/when openssl configuration is loaded and used by openssl. One should verify that: 1) wget continues to work without openssl.cnf 2) wget continues to work with stock ubuntu unmodified openssl.cnf 3) wget continue to honor and use custom TLS settings that one may have specified in openssl.cnf (for example custom engine) [Where problems could occur] wget: This is an upstream change that changes initialization and is in use in later releases. Since it mostly removes an unneeded call to the load file function, a regression could be a config file being ignored, but it seems unlikely given the use in later releases [Original bug report] "double free" error is seen when using curl utility. Error is from libcrypto.so which is part of the OpenSSL package. This happens only when OpenSSL is configured to use a dynamic engine. OpenSSL version is 1.1.1f The issue is not encountered if http://www.openssl.org/source/openssl-1.1.1f.tar.gz is used instead. OpenSSL can be configured to use a dynamic engine by editing the default openssl config file which is located at '/etc/ssl/openssl.cnf' on Ubuntu systems. On Bluefield systems, config diff to enable PKA dynamic engine, is as below: +openssl_conf = conf_section + # Extra OBJECT IDENTIFIER info: #oid_file = $ENV::HOME/.oid oid_section= new_oids +[ conf_section ] +engines = engine_section + +[ engine_section ] +bf = bf_section + +[ bf_section ] +engine_id=pka +dynamic_path=/usr/lib/aarch64-linux-gnu/engines-1.1/pka.so +init=0 + engine_id above refers to dynamic engine name/identifier. dynamic_path points to the .so file for the dynamic engine. # curl -O https://tpo.pe/pathogen.vim double free or corruption (out) Aborted (core dumped) To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/wget/+bug/1921518/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1921518] Re: OpenSSL "double free" error
following my request, OpenSSL just integrated a fix to avoid loading an engine twice even if the configuration is parsed more than once: https://github.com/openssl/openssl/commit/9b06ebb1edfddffea083ba36090af7eb7cad207b Integrating this patch in the existing OpenSSL 1.1.1 package (or at least packaging the relevant OpenSSL 1.1.1 version that will include it) will ensure that no additional project will crash if it uses an engine (such as PKA) and the configuration is parsed twice. In the long term, this aims to be a robust solution to this double-load issue, so that instead of playing whack-a-mole on all 3rd party projects that might load the config twice, the issue will be resolved at OpenSSL itself. -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to wget in Ubuntu. https://bugs.launchpad.net/bugs/1921518 Title: OpenSSL "double free" error Status in wget package in Ubuntu: Fix Released Status in wget source package in Focal: Fix Committed Bug description: [Impact] openssl config file is being loaded twice, causing engines to be loaded twice if specified therein, causing double free errors and other strange behavior. [Test plan] Run the command of the package being tested in gdb -ex "break CONF_modules_load_file" -ex "run" --args and make sure it only breaks one. Regression test: In default Ubuntu configuration, either no openssl configuration is provided, or it contains no settings that affect wget. This code path changes how/when openssl configuration is loaded and used by openssl. One should verify that: 1) wget continues to work without openssl.cnf 2) wget continues to work with stock ubuntu unmodified openssl.cnf 3) wget continue to honor and use custom TLS settings that one may have specified in openssl.cnf (for example custom engine) [Where problems could occur] wget: This is an upstream change that changes initialization and is in use in later releases. Since it mostly removes an unneeded call to the load file function, a regression could be a config file being ignored, but it seems unlikely given the use in later releases [Original bug report] "double free" error is seen when using curl utility. Error is from libcrypto.so which is part of the OpenSSL package. This happens only when OpenSSL is configured to use a dynamic engine. OpenSSL version is 1.1.1f The issue is not encountered if http://www.openssl.org/source/openssl-1.1.1f.tar.gz is used instead. OpenSSL can be configured to use a dynamic engine by editing the default openssl config file which is located at '/etc/ssl/openssl.cnf' on Ubuntu systems. On Bluefield systems, config diff to enable PKA dynamic engine, is as below: +openssl_conf = conf_section + # Extra OBJECT IDENTIFIER info: #oid_file = $ENV::HOME/.oid oid_section= new_oids +[ conf_section ] +engines = engine_section + +[ engine_section ] +bf = bf_section + +[ bf_section ] +engine_id=pka +dynamic_path=/usr/lib/aarch64-linux-gnu/engines-1.1/pka.so +init=0 + engine_id above refers to dynamic engine name/identifier. dynamic_path points to the .so file for the dynamic engine. # curl -O https://tpo.pe/pathogen.vim double free or corruption (out) Aborted (core dumped) To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/wget/+bug/1921518/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1921518] Re: OpenSSL "double free" error
Loading the configuration only once will resolve this issue, and is the recommended code fix. On top of this bug fix, and as mentioned above, we recommend that future versions will incorporate an API change that will shift the ownership on releasing the pointers to the engine that allocated them on the first place. We understand this is an API change and that it isn't relevant in the near future. Regarding workarounds that will apply till OpenSSL fix their double loading of the configuration, as I mentioned in #35, we are at a loss here because OpenSSL is actively freeing the memory that the engine allocated. The struct ENGINE is an OpenSSL implementation detail, and is allocated in an OpenSSL function ENGINE_new(). I fail to see how a given engine can add fields to it / save its own context on top of it. 1. The engine library could maintain some Data structure with all "known" engine instances of itself, but OpenSSL guaranteed developers a singleton invocation. This is a drastic implementation change on behalf of the engine developers, and I doubt developers will go to this effort when they know it is temporary until OpenSSL fixes their code and restore back the singleton guarantee. 2. It is a bit trickier, because OpenSSL expect the memory to be initialized using EVP_PKEY_meth_new() and then the pointer to be set using EVP_PKEY_meth_set_derive(). Hence, they've taken the liberty to invoke EVP_PKEY_meth_free() and directly free the instance that was given to them. If all engine instances use the same static field, it will get freed by OpenSSL. If there will be an instance per engine, it throws us back to point (1) of maintaining a data structure of the additional engine metadata per instance. 3. In our deployment environments any environment variable based hack is out of the question. As such, any temporary fix will be a hack, and will need to bypass the break of the singleton engine guarantee that was given to engine developers. Disabling the double parsing of the configuration, and double load of the engine is the code fix that should be done, and it should be done on OpenSSL's side. -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to openssl in Ubuntu. https://bugs.launchpad.net/bugs/1921518 Title: OpenSSL "double free" error Status in openssl package in Ubuntu: Incomplete Status in openssl source package in Focal: Incomplete Bug description: "double free" error is seen when using curl utility. Error is from libcrypto.so which is part of the OpenSSL package. This happens only when OpenSSL is configured to use a dynamic engine. OpenSSL version is 1.1.1f The issue is not encountered if http://www.openssl.org/source/openssl-1.1.1f.tar.gz is used instead. OpenSSL can be configured to use a dynamic engine by editing the default openssl config file which is located at '/etc/ssl/openssl.cnf' on Ubuntu systems. On Bluefield systems, config diff to enable PKA dynamic engine, is as below: +openssl_conf = conf_section + # Extra OBJECT IDENTIFIER info: #oid_file = $ENV::HOME/.oid oid_section= new_oids +[ conf_section ] +engines = engine_section + +[ engine_section ] +bf = bf_section + +[ bf_section ] +engine_id=pka +dynamic_path=/usr/lib/aarch64-linux-gnu/engines-1.1/pka.so +init=0 + engine_id above refers to dynamic engine name/identifier. dynamic_path points to the .so file for the dynamic engine. # curl -O https://tpo.pe/pathogen.vim double free or corruption (out) Aborted (core dumped) To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1921518/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1921518] Re: OpenSSL "double free" error
While trying to understand why a fix in PKA that guards against multiple destroys (https://github.com/Mellanox/pka/pull/37/files) didn't bypass this issue, I found the following. bind() operation of engines is expected to populate the pmeths and ameths of an existing engine (https://github.com/gost- engine/engine/blob/df3ead272bd2019f98d16e6787f5df51556c0603/gost_eng.c#L375, https://github.com/Mellanox/pka/blob/master/engine/e_bluefield.c#L1615). This means that the engine uses EVP_PKEY_meth_new (for instance) as part of this registration. However, on teardown, OpenSSL's engine_free_util() is invoking engine_pkey_meths_free() and engine_pkey_asn1_meths_free(). Both of which iterate the list of registered methods, and invoke EVP_PKEY_meth_free() on each on of them. Only after OpenSSL freed these methods it calls the engine's destroy() method which allows the registered engine to do its own cleanup. As long as this design is used, an engine using pkey methods can't protect itself against multiple destroy operations, because OpenSSL is the one freeing it's methods and there isn't much the engine can do about it. For future versions, it might be recommended to update this API and grant the engine the ownership on clearing up the memory that it allocated on the first place. -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to openssl in Ubuntu. https://bugs.launchpad.net/bugs/1921518 Title: OpenSSL "double free" error Status in openssl package in Ubuntu: Incomplete Status in openssl source package in Focal: Incomplete Bug description: "double free" error is seen when using curl utility. Error is from libcrypto.so which is part of the OpenSSL package. This happens only when OpenSSL is configured to use a dynamic engine. OpenSSL version is 1.1.1f The issue is not encountered if http://www.openssl.org/source/openssl-1.1.1f.tar.gz is used instead. OpenSSL can be configured to use a dynamic engine by editing the default openssl config file which is located at '/etc/ssl/openssl.cnf' on Ubuntu systems. On Bluefield systems, config diff to enable PKA dynamic engine, is as below: +openssl_conf = conf_section + # Extra OBJECT IDENTIFIER info: #oid_file = $ENV::HOME/.oid oid_section= new_oids +[ conf_section ] +engines = engine_section + +[ engine_section ] +bf = bf_section + +[ bf_section ] +engine_id=pka +dynamic_path=/usr/lib/aarch64-linux-gnu/engines-1.1/pka.so +init=0 + engine_id above refers to dynamic engine name/identifier. dynamic_path points to the .so file for the dynamic engine. # curl -O https://tpo.pe/pathogen.vim double free or corruption (out) Aborted (core dumped) To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1921518/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
[Touch-packages] [Bug 1921518] Re: OpenSSL "double free" error
Hi,
Sorry for interrupting your thread, this bug has been prioritized on our
end (I work at NVIDIA) so I joined the triaging effort and I believe I
found the root cause for the crash. For the record, I used wget, but it
behaves the same to curl.
First of all, it seems that it isn't that far off from an old case -
https://openssl-dev.openssl.narkive.com/97zf5qT2/what-is-the-really-
proper-way-to-use-an-engine. The reason being that the configuration
file is loaded twice:
#0 0xf7c80304 in CONF_modules_load_file () from
/lib/aarch64-linux-gnu/libcrypto.so.1.1
#1 0xf7c805e8 in ?? () from /lib/aarch64-linux-gnu/libcrypto.so.1.1
#2 0xf7cfa03c in ?? () from /lib/aarch64-linux-gnu/libcrypto.so.1.1
#3 0xf79dadd4 in __pthread_once_slow (once_control=0xf7e44550,
init_routine=0xf7cfa020) at pthread_once.c:116
#4 0xf7d434c4 in CRYPTO_THREAD_run_once () from
/lib/aarch64-linux-gnu/libcrypto.so.1.1
#5 0xf7cfa608 in OPENSSL_init_crypto () from
/lib/aarch64-linux-gnu/libcrypto.so.1.1
#6 0xf7c80578 in OPENSSL_config () from
/lib/aarch64-linux-gnu/libcrypto.so.1.1
#7 0xaaae9e24 in ?? ()
#8 0xaaacfbe0 in ?? ()
#9 0xaaad29cc in ?? ()
#10 0xaaadde5c in ?? ()
#11 0xaaab8874 in ?? ()
And later again (practically in the same function):
#0 0xf7c80304 in CONF_modules_load_file () from
/lib/aarch64-linux-gnu/libcrypto.so.1.1
#1 0xaaae9e68 in ?? ()
#2 0xaaacfbe0 in ?? ()
#3 0xaaad29cc in ?? ()
#4 0xaaadde5c in ?? ()
#5 0xaaab8874 in ?? ()
This means that the engine parsing and creation flow is done twice,
which breaks several assumptions of both OpenSSL and the loaded engines:
1. Parsing the name "pka"
2. Creating a dynamic engine
3. Cloning the dynamic engine inside ENGINE_by_id()
3. Using the dynamic path to call the "bind" method of the loaded .so file
(pka.so)
4. Populating the fields of the cloned dynamic engine on top of what the user
still holds as ("e"):
else if (strcmp(ctrlname, "dynamic_path") == 0) {
e = ENGINE_by_id("dynamic");
5. dynamic_load() will try to once again ENGINE_add() the engine, but it is
already in the engine list
6. The Add will fail, and the reference count won't be incremented, hence will
stay at 1
7. The error will propagate and ENGINE_free() will get called because the load
failed:
if (!ENGINE_ctrl_cmd_string(e, "LOAD", NULL, 0))
goto err;
8. The free()ed engine is actually the PKA engine which got created twice, and
the destruction of the engine (which assumes it only get created/destroyed
once) will free memory pointed by static variables inside of the PKA module.
9. The first PKA engine is still "alive" and when used it will access free()ed
memory pointed by the static variables of PKA
10. We get a BUS error when invoking a function using a garbled fptr which got
overridden because it is stored in a free()ed memory buffer. On ARM, BUS error
is very common because the CPU can't execute commands from misaligned
addresses, and with high probability the garbled value isn't divisible by 4.
It seems that several fixes could resolve this issue:
1. The configuration file shouldn't be loaded twice, this breaks several
assumptions of the engines (as shown in the bug I linked above, which is from
11 years ago)
2. The fact that the dynamic engine starts as "dynamic" and the load replaces
it with the loaded engine is extremely counter intuitive. The caller holds a
single pointer, and it gets transformed into the loaded engine. Seeing there is
already a following call of "e = ENGINE_by_id(name)", this should be the code
line that transforms the user's engine to the one that was loaded, the dynamic
engine shouldn't change it in-memory when the user thinks their pointer points
at a dynamic engine.
3. If engines are to be expected to get added/freed multiple times, it should
be announced clearly, so that engine developers would know they are expected to
maintain their own reference count if using static variables. In general, it
will probably be best if engines would only be loaded once (and point #1 will
be fixed).
I hope my analysis will help resolve this issue. Feel free to reach out
if more information is needed.
--
You received this bug notification because you are a member of Ubuntu
Touch seeded packages, which is subscribed to openssl in Ubuntu.
https://bugs.launchpad.net/bugs/1921518
Title:
OpenSSL "double free" error
Status in openssl package in Ubuntu:
Incomplete
Status in openssl source package in Focal:
Incomplete
Bug description:
"double free" error is seen when using curl utility. Error is from
libcrypto.so which is part of the OpenSSL package. This happens only
when OpenSSL is configured to use a dynamic engine.
OpenSSL version is 1.1.1f
The issue is not encountered if
http://www.openssl.org/source/openssl-1.1.1f.tar.gz is used
