crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.51k stars 1.62k forks source link

`libssl` bindings fall back to ancient library version #14254

Open straight-shoota opened 10 months ago

straight-shoota commented 10 months ago

The bindings for libssl try to figure out the installed version of the library via pkg-config. If that does not work out (pkg-config missing or the specific configuration for libssl is missing), it falls back to a default version. This default version is currently 0.0.0, i.e. the lowest version number. That means all version comparisons regarding more recent versions will report the version to be lower. Newer versions of libssl have a number of breaking ABI changes compared to ancient versions which are still supported in the bindings. As as result, if pkg-config fails, the bindings will be tailored for an old version and most likely won't work with whatever reasonable version is available on the system.

I think this is a very poor fall back behaviour. Of course, ideally, pkg-config should be available and provide the necessary information. But that's not always the case. I think a more reasonable fall back would be to a healthily more modern version. This should allow most parts of the bindings to work by default with more modern version of the library. I think a good choice would probably be 3.0.0, or maybe 1.1.0 although the former has spread pretty wide by now.

HertzDevil commented 9 months ago

Related: #7244, #11069

ysbaddaden commented 9 months ago

I still believe #7245 was the correct solution: get the information from C headers when pkg-config is missing. It would detect the actual version of libressl or openssl and fail when the headers are missing.

We can't guess what version or which of libressl or openssl is installed, and we shouldn't have to.

straight-shoota commented 9 months ago

@ysbaddaden #7245 could be helpful, but it's not a proper solution. The whole problem is more complex and improving autodiscovery only get's us so far.

This issue is about the current fallback solution if autodiscovery of the library version doesn't work. #7245 wouldn't solve this problem because C headers could be missing as well. So we still need a default behaviour when autodiscovery doesn't work. I don't think you should need C headers for building Crystal bindings just for figuring out the version. If it works, that's fine. But it must not be a requirement. A solution could be a mechanism for explicitly configuring the library version.

straight-shoota commented 9 months ago

I'd like to keep this issue focused narrowly on the fallback version in the current state of things. It could be relatively easy to gain a small improvement by updating the fallback to a more recent version. That's certainly not a wholesome solution to the overall problem, but it's managable and has a direct benefit.

We should have another discussion about the whole topic of library information. I'm collecting some talking points for that and will open a thread on the forum.

ysbaddaden commented 9 months ago

There is no fallback in #7245: compilation will fail if the C headers aren't installed (or reachable).

ysbaddaden commented 1 month ago

Coming back to this: the openssl situation can't be solved like the other libraries.

There are two conflicting forks, each with their own set of features, but the same set of header files (sic) and lib names (sic) and conflicting version numbers (sic), the pkg-config can be missing or report an older version (sic) and we can't tell libressl vs openssl anyway.

We try to guess and we fail in the worst possible manner: by happily building an invalid executable that will crash or raise exceptions without any sort of explanation :sob:

The only way to tell OpenSSL from LibreSSL and which version they are is to look at OPENSSL_VERSION_NUMBER and LIBRESSL_VERSION_NUMBER from the C headers. We MUST require the C headers to be present and we MUST use the above constants to determine their version (aka #7245).

I prefer to fail compilation with an explicit and nice:

please install openssl or libressl development files (you can specify OPENSSL_DIR to target a specific installation).

straight-shoota commented 1 month ago

Might not help much, but it should be possible to tell LibreSSL vs. OpenSSL form pkg-config data. Just not directly with the pkg-config tool alone.

Usually the package config file contains additional metadata such as Name and Description. And I presume they should mention whether the package is libre or open. Might not be guaranteed, though. And these fields are not required as far as I know, so the information could just be missing.

The metadata is not available via the pkg-config CLI. But it can tell the location of the discovered pc file via the variable pcfiledir. This lets us construct the full path as $(pkg-config --variable=pcfiledir $modname)/$modname.pc. We can read the file and check if it contains LibreSSL.

But yeah, the entire situation is pretty messed up.

ysbaddaden commented 1 month ago

Sadly it doesn't :sob:

[vagrant@openbsd7 lib]$ cat /usr/lib/pkgconfig/libssl.pc
prefix=/usr
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: OpenSSL-libssl
Description: Secure Sockets Layer and cryptography libraries
Version: 2.0.0
Requires.private: libcrypto
Libs: -L${libdir} -lssl
Cflags: -I${includedir}

[vagrant@openbsd7 lib]$ grep VERSION_NUMBER /usr/include/openssl/opensslv.h
#define LIBRESSL_VERSION_NUMBER 0x4000000fL
#define OPENSSL_VERSION_NUMBER  0x20000000L
#define SHLIB_VERSION_NUMBER "1.0.0"

So it's technically not lying: the libressl pc file reports the "openssl" version (that never existed) :shrug:

ysbaddaden commented 1 month ago

Maybe the shell script from #7245 then we rebuild a simplified semantic version string from the version number:

0x10200000L => 0x1_02_00_00_f => "1.0.2"
0x30202000L => 0x3_02_02_00_f => "3.2.2"
0x4000000fL => 0x4_00_00_00_f => "4.0.0"

The script would allow to autodetect OpenSSL vs LibreSSL from installed headers, but also allow to override the version string with environment variables, for example OPENSSL_VERSION=1.1.1 and LIBRESSL_VERSION=4.0.0 to skip the autodetection.