You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2019/10/20 10:50:33 UTC

svn commit: r1868645 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_init.c ssl_engine_kernel.c ssl_private.h

Author: ylavic
Date: Sun Oct 20 10:50:33 2019
New Revision: 1868645

URL: http://svn.apache.org/viewvc?rev=1868645&view=rev
Log:
mod_ssl: negotiate the TLS protocol version per name based vhost configuration.

By using the new ClientHello callback provided by OpenSSL 1.1.1, which runs at
the earliest connection stage, we can switch the SSL_CTX of the SSL connection
early enough for OpenSSL to take into account the protocol configuration of the
vhost.

In other words:
    SSL_set_SSL_CTX(c->SSL, s->SSL_CTX)
followed by:
    SSL_set_{min,max}_proto_version(SSL_CTX_get_{min,max}_proto_version(s->SSL_CTX))
works as expected at this stage (while the same from the SNI callback is
ignored by/due to OpenSSL's state machine).

Extracting the SNI (to select the relevant vhost) in the ClientHello callback
is not as easy as calling SSL_get_servername() though, we have to work with
the raw TLS extensions helpers provided by OpenSSL. I stole this code from a
test in the OpenSSL source code (i.e. client_hello_select_server_ctx() in
test/handshake_helper.c).

We can then call init_vhost() as with the SNI callback (in use only for OpenSSL
versions earlier than 1.1.1 now), and pass it the extracted SNI.

Modified:
    httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
    httpd/httpd/trunk/modules/ssl/ssl_private.h

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c?rev=1868645&r1=1868644&r2=1868645&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Sun Oct 20 10:50:33 2019
@@ -500,8 +500,11 @@ static apr_status_t ssl_init_ctx_tls_ext
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01893)
                  "Configuring TLS extension handling");
 
+#if OPENSSL_VERSION_NUMBER < 0x10101000L || defined(LIBRESSL_VERSION_NUMBER)
     /*
-     * Server name indication (SNI)
+     * The Server Name Indication (SNI) provided by the ClientHello can be
+     * used to select the right (name-based-)vhost and its SSL configuration
+     * before the handshake takes place.
      */
     if (!SSL_CTX_set_tlsext_servername_callback(mctx->ssl_ctx,
                           ssl_callback_ServerNameIndication) ||
@@ -512,6 +515,15 @@ static apr_status_t ssl_init_ctx_tls_ext
         ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s);
         return ssl_die(s);
     }
+#else
+    /*
+     * The ClientHello callback also allows to retrieve the SNI, but since it
+     * runs at the earliest possible connection stage we can even set the TLS
+     * protocol version(s) according to the selected (name-based-)vhost, which
+     * is not possible at the SNI callback stage (due to OpenSSL internals).
+     */
+    SSL_CTX_set_client_hello_cb(mctx->ssl_ctx, ssl_callback_ClientHello, NULL);
+#endif
 
 #ifdef HAVE_OCSP_STAPLING
     /*
@@ -706,7 +718,7 @@ static apr_status_t ssl_init_ctx_protoco
 #else /* #if OPENSSL_VERSION_NUMBER < 0x10100000L */
     /* We first determine the maximum protocol version we should provide */
 #if SSL_HAVE_PROTOCOL_TLSV1_3
