Closed JohnDoee closed 8 years ago
reaperhulk
commented
8 years ago Thanks for the report. @hynek, @alex do we want to do a quick release to disable the osrandom engine to prevent these types of regressions while we work on a more permanent fix? (Will also fix #542)
alex
commented
8 years ago @reaperhulk how do you imagine that PR looking, just moving the import into the relevant functions?
I won't object to such a patch, however I'm a bit concerned that it's moving the peas around our plate and hoping our parents don't notice. Specifically, anyone who uses those functions (which might be limited ATM, tbh) will continue to hit issues, and they'll just be harder to debug. Also anyone who uses both pyOpenSSL and cryptography's backend APIs will as well.
reaperhulk
commented
8 years ago I was envisioning it as moving the backend import to a separate module and calling backend.activate_builtin_random(), then importing backend from that where we use it. Since from cryptography.hazmat.backends.openssl.backend import backend is effectively a singleton (we're importing an instance) backend.activate_builtin_random() will stop the use of the osrandom engine for the lion's share of cases. If a user separately imports from cryptography.hazmat.backends.openssl.backend import Backend and instantiates Backend() then it would re-activate osrandom of course.
The disadvantage to this approach is obviously that it stops cryptography from using the superior osrandom engine, but based on the scant reports of problems before we activated it in pyOpenSSL I'm guessing we don't have a great deal of overlap in users of our hazmat/recipes and pyOpenSSL. Long term obviously we don't want to be deactivating osrandom, but we won't have the replacement ready immediately.
alex
commented
8 years ago As I've said before, I strongly object to calling activate_builtin_random;
I think moving the import into the function gets us the majority of the
benefit while minimizing risk.
On Wed, Oct 5, 2016 at 8:01 AM, Paul Kehrer notifications@github.com wrote:
I was envisioning it as moving the backend import to a separate module and calling backend.activate_builtin_random(), then importing backend from that where we use it. Since from cryptography.hazmat.backends.openssl.backend import backend is effectively a singleton (we're importing an instance) backend.activate_builtin_random() will stop the use of the osrandom engine for the lion's share of cases. If a user separately imports from cryptography.hazmat.backends.openssl.backend import Backend and instantiates Backend() then it would re-activate osrandom of course.
The disadvantage to this approach is obviously that it stops cryptography from using the superior osrandom engine, but based on the scant reports of problems before we activated it in pyOpenSSL I'm guessing we don't have a great deal of overlap in users of our hazmat/recipes and pyOpenSSL. Long term obviously we don't want to be deactivating osrandom, but we won't have the replacement ready immediately.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pyca/pyopenssl/issues/551#issuecomment-251654997, or mute the thread https://github.com/notifications/unsubscribe-auth/AAADBEzhWedUUMSI8kKP5t-pYedU5XWbks5qw5GWgaJpZM4KOGJ2 .
"I disapprove of what you say, but I will defend to the death your right to say it." -- Evelyn Beatrice Hall (summarizing Voltaire) "The people's good is the highest law." -- Cicero GPG Key fingerprint: D1B3 ADC0 E023 8CA6
reaperhulk
commented
8 years ago Certainly moving the import into the function will fix it for most users (who are not calling those conversion routines). Maybe that's sufficient. Obviously the tradeoff is that if a user does call the offending functions at some point during their program execution, is in a multiprocess/multithread execution environment, and subsequently makes a TLS connection using pyOpenSSL they'll trigger the bug.
I don't have a strong preference since every choice has drawbacks. I will force you to express an opinion tonight @hynek ;)
JohnDoee
commented
8 years ago There's one thing I'd love to get explained, why does libtorrent, a C++ library with Python bindings, end up calling libssl that calls back into Python ? The disconnect for me is in Thread 3 - # 9 # 10 # 11
reaperhulk
commented
8 years ago There's only one instance of libssl loading into the process space and when pyOpenSSL loads up it is activating an engine that calls back into Python. So even invocations of TLS calls by libtorrent will trigger the callback.
reaperhulk
commented
8 years ago This should be resolved in 16.2.0
mcepl
commented
6 years ago Hmm, when applying a84a52ceeb8deb602515ebf76d3de3ec0e24ba56 to pyOpenSSL 16.0.0 (python-cryptography 1.3.1) on SLE 12 (SUSE enterprise Linux distribution) I get some failed tests on x86_64. See build log for more details. The fail is intermittent, when rerunning, it passes.
Any thoughts on this?
When using Deluge with pyOpenSSL 16.1.0 and torrents with trackers using https then Deluge will end up in a deadlock. It works fine with 16.0.0.
I've attached the stacktrace and I think the deadlock is between Thread 3 trying to acquire GIL lock and Thread 1 currently holding it. I'm unsure why this is happening as libtorrent-rasterbar is a C library.
You can reproduce this by installing pyOpenSSL 16.1.0, any recent version of Deluge, any recent version of libtorrent and any recent version of OpenSSL. Add a few torrents with https trackers (these trackers does not need to work, loading the SSL context seems sufficient). Restart Deluge until it locks up during boot.
After it locks up once it'll keep locking up.