cpython icon indicating copy to clipboard operation
cpython copied to clipboard

Use OpenSSL 3.0 in our binary builds

Open gpshead opened this issue 3 years ago • 3 comments
trafficstars

Feature or enhancement

We currently use OpenSSL 1.1.1 series in our Windows and macOS binary builds.

Per https://www.openssl.org/source/, that is only supported through September of 2023.

Thus we need to switch to a supported version of OpenSSL before 3.12 is released. (And likely consider moving 3.11 to use it if deemed feasible)

There are a pile of bugs related to OpenSSL 3 that may or may not be blockers:

  • https://github.com/python/cpython/issues/90728
  • https://github.com/python/cpython/issues/90307
  • https://github.com/python/cpython/issues/95494
  • ... edit this list to link to others ...

We have a longer term desire to not be so beholden to OpenSSL at all. But this issue is being filed as a practical response to untangling that not being likely feasible before 3.12beta1.

gpshead avatar Nov 03 '22 22:11 gpshead

@gpshead Out of curiosity, Did we have any discussion about using boringSSL by vendoring them to the CPython repo?

corona10 avatar Nov 06 '22 11:11 corona10

Did we have any discussion about using boringSSL by vendoring them to the CPython repo?

I'm not sure how well that would work out. boringSSL isn't really intended to be used by other projects, it doesn't guarantee a stable API. Even for security updates. And it lacks some interfaces and features.

test_ssl.py needs significant modification to "pass" when linked with boringSSL and various pieces of the _ssl extension code need modifications as well as workarounds to disable unsupported features in some places. We carry patches on our Python runtime internally at Google that does some this, but they wouldn't be acceptable as is upstream as they disable features that need to work for global users and skip some tests rather than making them work. If we want to consider official boringSSL support in the CPython repo as well that should get tracked in another issue.

gpshead avatar Nov 11 '22 09:11 gpshead

it doesn't guarantee a stable AP

Oh, I thought that we can manage the boringSSL as the vendored library likewise libexpat or mimalloc.

We carry patches on our Python runtime internally at Google that does some this, but they wouldn't be acceptable as is upstream as they disable features that need to work for global users and skip some tests rather than making them work

Oh got it..

If we want to consider official boringSSL support in the CPython repo as well that should get tracked in another issue.

Yeah and we also need to write PEP also if we try to do, but we may need to know the possibility of the porting boringSSL into our repository first.

corona10 avatar Nov 11 '22 10:11 corona10

it doesn't guarantee a stable AP

Oh, I thought that we can manage the boringSSL as the vendored library likewise libexpat or mimalloc.

We could and would if we did this. It's mostly noteworthy as a risk as it could involve some refactoring larger than we like to do within security fix releases several years down the line in order to pull in other updates.

gpshead avatar Nov 11 '22 22:11 gpshead

Did we have any discussion about using boringSSL by vendoring them to the CPython repo?

Now on DPO!

Moving away from OpenSSL completely has some large maintenance benefits (but IMO even larger downsides too).

Moving to boringSSL sounds like a very bad proposition to my mind. OpenSSL is a heavy dependency, but it goes very far out of its way in terms of stability (which nowadays includes being buildable everywhere[^1]), e.g. it promises:

  • No API or ABI breaking changes are allowed in a minor or patch release.
  • No existing public interface can be modified except where changes are unlikely to break source compatibility [...]
  • No existing public interface can be removed until its replacement has been in place in an LTS stable release. The original interface must also have been documented as deprecated for at least 5 years.

I don't see sufficiently huge issues with OpenSSL (or sufficiently huge improvements with boringSSL) that a switch would look even remotely reasonable from my POV.

[^1]: not least still being on C89 to support some ultra-exotic platforms without modern compilers

h-vetinari avatar Nov 29 '22 00:11 h-vetinari

I built python 3.11.1 with OPENSSL 3.0.7 on windows 10 platform, when I run test_ssl.py, it shows no OPENSSL_Applink, error and crashed. I confirmed that applink.c file was added and complied, so I have no idea about that issue. I think maybe need some other code change about that?

https://github.com/python/cpython/issues/101401

mzhao-dev avatar Jan 30 '23 01:01 mzhao-dev

Marking this as a release blocker again because we really should do this before beta1 so that people test against a relevant representation of what the next release will contain.

gpshead avatar Apr 27 '23 21:04 gpshead

Can cpython limit openssl3 version? Some version of openssl3 (>3.0.8 ~&& <= 3.1.0~) breaks debuging libcrypto on Apple Silicon: https://github.com/openssl/openssl/issues/20188

Since libcrypto is required by fundamental modules like _ssl and _hashlib in cpython, this totally breaks debugging cpython on Apple chips. I think version restriction is worth considering.

Update: This is not fixed in any release branch of openssl. According to other comments, not only debugging is affected, but I have no problem normally running with those openssl versions.