-    if (SSL_HAVE_PROTOCOL_TLSV1_3 && (protocol & SSL_PROTOCOL_TLSV1_3)) {
+    if (protocol & SSL_PROTOCOL_TLSV1_3) {
         prot = TLS1_3_VERSION;
     } else
 #endif

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=1868645&r1=1868644&r2=1868645&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Sun Oct 20 10:50:33 2019
@@ -2346,9 +2346,8 @@ static apr_status_t set_challenge_creds(
  * This function sets the virtual host from an extended
  * client hello with a server name indication extension ("SNI", cf. RFC 6066).
  */
-static apr_status_t init_vhost(conn_rec *c, SSL *ssl)
+static apr_status_t init_vhost(conn_rec *c, SSL *ssl, const char *servername)
 {
-    const char *servername;
     X509 *cert;
     EVP_PKEY *key;
     
@@ -2360,7 +2359,9 @@ static apr_status_t init_vhost(conn_rec
             return APR_SUCCESS;
         }
         
-        servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
+        if (!servername) {
+            servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
+        }
         if (servername) {
             if (ap_vhost_iterate_given_conn(c, ssl_find_vhost,
                                             (void *)servername)) {
@@ -2410,6 +2411,7 @@ static apr_status_t init_vhost(conn_rec
     return APR_NOTFOUND;
 }
 
+#if OPENSSL_VERSION_NUMBER < 0x10101000L || defined(LIBRESSL_VERSION_NUMBER)
 /*
  * This callback function is executed when OpenSSL encounters an extended
  * client hello with a server name indication extension ("SNI", cf. RFC 6066).
@@ -2417,10 +2419,69 @@ static apr_status_t init_vhost(conn_rec
 int ssl_callback_ServerNameIndication(SSL *ssl, int *al, modssl_ctx_t *mctx)
 {
     conn_rec *c = (conn_rec *)SSL_get_app_data(ssl);
-    apr_status_t status = init_vhost(c, ssl);
+    apr_status_t status = init_vhost(c, ssl, NULL);
     
     return (status == APR_SUCCESS)? SSL_TLSEXT_ERR_OK : SSL_TLSEXT_ERR_NOACK;
 }
+#else /* OPENSSL_VERSION_NUMBER < 0x10101000L */
+/*
+ * This callback function when the ClientHello is received.
+ */
+int ssl_callback_ClientHello(SSL *ssl, int *al, void *arg)
+{
+    char *servername = NULL;
+    conn_rec *c = (conn_rec *)SSL_get_app_data(ssl);
+    const unsigned char *pos;
+    size_t len, remaining;
+    (void)arg;
+ 
+    /* We can't use SSL_get_servername() at this earliest OpenSSL connection
+     * stage, and there is no SSL_client_hello_get0_servername() provided as
+     * of OpenSSL 1.1.1. So the code below, that extracts the SNI from the
+     * ClientHello's TLS extensions, is taken from some test code in OpenSSL,
+     * i.e. client_hello_select_server_ctx() in "test/handshake_helper.c".
+     */
+
+    /*
+     * The server_name extension was given too much extensibility when it
+     * was written, so parsing the normal case is a bit complex.
+     */
+    if (!SSL_client_hello_get0_ext(ssl, TLSEXT_TYPE_server_name, &pos,
+                                   &remaining)
+            || remaining <= 2) 
+        goto give_up;
+
+    /* Extract the length of the supplied list of names. */
+    len = (*(pos++) << 8);
+    len += *(pos++);
+    if (len + 2 != remaining)
+        goto give_up;
+    remaining = len;
+
+    /*
+     * The list in practice only has a single element, so we only consider
+     * the first one.
+     */
+    if (remaining <= 3 || *pos++ != TLSEXT_NAMETYPE_host_name)
+        goto give_up;
+    remaining--;
+
+    /* Now we can finally pull out the byte array with the actual hostname. */
+    len = (*(pos++) << 8);
+    len += *(pos++);
+    if (len + 2 != remaining)
+        goto give_up;
+
+    /* Use the SNI to switch to the relevant vhost, should it differ from
+     * c->base_server.
+     */
+    servername = apr_pstrmemdup(c->pool, (const char *)pos, len);
+
+give_up:
+    init_vhost(c, ssl, servername);
+    return SSL_CLIENT_HELLO_SUCCESS;
+}
+#endif /* OPENSSL_VERSION_NUMBER < 0x10101000L */
 
 /*
  * Find a (name-based) SSL virtual host where either the ServerName
@@ -2441,12 +2502,19 @@ static int ssl_find_vhost(void *serverna
     if (found && (ssl = sslcon->ssl) &&
         (sc = mySrvConfig(s))) {
         SSL_CTX *ctx = SSL_set_SSL_CTX(ssl, sc->server->ssl_ctx);
+
         /*
          * SSL_set_SSL_CTX() only deals with the server cert,
          * so we need to duplicate a few additional settings
          * from the ctx by hand
          */
         SSL_set_options(ssl, SSL_CTX_get_options(ctx));
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L \
+        && (!defined(LIBRESSL_VERSION_NUMBER) \
+            || LIBRESSL_VERSION_NUMBER >= 0x20800000L)
+        SSL_set_min_proto_version(ssl, SSL_CTX_get_min_proto_version(ctx));
+        SSL_set_max_proto_version(ssl, SSL_CTX_get_max_proto_version(ctx));
+#endif
         if ((SSL_get_verify_mode(ssl) == SSL_VERIFY_NONE) ||
             (SSL_num_renegotiations(ssl) == 0)) {
            /*
@@ -2643,7 +2711,7 @@ int ssl_callback_alpn_select(SSL *ssl,
      * they callback the SNI. We need to make sure that we know which vhost
      * we are dealing with so we respect the correct protocols.
      */
-    init_vhost(c, ssl);
+    init_vhost(c, ssl, NULL);
     
     proposed = ap_select_protocol(c, NULL, sslconn->server, client_protos);
     if (!proposed) {

Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=1868645&r1=1868644&r2=1868645&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_private.h Sun Oct 20 10:50:33 2019
@@ -915,7 +915,11 @@ SSL_SESSION *ssl_callback_GetSessionCach
 void         ssl_callback_DelSessionCacheEntry(SSL_CTX *, SSL_SESSION *);
 void         ssl_callback_Info(const SSL *, int, int);
 #ifdef HAVE_TLSEXT
+#if OPENSSL_VERSION_NUMBER < 0x10101000L || defined(LIBRESSL_VERSION_NUMBER)
 int          ssl_callback_ServerNameIndication(SSL *, int *, modssl_ctx_t *);
+#else
+int          ssl_callback_ClientHello(SSL *, int *, void *);
+#endif
 #endif
 #ifdef HAVE_TLS_SESSION_TICKETS
 int         ssl_callback_SessionTicket(SSL *, unsigned char *, unsigned char *,



Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 5:03 PM Stefan Eissing
<st...@greenbytes.de> wrote:
>
> What one wants is that it inherits as before, in that no beginner understands it sort of way. drat.

Exactly, we need an option for the beginners...
I hope you emptied your bottle enough to find a good name :)

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Stefan Eissing <st...@greenbytes.de>.
What one wants is that it inherits as before, in that no beginner understands it sort of way. drat. 

> Am 25.10.2019 um 16:19 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Fri, Oct 25, 2019 at 2:48 PM Yann Ylavic <yl...@gmail.com> wrote:
>> 
>> Nice idea, I suppose I could make the callback check for
>> ->protocol_set == 0 and not switch in this case.
>> The opt-in may not be that useful then, without it (or "off") the
>> default would be the base server's SSLProtocol, while "on" would be
>> whatever SSLProtocol default is?
> 
> By doing that change I realized that mod_ssl did not merge
> ->protocol_set (r1868934), but now I realize what merging means wrt to
> the above semantics...
> 
> In my previous server1/server2 example, where no SSLProtocol is
> configured in server2, what if SSLProtocol is configured at the server
> config (root) level? Should server2 take the value of its base server
> or the root one?
> The current status is that, without an opt-in/out, it takes the root
> value if configured, or the base server's otherwise. Not very
> intuitive...
> 
> Thoughts?

RE: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by hq...@hquest.pro.br.
As per what should be taken as "default" if no _default_ is given and an old OpenSSL code is in use: we are in 2019, almost 2020. What is the recommended best practice for HTTPS servers? I think SSLv3 and TLSv1.0 are out of the picture, so it is on the dev team to make the judgement call on how secure they want their product out of the box to initialize the SSL subsystem without an explicit _default_ VHost. If your predefined, "hardcoded" settings are not what the admin expects, you leave for them to adjust their standards on each and every of their VHost. But perhaps a few lines of comments on the configuration files can save a lot of lines of code and maintenance down the road.

Just my $.02.

-----Original Message-----
From: hquest@hquest.pro.br <hq...@hquest.pro.br> 
Sent: Friday, October 25, 2019 4:32 PM
To: dev@httpd.apache.org
Subject: RE: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

So the
# global
SSLProtocol TLSv1.3
is an OpenSSL-1.1.1 and onwards feature, where before it was whatever defined on the first <VirtualHost>. Oddly enough, I clearly missed...

Anyway, the fact a standard Apache httpd-ssl.conf configuration file comes with a <VirtualHost _default_:443> key entry should give one a hint this is where the default settings should be applied. If the _default_ VHost is removed/renamed, then there should be no default/inheritance, only if an older OpenSSL is in use; assuming the first <VirtualHost> dictates things for all VHosts is not the right approach to me. So yes, on this scenario, it is unintuitive. I would vote on a clear _default_ VHost for defaults versus reinventing the wheel, but that's only me - and a note on the config stating the SSLProtocol under #global only applies to OpenSSL-1.1.1 and onwards: this can cause confusion/false sense of security if you set things up there and the VHost ends up not making use of these settings.

-----Original Message-----
From: Yann Ylavic <yl...@gmail.com> 
Sent: Friday, October 25, 2019 3:32 PM
To: httpd-dev <de...@httpd.apache.org>
Subject: Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

On Fri, Oct 25, 2019 at 7:42 PM Alex Hautequest <hq...@hquest.pro.br> wrote:
>
> IMHO, it *is* intuitive. If you want no default configuration, do not set one by default, otherwise inheritance applies - just as everything else in this daemon.

Possibly the inheritance we are talking about here is not the one you are thinking of, not the usual one in httpd at least.

In current httpd 2.4, the SSLProtocol which applies for name-based vhosts is the one of the base vhost (the first vhost declared on the IP:port), because until OpenSSL-1.1.1 there was no way to change the protocol of a TLS connection, and httpd needs a TLS connection first to start the handshake, and OpenSSL wants a protocol to create the connection, chicken and egg...
So the SSLProtocol used to create the TLS connection is the one based on the IP:port the connection is accepted on, i.e. the base vhost's.

Now we can and want to be able to configure/honor SSLProtocol per vhost, but the de facto default is the base vhost for SSLProtocol, not the global/root server where directives usually inherit from.

Suppose a configuration like this:

# global
SSLProtocol TLSv1.3

# base vhost
<VirtualHost *:443>
  ServerName name1
  SSLProtocol TLSv1.2
</VirtualHost>

# non-base vhost
<VirtualHost *:443>
  ServerName name2
  # no SSLProtocol
</VirtualHost>

Which SSLProtocol name2 should apply?
For compatibility with 2.4, that's TLSv1.2, your/one's intuition?
If unintuitve, we need some option to address both 2.4 compatibility (with some default there) and intuition/POLS (with another default for next versions).


Regards,
Yann.




RE: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by hq...@hquest.pro.br.
So the
# global
SSLProtocol TLSv1.3
is an OpenSSL-1.1.1 and onwards feature, where before it was whatever defined on the first <VirtualHost>. Oddly enough, I clearly missed...

Anyway, the fact a standard Apache httpd-ssl.conf configuration file comes with a <VirtualHost _default_:443> key entry should give one a hint this is where the default settings should be applied. If the _default_ VHost is removed/renamed, then there should be no default/inheritance, only if an older OpenSSL is in use; assuming the first <VirtualHost> dictates things for all VHosts is not the right approach to me. So yes, on this scenario, it is unintuitive. I would vote on a clear _default_ VHost for defaults versus reinventing the wheel, but that's only me - and a note on the config stating the SSLProtocol under #global only applies to OpenSSL-1.1.1 and onwards: this can cause confusion/false sense of security if you set things up there and the VHost ends up not making use of these settings.

-----Original Message-----
From: Yann Ylavic <yl...@gmail.com> 
Sent: Friday, October 25, 2019 3:32 PM
To: httpd-dev <de...@httpd.apache.org>
Subject: Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

On Fri, Oct 25, 2019 at 7:42 PM Alex Hautequest <hq...@hquest.pro.br> wrote:
>
> IMHO, it *is* intuitive. If you want no default configuration, do not set one by default, otherwise inheritance applies - just as everything else in this daemon.

Possibly the inheritance we are talking about here is not the one you are thinking of, not the usual one in httpd at least.

In current httpd 2.4, the SSLProtocol which applies for name-based vhosts is the one of the base vhost (the first vhost declared on the IP:port), because until OpenSSL-1.1.1 there was no way to change the protocol of a TLS connection, and httpd needs a TLS connection first to start the handshake, and OpenSSL wants a protocol to create the connection, chicken and egg...
So the SSLProtocol used to create the TLS connection is the one based on the IP:port the connection is accepted on, i.e. the base vhost's.

Now we can and want to be able to configure/honor SSLProtocol per vhost, but the de facto default is the base vhost for SSLProtocol, not the global/root server where directives usually inherit from.

Suppose a configuration like this:

# global
SSLProtocol TLSv1.3

# base vhost
<VirtualHost *:443>
  ServerName name1
  SSLProtocol TLSv1.2
</VirtualHost>

# non-base vhost
<VirtualHost *:443>
  ServerName name2
  # no SSLProtocol
</VirtualHost>

Which SSLProtocol name2 should apply?
For compatibility with 2.4, that's TLSv1.2, your/one's intuition?
If unintuitve, we need some option to address both 2.4 compatibility (with some default there) and intuition/POLS (with another default for next versions).


Regards,
Yann.


Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 7:42 PM Alex Hautequest <hq...@hquest.pro.br> wrote:
>
> IMHO, it *is* intuitive. If you want no default configuration, do not set one by default, otherwise inheritance applies - just as everything else in this daemon.

Possibly the inheritance we are talking about here is not the one you
are thinking of, not the usual one in httpd at least.

In current httpd 2.4, the SSLProtocol which applies for name-based
vhosts is the one of the base vhost (the first vhost declared on the
IP:port), because until OpenSSL-1.1.1 there was no way to change the
protocol of a TLS connection, and httpd needs a TLS connection first
to start the handshake, and OpenSSL wants a protocol to create the
connection, chicken and egg...
So the SSLProtocol used to create the TLS connection is the one based
on the IP:port the connection is accepted on, i.e. the base vhost's.

Now we can and want to be able to configure/honor SSLProtocol per
vhost, but the de facto default is the base vhost for SSLProtocol, not
the global/root server where directives usually inherit from.

Suppose a configuration like this:

# global
SSLProtocol TLSv1.3

# base vhost
<VirtualHost *:443>
  ServerName name1
  SSLProtocol TLSv1.2
</VirtualHost>

# non-base vhost
<VirtualHost *:443>
  ServerName name2
  # no SSLProtocol
</VirtualHost>

Which SSLProtocol name2 should apply?
For compatibility with 2.4, that's TLSv1.2, your/one's intuition?
If unintuitve, we need some option to address both 2.4 compatibility
(with some default there) and intuition/POLS (with another default for
next versions).


Regards,
Yann.

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Alex Hautequest <hq...@hquest.pro.br>.
IMHO, it *is* intuitive. If you want no default configuration, do not set one by default, otherwise inheritance applies - just as everything else in this daemon.

Or are you all planning to opt in/out every other settings as well, to make a standard "intuitive-driven” configuration approach?

> On Oct 25, 2019, at 10:18 AM, Yann Ylavic <yl...@gmail.com> wrote:
> 
> The current status is that, without an opt-in/out, it takes the root
> value if configured, or the base server's otherwise. Not very
> intuitive...


Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Nov 5, 2019 at 9:19 AM Rüdiger Plüm <r....@gmx.de> wrote:
>
> On 10/28/2019 10:54 AM, Yann Ylavic wrote:
> >
> > Once/if backported, I plan to completely remove the base vhost from
> > the game, in trunk (usual merging applies).
>
> So you want to revert r1868929 after the backport?

Yes, exactly.

>
> As far as I can tell r1868929 keeps the inheritance behavior closer to the
> previous 2.4.x and trunk behavior, but is different compared to the
> inheritance behavior of already SNI respecting directives like e.g. SSLCipherSuite.
> Removing r1868929 would bring both (the directives respecting SNI so far
> and the ones that NOW respect SNI) to the same inheritance level, correct?

That's it, and consistent with all other RSRC_CONF directives merging.
The difference between 2.4 (with r1868929) and trunk (without) would
be only if no global SSLProtocol is configured.
In this case any vhost with no SSLProtocol either would take the
default value (the hard coded "ALL -SSLv3" currently), instead of
first name based vhost's (which wouldn't be involved anymore for
SSLProtocol).

Sounds good?

Thanks,
Yann.

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Rüdiger Plüm <r....@gmx.de>.

On 10/28/2019 10:54 AM, Yann Ylavic wrote:
> On Mon, Oct 28, 2019 at 9:24 AM Stefan Eissing
> <st...@greenbytes.de> wrote:
>>
>> Ok, let me summarize:
>>
>> - SSLProtocol on base server applies, unless vhost has its own setting
>> - no SSLProtocol on base server, SSLProtocol on vhost applies
>> - no SSLProtocol on base server, no SSLProtocol on vhost, possible SSLProtocol on base vhost applies
>
> That's it, I'd call "base server" the "global server", though, to
> avoid confusion w.r.t. to c->base_server (the "base vhost" in the
> above).
>
> For 2.4.x, this means that there is a behavioural change when:
> - SSLProtocol is specified in a non-base vhost (but this is the point),
> - no SSLProtocol is specified in a non-base vhost AND one is specified
> globally (here the global applies, whereas previously the base vhost's
> applied).
>
> Once/if backported, I plan to completely remove the base vhost from
> the game, in trunk (usual merging applies).

So you want to revert r1868929 after the backport?

As far as I can tell r1868929 keeps the inheritance behavior closer to the
previous 2.4.x and trunk behavior, but is different compared to the
inheritance behavior of already SNI respecting directives like e.g. SSLCipherSuite.
Removing r1868929 would bring both (the directives respecting SNI so far
and the ones that NOW respect SNI) to the same inheritance level, correct?

Regards

Rüdiger


Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Oct 28, 2019 at 9:24 AM Stefan Eissing
<st...@greenbytes.de> wrote:
>
> Ok, let me summarize:
>
> - SSLProtocol on base server applies, unless vhost has its own setting
> - no SSLProtocol on base server, SSLProtocol on vhost applies
> - no SSLProtocol on base server, no SSLProtocol on vhost, possible SSLProtocol on base vhost applies

That's it, I'd call "base server" the "global server", though, to
avoid confusion w.r.t. to c->base_server (the "base vhost" in the
above).

For 2.4.x, this means that there is a behavioural change when:
- SSLProtocol is specified in a non-base vhost (but this is the point),
- no SSLProtocol is specified in a non-base vhost AND one is specified
globally (here the global applies, whereas previously the base vhost's
applied).

Once/if backported, I plan to completely remove the base vhost from
the game, in trunk (usual merging applies).

> Thanks for saving me having to come up with a name, Yann!

My pleasure ;)

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Stefan Eissing <st...@greenbytes.de>.
Ok, let me summarize:

- SSLProtocol on base server applies, unless vhost has its own setting
- no SSLProtocol on base server, SSLProtocol on vhost applies
- no SSLProtocol on base server, no SSLProtocol on vhost, possible SSLProtocol on base vhost applies

To me, this is an improvement over the current, obscure workings.

Thanks for saving me having to come up with a name, Yann!

- Stefan

> Am 27.10.2019 um 13:22 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Fri, Oct 25, 2019 at 4:18 PM Yann Ylavic <yl...@gmail.com> wrote:
>> 
>> The current status is that, without an opt-in/out, it takes the root
>> value if configured, or the base server's otherwise. Not very
>> intuitive...
> 
> Thinking more about this, I think it's not so bad. If no SSLProtocol
> is configured neither globally nor in the non-base NVH then we use the
> SSLProtocol of the base VNH, otherwise we use the one configured
> (either in the VNH or globally). It looks satisfactory to me for 2.4.x
> finally, no opt-in/out.
> 
> For trunk I think we should let the usual merging apply, that is, if
> no SSLProtocol is defined in the VNH nor globally, use the default
> value ("all -SSLv3"), the base vhost is irrelevent in any case.
> 
> WDYT?


Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 4:18 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> The current status is that, without an opt-in/out, it takes the root
> value if configured, or the base server's otherwise. Not very
> intuitive...

Thinking more about this, I think it's not so bad. If no SSLProtocol
is configured neither globally nor in the non-base NVH then we use the
SSLProtocol of the base VNH, otherwise we use the one configured
(either in the VNH or globally). It looks satisfactory to me for 2.4.x
finally, no opt-in/out.

For trunk I think we should let the usual merging apply, that is, if
no SSLProtocol is defined in the VNH nor globally, use the default
value ("all -SSLv3"), the base vhost is irrelevent in any case.

WDYT?

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 2:48 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> Nice idea, I suppose I could make the callback check for
> ->protocol_set == 0 and not switch in this case.
> The opt-in may not be that useful then, without it (or "off") the
> default would be the base server's SSLProtocol, while "on" would be
> whatever SSLProtocol default is?

By doing that change I realized that mod_ssl did not merge
->protocol_set (r1868934), but now I realize what merging means wrt to
the above semantics...

In my previous server1/server2 example, where no SSLProtocol is
configured in server2, what if SSLProtocol is configured at the server
config (root) level? Should server2 take the value of its base server
or the root one?
The current status is that, without an opt-in/out, it takes the root
value if configured, or the base server's otherwise. Not very
intuitive...

Thoughts?

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 3:16 PM Stefan Eissing
<st...@greenbytes.de> wrote:
>
> > Am 25.10.2019 um 14:48 schrieb Yann Ylavic <yl...@gmail.com>:
> >
> > On Fri, Oct 25, 2019 at 2:08 PM Eric Covener <co...@gmail.com> wrote:
> >>
> >> Could the callback behave differently in the omitted case (opt-in)?
> >> That would allow the case where it's explicit to be handled better
> >> OOTB (not even opt-out really)
> >
> > Nice idea, I suppose I could make the callback check for
> > ->protocol_set == 0 and not switch in this case.
> > The opt-in may not be that useful then, without it (or "off") the
> > default would be the base server's SSLProtocol, while "on" would be
> > whatever SSLProtocol default is?
> >
> > If we don't care about the compatibility of an explicit SSLProtocol
> > (e.g any SSLProtocol specified in my server2 example) which was
> > ignored until now but which suddenly isn't, I can go with that change.
>
> +1

r1868929

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Stefan Eissing <st...@greenbytes.de>.

> Am 25.10.2019 um 14:48 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Fri, Oct 25, 2019 at 2:08 PM Eric Covener <co...@gmail.com> wrote:
>> 
>> Could the callback behave differently in the omitted case (opt-in)?
>> That would allow the case where it's explicit to be handled better
>> OOTB (not even opt-out really)
> 
> Nice idea, I suppose I could make the callback check for
> ->protocol_set == 0 and not switch in this case.
> The opt-in may not be that useful then, without it (or "off") the
> default would be the base server's SSLProtocol, while "on" would be
> whatever SSLProtocol default is?
> 
> If we don't care about the compatibility of an explicit SSLProtocol
> (e.g any SSLProtocol specified in my server2 example) which was
> ignored until now but which suddenly isn't, I can go with that change.

+1

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 2:08 PM Eric Covener <co...@gmail.com> wrote:
>
> Could the callback behave differently in the omitted case (opt-in)?
> That would allow the case where it's explicit to be handled better
> OOTB (not even opt-out really)

Nice idea, I suppose I could make the callback check for
->protocol_set == 0 and not switch in this case.
The opt-in may not be that useful then, without it (or "off") the
default would be the base server's SSLProtocol, while "on" would be
whatever SSLProtocol default is?

If we don't care about the compatibility of an explicit SSLProtocol
(e.g any SSLProtocol specified in my server2 example) which was
ignored until now but which suddenly isn't, I can go with that change.

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Eric Covener <co...@gmail.com>.
On Fri, Oct 25, 2019 at 7:59 AM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Fri, Oct 25, 2019 at 1:21 PM Eric Covener <co...@gmail.com> wrote:
> >
> > > I am pretty conservative on these usually but I think opt-out would be OK.
> >
> > I am not even sure opt-out makes sense vs. just moving the directives
> > not expected to be used.
>
> Yes, opt-out is possibly no better than adjusting the configuration.
> A oneliner may help though for complex/splitted configurations.
>
> > I guess some obscure config could reach the same VH over a non-SNI
> > alternate address AND different protocols are desired? Seems pretty
> > unlikely.
>
> I'm not sure I understand what you mean.

I only meant where some actual opt-out would be useful vs. config fix.

>
> Suppose a config like the below (untested, will do):
>
> <VirtualHost *:443>
>   ServerName name1
>   SSLProtocol TLSv1.2
> </VirtualHost>
>
> <VirtualHost *:443>
>   ServerName name2
>   # no SSLProtocol
> </VirtualHost>
>
> I think that currently (2.4.x), name2 is de facto "TLSv1.2" (like its
> base server), but with r1868645 it's now "all -SSLv3" (the default for
> SSLProtocol).
> If an upgrade moves name2 from an A+++ to a B- it may well be the end
> of the world :p
>
> I will test that and confirm (or not).

Could the callback behave differently in the omitted case (opt-in)?
That would allow the case where it's explicit to be handled better
OOTB (not even opt-out really)

-- 
Eric Covener
covener@gmail.com

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 1:21 PM Eric Covener <co...@gmail.com> wrote:
>
> > I am pretty conservative on these usually but I think opt-out would be OK.
>
> I am not even sure opt-out makes sense vs. just moving the directives
> not expected to be used.

Yes, opt-out is possibly no better than adjusting the configuration.
A oneliner may help though for complex/splitted configurations.

> I guess some obscure config could reach the same VH over a non-SNI
> alternate address AND different protocols are desired? Seems pretty
> unlikely.

I'm not sure I understand what you mean.

Suppose a config like the below (untested, will do):

<VirtualHost *:443>
  ServerName name1
  SSLProtocol TLSv1.2
</VirtualHost>

<VirtualHost *:443>
  ServerName name2
  # no SSLProtocol
</VirtualHost>

I think that currently (2.4.x), name2 is de facto "TLSv1.2" (like its
base server), but with r1868645 it's now "all -SSLv3" (the default for
SSLProtocol).
If an upgrade moves name2 from an A+++ to a B- it may well be the end
of the world :p

I will test that and confirm (or not).

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Eric Covener <co...@gmail.com>.
> I am pretty conservative on these usually but I think opt-out would be OK.

I am not even sure opt-out makes sense vs. just moving the directives
not expected to be used.
I guess some obscure config could reach the same VH over a non-SNI
alternate address AND different protocols are desired? Seems pretty
unlikely.

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Eric Covener <co...@gmail.com>.
On Fri, Oct 25, 2019 at 4:09 AM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Fri, Oct 25, 2019 at 9:56 AM Stefan Eissing
> <st...@greenbytes.de> wrote:
> >
> > If I understand this correctly: if someone has some old SSLProtocol/Cipher/etc. setting sitting in a vhost, *ineffective now since it is not the first vhost*, this change would activate it.
>
> Ciphers/etc work per vhost already thanks to the SNI callback, it's
> only SSLProtocol that can't be changed from there due to OpenSSL
> internals (AIUI), but still..
>
> > So it could expose a site to a TLS setting that is not appropriate for it. One could argue that the first mistake was for the admin to leave that setting there, but...
>
> Yeah, my fear as well.

I am pretty conservative on these usually but I think opt-out would be OK.

-- 
Eric Covener
covener@gmail.com

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 9:56 AM Stefan Eissing
<st...@greenbytes.de> wrote:
>
> If I understand this correctly: if someone has some old SSLProtocol/Cipher/etc. setting sitting in a vhost, *ineffective now since it is not the first vhost*, this change would activate it.

Ciphers/etc work per vhost already thanks to the SNI callback, it's
only SSLProtocol that can't be changed from there due to OpenSSL
internals (AIUI), but still..

> So it could expose a site to a TLS setting that is not appropriate for it. One could argue that the first mistake was for the admin to leave that setting there, but...

Yeah, my fear as well.

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
Sure thing! Wine usually disinhibits discussions :)

On Fri, Oct 25, 2019 at 11:45 AM Stefan Eissing
<st...@greenbytes.de> wrote:
>
> Can we wait with the naming discussion after I opened my Friday evening Wine bottle?
>
> > Am 25.10.2019 um 10:16 schrieb Yann Ylavic <yl...@gmail.com>:
> >
> > On Fri, Oct 25, 2019 at 9:56 AM Stefan Eissing
> > <st...@greenbytes.de> wrote:
> >>
> >> While I like this change and think, ideally, it would have behaved like this all the time, I think we need to make this "opt-in" for 2.4.
> >
> > So now the "how" and name bikeshedding :)
> >
> > SSLHonorVhostProtocol on/off (default: off) at the server config scope (only)?
> >>
> >> If I understand this correctly: if someone has some old SSLProtocol/Cipher/etc. setting sitting in a vhost, *ineffective now since it is not the first vhost*, this change would activate it. So it could expose a site to a TLS setting that is not appropriate for it. One could argue that the first mistake was for the admin to leave that setting there, but...
> >>
> >> - Stefan
> >>
> >>> Am 25.10.2019 um 09:46 schrieb Yann Ylavic <yl...@gmail.com>:
> >>>
> >>> On Sun, Oct 20, 2019 at 12:50 PM <yl...@apache.org> wrote:
> >>>>
> >>>> Author: ylavic
> >>>> Date: Sun Oct 20 10:50:33 2019
> >>>> New Revision: 1868645
> >>>>
> >>>> URL: http://svn.apache.org/viewvc?rev=1868645&view=rev
> >>>> Log:
> >>>> mod_ssl: negotiate the TLS protocol version per name based vhost configuration.
> >>>
> >>> I'm planning to propose this for backport to 2.4.x, but wonder if it
> >>> should be opt in/out.
> >>>
> >>> I can see potential behaviour change for existing configurations if,
> >>> for instance, one has specified some SSLProtocol at the base server
> >>> level but none (relying on the current behaviour) or something
> >>> different (somehow working unwittingly of his/her own free will) at
> >>> the other name-based vhost(s) level.
> >>>
> >>> Thoughts?
> >>>
> >>> Regards,
> >>> Yann.
> >>
>

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Stefan Eissing <st...@greenbytes.de>.
Can we wait with the naming discussion after I opened my Friday evening Wine bottle?

> Am 25.10.2019 um 10:16 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Fri, Oct 25, 2019 at 9:56 AM Stefan Eissing
> <st...@greenbytes.de> wrote:
>> 
>> While I like this change and think, ideally, it would have behaved like this all the time, I think we need to make this "opt-in" for 2.4.
> 
> So now the "how" and name bikeshedding :)
> 
> SSLHonorVhostProtocol on/off (default: off) at the server config scope (only)?
>> 
>> If I understand this correctly: if someone has some old SSLProtocol/Cipher/etc. setting sitting in a vhost, *ineffective now since it is not the first vhost*, this change would activate it. So it could expose a site to a TLS setting that is not appropriate for it. One could argue that the first mistake was for the admin to leave that setting there, but...
>> 
>> - Stefan
>> 
>>> Am 25.10.2019 um 09:46 schrieb Yann Ylavic <yl...@gmail.com>:
>>> 
>>> On Sun, Oct 20, 2019 at 12:50 PM <yl...@apache.org> wrote:
>>>> 
>>>> Author: ylavic
>>>> Date: Sun Oct 20 10:50:33 2019
>>>> New Revision: 1868645
>>>> 
>>>> URL: http://svn.apache.org/viewvc?rev=1868645&view=rev
>>>> Log:
>>>> mod_ssl: negotiate the TLS protocol version per name based vhost configuration.
>>> 
>>> I'm planning to propose this for backport to 2.4.x, but wonder if it
>>> should be opt in/out.
>>> 
>>> I can see potential behaviour change for existing configurations if,
>>> for instance, one has specified some SSLProtocol at the base server
>>> level but none (relying on the current behaviour) or something
>>> different (somehow working unwittingly of his/her own free will) at
>>> the other name-based vhost(s) level.
>>> 
>>> Thoughts?
>>> 
>>> Regards,
>>> Yann.
>> 


Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Oct 25, 2019 at 9:56 AM Stefan Eissing
<st...@greenbytes.de> wrote:
>
> While I like this change and think, ideally, it would have behaved like this all the time, I think we need to make this "opt-in" for 2.4.

