Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup
Hi, On 19/03/2021 16:31, Arne Schwabe wrote: > This option allow migration to a non compression server config while > still retraining compatibility with client that have a compression > setting in their config. > > For existing setups that used to have comp-lzo no or another > compression setting in their configs it is a difficult to migrate to > a setup without compression without replacing all client configs at > once especially if OpenVPN 2.3 or earlier clients are in the mix that > do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients > that support pushing this is not a satisfying solution as the clients > log occ mismatches and the "push stub-v2" needs to be in the server > config "forever". As discussed on other channels, I think it would be beneficial to report in the commit message also the "what is this patch doing to achieve this goal?". Basically a shorter version of what you are already putting in the manpage. > > Signed-off-by: Arne Schwabe > --- > doc/man-sections/protocol-options.rst | 12 +++- > src/openvpn/comp.h| 1 + > src/openvpn/multi.c | 41 + > src/openvpn/options.c | 6 ++ > src/openvpn/ssl.c | 34 ++- > src/openvpn/ssl_common.h | 1 + > src/openvpn/ssl_util.c| 43 ++ > src/openvpn/ssl_util.h| 15 + > tests/unit_tests/openvpn/Makefile.am | 14 - > tests/unit_tests/openvpn/test_misc.c | 83 +++ > 10 files changed, 245 insertions(+), 5 deletions(-) > create mode 100644 tests/unit_tests/openvpn/test_misc.c > > diff --git a/doc/man-sections/protocol-options.rst > b/doc/man-sections/protocol-options.rst > index e9d5d63d..c5cd76dd 100644 > --- a/doc/man-sections/protocol-options.rst > +++ b/doc/man-sections/protocol-options.rst > @@ -84,10 +84,10 @@ configured in a compatible way between both the local and > remote side. > --compress algorithm >**DEPRECATED** Enable a compression algorithm. Compression is generally >not recommended. VPN tunnels which use compression are susceptible to > - the VORALCE attack vector. > + the VORALCE attack vector. See also the :code:`migrate` parameter below. > >The ``algorithm`` parameter may be :code:`lzo`, :code:`lz4`, > - :code:`lz4-v2`, :code:`stub`, :code:`stub-v2` or empty. > + :code:`lz4-v2`, :code:`stub`, :code:`stub-v2`, :code:`migrate` or empty. >LZO and LZ4 are different compression algorithms, with LZ4 generally >offering the best performance with least CPU usage. > > @@ -106,6 +106,14 @@ configured in a compatible way between both the local > and remote side. >Note: the :code:`stub` (or empty) option is NOT compatible with the older >option ``--comp-lzo no``. > > + The :code:`migrate` algorithm is a special and is intended to allow > + migration away from ``--compress``/``--comp-lzo`` options to no > compression. > + This options set the server to no compression mode. If a client > + is detected that indicates that compression is used the will automatically typ0 - missing $something after 'the' > + add ``--push compress stub-v2`` to the client specific configuration > + if supported by the client and otherwise switch to ``comp-lzo no`` > + and also ``--push comp-lzo`` to the client specific configuration. > + >***Security Considerations*** > >Compression and encryption is a tricky combination. If an attacker knows > diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h > index 5c0322ca..a880e198 100644 > --- a/src/openvpn/comp.h > +++ b/src/openvpn/comp.h > @@ -58,6 +58,7 @@ > #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 */ > > > /* > diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c > index f7e0f680..3060410d 100644 > --- a/src/openvpn/multi.c > +++ b/src/openvpn/multi.c > @@ -2469,6 +2469,46 @@ multi_client_connect_early_setup(struct multi_context > *m, > multi_client_connect_setenv(m, mi); > } > > +/** > + * Do the necessary modification for doing the compress mitigrate. This is typ0: mitigrate > + * implemented as a connect handler as it fits the modify config for a > client > + * paradigm and also is early enough in the chain to be overwritten by > another > + * ccd/script to do compression on a special client. > + */ > +static enum client_connect_return > +multi_client_connect_compress_migrate(struct multi_context *m, > + struct multi_instance *mi, > + bool
Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup
On 21/03/2021 13:56, Arne Schwabe wrote:
Am 20.03.21 um 14:20 schrieb David Sommerseth:
On 19/03/2021 16:31, Arne Schwabe wrote:
This option allow migration to a non compression server config while
still retraining compatibility with client that have a compression
setting in their config.
For existing setups that used to have comp-lzo no or another
compression setting in their configs it is a difficult to migrate to
a setup without compression without replacing all client configs at
once especially if OpenVPN 2.3 or earlier clients are in the mix that
do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients
that support pushing this is not a satisfying solution as the clients
log occ mismatches and the "push stub-v2" needs to be in the server
config "forever".
Signed-off-by: Arne Schwabe
---
doc/man-sections/protocol-options.rst | 12 +++-
src/openvpn/comp.h | 1 +
src/openvpn/multi.c | 41 +
src/openvpn/options.c | 6 ++
src/openvpn/ssl.c | 34 ++-
src/openvpn/ssl_common.h | 1 +
src/openvpn/ssl_util.c | 43 ++
src/openvpn/ssl_util.h | 15 +
tests/unit_tests/openvpn/Makefile.am | 14 -
tests/unit_tests/openvpn/test_misc.c | 83 +++
10 files changed, 245 insertions(+), 5 deletions(-)
create mode 100644 tests/unit_tests/openvpn/test_misc.c
This fails compiling:
../../../src/openvpn/ssl.c: In function ‘key_method_2_write’:
../../../src/openvpn/ssl.c:2280:13: error: ‘multi’ undeclared (first use
in this function)
if (multi->remote_usescomp && session->opt->mode == MODE_SERVER
^
../../../src/openvpn/ssl.c:2280:13: note: each undeclared identifier is
reported only once for each function it appears in
make[3]: *** [Makefile:742: ssl.o] Error 1
Oh the fun of rebasing. The multi argument was added by another patch.
Can you just the small patch to review the rest of the patch?:
static bool
-key_method_2_write(struct buffer *buf, struct tls_session *session)
+key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct
tls_session *session)
{
struct key_state *ks = >key[KS_PRIMARY]; /* primary
key */
@@ -2856,7 +2856,7 @@ tls_process(struct tls_multi *multi,
if (!buf->len && ((ks->state == S_START && !session->opt->server)
|| (ks->state == S_GOT_KEY &&
session->opt->server)))
{
-if (!key_method_2_write(buf, session))
+if (!key_method_2_write(buf, multi, session))
{
goto error;
}
I'll give this a spin and continue the review. Thx!
--
kind regards,
David Sommerseth
OpenVPN Inc
OpenPGP_signature
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup
Am 20.03.21 um 14:20 schrieb David Sommerseth:
> On 19/03/2021 16:31, Arne Schwabe wrote:
>> This option allow migration to a non compression server config while
>> still retraining compatibility with client that have a compression
>> setting in their config.
>>
>> For existing setups that used to have comp-lzo no or another
>> compression setting in their configs it is a difficult to migrate to
>> a setup without compression without replacing all client configs at
>> once especially if OpenVPN 2.3 or earlier clients are in the mix that
>> do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients
>> that support pushing this is not a satisfying solution as the clients
>> log occ mismatches and the "push stub-v2" needs to be in the server
>> config "forever".
>>
>> Signed-off-by: Arne Schwabe
>> ---
>> doc/man-sections/protocol-options.rst | 12 +++-
>> src/openvpn/comp.h | 1 +
>> src/openvpn/multi.c | 41 +
>> src/openvpn/options.c | 6 ++
>> src/openvpn/ssl.c | 34 ++-
>> src/openvpn/ssl_common.h | 1 +
>> src/openvpn/ssl_util.c | 43 ++
>> src/openvpn/ssl_util.h | 15 +
>> tests/unit_tests/openvpn/Makefile.am | 14 -
>> tests/unit_tests/openvpn/test_misc.c | 83 +++
>> 10 files changed, 245 insertions(+), 5 deletions(-)
>> create mode 100644 tests/unit_tests/openvpn/test_misc.c
>>
>
> This fails compiling:
>
> ../../../src/openvpn/ssl.c: In function ‘key_method_2_write’:
> ../../../src/openvpn/ssl.c:2280:13: error: ‘multi’ undeclared (first use
> in this function)
> if (multi->remote_usescomp && session->opt->mode == MODE_SERVER
> ^
> ../../../src/openvpn/ssl.c:2280:13: note: each undeclared identifier is
> reported only once for each function it appears in
> make[3]: *** [Makefile:742: ssl.o] Error 1
Oh the fun of rebasing. The multi argument was added by another patch.
Can you just the small patch to review the rest of the patch?:
static bool
-key_method_2_write(struct buffer *buf, struct tls_session *session)
+key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct
tls_session *session)
{
struct key_state *ks = >key[KS_PRIMARY]; /* primary
key */
@@ -2856,7 +2856,7 @@ tls_process(struct tls_multi *multi,
if (!buf->len && ((ks->state == S_START && !session->opt->server)
|| (ks->state == S_GOT_KEY &&
session->opt->server)))
{
-if (!key_method_2_write(buf, session))
+if (!key_method_2_write(buf, multi, session))
{
goto error;
}
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup
On 19/03/2021 16:31, Arne Schwabe wrote: This option allow migration to a non compression server config while still retraining compatibility with client that have a compression setting in their config. For existing setups that used to have comp-lzo no or another compression setting in their configs it is a difficult to migrate to a setup without compression without replacing all client configs at once especially if OpenVPN 2.3 or earlier clients are in the mix that do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients that support pushing this is not a satisfying solution as the clients log occ mismatches and the "push stub-v2" needs to be in the server config "forever". Signed-off-by: Arne Schwabe --- doc/man-sections/protocol-options.rst | 12 +++- src/openvpn/comp.h| 1 + src/openvpn/multi.c | 41 + src/openvpn/options.c | 6 ++ src/openvpn/ssl.c | 34 ++- src/openvpn/ssl_common.h | 1 + src/openvpn/ssl_util.c| 43 ++ src/openvpn/ssl_util.h| 15 + tests/unit_tests/openvpn/Makefile.am | 14 - tests/unit_tests/openvpn/test_misc.c | 83 +++ 10 files changed, 245 insertions(+), 5 deletions(-) create mode 100644 tests/unit_tests/openvpn/test_misc.c This fails compiling: ../../../src/openvpn/ssl.c: In function ‘key_method_2_write’: ../../../src/openvpn/ssl.c:2280:13: error: ‘multi’ undeclared (first use in this function) if (multi->remote_usescomp && session->opt->mode == MODE_SERVER ^ ../../../src/openvpn/ssl.c:2280:13: note: each undeclared identifier is reported only once for each function it appears in make[3]: *** [Makefile:742: ssl.o] Error 1 -- kind regards, David Sommerseth OpenVPN Inc OpenPGP_signature Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup
This option allow migration to a non compression server config while
still retraining compatibility with client that have a compression
setting in their config.
For existing setups that used to have comp-lzo no or another
compression setting in their configs it is a difficult to migrate to
a setup without compression without replacing all client configs at
once especially if OpenVPN 2.3 or earlier clients are in the mix that
do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients
that support pushing this is not a satisfying solution as the clients
log occ mismatches and the "push stub-v2" needs to be in the server
config "forever".
Signed-off-by: Arne Schwabe
---
doc/man-sections/protocol-options.rst | 12 +++-
src/openvpn/comp.h| 1 +
src/openvpn/multi.c | 41 +
src/openvpn/options.c | 6 ++
src/openvpn/ssl.c | 34 ++-
src/openvpn/ssl_common.h | 1 +
src/openvpn/ssl_util.c| 43 ++
src/openvpn/ssl_util.h| 15 +
tests/unit_tests/openvpn/Makefile.am | 14 -
tests/unit_tests/openvpn/test_misc.c | 83 +++
10 files changed, 245 insertions(+), 5 deletions(-)
create mode 100644 tests/unit_tests/openvpn/test_misc.c
diff --git a/doc/man-sections/protocol-options.rst
b/doc/man-sections/protocol-options.rst
index e9d5d63d..c5cd76dd 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -84,10 +84,10 @@ configured in a compatible way between both the local and
remote side.
--compress algorithm
**DEPRECATED** Enable a compression algorithm. Compression is generally
not recommended. VPN tunnels which use compression are susceptible to
- the VORALCE attack vector.
+ the VORALCE attack vector. See also the :code:`migrate` parameter below.
The ``algorithm`` parameter may be :code:`lzo`, :code:`lz4`,
- :code:`lz4-v2`, :code:`stub`, :code:`stub-v2` or empty.
+ :code:`lz4-v2`, :code:`stub`, :code:`stub-v2`, :code:`migrate` or empty.
LZO and LZ4 are different compression algorithms, with LZ4 generally
offering the best performance with least CPU usage.
@@ -106,6 +106,14 @@ configured in a compatible way between both the local and
remote side.
Note: the :code:`stub` (or empty) option is NOT compatible with the older
option ``--comp-lzo no``.
+ The :code:`migrate` algorithm is a special and is intended to allow
+ migration away from ``--compress``/``--comp-lzo`` options to no compression.
+ This options set the server to no compression mode. If a client
+ is detected that indicates that compression is used the will automatically
+ add ``--push compress stub-v2`` to the client specific configuration
+ if supported by the client and otherwise switch to ``comp-lzo no``
+ and also ``--push comp-lzo`` to the client specific configuration.
+
***Security Considerations***
Compression and encryption is a tricky combination. If an attacker knows
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 5c0322ca..a880e198 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -58,6 +58,7 @@
#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 */
/*
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f7e0f680..3060410d 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2469,6 +2469,46 @@ multi_client_connect_early_setup(struct multi_context *m,
multi_client_connect_setenv(m, mi);
}
+/**
+ * Do the necessary modification for doing the compress mitigrate. This is
+ * implemented as a connect handler as it fits the modify config for a client
+ * paradigm and also is early enough in the chain to be overwritten by another
+ * ccd/script to do compression on a special client.
+ */
+static enum client_connect_return
+multi_client_connect_compress_migrate(struct multi_context *m,
+ struct multi_instance *mi,
+ bool deferred,
+ unsigned int *option_types_found)
+{
+struct options *o = >context.options;
+const char *const peer_info =mi->context.c2.tls_multi->peer_info;
+
+if (!peer_info)
+{
+return CC_RET_SUCCEEDED;
+}
+
+
+if (o->comp.flags & COMP_F_MIGRATE &&
mi->context.c2.tls_multi->remote_usescomp)
+{
+if(strstr(peer_info, "IV_COMP_STUBv2=1"))
+{
+push_option(o, "compress stub-v2", M_USAGE);
+}
+else
+{
+/* Client is
