Merge r1877397, r1877795 from trunk:

mod_ssl: Switch to using SSL_OP_NO_RENEGOTATION (where available) to
block client-initiated renegotiation with TLSv1.2 and earlier.

* modules/ssl/ssl_private.h: Define modssl_reneg_state enum,
  modssl_set_reneg_state function.

* modules/ssl/ssl_engine_io.c (bio_filter_out_write,
  bio_filter_in_read): #ifdef-out reneg protection if
  SSL_OP_NO_RENEGOTATION is defined.

* modules/ssl/ssl_engine_init.c (ssl_init_ctx_protocol):
  Enable SSL_OP_NO_RENEGOTATION.
  (ssl_init_ctx_callbacks): Only enable the "info" callback if
  debug-level logging *or* OpenSSL doesn't support SSL_OP_NO_RENEGOTATION.

* modules/ssl/ssl_engine_kernel.c (ssl_hook_Access_classic): Use
  modssl_set_reneg_state to set the reneg protection mode.
  (ssl_hook_Access_modern): Drop manipulation of the reneg mode which
  does nothing for TLSv1.3 already.
  (ssl_callback_Info): Only enable reneg protection if
  SSL_OP_NO_RENEGOTATION is *not* defined.

* modules/ssl/ssl_util_ssl.c (modssl_set_reneg_state): New function.

mod_ssl: follow up to r1877397: fix SSL_OP_NO_RENEGOT*I*ATION typo.

Should work better now :)

Submitted by: jorton, ylavic
Reviewed by: jorton, ylavic, icing
Github: closes #426

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

diff --git a/changes-entries/modssl-no-reneg.txt b/changes-entries/modssl-no-reneg.txt
new file mode 100644 (file)
index 0000000..c2a3eb6
--- /dev/null
+++ b/changes-entries/modssl-no-reneg.txt
@@ -0,0 +1,2 @@
+  *) mod_ssl: Reject client-initiated renegotiation with a TLS alert
+     (rather than connection closure).  [Joe Orton, Yann Ylavic]

diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c
index 12c767cac0f21c6ec28f6b111e194b9ae9e5ed85..443eac4513cee04f9a59bd22955ff090847ddc17 100644 (file)
--- a/modules/ssl/ssl_engine_init.c
+++ b/modules/ssl/ssl_engine_init.c
@@ -844,6 +844,13 @@ static apr_status_t ssl_init_ctx_protocol(server_rec *s,
     }
 #endif
 
+#ifdef SSL_OP_NO_RENEGOTIATION
+    /* For server-side SSL_CTX, disable renegotiation by default.. */
+    if (!mctx->pkp) {
+        SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION);
+    }
+#endif
+
 #ifdef SSL_OP_IGNORE_UNEXPECTED_EOF
     /* For server-side SSL_CTX, enable ignoring unexpected EOF */
     /* (OpenSSL 1.1.1 behavioural compatibility).. */
@@ -872,6 +879,14 @@ static void ssl_init_ctx_session_cache(server_rec *s,
     }
 }
 
