From patchwork Thu May 20 05:11:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1829 X-Patchwork-Delegate: a@unstable.cc Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director14.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net with LMTP id WMD4G3R8pmASbwAAIUCqbw (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from proxy12.mail.ord1d.rsapps.net ([172.30.191.6]) by director14.mail.ord1d.rsapps.net with LMTP id sAK0G3R8pmB7HwAAeJ7fFg (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -0400 Received: from smtp9.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy12.mail.ord1d.rsapps.net with LMTPS id cNthG3R8pmCICAAA7PHxkg (envelope-from ) for ; Thu, 20 May 2021 11:12:52 -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.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: d7d9ba2e-b97d-11eb-8dbd-0026b95bddb7-1-1 Received: from [216.105.38.7] ([216.105.38.7:34366] helo=lists.sourceforge.net) by smtp9.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 7F/0A-18268-37C76A06; Thu, 20 May 2021 11:12:51 -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 1ljkL9-0005kf-GV; Thu, 20 May 2021 15:11:59 +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 1ljkL7-0005k1-Bj for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:57 +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: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:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=LVZZbUlYJR2WrKM10Tmn2MUIBZ8VdsdV06DeGMok08Y=; b=jF7QGlOvg12tANrm5oyn1MSM/d yuvnLw3kbL8nDJWWdG2RO3fEGi9YrGC+nURDQ6/gHPVlUY/4gKzxvVUGXL8WyzknaH3EbNgPUk+60 gpOj8kx3yxTAOKS6FeBUvHzydoVExq00NO8RGsgudNTeC78jhbUHM7QqH7WHpKLPPtpU=; 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: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:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=LVZZbUlYJR2WrKM10Tmn2MUIBZ8VdsdV06DeGMok08Y=; b=mXAnUWRfutkZPUJz5TpXeNRChY BTM9hHVqV4HukrZYHiHR240ToLp6BL/HwWcQnd4vSs4ItwUYgJ7xfzR+V7prDh0oCIvU+YGyRVKub zJrgGKmrlwK/3O4DmpoqE//QnJ0LyKpZ6oYBFwoLP//lEyB1EFid9CtRq0Ny4u3F8KaY=; 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 1ljkL5-00GeSo-PQ for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 15:11:57 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94.2 (FreeBSD)) (envelope-from ) id 1ljkKy-000C2Z-OZ for openvpn-devel@lists.sourceforge.net; Thu, 20 May 2021 17:11:48 +0200 Received: (nullmailer pid 2565647 invoked by uid 10006); Thu, 20 May 2021 15:11:49 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Thu, 20 May 2021 17:11:47 +0200 Message-Id: <20210520151148.2565578-8-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210520151148.2565578-1-arne@rfc2549.org> References: <20210520151148.2565578-1-arne@rfc2549.org> 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: 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: 1ljkL5-00GeSo-PQ Subject: [Openvpn-devel] [PATCH v2 8/9] Remove --ncp-disable option 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 NCP has proven to be stable and apart from the one VPN Provider doing hacky things with homebrewed NCP we have not had any reports about ncp-disable being required. Remove ncp-disable to simplify code paths. Note: This patch breaks client without --pull. The follow up patch for P2P NCP will restore that. But to avoid all the NCP/non-NCP special cases to be implemented in P2P. P2P will directly switch from always non-NCP to always NCP. Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- Changes.rst | 4 +++ doc/man-sections/protocol-options.rst | 8 ++---- src/openvpn/init.c | 17 ++++--------- src/openvpn/multi.c | 4 --- src/openvpn/options.c | 36 +++------------------------ src/openvpn/options.h | 1 - src/openvpn/ssl.c | 3 +-- src/openvpn/ssl_common.h | 1 - src/openvpn/ssl_ncp.c | 4 --- 9 files changed, 16 insertions(+), 62 deletions(-) diff --git a/Changes.rst b/Changes.rst index 9185b55f7..e7ae6abed 100644 --- a/Changes.rst +++ b/Changes.rst @@ -57,6 +57,10 @@ Deprecated features is considered "too complicated", using ``--peer-fingerprint`` makes TLS mode about as easy as using ``--secret``. +``ncp-disable`` has been removed + This option mainly served a role as debug option when NCP was first + introduced. It should now no longer be necessary. + Overview of changes in 2.5 ========================== diff --git a/doc/man-sections/protocol-options.rst b/doc/man-sections/protocol-options.rst index 34d4255ee..5ae780e1f 100644 --- a/doc/man-sections/protocol-options.rst +++ b/doc/man-sections/protocol-options.rst @@ -65,8 +65,8 @@ configured in a compatible way between both the local and remote side. The default is :code:`BF-CBC`, an abbreviation for Blowfish in Cipher Block Chaining mode. When cipher negotiation (NCP) is allowed, OpenVPN 2.4 and newer on both client and server side will automatically - upgrade to :code:`AES-256-GCM`. See ``--data-ciphers`` and - ``--ncp-disable`` for more details on NCP. + upgrade to :code:`AES-256-GCM`. See ``--data-ciphers`` for more details + on NCP. Using :code:`BF-CBC` is no longer recommended, because of its 64-bit block size. This small block size allows attacks based on collisions, as @@ -235,10 +235,6 @@ configured in a compatible way between both the local and remote side. have been configured with `--enable-small` (typically used on routers or other embedded devices). ---ncp-disable - **DEPRECATED** Disable "Negotiable Crypto Parameters". This completely - disables cipher negotiation. - --secret args **DEPRECATED** Enable Static Key encryption mode (non-TLS). Use pre-shared secret ``file`` which was generated with ``--genkey``. diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 4335d4870..38abf9f3a 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2227,18 +2227,14 @@ pull_permission_mask(const struct context *c) | OPT_P_EXPLICIT_NOTIFY | OPT_P_ECHO | OPT_P_PULL_MODE - | OPT_P_PEER_ID; + | OPT_P_PEER_ID + | OPT_P_NCP; if (!c->options.route_nopull) { flags |= (OPT_P_ROUTE | OPT_P_IPWIN32); } - if (c->options.ncp_enabled) - { - flags |= OPT_P_NCP; - } - return flags; } @@ -2720,8 +2716,6 @@ do_init_crypto_tls_c1(struct context *c) * * Therefore, the key structure has to be initialized when: * - any non-BF-CBC cipher was selected; or - * - BF-CBC is selected and NCP is disabled (explicit request to - * use the BF-CBC cipher); or * - BF-CBC is selected, NCP is enabled and fallback is enabled * (BF-CBC will be the fallback). * - BF-CBC is in data-ciphers and we negotiate to use BF-CBC: @@ -2731,12 +2725,12 @@ do_init_crypto_tls_c1(struct context *c) * Note that BF-CBC will still be part of the OCC string to retain * backwards compatibility with older clients. */ - if (!streq(options->ciphername, "BF-CBC") || !options->ncp_enabled - || (options->ncp_enabled && tls_item_in_cipher_list("BF-CBC", options->ncp_ciphers)) + if (!streq(options->ciphername, "BF-CBC") + || tls_item_in_cipher_list("BF-CBC", options->ncp_ciphers) || options->enable_ncp_fallback) { /* Do not warn if the if the cipher is used only in OCC */ - bool warn = !options->ncp_enabled || options->enable_ncp_fallback; + bool warn = options->enable_ncp_fallback; init_key_type(&c->c1.ks.key_type, options->ciphername, options->authname, true, warn); } @@ -2838,7 +2832,6 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.tcp_mode = link_socket_proto_connection_oriented(options->ce.proto); to.config_ciphername = c->options.ciphername; to.config_ncp_ciphers = c->options.ncp_ciphers; - to.ncp_enabled = options->ncp_enabled; to.transition_window = options->transition_window; to.handshake_window = options->handshake_window; to.packet_timeout = options->tls_timeout; diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 7cb9e86aa..2507108e2 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1791,10 +1791,6 @@ multi_client_set_protocol_options(struct context *c) #endif /* Select cipher if client supports Negotiable Crypto Parameters */ - if (!o->ncp_enabled) - { - return true; - } /* if we have already created our key, we cannot *change* our own * cipher -> so log the fact and push the "what we have now" cipher diff --git a/src/openvpn/options.c b/src/openvpn/options.c index fa3ee50d6..2f4ccaa09 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -526,7 +526,6 @@ static const char usage_message[] = " (default=%s).\n" " Set alg=none to disable encryption.\n" "--data-ciphers list : List of ciphers that are allowed to be negotiated.\n" - "--ncp-disable : (DEPRECATED) Disable cipher negotiation.\n" "--prng alg [nsl] : For PRNG, use digest algorithm alg, and\n" " nonce_secret_len=nsl. Set alg=none to disable PRNG.\n" #ifndef ENABLE_CRYPTO_MBEDTLS @@ -843,7 +842,6 @@ init_options(struct options *o, const bool init_gc) o->stale_routes_check_interval = 0; o->ifconfig_pool_persist_refresh_freq = 600; o->scheduled_exit_interval = 5; - o->ncp_enabled = true; o->ncp_ciphers = "AES-256-GCM:AES-128-GCM"; o->authname = "SHA1"; o->prng_hash = "SHA1"; @@ -1719,7 +1717,6 @@ show_settings(const struct options *o) SHOW_STR_INLINE(shared_secret_file); SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false, true), "%s"); SHOW_STR(ciphername); - SHOW_BOOL(ncp_enabled); SHOW_STR(ncp_ciphers); SHOW_STR(authname); SHOW_STR(prng_hash); @@ -3082,7 +3079,6 @@ options_postprocess_cipher(struct options *o) if (!o->pull && !(o->mode == MODE_SERVER)) { /* we are in the classic P2P mode */ - o->ncp_enabled = false; msg( M_WARN, "Cipher negotiation is disabled since neither " "P2MP client nor server mode is enabled"); @@ -3099,18 +3095,6 @@ options_postprocess_cipher(struct options *o) /* pull or P2MP mode */ if (!o->ciphername) { - if (!o->ncp_enabled) - { - msg(M_USAGE, "--ncp-disable needs an explicit --cipher or " - "--data-ciphers-fallback config option"); - } - - msg(M_WARN, "--cipher is not set. Previous OpenVPN version 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."); - /* 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"; @@ -3152,13 +3136,10 @@ options_postprocess_mutate(struct options *o) options_postprocess_cipher(o); options_postprocess_mutate_invariant(o); - if (o->ncp_enabled) + o->ncp_ciphers = mutate_ncp_cipher_list(o->ncp_ciphers, &o->gc); + if (o->ncp_ciphers == NULL) { - o->ncp_ciphers = mutate_ncp_cipher_list(o->ncp_ciphers, &o->gc); - if (o->ncp_ciphers == NULL) - { - msg(M_USAGE, "NCP cipher list contains unsupported ciphers or is too long."); - } + msg(M_USAGE, "NCP cipher list contains unsupported ciphers or is too long."); } if (o->remote_list && !o->connection_list) @@ -3908,8 +3889,7 @@ options_string(const struct options *o, } /* Only announce the cipher to our peer if we are willing to * support it */ - if (p2p_nopull || !o->ncp_enabled - || tls_item_in_cipher_list(ciphername, o->ncp_ciphers)) + if (p2p_nopull || tls_item_in_cipher_list(ciphername, o->ncp_ciphers)) { buf_printf(&out, ",cipher %s", ciphername); } @@ -7994,14 +7974,6 @@ add_option(struct options *options, msg(msglevel, "Unknown key-derivation method %s", p[1]); } } - else if (streq(p[0], "ncp-disable") && !p[1]) - { - VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE); - options->ncp_enabled = false; - msg(M_WARN, "DEPRECATED OPTION: ncp-disable. Disabling " - "cipher negotiation is a deprecated debug feature that " - "will be removed in OpenVPN 2.6"); - } else if (streq(p[0], "prng") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_GENERAL); diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 41e84f7e1..69897c5a7 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -512,7 +512,6 @@ struct options const char *ciphername; bool enable_ncp_fallback; /**< If defined fall back to * ciphername if NCP fails */ - bool ncp_enabled; const char *ncp_ciphers; const char *authname; const char *prng_hash; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index b28d8edf8..dd6e462d0 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2187,8 +2187,7 @@ push_peer_info(struct buffer *buf, struct tls_session *session) } /* support for Negotiable Crypto Parameters */ - if (session->opt->ncp_enabled - && (session->opt->mode == MODE_SERVER || session->opt->pull)) + if (session->opt->mode == MODE_SERVER || session->opt->pull) { if (tls_item_in_cipher_list("AES-128-GCM", session->opt->config_ncp_ciphers) && tls_item_in_cipher_list("AES-256-GCM", session->opt->config_ncp_ciphers)) diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 928e80929..43af51ee9 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -324,7 +324,6 @@ struct tls_options const char *config_ciphername; const char *config_ncp_ciphers; - bool ncp_enabled; bool tls_crypt_v2; const char *tls_crypt_v2_verify_script; diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index f02a3103c..722256b42 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -289,10 +289,6 @@ check_pull_client_ncp(struct context *c, const int found) return true; } - if (!c->options.ncp_enabled) - { - return true; - } /* If the server did not push a --cipher, we will switch to the * remote cipher if it is in our ncp-ciphers list */ if(tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername))