Browse code

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 <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241108173851.436-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29718.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Frank Lichtenheld authored on 2024/11/09 02:38:51
Showing 7 changed files
... ...
@@ -59,6 +59,12 @@ OpenSSL 1.0.2 support
59 59
     Support for building with OpenSSL 1.0.2 has been removed. The minimum
60 60
     supported OpenSSL version is now 1.1.0.
61 61
 
62
+Compression on send
63
+    OpenVPN 2.7 will never compress data before sending. Decompression of
64
+    received data is still supported.
65
+    ``--allow-compression yes`` is now an alias for
66
+    ``--allow-compression asym``.
67
+
62 68
 Overview of changes in 2.6
63 69
 ==========================
64 70
 
... ...
@@ -10,17 +10,11 @@ configured in a compatible way between both the local and remote side.
10 10
   dangerous option.  This option allows controlling the behaviour of
11 11
   OpenVPN when compression is used and allowed.
12 12
 
13
-  Valid syntaxes:
14
-  ::
15
-
16
-      allow-compression
17
-      allow-compression mode
18
-
19 13
   The ``mode`` argument can be one of the following values:
20 14
 
21 15
   :code:`asym`
22
-      OpenVPN will only *decompress downlink packets* but *not compress
23
-      uplink packets*.  This also allows migrating to disable compression
16
+      OpenVPN will only *decompress incoming packets* but *not compress
17
+      outgoing packets*.  This also allows migrating to disable compression
24 18
       when changing both server and client configurations to remove
25 19
       compression at the same time is not a feasible option.
26 20
 
... ...
@@ -30,7 +24,9 @@ configured in a compatible way between both the local and remote side.
30 30
       framing (stub).
31 31
 
32 32
   :code:`yes`
33
-      OpenVPN will send and receive compressed packets.
33
+      **DEPRECATED** This option is an alias for :code:`asym`. Previously
34
+      it did enable compression for outgoing packets, but OpenVPN never
35
+      compresses packets on send now.
34 36
 
35 37
 --auth alg
36 38
   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.
135 135
   entirely sure that the above does not apply to your traffic, you are
136 136
   advised to *not* enable compression.
137 137
 
138
+  For this reason compression support was removed from current versions
139
+  of OpenVPN. It will still decompress compressed packets received via
140
+  a VPN connection but it will never compress any outgoing packets.
141
+
138 142
 --comp-lzo mode
139 143
   **DEPRECATED** Enable LZO compression algorithm.  Compression is
140 144
   generally not recommended.  VPN tunnels which uses compression are
141 145
   suspectible to the VORALCE attack vector.
142 146
 
143
-  Use LZO compression -- may add up to 1 byte per packet for incompressible
144
-  data. ``mode`` may be :code:`yes`, :code:`no`, or :code:`adaptive`
145
-  (default).
146
-
147
-  In a server mode setup, it is possible to selectively turn compression
148
-  on or off for individual clients.
149
-
150
-  First, make sure the client-side config file enables selective
151
-  compression by having at least one ``--comp-lzo`` directive, such as
152
-  ``--comp-lzo no``. This will turn off compression by default, but allow
153
-  a future directive push from the server to dynamically change the
154
-  :code:`on`/:code:`off`/:code:`adaptive` setting.
155
-
156
-  Next in a ``--client-config-dir`` file, specify the compression setting
157
-  for the client, for example:
158
-  ::
147
+  Allows the other side of the connection to use LZO compression. Due
148
+  to difference in packet format this may add 1 additional byte per packet.
149
+  With current versions of OpenVPN no actual compression will happen.
159 150
 
160
-    comp-lzo yes
161
-    push "comp-lzo yes"
151
+  ``mode`` may be :code:`yes`, :code:`no`, or :code:`adaptive`
152
+  but there is no actual change in behavior anymore.
162 153
 
163
-  The first line sets the ``comp-lzo`` setting for the server side of the
164
-  link, the second sets the client side.
165 154
 
166 155
 --comp-noadapt
167
-  **DEPRECATED** When used in conjunction with ``--comp-lzo``, this option
168
-  will disable OpenVPN's adaptive compression algorithm. Normally, adaptive
169
-  compression is enabled with ``--comp-lzo``.
170
-
171
-  Adaptive compression tries to optimize the case where you have
172
-  compression enabled, but you are sending predominantly incompressible
173
-  (or pre-compressed) packets over the tunnel, such as an FTP or rsync
174
-  transfer of a large, compressed file. With adaptive compression, OpenVPN
175
-  will periodically sample the compression process to measure its
176
-  efficiency. If the data being sent over the tunnel is already
177
-  compressed, the compression efficiency will be very low, triggering
178
-  openvpn to disable compression for a period of time until the next
179
-  re-sample test.
156
+  **DEPRECATED** This option does not have any effect anymore since current
157
+  versions of OpenVPN never compress outgoing packets.
180 158
 
