On Mon, Dec 05, 2005 at 10:03:43PM +0100, Ruediger Pluem wrote:
> I just tried to fix PR37791. Although trunk is CTR I do not want to
> commit something completely stupid :-). So please a quick remote eye
> by some SSL guy. To me it does not seem to make sense to continue
> ssl_hook_Access if ssl is still NULL at this stage of the code.
This is slightly tricky. (not only because I have to use pen, paper,
and de Morgan to make sense of that conditional ;)
The access control checks here are actually more important for the
optional-SSL-not-upgraded case rather than the HTTP-on-HTTPS-port error
case. Your change makes the test equivalent to:
if (sc->enabled == SSL_ENABLED_FALSE || !ssl)
and the effect is that SSLRequire checks will not be applied even for a
vhost with "SSLEngine optional", which I think is wrong. It should be
the case that:
SSLRequire %{HTTPS} eq "on"
is always exactly equivalent to "SSLRequireSSL". So I think the right
fix is going to be modify each of the big if() statements in that
function to check that ssl != NULL too.
> Index: modules/ssl/ssl_engine_kernel.c
> ===================================================================
> --- modules/ssl/ssl_engine_kernel.c (Revision 354171)
> +++ modules/ssl/ssl_engine_kernel.c (Arbeitskopie)
> @@ -204,7 +204,9 @@
> /*
> * Check to see if SSL protocol is on
> */
> - if (!((sc->enabled == SSL_ENABLED_TRUE) || (sc->enabled ==
> SSL_ENABLED_OPTIONAL) || ssl)) {
> + if (!(((sc->enabled == SSL_ENABLED_TRUE)
> + || (sc->enabled == SSL_ENABLED_OPTIONAL))
> + && ssl)) {
> return DECLINED;
> }
> /*
