Remove support for compression on send

We can't disable compression support on receive because that would break too many configurations out there. But we can remove the support for compressing outgoing traffic, it was disabled by default anyway. Makes "--allow-compression yes" an alias for "--allow-compression asym" and removes all resulting dead code. Change-Id: I402ba016b75cfcfec4fc8b2b01cc4eca7e2bcc60 Signed-off-by: Frank Lichtenheld <[email protected]> Acked-by: Arne Schwabe <[email protected]> Acked-by: Gert Doering <[email protected]> Message-Id: <[email protected]> URL: https://www.mail-archive.com/[email protected]/msg29718.html Signed-off-by: Gert Doering <[email protected]>
OpenVPN · Nov 9, 2024 · 7f0214c · 7f0214c
1 parent d5b4713
commit 7f0214c
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 327 deletions.
diff --git a/Changes.rst b/Changes.rst
@@ -59,6 +59,12 @@ OpenSSL 1.0.2 support
     Support for building with OpenSSL 1.0.2 has been removed. The minimum
     supported OpenSSL version is now 1.1.0.
 
+Compression on send
+    OpenVPN 2.7 will never compress data before sending. Decompression of
+    received data is still supported.
+    ``--allow-compression yes`` is now an alias for
+    ``--allow-compression asym``.
+
 Overview of changes in 2.6
 ==========================
 

diff --git a/doc/man-sections/protocol-options.rst b/doc/man-sections/protocol-options.rst
@@ -10,17 +10,11 @@ configured in a compatible way between both the local and remote side.
   dangerous option.  This option allows controlling the behaviour of
   OpenVPN when compression is used and allowed.
 
-  Valid syntaxes:
-  ::
-
-      allow-compression
-      allow-compression mode
-
   The ``mode`` argument can be one of the following values:
 
   :code:`asym`
-      OpenVPN will only *decompress downlink packets* but *not compress
-      uplink packets*.  This also allows migrating to disable compression
+      OpenVPN will only *decompress incoming packets* but *not compress
+      outgoing 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.
 
@@ -30,7 +24,9 @@ configured in a compatible way between both the local and remote side.
       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 outgoing packets, but OpenVPN never
+      compresses packets on send now.
 
 --auth alg
   Authenticate data channel packets and (if enabled) ``tls-auth`` control
@@ -135,48 +131,26 @@ configured in a compatible way between both the local and remote side.
   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 received 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).
-
-  In a server mode setup, it is possible to selectively turn compression
-  on or off for individual clients.
-
-  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:
-  ::
+  Allows the other side of the connection to use LZO compression. Due
+  to difference in packet format this may add 1 additional byte per packet.
+  With current versions of OpenVPN no actual compression will happen.
 
-    comp-lzo yes
-    push "comp-lzo yes"
+  ``mode`` may be :code:`yes`, :code:`no`, or :code:`adaptive`
+  but there is no actual change in behavior anymore.
 
-  The first line sets the ``comp-lzo`` setting for the server side of the
-  link, the second sets the client side.
 
 --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-lz4.c b/src/openvpn/comp-lz4.c
@@ -55,129 +55,40 @@ lz4_compress_uninit(struct compress_context *compctx)
 {
 }
 
-static bool
-do_lz4_compress(struct buffer *buf,
-                struct buffer *work,
-                struct compress_context *compctx,
-                const struct frame *frame)
-{
-    /*
-     * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
-     * and asymmetric compression must be disabled
-     */
-    if (buf->len >= COMPRESS_THRESHOLD && (compctx->flags & COMP_F_ALLOW_COMPRESS))
-    {
-        const size_t ps = frame->buf.payload_size;
-        int zlen_max = ps + COMP_EXTRA_BUFFER(ps);
-        int zlen;
-
-        ASSERT(buf_init(work, frame->buf.headroom));
-        ASSERT(buf_safe(work, zlen_max));
-
-        if (buf->len > ps)
-        {
-            dmsg(D_COMP_ERRORS, "LZ4 compression buffer overflow");
-            buf->len = 0;
-            return false;
-        }
-
-        zlen = LZ4_compress_default((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max);
-
-        if (zlen <= 0)
-        {
-            dmsg(D_COMP_ERRORS, "LZ4 compression error");
-            buf->len = 0;
-            return false;
-        }
-
-        ASSERT(buf_safe(work, zlen));
-        work->len = zlen;
-
-
-        dmsg(D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
-        compctx->pre_compress += buf->len;
-        compctx->post_compress += work->len;
-        return true;
-    }
-    return false;
-}
-
-
+/* Doesn't do any actual compression anymore */
 static void
 lz4_compress(struct buffer *buf, struct buffer work,
              struct compress_context *compctx,
              const struct frame *frame)
 {
-    bool compressed;
     if (buf->len <= 0)
     {
         return;
     }
 
-    compressed = do_lz4_compress(buf, &work, compctx, frame);
-
-    /* On error do_lz4_compress sets buf len to zero, just return */
-    if (buf->len == 0)
-    {
-        return;
-    }
+    uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
+    uint8_t *head = BPTR(buf);
+    uint8_t *tail = BEND(buf);
+    ASSERT(buf_safe(buf, 1));
+    ++buf->len;
 
-    /* did compression save us anything? */
-    {
-        uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
-        if (compressed && work.len < buf->len)
-        {
-            *buf = work;
-            comp_head_byte = LZ4_COMPRESS_BYTE;
-        }
-
-        {
-            uint8_t *head = BPTR(buf);
-            uint8_t *tail  = BEND(buf);
-            ASSERT(buf_safe(buf, 1));
-            ++buf->len;
-
-            /* move head byte of payload to tail */
-            *tail = *head;
-            *head = comp_head_byte;
-        }
-    }
+    /* move head byte of payload to tail */
+    *tail = *head;
+    *head = comp_head_byte;
 }
 
-
+/* Doesn't do any actual compression anymore */
 static void
 lz4v2_compress(struct buffer *buf, struct buffer work,
                struct compress_context *compctx,
                const struct frame *frame)
 {
-    bool compressed;
     if (buf->len <= 0)
     {
         return;
     }
 
-    compressed = do_lz4_compress(buf, &work, compctx, frame);
-
-    /* On Error just return */
-    if (buf->len == 0)
-    {
-        return;
-    }
-
-    /* did compression save us anything?  Include 2 byte compression header
-     * in calculation */
-    if (compressed && work.len + 2 < buf->len)
-    {
-        ASSERT(buf_prepend(&work, 2));
-        uint8_t *head = BPTR(&work);
-        head[0] = COMP_ALGV2_INDICATOR_BYTE;
-        head[1] = COMP_ALGV2_LZ4_BYTE;
-        *buf = work;
-    }
-    else
-    {
-        compv2_escape_data_ifneeded(buf);
-    }
+    compv2_escape_data_ifneeded(buf);
 }
 
 static void

diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
@@ -32,8 +32,10 @@
  * outside of the USE_COMP define */
 
 /* 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 */
+/* Removed
+ #define COMP_F_ADAPTIVE             (1<<0) / * COMP_ALG_LZO only * /
+ #define COMP_F_ALLOW_COMPRESS       (1<<1) / * not only incoming is compressed but also outgoing * /
+ */
 #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

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
@@ -426,8 +426,7 @@ dco_check_option(int msglevel, const struct options *o)
 
 #if defined(USE_COMP)
     if (o->comp.alg != COMP_ALG_UNDEF
-        || o->comp.flags & COMP_F_ALLOW_ASYM
-        || o->comp.flags & COMP_F_ALLOW_COMPRESS)
+        || o->comp.flags & COMP_F_ALLOW_ASYM)
     {
         msg(msglevel, "Note: '--allow-compression' is not set to 'no', disabling data channel offload.");