Yann Ylavic wrote:
> On Mon, Dec 23, 2013 at 9:49 PM, Ruediger Pluem <[email protected]> wrote:
>>
>>
>> Kaspar Brand wrote:
>>> On 21.12.2013 14:21, Ruediger Pluem wrote:
>>>>> I guess a more general fix for this would be:
>>>>
>>>> No further comments / feedback? If not then I would commit the patch.
>>>
>>> The change looks fine to me (for easier comparison/review,
>>> a whitespace-change-ignoring version is attached).
>>>
>>> What would probably make sense is to amend the following comment
>>> on this occasion:
>>>
>>> /*
>>> * The SNI extension supplied a hostname. So don't accept requests
>>> * with either no hostname or a different hostname.
>>> */
>>>
>>> It doesn't say anything about the rationale right now, and as
>>> recent discussions have shown, it would be helpful to explain
>>> why this is done.
>>
>> Done. I have tried to improve the comment and added a TODO if my
>> reasoning for the checks is really true.
>
> Sorry to be late, I'm overwhelmed these days...
>
> + /*
> + * The SNI extension supplied a hostname. So don't accept
> requests
> + * with either no hostname or a different hostname as this could
> + * cause us to end up in a different virtual host as the one that
> + * was used for the handshake causing different SSL parameters to
> + * be applied.
> + * XXX: TODO check if this is really true and that there are
> + * SSL parameters that are not fixed by a renegotiation in
> + * ssl_hook_Access.
> + */
>
> According to
> http://mail-archives.apache.org/mod_mbox/httpd-dev/200806.mbox/%[email protected]%3E,
> the (great) analyse Kaspar made in 2008, the only parameters which
> won't be renegotiated are SSLCACertificateFile/Path and
> SSLCADNRequestFile/Path.
Thanks for the digging, but doesn't this mean that I can have a client cert
verification to the SNI host first
with a cert that is not accepted in the host of the host header and hence
circumvent a proper client cert
checking during renegotiation?
Hence it would be unsafe to allow switching the virtual host (at least with
client certs) from the SNI chosen one
to a host header chosen one.
Hence the checks are a safe thing to do at least in the client cert
verification case.
Furthermore SSLProtocol is mentioned as "not switchable" during a
renegotiation. Hence it would be possible to
circumvent stricter SSLProtocol requirements in one host by more open once in
another host.
> This is because of the lacking OpenSSL's SSL_set_cert_store()
> function, which always seem to be the case with the latest versions
> (AFAICT).
> There may also be issues when SSLVerifyClient is "optional*", but
> optional issues IMHO.
>
> + if (!r->hostname) {
> + ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
> APLOGNO(02031)
> + "Hostname %s provided via SNI, but no hostname"
> + " provided in HTTP request", servername);
> + return HTTP_BAD_REQUEST;
> + }
>
> One thing this code is possibly missing is HTTP < 1.1 requests (as
> William noted in his proposed patch), where no Host is provided,
> couldn't SNI be used by the client in this case?
Maybe, but how many HTTP 0.9 clients / HTTP 1.0 clients that do not sent a host
header and do SNI do you know?
Regards
Rüdiger
