Yann Ylavic wrote: > On Mon, Dec 23, 2013 at 9:49 PM, Ruediger Pluem <rpl...@apache.org> 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/%3c48592955.2090...@velox.ch%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