-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-30008: OpenSSL 1.1 compatibility without using deprecated API #3943
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.
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
Conversation
Note: RAND_pseudo_bytes() is deprecated so RAND_bytes() is used when pseudo is requested.
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks,
you also need to sign the contributor agreement and add a news file with blurb.
@@ -1012,7 +1012,7 @@ PyInit__hashlib(void) | |||
{ | |||
PyObject *m, *openssl_md_meth_names; | |||
|
|||
#ifndef OPENSSL_VERSION_1_1 | |||
#if (OPENSSL_VERSION_NUMBER < 0x10100000L) || defined(LIBRESSL_VERSION_NUMBER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, but this is an unrelated fix and should be tracked in a separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, however it was the compiler that told me I needed to make this change
to correct this compiler error that occurs building Python 3.7.0a2 with openssl
1.1.0g built with disable-deprecated:
/var/tmp/portage/dev-lang/python-3.7.0_alpha2/work/Python-3.7.0a2/Modules/_hashopenssl.c: In function 'PyInit__hashlib':
/var/tmp/portage/dev-lang/python-3.7.0_alpha2/work/Python-3.7.0a2/Modules/_hashopenssl.c:1017:5: error: implicit declaration of function 'OPENSSL_add_all_algorithms_noconf' [-Werror=implicit-function-declaration]
OPENSSL_add_all_algorithms_noconf();
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/tmp/portage/dev-lang/python-3.7.0_alpha2/work/Python-3.7.0a2/Modules/_hashopenssl.c:1018:5: error: implicit declaration of function 'ERR_load_crypto_strings'; did you mean 'ERR_load_ERR_strings'? [-Werror=implicit-function-declaration]
ERR_load_crypto_strings();
^~~~~~~~~~~~~~~~~~~~~~~
ERR_load_ERR_strings
@@ -63,6 +63,7 @@ static PySocketModule_APIObject PySocketModule; | |||
#include "openssl/err.h" | |||
#include "openssl/rand.h" | |||
#include "openssl/bio.h" | |||
#include "openssl/dh.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is dh.h required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To fix thie compiler error that occurs building Python 3.7.0a2 with openssl 1.1.0g built with disable-deprecated:
/var/tmp/portage/dev-lang/python-3.7.0_alpha2/work/Python-3.7.0a2/Modules/_ssl.c: In function '_ssl__SSLContext_load_dh_params':
/var/tmp/portage/dev-lang/python-3.7.0_alpha2/work/Python-3.7.0a2/Modules/_ssl.c:3757:5: error: implicit declaration of function 'DH_free'; did you mean 'bt_free'? [-Werror=implicit-function-declaration]
DH_free(dh);
^~~~~~~
bt_free
Modules/_ssl.c
Outdated
@@ -5264,7 +5277,9 @@ PyInit__ssl(void) | |||
return NULL; | |||
PySocketModule = *socket_api; | |||
|
|||
#ifndef OPENSSL_VERSION_1_1 | |||
#ifdef OPENSSL_VERSION_1_1 | |||
OPENSSL_init_ssl(0, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to call this, OpenSSL calls this function internally and automatically.
https://www.openssl.org/docs/man1.1.0/ssl/OPENSSL_init_ssl.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, reverted in a separate commit. I can squash all the commits into 1 later if you like.
PS: I didn't expect the Spanish Inquisition!
Modules/_ssl.c
Outdated
@@ -4625,7 +4634,11 @@ PySSL_RAND(int len, int pseudo) | |||
if (bytes == NULL) | |||
return NULL; | |||
if (pseudo) { | |||
#ifdef OPENSSL_VERSION_1_1 | |||
ok = RAND_bytes((unsigned char*)PyBytes_AS_STRING(bytes), len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is safe to replace RAND_pseudo_bytes
with RAND_bytes
. A RAND engine may implement bytes
and pseudobytes
differently, e.g. bytes
may not fill the buffer to the end in case of an error.
The function https://docs.python.org/3/library/ssl.html#ssl.RAND_pseudo_bytes has been deprecated. We can drop when OpenSSL 1.2.0 removes the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I replaced it with:
ok = (_PyOS_URandom((unsigned char*)PyBytes_AS_STRING(bytes), len) == 0 ? 1 : 0);
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
This patch is not working for me:
|
@ivvolnov-radargeo The patch was split into 2 pull requests, you need both, see: |
to call this, OpenSSL calls this function internally and automatically.
264b6f3
to
8b5d5c2
Compare
I didn't expect the Spanish Inquisition! |
Nobody expects the Spanish Inquisition! @tiran: please review the changes made to this pull request. |
@tiran What's the status of this? |
Backport of PRs python#3934 and python#3943 Signed-off-by: Eneas U de Queiroz <[email protected]>
This PR rebased to Python 3.8.0: https://raw.githubusercontent.com/neheb/packages/1acf047c7f5c9d807ad49b9787aaaad543734be8/lang/python/python3/patches/020-ssl-module-emulate-tls-methods.patch Additional fixes:
|
This PR is stale because it has been open for 30 days with no activity. |
I believe Python 3 compiles without deprecated OpenSSL 1.1 APIs now. 3.0 is a much different story. |
This PR is stale because it has been open for 30 days with no activity. |
This pull request can be closed, per this comment:
Originally posted by @tiran in #20397 (comment) |
This PR is stale because it has been open for 30 days with no activity. |
Closing after #3943 (comment). |
Note: RAND_pseudo_bytes() is deprecated so RAND_bytes() is used when pseudo is requested.
https://bugs.python.org/issue30008