From patchwork Tue Dec 10 12:18:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "flichtenheld (Code Review)" X-Patchwork-Id: 3983 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:750c:b0:5e7:b9eb:58e8 with SMTP id r12csp71998mai; Tue, 10 Dec 2024 04:18:27 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCVZ+MVyuWWr0z04lqkoHhO8fRA82O7bUCEGXJlddedV2VRpU3zhSeoAbkTTVu0ZYs0bSCbTgA/84Ig=@openvpn.net X-Google-Smtp-Source: AGHT+IFCL/cax21RLs0AH5wkhmYiIllV7XGtNPCbF6eNxjJu+mOcsM47XsPah5HvRKfmxoGtohqM X-Received: by 2002:a05:6870:1656:b0:29e:7c10:1afe with SMTP id 586e51a60fabf-29f734cf6edmr13182107fac.30.1733833107555; Tue, 10 Dec 2024 04:18:27 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1733833107; cv=none; d=google.com; s=arc-20240605; b=LaAzGZCme0Nmr14bohWmwIY6v6fdecXhj9L73wjZk+talfbNOpgfGO1oo1QBE6XM9z RxUaa35g63kUI3f0ocz/83X4uR6gObFJCgij7iD5Ogf5ccJ31veq8ebGUb1XcAgZKg45 51frGYQLzCbiQ7OUThencU1f4+i46Cyc9MDpiDMIHFSLhEleHMK6SQ5njtWNyZZ+2ty+ GsObZtkWBozRW1hHE4glwKyZgQ0vYhNizLz8uCuMtzlb/SYVZV29sfy2E3y6Egc+LPUH f7u+f1NccvGA18q+xFhtF455v/qvrUzeZXD0ba5q2aCm1nQ4lrJad49X5FSrBCsGVmL7 csrg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=I6D4kjPzqbQ4k+rnxUFQOFDg1DXKgMg+CmGKaRj+AJ8=; fh=lm0MLPW7DntlrDqRECIiC9JlE1uPxhepE0URYHIf+eE=; b=JivuoIZVpsTzPduYD83fO3vtWQmW9LaaVMMNWt9ARPAlofDx9suBN+VxNjoN3TzA8M kiEUGHiRaqAe3RX7bhheCyUriDZhnUeSKi5cm5t0KknO9KU+Kl7Qh08ZGro7mSzrYqn4 a5JZQVD/OFgaTAlqoA1HH4yo9kPkAzuoEvZ35mkkFMEXh4mQOlXSFU/Kddd8BiV9B9xV gTyVIguVAHRBo5zZBbTTwAANsMHagNtCoo6JHcH1o0S5GZwLAVpZbGqagzwTwJRL8qE3 htaeWRQcTcrNPem0snsc8xm3leDQVD8ARiFE80kkFRC4K3ZhHG6/uPH6N08ibjeReG/6 zQlQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=EMUJvS7h; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=dZxUKgop; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=gwSuSAhA; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 586e51a60fabf-29f8a25b7ecsi4310549fac.45.2024.12.10.04.18.27 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Dec 2024 04:18:27 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=EMUJvS7h; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=dZxUKgop; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=gwSuSAhA; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1tKzC1-0006uV-Fn; Tue, 10 Dec 2024 12:18:20 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-3.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1tKzBz-0006uM-TP for openvpn-devel@lists.sourceforge.net; Tue, 10 Dec 2024 12:18:19 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=rWT5h3i5ifnDPOPIRNvmeOYMjjDndF8PFp5BlXKaMG8=; b=EMUJvS7hO33YD5AV2AAhk9rkeQ hXuWTgfMHfhRbymUMgAiBEEKQ8Dz89kRq1g3jcyvM7CYZmj+TaGmOUbHg0cs55H4iJKcy0GV7DyA0 GibvLkA+yT2GYYQIl9mb3H9ty7EG6kNI2iDH+M8YYFEHGLmLTw00p4S82tsFjX3Ebw+8=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=rWT5h3i5ifnDPOPIRNvmeOYMjjDndF8PFp5BlXKaMG8=; b=d ZxUKgop5sxVafXQgf3tHrd3ypQOdu9VFJcdox2xGJkDdKiVMBCRJOgRR7LxviCr0WU/E4gyRhJnk/ lJTfBcgzDraNXHvZz5QeiQ7exzVtrS21N8/vVTxBJIIv9XRoRlKFqRRczEnCcavECl81Lcj7WMPrr p7ujl8bn2jZ1Qrx4=; Received: from mail-ej1-f53.google.com ([209.85.218.53]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1tKzBw-0002vT-7p for openvpn-devel@lists.sourceforge.net; Tue, 10 Dec 2024 12:18:19 +0000 Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-aa67ac42819so396290666b.0 for ; Tue, 10 Dec 2024 04:18:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1733833089; x=1734437889; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=rWT5h3i5ifnDPOPIRNvmeOYMjjDndF8PFp5BlXKaMG8=; b=gwSuSAhACz1z3YP3N/xSwF6PmxP8CYXwEEHUo5C3k5FNbm6hOo+Ue3nLklpR7OnQvr eBTvPOnjSALfKBMZf5VQkIn9DsWlI+5fwI5ShcAYMTBNZBjSuMgGjILJq6qDZOoSjMIq H0kMy5UrgQlm1gsRr5Tp9PQqkjkRbCMEBC688caFo5bBko1QLxaytRpxEcqInCdwsD5j EV5mc5rVoJQ8orWJL5pHe9tLTEw4wd3TEs9m0Uk2Fz1ap49m9yKYwQFexUyw644rjgfy miXWDdDqMj50M8TMHpHYyALH7dwAQCTJ5ZRydGnJB9kJDmQSOddwrMWaf8rlVV8POW47 4mMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733833089; x=1734437889; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rWT5h3i5ifnDPOPIRNvmeOYMjjDndF8PFp5BlXKaMG8=; b=uCf0nIMnhDev/ylLsE3kS3m7BwnTmQKlxg+D2di1J55UkGZ1cBoCAMJJVpywYkcs5x R2F1OJO0mhazXeJMB52iqPXYaX7+xiCEGuGblJOWLmaUqjDLUFxH5aufEisygCqAgbDZ kwN5Jzi1yWPHwPSzX5wyYkp3BeCblqRCpCez56CIwjZTFuLcHFAoLjJTr83f0zSlfGkA uoC1mR9y6mS+sxsAKS1LZk88S64wSZCum7LeaXRCAIRaJaSEhwwt0CEBWf6DoqHUKjS3 IMRl+l5lICRXwlfJLpcqYUfj3O9Rd9kbj3YPjYU4bfca2XKWLcSoyL/oSvpfxRiupxzW b8OQ== X-Gm-Message-State: AOJu0YxQjJp9xyqiRVn38fYZfUDsQqfPCunCctuv3c1/u0yXv+00DLTD S8ymvDqcKUyo3GEqyPBN8E1/W2qJ8AsMWFTbS/N1/0Vhq1WkAFnXDh2KYNrrUkDncsm+12B/Nb9 7 X-Gm-Gg: ASbGncsrd33K97QHewc36pJBQb++LiEcbNamhyW/8Az0pjPp4SXDhenlPMU9fTTwXsi CeZaGDIZa7HrxdQHGaWkadmslPevaDbIlOfU9k/nkUmGYwpY7Ha+uovLknmxr4J8rgD5ll+tF9X KUTRH0lJZYfRpU6+7EnFhjBmBYPOa6UIOo76lkX3ARo4Z1BvYjqRAVcBxdJLCmr0kXkt8CN/11b pLojQrnxM1qWMy4lskeGTn0PoogjzlDLdsLAasxi5RpqYoEnFGXmy2fNWMysGwCdcUQKXCKqlfn T9kN6ASxLjo0YGElkS0RKMK3JMqP+zPQqrtbJzgNTG/diw== X-Received: by 2002:a17:906:3084:b0:aa6:8e61:66a2 with SMTP id a640c23a62f3a-aa68e6167d7mr601728666b.1.1733833088916; Tue, 10 Dec 2024 04:18:08 -0800 (PST) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-aa6873814d4sm305943066b.54.2024.12.10.04.18.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Dec 2024 04:18:08 -0800 (PST) From: "plaisthos (Code Review)" X-Google-Original-From: "plaisthos (Code Review)" X-Gerrit-PatchSet: 1 Date: Tue, 10 Dec 2024 12:18:07 +0000 To: flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: Ia1c5209022d3ab4c0dac6438c41891c7d059f812 X-Gerrit-Change-Number: 828 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: d3073cbcb21734d960a27c7e09c3c8fb2ff8c508 References: Message-ID: MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 RCVD_IN_VALIDITY_RPBL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.218.53 listed in bl.score.senderscore.com] 0.0 RCVD_IN_VALIDITY_SAFE_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.218.53 listed in sa-accredit.habeas.com] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3) [209.85.218.53 listed in wl.mailspike.net] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.218.53 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.0 RCVD_IN_MSPIKE_WL Mailspike good senders 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1tKzBw-0002vT-7p Subject: [Openvpn-devel] [L] Change in openvpn[master]: Allow DEFAULT in data-ciphers and report both expanded and user set o... 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: , Reply-To: arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1818055784625623574?= X-GMAIL-MSGID: =?utf-8?q?1818055784625623574?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/828?usp=email to review the following change. Change subject: Allow DEFAULT in data-ciphers and report both expanded and user set option ...................................................................... Allow DEFAULT in data-ciphers and report both expanded and user set option This adds support for parsing DEFAULT in data-ciphers, the idea is that people can modify the default without repeating the default ciphers. In the past we have seem that people will use data-ciphers BF-CBC or data-ciphers AES-128-CBC when getting the warning that the cipher is not supported by the server. This commit aims to provide a better way for these situation as we still want people to rely on default cipher selection from OpenVPN when possible. Change-Id: Ia1c5209022d3ab4c0dac6438c41891c7d059f812 --- M Changes.rst M doc/man-sections/protocol-options.rst M src/openvpn/multi.c M src/openvpn/options.c M src/openvpn/options.h M src/openvpn/ssl_ncp.c M src/openvpn/ssl_ncp.h M tests/unit_tests/openvpn/test_ncp.c 8 files changed, 276 insertions(+), 48 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/28/828/1 diff --git a/Changes.rst b/Changes.rst index a6cc3be..0bb2e1c 100644 --- a/Changes.rst +++ b/Changes.rst @@ -22,6 +22,11 @@ For more details see [lwipovpn on Gihtub](https://github.com/OpenVPN/lwipovpn). +Default ciphers in ``--data-ciphers`` + Ciphers in ``--data-ciphers`` can contain the string DEFAULT that is + replaced by the default ciphers used by OpenVPN, making it easier to + add an allowed cipher without having to spell out the default ciphers. + Deprecated features ------------------- ``secret`` support has been removed by default. diff --git a/doc/man-sections/protocol-options.rst b/doc/man-sections/protocol-options.rst index 05f87cc..fc7648d 100644 --- a/doc/man-sections/protocol-options.rst +++ b/doc/man-sections/protocol-options.rst @@ -178,6 +178,12 @@ Chacha20-Poly1305 if the underlying SSL library (and its configuration) supports it. + Starting with OpenVPN 2.7 the special keyword DEFAULT can be used in the + string and is replaced by the default ciphers. This can be used add + an additional allowed cipher to the allowed ciphers, e.g. + :code:`DEFAULT:AES-192-CBC` to use the default ciphers but also allow + :code:`AES-192-CBC`. + Cipher negotiation is enabled in client-server mode only. I.e. if ``--mode`` is set to `server` (server-side, implied by setting ``--server`` ), or if ``--pull`` is specified (client-side, implied by diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 45b3cfa..449dcb2 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1896,14 +1896,15 @@ if (strlen(peer_ciphers) > 0) { msg(M_INFO, "PUSH: No common cipher between server and client. " - "Server data-ciphers: '%s', client supported ciphers '%s'", - o->ncp_ciphers, peer_ciphers); + "Server data-ciphers: '%s%s', client supported ciphers '%s'", + o->ncp_ciphers_conf, ncp_expanded_ciphers(o, &gc), peer_ciphers); } else if (tls_multi->remote_ciphername) { msg(M_INFO, "PUSH: No common cipher between server and client. " - "Server data-ciphers: '%s', client supports cipher '%s'", - o->ncp_ciphers, tls_multi->remote_ciphername); + "Server data-ciphers: '%s'%s, client supports cipher '%s'", + o->ncp_ciphers_conf, ncp_expanded_ciphers(o, &gc), + tls_multi->remote_ciphername); } else { diff --git a/src/openvpn/options.c b/src/openvpn/options.c index d8dd518..6ea2928 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3486,40 +3486,6 @@ } } - -/** - * Checks for availibility of Chacha20-Poly1305 and sets - * the ncp_cipher to either AES-256-GCM:AES-128-GCM or - * AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305. - */ -static void -options_postprocess_setdefault_ncpciphers(struct options *o) -{ - if (o->ncp_ciphers) - { - /* custom --data-ciphers set, keep list */ - return; - } - - /* check if crypto library supports chacha */ - bool can_do_chacha = cipher_valid("CHACHA20-POLY1305"); - - if (can_do_chacha && dco_enabled(o)) - { - /* also make sure that dco supports chacha */ - can_do_chacha = tls_item_in_cipher_list("CHACHA20-POLY1305", dco_get_supported_ciphers()); - } - - if (can_do_chacha) - { - o->ncp_ciphers = "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305"; - } - else - { - o->ncp_ciphers = "AES-256-GCM:AES-128-GCM"; - } -} - static void options_postprocess_cipher(struct options *o) { @@ -3550,7 +3516,8 @@ "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."); + "and/or add BF-CBC to --data-ciphers. E.g. " + "--data-ciphers %s:BF-CBC", o->ncp_ciphers_conf); } else if (!o->enable_ncp_fallback && !tls_item_in_cipher_list(o->ciphername, o->ncp_ciphers)) @@ -3558,7 +3525,7 @@ 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); + o->ciphername, o->ncp_ciphers_conf); } } diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 6ab92e2..55f12dd 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -559,6 +559,8 @@ const char *ciphername; bool enable_ncp_fallback; /**< If defined fall back to * ciphername if NCP fails */ + /** The original ncp_ciphers specified by the user in the configuration*/ + const char *ncp_ciphers_conf; const char *ncp_ciphers; const char *authname; const char *engine; diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index 968858e..27db2b1 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -39,6 +39,8 @@ #include "config.h" #endif +#include + #include "syshead.h" #include "win32.h" @@ -222,7 +224,6 @@ return token != NULL; } - const char * tls_peer_ncp_list(const char *peer_info, struct gc_arena *gc) { @@ -334,15 +335,16 @@ return true; } - /* We failed negotiation, give appropiate error message */ + /* We failed negotiation, give appropriate error message */ if (c->c2.tls_multi->remote_ciphername) { msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to negotiate " "cipher with server. Add the server's " - "cipher ('%s') to --data-ciphers (currently '%s') if " - "you want to connect to this server.", + "cipher ('%s') to --data-ciphers (currently '%s'), e.g." + "--data-ciphers %s:%s if you want to connect to this server.", c->c2.tls_multi->remote_ciphername, - c->options.ncp_ciphers); + c->options.ncp_ciphers_conf, c->options.ncp_ciphers_conf, + c->c2.tls_multi->remote_ciphername); return false; } @@ -516,10 +518,13 @@ if (!session->opt->server && !cipher_allowed_as_fallback && !tls_item_in_cipher_list(options->ciphername, options->ncp_ciphers)) { - msg(D_TLS_ERRORS, "Error: negotiated cipher not allowed - %s not in %s", - options->ciphername, options->ncp_ciphers); + struct gc_arena gc = gc_new(); + msg(D_TLS_ERRORS, "Error: negotiated cipher not allowed - %s not in %s%s", + options->ciphername, options->ncp_ciphers_conf, + ncp_expanded_ciphers(options, &gc)); /* undo cipher push, abort connection setup */ options->ciphername = session->opt->config_ciphername; + gc_free(&gc); return false; } else @@ -527,3 +532,99 @@ return true; } } + +/** + * Replaces the string DEFAULT with the string \param replace. The + * @param o Options struct to modify and to use the gc from + * @param replace string used to replace the DEFAULT string + */ +static void +replace_default_in_ncp_ciphers_option(struct options *o, const char *replace) +{ + const char *search = "DEFAULT"; + const int ncp_ciphers_len = strlen(o->ncp_ciphers) + strlen(replace) - strlen(search) + 1; + + uint8_t *ncp_ciphers = gc_malloc(ncp_ciphers_len, true, &o->gc); + + struct buffer ncp_ciphers_buf; + buf_set_write(&ncp_ciphers_buf, ncp_ciphers, ncp_ciphers_len); + + const char *def = strstr(o->ncp_ciphers, search); + + /* Copy everything before the DEFAULT string */ + buf_write(&ncp_ciphers_buf, o->ncp_ciphers, def - o->ncp_ciphers); + + /* copy the default string. */ + buf_write(&ncp_ciphers_buf, replace, strlen(replace)); + + /* copy the rest of the ncp cipher string */ + const char *after_default = def + strlen(search); + buf_write(&ncp_ciphers_buf, after_default, strlen(after_default)); + + o->ncp_ciphers = (char *) ncp_ciphers; +} + +/** + * Checks for availibility of Chacha20-Poly1305 and sets + * the ncp_cipher to either AES-256-GCM:AES-128-GCM or + * AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305. + */ +void +options_postprocess_setdefault_ncpciphers(struct options *o) +{ + bool default_in_cipher_list = o->ncp_ciphers + && tls_item_in_cipher_list("DEFAULT", o->ncp_ciphers); + + /* preserve the values that the user put into the configuration */ + o->ncp_ciphers_conf = o->ncp_ciphers; + + /* check if crypto library supports chacha */ + bool can_do_chacha = cipher_valid("CHACHA20-POLY1305"); + + if (can_do_chacha && dco_enabled(o)) + { + /* also make sure that dco supports chacha */ + can_do_chacha = tls_item_in_cipher_list("CHACHA20-POLY1305", dco_get_supported_ciphers()); + } + + const char *default_ciphers = "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305"; + + if (!can_do_chacha) + { + default_ciphers = "AES-256-GCM:AES-128-GCM"; + } + + /* want to rather print DEFAULT instead of a manually set default list */ + if (!o->ncp_ciphers_conf || !strcmp(default_ciphers, o->ncp_ciphers_conf)) + { + o->ncp_ciphers = default_ciphers; + o->ncp_ciphers_conf = "DEFAULT"; + } + else if (!default_in_cipher_list) + { + /* custom cipher list without DEFAULT string in it, + * nothing to replace/mutate */ + return; + } + else + { + replace_default_in_ncp_ciphers_option(o, default_ciphers); + } +} + +const char * +ncp_expanded_ciphers(struct options *o, struct gc_arena *gc) +{ + if (!strcmp(o->ncp_ciphers, o->ncp_ciphers_conf)) + { + /* expanded ciphers and user set ciphers are identical, no need to + * add an expanded version */ + return ""; + } + + /* two extra brackets, one space, NUL byte */ + struct buffer expanded_ciphers_buf = alloc_buf_gc(strlen(o->ncp_ciphers) + 4, gc); + + buf_printf(&expanded_ciphers_buf, " (%s)", o->ncp_ciphers); + return BSTR(&expanded_ciphers_buf); +} diff --git a/src/openvpn/ssl_ncp.h b/src/openvpn/ssl_ncp.h index afbe331..2378221 100644 --- a/src/openvpn/ssl_ncp.h +++ b/src/openvpn/ssl_ncp.h @@ -156,4 +156,24 @@ bool check_session_cipher(struct tls_session *session, struct options *options); +/** + * Checks for availability of Chacha20-Poly1305 and sets + * the ncp_cipher to either AES-256-GCM:AES-128-GCM or + * AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305 if not set. + * + * If DEFAULT is in the ncp_cipher string, it will be replaced + * by the default cipher string as defined above. + */ +void +options_postprocess_setdefault_ncpciphers(struct options *o); + +/** returns the o->ncp_ciphers in brackets, e.g. + * (AES-256-GCM:CHACHA20-POLY1305) if o->ncp_ciphers_conf + * and o->ncp_ciphers differ, otherwise an empty string + * + * The returned string will be allocated in the passed \param gc + * + */ +const char * +ncp_expanded_ciphers(struct options *o, struct gc_arena *gc); #endif /* ifndef OPENVPN_SSL_NCP_H */ diff --git a/tests/unit_tests/openvpn/test_ncp.c b/tests/unit_tests/openvpn/test_ncp.c index a8e1021..0b763e4 100644 --- a/tests/unit_tests/openvpn/test_ncp.c +++ b/tests/unit_tests/openvpn/test_ncp.c @@ -54,6 +54,17 @@ ASSERT(0); } + +/* Define a dummy dco cipher option to avoid linking against all the DCO + * units */ +#if defined(ENABLE_DCO) +const char * +dco_get_supported_ciphers(void) +{ + return "AES-192-GCM:AES-128-CBC:AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305"; +} +#endif + static void test_check_ncp_ciphers_list(void **state) { @@ -260,13 +271,128 @@ gc_free(&gc); } +static void +test_ncp_default(void **state) +{ + bool have_chacha = cipher_valid("CHACHA20-POLY1305"); + + struct options o = { 0 }; + + o.gc = gc_new(); + + /* no user specified string */ + o.ncp_ciphers = NULL; + options_postprocess_setdefault_ncpciphers(&o); + + if (have_chacha) + { + assert_string_equal(o.ncp_ciphers, "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305"); + } + else + { + assert_string_equal(o.ncp_ciphers, "AES-256-GCM:AES-128-GCM"); + } + assert_string_equal(o.ncp_ciphers_conf, "DEFAULT"); + + /* check that a default string is replaced with DEFAULT */ + if (have_chacha) + { + o.ncp_ciphers = "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305"; + } + else + { + o.ncp_ciphers = "AES-256-GCM:AES-128-GCM"; + } + + options_postprocess_setdefault_ncpciphers(&o); + assert_string_equal(o.ncp_ciphers_conf, "DEFAULT"); + + /* test default in the middle of the string */ + o.ncp_ciphers = "BF-CBC:DEFAULT:AES-128-CBC:AES-256-CBC"; + options_postprocess_setdefault_ncpciphers(&o); + + if (have_chacha) + { + assert_string_equal(o.ncp_ciphers, "BF-CBC:AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305:AES-128-CBC:AES-256-CBC"); + } + else + { + assert_string_equal(o.ncp_ciphers, "BF-CBC:AES-256-GCM:AES-128-GCM:AES-128-CBC:AES-256-CBC"); + } + assert_string_equal(o.ncp_ciphers_conf, "BF-CBC:DEFAULT:AES-128-CBC:AES-256-CBC"); + + /* string at the beginning */ + o.ncp_ciphers = "DEFAULT:AES-128-CBC:AES-192-CBC"; + options_postprocess_setdefault_ncpciphers(&o); + + if (have_chacha) + { + assert_string_equal(o.ncp_ciphers, "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305:AES-128-CBC:AES-192-CBC"); + } + else + { + assert_string_equal(o.ncp_ciphers, "AES-256-GCM:AES-128-GCM:AES-128-CBC:AES-192-CBC"); + } + assert_string_equal(o.ncp_ciphers_conf, "DEFAULT:AES-128-CBC:AES-192-CBC"); + + /* DEFAULT at the end */ + o.ncp_ciphers = "AES-192-GCM:AES-128-CBC:DEFAULT"; + options_postprocess_setdefault_ncpciphers(&o); + + if (have_chacha) + { + assert_string_equal(o.ncp_ciphers, "AES-192-GCM:AES-128-CBC:AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305"); + } + else + { + assert_string_equal(o.ncp_ciphers, "AES-192-GCM:AES-128-CBC:AES-256-GCM:AES-128-GCM"); + } + assert_string_equal(o.ncp_ciphers_conf, "AES-192-GCM:AES-128-CBC:DEFAULT"); + + gc_free(&o.gc); +} + +static void +test_ncp_expand(void **state) +{ + bool have_chacha = cipher_valid("CHACHA20-POLY1305"); + struct options o = {0}; + + o.gc = gc_new(); + struct gc_arena gc = gc_new(); + + /* no user specified string */ + o.ncp_ciphers = NULL; + options_postprocess_setdefault_ncpciphers(&o); + + const char *expanded = ncp_expanded_ciphers(&o, &gc); + + if (have_chacha) + { + assert_string_equal(expanded, " (AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305)"); + } + else + { + assert_string_equal(expanded, " (AES-256-GCM:AES-128-GCM)"); + } + + o.ncp_ciphers = "AES-192-GCM:BF-CBC"; + options_postprocess_setdefault_ncpciphers(&o); + + assert_string_equal(ncp_expanded_ciphers(&o, &gc), ""); + + gc_free(&o.gc); + gc_free(&gc); +} const struct CMUnitTest ncp_tests[] = { cmocka_unit_test(test_check_ncp_ciphers_list), cmocka_unit_test(test_extract_client_ciphers), cmocka_unit_test(test_poor_man), - cmocka_unit_test(test_ncp_best) + cmocka_unit_test(test_ncp_best), + cmocka_unit_test(test_ncp_default), + cmocka_unit_test(test_ncp_expand), };