8000 gh-99951: Warn if there is an OpenSSL major version mismatch by moonsikpark · Pull Request #100641 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
+9 −0
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c5814a
warn when python is using different openssl version
moonsikpark Dec 31, 2022
c88b3d9
add NEWS entry
moonsikpark Dec 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
8000
Diff view
8 changes: 8 additions & 0 deletions Lib/ssl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,14 @@ class _TLSMessageType:
import errno
import warnings

if not OPENSSL_VERSION_INFO[:2] == _OPENSSL_API_VERSION[:2]:
warnings.warn(
"Python was compiled against OpenSSL "
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.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author
@moonsikpark moonsikpark Jan 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others 10000 . Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author
@moonsikpark moonsikpark Jan 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 actually 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.)

Copy link
Contributor Author
@moonsikpark moonsikpark Jan 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor
@eli-schwartz eli-schwartz Jan 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

category=RuntimeWarning, stacklevel=2)

socket_error = OSError # keep that public name in module namespace

Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2022-12-31-13-27-16.gh-issue-99951.BGyDJT.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Raises :exc:`RuntimeWarning` when there is an OpenSSL major version mismatch between the version python was compiled against and the version python is currently using.
0