[Openvpn-devel,2/2] options.c: enforce a minimal fragment size

Message ID 20230301091851.82243-1-kprovost@netgate.com
State Accepted
Headers show
Series [Openvpn-devel,1/2] configure: improve FreeBSD DCO check | expand

Commit Message

Kristof Provost March 1, 2023, 9:18 a.m. UTC
From: Kristof Provost <kp@FreeBSD.org>

Very low values for 'fragment' can result in a division by zero in
optimal_fragment_size() (because it rounds max_frag_size down with
FRAG_SIZE_ROUND_MASK).

Enforce a minimal fragment size of 68 bytes, based on RFC 791 ("Every
internet module must be able to forward a datagram of 68 octets without
further fragmentation.")

Signed-off-by: Kristof Provost <kprovost@netgate.com>
---
 src/openvpn/options.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Gert Doering March 2, 2023, 5:19 p.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Straightforward :-) - and we really shouldn't divide by zero..

I have adjusted the message to read "--fragment ..." (with dashes),
because that's what we seem to do in other option-related error
messages.

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

commit 78e504210add19343e65f5c5b80be9ea6e9e95ab (master)
commit b9a9de156bc3ad517bfc6d1042ad0ef0350b638e (release/2.6)
Author: Kristof Provost
Date:   Wed Mar 1 10:18:51 2023 +0100

     options.c: enforce a minimal fragment size

     Signed-off-by: Kristof Provost <kprovost@netgate.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20230301091851.82243-1-kprovost@netgate.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26313.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9105449c..9f79da09 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6549,6 +6549,12 @@  add_option(struct options *options,
         VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION);
         options->ce.fragment = positive_atoi(p[1]);
 
+        if (options->ce.fragment < 68)
+        {
+            msg(msglevel, "fragment needs to be at least 68");
+            goto err;
+        }
+
         if (p[2] && streq(p[2], "mtu"))
         {
             options->ce.fragment_encap = true;