Update 2: Given the condition where this error would be triggered (compile from source, Apple chip, use openssl >= 3.1, debugging with LLDB), I guess a warning message would be sufficient, maybe near the The necessary bits to build these optional modules were not found: after build?

oraluben avatar May 12 '23 05:05 oraluben

@ronaldoussoren, @ned-deily here is a macOS-specific OpenSSL issue reported in the previous comment by @oraluben.

arhadthedev avatar May 12 '23 05:05 arhadthedev

Update: This is not fixed in any release branch of openssl.

This is fixed in a branch (master), but not in any released version. I made a comment in the issue if the fix can be backported to the next 3.1.x release, because OpenSSL 3.2 will probably still take a while.

h-vetinari avatar May 12 '23 06:05 h-vetinari

it looks like they've at least got a pending PR that'll backport it to 3.1, but it isn't clear to me exactly which version the problem was introduced in. @oraluben mentions >3.0.8 as okay but from the linked openssl issue it is not obvious to anyone not intimately familiar with the openssl workflow or codebase which versions which offending changes landed in (read: I'm not going to be that person).

Regardless, a configure.ac check on Darwin to avoid a specific version range makes sense.

gpshead avatar May 12 '23 20:05 gpshead

I'm not sure why we should call this transitory issue out. Presumably it affects other projects than cPython as well and, in the end, we have no control over what version of OpenSSL or equivalent that builders or distributors use other than the python.org macOS installers which currently don't use OpenSSL 3.x anyway.

ned-deily avatar May 12 '23 21:05 ned-deily

but it isn't clear to me exactly which version the problem was introduced in

Well, I also misunderstood in my first comment. Here's my current understanding:

tldr: now, and hopefully only openssl 3.1.0 is affected.

