You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2008/02/14 21:34:40 UTC
Re: svn commit: r627699 - /httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
On 02/14/2008 11:24 AM, dirkx@apache.org wrote:
> Author: dirkx
> Date: Thu Feb 14 02:24:04 2008
> New Revision: 627699
>
> URL: http://svn.apache.org/viewvc?rev=627699&view=rev
> Log:
> Kasper Brand came across a flaw in the current implementation when CRL
> information - i.e. SSLCARevocationFile/SSLCARevocationPath - is set
> on a per-vhost basis (don't know how much sense it makes to have
> non-global CRLs, but anyway...).
>
> The attached patch (47B2B1A7.1060009@velox.ch on httpd-dev) addresses
> this issue, and it also improves the logging behavior for an SNI
> enabled configuration (previously some of the messages would
> always go to the first vhost, or wouldn't appear at
> all, depending on the LogLevel of the first vhost).
>
> reviewed: dirkx
>
>
> Modified:
> httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=627699&r1=627698&r2=627699&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Thu Feb 14 02:24:04 2008
> @@ -2022,6 +2022,26 @@
> SSL_CTX_get_verify_callback(ssl->ctx));
> }
>
> + /*
> + * We also need to make sure that the correct mctx is
> + * assigned to the connection - the CRL callback e.g.
> + * makes use of it for retrieving its store (mctx->crl).
> + */
> + c->base_server = s;
Is this correct? This changes the behaviour for SNI in comparison
to name based virtual hosts in the non-SSL case as in their case the base_server
is always the first (or is it the last, I cannot memorize this :-() configured
server on this IP/port pair. IMHO SNI in SSL should be handled the same way as usual
name based virtual hosts in the HTTP case. This may mean that we need to add another
server_rec field to the conn_rec struct that contains s and that mod_ssl needs to
work with this field instead of base_server. But to be honest I haven't analysed
this further.
Regards
Rüdiger
Re: svn commit: r627699 - /httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 02/14/2008 09:46 PM, Dirk-Willem van Gulik wrote:
>
> On Feb 14, 2008, at 9:34 PM, Ruediger Pluem wrote:
>
>> server on this IP/port pair. IMHO SNI in SSL should be handled the
>> same way as usual
>> name based virtual hosts in the HTTP case. This may mean that we need
>> to add another
>> server_rec field to the conn_rec struct that contains s and that
>> mod_ssl needs to
>> work with this field instead of base_server. But to be honest I
>> haven't analysed
>> this further.
>
> My test suggest that it does the right thing - but I understand your
> concern -- and have not tried your senario in a wider case. Though my
> guess this still behaves correct ? Unfortunately I won't be able to dive
> into this in the next few days. Feel free back this change out if you
> think it break things - or hack on it :) It is not super critical.
Agreed. From a first checking I see the following difference in behaviour
between SNI / HTTP name based virtual hosts (NBVH):
ap_log_cerror:
SNI: Logs to error_log of vhost with correct SNI name as soon as
we adjusted base_server.
NBVH: Logs to error log of the first vhost
Timeout:
SNI: Timeout for request reading after a keepalive is set to
the value of the vhost that handled the request before.
NBVH: Timeout for request reading after a keepalive is set to
the value of the first vhost.
mod_dbd (ap_dbd_cacquire):
SNI: Takes its config from vhost with correct SNI name as soon as
we adjusted base_server.
NBVH: Takes its config from the first vhost.
Regards
Rüdiger
Re: svn commit: r627699 - /httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
Posted by Dirk-Willem van Gulik <di...@webweaving.org>.
On Feb 14, 2008, at 9:34 PM, Ruediger Pluem wrote:
> On 02/14/2008 11:24 AM, dirkx@apache.org wrote:
>> Author: dirkx
>> Date: Thu Feb 14 02:24:04 2008
>> New Revision: 627699
>> URL: http://svn.apache.org/viewvc?rev=627699&view=rev
>> Log:
>> Kasper Brand came across a flaw in the current implementation when
>> CRL information - i.e. SSLCARevocationFile/SSLCARevocationPath -
>> is set on a per-vhost basis (don't know how much sense it makes to
>> have non-global CRLs, but anyway...).
>> The attached patch (47B2B1A7.1060009@velox.ch on httpd-dev)
>> addresses this issue, and it also improves the logging behavior for
>> an SNI enabled configuration (previously some of the messages would
>> always go to the first vhost, or wouldn't appear at
>> all, depending on the LogLevel of the first vhost).
>> reviewed: dirkx
>> Modified:
>> httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=627699&r1=627698&r2=627699&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Thu Feb 14
>> 02:24:04 2008
>> @@ -2022,6 +2022,26 @@
>> SSL_CTX_get_verify_callback(ssl->ctx));
>> }
>> + /*
>> + * We also need to make sure that the correct mctx is
>> + * assigned to the connection - the CRL callback e.g.
>> + * makes use of it for retrieving its store (mctx->crl).
>> + */
>> + c->base_server = s;
>
> Is this correct? This changes the behaviour for SNI in comparison
> to name based virtual hosts in the non-SSL case as in their case the
> base_server
> is always the first (or is it the last, I cannot memorize this :-()
> configured
first.
> server on this IP/port pair. IMHO SNI in SSL should be handled the
> same way as usual
> name based virtual hosts in the HTTP case. This may mean that we
> need to add another
> server_rec field to the conn_rec struct that contains s and that
> mod_ssl needs to
> work with this field instead of base_server. But to be honest I
> haven't analysed
> this further.
My test suggest that it does the right thing - but I understand your
concern -- and have not tried your senario in a wider case. Though my
guess this still behaves correct ? Unfortunately I won't be able to
dive into this in the next few days. Feel free back this change out if
you think it break things - or hack on it :) It is not super critical.
Dw