[Openvpn-devel,v1] init: Fix datav2_enabled check in options import

Message ID 20251016103308.4685-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] init: Fix datav2_enabled check in options import | expand

Commit Message

Gert Doering Oct. 16, 2025, 10:33 a.m. UTC
From: Frank Lichtenheld <frank@lichtenheld.com>

peer_id is unsigned, so the previous check was partly
useless. Instead check use_peer_id.

Change-Id: Ia713a2ecfcad7032863867630a0c306ff9f90385
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1273
---

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/+/1273
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering Oct. 16, 2025, 10:46 a.m. UTC | #1
This is one of the "integer" patches that uncovered an actual logic
error - checking the wrong variable for the trigger "can I do epoch data?".

This did not hurt so far (because the other checks ensure the right
outcome) but I think it de-fuses the "data_v2 must be enabled!" sanity
check that would lead to not detecting a non-compliant client.  So it's
good we found this in time :-)

Not tested beyond "looks reasonable, the BBs say it compiles fine"
(the netbsd test fails are due to stuck t_client process from a previous
test run abort).

Your patch has been applied to the master branch.

commit c124e50317e530394d9011ddde5653ccfdd94482
Author: Frank Lichtenheld
Date:   Thu Oct 16 12:33:03 2025 +0200

     init: Fix datav2_enabled check in options import

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1273
     Message-Id: <20251016103308.4685-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33365.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index aaa0573..aa2611d 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2728,8 +2728,8 @@ 
 
     /* Ensure that for epoch data format is only enabled if also data v2
      * is enabled */
-    bool epoch_data = (c->options.imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT);
-    bool datav2_enabled = (c->options.peer_id >= 0 && c->options.peer_id < MAX_PEER_ID);
+    bool epoch_data = c->options.imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT;
+    bool datav2_enabled = c->options.use_peer_id && c->options.peer_id < MAX_PEER_ID;
 
     if (epoch_data && !datav2_enabled)
     {