So now the "how" and name bikeshedding :)

SSLHonorVhostProtocol on/off (default: off) at the server config scope (only)?
>
> If I understand this correctly: if someone has some old SSLProtocol/Cipher/etc. setting sitting in a vhost, *ineffective now since it is not the first vhost*, this change would activate it. So it could expose a site to a TLS setting that is not appropriate for it. One could argue that the first mistake was for the admin to leave that setting there, but...
>
> - Stefan
>
> > Am 25.10.2019 um 09:46 schrieb Yann Ylavic <yl...@gmail.com>:
> >
> > On Sun, Oct 20, 2019 at 12:50 PM <yl...@apache.org> wrote:
> >>
> >> Author: ylavic
> >> Date: Sun Oct 20 10:50:33 2019
> >> New Revision: 1868645
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1868645&view=rev
> >> Log:
> >> mod_ssl: negotiate the TLS protocol version per name based vhost configuration.
> >
> > I'm planning to propose this for backport to 2.4.x, but wonder if it
> > should be opt in/out.
> >
> > I can see potential behaviour change for existing configurations if,
> > for instance, one has specified some SSLProtocol at the base server
> > level but none (relying on the current behaviour) or something
> > different (somehow working unwittingly of his/her own free will) at
> > the other name-based vhost(s) level.
> >
> > Thoughts?
> >
> > Regards,
> > Yann.
>

