gh-99951: Warn if there is an OpenSSL major version mismatch #100641

moonsikpark · 2022-12-31T13:14:41Z

While it is a rare edge case well beyond the average use case, there could be situation where there is an OpenSSL version mismatch between the version python was compiled against, and the version currently loaded. Because OpenSSL states that major releases can break compatibility with previous versions and checks are cheap, there seems to be no harm if python warns when it happens.

tl;dr: This PR warns users when importing the ssl module when the OpenSSL major version mismatch between the version python was compiled against, and the one it is using.

Issue: Signal if there is an OpenSSL major version mismatch #99951

ghost · 2022-12-31T13:14:43Z

All commit authors signed the Contributor License Agreement.

bedevere-bot · 2022-12-31T13:14:46Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

This reverts commit f9171f0.

eli-schwartz

IMO this is really a Node.js bug for exposing bad symbols, but arguing that it's a bug in the other program does still leave python users having issues so I guess there's a rationale to say python might still want to check it.

That being said, I don't think this is the right fix, it should be an error as it's guaranteed to not work -- you should be prevented from successfully importing the module, just like you would be if the _ssl module failed to load because the libssl library was missing, rather than being overridden.

(Maybe the error should be raised in _ssl rather than ssl? idk)

eli-schwartz · 2023-01-01T05:24:24Z

Lib/ssl.py

+        f"{_OPENSSL_API_VERSION[0]}.{_OPENSSL_API_VERSION[1]}, "
+        "but is using OpenSSL "
+        f"{OPENSSL_VERSION_INFO[0]}.{OPENSSL_VERSION_INFO[1]}. "
+        "OpenSSL does not guarantee compatibility between different major versions.",


This is incorrect -- it's guaranteed to not work. :P

On Linux, at least, openssl's util/mkdef.pl ensures the build has versioned symbols, specifically to ensure that it's safe to load multiple libssl libraries into a single process and you will always get the symbols you actually linked to, and cannot ever get the ones you are guaranteed to be unable to safely and compatibly use.

Are you saying that it does not matter if the runtime loaded openssl version does not match with the openssl header version used for compilation as long as the SHLIB_VERSION_NUMBER or OPENSSL_SHLIB_VERSION are the same?

I'm concerned specifically in this scenario where I compiled python with 3.1beta02 headers but is loading 3.0 so files. (or vice versa)

root@0e68e1d0c74e:~/cpython# python3.12 Python 3.12.0a3+ (heads/main:b7a68ab824, Jan 2 2023, 09:02:24) [GCC 11.3.0] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import ssl >>> ssl.OPENSSL_VERSION_INFO (3, 0, 0, 2, 0) >>> ssl._OPENSSL_API_VERSION (3, 1, 0, 0, 0)

If so, will it be beneficial to expose SHLIB_VERSION_NUMBER in _ssl.c and compare those instead?

Are you saying that it does not matter if the runtime loaded openssl version does not match with the openssl header version used for compilation?

Specifically in this scenario where I compiled python with 3.1beta02 headers but is loading 3.0 so files? (or vice versa)

Correct -- assuming that openssl has been built correctly per upstream OpenSSL's build system.

I haven't tested openssl 3.1 betas myself, but presumably this installs libssl.so.3 to replace the libssl.so.3 which was installed by openssl 3.0.7? This should still be binary compatible.

I've seen too many projects that add runtime checks for the versions of their dependencies, e.g. hdf5 is an excellent example of what not to ever do (they would wrap all functions in a compile-time macro that first checks if the full patch-level version embedded at compile time, exactly matches the full patch-level version loaded from the library at runtime). The ABI soname is already supposed to describe when things are compatible, and really, this is bulletproof unless you actually have preloaded symbols from another version loaded into the same process space and they aren't versioned. Which seems to be the case for Node.js here.

But you really do not want to over-correct and report errors even when the ABI soname matches, just because a minor or patchlevel release changes.

tl;dr

Checking for ssl._OPENSSL_API_VERSION tells you the API, but you're trying to check if the ABI matches. The two are not related, although frequently they are near each other. Ideally, you'd be able to compare the compile-time ABI with the runtime ABI.

Thanks for the insight.

