From patchwork Mon Apr 8 12:49:33 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Lichtenheld X-Patchwork-Id: 3683 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:292f:b0:569:ad12:4fde with SMTP id f15csp2231066maw; Mon, 8 Apr 2024 05:50:27 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCV4WQv7KuRRuKVhbT5yD6vctvhnLQqWFPDmAUZ0O7LXcG2vptPhCOPeCirJvL2djf/DeDHYCbiLL4D3tIl0WgZWZoVUBD0= X-Google-Smtp-Source: AGHT+IGy4Hx7VoYowgszE272um62Wqus0iZ6ff6iSjI7ebMlHSxFol0Yf+S+1bxx/p3bREAhEpSh X-Received: by 2002:a17:90a:3003:b0:2a2:2dda:6802 with SMTP id g3-20020a17090a300300b002a22dda6802mr7348094pjb.0.1712580626927; Mon, 08 Apr 2024 05:50:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1712580626; cv=none; d=google.com; s=arc-20160816; b=yMHR9OBPW1LRiZf+Z1iM4vPjLzsT2LmExEmFlZRA7B9KCcPdHqw9ZC1dOKjCD2qUAr 305TjjcIeKRVDKt/HLkJJqMlduFIGRmJAAPnUYJrV/Rj/kLgIbaClDqpkSZhZy5H1ndS xfV9SxV+C+449K5DrgnBThcgDbXSQmgufX8R91kbpw8Tz/KrFXvphIqPFVFC7PjI9DaF ceuxCgM3FEopslwvZhVEHLlevNQKEfrpoedeKxp2e1s6odJG3AX3crPme07Kke3ycjy5 e4vPuvXeCKlR+MvRG2Fs2Y1xK+74mYPmgGBcSV8gyWhgy8+9bwQGpg3/G96t78ZlymqO ZZHw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature; bh=AMLkVdddzzp9HVnMl8bY/31sWDsxsvETgaVRt4VvrrI=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=OvhuYDNBVddj1U5WyP4eKyf51GWqcGMlNeM7wpYBbu7u7xWada1/Arf6t0KIkTDUnO 3IgkVPSKm0Lu6KCIA+ReSJihe7u37rYq5uzUABcmyWc0Qi5KjHckl+c4VFn9c9CSPno8 8h8TU9voFw10yunxya/34dw9uPbXMQ4QSJ4iMA5N0iLJ4fhBgOVh9lOMzeb+ZOnwHjYk BtDDuPS93Wc7NBruwPrxySpYm86dNIeYTNyF/+yr7O4hwskRwBpNu4SqeUDyPYviLH8Z Lk0zg2RXzlgAX/kjSYpxPHP9UEQ1EcHdayI7y3cLXcLkWBz2kWsxEN25/VSR1sjP3rYe UL+A==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=RjaNW3Cu; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=LjCEE6nC; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b=XJIJCjMH; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id h6-20020a17090adb8600b002a09051a0e8si6274839pjv.156.2024.04.08.05.50.26 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Apr 2024 05:50:26 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=RjaNW3Cu; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=LjCEE6nC; dkim=neutral (body hash did not verify) header.i=@lichtenheld.com header.s=MBO0001 header.b=XJIJCjMH; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1rtoRe-0007D9-If; Mon, 08 Apr 2024 12:49:54 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rtoRb-0007Cy-Er for openvpn-devel@lists.sourceforge.net; Mon, 08 Apr 2024 12:49:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=XluyCd18ECWMHCXmgg4+uVEcizohZ17z8DKZZOZDl7Y=; b=RjaNW3Cu6GcryMhiPFvtm5q887 mPU6QXt37Xr0Fr4nrJo6o30o6EB/7qHITnonvF/gPISYiXgyfb1m827YjyWtC/sPalj/w7A+Bty09 QDcKu8yWUge42FLQBU96ocpgTRtnFGYWZs699EfmLxtv6EEdUYUcQBLOeqv8Te3vTWWM=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=XluyCd18ECWMHCXmgg4+uVEcizohZ17z8DKZZOZDl7Y=; b=LjCEE6nCt37531e1XOJYqzlJO9 uUIThvYyKojrrXqLeLSBGYw6+WEZxhXQMo2aUEo1r0Iry+3z2wWgGb0BpXkbL6ybX+DgT61D5Xsgw vI24ORuOLzcJGZ2rLpvF6S0t/y+GnpFX9h3kMwFASlYstnmdIyMYwxBQCicqj9ORhIDA=; Received: from mout-p-103.mailbox.org ([80.241.56.161]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1rtoRZ-0001rq-0k for openvpn-devel@lists.sourceforge.net; Mon, 08 Apr 2024 12:49:51 +0000 Received: from smtp2.mailbox.org (smtp2.mailbox.org [IPv6:2001:67c:2050:b231:465::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-103.mailbox.org (Postfix) with ESMTPS id 4VCpnq5Syvz9sW5; Mon, 8 Apr 2024 14:49:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lichtenheld.com; s=MBO0001; t=1712580575; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XluyCd18ECWMHCXmgg4+uVEcizohZ17z8DKZZOZDl7Y=; b=XJIJCjMHkyigKI2AfkbPUudlAnjmVI0BVAUynloqR8lQ6aizyFsrMek4j0IqNPnddpZAta GNX7216lSgT4zCi+8D/03hEjE6evSUXP8HefzqlqDonh38mAwncSbLTxfB2L08dJ7sLo0c z65BHRYY05N+gfJaBss+SNDUXiPzxbMWPqbY0OFLyroS98fD4G8qufTqulRH2Rxakg/TqG L5FVlEQWa0NhKyHbwmZAkh3JSBLG2y4SRqOwEV8CYB3a083h767H+aE9FjJEgShdGbtKTv kZlYtcCeCzSCBLL1459YSMSio73OqFsJDhV9kGYRLMIjpHn44AcoLoSmtlhd3Q== From: Frank Lichtenheld To: openvpn-devel@lists.sourceforge.net Date: Mon, 8 Apr 2024 14:49:33 +0200 Message-Id: <20240408124933.243991-1-frank@lichtenheld.com> In-Reply-To: References: MIME-Version: 1.0 X-Rspamd-Queue-Id: 4VCpnq5Syvz9sW5 X-Spam-Score: -0.9 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Arne Schwabe Previous OpenVPN versions shut down the TLS control channel immediately when encountering an error. This also meant that we would not send out TLS alerts to notify a client about potential problems li [...] Content analysis details: (-0.9 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at https://www.dnswl.org/, low trust [80.241.56.161 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-Headers-End: 1rtoRZ-0001rq-0k Subject: [Openvpn-devel] [PATCH v6] Allow the TLS session to send out TLS alerts X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1795770943470824909?= X-GMAIL-MSGID: =?utf-8?q?1795770943470824909?= From: Arne Schwabe Previous OpenVPN versions shut down the TLS control channel immediately when encountering an error. This also meant that we would not send out TLS alerts to notify a client about potential problems like mismatching TLS versions or having no common cipher. This commit adds a new key_state S_ERROR_PRE which still allows to send out the remaining TLS packets of the control session which are typically the alert message and then going to S_ERROR. We do not wait for retries. So this is more a one-shot notify but that is acceptable in this situation. Sending out alerts is a slight compromise in security as alerts give out a bit of information that otherwise is not given out. But since all other consumers TLS implementations are already doing this and TLS implementations (nowadays) are very careful not to leak (sensitive) information by alerts and since the user experience is much better with alerts, this compromise is worth it. Change-Id: I0ad48915004ddee587e97c8ed190ba8ee989e48d Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/449 This mail reflects revision 6 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/Changes.rst b/Changes.rst index 69c811d..52ff8db 100644 --- a/Changes.rst +++ b/Changes.rst @@ -1,5 +1,14 @@ Overview of changes in 2.7 ========================== +New features +------------ +TLS alerts + OpenVPN 2.7 will send out TLS alerts to peers informing them if the TLS + session shuts down or when the TLS implementation informs the peer about + an error in the TLS session (e.g. mismatching TLS versions). This improves + the user experience as the client shows an error instead of running into + a timeout when the server just stops responding completely. + Deprecated features ------------------- ``secret`` support has been removed by default. diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 33c8670..f287e4d 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -690,6 +690,9 @@ case S_ERROR: return "S_ERROR"; + case S_ERROR_PRE: + return "S_ERROR_PRE"; + case S_GENERATED_KEYS: return "S_GENERATED_KEYS"; @@ -2683,6 +2686,25 @@ } static bool +check_outgoing_ciphertext(struct key_state *ks, struct tls_session *session, + bool *continue_tls_process) +{ + /* Outgoing Ciphertext to reliable buffer */ + if (ks->state >= S_START) + { + struct buffer *buf = reliable_get_buf_output_sequenced(ks->send_reliable); + if (buf) + { + if (!write_outgoing_tls_ciphertext(session, continue_tls_process)) + { + return false; + } + } + } + return true; +} + +static bool tls_process_state(struct tls_multi *multi, struct tls_session *session, struct buffer *to_link, @@ -2706,7 +2728,7 @@ } /* Are we timed out on receive? */ - if (now >= ks->must_negotiate && ks->state < S_ACTIVE) + if (now >= ks->must_negotiate && ks->state >= S_UNDEF && ks->state < S_ACTIVE) { msg(D_TLS_ERRORS, "TLS Error: TLS key negotiation failed to occur within %d seconds (check your network connectivity)", @@ -2760,6 +2782,16 @@ return false; } + if (ks->state == S_ERROR_PRE) + { + /* When we end up here, we had one last chance to send an outstanding + * packet that contained an alert. We do not ensure that this packet + * has been successfully delivered (ie wait for the ACK etc) + * but rather stop processing now */ + ks->state = S_ERROR; + return false; + } + /* Write incoming ciphertext to TLS object */ struct reliable_entry *entry = reliable_get_entry_sequenced(ks->rec_reliable); if (entry) @@ -2840,29 +2872,31 @@ dmsg(D_TLS_DEBUG, "Outgoing Plaintext -> TLS"); } } - - /* Outgoing Ciphertext to reliable buffer */ - if (ks->state >= S_START) + if (!check_outgoing_ciphertext(ks, session, &continue_tls_process)) { - buf = reliable_get_buf_output_sequenced(ks->send_reliable); - if (buf) - { - if (!write_outgoing_tls_ciphertext(session, &continue_tls_process)) - { - goto error; - } - } + goto error; } return continue_tls_process; error: tls_clear_error(); - ks->state = S_ERROR; + + /* Shut down the TLS session but do a last read from the TLS + * object to be able to read potential TLS alerts */ + key_state_ssl_shutdown(&ks->ks_ssl); + check_outgoing_ciphertext(ks, session, &continue_tls_process); + + /* Put ourselves in the pre error state that will only send out the + * control channel packets but nothing else */ + ks->state = S_ERROR_PRE; + msg(D_TLS_ERRORS, "TLS Error: TLS handshake failed"); INCR_ERROR; - return false; - + return true; } + + + /* * This is the primary routine for processing TLS stuff inside the * the main event loop. When this routine exits @@ -2970,7 +3004,7 @@ } /* When should we wake up again? */ - if (ks->state >= S_INITIAL) + if (ks->state >= S_INITIAL || ks->state == S_ERROR_PRE) { compute_earliest_wakeup(wakeup, reliable_send_timeout(ks->send_reliable)); @@ -3123,7 +3157,7 @@ session_id_print(&ks->session_id_remote, &gc), print_link_socket_actual(&ks->remote_addr, &gc)); - if (ks->state >= S_INITIAL && link_socket_actual_defined(&ks->remote_addr)) + if ((ks->state >= S_INITIAL || ks->state == S_ERROR_PRE) && link_socket_actual_defined(&ks->remote_addr)) { struct link_socket_actual *tla = NULL; @@ -3201,7 +3235,8 @@ { msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed"); ks->authenticated = KS_AUTH_FALSE; - ks->state = S_ERROR; + key_state_ssl_shutdown(&ks->ks_ssl); + ks->state = S_ERROR_PRE; } /* Update auth token on the client if needed on renegotiation diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h index b9466ce..a4d4dd4 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -363,6 +363,13 @@ const struct tls_root_ctx *ssl_ctx, bool is_server, struct tls_session *session); /** + * Sets a TLS session to be shutdown state, so the TLS library will generate + * a shutdown alert. + */ +void +key_state_ssl_shutdown(struct key_state_ssl *ks_ssl); + +/** * Free the SSL channel part of the given key state. * * @param ks_ssl The SSL channel's state info to free diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 925660b..ff455b3 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -74,7 +74,10 @@ * * @{ */ -#define S_ERROR -1 /**< Error state. */ +#define S_ERROR (-2) /**< Error state. */ +#define S_ERROR_PRE (-1) /**< Error state but try to send out alerts + * before killing the keystore and moving + * it to S_ERROR */ #define S_UNDEF 0 /**< Undefined state, used after a \c * key_state is cleaned up. */ #define S_INITIAL 1 /**< Initial \c key_state state after diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index cc88484..5e6726c 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1276,6 +1276,14 @@ ssl_bio_read, NULL); } + +void +key_state_ssl_shutdown(struct key_state_ssl *ks_ssl) +{ + mbedtls_ssl_send_alert_message(ks_ssl->ctx, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_CLOSE_NOTIFY); +} + void key_state_ssl_free(struct key_state_ssl *ks_ssl) { diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index c30e6a9..552d751 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1962,6 +1962,12 @@ } void +key_state_ssl_shutdown(struct key_state_ssl *ks_ssl) +{ + SSL_set_shutdown(ks_ssl->ssl, SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN); +} + +void key_state_ssl_free(struct key_state_ssl *ks_ssl) { if (ks_ssl->ssl)