Re: Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Stefan Eissing <st...@greenbytes.de>.
While I like this change and think, ideally, it would have behaved like this all the time, I think we need to make this "opt-in" for 2.4.

If I understand this correctly: if someone has some old SSLProtocol/Cipher/etc. setting sitting in a vhost, *ineffective now since it is not the first vhost*, this change would activate it. So it could expose a site to a TLS setting that is not appropriate for it. One could argue that the first mistake was for the admin to leave that setting there, but...

- Stefan

> Am 25.10.2019 um 09:46 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Sun, Oct 20, 2019 at 12:50 PM <yl...@apache.org> wrote:
>> 
>> Author: ylavic
>> Date: Sun Oct 20 10:50:33 2019
>> New Revision: 1868645
>> 
>> URL: http://svn.apache.org/viewvc?rev=1868645&view=rev
>> Log:
>> mod_ssl: negotiate the TLS protocol version per name based vhost configuration.
> 
> I'm planning to propose this for backport to 2.4.x, but wonder if it
> should be opt in/out.
> 
> I can see potential behaviour change for existing configurations if,
> for instance, one has specified some SSLProtocol at the base server
> level but none (relying on the current behaviour) or something
> different (somehow working unwittingly of his/her own free will) at
> the other name-based vhost(s) level.
> 
> Thoughts?
> 
> Regards,
> Yann.


Opt in(/out?) for TLS protocol per vhost (was: svn commit: r1868645)

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Oct 20, 2019 at 12:50 PM <yl...@apache.org> wrote:
>
> Author: ylavic
> Date: Sun Oct 20 10:50:33 2019
> New Revision: 1868645
>
> URL: http://svn.apache.org/viewvc?rev=1868645&view=rev
> Log:
> mod_ssl: negotiate the TLS protocol version per name based vhost configuration.

I'm planning to propose this for backport to 2.4.x, but wonder if it
should be opt in/out.

I can see potential behaviour change for existing configurations if,
for instance, one has specified some SSLProtocol at the base server
level but none (relying on the current behaviour) or something
different (somehow working unwittingly of his/her own free will) at
the other name-based vhost(s) level.

Thoughts?

Regards,
Yann.