Browse code

Refuse connection if server pushes an option contradicting allow-compress

This removes also the checks in options.c itself as they we now bail out
later and no longer need to ignore them during parsing.

Change-Id: I872c06f402c35112194ba77c3d6aee78e22547cb
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230323170601.1256132-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26503.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e86bc8b2967484afdb1e96efddb8d91185c4cc2c)

Arne Schwabe authored on 2023/03/24 02:05:59
Showing 6 changed files
... ...
@@ -10,6 +10,10 @@ User visible changes
10 10
 - The ``client-pending-auth`` management command now requires also the
11 11
   key id. The management version has been changed to 5 to indicate this change.
12 12
 
13
+- A client will now refuse a connection if pushed compression settings
14
+  will contradict the setting of ``allow-compression`` as this almost
15
+  always results in a non-working connection.
16
+
13 17
 
14 18
 Overview of changes in 2.6.1
15 19
 ============================
... ...
@@ -157,4 +157,33 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
157 157
     }
158 158
 }
159 159
 
160
+bool
161
+check_compression_settings_valid(struct compress_options *info, int msglevel)
162
+{
163
+    if ((info->flags & COMP_F_ALLOW_STUB_ONLY) && comp_non_stub_enabled(info))
164
+    {
165
+        msg(msglevel, "Compression is not allowed since allow-compression is "
166
+            "set to 'no'");
167
+        return false;
168
+    }
169
+#ifndef ENABLE_LZ4
170
+    if (info->alg == COMP_ALGV2_LZ4 || info->alg == COMP_ALG_LZ4)
171
+    {
172
+        msg(msglevel, "OpenVPN is compiled without LZ4 support. Requested "
173
+            "compression cannot be enabled.");
174
+        return false;
175
+    }
176
+#endif
177
+#ifndef ENABLE_LZO
178
+    if (info->alg == COMP_ALG_LZO || info->alg == COMP_ALG_LZ4)
179
+    {
180
+        msg(msglevel, "OpenVPN is compiled without LZO support. Requested "
181
+            "compression cannot be enabled.");
182
+        return false;
183
+    }
184
+#endif
185
+    return true;
186
+}
187
+
188
+
160 189
 #endif /* USE_COMP */
... ...
@@ -196,5 +196,13 @@ comp_non_stub_enabled(const struct compress_options *info)
196 196
            && info->alg != COMP_ALG_UNDEF;
197 197
 }
198 198
 
199
+/**
200
+ * Checks if the compression settings are valid. Takes into account the
201
+ * flags of allow-compression and also the whether algorithms are compiled
202
+ * in
203
+ */
204
+bool
205
+check_compression_settings_valid(struct compress_options *info, int msglevel);
206
+
199 207
 #endif /* USE_COMP */
200 208
 #endif /* ifndef OPENVPN_COMP_H */
... ...
@@ -2637,6 +2637,14 @@ do_deferred_options(struct context *c, const unsigned int found)
2637 2637
 #ifdef USE_COMP
2638 2638
     if (found & OPT_P_COMP)
2639 2639
     {
2640
+        if (!check_compression_settings_valid(&c->options.comp, D_PUSH_ERRORS))
2641
+        {
2642
+            msg(D_PUSH_ERRORS, "OPTIONS ERROR: server pushed compression "
2643
+                "settings that are not allowed and will result "
2644
+                "in a non-working connection. "
2645
+                "See also allow-compression in the manual.");
2646
+            return false;
2647
+        }
2640 2648
         msg(D_PUSH_DEBUG, "OPTIONS IMPORT: compression parms modified");
2641 2649
         comp_uninit(c->c2.comp_context);
2642 2650
         c->c2.comp_context = comp_init(&c->options.comp);
... ...
@@ -2766,6 +2766,14 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
2766 2766
         cc_succeeded = false;
2767 2767
     }
2768 2768
 
