On Fri, Apr 13, 2018 at 01:55:40PM +0200, Yann Ylavic wrote: > At runtime, *with SNI*, when ssl_callback_ServerNameIndication() gets > called and ap_vhost_iterate_given_conn()::ssl_find_vhost() reaches the > second nvh (beta, with no SSL) corresponding to the SNI, we do > SSL_set_SSL_CTX to switch the SSL's SSL_CTX from > SSLSrvConfig(alpha)->ssl_ctx (pre-set at ssl_hook_pre_connection() > time) to SSLSrvConfig(beta)->ssl_ctx -- sorry for the overlong > sentence. > However here SSLSrvConfig(beta)->ssl_ctx is NULL (No SSL* directive > configured), so SSL_set_SSL_CTX(ssl, NULL) ends up preserving alpha's > and thus we continue with the context of alpha to handle the > connection. > > This doesn't look right to me, maybe it is for compatibility on 2.4.x, > but I feel that we should bail out there since beta has really nothing > to do with accepting SSL connections if not explicitely configured to > (is the port in VirtualHost a good criteria? no own SSLCertificate > really? browsers will complain any with no wildcard magic).
Thanks for reading, and I hadn't followed through the SNI paths so thanks for explaining that too. It seems quite lucky this happens to work then, so I'd be fine with using the patch on 2.4 only, for backwards compat, and retaining the startup error for this config in trunk? It annoys me now that the "protocol" argument to "Listen" is really quite ill-defined. "https" is not even really a protocol, more a URL scheme, and I can't see it documented that enables SSLEngine anywhere. We also have Protocols which does something completely different! Regards, Joe
