Re: [Openvpn-devel] [PATCH] unit_tests: make cert_data.h compile with MSVC
Am 24.03.23 um 22:12 schrieb Selva Nair: Hi, On Fri, Mar 24, 2023 at 4:18 PM Matthias Andree wrote: Am 23.03.23 um 15:31 schrieb Frank Lichtenheld: > Currently this is not obvious since we never build the > UTs with MSVC, but it doesn't like the initializers with > "const" variables. They cause > error C2099: initializer is not a constant What MSVC version are you using? What options? I've tried with a minimal C program with static const char *const foo = "something"; and it compiled with the latest 2017 or 2022 MSVC. That will work but the following wont: const char *const foo = "something"; const char *const bar = foo; which is essentially the issue at hand. And more of the same with struct initialization. As bar has static storage duration, it has to be intialized by a "constant expression" which in C does not include "const variables". Though it works with gcc. In C99, automatic variables can be intialized so, and the alternative I suggested uses that approach. I must have missed that bar = foo part, which is indeed nonconforming code in C. With Microsoft tools, I always wonder if it's not easier in practice to make sure the same source code is also valid C++ and grind it in the C++ mixer^Wcompiler instead. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] unit_tests: make cert_data.h compile with MSVC
Hi, On Fri, Mar 24, 2023 at 4:18 PM Matthias Andree wrote: > Am 23.03.23 um 15:31 schrieb Frank Lichtenheld: > > Currently this is not obvious since we never build the > > UTs with MSVC, but it doesn't like the initializers with > > "const" variables. They cause > > error C2099: initializer is not a constant > > What MSVC version are you using? What options? I've tried with a minimal > C program with static const char *const foo = "something"; and it > compiled with the latest 2017 or 2022 MSVC. > That will work but the following wont: const char *const foo = "something"; const char *const bar = foo; which is essentially the issue at hand. And more of the same with struct initialization. As bar has static storage duration, it has to be intialized by a "constant expression" which in C does not include "const variables". Though it works with gcc. In C99, automatic variables can be intialized so, and the alternative I suggested uses that approach. Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] unit_tests: make cert_data.h compile with MSVC
Am 23.03.23 um 15:31 schrieb Frank Lichtenheld: Currently this is not obvious since we never build the UTs with MSVC, but it doesn't like the initializers with "const" variables. They cause error C2099: initializer is not a constant What MSVC version are you using? What options? I've tried with a minimal C program with static const char *const foo = "something"; and it compiled with the latest 2017 or 2022 MSVC. How are you connecting the unit test to the build? when used in an initializer. So change all of them to preprocessor defines instead. It also doesn't like the empty initializer. Of course not, because it's ill-formed code prior to C23. So, NAK. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] unit_tests: make cert_data.h compile with MSVC
Hi,
On Thu, Mar 23, 2023 at 10:31 AM Frank Lichtenheld
wrote:
> Currently this is not obvious since we never build the
> UTs with MSVC, but it doesn't like the initializers with
> "const" variables. They cause
> error C2099: initializer is not a constant
> when used in an initializer.
> So change all of them to preprocessor defines instead.
>
> It also doesn't like the empty initializer.
> error C2059: syntax error: '}'
>
> CC: Selva Nair
> Signed-off-by: Frank Lichtenheld
> ---
> tests/unit_tests/openvpn/cert_data.h | 240 +++---
> tests/unit_tests/openvpn/test_cryptoapi.c | 10 +-
> tests/unit_tests/openvpn/test_pkcs11.c| 10 +-
> 3 files changed, 127 insertions(+), 133 deletions(-)
>
> Notes:
> * this is an prereq for my CMake patch but is otherwise independent,
> so submitting separately
> * this is on top of Selva's "Unit tests: Test for PKCS#11 using a
> softhsm2 token"
>
> diff --git a/tests/unit_tests/openvpn/cert_data.h
> b/tests/unit_tests/openvpn/cert_data.h
> index 33de35ec..359718bb 100644
> --- a/tests/unit_tests/openvpn/cert_data.h
> +++ b/tests/unit_tests/openvpn/cert_data.h
> @@ -36,131 +36,125 @@
> */
>
...
> /* sample-ec.crt */
> +#define cd_cert1 "-BEGIN CERTIFICATE-\n" \
> +"MIIClzCCAX+gAwIBAgIRAIJr3cy95V63CPEtaAA8JN4wDQYJKoZIhvcNAQELBQAw\n" \
> +"GDEWMBQGA1UEAwwNT1ZQTiBURVNUIENBMTAgFw0yMzAzMTMxNjExMjhaGA8yMTIz\n" \
> +"MDIxNzE2MTEyOFowGDEWMBQGA1UEAwwNb3Zwbi10ZXN0LWVjMTBZMBMGByqGSM49\n" \
> +"AgEGCCqGSM49AwEHA0IABHhJG+dK4Z0mY+K0pupwVtyDLOwwGWHjBY6u3LgjRmUh\n" \
> +"fFjaoSfJvdgrPg50wbOkrsUt9Bl6EeDosZuVwuzgRbujgaQwgaEwCQYDVR0TBAIw\n" \
> +"ADAdBgNVHQ4EFgQUPWeU5BEmD8VEOSKeNf9kAvhcVuowUwYDVR0jBEwwSoAU3MLD\n" \
> +"NDOK13DqflQ8ra7FeGBXK06hHKQaMBgxFjAUBgNVBAMMDU9WUE4gVEVTVCBDQTGC\n" \
> +"FD55ErHXpK2JXS3WkfBm0NB1r3vKMBMGA1UdJQQMMAoGCCsGAQUFBwMCMAsGA1Ud\n" \
> +"DwQEAwIHgDANBgkqhkiG9w0BAQsFAAOCAQEAhH/wOFqP4R+FK5QvU+oW/XacFMku\n" \
> +"+qT8lL9J7BG28WhZ0ZcAy/AmtnyynkDyuZSwnlzGgJ5m4L/RfwTzJKhEHiSU3BvB\n" \
> +"5C1Z1Q8k67MHSfb565iCn8GzPUQLK4zsILCoTkJPvimv2bJ/RZmNaD+D4LWiySD4\n" \
> +"tuOEdHKrxIrbJ5eAaN0WxRrvDdwGlyPvbMFvfhXzd/tbkP4R2xvlm7S2DPeSTJ8s\n" \
> +"srXMaPe0lAea4etMSZsjIRPwGRMXBrwbRmb6iN2Cq40867HdaJoAryYig7IiDwSX\n" \
> +"htCbOA6sX+60+FEOYDEx5cmkogl633Pw7LJ3ICkyzIrUSEt6BOT1Gsc1eQ==\n" \
> +"-END CERTIFICATE-\n"
>
I would like to avoid such long preprocessor #defines with continuation
lines.
Would the attached small patch be acceptable instead? It covers only
test_cryptoapi --- if this will do, I can incorporate similar changes for
test_pkcs11 into a v2 of the relevant patch as those have not been
ACK-ed yet.
> --- a/tests/unit_tests/openvpn/test_pkcs11.c
> +++ b/tests/unit_tests/openvpn/test_pkcs11.c
> @@ -113,11 +113,11 @@ static struct test_cert
> uint8_t hash[HASHSIZE]; /* SHA1 fingerprint: computed and
> filled in later */
> char *p11_id; /* PKCS#11 id -- filled in later
> */
> } certs[] = {
> -{cert1, key1, cname1, "OVPN TEST CA1", "OVPN Test Cert 1", {},
> NULL},
> -{cert2, key2, cname2, "OVPN TEST CA2", "OVPN Test Cert 2", {},
> NULL},
> -{cert3, key3, cname3, "OVPN TEST CA1", "OVPN Test Cert 3", {},
> NULL},
> -{cert4, key4, cname4, "OVPN TEST CA2", "OVPN Test Cert 4", {},
> NULL},
> -{}
> +{cd_cert1, cd_key1, cd_cname1, "OVPN TEST CA1", "OVPN Test Cert
> 1", {}, NULL},
>
Isn't it necessary to change these {} to {0} as well? The build may not
report it now as we haven't yet enabled test_pkcs11 on Windows.
> +{cd_cert2, cd_key2, cd_cname2, "OVPN TEST CA2", "OVPN Test Cert
> 2", {}, NULL},
> +{cd_cert3, cd_key3, cd_cname3, "OVPN TEST CA1", "OVPN Test Cert
> 3", {}, NULL},
> +{cd_cert4, cd_key4, cd_cname4, "OVPN TEST CA2", "OVPN Test Cert
> 4", {}, NULL},
> +{NULL, NULL, NULL, NULL, NULL,
>{}, NULL}
>
A single {0} should be enough here as well.
Selva
From 08d4813fdc3c90969c49dc07be529111a7e84ac4 Mon Sep 17 00:00:00 2001
From: Selva Nair
Date: Thu, 23 Mar 2023 23:24:38 -0400
Subject: [PATCH 1/2] Make cert_data.h and test_cryptoapi.c MSVC compliant
- Do not use non-literal initializers for static objects
- Replace empty initializer {} by {0}
Signed-off-by: Selva Nair
---
tests/unit_tests/openvpn/cert_data.h | 6 +++---
tests/unit_tests/openvpn/test_cryptoapi.c | 24 ---
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/tests/unit_tests/openvpn/cert_data.h b/tests/unit_tests/openvpn/cert_data.h
index 33de35ec..0886b071 100644
--- a/tests/unit_tests/openvpn/cert_data.h
+++ b/tests/unit_tests/openvpn/cert_data.h
@@ -79,7 +79,7 @@ static const char *const cert2 =
"HeTsAlHjfFEReVDiNCI9vMQLKFKKWnAorT2+iyRueA3bt2gchf863BBhZvJddL7Q\n"
"KBa0osXw+eGBRAwsm7m1qCho3b3fN2nFAa+k07ptRkOeablmFdXE81nVlA==\n"
"-END CERTIFICATE-\n";
-static const char
Re: [Openvpn-devel] [PATCH] Fix compilation without compression
Hi, On Fri, Mar 24, 2023 at 09:38:16AM +0100, Gert Doering wrote: > The previous commit (e86bc8b2967) breaks compilation if all compression > algorithms are disabled (--disable-lz4 --disable-lzo). A later patch > in the series would fix this but can not be merged yet. Just for reference, this got fixed with Arne's commit a8170dd0e76 now, so this patch is no longer needed. Marked as such in Patchwork. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Don't overwrite socket flags when using DCO on Windows
Acked-by: Gert Doering "Because it makes sense". We didn't discover this before as it needs a server actually pushing sock-options *and* a DCO-on-Windows client (*and* TCP?). I haven't actually tested this, but discussed this beforehand with Lev, and he has (and the change looks good). Your patch has been applied to the master and release/2.6 branch. commit 82e7d5cfd81f03f045ace2bf1d3590b79441ea17 (master) commit cfc5228f9aeaa99c75fb7538435780e4dd7fb7de (release/2.6) Author: Lev Stipakov Date: Fri Mar 24 14:18:18 2023 +0200 Don't overwrite socket flags when using DCO on Windows Signed-off-by: Lev Stipakov Acked-by: Gert Doering Message-Id: <20230324121818.2358-1-lstipa...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26513.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Parse compression options and bail out when compression is disabled
Acked-by: Gert Doering This is best viewed with "--color-moved=zebra", as it really just moves stuff around (and gets rid of a few #ifdef USE_COMP). That we now have #include statements after code is a big ugly, but saves having another set of #ifdef... Client-tested with and without compression built in, and pushed to GHA for verification that nothing is breaking anymore. Your patch has been applied to master and release/2.6 branch. commit a8170dd0e76a7440f3291ad26d78f8ca247a191b (master) commit 5a189d5e249c89c55ef7cad4d19a9d8e89a20ff1 (release/2.6) Author: Arne Schwabe Date: Fri Mar 24 13:10:50 2023 +0100 Parse compression options and bail out when compression is disabled Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20230324121050.1350913-1-a...@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26512.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Don't overwrite socket flags when using DCO on Windows
From: Lev Stipakov
Socket flags can be pushed, in which case they overwrite
existing value. We use socket flags to distingust between
DCO handle and socket on Windows. If server pushes --socket-flags,
we treat DCO handle as socket and everything explodes.
Fix by making link_socket_update_flags() update flags
(like name suggests) instead of overwriting them. Also
do not set TCP_NODELAY on DCO handle on Windows because
it doesn't make sense.
Change-Id: Ia34d73ca49041cb0ce22b84751cdbff57de96048
Signed-off-by: Lev Stipakov
---
src/openvpn/socket.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 216f2ad7..ab8cc754 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -957,7 +957,8 @@ socket_set_mark(socket_descriptor_t sd, int mark)
static bool
socket_set_flags(socket_descriptor_t sd, unsigned int sockflags)
{
-if (sockflags & SF_TCP_NODELAY)
+/* SF_TCP_NODELAY doesn't make sense for dco-win */
+if ((sockflags & SF_TCP_NODELAY) && (!(sockflags & SF_DCO_WIN)))
{
return socket_set_tcp_nodelay(sd, 1);
}
@@ -972,7 +973,8 @@ link_socket_update_flags(struct link_socket *ls, unsigned
int sockflags)
{
if (ls && socket_defined(ls->sd))
{
-return socket_set_flags(ls->sd, ls->sockflags = sockflags);
+ls->sockflags |= sockflags;
+return socket_set_flags(ls->sd, ls->sockflags);
}
else
{
--
2.38.1.windows.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4] Parse compression options and bail out when compression is disabled
This change keeps the option parsing of compression options even when
compression is disabled. This allows OpenVPN to also refuse/reject connections
that try to use compression when compression is completely disabled.
Patch v4: fix one missing USE_COMP
Change-Id: I9d7afd8f1d67d2455b4ec6bc12f4dcde80140c4f
Signed-off-by: Arne Schwabe
---
src/openvpn/comp.c| 14 ---
src/openvpn/comp.h| 85 ++-
src/openvpn/init.c| 4 +-
src/openvpn/multi.c | 2 -
src/openvpn/options.c | 12 +-
src/openvpn/options.h | 4 --
6 files changed, 56 insertions(+), 65 deletions(-)
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index c7a562f5a..27b640ce0 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -29,10 +29,11 @@
#include "syshead.h"
-#ifdef USE_COMP
-
#include "comp.h"
#include "error.h"
+
+#ifdef USE_COMP
+
#include "otime.h"
#include "memdbg.h"
@@ -158,6 +159,7 @@ comp_generate_peer_info_string(const struct
compress_options *opt, struct buffer
buf_printf(out, "IV_COMP_STUB=1\n");
buf_printf(out, "IV_COMP_STUBv2=1\n");
}
+#endif /* USE_COMP */
bool
check_compression_settings_valid(struct compress_options *info, int msglevel)
@@ -170,8 +172,13 @@ check_compression_settings_valid(struct compress_options
*info, int msglevel)
if (info->alg != COMP_ALGV2_UNCOMPRESSED && info->alg != COMP_ALG_UNDEF
&& (info->flags & COMP_F_ALLOW_NOCOMP_ONLY))
{
+#ifdef USE_COMP
msg(msglevel, "Compression or compression stub framing is not allowed "
"since data-channel offloading is enabled.");
+#else
+msg(msglevel, "Compression or compression stub framing is not allowed "
+"since OpenVPN was built without compression support.");
+#endif
return false;
}
@@ -199,6 +206,3 @@ check_compression_settings_valid(struct compress_options
*info, int msglevel)
#endif
return true;
}
-
-
-#endif /* USE_COMP */
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 71b350d09..89940cf3a 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -28,12 +28,19 @@
#ifndef OPENVPN_COMP_H
#define OPENVPN_COMP_H
-#ifdef USE_COMP
+/* We always parse all compression options, so we include these defines/struct
+ * outside of the USE_COMP define */
-#include "buffer.h"
-#include "mtu.h"
-#include "common.h"
-#include "status.h"
+/* Compression flags */
+#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
+#define COMP_F_ALLOW_COMPRESS (1<<1) /* not only downlink is compressed
but also uplink */
+#define COMP_F_SWAP (1<<2) /* initial command byte is swapped
with last byte in buffer to preserve payload alignment */
+#define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support
compression stubs */
+#define COMP_F_ALLOW_STUB_ONLY (1<<4) /* Only accept stub compression,
even with COMP_F_ADVERTISE_STUBS_ONLY
+* we still accept other
compressions to be pushed */
+#define COMP_F_MIGRATE (1<<5) /* push stub-v2 or comp-lzo no when
we see a client with comp-lzo in occ */
+#define COMP_F_ALLOW_ASYM (1<<6) /* Compression was explicitly set
to allow asymetric compression */
+#define COMP_F_ALLOW_NOCOMP_ONLY(1<<7) /* Do not allow compression framing
(breaks DCO) */
/* algorithms */
#define COMP_ALG_UNDEF 0
@@ -51,16 +58,37 @@
#define COMP_ALGV2_SNAPPY 13
*/
-/* Compression flags */
-#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
-#define COMP_F_ALLOW_COMPRESS (1<<1) /* not only downlink is compressed
but also uplink */
-#define COMP_F_SWAP (1<<2) /* initial command byte is swapped
with last byte in buffer to preserve payload alignment */
-#define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support
compression stubs */
-#define COMP_F_ALLOW_STUB_ONLY (1<<4) /* Only accept stub compression,
even with COMP_F_ADVERTISE_STUBS_ONLY
-* we still accept other
compressions to be pushed */
-#define COMP_F_MIGRATE (1<<5) /* push stub-v2 or comp-lzo no when
we see a client with comp-lzo in occ */
-#define COMP_F_ALLOW_ASYM (1<<6) /* Compression was explicitly set
to allow asymetric compression */
-#define COMP_F_ALLOW_NOCOMP_ONLY(1<<7) /* Do not allow compression framing
(breaks DCO) */
+/*
+ * Information that basically identifies a compression
+ * algorithm and related flags.
+ */
+struct compress_options
+{
+int alg;
+unsigned int flags;
+};
+
+static inline bool
+comp_non_stub_enabled(const struct compress_options *info)
+{
+return info->alg != COMP_ALGV2_UNCOMPRESSED
+ && info->alg != COMP_ALG_STUB
+ && info->alg != COMP_ALG_UNDEF;
+}
+
+/**
+ * Checks if the compression settings are valid. Takes into account the
+ * flags of allow-compression and also the whether
Re: [Openvpn-devel] [PATCH v3 4/4] Parse compression options and bail out when compression is disabled
Hi, On Thu, Mar 23, 2023 at 06:06:01PM +0100, Arne Schwabe wrote: > This change keeps the option parsing of compression options even when > compression is disabled. This allows OpenVPN to also refuse/reject connections > that try to use compression when compression is completely disabled. Amazing as that might be, if you actually disable compression, master + 4/4 does not compile anymore... depbase=`echo init.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`; cc -DHAVE_CONFIG_H -I. -I../.. -I../../include -I../../include -I../../src/compat -DPLUGIN_LIBDIR=\"/usr/local/lib/openvpn/plugins\" -Wall -g -O2 -std=c99 -MT init.o -MD -MP -MF $depbase.Tpo -c -o init.o init.c && mv -f $depbase.Tpo $depbase.Po init.c:2648:9: warning: implicit declaration of function 'comp_uninit' is invalid in C99 [-Wimplicit-function-declaration] comp_uninit(c->c2.comp_context); ^ init.c:2648:27: error: no member named 'comp_context' in 'struct context_2'; did you mean 'mda_context'? comp_uninit(c->c2.comp_context); ^~~~ mda_context ./openvpn.h:459:33: note: 'mda_context' declared here struct man_def_auth_context mda_context; ^ init.c:2649:30: warning: implicit declaration of function 'comp_init' is invalid in C99 [-Wimplicit-function-declaration] c->c2.comp_context = comp_init(>options.comp); init.c:2649:15: error: no member named 'comp_context' in 'struct context_2'; did you mean 'mda_context'? c->c2.comp_context = comp_init(>options.comp); ^~~~ mda_context ./openvpn.h:459:33: note: 'mda_context' declared here struct man_def_auth_context mda_context; ^ init.c:2649:28: error: assigning to 'struct man_def_auth_context' from incompatible type 'int' c->c2.comp_context = comp_init(>options.comp); ^ ~~~ 2 warnings and 3 errors generated. *** Error code 1 so, NAK. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add 'allow-compression stub-only' internally for DCO
Acked-by: Gert Doering This is the actual thing we want to fix: if a server pushes 'comp-lzo no', a non-DCO client will enable compression framing, while a DCO client can not do this, and silently stays on "no framing" - and then both sides will drop all data packets because "incorrect format". We can not "make it work", but we *can* abort the connection with a clear message so the VPN provider / server operator can fix their setup. This change also removes sending of all IV_COMP* variables to the server if DCO is active - so a "server that cares" knows that it must not send any compression settings. I have run this through the t_client/t_server tests on DCO and non DCO hosts, with and without compression, and all the existing setups still work fine, including compatibility to older versions. I have also tested pushed options and ccd/ options on "no compression enabled" setups - pushing 'comp-lzo no' with no DCO --> accepted, do "stub" framing - pushing 'comp-lzo no' with DCO active --> refused, SIGUSR1 restart 2023-03-24 09:09:48 Compression or compression stub framing is not allowed since data-channel offloading is enabled. 2023-03-24 09:09:48 OPTIONS ERROR: server pushed compression settings that are not allowed and will result in a non-working connection. See also allow-compression in the manual. - pushing 'compress lz4' is refused in both cases, unless "allow-compression asym/yes" is set - ccd file producing 'comp-lzo no' - ccd file producing 'compress stub-v2' - ccd file producing 'compress lz4' --> this all works as expected (refusing the client with AUTH_FAILED), though we have started to be "just a bit" chatty about this... tun-udp-p2mp[564755]: peer-id=1 OPTIONS IMPORT: reading client specific options from: ccd/freebsd-14-amd64 tun-udp-p2mp[564755]: peer-id=1 Note: '--allow-compression' is not set to 'no', disabling data channel offload. tun-udp-p2mp[564755]: peer-id=1 MULTI: client has been rejected due to incompatible DCO options tun-udp-p2mp[564755]: peer-id=1 Compression or compression stub framing is not allowed since data-channel offloading is enabled. tun-udp-p2mp[564755]: peer-id=1 MULTI: client has been rejected due to invalid compression options Compilation with --disable-lzo --disable-lz4 is still broken with this commit - this was overlooked in part 2/4, and will be fixed in 4/4. Your patch has been applied to the master and release/2.6 branch. commit 4117d950788eebfaf6c9b5dde278e3a81b9e805d (master) commit 2ac91ea73b76dd17d5cdf78740790ed928e08bff (release/2.6) Author: Arne Schwabe Date: Fri Mar 24 11:06:40 2023 +0100 Add 'allow-compression stub-only' internally for DCO Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20230324100640.1340535-1-a...@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26509.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4] Add 'allow-compression stub-only' internally for DCO
This changes the "no" setting of allow-compression to also refuse framing.
This is important for our DCO implementation as these do not implement framing.
This behaviour surfaced when a commercial VPN provider was pushing
"comp-lzo no" to a client with DCO. While we are technically at fault here
for announcing comp-lzo no support by announcing IV_LZO_STUB=1, the
VPN provider continues to push "comp-lzo no" even in absense of that
flag.
As the new default we default to allow-compression stub-only if a stub option
is found in the config and to allow-compression no otherwise. This ensures
that we only enable DCO when no compression framing is used.
This will now also bail out if the server pushes a compression setting that
we do not support as mismatching compression is almost never a working
connection. In my lz4-v2 and lzo-v2 you might have a connection that works
mostly but some packets will be dropped since they compressed which is not
desirable either since it becomes very hard to debug.
Patch v2: bail out if server pushes an unsupported method. Also include this
bail out logic when OpenVPN is compiled without compression support.
Patch v3: always parse all compression option and move logic to check method
Patch v4: fix for not setting correct default for non-dco
Change-Id: Ibd0c77af24e2214b3055d585dc23a4b06dccd414
Signed-off-by: Arne Schwabe
---
doc/man-sections/protocol-options.rst | 4 ++-
src/openvpn/comp.c| 47 ++-
src/openvpn/comp.h| 2 +-
src/openvpn/options.c | 14 ++--
4 files changed, 46 insertions(+), 21 deletions(-)
diff --git a/doc/man-sections/protocol-options.rst
b/doc/man-sections/protocol-options.rst
index 248f65cfd..055d08f95 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -25,7 +25,9 @@ configured in a compatible way between both the local and
remote side.
compression at the same time is not a feasible option.
:code:`no` (default)
- OpenVPN will refuse any non-stub compression.
+ OpenVPN will refuse any compression. If data-channel offloading
+ is enabled. OpenVPN will additionally also refuse compression
+ framing (stub).
:code:`yes`
OpenVPN will send and receive compressed packets.
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index d6d8029da..c7a562f5a 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -134,36 +134,51 @@ comp_print_stats(const struct compress_context *compctx,
struct status_output *s
void
comp_generate_peer_info_string(const struct compress_options *opt, struct
buffer *out)
{
-if (opt)
+if (!opt || opt->flags & COMP_F_ALLOW_NOCOMP_ONLY)
+{
+return;
+}
+
+bool lzo_avail = false;
+if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
{
-bool lzo_avail = false;
-if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
-{
#if defined(ENABLE_LZ4)
-buf_printf(out, "IV_LZ4=1\n");
-buf_printf(out, "IV_LZ4v2=1\n");
+buf_printf(out, "IV_LZ4=1\n");
+buf_printf(out, "IV_LZ4v2=1\n");
#endif
#if defined(ENABLE_LZO)
-buf_printf(out, "IV_LZO=1\n");
-lzo_avail = true;
+buf_printf(out, "IV_LZO=1\n");
+lzo_avail = true;
#endif
-}
-if (!lzo_avail)
-{
-buf_printf(out, "IV_LZO_STUB=1\n");
-}
-buf_printf(out, "IV_COMP_STUB=1\n");
-buf_printf(out, "IV_COMP_STUBv2=1\n");
}
+if (!lzo_avail)
+{
+buf_printf(out, "IV_LZO_STUB=1\n");
+}
+buf_printf(out, "IV_COMP_STUB=1\n");
+buf_printf(out, "IV_COMP_STUBv2=1\n");
}
bool
check_compression_settings_valid(struct compress_options *info, int msglevel)
{
+/*
+ * We also allow comp-stub-v2 here as it technically allows escaping of
+ * weird mac address and IPv5 protocol but practically always is used
+ * as an way to disable all framing.
+ */
+if (info->alg != COMP_ALGV2_UNCOMPRESSED && info->alg != COMP_ALG_UNDEF
+&& (info->flags & COMP_F_ALLOW_NOCOMP_ONLY))
+{
+msg(msglevel, "Compression or compression stub framing is not allowed "
+"since data-channel offloading is enabled.");
+return false;
+}
+
if ((info->flags & COMP_F_ALLOW_STUB_ONLY) && comp_non_stub_enabled(info))
{
msg(msglevel, "Compression is not allowed since allow-compression is "
-"set to 'no'");
+"set to 'stub-only'");
return false;
}
#ifndef ENABLE_LZ4
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 8636727ab..71b350d09 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -60,7 +60,7 @@
* we still accept other
compressions to be pushed */
#define COMP_F_MIGRATE (1<<5) /* push stub-v2 or comp-lzo no when
we see a client with comp-lzo in
[Openvpn-devel] [PATCH] Fix compilation without compression
The previous commit (e86bc8b2967) breaks compilation if all compression algorithms are disabled (--disable-lz4 --disable-lzo). A later patch in the series would fix this but can not be merged yet. Signed-off-by: Gert Doering --- src/openvpn/options.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 435e1ca9..e3d14e94 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3779,8 +3779,10 @@ options_postprocess_mutate(struct options *o, struct env_set *es) /* this depends on o->windows_driver, which is set above */ options_postprocess_mutate_invariant(o); +#ifdef USE_COMP /* check that compression settings in the options are okay */ check_compression_settings_valid(>comp, M_USAGE); +#endif /* * Save certain parms before modifying options during connect, especially -- 2.39.2 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 3/4] Add 'allow-compression stub-only' internally for DCO
Hi,
On Thu, Mar 23, 2023 at 06:06:00PM +0100, Arne Schwabe wrote:
> index 435e1ca9e..92f7456a4 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -3644,10 +3644,16 @@ options_set_backwards_compatible_options(struct
> options *o)
> *
> * Disable compression by default starting with 2.6.0 if no other
> * compression related option has been explicitly set */
> -if (!comp_non_stub_enabled(>comp) && !need_compatibility_before(o,
> 20600)
> -&& (o->comp.flags == 0))
> +if (!need_compatibility_before(o, 20600) && (o->comp.flags == 0))
> {
> -o->comp.flags = COMP_F_ALLOW_STUB_ONLY|COMP_F_ADVERTISE_STUBS_ONLY;
> +if (o->comp.alg == COMP_ALG_UNDEF)
> +{
> +o->comp.flags = COMP_F_ALLOW_NOCOMP_ONLY;
> +}
This breaks the scenario in question ("no compress in config, server pushes
'comp-lzo no'") unconditionally.
I thought we agreed that we only want to break this for "if DCO",
because this is what we *have* to break.
> @@ -3749,6 +3755,12 @@ options_postprocess_mutate(struct options *o, struct
> env_set *es)
> o->tuntap_options.disable_dco = !dco_check_option(D_DCO, o)
> || !dco_check_startup_option(D_DCO,
> o);
> }
> +#ifdef USE_COMP
> +if (dco_enabled(o))
> +{
> +o->comp.flags |= COMP_F_ALLOW_NOCOMP_ONLY;
> +}
> +#endif
... and that's the code for it.
So, NAK, some more hours wasted on testing.
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
