httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: [Patch] PR37791
Date Tue, 06 Dec 2005 08:08:17 GMT
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;
>      }
>      /*



Mime
View raw message