plack / Plack

PSGI toolkit and server adapters
http://plackperl.org/
Other
486 stars 214 forks source link

Hiding $ENV{MOD_PERL} causes segfaults with Net::SSLeay #470

Open alexmv opened 10 years ago

alexmv commented 10 years ago

Plack::Handler::Apache2 hides $ENV{MOD_PERL} before preloading applications. This is in an attempt to:

        # Trick Catalyst, CGI.pm, CGI::Cookie and others that check
        # for $ENV{MOD_PERL}.
        #
        # Note that we delete it instead of just localizing
        # $ENV{MOD_PERL} because some users may check if the key
        # exists, and we do it this way because "delete local" is new
        # in 5.12:
        # http://perldoc.perl.org/5.12.0/perldelta.html#delete-local

However, this interacts poorly with modules which need to be aware of if they are loaded in a mod_perl environment. Take, for instance, Net::SSLeay, which does:

    /* If we running under ModPerl, we dont need our own thread locking because
     * perl threads are not supported under mod-perl, and we can fall back to the thread
     * locking built in to mod-ssl      
     */
     if (!hv_fetch(get_hv("ENV", 1), "MOD_PERL", 8, 0))
        openssl_threads_init();

This means that the following minimal PSGI app, if preloaded, segfaults Apache during startup:

require Net::SSLeay;

return sub { return [200, ['Content-Type' => 'text/plain'], ["OK!"]]};

...if run on an SSL-enabled Apache with the suggested configuration:

<VirtualHost _default_:443>
    SSLEngine on
    SSLCertificateFile    /etc/apache2/certs/apache.pem
    SSLCertificateKeyFile /etc/apache2/certs/apache.key

    <Location />
        Order allow,deny
        Allow from all

        SetHandler perl-script
        PerlResponseHandler Plack::Handler::Apache2
        PerlSetVar psgi_app /opt/rt4/sbin/rt-server
    </Location>
    <Perl>
        use Plack::Handler::Apache2;
        Plack::Handler::Apache2->preload("/path/to/code/above.pl");
    </Perl>
</VirtualHost>

The only recourse apparently available to PSGI apps is to kludgily attempt to determine if they're running under mod_perl via:

$ENV{MOD_PERL} = 1 if $INC{'Plack/Handler/Apache2.pm'};

...before loading any code which might load Net::SSLeay.

Is it known which Plack-compatible applications still inspect $ENV{MOD_PERL} in the context of PSGI preloading? Does Catalyst still care?

miyagawa commented 10 years ago

This is one of the "can't make everyone happy" problem. I'm sure we'll see loads of issues when we disable this check because MOD_PERL env can be used for variety of things like we see here.

I guess the workaround here is to wrap the loading of Net::SSLeay into a block that adds MOD_PERL so that initialization phase will see it but not elsewhere.

alexmv commented 10 years ago

When you say "wrap the loading of Net::SSLeay," the only way that I'm coming up with to do that is pushing a coderef onto @INC, or something equally horrid. Was there something less twitch-worthy you had in mind?

I feel this at least warrants a warning in the documentation, because Net::SSLeay is really not that odd a module to require. Would you take a doc patch?

miyagawa commented 10 years ago

Oh I was thinking of little more explicit, like

BEGIN { local $ENV{MOD_PERL} = 1; require Net::SSLeay; }

and that obviously should happen before loading any modules that attempts to load Net::SSLeay underneath.

miyagawa commented 10 years ago

I wonder - could there be any better way for Net::SSLeay to detect that it is running under mod_perl, besides MOD_PERL env variable? Just curious.

alexmv commented 10 years ago

The problem is that places the burden on all modules which might require Net::SSLeay. This is biting us in the instance of RT, where an RT plugin might use TLS to talk to a remote LDAP server, or similar. Either we can add that code to every RT plugin which uses Net::SSLeay, or we can do that globally before loading all RT plugins -- and hope that all of the modules that all possible plugins might load will do good things with detecting $ENV{MOD_PERL} and not evil things. The former means having to repeat oneself in a slew of modules, the latter is possibly over-broad.

alexmv commented 10 years ago

I'm not aware of any cleaner way to detect the presence of mod_perl from C, offhand.

miyagawa commented 10 years ago

I get that pain, and would love to kill this kludge but there probably needs a configuration flag, and maybe documentation first, to switch this behavior?

alexmv commented 10 years ago

Yeah, I agree that any change here needs to be made carefully, soas to not break code which tries to seize the reins when mod_perl is involved. Which is why I was pondering earlier about what we know about the current failure modes (from Catalyst, CGI, etc) if it's not deleted from %ENV.