Re: [Openvpn-devel] [PATCH v5] Make compression asymmetric by default and add warnings
Checked that only diffs are warning messages (as agreed) and different indentation in #define VERIFY_PERMISSION (probably caused by uncrustify). Acked-by: Lev Stipakov pe 26. kesäk. 2020 klo 14.06 Arne Schwabe (a...@rfc2549.org) kirjoitti: > > This commit introduces the allow-compression option that allow > changing the new default to the previous default or to a stricter > version. > > Warning for comp-lzo/compress are not generated in the post option check > (options_postprocess_mutate) since these warnings should also be shown > on pushed options. Moving the showing the warning showing for > allow-compression to options_postprocess_mutate will complicate the > option handling without giving any other benefit. > > Patch V2: fix spelling and grammer (thanks tincantech), also fix >uncompressiable to incompressible in three other instances in the >source code > > Patch V3: fix overlong lines. Do not allow compression to be pushed > > Patch V4: rename COMP_F_NO_ASYM to COMP_F_ALLOW_COMPRESS, fix style. > The logic of warnings etc in options.c has not been changed > since adding all the code to mutate_options would a lot more > and more complicated code and after discussion we decided that > it is okay as is. > > Patch V5: Reword warnings, rebase on master > > Signed-off-by: Arne Schwabe > --- > doc/openvpn.8 | 44 +--- > src/openvpn/comp-lz4.c | 3 +- > src/openvpn/comp.c | 2 +- > src/openvpn/comp.h | 16 -- > src/openvpn/lzo.c | 2 +- > src/openvpn/mtu.h | 2 +- > src/openvpn/options.c | 115 +++-- > 7 files changed, 153 insertions(+), 31 deletions(-) > > diff --git a/doc/openvpn.8 b/doc/openvpn.8 > index dcc72abe..03ae5ac5 100644 > --- a/doc/openvpn.8 > +++ b/doc/openvpn.8 > @@ -2545,26 +2545,54 @@ Enable a compression algorithm. > > The > .B algorithm > -parameter may be "lzo", "lz4", or empty. LZO and LZ4 > -are different compression algorithms, with LZ4 generally > +parameter may be "lzo", "lz4", "lz4\-v2", "stub", "stub\-v2" or empty. > +LZO and LZ4 are different compression algorithms, with LZ4 generally > offering the best performance with least CPU usage. > For backwards compatibility with OpenVPN versions before v2.4, use "lzo" > (which is identical to the older option "\-\-comp\-lzo yes"). > > +The "lz4\-v2" and "stub\-v2" variants implement a better framing that does > not add > +overhead when packets cannot be compressed. All other variants always add > one extra > +framing byte compared to no compression framing. > + > If the > .B algorithm > -parameter is empty, compression will be turned off, but the packet > -framing for compression will still be enabled, allowing a different > -setting to be pushed later. > +parameter is "stub", "stub\-v2", or empty, compression will be turned off, > but > +the packet framing for compression will still be enabled, allowing a > different > +setting to be pushed later. Additionally, "stub" and "stub\-v2" will disable > +announcing lzo and lz4 compression support via "IV_" variables to the server. > + > > .B Security Considerations > > Compression and encryption is a tricky combination. If an attacker knows or > is > able to control (parts of) the plaintext of packets that contain secrets, the > attacker might be able to extract the secret if compression is enabled. See > -e.g. the CRIME and BREACH attacks on TLS which also leverage compression to > -break encryption. If you are not entirely sure that the above does not apply > -to your traffic, you are advised to *not* enable compression. > +e.g. the CRIME and BREACH attacks on TLS and VORACLE on VPNs which also > leverage > +compression to break encryption. If you are not entirely sure that the > above does > +not apply to your traffic, you are advised to *not* enable compression. > + > +.\"* > +.TP > +.B \-\-allow\-compression [mode] > +As described in > +\.B \-\-compress > +option, compression is potentially dangerous option. This option allows > +controlling the behaviour of OpenVPN when compression is used and allowed. > +.B mode > +may be "yes", "no", or "asym" (default). > + > +With allow\-compression set to "no", OpenVPN will refuse any non stub > +compression. With "yes" OpenVPN will send and receive compressed packets. > +With "asym", the default, OpenVPN will only decompress (downlink) packets but > +not compress (uplink) packets. This also allows migrating to disable > compression > +when changing both server and client configurations to remove compression at > the > +same time is not a feasible option. > + > +The default of asym has been chosen to maximise compatibility with existing > +configuration while at the same time phasing out compression in existing > +deployment by disabling compression on the uplink, effectively completely > disabling > +compression if both
[Openvpn-devel] [PATCH v5] Make compression asymmetric by default and add warnings
This commit introduces the allow-compression option that allow
changing the new default to the previous default or to a stricter
version.
Warning for comp-lzo/compress are not generated in the post option check
(options_postprocess_mutate) since these warnings should also be shown
on pushed options. Moving the showing the warning showing for
allow-compression to options_postprocess_mutate will complicate the
option handling without giving any other benefit.
Patch V2: fix spelling and grammer (thanks tincantech), also fix
uncompressiable to incompressible in three other instances in the
source code
Patch V3: fix overlong lines. Do not allow compression to be pushed
Patch V4: rename COMP_F_NO_ASYM to COMP_F_ALLOW_COMPRESS, fix style.
The logic of warnings etc in options.c has not been changed
since adding all the code to mutate_options would a lot more
and more complicated code and after discussion we decided that
it is okay as is.
Patch V5: Reword warnings, rebase on master
Signed-off-by: Arne Schwabe
---
doc/openvpn.8 | 44 +---
src/openvpn/comp-lz4.c | 3 +-
src/openvpn/comp.c | 2 +-
src/openvpn/comp.h | 16 --
src/openvpn/lzo.c | 2 +-
src/openvpn/mtu.h | 2 +-
src/openvpn/options.c | 115 +++--
7 files changed, 153 insertions(+), 31 deletions(-)
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index dcc72abe..03ae5ac5 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -2545,26 +2545,54 @@ Enable a compression algorithm.
The
.B algorithm
-parameter may be "lzo", "lz4", or empty. LZO and LZ4
-are different compression algorithms, with LZ4 generally
+parameter may be "lzo", "lz4", "lz4\-v2", "stub", "stub\-v2" or empty.
+LZO and LZ4 are different compression algorithms, with LZ4 generally
offering the best performance with least CPU usage.
For backwards compatibility with OpenVPN versions before v2.4, use "lzo"
(which is identical to the older option "\-\-comp\-lzo yes").
+The "lz4\-v2" and "stub\-v2" variants implement a better framing that does not
add
+overhead when packets cannot be compressed. All other variants always add one
extra
+framing byte compared to no compression framing.
+
If the
.B algorithm
-parameter is empty, compression will be turned off, but the packet
-framing for compression will still be enabled, allowing a different
-setting to be pushed later.
+parameter is "stub", "stub\-v2", or empty, compression will be turned off, but
+the packet framing for compression will still be enabled, allowing a different
+setting to be pushed later. Additionally, "stub" and "stub\-v2" will disable
+announcing lzo and lz4 compression support via "IV_" variables to the server.
+
.B Security Considerations
Compression and encryption is a tricky combination. If an attacker knows or is
able to control (parts of) the plaintext of packets that contain secrets, the
attacker might be able to extract the secret if compression is enabled. See
-e.g. the CRIME and BREACH attacks on TLS which also leverage compression to
-break encryption. If you are not entirely sure that the above does not apply
-to your traffic, you are advised to *not* enable compression.
+e.g. the CRIME and BREACH attacks on TLS and VORACLE on VPNs which also
leverage
+compression to break encryption. If you are not entirely sure that the above
does
+not apply to your traffic, you are advised to *not* enable compression.
+
+.\"*
+.TP
+.B \-\-allow\-compression [mode]
+As described in
+\.B \-\-compress
+option, compression is potentially dangerous option. This option allows
+controlling the behaviour of OpenVPN when compression is used and allowed.
+.B mode
+may be "yes", "no", or "asym" (default).
+
+With allow\-compression set to "no", OpenVPN will refuse any non stub
+compression. With "yes" OpenVPN will send and receive compressed packets.
+With "asym", the default, OpenVPN will only decompress (downlink) packets but
+not compress (uplink) packets. This also allows migrating to disable
compression
+when changing both server and client configurations to remove compression at
the
+same time is not a feasible option.
+
+The default of asym has been chosen to maximise compatibility with existing
+configuration while at the same time phasing out compression in existing
+deployment by disabling compression on the uplink, effectively completely
disabling
+compression if both client and server are upgraded.
.\"*
.TP
diff --git a/src/openvpn/comp-lz4.c b/src/openvpn/comp-lz4.c
index f52fdbfb..30e6da95 100644
--- a/src/openvpn/comp-lz4.c
+++ b/src/openvpn/comp-lz4.c
@@ -70,8 +70,9 @@ do_lz4_compress(struct buffer *buf,
{
/*
* In order to attempt compression, length must be at least
COMPRESS_THRESHOLD.
+ * and asymmetric compression must be disabled
