Merge r1585090 from trunk:

Bring SNI behavior into better conformance with RFC 6066:

- no longer send a warning-level unrecognized_name(112) alert
  when no matching vhost is found (PR 56241)

- at startup, only issue warnings about IP/port conflicts and name-based
  SSL vhosts when running with an OpenSSL without TLS extension support
  (almost 5 years after SNI was added to 2.2.x, the
  "[...] only work for clients with TLS server name indication support"
  warning feels obsolete)

Proposed by: kbrand
Reviewed by: jorton, ylavic


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1588424 13f79535-47bb-0310-9956-ffa450edef68

CHANGES

@@ -2,6 +2,11 @@
 
 Changes with Apache 2.4.10
 
+  *) mod_ssl: bring SNI behavior into better conformance with RFC 6066:
+     no longer send warning-level unrecognized_name(112) alerts,
+     and limit startup warnings to cases where an OpenSSL version
+     without TLS extension support is used. PR 56241. [Kaspar Brand]
+
   *) mod_proxy_html: Avoid some possible memory access violation in case of
      specially crafted files, when the ProxyHTMLMeta directive is turned on.
      Follow up of PR 56287 [Christophe Jaillet]

STATUS

@@ -112,12 +112,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
      2.4.x patch: http://people.apache.org/~jailletc36/r1514255.patch
      +1: jailletc36, gsmith, ylavic
 
-   * mod_ssl: bring SNI behavior into better conformance with RFC 6066
-     (also addresses PR 56241)
-     trunk patch: https://svn.apache.org/r1585090
-     2.4.x patch: trunk patch works (modulo CHANGES)
-     +1: kbrand, jorton, ylavic
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

modules/ssl/ssl_engine_init.c

@@ -1399,13 +1399,16 @@ apr_status_t ssl_init_ConfigureServer(server_rec *s,
 
 apr_status_t ssl_init_CheckServers(server_rec *base_server, apr_pool_t *p)
 {
-    server_rec *s, *ps;
+    server_rec *s;
     SSLSrvConfigRec *sc;
+#ifndef HAVE_TLSEXT
+    server_rec *ps;
     apr_hash_t *table;
     const char *key;
     apr_ssize_t klen;
 
     BOOL conflict = FALSE;
+#endif
 
     /*
      * Give out warnings when a server has HTTPS configured
@@ -1433,11 +1436,11 @@ apr_status_t ssl_init_CheckServers(server_rec *base_server, apr_pool_t *p)
         }
     }
 
+#ifndef HAVE_TLSEXT
     /*
      * Give out warnings when more than one SSL-aware virtual server uses the
-     * same IP:port. This doesn't work because mod_ssl then will always use
+     * same IP:port and an OpenSSL version without support for TLS extensions
-     * just the certificate/keys of one virtual host (which one cannot be said
+     * (SNI in particular) is used.
-     * easily - but that doesn't matter here).
      */
     table = apr_hash_make(p);
 
@@ -1455,17 +1458,10 @@ apr_status_t ssl_init_CheckServers(server_rec *base_server, apr_pool_t *p)
         klen = strlen(key);
 
         if ((ps = (server_rec *)apr_hash_get(table, key, klen))) {
-#ifndef HAVE_TLSEXT
+            ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server,
-            int level = APLOG_WARNING;
+                         "Init: SSL server IP/port conflict: "
-            const char *problem = "conflict";
-#else
-            int level = APLOG_DEBUG;
-            const char *problem = "overlap";
-#endif
-            ap_log_error(APLOG_MARK, level, 0, base_server,
-                         "Init: SSL server IP/port %s: "
                          "%s (%s:%d) vs. %s (%s:%d)",
-                         problem, ssl_util_vhostid(p, s),
+                         ssl_util_vhostid(p, s),
                          (s->defn_name ? s->defn_name : "unknown"),
                          s->defn_line_number,
                          ssl_util_vhostid(p, ps),
@@ -1479,17 +1475,14 @@ apr_status_t ssl_init_CheckServers(server_rec *base_server, apr_pool_t *p)
     }
 
     if (conflict) {
-#ifndef HAVE_TLSEXT
         ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server, APLOGNO(01917)
-                     "Init: You should not use name-based "
+                     "Init: Name-based SSL virtual hosts require "
-                     "virtual hosts in conjunction with SSL!!");
+                     "an OpenSSL version with support for TLS extensions "
-#else
+                     "(RFC 6066 - Server Name Indication / SNI), "
-        ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server, APLOGNO(02292)
+                     "but the currently used library version (%s) is "
-                     "Init: Name-based SSL virtual hosts only "
+                     "lacking this feature", SSLeay_version(SSLEAY_VERSION));
-                     "work for clients with TLS server name indication "
-                     "support (RFC 4366)");
-#endif
     }
+#endif
 
     return APR_SUCCESS;
 }

modules/ssl/ssl_engine_kernel.c

@@ -1905,7 +1905,7 @@ void ssl_callback_Info(const SSL *ssl, int where, int rc)
 #ifdef HAVE_TLSEXT
 /*
  * This callback function is executed when OpenSSL encounters an extended
- * client hello with a server name indication extension ("SNI", cf. RFC 4366).
+ * client hello with a server name indication extension ("SNI", cf. RFC 6066).
  */
 int ssl_callback_ServerNameIndication(SSL *ssl, int *al, modssl_ctx_t *mctx)
 {
@@ -1927,7 +1927,21 @@ int ssl_callback_ServerNameIndication(SSL *ssl, int *al, modssl_ctx_t *mctx)
                               "No matching SSL virtual host for servername "
                               "%s found (using default/first virtual host)",
                               servername);
-                return SSL_TLSEXT_ERR_ALERT_WARNING;
+                /*
+                 * RFC 6066 section 3 says "It is NOT RECOMMENDED to send
+                 * a warning-level unrecognized_name(112) alert, because
+                 * the client's behavior in response to warning-level alerts
+                 * is unpredictable."
+                 *
+                 * To maintain backwards compatibility in mod_ssl, we
+                 * no longer send any alert (neither warning- nor fatal-level),
+                 * i.e. we take the second action suggested in RFC 6066:
+                 * "If the server understood the ClientHello extension but
+                 * does not recognize the server name, the server SHOULD take
+                 * one of two actions: either abort the handshake by sending
+                 * a fatal-level unrecognized_name(112) alert or continue
+                 * the handshake."
+                 */
             }
         }
     }