From: Kaspar Brand Date: Fri, 18 Apr 2014 08:29:11 +0000 (+0000) Subject: Merge r1585090 from trunk: X-Git-Tag: 2.4.10~314 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b31ae3b6443be6f0dc132963b81e203884c97c02;p=apache 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 --- diff --git a/CHANGES b/CHANGES index 2ecbcbc79b..48003f1212 100644 --- a/CHANGES +++ b/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] diff --git a/STATUS b/STATUS index c5d1585c27..0058313812 100644 --- a/STATUS +++ b/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 ] diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index 15cd6f56af..6512992dae 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/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 - * just the certificate/keys of one virtual host (which one cannot be said - * easily - but that doesn't matter here). + * same IP:port and an OpenSSL version without support for TLS extensions + * (SNI in particular) is used. */ 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 - int level = APLOG_WARNING; - 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: " + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server, + "Init: SSL server IP/port conflict: " "%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 " - "virtual hosts in conjunction with SSL!!"); -#else - ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server, APLOGNO(02292) - "Init: Name-based SSL virtual hosts only " - "work for clients with TLS server name indication " - "support (RFC 4366)"); -#endif + "Init: Name-based SSL virtual hosts require " + "an OpenSSL version with support for TLS extensions " + "(RFC 6066 - Server Name Indication / SNI), " + "but the currently used library version (%s) is " + "lacking this feature", SSLeay_version(SSLEAY_VERSION)); } +#endif return APR_SUCCESS; } diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c index c60f0a6c66..178a7cb7ed 100644 --- a/modules/ssl/ssl_engine_kernel.c +++ b/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." + */ } } }