Regression: Cannot use cyrptography in embedded wsgi

[Naddiseo]

We have discovered at $work that the 1.0 cryptography release does not work with apache2+mod_wsgi. The error log spits out "Truncated or oversized response headers received from daemon process", which seems to happen if the wsgi process dies. We've tracked the issue down to an upgrade with cryptography, and something to do with threading and/or the Python GIL (a work around is in the wsgi config to set WSGIApplicationGroup to %{GLOBAL}).

I did a git bisect from 0.9.3 and 1.0, and found that the issue was introduced in commit b3d37a5.

[reaperhulk]

Wow, would not have expected it to come from that...

/cc @glyph

[Naddiseo]

Me either, the commit itself doesn't look like it could cause something like this, I think that it may have been an earlier commit that introduced code paths that weren't called until the triggering commit.

[reaperhulk]

Out of curiosity... Calling backend.activate_builtin_random() should disable this code path (you'll need to have a backend available in that scope of course. from cryptography.hazmat.backends.openssl.backend import backend will do it). Does this resolve your issue?

[Naddiseo]

Yes, those two lines also resolve the issue.

[glyph]

OK, so here's a hypothesis:

This is a problem because sub-interpreters have an entirely different Python namespace (different modules, etc) but the same C namespace (same GIL, same C-level global variables, shared libraries, etc). I'm not sure exactly why initializing the engine a second time causes a problem, but it's C-level runtime initialization stuff, so ¯_(ツ)_/¯. I'm sure that cryptography being surprised to discover an already-initialized OpenSSL causes all kinds of exciting issues.

@reaperhulk - you should like this one since it would also explain some of the other concurrency issues you're seeing at import time.

(The main hypothesis I guess I'm describing here is that subinterpreters are just a completely broken concept nobody should use, but maybe cryptography could be made to work around these issues.)

[reaperhulk]

If we work on that assumption we can make engine addition idempotent at the C layer (see #2293) but since the random engine has callbacks in Python (that potentially point to functions not in the "current" sub-interpreter) it would still fail in the sub-interpreter...correct?

[glyph]

There's a pretty good explanation here - http://emptysqua.re/blog/python-c-extensions-and-mod-wsgi/#but-c-extensions-are-not-isolated - of how C extensions are treated by mod_wsgi.

[glyph]

I am pretty sure that you can call between different sub-interpreters just fine, since they share a GIL.

[glyph]

I flied a cffi bug to ask for a way to keep the GIL while calling C: https://bitbucket.org/cffi/cffi/issues/220/some-way-to-annotate-a-call-to-ask-it-not

[glyph]

One perhaps-obvious solution here would be to move all this library initialization to the module scope so that it's protected by the import lock, rather than trying to protect it with an ad-hoc lock. It seems counter to the way that cryptography is constructed, to avoid shared mutable state, but in this case there is just the illusion of such avoidance: the mutable state does in fact exist and attempting to abstract it away is hard in a multi-interpreter environment.

[public]

Sub interpreters could definitely trigger a race in the engine setup code. #2293 proposes making this process idempotent which seems like a fairly reasonable solution to me.

[reaperhulk]

@glyph I'm not opposed to that solution if it resolves our problem, but does it given the change in behavior in python 3.4?

[glyph]

Possibly not; I can't figure out at a glance how the per-module locks are initialized. @cyli suggests that cryptography could just take out a filesystem lock in /tmp ;-)

[reaperhulk]

Okay, reviving this conversation. Proposal for the sake of discussion: To avoid issues with sub-interpreters we should do the following...

• Move os random engine back to C
• Make os random engine registration idempotent (we can hold a lock at the C layer which will prevent races with the sub-interpreters)
• Move the locking callbacks to C (since they are presumably subject to the same issues)
• Make locking callback registration idempotent (this part might be tricky if we want to detect locking callbacks already being registered by things like import _ssl)

Do people agree that this would resolve the issue? If not, why not? And if so, is there a way to do this without doing it all in C because obviously we'd prefer to not do that...

[alex]

FWIW, I'm strictly opposed to moving this code back itno C.

On Tue, Sep 8, 2015 at 11:05 AM, Paul Kehrer notifications@github.com
wrote:

Okay, reviving this conversation. Proposal for the sake of discussion: To
avoid issues with sub-interpreters we should do the following:

• Move os random engine back to C
• Make os random engine registration idempotent (we can hold a lock at
  the C layer which will prevent races with the sub-interpreters)
• Move the locking callbacks to C (since they are presumably subject
  to the same issues)
• Make locking callback registration idempotent (this part might be
  tricky if we want to detect locking callbacks already being registered by
  things like import _ssl)

Do people agree that this would resolve the issue? If not, why not? And if
so, is there a way to do this without doing it all in C because obviously
we'd prefer to not do that...

—
Reply to this email directly or view it on GitHub
#2299 (comment).

"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: 125F 5C67 DFE9 4084