From: Joe Orton Date: Fri, 21 Jun 2024 14:28:25 +0000 (+0000) Subject: Merge r1877397, r1877795 from trunk: X-Git-Tag: 2.4.60-rc1-candidate~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=644fcf5fdb3d49d7d4594dc23e7cd5d65ee3edad;p=thirdparty%2Fapache%2Fhttpd.git 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 index 00000000000..c2a3eb614b6 --- /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 12c767cac0f..443eac4513c 100644 --- 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 b91f784f842..9c7d2163f8a 100644 --- 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 fe0496f90b5..fa1b3a81567 100644 --- 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 859e932507f..25d79ce8dfc 100644 --- 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 44930b70e97..8bd9c8a2d23 100644 --- 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 +}