| Message ID | 20211105150742.2909443-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v2] Default to --cipher BF-CBC if not set and compat-mode < 2.4.0 | expand |
Hi, On 05/11/2021 16:07, Arne Schwabe wrote: > When we try to make a configuration compatible to a version earlier > than 2.4.0 we probably need to have a --cipher configured since NCP > is not available. In configuration where --cipher is not specified > we default to BF-CBC to support these old clients. > > Note that with OpenSSL 3.0 you will also need to enable the legacy > provider otherwise we bail out since BF-CBC is no longer supported. > > Also move the condition so BF-CBC gets included in the data-ciphers > list. > > Patch v2: move the comment to a better place. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> Reviewed-by: Antonio Quartulli <a@unstable.cc> Unfortunately I cannot fully ACK this patch because I'd need to compile 2.3 to run a test, but this turned to be a mission impossible (due to OpenSSL compatibility issues). The change makes sense and it should do what we expect. If anybody wants to test against an openvpn2.3 peer is most welcome. Cheers,
Am 04.02.22 um 17:51 schrieb Antonio Quartulli: > Hi, > > On 05/11/2021 16:07, Arne Schwabe wrote: >> When we try to make a configuration compatible to a version earlier >> than 2.4.0 we probably need to have a --cipher configured since NCP >> is not available. In configuration where --cipher is not specified >> we default to BF-CBC to support these old clients. >> >> Note that with OpenSSL 3.0 you will also need to enable the legacy >> provider otherwise we bail out since BF-CBC is no longer supported. >> >> Also move the condition so BF-CBC gets included in the data-ciphers >> list. >> >> Patch v2: move the comment to a better place. >> >> Signed-off-by: Arne Schwabe <arne@rfc2549.org> > > Reviewed-by: Antonio Quartulli <a@unstable.cc> > > Unfortunately I cannot fully ACK this patch because I'd need to compile > 2.3 to run a test, but this turned to be a mission impossible (due to > OpenSSL compatibility issues). > > The change makes sense and it should do what we expect. > > If anybody wants to test against an openvpn2.3 peer is most welcome. A 2.4 client with ncp-disable should work nicely as well. We just do not make ourselves compatible with 2.4.0 with ncp-disable if you select 2.4.0, you need 2.3.0 for that. Arne
Hi, On 04/02/2022 20:12, Arne Schwabe wrote: > A 2.4 client with ncp-disable should work nicely as well. We just do not > make ourselves compatible with 2.4.0 with ncp-disable if you select > 2.4.0, you need 2.3.0 for that. Thanks for the hint! Went the lazy route and tested against 2.4.11 with --ncp-disable. Server: 2.4.11 with --ncp-disable (to mimic 2.3.x) and no --cipher specified (BF-CBC is therefore the default). Client: master + this patch and no --cipher specified. * First attempt: wrong cipher negotiated and connection breaks. * Second attempt: add --compat-mode 2.3.0 to client (to exercise the code implemented by this patch) and I could happily connect to the 2.3.x-like server using BF-CBC. Acked-by: Antonio Quartulli <a@unstable.cc>
I have not given this much testing, as Antonio has tested the relevant
combinations. Code looks good. Client-tested.
Your patch has been applied to the master branch.
commit fd72a3b99dcb110953d8466bfe6c47dab3a29657
Author: Arne Schwabe
Date: Fri Nov 5 16:07:42 2021 +0100
Default to --cipher BF-CBC if not set and compat-mode < 2.4.0
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20211105150742.2909443-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23100.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 4dc70e4f3..6751084af 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3186,6 +3186,19 @@ options_set_backwards_compatible_options(struct options *o) } } + if (need_compatibility_before(o, 20400)) + { + if (!o->ciphername) + { + /* If ciphername is not set default to BF-CBC when targeting these + * old versions that do not have NCP */ + o->ciphername = "BF-CBC"; + } + /* Versions < 2.4.0 additionally might be compiled with --enable-small and + * not have OCC strings required for "poor man's NCP" */ + o->enable_ncp_fallback = true; + } + /* 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. @@ -3198,13 +3211,6 @@ options_set_backwards_compatible_options(struct options *o) append_cipher_to_ncp_list(o, o->ciphername); } - /* Versions < 2.4.0 additionally might be compiled with --enable-small and - * not have OCC strings required for "poor man's NCP" */ - if (o->ciphername && need_compatibility_before(o, 20400)) - { - o->enable_ncp_fallback = true; - } - #ifdef USE_COMP /* Compression is deprecated and we do not want to announce support for it * by default anymore, additionally DCO breaks with compression.
When we try to make a configuration compatible to a version earlier than 2.4.0 we probably need to have a --cipher configured since NCP is not available. In configuration where --cipher is not specified we default to BF-CBC to support these old clients. Note that with OpenSSL 3.0 you will also need to enable the legacy provider otherwise we bail out since BF-CBC is no longer supported. Also move the condition so BF-CBC gets included in the data-ciphers list. Patch v2: move the comment to a better place. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/options.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)