Always disable TLS renegotiations

Renegotiations have been troublesome in the past and also the recent
OpenSSL security problem (CVE-2021-3449) is only exploitable if
TLS renegotiation is enabled.

mbed TLS disables it by default and says in the documentation:

Warning: It is recommended to always disable renegotation unless you
know you need it and you know what you're doing. In the past, there
have been several issues associated with renegotiation or a poor
understanding of its properties.

TLS renegotiation can be used to restart a session with different
parameters (e.g. now with client certs). This something that OpenVPN does
not use.

For OpenSSL 1.0.2 the workaround to disable renegotiation is rather
cumbersome. So we keep this to 1.1.1 only since 1.0.2 is on its way to
deprecation anyway.

Furthermore because of all these problems, also TLS 1.3 completely
drops support for renegotiations.

Patch V2: Improve comments and commit message
Patch V3: Only disable renegotiation where the SSL_OP_NO_RENEGOTIATION
          define is available. LibreSSL, wolfSSL and OpenSSL 1.0.2 are
          lacking this macro.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210401110003.19689-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21939.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 4626e98389ab2ab2729411bc1974526dae75ef4e..8917fb1885638e3bbff2e38733377badd01fc409 100644 (file)
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1086,6 +1086,10 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl,
     {
         mbedtls_ssl_conf_curves(ks_ssl->ssl_config, ssl_ctx->groups);
     }
+    /* Disable TLS renegotiations. OpenVPN's renegotiation creates new SSL
+     * session and does not depend on this feature. And TLS renegotiations have
+     * been problematic in the past */
+    mbedtls_ssl_conf_renegotiation(ks_ssl->ssl_config, MBEDTLS_SSL_RENEGOTIATION_DISABLED);
 
     /* Disable record splitting (for now).  OpenVPN assumes records are sent
      * unfragmented, and changing that will require thorough review and

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index b85f95be155c14ed2727483d1a2fa0b9d078601e..cb8ac772734a274ce611b8df80c32df2b8de4321 100644 (file)
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -320,6 +320,12 @@ tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags)
     sslopt |= SSL_OP_CIPHER_SERVER_PREFERENCE;
 #endif
     sslopt |= SSL_OP_NO_COMPRESSION;
+    /* Disable TLS renegotiations. OpenVPN's renegotiation creates new SSL
+     * session and does not depend on this feature. And TLS renegotiations have
+     * been problematic in the past */
+#ifdef SSL_OP_NO_RENEGOTIATION
+    sslopt |= SSL_OP_NO_RENEGOTIATION;
+#endif
 
     SSL_CTX_set_options(ctx->ctx, sslopt);