From patchwork Fri Mar 13 05:59:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lev Stipakov X-Patchwork-Id: 1041 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director12.mail.ord1d.rsapps.net ([172.31.255.6]) by backend30.mail.ord1d.rsapps.net with LMTP id GGXrCky8a16LUwAAIUCqbw for ; Fri, 13 Mar 2020 13:01:00 -0400 Received: from proxy12.mail.iad3b.rsapps.net ([172.31.255.6]) by director12.mail.ord1d.rsapps.net with LMTP id gHy6Aky8a16vLgAAIasKDg ; Fri, 13 Mar 2020 13:01:00 -0400 Received: from smtp9.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.iad3b.rsapps.net with LMTP id KDF4OEu8a17FQgAAEsW3lA ; Fri, 13 Mar 2020 13:00:59 -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: smtp9.gate.iad3b.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=gmail.com; dmarc=fail (p=none; dis=none) header.from=gmail.com X-Suspicious-Flag: YES X-Classification-ID: 35eb855c-654c-11ea-807f-525400f4d366-1-1 Received: from [216.105.38.7] ([216.105.38.7:54172] helo=lists.sourceforge.net) by smtp9.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 26/69-30495-A4CBB6E5; Fri, 13 Mar 2020 13:00:59 -0400 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.90_1) (envelope-from ) id 1jCnf6-0007hf-BL; Fri, 13 Mar 2020 16:59:52 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jCnf5-0007hS-8G for openvpn-devel@lists.sourceforge.net; Fri, 13 Mar 2020 16:59:51 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: MIME-Version:Content-Type:Content-Transfer-Encoding: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=GkBgqZb7Hu/f1H3gSuKFCX6vxt43X1nnzGwjkB/QwEc=; b=ew/0cJJcF/o8zUybERrDHOaoH/ hFNzbu8w3It+TWLkqbwZD2UsDVliu2wWE6uXLFBBL15dHgTA1t2B7WeY7AwPfXdT1l2pjuXpuaegx b3ME2kcpSNQK11tefDkBywWjqw4fQNW/+Vk+X5IHzW1kMLNA3grjSqQjuYcto5N1hx/Q=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:MIME-Version: Content-Type:Content-Transfer-Encoding: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=GkBgqZb7Hu/f1H3gSuKFCX6vxt43X1nnzGwjkB/QwEc=; b=Vs6A81Q297qCtFp6oQsqF+u+km mbbXfsJYtJBjfr6RdXPhJcKM9x5VUbqdE8sQ9HPnyNKLqfjAquCqKD4A8esp2RGcDo6B3OAZQGUlP fzV2d/gzh3Nea2twtNi+jV+WDLAwxnlcipuK2kXXkHfBJP0zS3l31H5K7fm+yUW++52o=; Received: from mail-qt1-f194.google.com ([209.85.160.194]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.92.2) id 1jCnf0-006j60-A4 for openvpn-devel@lists.sourceforge.net; Fri, 13 Mar 2020 16:59:51 +0000 Received: by mail-qt1-f194.google.com with SMTP id e20so8088821qto.5 for ; Fri, 13 Mar 2020 09:59:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=GkBgqZb7Hu/f1H3gSuKFCX6vxt43X1nnzGwjkB/QwEc=; b=P2ZjR7A5fmjOA8z1mmFFF5+XjUFP6/VM10XuFkbu3yTFmNwAvoxm5YFfR4RCiU4adZ B1r/KAGmZxgLsM/m0KuVGfER9qQiGvml+Z7uZomWDlQTClLzKdYxDLfflHa4yQby7LqR 2r+LpKwraMwL8YXGVOlvVUix7tJToXeF6uk/fiyXQOH/IQJL2P9l8LqjtrHDsZnXdtEg E0wZP4W7IBKxjigph1Cpe7CQhMWzUPpFJdzbKiAh+oXMA9lrOln8329jCxQ8CbB4BCXn nJ4wV520fiQj64jQPgEIiOFgNyG0jp8v2c+r6+EY+s47sSiHCf7uOFVQ/6vkaZXmNpV0 OkbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=GkBgqZb7Hu/f1H3gSuKFCX6vxt43X1nnzGwjkB/QwEc=; b=mLR1L7f8CXH2KNAuJOUqpXsGGM3MO1aIMvB04lBn6M9BdIwmv+L0pVDgwBhTokiDdq OpsVMJZ0sFv/ti6yq2ap4OycCCWxbOucv34SdRhpOJ8IDtcYIgJ5MECjokw9H2iPzSSc ZO9MjAcGZVWOYx44CjMMumca8SD8yvcZkFNklEudZrt8yeXPsvi1DFNn3IZLBCI2+C0n PaGh3wYSmLRtyAqg6+kukrRyS+ZnDtHuI/xbjH/ozvdBBZ0QDtD5r3rWn+G2a/5GV3Uc 4IKx6DLI2CIyTwEFkgr05/CAQx8xhzBfhfUcQ09S8hFtW32i4wmWK8tZ/1JUF/H0xdAu aH3A== X-Gm-Message-State: ANhLgQ0QXqD6BHUnru0cXGW6BpfLNOX9kxTvNUw57Xr/Cn42XQnvmF5n /9muevZzgL4xGEXkGRrCWGF1DaGu X-Google-Smtp-Source: ADFU+vuGcr9wf505osP98a+5AWNL8RSIzM5wrocuOzYH9R3n1ZU7nesew8aEZi0f6d9O2zP1sfEf+Q== X-Received: by 2002:ac8:2648:: with SMTP id v8mr13653052qtv.65.1584118779777; Fri, 13 Mar 2020 09:59:39 -0700 (PDT) Received: from LAPTOP-4L3N7KFS.localdomain (nat2.panoulu.net. [185.38.2.2]) by smtp.gmail.com with ESMTPSA id d141sm9436354qke.68.2020.03.13.09.59.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Mar 2020 09:59:39 -0700 (PDT) From: Lev Stipakov To: openvpn-devel@lists.sourceforge.net Date: Fri, 13 Mar 2020 18:59:13 +0200 Message-Id: <20200313165913.12682-1-lstipakov@gmail.com> X-Mailer: git-send-email 2.17.1 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (lstipakov[at]gmail.com) 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: openvpn.net] 0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3) [209.85.160.194 listed in wl.mailspike.net] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.160.194 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_VALID_AU Message has a valid DKIM or DK signature from author's 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 RCVD_IN_MSPIKE_WL Mailspike good senders 0.6 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1jCnf0-006j60-A4 Subject: [Openvpn-devel] [PATCH] Fix broken async push with NCP is used 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: , Cc: Lev Stipakov MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox From: Lev Stipakov With NCP and deferred auth, we perform cipher negotiation and generate data channel keys on incoming push request, assuming that auth succeeded. With async push, when auth succeeds in between push requests, we send push reply immediately. The code which generates data channel keys is only called on handling incoming push requests (incoming_push_message). It might not be called with NCP, deferred auth and async push because on incoming push request auth might not be complete yet. When auth is complete in between push requests, push reply is sent and it is assumed that connection is established. However, since data channel keys are not generated on the server side, connection doesn't work. Fix by generating data channel keys when async push is triggered. Reported-by: smaxfield@duosecurity.com Signed-off-by: Lev Stipakov --- src/openvpn/init.c | 6 ++---- src/openvpn/multi.c | 24 +++++++++++++++++++++++- src/openvpn/push.c | 6 ++---- src/openvpn/ssl.c | 6 ++++++ src/openvpn/ssl.h | 5 +++-- 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 824b6667..b3096dc8 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2343,10 +2343,8 @@ do_deferred_options(struct context *c, const unsigned int found) } #endif - /* Do not regenerate keys if server sends an extra push reply */ - if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized - && !tls_session_update_crypto_params(session, &c->options, &c->c2.frame, - frame_fragment)) + if (!tls_session_update_crypto_params(session, &c->options, &c->c2.frame, + frame_fragment)) { msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options"); return false; diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index da0aeeba..b42bcec9 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2137,8 +2137,30 @@ multi_process_file_closed(struct multi_context *m, const unsigned int mpp_flags) { if (mi) { - /* continue authentication and send push_reply */ + /* continue authentication, perform NCP negotiation and send push_reply */ multi_process_post(m, mi, mpp_flags); + + /* With NCP and deferred authentication, we perform cipher negotiation and + * data channel keys generation on incoming push request, assuming that auth + * succeeded. When auth succeeds in between push requests and async push is used, + * we send push reply immediately. Above multi_process_post() call performs + * NCP negotiation and here we do keys generation. */ + + struct context *c = &mi->context; + struct frame *frame_fragment = NULL; +#ifdef ENABLE_FRAGMENT + if (c->options.ce.fragment) + { + frame_fragment = &c->c2.frame_fragment; + } +#endif + struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE]; + if (!tls_session_update_crypto_params(session, &c->options, + &c->c2.frame, frame_fragment)) + { + msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed"); + register_signal(c, SIGUSR1, "init-data-channel-failed"); + } } else { diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 71f22e94..aef00d34 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -298,10 +298,8 @@ incoming_push_message(struct context *c, const struct buffer *buffer) } #endif struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE]; - /* Do not regenerate keys if client send a second push request */ - if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized - && !tls_session_update_crypto_params(session, &c->options, - &c->c2.frame, frame_fragment)) + if (!tls_session_update_crypto_params(session, &c->options, + &c->c2.frame, frame_fragment)) { msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed"); goto error; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index e21279f1..56d0576a 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1956,6 +1956,12 @@ tls_session_update_crypto_params(struct tls_session *session, struct options *options, struct frame *frame, struct frame *frame_fragment) { + if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized) + { + /* keys already generated, nothing to do */ + return true; + } + if (!session->opt->server && 0 != strcmp(options->ciphername, session->opt->config_ciphername) && !tls_item_in_cipher_list(options->ciphername, options->ncp_ciphers)) diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 3449d91e..f0a8ef54 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -474,7 +474,8 @@ void tls_update_remote_addr(struct tls_multi *multi, /** * Update TLS session crypto parameters (cipher and auth) and derive data - * channel keys based on the supplied options. + * channel keys based on the supplied options. Does nothing if keys are already + * generated. * * @param session The TLS session to update. * @param options The options to use when updating session. @@ -482,7 +483,7 @@ void tls_update_remote_addr(struct tls_multi *multi, * adjusted based on the selected cipher/auth). * @param frame_fragment The fragment frame options. * - * @return true if updating succeeded, false otherwise. + * @return true if updating succeeded or keys are already generated, false otherwise. */ bool tls_session_update_crypto_params(struct tls_session *session, struct options *options,