From patchwork Wed May 20 08:34:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Evans X-Patchwork-Id: 1120 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director9.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id wOzTBsV8xV6sZgAAIUCqbw for ; Wed, 20 May 2020 14:53:57 -0400 Received: from proxy19.mail.ord1d.rsapps.net ([172.30.191.6]) by director9.mail.ord1d.rsapps.net with LMTP id OIiiBsV8xV5WNgAAalYnBA ; Wed, 20 May 2020 14:53:57 -0400 Received: from smtp5.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy19.mail.ord1d.rsapps.net with LMTP id SKLFBMV8xV68GwAAyH2SIw ; Wed, 20 May 2020 14:53:57 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp5.gate.ord1d.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dkim=fail (signature verification failed) header.d=mxes.net; dmarc=none (p=nil; dis=none) header.from=jeremyevans.net X-Suspicious-Flag: YES X-Classification-ID: 410c5772-9acb-11ea-bfab-525400d73c44-1-1 Received: from [216.105.38.7] ([216.105.38.7:49122] helo=lists.sourceforge.net) by smtp5.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id AC/CB-20291-3CC75CE5; Wed, 20 May 2020 14:53:56 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1jbTpm-0007Q7-KL; Wed, 20 May 2020 18:52:54 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jbTpl-0007Pw-CD for openvpn-devel@lists.sourceforge.net; Wed, 20 May 2020 18:52:53 +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:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=6G36FJXr+RUOKFtKI9ERi61Bic4lfcNAly678nkS/oA=; b=XiKzyIf/axa7tHt/59eP6u8AbB +1SwMlukNyRu8BTXwtUjOZj7xpak/UbsZHPTVPbEultn7i1DJgY7NmR5eVE9f/hk6kGsb9sOd+Cq8 rTnDgEhWkk3dHqHGSvgTuYOGllqv6F9Ii/++4zIIZOaTEK2IWrvpO/8zNGs6fUZit4eY=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject:To:From: Sender:Reply-To:Cc:Content-Type:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=6G36FJXr+RUOKFtKI9ERi61Bic4lfcNAly678nkS/oA=; b=J G5/boNVbOB0mrE6ZGIWdyXv5elEeVIj/GU0+Qyp8a8Kr6P5bwWLX1qI5P8uo2jQt+AcxMqHcWT49T KduR2sOUKG3S7Ok6j4bXvsb60JAX2nd5iqpp+B1amoHfjWkM3DLASjnKtxcruEsneNDMK0C4Mt1lL gOb9ty7Vuf6s99SI=; Received: from fbo-3.mxes.net ([198.205.123.65]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1jbTph-00GxwP-Hi for openvpn-devel@lists.sourceforge.net; Wed, 20 May 2020 18:52:53 +0000 Received: from smtp-out-4.mxes.net (smtp-out-4.mxes.net [198.205.123.69]) by fbi-3.mxes.net (Postfix) with ESMTP id 25AA5759A1 for ; Wed, 20 May 2020 14:34:18 -0400 (EDT) Received: from Customer-MUA (mua.mxes.net [10.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp.mxes.net (Postfix) with ESMTPSA id 09A1F75964 for ; Wed, 20 May 2020 14:34:04 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mxes.net; s=mta; t=1589999645; bh=xBThkSaTkUDBzm99jHXTlGHS+rUVWBcPT3yxo3aTg9I=; h=From:To:Subject:Date:Message-Id:MIME-Version; b=YQcObp48mJj3d94nJtskS+qYOLSfxlRk3t5750sfoZtU9bhL9+EL4BeSuhXnQBNsS vMEr+/0LR2F1pG4IMIv09B8I59UzvV1bwJgRbYqMNAB5GpcMSHY78c5mSMnM+qOVn5 +7SxbbWtSeU1pv9p8dK0LP+/8HJ+YxCIK5NIj3ec= Received: from localhost (battleground.jeremyevans.local [local]) by battleground.jeremyevans.local (OpenSMTPD) with ESMTPA id 8cdb4b2e for ; Wed, 20 May 2020 11:34:04 -0700 (PDT) From: Jeremy Evans To: openvpn-devel@lists.sourceforge.net Date: Wed, 20 May 2020 11:34:04 -0700 Message-Id: <20200520183404.54822-1-code@jeremyevans.net> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-Sent-To: X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 SPF_HELO_PASS SPF: HELO matches SPF record 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid 0.0 UNPARSEABLE_RELAY Informational: message has unparseable relay lines 0.1 DKIM_INVALID DKIM or DK signature exists, but is not valid X-Headers-End: 1jbTph-00GxwP-Hi Subject: [Openvpn-devel] [PATCH] Switch assertion failure to returning false 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 This assertion failure can be hit in production, which causes the openvpn server process to stop and all clients to be disconnected. Bug #1270 has been filed for this issue on Trac by another user who has experienced the issue, and this patch attempts to address it. Tracing callers, it appears that some callers check ks->authenticated before calling, but others do not. It may be possible to add the check for the callers that do not check, but this seems to be a simpler solution. To give some background, we hit this assertion failure, with the following log output: ``` Tue May 19 15:57:05 2020 username/73.135.141.11:1194 PUSH: Received control message: 'PUSH_REQUEST' Tue May 19 15:57:05 2020 username/73.135.141.11:1194 SENT CONTROL [username]: 'PUSH_REPLY,redirect-gateway def1,comp-lzo,persist-key,persist-tun,route-gateway 10.28.47.1,topology subnet,ping 10,ping-restart 120,ifconfig 10.28.47.38 255.255.255.0,peer-id 89' (status=1) Tue May 19 15:57:05 2020 username/73.135.141.11:1194 Assertion failed at /path/to/openvpn-2.4.7/src/openvpn/ssl.c:1944 (ks->authenticated) Tue May 19 15:57:05 2020 username/73.135.141.11:1194 Exiting due to fatal error Tue May 19 15:57:05 2020 username/73.135.141.11:1194 Closing TUN/TAP interface ``` using the following OpenVPN server configuration: ``` port 1194 proto udp dev-type tun ca ca.crt cert server.crt key server.key dh dh.pem topology subnet push "redirect-gateway def1" push "comp-lzo" push "persist-key" push "persist-tun" keepalive 10 120 comp-lzo user nobody group nobody persist-key persist-tun cd /home/openvpn/server chroot /var/empty daemon verb 3 crl-verify crl.pem tls-auth ta.key 0 cipher AES-256-CBC tls-version-min 1.2 tls-cipher ECDHE-RSA-AES256-GCM-SHA384 ncp-disable mute-replay-warnings script-security 3 auth-user-pass-verify "ldap-auth/ldap-auth" via-env auth-user-pass-optional ``` and the following command line options: ``` --config openvpn.conf --dev tun1 --local 206.131.72.52 \ --log-append openvpn.log --status openvpn-status.log \ --server 10.28.47.0 255.255.255.0 ``` The failed assertion is inside the function `tls_session_generate_data_channel_keys`, which is called 3 other places in `ssl.c.`: * `key_method_2_write`: checks for `ks->authenticated` before calling * `key_method_2_read`: appears to run in client mode but not in server mode * `tls_session_update_crypto_params`: runs in server mode and does not check before calling That leads me to believe the problem caller is `tls_session_update_crypto_params`. There.s three callers of `tls_session_update_crypto_params`:. * `incoming_push_message` (`push.c`): Probably this caller, since the server pushes configuration to clients, and the log shows the assertion failure right after the push reply. * `multi_process_file_closed` (`multi.c`): Not this caller. NCP is disabled in config, and async push was not enabled when compiling. * `do_deferred_options` (`init.c`): Not this caller. The server configuration doesn't pull. Changing the assertion to returning false appears to be the simplest fix. Another approach would be changing callers to check `ks->authenticated` before calling, either `tls_session_update_crypto_params` or `incoming_push_message`. Signed-off-by: Jeremy Evans Acked-by: Steffan Karger --- src/openvpn/ssl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index c2e9a4f3..c8b3a959 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1930,7 +1930,10 @@ tls_session_generate_data_channel_keys(struct tls_session *session) const struct session_id *server_sid = !session->opt->server ? &ks->session_id_remote : &session->session_id; - ASSERT(ks->authenticated); + if (!ks->authenticated) { + msg(D_TLS_ERRORS, "TLS Error: key_state not authenticated"); + goto cleanup; + } ks->crypto_options.flags = session->opt->crypto_flags; if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi,