Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/756?usp=email
to review the following change.
Change subject: Automatically enable --compress migrate on the server
......................................................................
Automatically enable --compress migrate on the server
If we enable LZO compression, automatically switch to
migrate mode.
Change-Id: I00209b880cfcedd93e28f97fc3941d8b85e095f3
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M doc/man-sections/protocol-options.rst
M src/openvpn/comp.h
M src/openvpn/options.c
3 files changed, 56 insertions(+), 62 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/56/756/1
diff --git a/doc/man-sections/protocol-options.rst
b/doc/man-sections/protocol-options.rst
index 8b061d2..b2a23fa 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -30,7 +30,9 @@
framing (stub).
:code:`yes`
- OpenVPN will send and receive compressed packets.
+ **DEPRECATED** This option is an alias for :code:`asym`. Previously
+ it did enable compression for uplink packets, but OpenVPN never
+ compresses uplink packets now.
--auth alg
Authenticate data channel packets and (if enabled) ``tls-auth`` control
@@ -125,6 +127,12 @@
configuration if supported by the client and otherwise switch to ``comp-lzo
no``
and add ``--push comp-lzo`` to the client specific configuration.
+ If used in a server configuration :code:`lzo` is an alias for
:code:`migrate` in
+ current versions of OpenVPN. Compression will only be enabled if there is no
+ other choice. Note that these versions of OpenVPN also never actually
compress
+ any packets. But they still will decompress packets received from the other
side
+ of the connection if required.
+
***Security Considerations***
Compression and encryption is a tricky combination. If an attacker knows
@@ -135,48 +143,31 @@
entirely sure that the above does not apply to your traffic, you are
advised to *not* enable compression.
+ For this reason compression support was removed from current versions
+ of OpenVPN. It will still decompress compressed packets removed via
+ a VPN connection but it will never compress any outgoing packets.
+
--comp-lzo mode
**DEPRECATED** Enable LZO compression algorithm. Compression is
generally not recommended. VPN tunnels which uses compression are
suspectible to the VORALCE attack vector.
- Use LZO compression -- may add up to 1 byte per packet for incompressible
- data. ``mode`` may be :code:`yes`, :code:`no`, or :code:`adaptive`
- (default).
+ Allows the other side of the connection to use LZO compression. Due
+ to difference in packet format this may adds 1 additional byte per packet.
+ With current versions of OpenVPN no actual compression will happen.
- In a server mode setup, it is possible to selectively turn compression
- on or off for individual clients.
+ ``mode`` may be :code:`yes`, :code:`no`, or :code:`adaptive`
+ but there is no actual change in behavior anymore.
- First, make sure the client-side config file enables selective
- compression by having at least one ``--comp-lzo`` directive, such as
- ``--comp-lzo no``. This will turn off compression by default, but allow
- a future directive push from the server to dynamically change the
- :code:`on`/:code:`off`/:code:`adaptive` setting.
-
- Next in a ``--client-config-dir`` file, specify the compression setting
- for the client, for example:
- ::
-
- comp-lzo yes
- push "comp-lzo yes"
-
- The first line sets the ``comp-lzo`` setting for the server side of the
- link, the second sets the client side.
+ In server mode we convert this setting to ``--compress migrate`` to
+ automatically disable it when the client doesn't need it. If you want
+ to remove this setting from your server config you might need to add
+ an explicit ``--compress migrate`` instead if some clients still have
+ any variant of ``--comp-lzo`` in their config.
--comp-noadapt
- **DEPRECATED** When used in conjunction with ``--comp-lzo``, this option
- will disable OpenVPN's adaptive compression algorithm. Normally, adaptive
- compression is enabled with ``--comp-lzo``.
-
- Adaptive compression tries to optimize the case where you have
- compression enabled, but you are sending predominantly incompressible
- (or pre-compressed) packets over the tunnel, such as an FTP or rsync
- transfer of a large, compressed file. With adaptive compression, OpenVPN
- will periodically sample the compression process to measure its
- efficiency. If the data being sent over the tunnel is already
- compressed, the compression efficiency will be very low, triggering
- openvpn to disable compression for a period of time until the next
- re-sample test.
+ **DEPRECATED** This option does not have any effect anymore since current
+ versions of OpenVPN never compress outgoing packets.
--key-direction
Alternative way of specifying the optional direction parameter for the
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index decf0d9..1bc4648 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -32,8 +32,8 @@
* outside of the USE_COMP define */
/* Compression flags */
-#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
/*Removed */
+/*#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 */
@@ -49,6 +49,7 @@
#define COMP_ALG_LZO 2 /* LZO algorithm */
#define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
#define COMP_ALG_LZ4 4 /* LZ4 algorithm */
+#define COMP_ALG_LZO_NO 5 /* --comp-lzo no which is similar to COMP_ALG_STUB,
but no SWAP */
/* algorithm v2 */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 4745ddf..f8688d4 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3448,6 +3448,24 @@
}
#endif /* ifdef _WIN32 */
+ if (options->mode == MODE_SERVER)
+ {
+ /*
+ * Enable comp migrate automatically on server
+ */
+ if (options->comp.alg == COMP_ALG_LZO || options->comp.alg ==
COMP_ALG_LZO_NO)
+ {
+ msg(M_INFO, "DEPRECATED OPTION: LZO compression enabled on the
server side. "
+ "We will enable --compress migrate instead.");
+ options->comp.alg = COMP_ALG_UNDEF;
+ options->comp.flags = COMP_F_MIGRATE;
+ }
+ }
+ else if (options->comp.alg == COMP_ALG_LZO_NO)
+ {
+ options->comp.alg = COMP_ALG_STUB;
+ }
+
#ifdef DEFAULT_PKCS11_MODULE
/* If p11-kit is present on the system then load its p11-kit-proxy.so
* by default if the user asks for PKCS#11 without otherwise specifying
@@ -8450,45 +8468,29 @@
/* All lzo variants do not use swap */
options->comp.flags &= ~COMP_F_SWAP;
+ options->comp.alg = COMP_ALG_LZO;
- if (p[1] && streq(p[1], "no"))
+ if (p[1])
{
- options->comp.alg = COMP_ALG_STUB;
- options->comp.flags &= ~COMP_F_ADAPTIVE;
- }
- else if (p[1])
- {
- if (streq(p[1], "yes"))
+ if (streq(p[1], "no"))
{
- options->comp.alg = COMP_ALG_LZO;
- options->comp.flags &= ~COMP_F_ADAPTIVE;
+ options->comp.alg = COMP_ALG_LZO_NO;
}
- else if (streq(p[1], "adaptive"))
- {
- options->comp.alg = COMP_ALG_LZO;
- options->comp.flags |= COMP_F_ADAPTIVE;
- }
- else
+ /* There is no actual difference anymore between these variants.
+ * We never compress. On the server side we replace this with
+ * --compress migrate later anyway.
+ */
+ else if (!(streq(p[1], "yes") || streq(p[1], "adaptive")))
{
msg(msglevel, "bad comp-lzo option: %s -- must be 'yes', 'no',
or 'adaptive'", p[1]);
goto err;
}
}
- else
- {
- options->comp.alg = COMP_ALG_LZO;
- options->comp.flags |= COMP_F_ADAPTIVE;
- }
show_compression_warning(&options->comp);
}
else if (streq(p[0], "comp-noadapt") && !p[1])
{
- /*
- * We do not need to check here if we allow compression since
- * it only modifies a flag if compression is enabled
- */
- VERIFY_PERMISSION(OPT_P_COMP);
- options->comp.flags &= ~COMP_F_ADAPTIVE;
+ /* NO-OP since we never compress anymore */
}
else if (streq(p[0], "compress") && !p[2])
{
@@ -8517,7 +8519,7 @@
else if (streq(alg, "lzo"))
{
options->comp.alg = COMP_ALG_LZO;
- options->comp.flags &= ~(COMP_F_ADAPTIVE | COMP_F_SWAP);
+ options->comp.flags &= ~COMP_F_SWAP;
}
else if (streq(alg, "lz4"))
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/756?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I00209b880cfcedd93e28f97fc3941d8b85e095f3
Gerrit-Change-Number: 756
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel