bpo-43466: Add -Wl,--exclude-libs to ssl #24989
Conversation
Add linker flags -Wl,--exclude-libs,libssl.a and libcrypto.a so statically linked OpenSSL builds don't export OpenSSL symbols. Signed-off-by: Christian Heimes <christian@python.org>
fc05994 to
9aa8cce
Compare
gpshead
left a comment
There was a problem hiding this comment.
overall good. one suggestion.
setup.py
Outdated
| else: | ||
| runtime_library_dirs = [openssl_rpath] | ||
|
|
||
| kw = dict( |
There was a problem hiding this comment.
I suggest renaming kw to openssl_extension_kwargs
setup.py
Outdated
| depends=['socketmodule.h', '_ssl/debughelpers.c']) | ||
| self.add( | ||
| Extension( | ||
| '_ssl', ['_ssl.c'], |
There was a problem hiding this comment.
Instead on reliying on the absence of the shared library for openssl, we should force the linker to pick the archive. The syntax is:
-l:ssl.a
this needs to be added to extra_link_args parameter and drop the one in libraries
There was a problem hiding this comment.
I'm strong -1. This feature is already beyond what I originally wanted to commit to.
Your request would make things even more complicated. I really don't want to support static linking because I have neither time nor motivation to deal with the additional support overhead. Static linking of OpenSSL has negative consequences for security: you cannot easily update.
PyCA/cryptography deals with update issue of static-linked code by tying the release cycle to OpenSSL security releases. Alex and Paul release a new version a couple of hours after each and every OpenSSL security release. Rhetorical question: As the new RM do you want to commit to the same release policy? (please say: "no").
There was a problem hiding this comment.
Agreed, the goal of this PR isn't to static link by default. It's just to enable the possibility of doing that in a reasonable isolated manner when someone builds against a static only OpenSSL.
There was a problem hiding this comment.
As the new RM do you want to commit to the same release policy? (please say: "no").
I say "no" :)
I'm strong -1. This feature is already beyond what I originally wanted to commit to.
If you are -1, let's not do it. I trust your juzguement here.
Your request would make things even more complicated. I really don't want to support static linking because I have neither time nor motivation to deal with the additional support overhead. Static linking of OpenSSL has negative consequences for security: you cannot easily update.
I think also that there is some misunderstanding in what I am trying to suggest here. I will try to explain it better:
- I don't want us to officially support static linking if we don't want.
- I don't want to statically link by default
- I don't want us to support many ways to do things
What I want here is to offer some safe enough alternative to those users that need to download their own version of OpenSSL in an environment where OpenSSL 1.0.2 is present. We don't need to officially support it, but I think we need to offer a way so this can be achieved without having to patch CPython. The reason this may be more important is due to the fact that shortly after 3.10 is released 3.9 will lose bugfix support so users in this situation will have no way to use a bugfix-supported Python.
I don't have special interest in pushing one solution or the other: I am triying to explain the concerns that exist in the general ecosystem. You are our trusted expert in SSL and security and we trust you and your juzguement.
Please, understand that my comments are suggestions and general questions so we can provide a good experience for everyone, they are not requirements or something you need to act on now :)
There was a problem hiding this comment.
Agreed, the goal of this PR isn't to static link by default. It's just to enable the possibility of doing that in a reasonable isolated manner when someone builds against a static only OpenSSL.
Right, I think I have not expressed myself correctly. What I was suggesting is to offer an alternative, opt-in way to statically compile openssl, even if they produce the shared object of SSL in the build.
There was a problem hiding this comment.
We could:
-
Configure option that explicitly states that is not an officially supported option in the help section.
-
Environment variable present that we check in the
setup.py> -
Maybe some specific flag in the
pyconfig.h
In general terms we either don't document it directly or (my preference) we explicitly state that whatever we use is not officially supported and it's only purpose is to make life easier for specific use cases and that users are responsible of making sure everything works correctly.
There was a problem hiding this comment.
How about PY_UNSUPPORTED_OPENSSL_BUILD=static make?
By the way your instructions are incorrect. :)
There was a problem hiding this comment.
How about ``PY_UNSUPPORTED_OPENSSL_BUILD=static make`
👍
By the way your instructions are incorrect. :)
Oh! What didn't work? I made a rough prototype with my instructions and that worked on my side. I can share the patch if you want. What errors are you experiencing?
There was a problem hiding this comment.
-l:ssl.a does not work for me (GCC 10). It has to be filename of the archive: -l:libssl.a
There was a problem hiding this comment.
-l:ssl.adoes not work for me (GCC 10). It has to be filename of the archive:-l:libssl.a
Oh, right, that was totally my fault. It should have said -l:libssl.a indeed :)
Signed-off-by: Christian Heimes <christian@python.org>
|
I'm rejecting my PR in favor of #25002. The other PR is simpler and accomplishes @pablogsal requirements. |
Add linker flags -Wl,--exclude-libs,libssl.a and libcrypto.a so
statically linked OpenSSL builds don't export OpenSSL symbols.
Signed-off-by: Christian Heimes christian@python.org
https://bugs.python.org/issue43466