diff mbox series

[Openvpn-devel,1/3] Remove saving initial frame code

Message ID 20231108124947.76816-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,1/3] Remove saving initial frame code | expand

Commit Message

Gert Doering Nov. 8, 2023, 12:49 p.m. UTC 
From: Arne Schwabe <arne@rfc2549.org>

This code was necessary before the frame/buffer refactoring as we
always did relative adjustment to the frame.

This also fixes also that previously initial_frame was initialised too
early before the fragment related options were initialised and contained
0 for the maximum frame size. This resulted in a DIV by 0 that caused an
abort on platforms that throw an exception for that.

CVE: 2023-46849

Only people with --fragment in their config are affected

Change-Id: Icc612bab5700879606290639e1b8773f61ec670d
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/forward.c |  9 ---------
 src/openvpn/init.c    | 19 ++++++++-----------
 src/openvpn/openvpn.h |  3 ---
 3 files changed, 8 insertions(+), 23 deletions(-)

Comments

Gert Doering Nov. 9, 2023, 11:31 a.m. UTC | #1 
Since this was security related (CVE) it was posted to the security@ list
first, and reviewed and ACKed by core team members there.  Re-Posting the
patches + the commit message here, after 2.6.7 release.

I have skimmed the code change and it looks reasonable - and I have
subjected this (+ 2/3 and 3/3) to the full set of t_client and t_server
patches, and things work as well as before.

Your patch has been applied to the release/2.6 and master branch.

commit 1cfca659244e362f372d9843351257f456392a2f (release/2.6)
commit a341914246d16ff277f2d521c2dc9273d41fa5ed (master)
Author: Arne Schwabe
Date:   Thu Oct 19 15:14:33 2023 +0200

     Remove saving initial frame code

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: David Sommerseth <davids@openvpn.net>
     Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
     Message-Id: <20231108124947.76816-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/search?l=mid&q=20231108124947.76816-1-gert@greenie.muc.de
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 2510410f9..0443ca0a0 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1078,15 +1078,6 @@  process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo
             if (tls_pre_decrypt(c->c2.tls_multi, &c->c2.from, &c->c2.buf, &co,
                                 floated, &ad_start))
             {
-                /* Restore pre-NCP frame parameters */
-                if (is_hard_reset_method2(opcode))
-                {
-                    c->c2.frame = c->c2.frame_initial;
-#ifdef ENABLE_FRAGMENT
-                    c->c2.frame_fragment = c->c2.frame_fragment_initial;
-#endif
-                }
-
                 interval_action(&c->c2.tmp_int);
 
                 /* reset packet received timer if TLS packet */
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 019f5a4f6..8c707a463 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3537,15 +3537,6 @@  do_init_frame(struct context *c)
      */
     frame_finalize_options(c, NULL);
 
-#ifdef ENABLE_FRAGMENT
-    /*
-     * Set frame parameter for fragment code.  This is necessary because
-     * the fragmentation code deals with payloads which have already been
-     * passed through the compression code.
-     */
-    c->c2.frame_fragment = c->c2.frame;
-    c->c2.frame_fragment_initial = c->c2.frame_fragment;
-#endif
 
 #if defined(ENABLE_FRAGMENT)
     /*
@@ -3736,6 +3727,14 @@  static void
 do_init_fragment(struct context *c)
 {
     ASSERT(c->options.ce.fragment);
+
+    /*
+     * Set frame parameter for fragment code.  This is necessary because
+     * the fragmentation code deals with payloads which have already been
+     * passed through the compression code.
+     */
+    c->c2.frame_fragment = c->c2.frame;
+
     frame_calculate_dynamic(&c->c2.frame_fragment, &c->c1.ks.key_type,
                             &c->options, get_link_socket_info(c));
     fragment_frame_init(c->c2.fragment, &c->c2.frame_fragment);
@@ -4640,8 +4639,6 @@  init_instance(struct context *c, const struct env_set *env, const unsigned int f
         c->c2.did_open_tun = do_open_tun(c, &error_flags);
     }
 
-    c->c2.frame_initial = c->c2.frame;
-
     /* print MTU info */
     do_print_data_channel_mtu_parms(c);
 
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 077effeb9..5b2be63f9 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -249,14 +249,11 @@  struct context_2
 
     /* MTU frame parameters */
     struct frame frame;                         /* Active frame parameters */
-    struct frame frame_initial;                 /* Restored on new session */
 
 #ifdef ENABLE_FRAGMENT
     /* Object to handle advanced MTU negotiation and datagram fragmentation */
     struct fragment_master *fragment;
     struct frame frame_fragment;
-    struct frame frame_fragment_initial;
-    struct frame frame_fragment_omit;
 #endif
 
     /*