+#ifdef SSL_OP_NO_RENEGOTIATION
+/* OpenSSL-level renegotiation protection. */
+#define MODSSL_BLOCKS_RENEG (0)
+#else
+/* mod_ssl-level renegotiation protection. */
+#define MODSSL_BLOCKS_RENEG (1)
+#endif
+
 static void ssl_init_ctx_callbacks(server_rec *s,
                                    apr_pool_t *p,
                                    apr_pool_t *ptemp,
@@ -885,7 +900,13 @@ static void ssl_init_ctx_callbacks(server_rec *s,
     SSL_CTX_set_tmp_dh_callback(ctx,  ssl_callback_TmpDH);
 #endif
 
-    SSL_CTX_set_info_callback(ctx, ssl_callback_Info);
+    /* The info callback is used for debug-level tracing.  For OpenSSL
+     * versions where SSL_OP_NO_RENEGOTIATION is not available, the
+     * callback is also used to prevent use of client-initiated
+     * renegotiation.  Enable it in either case. */
+    if (APLOGdebug(s) || MODSSL_BLOCKS_RENEG) {
+        SSL_CTX_set_info_callback(ctx, ssl_callback_Info);
+    }
 
 #ifdef HAVE_TLS_ALPN
     SSL_CTX_set_alpn_select_cb(ctx, ssl_callback_alpn_select, NULL);

diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c
index b91f784f842e1a915bbdf004d33562a2b1b0e3c7..9c7d2163f8a742a984bf668ce1942eba55654681 100644 (file)
--- a/modules/ssl/ssl_engine_io.c
+++ b/modules/ssl/ssl_engine_io.c
@@ -208,11 +208,13 @@ static int bio_filter_out_write(BIO *bio, const char *in, int inl)
 
     BIO_clear_retry_flags(bio);
 
+#ifndef SSL_OP_NO_RENEGOTIATION
     /* Abort early if the client has initiated a renegotiation. */
     if (outctx->filter_ctx->config->reneg_state == RENEG_ABORT) {
         outctx->rc = APR_ECONNABORTED;
         return -1;
     }
+#endif
 
     ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, outctx->c,
                   "bio_filter_out_write: %i bytes", inl);
@@ -473,11 +475,13 @@ static int bio_filter_in_read(BIO *bio, char *in, int inlen)
 
     BIO_clear_retry_flags(bio);
 
+#ifndef SSL_OP_NO_RENEGOTIATION
     /* Abort early if the client has initiated a renegotiation. */
     if (inctx->filter_ctx->config->reneg_state == RENEG_ABORT) {
         inctx->rc = APR_ECONNABORTED;
         return -1;
     }
+#endif
 
     if (!inctx->bb) {
         inctx->rc = APR_EOF;

diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c
index fe0496f90b5238453f152bd80b2eb0f8b58a8b22..fa1b3a81567fe676bb5928112c9f70c387c67155 100644 (file)
--- a/modules/ssl/ssl_engine_kernel.c
+++ b/modules/ssl/ssl_engine_kernel.c
@@ -992,7 +992,7 @@ static int ssl_hook_Access_classic(request_rec *r, SSLSrvConfigRec *sc, SSLDirCo
 
             /* Toggle the renegotiation state to allow the new
              * handshake to proceed. */
-            sslconn->reneg_state = RENEG_ALLOW;
+            modssl_set_reneg_state(sslconn, RENEG_ALLOW);
 
             SSL_renegotiate(ssl);
             SSL_do_handshake(ssl);
@@ -1019,7 +1019,7 @@ static int ssl_hook_Access_classic(request_rec *r, SSLSrvConfigRec *sc, SSLDirCo
              */
             SSL_peek(ssl, peekbuf, 0);
 
-            sslconn->reneg_state = RENEG_REJECT;
+            modssl_set_reneg_state(sslconn, RENEG_REJECT);
 
             if (!SSL_is_init_finished(ssl)) {
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02261)
@@ -1078,7 +1078,7 @@ static int ssl_hook_Access_modern(request_rec *r, SSLSrvConfigRec *sc, SSLDirCon
         (sc->server->auth.verify_mode != SSL_CVERIFY_UNSET)) {
         int vmode_inplace, vmode_needed;
         int change_vmode = FALSE;
-        int old_state, n, rc;
+        int n, rc;
 
         vmode_inplace = SSL_get_verify_mode(ssl);
         vmode_needed = SSL_VERIFY_NONE;
@@ -1180,8 +1180,6 @@ static int ssl_hook_Access_modern(request_rec *r, SSLSrvConfigRec *sc, SSLDirCon
                 return HTTP_FORBIDDEN;
             }
             
-            old_state = sslconn->reneg_state;
-            sslconn->reneg_state = RENEG_ALLOW;
             modssl_set_app_data2(ssl, r);
 
             SSL_do_handshake(ssl);
@@ -1191,7 +1189,6 @@ static int ssl_hook_Access_modern(request_rec *r, SSLSrvConfigRec *sc, SSLDirCon
              */
             SSL_peek(ssl, peekbuf, 0);
 
-            sslconn->reneg_state = old_state;
             modssl_set_app_data2(ssl, NULL);
 
             /*
@@ -2263,8 +2260,8 @@ static void log_tracing_state(const SSL *ssl, conn_rec *c,
 /*
  * This callback function is executed while OpenSSL processes the SSL
  * handshake and does SSL record layer stuff.  It's used to trap
- * client-initiated renegotiations, and for dumping everything to the
- * log.
+ * client-initiated renegotiations (where SSL_OP_NO_RENEGOTIATION is
+ * not available), and for dumping everything to the log.
  */
 void ssl_callback_Info(const SSL *ssl, int where, int rc)
 {
@@ -2276,14 +2273,12 @@ void ssl_callback_Info(const SSL *ssl, int where, int rc)
         return;
     }
 
-    /* With TLS 1.3 this callback may be called multiple times on the first
-     * negotiation, so the below logic to detect renegotiations can't work.
-     * Fortunately renegotiations are forbidden starting with TLS 1.3, and
-     * this is enforced by OpenSSL so there's nothing to be done here.
-     */
-#if SSL_HAVE_PROTOCOL_TLSV1_3
-    if (SSL_version(ssl) < TLS1_3_VERSION)
-#endif
+#ifndef SSL_OP_NO_RENEGOTIATION
+    /* With OpenSSL < 1.1.1 (implying TLS v1.2 or earlier), this
+     * callback is used to block client-initiated renegotiation.  With
+     * TLSv1.3 it is unnecessary since renegotiation is forbidden at
+     * protocol level.  Otherwise (TLSv1.2 with OpenSSL >=1.1.1),
+     * SSL_OP_NO_RENEGOTIATION is used to block renegotiation. */
     {
         SSLConnRec *sslconn;
 
@@ -2308,6 +2303,7 @@ void ssl_callback_Info(const SSL *ssl, int where, int rc)
             sslconn->reneg_state = RENEG_REJECT;
         }
     }
+#endif
 
     s = mySrvFromConn(c);
     if (s && APLOGdebug(s)) {

diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index 859e932507f6b498654287e0059e1ace42aed36f..25d79ce8dfca33bab9e23dc794815d3507340379 100644 (file)
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -549,6 +549,16 @@ typedef struct {
     apr_time_t     source_mtime;
 } ssl_asn1_t;
 
+typedef enum {
+    RENEG_INIT = 0, /* Before initial handshake */
+    RENEG_REJECT,   /* After initial handshake; any client-initiated
+                     * renegotiation should be rejected */
+    RENEG_ALLOW,    /* A server-initiated renegotiation is taking
+                     * place (as dictated by configuration) */
+    RENEG_ABORT     /* Renegotiation initiated by client, abort the
+                     * connection */
+} modssl_reneg_state;
+
 /**
  * Define the mod_ssl per-module configuration structure
  * (i.e. the global configuration for each httpd process)
@@ -580,18 +590,13 @@ typedef struct {
         NON_SSL_SET_ERROR_MSG  /* Need to set the error message */
     } non_ssl_request;
 
-    /* Track the handshake/renegotiation state for the connection so
-     * that all client-initiated renegotiations can be rejected, as a
-     * partial fix for CVE-2009-3555. */
-    enum {
-        RENEG_INIT = 0, /* Before initial handshake */
-        RENEG_REJECT,   /* After initial handshake; any client-initiated
-                         * renegotiation should be rejected */
-        RENEG_ALLOW,    /* A server-initiated renegotiation is taking
-                         * place (as dictated by configuration) */
-        RENEG_ABORT     /* Renegotiation initiated by client, abort the
-                         * connection */
-    } reneg_state;
+#ifndef SSL_OP_NO_RENEGOTIATION
+    /* For OpenSSL < 1.1.1, track the handshake/renegotiation state
+     * for the connection to block client-initiated renegotiations.
+     * For OpenSSL >=1.1.1, the SSL_OP_NO_RENEGOTIATION flag is used in
+     * the SSL * options state with equivalent effect. */
+    modssl_reneg_state reneg_state;
+#endif
 
     server_rec *server;
     SSLDirConfigRec *dc;
@@ -1198,6 +1203,9 @@ int ssl_is_challenge(conn_rec *c, const char *servername,
  * the configured ENGINE. */
 int modssl_is_engine_id(const char *name);
 
+/* Set the renegotation state for connection. */
+void modssl_set_reneg_state(SSLConnRec *sslconn, modssl_reneg_state state);
+
 #endif /* SSL_PRIVATE_H */
 /** @} */
 

diff --git a/modules/ssl/ssl_util_ssl.c b/modules/ssl/ssl_util_ssl.c
index 44930b70e973cd3eafbdc6cbf5e594644f0f55e2..8bd9c8a2d236f20993cae42845b7e8b18ad57f77 100644 (file)
--- a/modules/ssl/ssl_util_ssl.c
+++ b/modules/ssl/ssl_util_ssl.c
@@ -612,3 +612,19 @@ cleanup:
     }
     return rv;
 }
+
+void modssl_set_reneg_state(SSLConnRec *sslconn, modssl_reneg_state state)
+{
+#ifdef SSL_OP_NO_RENEGOTIATION
+    switch (state) {
+    case RENEG_ALLOW:
+        SSL_clear_options(sslconn->ssl, SSL_OP_NO_RENEGOTIATION);
+        break;
+    default:
+        SSL_set_options(sslconn->ssl, SSL_OP_NO_RENEGOTIATION);
+        break;
+    }
+#else
+    sslconn->reneg_state = state;
+#endif
+}