On Fri, Apr 13, 2018 at 1:11 PM, Joe Orton <jor...@redhat.com> wrote: > > TL;DR: > 1. my head hurts > 2. I think my patch is OK > > Anyone read this far?
Yep, I was about to write almost the same long one too :) One thing to add maybe. 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). Regards, Yann.