2769
+#ifdef USE_COMP
2770
+    if (!check_compression_settings_valid(&mi->context.options.comp, D_MULTI_ERRORS))
2771
+    {
2772
+        msg(D_MULTI_ERRORS, "MULTI: client has been rejected due to invalid compression options");
2773
+        cc_succeeded = false;
2774
+    }
2775
+#endif
2776
+
2769 2777
     if (cc_succeeded)
2770 2778
     {
2771 2779
         multi_client_connect_late_setup(m, mi, *option_types_found);
... ...
@@ -3779,6 +3779,9 @@ options_postprocess_mutate(struct options *o, struct env_set *es)
3779 3779
     /* this depends on o->windows_driver, which is set above */
3780 3780
     options_postprocess_mutate_invariant(o);
3781 3781
 
3782
+    /* check that compression settings in the options are okay */
3783
+    check_compression_settings_valid(&o->comp, M_USAGE);
3784
+
3782 3785
     /*
3783 3786
      * Save certain parms before modifying options during connect, especially
3784 3787
      * when using --pull
... ...
@@ -8405,21 +8408,12 @@ add_option(struct options *options,
8405 8405
 
8406 8406
         /* All lzo variants do not use swap */
8407 8407
         options->comp.flags &= ~COMP_F_SWAP;
8408
-#if defined(ENABLE_LZO)
8408
+
8409 8409
         if (p[1] && streq(p[1], "no"))
8410
-#endif
8411 8410
         {
8412 8411
             options->comp.alg = COMP_ALG_STUB;
8413 8412
             options->comp.flags &= ~COMP_F_ADAPTIVE;
8414 8413
         }
8415
-#if defined(ENABLE_LZO)
8416
-        else if (options->comp.flags & COMP_F_ALLOW_STUB_ONLY)
8417
-        {
8418
-            /* Also printed on a push to hint at configuration problems */
8419
-            msg(msglevel, "Cannot set comp-lzo to '%s', "
8420
-                "allow-compression is set to 'no'", p[1]);
8421
-            goto err;
8422
-        }
8423 8414
         else if (p[1])
8424 8415
         {
8425 8416
             if (streq(p[1], "yes"))
... ...
@@ -8444,7 +8438,6 @@ add_option(struct options *options,
8444 8444
             options->comp.flags |= COMP_F_ADAPTIVE;
8445 8445
         }
8446 8446
         show_compression_warning(&options->comp);
8447
-#endif /* if defined(ENABLE_LZO) */
8448 8447
     }
8449 8448
     else if (streq(p[0], "comp-noadapt") && !p[1])
8450 8449
     {
... ...
@@ -8478,23 +8471,12 @@ add_option(struct options *options,
8478 8478
         {
8479 8479
             options->comp.alg = COMP_ALG_UNDEF;
8480 8480
             options->comp.flags = COMP_F_MIGRATE;
8481
-
8482 8481
         }
8483
-        else if (options->comp.flags & COMP_F_ALLOW_STUB_ONLY)
8484
-        {
8485
-            /* Also printed on a push to hint at configuration problems */
8486
-            msg(msglevel, "Cannot set compress to '%s', "
8487
-                "allow-compression is set to 'no'", alg);
8488
-            goto err;
8489
-        }
8490
-#if defined(ENABLE_LZO)
8491 8482
         else if (streq(alg, "lzo"))
8492 8483
         {
8493 8484
             options->comp.alg = COMP_ALG_LZO;
8494 8485
             options->comp.flags &= ~(COMP_F_ADAPTIVE | COMP_F_SWAP);
8495 8486
         }
8496
-#endif
8497
-#if defined(ENABLE_LZ4)
8498 8487
         else if (streq(alg, "lz4"))
8499 8488
         {
8500 8489
             options->comp.alg = COMP_ALG_LZ4;
... ...
@@ -8504,7 +8486,6 @@ add_option(struct options *options,
8504 8504
         {
8505 8505
             options->comp.alg = COMP_ALGV2_LZ4;
8506 8506
         }
8507
-#endif
8508 8507
         else
8509 8508
         {
8510 8509
             msg(msglevel, "bad comp option: %s", alg);