181 159
 --key-direction
182 160
   Alternative way of specifying the optional direction parameter for the
... ...
@@ -55,129 +55,40 @@ lz4_compress_uninit(struct compress_context *compctx)
55 55
 {
56 56
 }
57 57
 
58
-static bool
59
-do_lz4_compress(struct buffer *buf,
60
-                struct buffer *work,
61
-                struct compress_context *compctx,
62
-                const struct frame *frame)
63
-{
64
-    /*
65
-     * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
66
-     * and asymmetric compression must be disabled
67
-     */
68
-    if (buf->len >= COMPRESS_THRESHOLD && (compctx->flags & COMP_F_ALLOW_COMPRESS))
69
-    {
70
-        const size_t ps = frame->buf.payload_size;
71
-        int zlen_max = ps + COMP_EXTRA_BUFFER(ps);
72
-        int zlen;
73
-
74
-        ASSERT(buf_init(work, frame->buf.headroom));
75
-        ASSERT(buf_safe(work, zlen_max));
76
-
77
-        if (buf->len > ps)
78
-        {
79
-            dmsg(D_COMP_ERRORS, "LZ4 compression buffer overflow");
80
-            buf->len = 0;
81
-            return false;
82
-        }
83
-
84
-        zlen = LZ4_compress_default((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max);
85
-
86
-        if (zlen <= 0)
87
-        {
88
-            dmsg(D_COMP_ERRORS, "LZ4 compression error");
89
-            buf->len = 0;
90
-            return false;
91
-        }
92
-
93
-        ASSERT(buf_safe(work, zlen));
94
-        work->len = zlen;
95
-
96
-
97
-        dmsg(D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
98
-        compctx->pre_compress += buf->len;
99
-        compctx->post_compress += work->len;
100
-        return true;
101
-    }
102
-    return false;
103
-}
104
-
105
-
58
+/* Doesn't do any actual compression anymore */
106 59
 static void
107 60
 lz4_compress(struct buffer *buf, struct buffer work,
108 61
              struct compress_context *compctx,
109 62
              const struct frame *frame)
110 63
 {
111
-    bool compressed;
112 64
     if (buf->len <= 0)
113 65
     {
114 66
         return;
115 67
     }
116 68
 
117
-    compressed = do_lz4_compress(buf, &work, compctx, frame);
118
-
119
-    /* On error do_lz4_compress sets buf len to zero, just return */
120
-    if (buf->len == 0)
121
-    {
122
-        return;
123
-    }
69
+    uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
70
+    uint8_t *head = BPTR(buf);
71
+    uint8_t *tail = BEND(buf);
72
+    ASSERT(buf_safe(buf, 1));
73
+    ++buf->len;
124 74
 
125
-    /* did compression save us anything? */
126
-    {
127
-        uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
128
-        if (compressed && work.len < buf->len)
129
-        {
130
-            *buf = work;
131
-            comp_head_byte = LZ4_COMPRESS_BYTE;
132
-        }
133
-
134
-        {
135
-            uint8_t *head = BPTR(buf);
136
-            uint8_t *tail  = BEND(buf);
137
-            ASSERT(buf_safe(buf, 1));
138
-            ++buf->len;
139
-
140
-            /* move head byte of payload to tail */
141
-            *tail = *head;
142
-            *head = comp_head_byte;
143
-        }
144
-    }
75
+    /* move head byte of payload to tail */
76
+    *tail = *head;
77
+    *head = comp_head_byte;
145 78
 }
146 79
 
147
-
80
+/* Doesn't do any actual compression anymore */
148 81
 static void
149 82
 lz4v2_compress(struct buffer *buf, struct buffer work,
150 83
                struct compress_context *compctx,
151 84
                const struct frame *frame)
152 85
 {
153
-    bool compressed;
154 86
     if (buf->len <= 0)
155 87
     {
156 88
         return;
157 89
     }
158 90
 
159
-    compressed = do_lz4_compress(buf, &work, compctx, frame);
160
-
161
-    /* On Error just return */
162
-    if (buf->len == 0)
163
-    {
164
-        return;
165
-    }
166
-
167
-    /* did compression save us anything?  Include 2 byte compression header
168
-     * in calculation */
169
-    if (compressed && work.len + 2 < buf->len)
170
-    {
171
-        ASSERT(buf_prepend(&work, 2));
172
-        uint8_t *head = BPTR(&work);
173
-        head[0] = COMP_ALGV2_INDICATOR_BYTE;
174
-        head[1] = COMP_ALGV2_LZ4_BYTE;
175
-        *buf = work;
176
-    }
177
-    else
178
-    {
179
-        compv2_escape_data_ifneeded(buf);
180
-    }
91
+    compv2_escape_data_ifneeded(buf);
181 92
 }
182 93
 
183 94
 static void
... ...
@@ -32,8 +32,10 @@
32 32
  * outside of the USE_COMP define */
33 33
 
34 34
 /* Compression flags */
35
-#define COMP_F_ADAPTIVE             (1<<0) /* COMP_ALG_LZO only */
36
-#define COMP_F_ALLOW_COMPRESS       (1<<1) /* not only downlink is compressed but also uplink */
35
+/* Removed
36
+ #define COMP_F_ADAPTIVE             (1<<0) / * COMP_ALG_LZO only * /
37
+ #define COMP_F_ALLOW_COMPRESS       (1<<1) / * not only incoming is compressed but also outgoing * /
38
+ */
37 39
 #define COMP_F_SWAP                 (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
38 40
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
39 41
 #define COMP_F_ALLOW_STUB_ONLY      (1<<4) /* Only accept stub compression, even with COMP_F_ADVERTISE_STUBS_ONLY
... ...
@@ -426,8 +426,7 @@ dco_check_option(int msglevel, const struct options *o)
426 426
 
427 427
 #if defined(USE_COMP)
428 428
     if (o->comp.alg != COMP_ALG_UNDEF
429
-        || o->comp.flags & COMP_F_ALLOW_ASYM
430
-        || o->comp.flags & COMP_F_ALLOW_COMPRESS)
429
+        || o->comp.flags & COMP_F_ALLOW_ASYM)
431 430
     {
432 431
         msg(msglevel, "Note: '--allow-compression' is not set to 'no', disabling data channel offload.");
433 432
 
... ...
@@ -39,61 +39,6 @@
39 39
 
40 40
 #include "memdbg.h"
41 41
 
42
-/**
43
- * Perform adaptive compression housekeeping.
44
- *
45
- * @param ac the adaptive compression state structure.
46
- *
47
- * @return
48
- */
49
-static bool
50
-lzo_adaptive_compress_test(struct lzo_adaptive_compress *ac)
51
-{
52
-    const bool save = ac->compress_state;
53
-    const time_t local_now = now;
54
-
55
-    if (!ac->compress_state)
56
-    {
57
-        if (local_now >= ac->next)
58
-        {
59
-            if (ac->n_total > AC_MIN_BYTES
60
-                && (ac->n_total - ac->n_comp) < (ac->n_total / (100 / AC_SAVE_PCT)))
61
-            {
62
-                ac->compress_state = true;
63
-                ac->next = local_now + AC_OFF_SEC;
64
-            }
65
-            else
66
-            {
67
-                ac->next = local_now + AC_SAMP_SEC;
68
-            }
69
-            dmsg(D_COMP, "lzo_adaptive_compress_test: comp=%d total=%d", ac->n_comp, ac->n_total);
70
-            ac->n_total = ac->n_comp = 0;
71
-        }
72
-    }
73
-    else
74
-    {
75
-        if (local_now >= ac->next)
76
-        {
77
-            ac->next = local_now + AC_SAMP_SEC;
78
-            ac->n_total = ac->n_comp = 0;
79
-            ac->compress_state = false;
80
-        }
81
-    }
82
-
83
-    if (ac->compress_state != save)
84
-    {
85
-        dmsg(D_COMP_LOW, "Adaptive compression state %s", (ac->compress_state ? "OFF" : "ON"));
86
-    }
87
-
88
-    return !ac->compress_state;
89
-}
90
-
91
-static inline void
92
-lzo_adaptive_compress_data(struct lzo_adaptive_compress *ac, int n_total, int n_comp)
93
-{
94
-    ac->n_total += n_total;
95
-    ac->n_comp += n_comp;
96
-}
97 42
 
98 43
 static void
99 44
 lzo_compress_init(struct compress_context *compctx)
... ...
@@ -118,92 +63,13 @@ lzo_compress_uninit(struct compress_context *compctx)
118 118
     compctx->wu.lzo.wmem = NULL;
119 119
 }
120 120
 
121
-static inline bool
122
-lzo_compression_enabled(struct compress_context *compctx)
123
-{
124
-    if (!(compctx->flags & COMP_F_ALLOW_COMPRESS))
125
-    {
126
-        return false;
127
-    }
128
-    else
129
-    {
130
-        if (compctx->flags & COMP_F_ADAPTIVE)
131
-        {
132
-            return lzo_adaptive_compress_test(&compctx->wu.lzo.ac);
133
-        }
134
-        else
135
-        {
136
-            return true;
137
-        }
138
-    }
139
-}
140
-
141 121
 static void
142 122
 lzo_compress(struct buffer *buf, struct buffer work,
143 123
              struct compress_context *compctx,
144 124
              const struct frame *frame)
145 125
 {
146
-    lzo_uint zlen = 0;
147
-    int err;
148
-    bool compressed = false;
149
-
150
-    if (buf->len <= 0)
151
-    {
152
-        return;
153
-    }
154
-
155
-    /*
156
-     * In order to attempt compression, length must be at least COMPRESS_THRESHOLD,
157
-     * and our adaptive level must give the OK.
158
-     */
159
-    if (buf->len >= COMPRESS_THRESHOLD && lzo_compression_enabled(compctx))
160
-    {
161
-        const size_t ps = frame->buf.payload_size;
162
-        ASSERT(buf_init(&work, frame->buf.headroom));
163
-        ASSERT(buf_safe(&work, ps + COMP_EXTRA_BUFFER(ps)));
164
-
165
-        if (buf->len > ps)
166
-        {
167
-            dmsg(D_COMP_ERRORS, "LZO compression buffer overflow");
168
-            buf->len = 0;
169
-            return;
170
-        }
171
-
172
-        err = LZO_COMPRESS(BPTR(buf), BLEN(buf), BPTR(&work), &zlen, compctx->wu.lzo.wmem);
173
-        if (err != LZO_E_OK)
174
-        {
175
-            dmsg(D_COMP_ERRORS, "LZO compression error: %d", err);
176
-            buf->len = 0;
177
-            return;
178
-        }
179
-
180
-        ASSERT(buf_safe(&work, zlen));
181
-        work.len = zlen;
182
-        compressed = true;
183
-
184
-        dmsg(D_COMP, "LZO compress %d -> %d", buf->len, work.len);
185
-        compctx->pre_compress += buf->len;
186
-        compctx->post_compress += work.len;
187
-
188
-        /* tell adaptive level about our success or lack thereof in getting any size reduction */
189
-        if (compctx->flags & COMP_F_ADAPTIVE)
190
-        {
191
-            lzo_adaptive_compress_data(&compctx->wu.lzo.ac, buf->len, work.len);
192
-        }
193
-    }
194
-
195
-    /* did compression save us anything ? */
196
-    if (compressed && work.len < buf->len)
197
-    {
198
-        uint8_t *header = buf_prepend(&work, 1);
199
-        *header = LZO_COMPRESS_BYTE;
200
-        *buf = work;
201
-    }
202
-    else
203
-    {
204
-        uint8_t *header = buf_prepend(buf, 1);
205
-        *header = NO_COMPRESS_BYTE;
206
-    }
126
+    uint8_t *header = buf_prepend(buf, 1);
127
+    *header = NO_COMPRESS_BYTE;
207 128
 }
208 129
 
209 130
 static void
... ...
@@ -5715,17 +5715,10 @@ show_compression_warning(struct compress_options *info)
5715 5715
 {
5716 5716
     if (comp_non_stub_enabled(info))
5717 5717
     {
5718
-        /*
5719
-         * Check if already displayed the strong warning and enabled full
5720
-         * compression
5721
-         */
5722
-        if (!(info->flags & COMP_F_ALLOW_COMPRESS))
5723
-        {
5724
-            msg(M_WARN, "WARNING: Compression for receiving enabled. "
5725
-                "Compression has been used in the past to break encryption. "
5726
-                "Sent packets are not compressed unless \"allow-compression yes\" "
5727
-                "is also set.");
5728
-        }
5718
+        msg(M_WARN, "WARNING: Compression for receiving enabled. "
5719
+            "Compression has been used in the past to break encryption. "
5720
+            "Compression support is deprecated and we recommend to disable "
5721
+            "it completely.");
5729 5722
     }
5730 5723
 }
5731 5724
 
... ...
@@ -8435,18 +8428,14 @@ add_option(struct options *options,
8435 8435
         }
8436 8436
         else if (streq(p[1], "asym"))
8437 8437
         {
8438
-            options->comp.flags &= ~COMP_F_ALLOW_COMPRESS;
8439 8438
             options->comp.flags |= COMP_F_ALLOW_ASYM;
8440 8439
         }
8441 8440
         else if (streq(p[1], "yes"))
8442 8441
         {
8443
-            msg(M_WARN, "WARNING: Compression for sending and receiving enabled. Compression has "
8444
-                "been used in the past to break encryption. Allowing compression allows "
8445
-                "attacks that break encryption. Using \"--allow-compression yes\" is "
8446
-                "strongly discouraged for common usage. See --compress in the manual "
8447
-                "page for more information ");
8442
+            msg(M_WARN, "DEPRECATED OPTION: \"--allow-compression yes\" has been removed. "
8443
+                "We will use \"asym\" mode instead. See the manual page for more information.");
8448 8444
 
8449
-            options->comp.flags |= COMP_F_ALLOW_COMPRESS;
8445
+            options->comp.flags |= COMP_F_ALLOW_ASYM;
8450 8446
         }
8451 8447
         else
8452 8448
         {
... ...
@@ -8461,45 +8450,29 @@ add_option(struct options *options,
8461 8461
 
8462 8462
         /* All lzo variants do not use swap */
8463 8463
         options->comp.flags &= ~COMP_F_SWAP;
8464
+        options->comp.alg = COMP_ALG_LZO;
8464 8465
 
8465
-        if (p[1] && streq(p[1], "no"))
8466
-        {
8467
-            options->comp.alg = COMP_ALG_STUB;
8468
-            options->comp.flags &= ~COMP_F_ADAPTIVE;
8469
-        }
8470
-        else if (p[1])
8466
+        if (p[1])
8471 8467
         {
8472
-            if (streq(p[1], "yes"))
8468
+            if (streq(p[1], "no"))
8473 8469
             {
8474
-                options->comp.alg = COMP_ALG_LZO;
8475
-                options->comp.flags &= ~COMP_F_ADAPTIVE;
8470
+                options->comp.alg = COMP_ALG_STUB;
8476 8471
             }
8477
-            else if (streq(p[1], "adaptive"))
8478
-            {
8479
-                options->comp.alg = COMP_ALG_LZO;
8480
-                options->comp.flags |= COMP_F_ADAPTIVE;
8481
-            }
8482
-            else
8472
+            /* There is no actual difference anymore between these variants.
8473
+             * We never compress. On the server side we replace this with
8474
+             * --compress migrate later anyway.
8475
+             */
8476
+            else if (!(streq(p[1], "yes") || streq(p[1], "adaptive")))
8483 8477
             {
8484 8478
                 msg(msglevel, "bad comp-lzo option: %s -- must be 'yes', 'no', or 'adaptive'", p[1]);
8485 8479
                 goto err;
8486 8480
             }
8487 8481
         }
8488
-        else
8489
-        {
8490
-            options->comp.alg = COMP_ALG_LZO;
8491
-            options->comp.flags |= COMP_F_ADAPTIVE;
8492
-        }
8493 8482
         show_compression_warning(&options->comp);
8494 8483
     }
8495 8484
     else if (streq(p[0], "comp-noadapt") && !p[1])
8496 8485
     {
8497
-        /*
8498
-         * We do not need to check here if we allow compression since
8499
-         * it only modifies a flag if compression is enabled
8500
-         */
8501
-        VERIFY_PERMISSION(OPT_P_COMP);
8502
-        options->comp.flags &= ~COMP_F_ADAPTIVE;
8486
+        /* NO-OP since we never compress anymore */
8503 8487
     }
8504 8488
     else if (streq(p[0], "compress") && !p[2])
8505 8489
     {
... ...
@@ -8528,7 +8501,7 @@ add_option(struct options *options,
8528 8528
         else if (streq(alg, "lzo"))
8529 8529
         {
8530 8530
             options->comp.alg = COMP_ALG_LZO;
8531
-            options->comp.flags &= ~(COMP_F_ADAPTIVE | COMP_F_SWAP);
8531
+            options->comp.flags &= ~COMP_F_SWAP;
8532 8532
         }
8533 8533
         else if (streq(alg, "lz4"))
8534 8534
         {