-
Notifications
You must be signed in to change notification settings - Fork 420
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
test segfault on NetBSD #596
Comments
By looking only at the test log, it seems like the crash happens in a callback-related test. i would assume it's the exact same crash as in cryptography. |
Unfortunately the nature of the static callbacks workaround in cryptography is not one that pyOpenSSL can pursue. This means we need a reduced test case so we can (probably) submit a bug to cffi. |
Add patch that makes tests on NetBSD progress further. But then there's a segfault. See pyca/pyopenssl#596 16.2.0 (2016-10-15) ------------------- Changes: ^^^^^^^^ - Fixed compatibility errors with OpenSSL 1.1.0. - Fixed an issue that caused failures with subinterpreters and embedded Pythons. `#552 <https://github.com/pyca/pyopenssl/pull/552>`_ 16.1.0 (2016-08-26) ------------------- Deprecations: ^^^^^^^^^^^^^ - Dropped support for OpenSSL 0.9.8. Changes: ^^^^^^^^ - Fix memory leak in ``OpenSSL.crypto.dump_privatekey()`` with ``FILETYPE_TEXT``. `#496 <https://github.com/pyca/pyopenssl/pull/496>`_ - Enable use of CRL (and more) in verify context. `#483 <https://github.com/pyca/pyopenssl/pull/483>`_ - ``OpenSSL.crypto.PKey`` can now be constructed from ``cryptography`` objects and also exported as such. `#439 <https://github.com/pyca/pyopenssl/pull/439>`_ - Support newer versions of ``cryptography`` which use opaque structs for OpenSSL 1.1.0 compatibility.
I think cffi is not the issue there. It fails its ffi.callback() tests too, indeed, but it works fine elsewhere. The problem might be in libffi instead. There are several reasons for why I marked ffi.callback() as deprecated, and this is one of them: it involves different paths inside libffi, which are typically less well-tested. And indeed, apparently libffi fails a third of its own tests on NetBSD... |
Converting to new-style callbacks might help, see https://cffi.readthedocs.io/en/latest/using.html#id3 |
@reaperhulk: for reference, you call it a "workaround" in cryptography, but I call it a clean up: it's a move away from the general mess of libffi callbacks, which keep hurting users on less-common architectures or with hardened systems. |
@arigo no argument, I only called it a workaround since it just changed the code path rather than attempting to understand the underlying issue 😄 Unfortunately switching to static callbacks in pyOpenSSL is more challenging as it will require us to define them in cryptography (even though it does not use them) so that pyOpenSSL can consume them. That's doable, but unfortunate from a code organization standpoint. |
and I suppose you don't want to add a compilation step to pyopenssl, even though a similar compilation step is needed for cryptography? |
It’s not really “even though”. We were really happy to make pyOpenSSL Python-only and focus all compiler weirdnesses on cryptography that comes with the appropriate (HUGE) fleet of CI builders. Keeping all C stuff in cryptography seems like the most pragmatic solution to me ATM. This also touches #595. :| |
@arigo Yeah I'd rather deal with it in cryptography since we have all the wheel and testing infrastructure in place. I think @tiran will find this interesting since he has previously wanted to make pyOpenSSL use static callbacks defined in cryptography and we were resistant at the time. Seems like it's time to revisit that. |
I looked into the matter when I assisted @arigo with testing new static callback and when I ported cryptography to static callbacks. It's doable but it would be a humongous undertaking in both PyOpenSSL and cryptography. Some OpenSSL callbacks take a user specified void ptr, some callbacks have to store context data on a struct.
|
@reaperhulk Yeah, I lost interest as soon as I realized the amount of work. I'm not sure if @hynek can stomach a major rewrite, too. |
I’d certainly not be happy and I’d even more certainly not actually do it. However it seems like the current callback situation is problematic. I dare to guess that #447 might be related too? So if we could form task force of people interested (and competent) for this kind of work, I’d more likely help than obstruct. |
I would be happy to take part in a "task force" to try to improve the libffi situation. However, we need ssh access to various relatively uncommon systems. Last time I looked, cffi's tests were more comprehensive than libffi's own tests for this kind of problems, so that's a good starting point. There is also the issue on hardened systems, which libffi solves using the wrong approach because it can crash after fork() in a security-exploitable manner; it could also be fixed inside libffi with enough efforts. It's harder work, though. (Fixing it would solve the uneasiness I have seeing a security-oriented library like pyopenssl using a feature that requires disabling some security checks on hardened systems.) |
This seems like a good long term idea since libffi is critical software. I don't know that I have the technical expertise in this arena to assist, but I can definitely organize access to systems. Outside of NetBSD what would you hypothetically like to have access to Armin? |
This is all great news! Frankly we need at least one expert on cffi (and I wouldn’t know anyone better than Armin :)) and at least one for OpenSSL. So if @tiran and @reaperhulk could share something…I’m happy to help out too, but I’m still mainly the janitor around here. :D |
libffi is fixed, at least on NetBSD 8.99.1 with the latest libffi from pkgsrc, see libffi/libffi#294 |
@jsonn fixed cffi (see https://bitbucket.org/cffi/cffi/issues/321/cffi-191-segmentation-fault-during-self). I don't see any segfaults in pyopenssl any longer, I'll report other test failures separately. |
py-acme does not work on NetBSD. Tracking down the problem we first fixed an issue in cryptography (with help from @reaperhulk in pyca/cryptography#3372). Now I see a segfault in pyopenssl (after applying the patch from #595):
The python backtrace is:
That's with python-3.6.0 and openssl-1.0.2j in case it matters.
I'm new to pypi. Any ideas on how to track this down?
The text was updated successfully, but these errors were encountered: