From patchwork Fri Sep 3 23:56:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Quartulli X-Patchwork-Id: 1936 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id eLZbBxdDM2EFLwAAIUCqbw (envelope-from ) for ; Sat, 04 Sep 2021 05:57:43 -0400 Received: from proxy6.mail.ord1d.rsapps.net ([172.30.191.6]) by director11.mail.ord1d.rsapps.net with LMTP id EFoMBxdDM2GRDwAAvGGmqA (envelope-from ) for ; Sat, 04 Sep 2021 05:57:43 -0400 Received: from smtp22.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy6.mail.ord1d.rsapps.net with LMTPS id IDfFBhdDM2GuVwAAQyIf0w (envelope-from ) for ; Sat, 04 Sep 2021 05:57:43 -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: smtp22.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=unstable.cc X-Suspicious-Flag: YES X-Classification-ID: 8bbd7f44-0d66-11ec-b706-a0369f0d84d2-1-1 Received: from [216.105.38.7] ([216.105.38.7:51538] helo=lists.sourceforge.net) by smtp22.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 45/39-32712-61343316; Sat, 04 Sep 2021 05:57:42 -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.92.3) (envelope-from ) id 1mMSQ2-0000fS-7T; Sat, 04 Sep 2021 09:57:02 +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.92.3) (envelope-from ) id 1mMSPq-0000eS-KT for openvpn-devel@lists.sourceforge.net; Sat, 04 Sep 2021 09:56:50 +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=2E8rdM7FRHBID/qNC3Apmos3icu17oT+u2FYXmG/7Mk=; b=LbMo8G6SOyO9Q5TBv8fR497/WD 7tF/u92syGNPWIRWEY3Wl0HegXVIc25hVNCIyqB+ti27D3nZUBug4uwr9JkT/9UZrl/hXPLhMXLt4 VTqJv0oleT3GTfOPEFPSsBWyJpWl8ndwRisX604EatvwcKJWQcmdnQTmH6J/zjKlpNUs=; 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=2E8rdM7FRHBID/qNC3Apmos3icu17oT+u2FYXmG/7Mk=; b=VpkkOX2vL/Kvtz5YGiXmwJgFsS QUl4R6s3qkhzbI91+KS+cbld11fWeLC1td8S0GJnNiKCLls6doIr745B2Brs6Ymu/lVoORrz9v7S6 y1Wi/u6FpfgtBThxsVwTrzYuhurQGylTP/xR+BNTDU5jonUbL57+C+r9PAw8et4ykNME=; Received: from s2.neomailbox.net ([5.148.176.60]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) id 1mMSPm-0006IS-LC for openvpn-devel@lists.sourceforge.net; Sat, 04 Sep 2021 09:56:50 +0000 From: Antonio Quartulli To: openvpn-devel@lists.sourceforge.net Date: Sat, 4 Sep 2021 11:56:26 +0200 Message-Id: <20210904095629.6273-5-a@unstable.cc> In-Reply-To: <20210904095629.6273-1-a@unstable.cc> References: <20210904095629.6273-1-a@unstable.cc> MIME-Version: 1.0 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: unstable.cc] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.2 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1mMSPm-0006IS-LC Subject: [Openvpn-devel] [PATCH 4/7] do not include --cipher value in data-ciphers 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: Antonio Quartulli Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox The --cipher option has been there since a while, but it became more and more confusing since the introduction of NCP (data cipher negotiation). The fallback cipher can now be specified via --data-cipher-fallback, while the list of accepted ciphers is specified via --data-ciphers. --cipher can still be used for compatibility reasons, but won't affect the cipher negotiation. Signed-off-by: Arne Schwabe Signed-off-by: Antonio Quartulli Acked-By: Arne Schwabe --- Changes.rst | 5 ++++ doc/man-sections/generic-options.rst | 2 ++ src/openvpn/options.c | 38 ++++++++++++++++------------ src/openvpn/ssl_ncp.c | 13 ++++++++++ src/openvpn/ssl_ncp.h | 8 ++++++ 5 files changed, 50 insertions(+), 16 deletions(-) diff --git a/Changes.rst b/Changes.rst index 65b838b9..f803b760 100644 --- a/Changes.rst +++ b/Changes.rst @@ -71,6 +71,11 @@ Deprecated features This option mainly served a role as debug option when NCP was first introduced. It should now no longer be necessary. +``--cipher`` argument is no longer included in ``--data-ciphers`` by default + Data cipher negotiation has been introduced in 2.4.0 and been significantly + improved in 2.5.0. The implicit fallback to the cipher specified in + ``--cipher`` has been removed. + Compression no longer enabled by default Unless an explicit compression option is specified in the configuration, ``--allow-compression`` defaults to ``no`` in OpeNVPN 2.6.0. diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst index a8d24572..8b26cd1a 100644 --- a/doc/man-sections/generic-options.rst +++ b/doc/man-sections/generic-options.rst @@ -66,6 +66,8 @@ which mode OpenVPN is configured as. - 2.5.x or lower: ``--allow-compression asym`` is automatically added to the configuration if no other compression options are present. + - 2.4.x or lower: The cipher in ``--cipher`` is appended to + ``--data-ciphers`` --config file Load additional config options from ``file`` where each line corresponds diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 21c76a69..88ac5bed 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3102,26 +3102,20 @@ options_postprocess_cipher(struct options *o) /* We still need to set the ciphername to BF-CBC since various other * parts of OpenVPN assert that the ciphername is set */ o->ciphername = "BF-CBC"; + + msg(M_INFO, "Note: --cipher is not set. OpenVPN versions before 2.6 " + "defaulted to BF-CBC as fallback when cipher negotiation " + "failed in this case. If you need this fallback please add " + "'--data-ciphers-fallback 'BF-CBC' to your configuration " + "and/or add BF-CBC to --data-ciphers."); } else if (!o->enable_ncp_fallback && !tls_item_in_cipher_list(o->ciphername, o->ncp_ciphers)) { - msg(M_WARN, "DEPRECATED OPTION: --cipher set to '%s' but missing in" - " --data-ciphers (%s). Future OpenVPN version will " - "ignore --cipher for cipher negotiations. " - "Add '%s' to --data-ciphers or change --cipher '%s' to " - "--data-ciphers-fallback '%s' to silence this warning.", - o->ciphername, o->ncp_ciphers, o->ciphername, - o->ciphername, o->ciphername); - o->enable_ncp_fallback = true; - - /* Append the --cipher to ncp_ciphers to allow it in NCP */ - size_t newlen = strlen(o->ncp_ciphers) + 1 + strlen(o->ciphername) + 1; - char *ncp_ciphers = gc_malloc(newlen, false, &o->gc); - - ASSERT(openvpn_snprintf(ncp_ciphers, newlen, "%s:%s", o->ncp_ciphers, - o->ciphername)); - o->ncp_ciphers = ncp_ciphers; + msg(M_WARN, "DEPRECATED OPTION: --cipher set to '%s' but missing in " + "--data-ciphers (%s). OpenVPN ignores --cipher for cipher " + "negotiations. ", + o->ciphername, o->ncp_ciphers); } } @@ -3146,6 +3140,18 @@ need_compatibility_before(const struct options *o, int version) static void options_set_backwards_compatible_options(struct options *o) { + /* Versions < 2.5.0 do need --cipher in the list of accepted ciphers. + * Version 2.4 might probably does not need it but NCP was not so + * good with 2.4 and ncp-disable might be more common on 2.4 peers. + * Only do this iif --cipher is not explicitly (BF-CBC). This is not + * 100% correct backwards compatible behaviour but 2.5 already behaved like + * this */ + if (o->ciphername && need_compatibility_before(o, 20500) + && !tls_item_in_cipher_list(o->ciphername, o->ncp_ciphers)) + { + append_cipher_to_ncp_list(o, o->ciphername); + } + /* Compression is deprecated and we do not want to announce support for it * by default anymore, additionally DCO breaks with compression. * diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index 6967e2bb..022a9dc3 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -172,6 +172,19 @@ mutate_ncp_cipher_list(const char *list, struct gc_arena *gc) return ret; } + +void +append_cipher_to_ncp_list(struct options *o, const char *ciphername) +{ + /* Append the --cipher to ncp_ciphers to allow it in NCP */ + size_t newlen = strlen(o->ncp_ciphers) + 1 + strlen(ciphername) + 1; + char *ncp_ciphers = gc_malloc(newlen, false, &o->gc); + + ASSERT(openvpn_snprintf(ncp_ciphers, newlen, "%s:%s", o->ncp_ciphers, + ciphername)); + o->ncp_ciphers = ncp_ciphers; +} + bool tls_item_in_cipher_list(const char *item, const char *list) { diff --git a/src/openvpn/ssl_ncp.h b/src/openvpn/ssl_ncp.h index 4a2601a2..09ddeb28 100644 --- a/src/openvpn/ssl_ncp.h +++ b/src/openvpn/ssl_ncp.h @@ -102,6 +102,14 @@ tls_peer_ncp_list(const char *peer_info, struct gc_arena *gc); char * mutate_ncp_cipher_list(const char *list, struct gc_arena *gc); +/** + * Appends the cipher specified by the ciphernamer parameter to to + * the o->ncp_ciphers list. + * @param o options struct to modify. Its gc is also used + * @param ciphername the ciphername to add + */ +void append_cipher_to_ncp_list(struct options *o, const char *ciphername); + /** * Return true iff item is present in the colon-separated zero-terminated * cipher list.