From patchwork Thu Jul 9 00:16:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1221 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director7.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id wABrEKfuBl+hVAAAIUCqbw for ; Thu, 09 Jul 2020 06:17:11 -0400 Received: from proxy7.mail.ord1d.rsapps.net ([172.30.191.6]) by director7.mail.ord1d.rsapps.net with LMTP id AMJQEKfuBl+tMwAAovjBpQ ; Thu, 09 Jul 2020 06:17:11 -0400 Received: from smtp8.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy7.mail.ord1d.rsapps.net with LMTP id SO21D6fuBl9qAwAAMe1Fpw ; Thu, 09 Jul 2020 06:17:11 -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: smtp8.gate.ord1c.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; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: 5980896c-c1cd-11ea-8c46-782bcb03304b-1-1 Received: from [216.105.38.7] ([216.105.38.7:46050] helo=lists.sourceforge.net) by smtp8.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 8D/D3-11983-6AEE60F5; Thu, 09 Jul 2020 06:17:10 -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 1jtTbU-0004Y9-GF; Thu, 09 Jul 2020 10:16:32 +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 1jtTbG-0004VV-OF for openvpn-devel@lists.sourceforge.net; Thu, 09 Jul 2020 10:16:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:To: From:Sender:Reply-To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: 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=CMebTfevPBTD+TeqTdVCXWJPs1d9XSn8Oj4HSOFz3D8=; b=aR1NSNFElyRomf18fL/xlAB8E4 kEyRwnxHeMwoc1kAELh8cLvf7/ZY+AmPQOwoyCH1WxwvEFXtypKj+rh0w0L3ML+EYwux9eMo9K8pw jmRPj3lT5udkFyDhf10DP48BhXPpGikqqKL1xpWO/8jF5MTTvPqgMFctCvOtck5Sa92k=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc :MIME-Version:Content-Type:Content-Transfer-Encoding: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=CMebTfevPBTD+TeqTdVCXWJPs1d9XSn8Oj4HSOFz3D8=; b=j79jVOs8w7ofV4Vt4FwOK06ftc nQwR9WKcvlepSHIvSTQu6OC/I3VNaKwLvXdw18Pkc5Xt+8udeCg3ebotOyD66AvEUr7t08YJ02X/L UyZv5sRu+wDkozbuw1i6JtYiC0lDZsl4bPI2AinXh0NKRVWYIsJ0J35CtINsUw1+wSGA=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1jtTbF-000ggd-Md for openvpn-devel@lists.sourceforge.net; Thu, 09 Jul 2020 10:16:18 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.92.3 (FreeBSD)) (envelope-from ) id 1jtTb1-000HY0-Tv for openvpn-devel@lists.sourceforge.net; Thu, 09 Jul 2020 12:16:03 +0200 Received: (nullmailer pid 12004 invoked by uid 10006); Thu, 09 Jul 2020 10:16:03 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 9 Jul 2020 12:16:01 +0200 Message-Id: <20200709101603.11941-6-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200709101603.11941-1-arne@rfc2549.org> References: <20200709101603.11941-1-arne@rfc2549.org> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 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: rfc2549.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1jtTbF-000ggd-Md Subject: [Openvpn-devel] [PATCH 6/8] Cleanup: Remove special case code for old poor man's NCP. 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: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox Ever since the NCPv2 the ncp_get_best_cipher uses the global options->ncp_enabled option and ignore the tls_session->ncp_enabled option. The server side's poor man's NCP is implemented as seeing the list of supported ciphers from the peer as just one cipher so this special handling for poor man's NCP of the older NCP here is not needed anymore. Theoretically we can now get rid of tls_session->ncp_enabled but doing so requires more refactoring since options is not available in the methods that still use it. And when we remove ncp-disable the variable will be removed anyway. This commit moves the data channel key generation for the corner case of a client not supporting NCP but having the same cipher as the server to the same function that also generates data channel keys for NCP and poort man's NCP. This has an unintended side effect of changing the calculated frame size for this special case. The old path did call tls_session_update_crypto_params. To avoid this change in behaviour, this patch adds a hacky workaround for this. A proper solution for this needs still be found but this allows the patch set to be merged. Document the remaining usage of tls_poor_mans_ncp better. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/init.c | 2 ++ src/openvpn/ssl.c | 21 +++++++-------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 91b919d5..e9c01629 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2376,6 +2376,8 @@ do_deferred_options(struct context *c, const unsigned int found) } else if (c->options.ncp_enabled) { + /* If the server did not push a --cipher, we will switch to the + * remote cipher if it is in our ncp-ciphers list */ tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername); } struct frame *frame_fragment = NULL; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index f3fe0ecf..668bcbd9 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1986,6 +1986,12 @@ tls_session_update_crypto_params(struct tls_session *session, options->keysize = 0; } } + else + { + /* Very hacky workaround and quick fix for our calculation + * not correct to avoid a regression */ + return tls_session_generate_data_channel_keys(session); + } init_key_type(&session->opt->key_type, options->ciphername, options->authname, options->keysize, true, true); @@ -2463,8 +2469,7 @@ key_method_2_write(struct buffer *buf, struct tls_session *session) * generation is postponed until after the pull/push, so we can process pushed * cipher directives. */ - if (session->opt->server && !(session->opt->ncp_enabled - && session->opt->mode == MODE_SERVER && ks->key_id <= 0)) + if (session->opt->server && !(session->opt->mode == MODE_SERVER && ks->key_id <= 0)) { if (ks->authenticated > KS_AUTH_FALSE) { @@ -2616,18 +2621,6 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio multi->remote_ciphername = options_string_extract_option(options, "cipher", NULL); - if (!tls_peer_supports_ncp(multi->peer_info)) - { - /* Peer does not support NCP, but leave NCP enabled if the local and - * remote cipher do not match to attempt 'poor-man's NCP'. - */ - if (multi->remote_ciphername == NULL - || 0 == strcmp(multi->remote_ciphername, multi->opt.config_ciphername)) - { - session->opt->ncp_enabled = false; - } - } - if (tls_session_user_pass_enabled(session)) { /* Perform username/password authentication */