Terrific analysis! But on the meta-question...

Instead of changing the behavior of httpd on each and every subversion
bump, is it time to revisit our revisioning discipline and hygiene?

I promise to stay out of such discussion provided that one equally stubborn
and intractable PMC member agrees to do the same, and let the balance of
the PMC make our decision, moving forwards.

On Fri, Apr 13, 2018, 06:11 Joe Orton <jor...@redhat.com> wrote:

> On Thu, Apr 12, 2018 at 09:38:46PM +0200, Ruediger Pluem wrote:
> > On 04/12/2018 09:28 AM, Joe Orton wrote:
> > > But logged is:
> > >
> > > ::1 - - [12/Apr/2018:08:11:12 +0100] "GET /agag HTTP/1.1" 404 12
> HTTPS=on SNI=localhost.localdomain
> > > 127.0.0.1 - - [12/Apr/2018:08:11:15 +0100] "GET /agag HTTP/1.1" 404 12
> HTTPS=- SNI=-
> > >
> > > Now mod_ssl only sees the "off" SSLSrvConfigRec in the second vhost so
> > > the logging is wrong.
> >
> > What does the same test result in with 2.4.29?
>
> Excellent question, I should have checked that.  Long e-mail follows,
> sorry.
>
> In fact it is the same with 2.4.29, because the SSLSrvConfigRec
> associated with the vhost's server_rec is the same as the default/base
> (non-SSL) server_rec, aka base_server passed to post_config hooks aka
> the ap_server_conf global.
>
> So, maybe I understand this a bit better now.
>
> Config with three vhosts / server_rec structs:
> a) base server config :80 non-SSL (<-- ap_server_conf/base_server)
> b) alpha vhost :443, explicit SSLEngine on, SSLCertificateFile etc
> c) beta vhost :443, no SSL*
>
> For 2.4.29 mod_ssl config derived is:
> a) SSLSrvConfigRec for base_server = { whatever config at global scope }
> b) SSLSrvConfigRec for alpha = { sc->enabled = TRUE, ... }
> c) SSLSrvConfigRec pointer for beta == SSLSrvConfigRec for base_server
>    in the lookup vector (pointer is copied prior to ALWAYS_MERGE flag)
>
> For 2.4.33 it is:
> a) and b) exactly as before
> c) separate SSLSrvConfigRec for beta = { merged copy of config at global }
>    time because of the ALWAYS_MERGE flag, i.e. still sc->enabled = UNSET
>
> When running ssl_init_Module(post_config hook), with 2.4.29:
> - SSLSrvConfig(base_server)->enabled = FALSE because UNSET previously
> - SSLSrvConfig(base_server)->vhost_id gets overwritten with vhost_id
>   for beta vhost because it's later in the loop and there's no check
>
> And with 2.4.33:
> - SSLSrvConfig(beta)->enabled is UNSET but gets flipped to ENABLED,
>   then startup fails (the issue in question)
>
> w/my patch for 2.4.33:
> - SSLSrvConfig(beta)->enabled is FALSE and startup works
>
> At run-time a request via SSL which matches the beta vhost via SNI/Host:
>
> For 2.4.29:
> - r->server is the beta vhost and mySrvConfig(r->server) still gives
>   you the ***base_server*** SSLSrvConfigRec i.e. sc->enabled=FALSE
> - thus e.g. ssl_hook_Fixup() does nada
>
> For 2.4.33 plus my patch:
> - r->server is the beta vhost and mySrvConfig(r->server) gives
>   you the SSLSrvConfigRec which is also sc->enabled = FALSE
> - thus e.g. ssl_hook_Fixup() also does nada
>
> I was trying to convince myself whether mySrvConfig(r->server) is going
> to change between 2.4.29 and .33+patch in this case, and I think it
> should be identical, because it is *only* the handling of ->enabled
> which has changed with _ALWAYS_MERGE.
>
> TL;DR:
> 1. my head hurts
> 2. I think my patch is OK
>
> Anyone read this far?
>

Reply via email to