However the OpenSSL Release Strategy document states the following, which seems to suggest that there could be an API or ABI breaking changes in a major release.

No API or ABI breaking changes are allowed in a minor or patch release.

Looking at openssl/openssl@opensslv.h.in#L52-L58,

* Shared library version * * This is strictly to express ABI version, which may or may not * be related to the API version expressed with the macros above. * This is defined in free form.

it seems OPENSSL_SHLIB_VERSION is to only express ABI version, not API version. Should we not cover API changes also by checking the major release? Hypothetically, an API could change while an ABI does not. (though it is not desired...)

If an API changes, but an ABI doesn't, then applications built against the old API do not care because after being compiled, they do not use the API anymore. :)

(It's actuall 8000 y desired reasonably often. Libraries can implement both the old and new versions of the library in a single build, which means the ABI does not change, but drop support for the old version in the API. This prevents programs from building against deprecated and removed API, without dropping support for already compiled binaries. GNU libc, for example, makes heavy use of this.)

Interesting!

However, won't it be a problem for the following cases, where you do different things depending on the API version? On an hypothetical scenario where openssl 4 has introduced breaking API change and not ABI change, and I compiled an application with openssl 4 headers but loaded openssl 3 libraries, if there is a code that checks for OPENSSL_VERSION_NUMBER >= 4 and does something different, won't it cause issues?

cpython/Modules/_hashopenssl.c

Lines 53 to 63 in 0709586

#if OPENSSL_VERSION_NUMBER >= 0x30000000L

#define PY_EVP_MD EVP_MD

#define PY_EVP_MD_fetch(algorithm, properties) EVP_MD_fetch(NULL, algorithm, properties)

#define PY_EVP_MD_up_ref(md) EVP_MD_up_ref(md)

#define PY_EVP_MD_free(md) EVP_MD_free(md)

#else

#define PY_EVP_MD const EVP_MD

#define PY_EVP_MD_fetch(algorithm, properties) EVP_get_digestbyname(algorithm)

#define PY_EVP_MD_up_ref(md) do {} while(0)

#define PY_EVP_MD_free(md) do {} while(0)

#endif

edit: OpenSSL versioning policy has changed, so 3.0 -> 3.1 is not a major version bump and will not introduce a breaking API/ABI change. edited the question accordingly.

That code is using the version number to select whether to bud against the 3.x API or the older 1.x API.

Either way, once the ssl module is compiled into a binary extension it uses the application binary interface (ABI) that the API names referenced.

eli-schwartz · 2023-01-01T05:44:20Z

The best way to check this would be to compare openssl/opensslv.h SHLIB_VERSION_NUMBER (1.1) or OPENSSL_SHLIB_VERSION (3.0) at build and runtime, but it's only available as a header macro... this is the information you want though, since it specifically encodes the ABI compatibility.

warn when python is using different openssl version

9c5814a

bedevere-bot mentioned this pull request Dec 31, 2022

Signal if there is an OpenSSL major version mismatch #99951

Open

bedevere-bot added the awaiting review label Dec 31, 2022

add NEWS entry

c88b3d9

This reverts commit f9171f0.

eli-schwartz suggested changes Jan 1, 2023

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Jan 1, 2023

moonsikpark closed this by deleting the head repository Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-99951: Warn if there is an OpenSSL major version mismatch #100641

gh-99951: Warn if there is an OpenSSL major version mismatch #100641

	#if OPENSSL_VERSION_NUMBER >= 0x30000000L
	#define PY_EVP_MD EVP_MD
	#define PY_EVP_MD_fetch(algorithm, properties) EVP_MD_fetch(NULL, algorithm, properties)
	#define PY_EVP_MD_up_ref(md) EVP_MD_up_ref(md)
	#define PY_EVP_MD_free(md) EVP_MD_free(md)
	#else
	#define PY_EVP_MD const EVP_MD
	#define PY_EVP_MD_fetch(algorithm, properties) EVP_get_digestbyname(algorithm)
	#define PY_EVP_MD_up_ref(md) do {} while(0)
	#define PY_EVP_MD_free(md) do {} while(0)
	#endif

gh-99951: Warn if there is an OpenSSL major version mismatch #100641

gh-99951: Warn if there is an OpenSSL major version mismatch #100641

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment