From patchwork Mon Nov 20 11:28:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3457 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:50e4:b0:f2:62eb:61c1 with SMTP id r4csp2199989dyd; Mon, 20 Nov 2023 03:29:05 -0800 (PST) X-Google-Smtp-Source: AGHT+IEftaXOGjZCbFqAQ5y9vyqYtrjiZEF7eUevbUC7LenxkgEbOM9uFICEbJ2hiYF02gd1Tx3L X-Received: by 2002:a05:6a21:3885:b0:18a:716d:4c6 with SMTP id yj5-20020a056a21388500b0018a716d04c6mr2051820pzb.6.1700479745117; Mon, 20 Nov 2023 03:29:05 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700479745; cv=none; d=google.com; s=arc-20160816; b=IMWcIEYnkVNd4KLcrks5ttFgUVaJQHWH7uLOUkl1coM8jcv4mF66fOAJNOTPD1cj3I Xge72CvxBCt7AVbQ44RqkZUZZo/Tpp4Q8+0QyOolB2UPThizQ5aUiHalguzvkurL5Lm6 p/PEALhqtk4bPwT+ARFc1oheP/7uggBe84EGmTaOpckMz/kYrL8HJSFD/O9St+pXFRLz Yjxp+vyabsIsxz9h54YgUqPorGE0cKQ/0smY415axs6mD9cxCqw+4seYS8DxGfHdOQ9q tXF9wPvbaaz+i5QHx8Vs3poa8F0UaoUKgtbqKc0TDEzd2U4zRoiP8SsqM8XrafwIV6Zr Lu5g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=aXki6KaSGIzHAHGy8zE7zwty3LsH+ecCN5fxv+x5oWM=; fh=lm0MLPW7DntlrDqRECIiC9JlE1uPxhepE0URYHIf+eE=; b=WFbeTpAANbUJDn4TWV3t5Sw4r2Vyec/lMry9eV3Kn8SroehaKPwr8WjhDtqZw7oeZe vWQQZsOGeM2+hQdYmIFsi2vg2RpBLjZpfUS0Z8Pf8VEYy1LWt4VnmvVL/5tdONGneDz7 vzebwAT2wa5K4YyCCRG4aPiwEZ8xy+B366+ATbDa4L+dLo9uZ0SoNIIziuylqZOc34ti aMvIkdGySTXLRDMcJI/fy1agNtMXmRdAnvPrIXRmcHiNhFa8pg3/vFJsnjKzas/1JQDX kyWgYAqe4VVgn6t3likmo/PvfzC4FMqOk6oWU1xoLhpOzCU5h0VdZMfYmn7+puyPZ9cq RMzg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=DCffAyaW; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="krYma/Cj"; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=I9JQMC8y; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id l65-20020a639144000000b005c1cc97c1e7si7703368pge.661.2023.11.20.03.29.04 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 20 Nov 2023 03:29:05 -0800 (PST) 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=DCffAyaW; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b="krYma/Cj"; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=I9JQMC8y; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1r52Rs-0003iK-2c; Mon, 20 Nov 2023 11:28:16 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1r52Rn-0003iD-OC for openvpn-devel@lists.sourceforge.net; Mon, 20 Nov 2023 11:28:11 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=rVJv37Oc63F4phTwNoogy/s1g0JuQiB96awc2gcZ/k4=; b=DCffAyaWA79z5TWZeOp0JcjPC7 hzDqpC2Q1FP6rP+eag9JRdYJTFKXfWvhAMIxvOkeGhKx8PrKg8eNmP3GuM7aMtH9IpFa3xnbwV+vW oebBhpXbhce7EuE5Q+AAe14wOptdFMU3Mi9hYKk25v+TC3swFUth+LkVGeOlDaKnxk4c=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=rVJv37Oc63F4phTwNoogy/s1g0JuQiB96awc2gcZ/k4=; b=k rYma/CjP5+i1REItpqwibcTek9AeIpT8d6VBrxRCFloKBHGgEui5Y2RyrEef7opp2kLMJqebinUs3 D0xL/bCTO6p6qSuyT7dQAAol3Iu+9fwynkTh9fV/CCvRpq9MJu8HcmPzE6CkhUaZ8XZ8LBqHbJUBp lwy2JDBWWtNOtvEY=; Received: from mail-lj1-f179.google.com ([209.85.208.179]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1r52Rm-00AxQx-1Z for openvpn-devel@lists.sourceforge.net; Mon, 20 Nov 2023 11:28:11 +0000 Received: by mail-lj1-f179.google.com with SMTP id 38308e7fff4ca-2c8880f14eeso523581fa.3 for ; Mon, 20 Nov 2023 03:28:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1700479683; x=1701084483; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=rVJv37Oc63F4phTwNoogy/s1g0JuQiB96awc2gcZ/k4=; b=I9JQMC8yAMqXbLTmIRYI4FbfqRwbwxUMgjCAFGR2BQSxBB9AzQljSISa0azqba3vs8 PTfId0hUghEr0p4kif3o7vfkn54dp8XC8HSGytDsEnXmlpVSSNJeC87qPOW9xNlEiByE 6EGZJpFK6/GxKm41REYkaCsJsb4wxJzqnBaC3JWmQPGsiq/jKEs8QVWssUw3yehs800Z UqZh+MYBT0+s3uy5Slx+1qcO1vd8akTdzXq0d5AaaKF0uexAMXqYlz4gHHuPbhecFDta e3/12GjXgoElwVLtSmHy6ZFylllb5Kl/If736wod1OrYYS5Sfmk8SZuCK3w9t9IDB9dG PRsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700479683; x=1701084483; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rVJv37Oc63F4phTwNoogy/s1g0JuQiB96awc2gcZ/k4=; b=N1q9gYT2iIoDRdMQuIDHHFp0fyyFbI1zUGqzwzeT9n6OqSaoUZMRI04kEgIYT5cbXa kBFr/yaspcf0ppYOfQC1cVBDZIucz7bZoYYyv1JilooQ/ReKUKwYpIz/eWSCXY4FMf6h uwlS922UwVTWpmSruElMYh3STuWU6YZJzZBmL0nBOlop0hI/zKZXPsXLDcCg1jVkjFvr bk/RQQp1nYgirQ1WEHDY+M7gSJyYpfz5eBQNFcLcWpWs53SaefY6Z7NmUcMlUbjJ+QjE 7hv1cw/72+s/h9GSK4xs8lrFlyA0AVXgk8BYGyCiWkpLxVOvDsOcumMHh1nkYJpW2Mtj 4nBw== X-Gm-Message-State: AOJu0Ywq/L+8P4ytZet7puS6rCgw5h2tmhY9BRpYcQdD5hQnfow3e6dC 69UmGQ/K4q5W0Yvf1Q3tAV4fFOHmkiB8n4VWgMg= X-Received: by 2002:a2e:a489:0:b0:2c8:3254:9dc7 with SMTP id h9-20020a2ea489000000b002c832549dc7mr4218795lji.41.1700479683029; Mon, 20 Nov 2023 03:28:03 -0800 (PST) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id z7-20020a05600c0a0700b003fc0505be19sm13349924wmp.37.2023.11.20.03.28.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Nov 2023 03:28:02 -0800 (PST) From: "plaisthos (Code Review)" X-Google-Original-From: "plaisthos (Code Review)" X-Gerrit-PatchSet: 1 Date: Mon, 20 Nov 2023 11:28:02 +0000 To: flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I0ad48915004ddee587e97c8ed190ba8ee989e48d X-Gerrit-Change-Number: 449 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 1c17880327486961b54c13500cd5e2c0778ad427 References: Message-ID: <1ddeb3a3dfc9ce8452e6db42302f420eb1226910-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-1.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: Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.208.179 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.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.208.179 listed in wl.mailspike.net] 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message -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 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.0 T_SCC_BODY_TEXT_LINE No description available. 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1r52Rm-00AxQx-1Z Subject: [Openvpn-devel] [M] Change in openvpn[master]: 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: , Reply-To: arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1783082249317260868?= X-GMAIL-MSGID: =?utf-8?q?1783082249317260868?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/449?usp=email to review the following change. Change subject: Allow the TLS session to send out TLS alerts ...................................................................... Allow the TLS session to send out TLS alerts 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 implementation are already doing this and TLS implementation (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 --- M Changes.rst M src/openvpn/ssl.c M src/openvpn/ssl_backend.h M src/openvpn/ssl_common.h M src/openvpn/ssl_mbedtls.c M src/openvpn/ssl_openssl.c 6 files changed, 98 insertions(+), 18 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/49/449/1 diff --git a/Changes.rst b/Changes.rst index 3676dce..4c8be39 100644 --- a/Changes.rst +++ b/Changes.rst @@ -1,5 +1,16 @@ Overview of changes in 2.7 ========================== +New features +------------ +TLS alerts + OpenVPN 2.7 will send out TLS alerts to peer 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 stop responding completely. + +Deprecated features +------------------- ``secret`` support has been removed by default. static key mode (non-TLS) is no longer considered "good and secure enough" for today's requirements. Use TLS mode instead. If deploying a PKI CA diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index b4cd8f5..16d3d22 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -853,6 +853,9 @@ case S_ERROR: return "S_ERROR"; + case S_ERROR_PRE: + return "S_ERROR_PRE"; + case S_GENERATED_KEYS: return "S_GENERATED_KEYS"; @@ -2839,6 +2842,35 @@ return true; } +/** + * Shut down an SSL session, so an SSL close notify is sent if there no other + * SSL notify. + * @param ks + */ +void +do_ssl_shutdown(struct key_state *ks) +{ +} + + +static bool +check_outgoing_ciphertext(struct key_state *ks, struct tls_session *session, + bool *state_change) +{ + /* Outgoing Ciphertext to reliable buffer */ + if (ks->state >= S_PRE_START) + { + struct buffer *buf = reliable_get_buf_output_sequenced(ks->send_reliable); + if (buf) + { + if (!write_outgoing_tls_ciphertext(session, state_change)) + { + return false; + } + } + } + return true; +} static bool tls_process_state(struct tls_multi *multi, @@ -2912,6 +2944,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) @@ -2992,29 +3034,31 @@ dmsg(D_TLS_DEBUG, "Outgoing Plaintext -> TLS"); } } - - /* Outgoing Ciphertext to reliable buffer */ - if (ks->state >= S_START) + if (!check_outgoing_ciphertext(ks, session, &state_change)) { - buf = reliable_get_buf_output_sequenced(ks->send_reliable); - if (buf) - { - if (!write_outgoing_tls_ciphertext(session, &state_change)) - { - goto error; - } - } + goto error; } return state_change; 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 */ + do_ssl_shutdown(ks); + 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 @@ -3122,7 +3166,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)); @@ -3275,7 +3319,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; @@ -3353,7 +3397,8 @@ { msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed"); ks->authenticated = KS_AUTH_FALSE; - ks->state = S_ERROR; + do_ssl_shutdown(ks); + 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 3854d59..ea31902 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -372,6 +372,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 altert. + */ +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 d3edc5f..d38d8ec 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 9c9167d..c8aca4f 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1275,6 +1275,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 82872bf..622b9b5 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1961,6 +1961,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)