Some instruction probing logic breaks debugging in darwin-aarch64, these commits are in the dev branch of openssl (master, SVE: https://github.com/openssl/openssl/commit/b1b2146ded9ce5a84c62f30c6c4a922b449f6c90, RNG: https://github.com/openssl/openssl/commit/efa1f22483ee43d84e1aee01b08c0bda04060c1c), and have been cherry-picked to 3.1 release branch (openssl-3.1, SVE: https://github.com/openssl/openssl/commit/a14eff6319ec254901e051d61e93d3272c451ebe, RNG: https://github.com/openssl/openssl/commit/45c74dea). They haven't been cherry-picked to 3.0, so openssl 3.0.x is not affected.

The fix (https://github.com/openssl/openssl/commit/52a38144b019cfda6b0e5eaa0aca88ae11661a26) is also in master, I submitted a PR to cherry-pick it in 3.1 (https://github.com/openssl/openssl/pull/20948), if it's merged, the only affected version would be 3.1.0. I guess 3.0.x would be ok forever, if openssl decided to apply those instruction tests I expect they won't forget the corresponding fix.

oraluben avatar May 13 '23 03:05 oraluben

My pardon for an attempt to reassign this issue to 3.13, I missed that it's a release blocker for 3.12.

However, will it be possible to resolve the child gh-90728 tomorrow at best, the next week at worst?

arhadthedev avatar May 20 '23 18:05 arhadthedev

deferring this until after beta1.

gpshead avatar May 20 '23 18:05 gpshead

Sounds like openssl/openssl#20948 ,the backport PR to fix the aforementioned problem, was closed by the maintainers due to the size of the change relative to the perceived impact of the breakage, with a note that it would be reconsidered it he latter was in fact larger than currently understood. If this is going to be a major problem for CPython, perhaps someone should comment over there?

CAM-Gerlach avatar Jun 08 '23 02:06 CAM-Gerlach

I am lowkey convinced that the upstream issue could also be solved more minimally, i.e. https://github.com/openssl/openssl/commit/52a38144b019cfda6b0e5eaa0aca88ae11661a26 did a refactor of some code and disabled it for apple silicon, but haven't investigated more deeply...

h-vetinari avatar Jun 08 '23 02:06 h-vetinari

retitled back to 3.0 as per https://www.openssl.org/policies/releasestrat.html 3.0.x is Long Term Support and thus the release with the longest support guarantee: until 2026-09.

I think that means we can skip the "openssl 3.1.x breaks debugging on macOS due to SIGILL" debacle if I even understand that openssl PR/issue right. (not that we'd block on that anyways - we don't have a choice)

gpshead avatar Jun 08 '23 05:06 gpshead

retitled back to 3.0 [...]

If we assume that that we should focus on 3.0.x support on macOS for the time being at least, I think the first step would be to update our GitHub Workflow macOS CI jobs for 3.11+ to use 3.0.x from Homebrew: currently they are using 1.1.x. We should also update the devguide recommendations for macOS to use 3.0.x; there is a WIP issue for the devguide that should be changed to use 3.0.x. Also, any macOS buildbots should be updated, as feasible. With that additional experience we can then move the macOS installers to using 3.0.x sometime in the not-too-distant future.

ned-deily avatar Jun 08 '23 05:06 ned-deily

What is left to do for this? Do the Windows and MacOS builders use 3.0 now?

Yhg1s avatar Jul 10 '23 13:07 Yhg1s

What is left to do for this? Do the Windows and MacOS builders use 3.0 now?

To the best of my knowledge and after a quick look, I believe it is the case that all Windows builds (that is, GitHub CI workflows, buildbots, and binary release artifacts) are currently using OpenSSL 1.1.1u. To move to 3.0.9, say, would require updating and testing our cpython-source-deps and cpython-binary-deps repos and then various build files in the cpython repo, like PCbuild/openssl.props and then ensuring all the users thereof (CI, buildbots, binary release artifacts) have no test regressions. @zooba, is that your understanding?

For macOS, we have fairly recently changed the GitHub CI workflows) to use 3.0.9. The Intel macOS buildbot appears to be using 3.1.1; the Apple Silicon macOS buildbot is using 3.0.0. And the macOS binary installer builds we ship still use 1.1.1u. We should now be able to fairly easily change the binary installer builds to use 3.0.9 at any point. There is some testing that needs to be done to ensure 3.0.9 builds and packages cleanly in our environment and tests pass across all our supported versions of macOS and Macs. I planned to do that for main and could expedite that. Backporting to 3.12 or earlier would be trivial.

That leaves open what we should do for 3.12 and earlier releases and when.

ned-deily avatar Jul 10 '23 21:07 ned-deily

Certainly the first step is to clone the sources and update our build scripts to produce the pre-built binaries we use. Assuming that goes smoothly, we can update the CPython build scripts to use that 3.0 build and see what changes in CPython itself (I assume something will change, there can't have been that much testing on Windows). Once that dust settles, we can merge that PR and consider backporting.

I've never even looked at OpenSSL 3.0, so I have no idea of how much work is involved. If they haven't changed their build system at all, it's probably straightforward for the first part. And they tend to be pretty good at cross-plat support, so I imagine the existing changes in _ssl will be okay. But we definitely have tests that are carefully constructed to detect weird behaviours, and those are likely to need updating as well.

zooba avatar Jul 10 '23 21:07 zooba

Looks like best case scenario for Windows. One small tweak to the license filename, but otherwise we should be good to move to 3.0.9.

zooba avatar Jul 11 '23 19:07 zooba

@Yhg1s The backport PR to 3.12 for Windows is ready to go. Up to you whether you want it at this stage for RC - feel free to hit merge if so.

zooba avatar Jul 12 '23 13:07 zooba

Excellent! I'm glad it's not a big change. All the existing (not Python-specific) issues with OpenSSL 3 notwithstanding, I'm much happier that we won't be shipping an included OpenSSL that's going to be end of life before 3.12.0 final.

Yhg1s avatar Jul 12 '23 14:07 Yhg1s

Since OpenSSL 1.1.1 will be EOL in September but Python 3.11 binaries are to be released until April, should we backport to 3.11 as well? Or not until we must?

zware avatar Jul 12 '23 15:07 zware

Technically (arguably?) updating from 1.1 to 3 will change our filesystem layout, at least on Windows, as libssl-1_1.dll becomes libssl-3.dll. Maybe we don't extend our promise to dependent libraries though, but I bet at least some people are relying on being able to resolve libssl-1_1.

Compatibility-wise, it's definitely safer to not backport. Security-wise, it's probably safer to backport. I'm not sure how best to resolve this... toss a coin?

zooba avatar Jul 12 '23 15:07 zooba

We need to backport to 3.11. Mention it in the release notes. At most just point Windows people at where they can get a prebuilt past EOL libssl-1_1.dll to be responsible for and include in their own application if some other code needs it.

gpshead avatar Jul 12 '23 16:07 gpshead

What's the status of macOS? Python 3.12 and 3.13 are now built with OpenSSL 3.0 on Windows, but OpenSSL 1.1 on macOS. See my comment on my PR fixing a compiler warning: https://github.com/python/cpython/pull/106700#issuecomment-1638447979

It seems like right now, OpenSSL 1.1 is used on Windows and macOS in the Python 3.11 branch.

vstinner avatar Jul 17 '23 16:07 vstinner

What's the status of macOS? Python 3.12 and 3.13 are now built with OpenSSL 3.0 on Windows, but OpenSSL 1.1 on macOS.

See comment above. Currently, AFAIK, we are now using OpenSSL 3.x for all macOS Github CI builds and buildbots. We also plan to provide OpenSSL 3.x for the Windows and macOS 3.12.0rc1 and future 3.11.x release binaries. (What is in build-installer.py has no effect on macOS CI or buildbots.) Are you seeing something different, @vstinner?

ned-deily avatar Jul 17 '23 17:07 ned-deily