-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
uwsgi backtrace with cryptography-2.0 #3804
Comments
Are there steps we could follow to be able to reproduce this ourselves? |
Also, are you installing from the wheels we provide on PyPI, or building cryptography yourself? |
One more follow up, assuming you're using our wheels (it looks like you are), can you try installing with |
Hi @alex I haven't found a simple reproducer yet. The one that triggers the problem reliably takes quite a bit of time but here it is nevertheless
There is a venv installed in I will see if I can create a simpler reproducer next week Regarding the wheels question, I will have to bring up that environment back so I will let you know early next week as well. |
-no-binary solves the issue. the conflict appears to be happening when two modules are loading two different versions of libssl.so into the same process context (the manylinux1 wheel from cryptography is 1.1 based) and libssl.so from python-2.7 (openssl 1.0 based in the case of openSUSE Leap 42.2/3). I believe actually this should be reproducible on other distributions as well (though I haven't tried), it just isn't exposed in infra because for all other distributions a wheel mirror is being provided that has a dst-local-rebuilt version of the wheels, avoiding the incompatibility. |
Interesting, the manylinux1 copy should be loading RTLD_LOCAL and not conflicting with other libssl versions, but maybe there's an issue with that... cc @njsmith since he has forgotten more about linkers than I have ever known.
… On Jul 24, 2017, at 11:14 AM, Dirk Mueller ***@***.***> wrote:
-no-binary solves the issue. the conflict appears to be happening when two modules are loading two different versions of libssl.so into the same process context (the manylinux1 wheel from cryptography is 1.1 based) and libssl.so from python-2.7 (openssl 1.0 based in the case of openSUSE Leap 42.2/3).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
I can also confirm that using |
Yeah, the wheels have countermeasures that are supposed to prevent this from happening, but clearly they aren't working somehow – we should not see Can you try reproducing with the env vars |
Oh, maybe I misread and it's not load dependent, just slow to reproduce because you have to install opensuse? |
yeah the currently reproducer (at least for me, maybe @dirkmueller has a simpler one) requires to use the vagrantfile to bring up a bunch of LXC containers with openstack services and wait for it to fail. I am not sure if opensuse is a hard requirements or maybe other distros with openssl-1.0 may suffer from the same issue |
Oh here's a thought – is the main uwsgi binary linked to libcrypto? What does |
It's hard to see what is actually going on. The shared libraries in wheel lack debug information. The small offset in the function call
|
@njsmith yes it s
uwsgi is installed from pip
|
Also noticed something odd when cryptography 2.0 came out, our Debian packaging of a Python project failed to build (on Ubuntu Trusty), might be related. I tried for hours to get it going but eventually reverted to cryptography 1.9 |
Ugh, I was right. I hate being right. So here's what's going on: because the top level uwsgi executable is linked to libcrypto, it's effectively doing an This is really bad. Basically uwsgi can't work with any manylinux wheel that uses openssl (or libxml2 or liblzma for that matter). I can see two possible solutions. Option 1: right now, auditwheel tries to avoid this kind of problem by patching vendored libraries to have unique names. This works great until someone clobbers is by jamming stuff into the global symbol namespace like this. We could avoid this if auditwheel also patched libraries to give every vendored symbol a unique name. Upside: this would fix this class of problems for everyone, forever. Downside: auditwheel leans on patchelf to do the heavy lifting here, and patchelf does not have the ability to rename individual symbols. In fact, AFAICT there does not currently exist any code out there that can rename symbols in ELF files. I've actually implemented this for Mach-O (don't ask) and in principle it's doable for ELF, but ELF is a much more complicated format than Mach-O; for example, ELF symbols are stored in a hash table that would need to be rebuilt. It's possible patchelf already solves the hardest parts here – I'm genuinely not sure – but it's definitely not an off-the-shelf solution. Option 2: convince uwsgi to linking stuff at the top level like this. I have no idea how amenable the authors would be to fixing this. I guess you could put the actual code in a libuwsgi.so and then have the main uwsgi binary just do |
So there needs to be an issue filed with uwsgi to discuss whether they'd be interested in changing their shared library loading behavior to accommodate how manylinux1 needs things to work. Should we also file an issue on the manylinux repo to discuss symbol name mangling (and see if anybody wants to heroically level up patchelf's capabilities)? It appears the only action cryptography can take at this time is to add a FAQ entry discussing the problems with uwsgi and manylinux1 unfortunately. |
@robvdl: hard to know if that's related from your description – I'd suggest filing a separate bug with more details? @reaperhulk: yeah, that all sounds right, though probably the auditwheel repo would be a better place than manylinux. |
@njsmith I went ahead and filed the auditwheel issue -- do you want me to try to summarize your findings to file with uwsgi or do it yourself? |
I'm not a linker wizard, but is it really correct that the parent binary having already loaded a library with a given name "supersedes" the local names in the manylinux-produced Would this be solved by statically linking OpenSSL into our wheel, instead of |
cryptography may bundle openssl in the wheel and that causes symbol conflicts if a different openssl is provided by the distribution. As such, it's probably safer to re-build cryptography ourselves just to be sure that the correct distro libraries are used. We may want to revert that once we start building wheel packages for openSUSE and distribute them in the OpenStack mirrors since every distribution well then get a proper wheel file for its needs. See related review https://review.openstack.org/#/c/486305/ Closes-Bug: 1705521 Link: pyca/cryptography#3804 Change-Id: I7e88935acda580d8522a1e6927ea498431d78bda
It seems like statically linking might resolve it? That would be relatively easy to confirm as well assuming we can construct an environment that replicates this. I'll take a look when I'm back home in the next few days. |
The canonical reference here is Drepper's DSO howto, but basically the ELF symbol resolution rules are:
Yes, this is bass-ackwards from everything else in computing. Yes, this is really confusing because things that seem like they shouldn't matter (linking libcrypto vs dlopening libcrypto, linking libcrypto from the binary versus linking from a dlopened library) actually matter a lot. AFAICT the underlying principle here is that the ELF designers at each point asked "which of these options gives us more opportunities for monkey-patching shared libraries?" and then that's what they did. I guess this is what happens when your infrastructure is maintained by long-term support companies.
Yes, this would work, though obviously this would require manual hacks to your build system and would only fix this one case, rather than being a general solution. |
Ok, at least I understood correctly. I am pro-us-using-static-linking since that will resolve this consistently. |
uwsgi bug: unbit/uwsgi#1590 |
#3811 solves this for our next release. |
Project: openstack/requirements 6ae571e013b6a802bd66c66bbcc7f4370a60b718 Blacklist cryptography 2.0 See pyca/cryptography#3804 -- cryptography introduced an manylinux1 wheel that is incompatible with uwsgi. Related-Bug: 1705521 Link: pyca/cryptography#3804 Change-Id: I9e0458742730743d1ba79349e9996beeed0be24f
See pyca/cryptography#3804 -- cryptography introduced an manylinux1 wheel that is incompatible with uwsgi. Related-Bug: 1705521 Link: pyca/cryptography#3804 Change-Id: I9e0458742730743d1ba79349e9996beeed0be24f
Project: openstack/requirements 6ae571e013b6a802bd66c66bbcc7f4370a60b718 Blacklist cryptography 2.0 See pyca/cryptography#3804 -- cryptography introduced an manylinux1 wheel that is incompatible with uwsgi. Related-Bug: 1705521 Link: pyca/cryptography#3804 Change-Id: I9e0458742730743d1ba79349e9996beeed0be24f
We've released 2.0.1, could you check to see if this segfault occurs with uwsgi now? |
Yep, will do tomorrow |
From #3824 it sounds like 2.0.1 did not solve this, and in fact now we're confused all over again about why this is broken (#3824 (comment)). And in any case it sounds like a 2.0.2 is imminent. |
cryptography may bundle openssl in the wheel and that causes symbol conflicts if a different openssl is provided by the distribution. As such, it's probably safer to re-build cryptography ourselves just to be sure that the correct distro libraries are used. This has been addressed in openstack-ansible-tests/test-vars.yaml (https://review.openstack.org/#/c/486580/) to fix the CI tests but the problem is also present on regular deployments so we set it in the group_variables for the repo_all group of hosts so it's built from source in the wheel repository. Related-Bug: 1705521 Link: pyca/cryptography#3804 Change-Id: I54ba3c1fa48a2f4c633930bc7e8cc65397f86659
Hi,
We are seeing the following failure in the OpenStack CI ever since cryptography-2.0 was released and installed by pip. Reverting back to cryptography-1.9 fixes the issue.
The downstream bug report is at https://bugs.launchpad.net/openstack-ansible/+bug/1705521
Let me know if you need me to provide more information
The text was updated successfully, but these errors were encountered: