diff mbox series

[Openvpn-devel,v2] Correctly calculate packet id size when epoch packet format is in use

Message ID 20260729064158.15731-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v2] Correctly calculate packet id size when epoch packet format is in use |

Commit Message

Gert Doering July 29, 2026, 6:41 a.m. UTC 
  From: Arne Schwabe <arne@rfc2549.org>

The code assumed that always when tls mode (without CFB/OFB) is in use
that the packet size is 4 bytes. With epoch packet format is incorrect
as that uses 64 bit.

Even thought packet_id_long_form has a the same size (8 byte) it is not
the same header format (32 bit time + 32 bit IV) as the epoch
format (16 bit epoch + 48 IV). Use a simple sizeof(uint64_t) to
avoid suggesting that it might be the same.

Closes: Github #1074

Change-Id: I5b862eabe032eb4d2229ff5b2f4f6af7406f6f4e
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@mandelbit.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1829
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1829
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Antonio Quartulli <antonio@mandelbit.com>

Comments

Antonio Quartulli July 29, 2026, 8:19 a.m. UTC | #1 
Hi,

On 29/07/2026 08:41, Gert Doering wrote:
[...]

> +    /* epoch uses a 64-bit packet ID consisting of 64 bit (24 bit epoch + 48 bit IV) */

As reported on GitHub, there is a typ0 here: 24 should be 16
Sorry for having missed that during review.

Regards,

> +    if (epoch)
> +    {
> +        return sizeof(uint64_t);
> +    }
>   
>       bool packet_id_long_form = !tlsmode || cipher_kt_mode_ofb_cfb(kt->cipher);
>   
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Gert Doering July 29, 2026, 4:34 p.m. UTC | #2 
I did not test this, as we have positive confirmation from the reporter
in GH#1074 and a +2 from Antonio.

After checking with Arne we agreed that I would fix the comment (24+48
is wrong, 16+48 is right) - looking at GFH#1074 I took the comment from
there, as it is more aligned with the wording in packet_id.c

  /* epoch format uses a 64-bit packet id: 16 bit epoch + 48 bit per-epoch counter */

(*only* the comment was changed, no code change).

Your patch has been applied to the master and release/2.7 branch (bugfix)
- earlier branches have no epoch support.

commit 2c8baca423b236ab12f7519b68ed88b4a709d0d1 (master)
commit 802571303f9fe83d2fbf1210bcd6fe9ea17cb888 (release/2.7)
Author: Arne Schwabe
Date:   Wed Jul 29 08:41:52 2026 +0200

     Correctly calculate packet id size when epoch packet format is in use

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Antonio Quartulli <antonio@mandelbit.com>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1829
     Message-Id: <20260729064158.15731-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37983.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
index e5db8ab..1f64a8a 100644
--- a/src/openvpn/mtu.c
+++ b/src/openvpn/mtu.c
@@ -35,6 +35,7 @@ 
 #include "crypto.h"
 
 #include "memdbg.h"
+#include "ssl_common.h"
 
 /* allocate a buffer for socket or tun layer */
 void
@@ -51,6 +52,13 @@ 
 calc_packet_id_size_dc(const struct options *options, const struct key_type *kt)
 {
     bool tlsmode = options->tls_server || options->tls_client;
+    bool epoch = options->imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT;
+
+    /* epoch uses a 64-bit packet ID consisting of 64 bit (24 bit epoch + 48 bit IV) */
+    if (epoch)
+    {
+        return sizeof(uint64_t);
+    }
 
     bool packet_id_long_form = !tlsmode || cipher_kt_mode_ofb_cfb(kt->cipher);