CMake: link gost.so statically to its caller

[yanovich]

If any executable loads gost.so, the executable already either has libcrypto.so loaded or is statically linked against libcrypto.a. Anyway it already has all libcrypto (and libssl) symbols present.

Without this patch gost.so is linked against libcrypto,so. As a result, a diamond dependency is introduced. If gost.so is then loaded by an executable which is statically linked against libcrypto, ld will insist on loading libcripto.so, despite the executable already having all necessary symbols. When the executable is statically linked, shared objects for libcrypto and libssl are usually not built, ls won't find them, and the caller will crush.

The patch removes this unnecessary link dependency in gost.so, allowing it to be used by executables which are statically linked against libcrypto.

[yanovich]

Here is the diff between original state and the fixed one:

$ cat .gitignore 
*.ts
*.o
*.o.d
*.a
*.bin
a.out
compiler_depend.*
bin/
Testing/
$ git diff
diff --git a/CMakeFiles/gost_engine.dir/build.make b/CMakeFiles/gost_engine.dir/build.make
index 83578e0..32ec190 100644
--- a/CMakeFiles/gost_engine.dir/build.make
+++ b/CMakeFiles/gost_engine.dir/build.make
@@ -94,7 +94,6 @@ bin/gost.so: CMakeFiles/gost_engine.dir/gost_eng.c.o
 bin/gost.so: CMakeFiles/gost_engine.dir/build.make
 bin/gost.so: libgost_core.a
 bin/gost.so: libgost_err.a
-bin/gost.so: /usr/lib/x86_64-linux-gnu/libcrypto.so
 bin/gost.so: CMakeFiles/gost_engine.dir/link.txt
        @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --green --bold --progress-dir=/home/s/src/crypto/openssl/gost-engine/build/CMakeFiles --progress-num=$(CMAKE_PROGRESS_2) "Linking C shared module bin/gost.so"
        $(CMAKE_COMMAND) -E cmake_link_script CMakeFiles/gost_engine.dir/link.txt --verbose=$(VERBOSE)
diff --git a/CMakeFiles/gost_engine.dir/link.txt b/CMakeFiles/gost_engine.dir/link.txt
index 50c9b2b..b3b08f5 100644
--- a/CMakeFiles/gost_engine.dir/link.txt
+++ b/CMakeFiles/gost_engine.dir/link.txt
@@ -1 +1 @@
-/usr/bin/cc -fPIC -O2 -shared  -o bin/gost.so CMakeFiles/gost_engine.dir/gost_eng.c.o  libgost_core.a libgost_err.a /usr/lib/x86_64-linux-gnu/libcrypto.so 
+/usr/bin/cc -fPIC -O2 -shared  -o bin/gost.so CMakeFiles/gost_engine.dir/gost_eng.c.o  libgost_core.a libgost_err.a

[beldmit]

I'm not sure that I want to link it this way by default.

[yanovich]

There are possible use cases on linux. E.g., the official docker image of node.js is built by statically linking against libcrypto.a. gost.so just works there with this patch and normal openssl.conf. This patch will also solve segfaults whengost.so is loaded by a different version of libcrypto than it was built against (#363).

[yanovich]

Please restart cancelled checks. They failed in before_script.sh by curl timeout.

[beldmit]

I'd consider this option as a separate build variant.

[yanovich]

Maybe I am wrong to call this type of linking static. It is inverse dynamic. GNU ld loads the engine then verifies that all symbol are resolved. Without my patch ld loads libcrypto.so which was present at link time before verification unless the exact version of it is already loaded. You don't want that. This causes subtle bugs like segfaults on version mismatch.

gost.so is not a valid shared library, it is a plugin for libcrypto (unlike 'libgost.so which is). When you invoke a function with side effects from gost.so, you expect that the application will change state. You link plugin as a shared library now, when gost.so invokes a function with side effects, it manipulates a separate global state, unless it is linked with the same shared library as the executable.

[vt-alt]

I'd consider this option as a separate build variant.

Agree. I discussed this slightly with @ldv-alt and he suggests that this should be build time option — as ALT certainly will not use this feature.

[yanovich]

Agree. I discussed this slightly with @ldv-alt and he suggests that this should be build time option — as ALT certainly will not use this feature.

I don't understand how linking against libcrypto makes the system any safer. If the caller uses a different version of libcrypto than the engine, it will segfault. And segfaults are bad for security.

The plugins are quite common. Kernel modules are plugins in the same sense as gost.so. The difference is that linux is specifically states that its ABI is a private moving target, so we rebuild all kernel modules when we update a single line of kernel code. But this is not the case with libcrypto. Its ABI is public and versioned. If openssl breaks libcrypto ABI, tons of packages will break.

Finally, all of this is the least concern possible for a distribution like ALT, which completely controls all the files it ships.

Dynamic linking is a mess for sure. That is why everyone tries to link statically: go (google has this policy in general), node.js, snap, flatpack. But there is no way to statically link gost.so. This makes the issue complicated. openssl itself links in-tree engines against libcrypto, which makes then difficult to use in statically linked executables.

My patch offers part of a solution, if libcrypto ABI can be frozen so that no interface change are allowed (only addition or removal is possible), then the solution for linux will be complete. I am not sure whether it is possible to solve this issue for other platforms at all.

[vt-alt]

I don't understand how linking against libcrypto makes the system any safer. If the caller uses a different version of libcrypto than the engine, it will segfault. And segfaults are bad for security.

@ldv-alt thinks this is better, because this will create strict ABI dependence for packages. Dynamic linker when loading gost.so will load libcrypto and check presence of all needed symbols. (This is different use case than we have for engine crash). Also, ALT have set-versions feature which automatically set dependence between packages with libraries and their users, which will not work if we disable -lcrypto.

[vt-alt]

If the caller uses a different version of libcrypto than the engine, it will segfault. And segfaults are bad for security.

Btw, this only happen to me when I play with manually built openssl OR engine and never when I use packages. And @ldv-alt's concern is packages.