openvpn-devel — OpenVPN developers list

[Openvpn-devel] [XS] Change in openvpn[master]: dco-win: fix broken ASSERT in dco_new_key

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email )

Change subject: dco-win: fix broken ASSERT in dco_new_key
......................................................................

dco-win: fix broken ASSERT in dco_new_key

Commit

  e77c343 ("dco_win: In dco_new_key, document size assumptions for the integer casts")

has added an ASSERT on key-id, but didn't take into account that
key-id 0 is a perfectly valid value and is the first key-id. This
essentially broke dco-win.

Fix by adjusting ASSERT to >= 0.

Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Frank Lichtenheld <fr...@li...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1223
Message-Id: <202...@li...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59240115/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_win.c
1 file changed, 1 insertion(+), 1 deletion(-)




diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 4dd307f..30307de 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -541,7 +541,7 @@
     ZeroMemory(&crypto_data, sizeof(crypto_data));
 
     crypto_data.CipherAlg = dco_get_cipher(ciphername);
-    ASSERT(keyid > 0 && keyid <= UCHAR_MAX);
+    ASSERT(keyid >= 0 && keyid <= UCHAR_MAX);
     crypto_data.KeyId = (unsigned char)keyid;
     crypto_data.PeerId = peerid;
     crypto_data.KeySlot = slot;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Gerrit-Change-Number: 1223
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [XS] Change in openvpn[master]: dco-win: fix broken ASSERT in dco_new_key

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#2) to the change originally created by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by flichtenheld


Change subject: dco-win: fix broken ASSERT in dco_new_key
......................................................................

dco-win: fix broken ASSERT in dco_new_key

Commit

  e77c343 ("dco_win: In dco_new_key, document size assumptions for the integer casts")

has added an ASSERT on key-id, but didn't take into account that
key-id 0 is a perfectly valid value and is the first key-id. This
essentially broke dco-win.

Fix by adjusting ASSERT to >= 0.

Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Frank Lichtenheld <fr...@li...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1223
Message-Id: <202...@li...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59240115/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_win.c
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/1223/2

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 4dd307f..30307de 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -541,7 +541,7 @@
     ZeroMemory(&crypto_data, sizeof(crypto_data));
 
     crypto_data.CipherAlg = dco_get_cipher(ciphername);
-    ASSERT(keyid > 0 && keyid <= UCHAR_MAX);
+    ASSERT(keyid >= 0 && keyid <= UCHAR_MAX);
     crypto_data.KeyId = (unsigned char)keyid;
     crypto_data.PeerId = peerid;
     crypto_data.KeySlot = slot;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Gerrit-Change-Number: 1223
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: dco-win: fix broken ASSERT in dco_new_key

[Gert D. <ge...@gr...>]

The moment you notice that you have a CI infrastructure that actually
caught the issue - and the notification went "somewhere"...

Your patch has been applied to the master branch.

commit 290a14cd9edf95f07940f63993c290dc045c2a44
Author: Lev Stipakov
Date:   Mon Sep 29 17:28:41 2025 +0200

     dco-win: fix broken ASSERT in dco_new_key

     Signed-off-by: Lev Stipakov <le...@op...>
     Acked-by: Frank Lichtenheld <fr...@li...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1223
     Message-Id: <202...@li...>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59240115/
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v1] dco-win: fix broken ASSERT in dco_new_key

[Frank L. <fr...@li...>]

From: Lev Stipakov <le...@op...>

Commit

  e77c343 ("dco_win: In dco_new_key, document size assumptions for the integer casts")

has added an ASSERT on key-id, but didn't take into account that
key-id 0 is a perfectly valid value and is the first key-id. This
essentially broke dco-win.

Fix by adjusting ASSERT to >= 0.

Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Frank Lichtenheld <fr...@li...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1223
---

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

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <fr...@li...>

        
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 4dd307f..30307de 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -541,7 +541,7 @@
     ZeroMemory(&crypto_data, sizeof(crypto_data));
 
     crypto_data.CipherAlg = dco_get_cipher(ciphername);
-    ASSERT(keyid > 0 && keyid <= UCHAR_MAX);
+    ASSERT(keyid >= 0 && keyid <= UCHAR_MAX);
     crypto_data.KeyId = (unsigned char)keyid;
     crypto_data.PeerId = peerid;
     crypto_data.KeySlot = slot;

[Openvpn-devel] [XS] Change in openvpn[master]: dco-win: fix broken ASSERT in dco_new_key

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos, stipa.

flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email )

Change subject: dco-win: fix broken ASSERT in dco_new_key
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Gerrit-Change-Number: 1223
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Mon, 29 Sep 2025 15:14:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: multipeer: introduce asymmetric peer-id

[ordex (C. Review) <ge...@op...>]

Attention is currently required from: cron2, flichtenheld, plaisthos.

ordex has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email )

Change subject: multipeer: introduce asymmetric peer-id
......................................................................


Set Ready For Review


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Gerrit-Change-Number: 1089
Gerrit-PatchSet: 3
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Mon, 29 Sep 2025 10:43:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: multipeer: introduce asymmetric peer-id

[its_Giaan (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

its_Giaan has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email )

Change subject: multipeer: introduce asymmetric peer-id
......................................................................


Patch Set 3:

(3 comments)

File src/openvpn/push.c:

http://gerrit.openvpn.net/c/openvpn/+/1089/comment/643315b6_ec74dded :
PS2, Line 657:                         tls_multi->rx_peer_id);
> Yes, but the idea of the protocol is: […]
Done


File src/openvpn/ssl_ncp.c:

http://gerrit.openvpn.net/c/openvpn/+/1089/comment/4e416967_34f098a3 :
PS2, Line 431:         multi->tx_peer_id =  2033;
> yeah that was just for testing purposes, will fix this.
Done


http://gerrit.openvpn.net/c/openvpn/+/1089/comment/110f46aa_7a264227 :
PS2, Line 474:                 multi->rx_peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
> I missing the code that implements the asymmetric peer-id here completely is what I am saying. […]
Done



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Gerrit-Change-Number: 1089
Gerrit-PatchSet: 3
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Mon, 29 Sep 2025 09:37:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: plaisthos <arn...@rf...>
Comment-In-Reply-To: its_Giaan <gia...@ma...>
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: multipeer: introduce asymmetric peer-id

[its_Giaan (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, its_Giaan.

Hello cron2, flichtenheld, plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email

to look at the new patch set (#3).


Change subject: multipeer: introduce asymmetric peer-id
......................................................................

multipeer: introduce asymmetric peer-id

In order to achieve a multipeer functionality, peers now
use separate IDs for sending (tx_peer_id) and receiving
(rx_peer_id).

Each peer announces its own ID through pushing peer-info
using 'ID=7f1' hex format so identification can still
happen even if IP/port changes.

In P2P mode, peer switch to using the announced IDs after
mutual exchange.
In P2MP mode, clients always announce their ID, and servers
can optionally respond with their own to enable the same
behavior.

Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
---
M src/openvpn/dco.c
M src/openvpn/init.c
M src/openvpn/misc.c
M src/openvpn/multi.c
M src/openvpn/push.c
M src/openvpn/push_util.c
M src/openvpn/ssl.c
M src/openvpn/ssl_common.h
M src/openvpn/ssl_ncp.c
M src/openvpn/ssl_util.c
M src/openvpn/ssl_util.h
11 files changed, 104 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/1089/3

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 6afc680..eb600f0 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -514,14 +514,15 @@
         c->c2.tls_multi->dco_peer_id = -1;
     }
 #endif
-    int ret = dco_new_peer(&c->c1.tuntap->dco, multi->peer_id, sock->sd, NULL,
-                           proto_is_dgram(sock->info.proto) ? remoteaddr : NULL, NULL, NULL);
+    int ret = dco_new_peer(&c->c1.tuntap->dco, multi->rx_peer_id, sock->sd, NULL,
+                           proto_is_dgram(sock->info.proto) ? remoteaddr : NULL,
+                           NULL, NULL);
     if (ret < 0)
     {
         return ret;
     }
 
-    c->c2.tls_multi->dco_peer_id = multi->peer_id;
+    c->c2.tls_multi->dco_peer_id = multi->rx_peer_id;
 
     return 0;
 }
@@ -596,7 +597,7 @@
 {
     struct context *c = &mi->context;
 
-    int peer_id = c->c2.tls_multi->peer_id;
+    int peer_id = c->c2.tls_multi->rx_peer_id;
     struct sockaddr *remoteaddr, *localaddr = NULL;
     struct sockaddr_storage local = { 0 };
     const socket_descriptor_t sd = c->c2.link_sockets[0]->sd;
@@ -667,8 +668,7 @@
     if (addrtype == MR_ADDR_IPV6)
     {
 #if defined(_WIN32)
-        dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits,
-                                c->c2.tls_multi->peer_id);
+        dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits, c->c2.tls_multi->rx_peer_id);
 #else
         net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits,
                          &mi->context.c2.push_ifconfig_ipv6_local, c->c1.tuntap->actual_name, 0,
@@ -678,8 +678,7 @@
     else if (addrtype == MR_ADDR_IPV4)
     {
 #if defined(_WIN32)
-        dco_win_add_iroute_ipv4(&c->c1.tuntap->dco, addr->v4.addr, addr->netbits,
-                                c->c2.tls_multi->peer_id);
+        dco_win_add_iroute_ipv4(&c->c1.tuntap->dco, addr->v4.addr, addr->netbits, c->c2.tls_multi->rx_peer_id);
 #else
         in_addr_t dest = htonl(addr->v4.addr);
         net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, &mi->context.c2.push_ifconfig_local,
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index f8a0fee..fa841b9 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2223,7 +2223,7 @@
 
     if (o->use_peer_id)
     {
-        buf_printf(&out, ", peer-id: %d", o->peer_id);
+        buf_printf(&out, ", rx_peer-id: %u, tx_peer-id: %u", c->c2.tls_multi->rx_peer_id, c->c2.tls_multi->tx_peer_id);
     }
 
 #ifdef USE_COMP
@@ -2702,7 +2702,7 @@
     {
         msg(D_PUSH_DEBUG, "OPTIONS IMPORT: peer-id set");
         c->c2.tls_multi->use_peer_id = true;
-        c->c2.tls_multi->peer_id = c->options.peer_id;
+        c->c2.tls_multi->tx_peer_id = c->options.peer_id;
     }
 
     /* process (potentially) pushed options */
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index caf4725..91ab391 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -765,7 +765,8 @@
     {
         chomp(line);
         if (validate_peer_info_line(line)
-            && (strncmp(line, "IV_", 3) == 0 || strncmp(line, "UV_", 3) == 0))
+            && (strncmp(line, "IV_", 3) == 0 || strncmp(line, "UV_", 3) == 0
+                || strncmp(line, "ID", 2) == 0))
         {
             msg(M_INFO, "peer info: %s", line);
             env_set_add(es, line);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 2863ff1..bc8cc7b 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -453,7 +453,7 @@
         if (mi->context.c2.tls_multi && check_debug_level(D_DCO_DEBUG)
             && dco_enabled(&mi->context.options))
         {
-            buf_printf(&out, " peer-id=%d", mi->context.c2.tls_multi->peer_id);
+            buf_printf(&out, " rx_peer-id=%d", mi->context.c2.tls_multi->rx_peer_id);
         }
         return BSTR(&out);
     }
@@ -628,9 +628,9 @@
         }
 #endif
 
-        if (mi->context.c2.tls_multi->peer_id != MAX_PEER_ID)
+        if (mi->context.c2.tls_multi->rx_peer_id != MAX_PEER_ID)
         {
-            m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
+            m->instances[mi->context.c2.tls_multi->rx_peer_id] = NULL;
         }
 
         schedule_remove_entry(m->schedule, (struct schedule_entry *)mi);
@@ -949,8 +949,7 @@
 #else
                         sep,
 #endif
-                        sep,
-                        mi->context.c2.tls_multi ? mi->context.c2.tls_multi->peer_id : UINT32_MAX,
+                        sep, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->rx_peer_id : UINT32_MAX,
                         sep, translate_cipher_name_to_openvpn(mi->context.options.ciphername));
                 }
                 gc_free(&gc);
@@ -1756,6 +1755,17 @@
     {
         tls_multi->use_peer_id = true;
         o->use_peer_id = true;
+        uint32_t peer_id = extract_asymmetric_peer_id(peer_info);
+        if (peer_id)
+        {
+            tls_multi->tx_peer_id = peer_id;
+            tls_multi->use_asymmetric_peer_id = true;
+        }
+        else
+        {
+            /*Client has no asymmetric peer-id capability */
+            tls_multi->tx_peer_id = tls_multi->rx_peer_id;
+        }
     }
     else if (dco_enabled(o))
     {
@@ -3153,12 +3163,12 @@
          * has, so we disallow it. This can happen if a DCO netlink notification
          * gets lost and we miss a floating step.
          */
-        if (m1->peer_id == m2->peer_id)
+        if (m1->rx_peer_id == m2->rx_peer_id)
         {
             msg(M_WARN,
                 "disallowing peer %" PRIu32 " (%s) from floating to "
                 "its own address (%s)",
-                m1->peer_id, tls_common_name(mi->context.c2.tls_multi, false),
+                m1->rx_peer_id, tls_common_name(mi->context.c2.tls_multi, false),
                 mroute_addr_print(&mi->real, &gc));
             goto done;
         }
@@ -3171,9 +3181,10 @@
     }
 
     msg(D_MULTI_MEDIUM, "peer %" PRIu32 " (%s) floated from %s to %s",
-        mi->context.c2.tls_multi->peer_id, tls_common_name(mi->context.c2.tls_multi, false),
-        mroute_addr_print_ex(&mi->real, MAPF_SHOW_FAMILY, &gc),
-        mroute_addr_print_ex(&real, MAPF_SHOW_FAMILY, &gc));
+        mi->context.c2.tls_multi->rx_peer_id,
+        tls_common_name(mi->context.c2.tls_multi, false),
+        mroute_addr_print(&mi->real, &gc),
+        print_link_socket_actual(&m->top.c2.from, &gc));
 
     /* remove old address from hash table before changing address */
     ASSERT(hash_remove(m->hash, &mi->real));
@@ -4159,7 +4170,7 @@
     {
         if (!m->instances[i])
         {
-            mi->context.c2.tls_multi->peer_id = i;
+            mi->context.c2.tls_multi->rx_peer_id = i;
             m->instances[i] = mi;
             break;
         }
@@ -4167,7 +4178,7 @@
 
     /* should not really end up here, since multi_create_instance returns null
      * if amount of clients exceeds max_clients */
-    ASSERT(mi->context.c2.tls_multi->peer_id < m->max_clients);
+    ASSERT(mi->context.c2.tls_multi->rx_peer_id < m->max_clients);
 }
 
 /**************************************************************************/
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index e7fc50c..d9e5803 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -659,9 +659,10 @@
                         print_in_addr_t(c->c2.push_ifconfig_remote_netmask, 0, gc));
     }
 
-    if (tls_multi->use_peer_id)
+    if (tls_multi->use_peer_id && !tls_multi->use_asymmetric_peer_id)
     {
-        push_option_fmt(gc, push_list, M_USAGE, "peer-id %d", tls_multi->peer_id);
+        push_option_fmt(gc, push_list, M_USAGE, "peer-id %d",
+                        tls_multi->rx_peer_id);
     }
     /*
      * If server uses --auth-gen-token and we have an auth token
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
index 9138bdb..b5d8a57 100644
--- a/src/openvpn/push_util.c
+++ b/src/openvpn/push_util.c
@@ -175,14 +175,14 @@
         if (process_incoming_push_update(c, pull_permission_mask(c), option_types_found, &msgs[i], true) == PUSH_MSG_ERROR)
         {
             msg(M_WARN, "Failed to process push update message sent to client ID: %u",
-                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
+                c->c2.tls_multi ? c->c2.tls_multi->rx_peer_id : UINT32_MAX);
             continue;
         }
         c->options.push_option_types_found |= *option_types_found;
         if (!options_postprocess_pull(&c->options, c->c2.es))
         {
             msg(M_WARN, "Failed to post-process push update message sent to client ID: %u",
-                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
+                c->c2.tls_multi ? c->c2.tls_multi->rx_peer_id : UINT32_MAX);
         }
     }
     return true;
@@ -264,7 +264,7 @@
         if (!send_single_push_update(&curr_mi->context, msgs, &option_types_found))
         {
             msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated",
-                curr_mi->context.c2.tls_multi ? curr_mi->context.c2.tls_multi->peer_id : UINT32_MAX);
+                curr_mi->context.c2.tls_multi ? curr_mi->context.c2.tls_multi->rx_peer_id : UINT32_MAX);
             continue;
         }
         if (option_types_found & OPT_P_UP)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 34036f2..fc22287 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1175,7 +1175,9 @@
     /* get command line derived options */
     ret->opt = *tls_options;
     ret->dco_peer_id = -1;
-    ret->peer_id = MAX_PEER_ID;
+    ret->use_asymmetric_peer_id = false;
+    ret->rx_peer_id = MAX_PEER_ID;
+    ret->tx_peer_id = MAX_PEER_ID;
 
     return ret;
 }
@@ -1880,7 +1882,7 @@
  * @return          true if no error was encountered
  */
 static bool
-push_peer_info(struct buffer *buf, struct tls_session *session)
+push_peer_info(struct buffer *buf, struct tls_session *session, uint32_t peer_id)
 {
     struct gc_arena gc = gc_new();
     bool ret = false;
@@ -1974,6 +1976,11 @@
 
         buf_printf(&out, "IV_PROTO=%d\n", iv_proto);
 
+        if (peer_id != MAX_PEER_ID)
+        {
+            buf_printf(&out, "ID=%x\n", peer_id);
+        }
+
         if (session->opt->push_peer_info_detail > 1)
         {
             /* push compression status */
@@ -2154,7 +2161,20 @@
         }
     }
 
-    if (!push_peer_info(buf, session))
+    /* Calculate the asymmetric peer-id */
+    if (multi->rx_peer_id == MAX_PEER_ID && session->opt->mode != MODE_SERVER)
+    {
+        uint8_t peerid[3];
+        srand((unsigned)time(NULL));
+        for (int i = 0; i < 3; i++)
+        {
+            peerid[i] = rand();
+        }
+
+        multi->rx_peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
+        multi->use_asymmetric_peer_id = true;
+    }
+    if (!push_peer_info(buf, session, multi->rx_peer_id))
     {
         goto error;
     }
@@ -4006,8 +4026,8 @@
     msg(D_TLS_DEBUG, __func__);
 
     ASSERT(ks);
-
-    peer = htonl(((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) << 24 | (multi->peer_id & 0xFFFFFF));
+    peer = htonl(((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) << 24
+                 | (multi->tx_peer_id & 0xFFFFFF));
     ASSERT(buf_write_prepend(buf, &peer, 4));
 }
 
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index de89d30..c9818d4 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -697,8 +697,10 @@
 #define AUTH_TOKEN_VALID_EMPTYUSER (1 << 2)
 
     /* For P_DATA_V2 */
-    uint32_t peer_id;
+    uint32_t rx_peer_id;
+    uint32_t tx_peer_id;
     bool use_peer_id;
+    bool use_asymmetric_peer_id;
 
     char *remote_ciphername; /**< cipher specified in peer's config file */
     bool remote_usescomp;    /**< remote announced comp-lzo in OCC string */
diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index 790e50f..0f0d6bb 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -406,6 +406,7 @@
 {
     /* will return 0 if peer_info is null */
     const unsigned int iv_proto_peer = extract_iv_proto(multi->peer_info);
+    const unsigned int tx_peer_id = extract_asymmetric_peer_id(multi->peer_info);
 
     /* The other peer does not support P2P NCP */
     if (!(iv_proto_peer & IV_PROTO_NCP_P2P))
@@ -416,9 +417,15 @@
     if (iv_proto_peer & IV_PROTO_DATA_V2)
     {
         multi->use_peer_id = true;
-        multi->peer_id = 0x76706e; /* 'v' 'p' 'n' */
+        multi->use_asymmetric_peer_id = true;
+        multi->rx_peer_id = 0x76706e; /* 'v' 'p' 'n' */
+        multi->tx_peer_id = 0x76706e; /* 'v' 'p' 'n' */
     }
 
+    if (tx_peer_id)
+    {
+        multi->tx_peer_id = tx_peer_id;
+    }
     if (iv_proto_peer & IV_PROTO_CC_EXIT_NOTIFY)
     {
         session->opt->crypto_flags |= CO_USE_CC_EXIT_NOTIFY;
@@ -458,7 +465,7 @@
             }
             else
             {
-                multi->peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
+                multi->rx_peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
             }
         }
     }
@@ -500,11 +507,13 @@
         common_cipher = BSTR(&out);
     }
 
-    msg(D_TLS_DEBUG_LOW,
-        "P2P mode NCP negotiation result: "
-        "TLS_export=%d, DATA_v2=%d, peer-id %d, epoch=%d, cipher=%s",
-        (bool)(session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT), multi->use_peer_id,
-        multi->peer_id, (bool)(session->opt->crypto_flags & CO_EPOCH_DATA_KEY_FORMAT),
+    msg(D_TLS_DEBUG_LOW, "P2P mode NCP negotiation result: "
+                         "TLS_export=%d, DATA_v2=%d, rx-peer-id %d, tx-peer-id %d, epoch=%d, cipher=%s",
+        (bool)(session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT),
+        multi->use_peer_id,
+        multi->rx_peer_id,
+        multi->tx_peer_id,
+        (bool)(session->opt->crypto_flags & CO_EPOCH_DATA_KEY_FORMAT),
         common_cipher);
 
     gc_free(&gc);
diff --git a/src/openvpn/ssl_util.c b/src/openvpn/ssl_util.c
index 50e8c03..655497e 100644
--- a/src/openvpn/ssl_util.c
+++ b/src/openvpn/ssl_util.c
@@ -72,6 +72,24 @@
     return 0;
 }
 
+uint32_t
+extract_asymmetric_peer_id(const char *peer_info)
+{
+    const char *optstr = peer_info ? strstr(peer_info, "ID=") : NULL;
+    if (optstr)
+    {
+        uint32_t peer_id = 0;
+        int r = sscanf(optstr, "ID=%x", &peer_id);
+        {
+            if (r == 1 && peer_id >= 0)
+            {
+                return peer_id;
+            }
+        }
+    }
+    return 0;
+}
+
 const char *
 options_string_compat_lzo(const char *options, struct gc_arena *gc)
 {
diff --git a/src/openvpn/ssl_util.h b/src/openvpn/ssl_util.h
index 007ed69..56e4d26 100644
--- a/src/openvpn/ssl_util.h
+++ b/src/openvpn/ssl_util.h
@@ -53,6 +53,8 @@
  */
 unsigned int extract_iv_proto(const char *peer_info);
 
+uint32_t extract_asymmetric_peer_id(const char *peer_info);
+
 /**
  * Takes a locally produced OCC string for TLS server mode and modifies as
  * if the option comp-lzo was enabled. This is to send a client in

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Gerrit-Change-Number: 1089
Gerrit-PatchSet: 3
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: its_Giaan <gia...@ma...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [XS] Change in openvpn[master]: dco-win: fix broken ASSERT in dco_new_key

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email

to review the following change.


Change subject: dco-win: fix broken ASSERT in dco_new_key
......................................................................

dco-win: fix broken ASSERT in dco_new_key

Commit

  e77c343 ("dco_win: In dco_new_key, document size assumptions for the integer casts")

has added an ASSERT on key-id, but didn't take into account that
key-id 0 is a perfectly valid value and is the first key-id. This
essentially broke dco-win.

Fix by adjusting ASSERT to >= 0.

Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/openvpn/dco_win.c
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/23/1223/1

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 4dd307f..30307de 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -541,7 +541,7 @@
     ZeroMemory(&crypto_data, sizeof(crypto_data));
 
     crypto_data.CipherAlg = dco_get_cipher(ciphername);
-    ASSERT(keyid > 0 && keyid <= UCHAR_MAX);
+    ASSERT(keyid >= 0 && keyid <= UCHAR_MAX);
     crypto_data.KeyId = (unsigned char)keyid;
     crypto_data.PeerId = peerid;
     crypto_data.KeySlot = slot;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1223?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3b1243461ec9b6e85897f452f78dc4b05f7e126d
Gerrit-Change-Number: 1223
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange

[Openvpn-devel] [M] Change in openvpn[master]: dco-win: support for epoch data channel

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld, plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1219?usp=email

to look at the new patch set (#3).


Change subject: dco-win: support for epoch data channel
......................................................................

dco-win: support for epoch data channel

Starting from 2.8.0, dco-win driver supports
epoch data channel. This adds missing userspace part.

While on it, fix broken assert introduced in e77c34.
Key-Id 0 is a perfectly valid.

Change-Id: Ib5ed5969dcd405a47e34ed8479b7ffaaa5c43080
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/openvpn/dco.c
M src/openvpn/dco.h
M src/openvpn/dco_freebsd.c
M src/openvpn/dco_internal.h
M src/openvpn/dco_linux.c
M src/openvpn/dco_win.c
M src/openvpn/ovpn_dco_win.h
7 files changed, 67 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1219/3

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 6afc680..8fb4662 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -56,8 +56,9 @@
                 const char *ciphername)
 
 {
-    msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d, currently %d keys installed", __func__,
-        multi->dco_peer_id, ks->key_id, multi->dco_keys_installed);
+    bool epoch = ks->crypto_options.flags & CO_EPOCH_DATA_KEY_FORMAT;
+    msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d epoch=%d, currently %d keys installed", __func__,
+        multi->dco_peer_id, ks->key_id, multi->dco_keys_installed, epoch);
 
     /* Install a key in the PRIMARY slot only when no other key exist.
      * From that moment on, any new key will be installed in the SECONDARY
@@ -71,7 +72,7 @@
     }
 
     int ret = dco_new_key(multi->dco, multi->dco_peer_id, ks->key_id, slot, encrypt_key, encrypt_iv,
-                          decrypt_key, decrypt_iv, ciphername);
+                          decrypt_key, decrypt_iv, ciphername, epoch);
     if ((ret == 0) && (multi->dco_keys_installed < 2))
     {
         multi->dco_keys_installed++;
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index a362977..e5e8709 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -251,11 +251,8 @@
  * Return whether the dco implementation supports the new protocol features of
  * a 64 bit packet counter and AEAD tag at the end.
  */
-static inline bool
-dco_supports_epoch_data(struct context *c)
-{
-    return false;
-}
+bool
+dco_supports_epoch_data(struct context *c);
 #else  /* if defined(ENABLE_DCO) */
 
 typedef void *dco_context_t;
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index b9f6bc7..947a769 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -487,14 +487,14 @@
 int
 dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, dco_key_slot_t slot,
             const uint8_t *encrypt_key, const uint8_t *encrypt_iv, const uint8_t *decrypt_key,
-            const uint8_t *decrypt_iv, const char *ciphername)
+            const uint8_t *decrypt_iv, const char *ciphername, bool epoch)
 {
     struct ifdrv drv;
     nvlist_t *nvl, *encrypt_nvl, *decrypt_nvl;
     int ret;
 
-    msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s", __func__, slot, keyid, peerid,
-        ciphername);
+    msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s, epoch %d", __func__, slot, keyid, peerid,
+        ciphername, epoch);
 
     nvl = nvlist_create(0);
 
@@ -876,4 +876,10 @@
     return "none:AES-256-GCM:AES-192-GCM:AES-128-GCM:CHACHA20-POLY1305";
 }
 
+bool
+dco_supports_epoch_data(struct context *c)
+{
+    return false;
+}
+
 #endif /* defined(ENABLE_DCO) && defined(TARGET_FREEBSD) */
diff --git a/src/openvpn/dco_internal.h b/src/openvpn/dco_internal.h
index 86af003..97a7048 100644
--- a/src/openvpn/dco_internal.h
+++ b/src/openvpn/dco_internal.h
@@ -66,7 +66,7 @@
 
 int dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, dco_key_slot_t slot,
                 const uint8_t *encrypt_key, const uint8_t *encrypt_iv, const uint8_t *decrypt_key,
-                const uint8_t *decrypt_iv, const char *ciphername);
+                const uint8_t *decrypt_iv, const char *ciphername, bool epoch);
 
 int dco_del_key(dco_context_t *dco, unsigned int peerid, dco_key_slot_t slot);
 
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index d46fa46..0ae30b1 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -596,10 +596,10 @@
 int
 dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, dco_key_slot_t slot,
             const uint8_t *encrypt_key, const uint8_t *encrypt_iv, const uint8_t *decrypt_key,
-            const uint8_t *decrypt_iv, const char *ciphername)
+            const uint8_t *decrypt_iv, const char *ciphername, bool epoch)
 {
-    msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s", __func__, slot, keyid, peerid,
-        ciphername);
+    msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s, epoch %d", __func__, slot, keyid, peerid,
+        ciphername, epoch);
 
     const int key_len = cipher_kt_key_size(ciphername);
     const int nonce_tail_len = 8;
@@ -1298,4 +1298,10 @@
     return "AES-128-GCM:AES-256-GCM:AES-192-GCM:CHACHA20-POLY1305";
 }
 
+bool
+dco_supports_epoch_data(struct context *c)
+{
+    return false;
+}
+
 #endif /* defined(ENABLE_DCO) && defined(TARGET_LINUX) */
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 4dd307f..0fb65c6 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -528,7 +528,7 @@
 int
 dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, dco_key_slot_t slot,
             const uint8_t *encrypt_key, const uint8_t *encrypt_iv, const uint8_t *decrypt_key,
-            const uint8_t *decrypt_iv, const char *ciphername)
+            const uint8_t *decrypt_iv, const char *ciphername, bool epoch)
 {
     msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s", __func__, slot, keyid, peerid,
         ciphername);
@@ -537,29 +537,41 @@
     size_t key_len = cipher_kt_key_size(ciphername);
     ASSERT(key_len <= 32);
 
-    OVPN_CRYPTO_DATA crypto_data;
+    OVPN_CRYPTO_DATA_V2 crypto_data;
     ZeroMemory(&crypto_data, sizeof(crypto_data));
 
-    crypto_data.CipherAlg = dco_get_cipher(ciphername);
-    ASSERT(keyid > 0 && keyid <= UCHAR_MAX);
-    crypto_data.KeyId = (unsigned char)keyid;
-    crypto_data.PeerId = peerid;
-    crypto_data.KeySlot = slot;
+    OVPN_CRYPTO_DATA *v1 = &crypto_data.V1;
 
-    CopyMemory(crypto_data.Encrypt.Key, encrypt_key, key_len);
-    crypto_data.Encrypt.KeyLen = (unsigned char)key_len;
-    CopyMemory(crypto_data.Encrypt.NonceTail, encrypt_iv, nonce_len);
+    v1->CipherAlg = dco_get_cipher(ciphername);
+    ASSERT(keyid >= 0 && keyid <= UCHAR_MAX);
+    v1->KeyId = (unsigned char)keyid;
+    v1->PeerId = peerid;
+    v1->KeySlot = slot;
 
-    CopyMemory(crypto_data.Decrypt.Key, decrypt_key, key_len);
-    crypto_data.Decrypt.KeyLen = (unsigned char)key_len;
-    CopyMemory(crypto_data.Decrypt.NonceTail, decrypt_iv, nonce_len);
+    CopyMemory(v1->Encrypt.Key, encrypt_key, key_len);
+    v1->Encrypt.KeyLen = (unsigned char)key_len;
+    CopyMemory(v1->Encrypt.NonceTail, encrypt_iv, nonce_len);
 
-    ASSERT(crypto_data.CipherAlg > 0);
+    CopyMemory(v1->Decrypt.Key, decrypt_key, key_len);
+    v1->Decrypt.KeyLen = (unsigned char)key_len;
+    CopyMemory(v1->Decrypt.NonceTail, decrypt_iv, nonce_len);
+
+    ASSERT(v1->CipherAlg > 0);
+
+    DWORD ioctl = OVPN_IOCTL_NEW_KEY;
+    VOID *buf = &crypto_data.V1;
+    DWORD bufSize = sizeof(crypto_data.V1);
+    if (epoch)
+    {
+        ioctl = OVPN_IOCTL_NEW_KEY_V2;
+        crypto_data.CryptoOptions |= CRYPTO_OPTIONS_EPOCH;
+        buf = &crypto_data;
+        bufSize = sizeof(crypto_data);
+    }
 
     DWORD bytes_returned = 0;
 
-    if (!DeviceIoControl(dco->tt->hand, OVPN_IOCTL_NEW_KEY, &crypto_data, sizeof(crypto_data), NULL,
-                         0, &bytes_returned, NULL))
+    if (!DeviceIoControl(dco->tt->hand, ioctl, buf, bufSize, NULL, 0, &bytes_returned, NULL))
     {
         msg(M_ERR, "DeviceIoControl(OVPN_IOCTL_NEW_KEY) failed");
         return -1;
@@ -1076,4 +1088,11 @@
     gc_free(&gc);
 }
 
+bool
+dco_supports_epoch_data(struct context *c)
+{
+    OVPN_VERSION ver = { 0 };
+    return dco_get_version(&ver) && ((ver.Major == 2 && ver.Minor >= 8) || (ver.Major > 2));
+}
+
 #endif /* defined(_WIN32) */
diff --git a/src/openvpn/ovpn_dco_win.h b/src/openvpn/ovpn_dco_win.h
index 9e1378a..e76770b 100644
--- a/src/openvpn/ovpn_dco_win.h
+++ b/src/openvpn/ovpn_dco_win.h
@@ -118,6 +118,13 @@
 	int PeerId;
 } OVPN_CRYPTO_DATA, * POVPN_CRYPTO_DATA;
 
+#define CRYPTO_OPTIONS_EPOCH (1<<1)
+
+typedef struct _OVPN_CRYPTO_DATA_V2 {
+    OVPN_CRYPTO_DATA V1;
+    UINT32 CryptoOptions;
+} OVPN_CRYPTO_DATA_V2, * POVPN_CRYPTO_DATA_V2;
+
 typedef struct _OVPN_MP_SET_PEER {
     int PeerId;
     LONG KeepaliveInterval;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1219?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ib5ed5969dcd405a47e34ed8479b7ffaaa5c43080
Gerrit-Change-Number: 1219
Gerrit-PatchSet: 3
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: Allowing installing FreeBSD routes with interface instead of next-hop

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1193?usp=email )

Change subject: Allowing installing FreeBSD routes with interface instead of next-hop
......................................................................


Patch Set 3:

(1 comment)

File src/openvpn/networking_freebsd.c:

http://gerrit.openvpn.net/c/openvpn/+/1193/comment/5c8a40a2_8ceda728 :
PS3, Line 23:     /* if gw is 0 (=0.0.0.0), which is not a valid gateway,
> are you passing in NULL or "a pointer to 0.0.0. […]
```
/* if we have a gateway (GW != NULL) install route to gateway IP
 * if not, install "connected" route to interface (required for ifconfig-push IPs)
 */
```
something like this?  I think this would help understanding the code and be in line with the `if(gw)...`



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1193?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I88e16e15fad065cb310d38f09924053efc3a6ce5
Gerrit-Change-Number: 1193
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Sun, 28 Sep 2025 11:24:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <ge...@gr...>
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Allowing installing FreeBSD routes with interface instead of next-hop

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1193?usp=email )

Change subject: Allowing installing FreeBSD routes with interface instead of next-hop
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

File src/openvpn/networking_freebsd.c:

http://gerrit.openvpn.net/c/openvpn/+/1193/comment/ca46df8e_422ac899 :
PS3, Line 23:     /* if gw is 0 (=0.0.0.0), which is not a valid gateway,
are you passing in NULL or "a pointer to 0.0.0.0"?  The code says "NULL" and the comment says "gw is 0.0.0.0" which would technically be "a pointer to 0.0.0.0"...

Maybe just word it the same as in IPv6... `if we have no gateway (gw is NULL), we are installing a route with the interface as target instead` or so.

But the logic is still confusing, the comment says "if we have no gateway" and the condition does "if(gateway)" and the "interface route" is down in the else branch...



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1193?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I88e16e15fad065cb310d38f09924053efc3a6ce5
Gerrit-Change-Number: 1193
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Sat, 27 Sep 2025 20:27:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: dco: Change sd argument to dco_new_peer from int to socket_descriptor_t

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1195?usp=email )

Change subject: dco: Change sd argument to dco_new_peer from int to socket_descriptor_t
......................................................................

dco: Change sd argument to dco_new_peer from int to socket_descriptor_t

Doesn't change anything for non-Win32 platforms.

Change-Id: I28f856c1c156b54089d95b2e2539ecdb374cdd37
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Antonio Quartulli <an...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1195
Message-Id: <202...@li...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59238248/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco.c
M src/openvpn/dco_internal.h
M src/openvpn/dco_win.c
3 files changed, 4 insertions(+), 13 deletions(-)




diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 2cf90af..6afc680 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -491,11 +491,6 @@
     return true;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 int
 dco_p2p_add_new_peer(struct context *c)
 {
@@ -604,7 +599,7 @@
     int peer_id = c->c2.tls_multi->peer_id;
     struct sockaddr *remoteaddr, *localaddr = NULL;
     struct sockaddr_storage local = { 0 };
-    int sd = c->c2.link_sockets[0]->sd;
+    const socket_descriptor_t sd = c->c2.link_sockets[0]->sd;
 
 
     if (c->mode == CM_CHILD_TCP)
@@ -650,10 +645,6 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 void
 dco_install_iroute(struct multi_context *m, struct multi_instance *mi, struct mroute_addr *addr)
 {
diff --git a/src/openvpn/dco_internal.h b/src/openvpn/dco_internal.h
index 83013ef..86af003 100644
--- a/src/openvpn/dco_internal.h
+++ b/src/openvpn/dco_internal.h
@@ -59,7 +59,7 @@
  * They are implemented by dco_linux.c
  */
 
-int dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd, struct sockaddr *localaddr,
+int dco_new_peer(dco_context_t *dco, unsigned int peerid, socket_descriptor_t sd, struct sockaddr *localaddr,
                  struct sockaddr *remoteaddr, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6);
 
 int dco_del_peer(dco_context_t *dco, unsigned int peerid);
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 7dd43d6..4dd307f 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -415,10 +415,10 @@
 }
 
 int
-dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd, struct sockaddr *localaddr,
+dco_new_peer(dco_context_t *dco, unsigned int peerid, socket_descriptor_t sd, struct sockaddr *localaddr,
              struct sockaddr *remoteaddr, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6)
 {
-    msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d", __func__, peerid, sd);
+    msg(D_DCO_DEBUG, "%s: peer-id %d, fd " SOCKET_PRINTF, __func__, peerid, sd);
 
     if (dco->ifmode == DCO_MODE_P2P)
     {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1195?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I28f856c1c156b54089d95b2e2539ecdb374cdd37
Gerrit-Change-Number: 1195
Gerrit-PatchSet: 8
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [PATCH applied] Re: dco: Change sd argument to dco_new_peer from int to socket_descriptor_t

[Gert D. <ge...@gr...>]

Straightforward *and* an ACK from Antonio :-)

Your patch has been applied to the master branch.

commit 36a09c8dbfb68c9df745943886dc974513998c07
Author: Frank Lichtenheld
Date:   Wed Sep 24 17:10:44 2025 +0200

     dco: Change sd argument to dco_new_peer from int to socket_descriptor_t

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: Antonio Quartulli <an...@ma...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1195
     Message-Id: <202...@li...>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59238248/
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: dco: Change sd argument to dco_new_peer from int to socket_descriptor_t

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#8) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1195?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by ordex


Change subject: dco: Change sd argument to dco_new_peer from int to socket_descriptor_t
......................................................................

dco: Change sd argument to dco_new_peer from int to socket_descriptor_t

Doesn't change anything for non-Win32 platforms.

Change-Id: I28f856c1c156b54089d95b2e2539ecdb374cdd37
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Antonio Quartulli <an...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1195
Message-Id: <202...@li...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59238248/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco.c
M src/openvpn/dco_internal.h
M src/openvpn/dco_win.c
3 files changed, 4 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/95/1195/8

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 2cf90af..6afc680 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -491,11 +491,6 @@
     return true;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 int
 dco_p2p_add_new_peer(struct context *c)
 {
@@ -604,7 +599,7 @@
     int peer_id = c->c2.tls_multi->peer_id;
     struct sockaddr *remoteaddr, *localaddr = NULL;
     struct sockaddr_storage local = { 0 };
-    int sd = c->c2.link_sockets[0]->sd;
+    const socket_descriptor_t sd = c->c2.link_sockets[0]->sd;
 
 
     if (c->mode == CM_CHILD_TCP)
@@ -650,10 +645,6 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 void
 dco_install_iroute(struct multi_context *m, struct multi_instance *mi, struct mroute_addr *addr)
 {
diff --git a/src/openvpn/dco_internal.h b/src/openvpn/dco_internal.h
index 83013ef..86af003 100644
--- a/src/openvpn/dco_internal.h
+++ b/src/openvpn/dco_internal.h
@@ -59,7 +59,7 @@
  * They are implemented by dco_linux.c
  */
 
-int dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd, struct sockaddr *localaddr,
+int dco_new_peer(dco_context_t *dco, unsigned int peerid, socket_descriptor_t sd, struct sockaddr *localaddr,
                  struct sockaddr *remoteaddr, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6);
 
 int dco_del_peer(dco_context_t *dco, unsigned int peerid);
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 7dd43d6..4dd307f 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -415,10 +415,10 @@
 }
 
 int
-dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd, struct sockaddr *localaddr,
+dco_new_peer(dco_context_t *dco, unsigned int peerid, socket_descriptor_t sd, struct sockaddr *localaddr,
              struct sockaddr *remoteaddr, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6)
 {
-    msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d", __func__, peerid, sd);
+    msg(D_DCO_DEBUG, "%s: peer-id %d, fd " SOCKET_PRINTF, __func__, peerid, sd);
 
     if (dco->ifmode == DCO_MODE_P2P)
     {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1195?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I28f856c1c156b54089d95b2e2539ecdb374cdd37
Gerrit-Change-Number: 1195
Gerrit-PatchSet: 8
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: ssl_openssl: Use uint16_t internally for TLS versions

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1212?usp=email )

Change subject: ssl_openssl: Use uint16_t internally for TLS versions
......................................................................

ssl_openssl: Use uint16_t internally for TLS versions

libressl changed the API for the involved functions. Since
uint16_t is a true subset of int it should be safe to switch
to that for all OpenSSL variants.

One trivial drive-by fix in unrelated code to be able
to enable -Wconversion fully for the file. This just
adds a cast where the comment says we intend a cast.

Change-Id: I9ea87531afb553f789289787403900a4758b8e1c
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: MaxF <ma...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1212
Message-Id: <202...@gr...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59238230/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/ssl_openssl.c
1 file changed, 7 insertions(+), 25 deletions(-)




diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 89deeaa..434df7d 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -235,8 +235,8 @@
 }
 
 /** Convert internal version number to openssl version number */
-static int
-openssl_tls_version(int ver)
+static uint16_t
+openssl_tls_version(unsigned int ver)
 {
     if (ver == TLS_VER_1_0)
     {
@@ -272,23 +272,18 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 static bool
 tls_ctx_set_tls_versions(struct tls_root_ctx *ctx, unsigned int ssl_flags)
 {
-    int tls_ver_min =
+    uint16_t tls_ver_min =
         openssl_tls_version((ssl_flags >> SSLF_TLS_VERSION_MIN_SHIFT) & SSLF_TLS_VERSION_MIN_MASK);
-    int tls_ver_max =
+    uint16_t tls_ver_max =
         openssl_tls_version((ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT) & SSLF_TLS_VERSION_MAX_MASK);
 
     if (!tls_ver_min)
     {
         /* Enforce at least TLS 1.0 */
-        int cur_min = SSL_CTX_get_min_proto_version(ctx->ctx);
+        uint16_t cur_min = (uint16_t)SSL_CTX_get_min_proto_version(ctx->ctx);
         tls_ver_min = cur_min < TLS1_VERSION ? TLS1_VERSION : cur_min;
     }
 
@@ -387,7 +382,7 @@
             /* %.*s format specifier expects length of type int, so guarantee */
             /* that length is small enough and cast to int. */
             msg(D_LOW, "No valid translation found for TLS cipher '%.*s'",
-                constrain_int(current_cipher_len, 0, 256), current_cipher);
+                constrain_int((int)current_cipher_len, 0, 256), current_cipher);
         }
         else
         {
@@ -429,10 +424,6 @@
     }
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 void
 tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 {
@@ -2522,11 +2513,6 @@
     msg(D_HANDSHAKE, "%s%s%s%s%s", s1, s2, s3, s4, s5);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 void
 show_available_tls_ciphers_list(const char *cipher_list, const char *tls_cert_profile, bool tls13)
 {
@@ -2541,7 +2527,7 @@
 #if defined(TLS1_3_VERSION)
     if (tls13)
     {
-        SSL_CTX_set_min_proto_version(tls_ctx.ctx, openssl_tls_version(TLS_VER_1_3));
+        SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);
         tls_ctx_restrict_ciphers_tls13(&tls_ctx, cipher_list);
     }
     else
@@ -2594,10 +2580,6 @@
     SSL_CTX_free(tls_ctx.ctx);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 /*
  * Show the Elliptic curves that are available for us to use
  * in the OpenSSL library.

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1212?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I9ea87531afb553f789289787403900a4758b8e1c
Gerrit-Change-Number: 1212
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: MaxF <ma...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [S] Change in openvpn[master]: ssl_openssl: Use uint16_t internally for TLS versions

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#3) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1212?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by MaxF


Change subject: ssl_openssl: Use uint16_t internally for TLS versions
......................................................................

ssl_openssl: Use uint16_t internally for TLS versions

libressl changed the API for the involved functions. Since
uint16_t is a true subset of int it should be safe to switch
to that for all OpenSSL variants.

One trivial drive-by fix in unrelated code to be able
to enable -Wconversion fully for the file. This just
adds a cast where the comment says we intend a cast.

Change-Id: I9ea87531afb553f789289787403900a4758b8e1c
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: MaxF <ma...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1212
Message-Id: <202...@gr...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59238230/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/ssl_openssl.c
1 file changed, 7 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/12/1212/3

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 89deeaa..434df7d 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -235,8 +235,8 @@
 }
 
 /** Convert internal version number to openssl version number */
-static int
-openssl_tls_version(int ver)
+static uint16_t
+openssl_tls_version(unsigned int ver)
 {
     if (ver == TLS_VER_1_0)
     {
@@ -272,23 +272,18 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 static bool
 tls_ctx_set_tls_versions(struct tls_root_ctx *ctx, unsigned int ssl_flags)
 {
-    int tls_ver_min =
+    uint16_t tls_ver_min =
         openssl_tls_version((ssl_flags >> SSLF_TLS_VERSION_MIN_SHIFT) & SSLF_TLS_VERSION_MIN_MASK);
-    int tls_ver_max =
+    uint16_t tls_ver_max =
         openssl_tls_version((ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT) & SSLF_TLS_VERSION_MAX_MASK);
 
     if (!tls_ver_min)
     {
         /* Enforce at least TLS 1.0 */
-        int cur_min = SSL_CTX_get_min_proto_version(ctx->ctx);
+        uint16_t cur_min = (uint16_t)SSL_CTX_get_min_proto_version(ctx->ctx);
         tls_ver_min = cur_min < TLS1_VERSION ? TLS1_VERSION : cur_min;
     }
 
@@ -387,7 +382,7 @@
             /* %.*s format specifier expects length of type int, so guarantee */
             /* that length is small enough and cast to int. */
             msg(D_LOW, "No valid translation found for TLS cipher '%.*s'",
-                constrain_int(current_cipher_len, 0, 256), current_cipher);
+                constrain_int((int)current_cipher_len, 0, 256), current_cipher);
         }
         else
         {
@@ -429,10 +424,6 @@
     }
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 void
 tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 {
@@ -2522,11 +2513,6 @@
     msg(D_HANDSHAKE, "%s%s%s%s%s", s1, s2, s3, s4, s5);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 void
 show_available_tls_ciphers_list(const char *cipher_list, const char *tls_cert_profile, bool tls13)
 {
@@ -2541,7 +2527,7 @@
 #if defined(TLS1_3_VERSION)
     if (tls13)
     {
-        SSL_CTX_set_min_proto_version(tls_ctx.ctx, openssl_tls_version(TLS_VER_1_3));
+        SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);
         tls_ctx_restrict_ciphers_tls13(&tls_ctx, cipher_list);
     }
     else
@@ -2594,10 +2580,6 @@
     SSL_CTX_free(tls_ctx.ctx);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 /*
  * Show the Elliptic curves that are available for us to use
  * in the OpenSSL library.

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1212?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I9ea87531afb553f789289787403900a4758b8e1c
Gerrit-Change-Number: 1212
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: MaxF <ma...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: ssl_openssl: Use uint16_t internally for TLS versions

[Gert D. <ge...@gr...>]

Looked at this as well, makes sense, MaxF was faster with the +2 ;-)

Your patch has been applied to the master branch.

commit 04d24fbeb529991734cc0951037cd4a94376b025
Author: Frank Lichtenheld
Date:   Wed Sep 24 17:02:55 2025 +0200

     ssl_openssl: Use uint16_t internally for TLS versions

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: MaxF <ma...@ma...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1212
     Message-Id: <202...@gr...>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59238230/
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: Fix -Wconversion warnings

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1220?usp=email )

Change subject: dco_linux: Fix -Wconversion warnings
......................................................................

dco_linux: Fix -Wconversion warnings

Combination of using the correct types and some
unavoidable safe size_t->int casts.

Change-Id: I473d345d10fc406f76fbdb131c28cc4fc54822fd
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Antonio Quartulli <an...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1220
Message-Id: <202...@gr...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59239172/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
1 file changed, 5 insertions(+), 14 deletions(-)




diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 395a38f..d46fa46 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -62,11 +62,6 @@
 
 typedef int (*ovpn_nl_cb)(struct nl_msg *msg, void *arg);
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 /**
  * @brief resolves the netlink ID for ovpn-dco
  *
@@ -110,7 +105,7 @@
 }
 
 static struct nl_msg *
-ovpn_dco_nlmsg_create(dco_context_t *dco, int cmd)
+ovpn_dco_nlmsg_create(dco_context_t *dco, uint8_t cmd)
 {
     struct nl_msg *nl_msg = nlmsg_alloc();
     if (!nl_msg)
@@ -346,7 +341,7 @@
 
     if (!(nlh->nlmsg_flags & NLM_F_CAPPED))
     {
-        ack_len += err->msg.nlmsg_len - sizeof(*nlh);
+        ack_len += err->msg.nlmsg_len - (int)sizeof(*nlh);
     }
 
     if (len <= ack_len)
@@ -360,8 +355,8 @@
     nla_parse(tb_msg, OVPN_NLMSGERR_ATTR_MAX, attrs, len, NULL);
     if (tb_msg[NLMSGERR_ATTR_MSG])
     {
-        len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
-                      nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
+        len = (int)strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
+                           nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
         msg(M_WARN, "kernel error: %*s", len, (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
     }
 
@@ -606,7 +601,7 @@
     msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s", __func__, slot, keyid, peerid,
         ciphername);
 
-    const size_t key_len = cipher_kt_key_size(ciphername);
+    const int key_len = cipher_kt_key_size(ciphername);
     const int nonce_tail_len = 8;
 
     struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_KEY_NEW);
@@ -1303,8 +1298,4 @@
     return "AES-128-GCM:AES-256-GCM:AES-192-GCM:CHACHA20-POLY1305";
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 #endif /* defined(ENABLE_DCO) && defined(TARGET_LINUX) */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1220?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I473d345d10fc406f76fbdb131c28cc4fc54822fd
Gerrit-Change-Number: 1220
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: Fix -Wconversion warnings

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#2) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1220?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by ordex


Change subject: dco_linux: Fix -Wconversion warnings
......................................................................

dco_linux: Fix -Wconversion warnings

Combination of using the correct types and some
unavoidable safe size_t->int casts.

Change-Id: I473d345d10fc406f76fbdb131c28cc4fc54822fd
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Antonio Quartulli <an...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1220
Message-Id: <202...@gr...>
URL: https://sourceforge.net/p/openvpn/mailman/message/59239172/
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
1 file changed, 5 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1220/2

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 395a38f..d46fa46 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -62,11 +62,6 @@
 
 typedef int (*ovpn_nl_cb)(struct nl_msg *msg, void *arg);
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 /**
  * @brief resolves the netlink ID for ovpn-dco
  *
@@ -110,7 +105,7 @@
 }
 
 static struct nl_msg *
-ovpn_dco_nlmsg_create(dco_context_t *dco, int cmd)
+ovpn_dco_nlmsg_create(dco_context_t *dco, uint8_t cmd)
 {
     struct nl_msg *nl_msg = nlmsg_alloc();
     if (!nl_msg)
@@ -346,7 +341,7 @@
 
     if (!(nlh->nlmsg_flags & NLM_F_CAPPED))
     {
-        ack_len += err->msg.nlmsg_len - sizeof(*nlh);
+        ack_len += err->msg.nlmsg_len - (int)sizeof(*nlh);
     }
 
     if (len <= ack_len)
@@ -360,8 +355,8 @@
     nla_parse(tb_msg, OVPN_NLMSGERR_ATTR_MAX, attrs, len, NULL);
     if (tb_msg[NLMSGERR_ATTR_MSG])
     {
-        len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
-                      nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
+        len = (int)strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
+                           nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
         msg(M_WARN, "kernel error: %*s", len, (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
     }
 
@@ -606,7 +601,7 @@
     msg(D_DCO_DEBUG, "%s: slot %d, key-id %d, peer-id %d, cipher %s", __func__, slot, keyid, peerid,
         ciphername);
 
-    const size_t key_len = cipher_kt_key_size(ciphername);
+    const int key_len = cipher_kt_key_size(ciphername);
     const int nonce_tail_len = 8;
 
     struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_KEY_NEW);
@@ -1303,8 +1298,4 @@
     return "AES-128-GCM:AES-256-GCM:AES-192-GCM:CHACHA20-POLY1305";
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 #endif /* defined(ENABLE_DCO) && defined(TARGET_LINUX) */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1220?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I473d345d10fc406f76fbdb131c28cc4fc54822fd
Gerrit-Change-Number: 1220
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: dco_linux: Fix -Wconversion warnings

[Gert D. <ge...@gr...>]

Looks reasonable, has an ACK by the owner of the code :-) and BB is
happy, so what shall I say...  (except "mail archive is not working").

Your patch has been applied to the master branch.

commit b2d5d7110cee0f6d1065617098fa8de4d0e24754
Author: Frank Lichtenheld
Date:   Fri Sep 26 16:24:36 2025 +0200

     dco_linux: Fix -Wconversion warnings

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: Antonio Quartulli <an...@ma...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1220
     Message-Id: <202...@gr...>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59239172/
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: route: handle default gateway (net_gateway) and nexthop towards VPN s...

[mrbff (C. Review) <ge...@op...>]

mrbff has abandoned this change. ( http://gerrit.openvpn.net/c/openvpn/+/902?usp=email )

Change subject: route: handle default gateway (net_gateway) and nexthop towards VPN server separately
......................................................................


Abandoned
-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/902?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ifc54be34101c0eb0f3dc479a9480d7219628cc76
Gerrit-Change-Number: 902
Gerrit-PatchSet: 1
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: abandon

[Openvpn-devel] [S] Change in openvpn[master]: dco_win: In dco_new_key, document size assumptions for the integer casts

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1221?usp=email )

Change subject: dco_win: In dco_new_key, document size assumptions for the integer casts
......................................................................

dco_win: In dco_new_key, document size assumptions for the integer casts

And make all casts explicit so that compiler doesn't complain.

Change-Id: I612bf3b1c56d70a89fc04fad6fe36fd9fadfd258
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: MaxF <ma...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1221
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33229.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_win.c
1 file changed, 5 insertions(+), 12 deletions(-)




diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 9e52859..7dd43d6 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -525,11 +525,6 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 int
 dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, dco_key_slot_t slot,
             const uint8_t *encrypt_key, const uint8_t *encrypt_iv, const uint8_t *decrypt_key,
@@ -540,21 +535,23 @@
 
     const int nonce_len = 8;
     size_t key_len = cipher_kt_key_size(ciphername);
+    ASSERT(key_len <= 32);
 
     OVPN_CRYPTO_DATA crypto_data;
     ZeroMemory(&crypto_data, sizeof(crypto_data));
 
     crypto_data.CipherAlg = dco_get_cipher(ciphername);
-    crypto_data.KeyId = keyid;
+    ASSERT(keyid > 0 && keyid <= UCHAR_MAX);
+    crypto_data.KeyId = (unsigned char)keyid;
     crypto_data.PeerId = peerid;
     crypto_data.KeySlot = slot;
 
     CopyMemory(crypto_data.Encrypt.Key, encrypt_key, key_len);
-    crypto_data.Encrypt.KeyLen = (char)key_len;
+    crypto_data.Encrypt.KeyLen = (unsigned char)key_len;
     CopyMemory(crypto_data.Encrypt.NonceTail, encrypt_iv, nonce_len);
 
     CopyMemory(crypto_data.Decrypt.Key, decrypt_key, key_len);
-    crypto_data.Decrypt.KeyLen = (char)key_len;
+    crypto_data.Decrypt.KeyLen = (unsigned char)key_len;
     CopyMemory(crypto_data.Decrypt.NonceTail, decrypt_iv, nonce_len);
 
     ASSERT(crypto_data.CipherAlg > 0);
@@ -570,10 +567,6 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 int
 dco_del_key(dco_context_t *dco, unsigned int peerid, dco_key_slot_t slot)
 {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1221?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I612bf3b1c56d70a89fc04fad6fe36fd9fadfd258
Gerrit-Change-Number: 1221
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: MaxF <ma...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [S] Change in openvpn[master]: dco_win: In dco_new_key, document size assumptions for the integer casts

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#2) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1221?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by MaxF


Change subject: dco_win: In dco_new_key, document size assumptions for the integer casts
......................................................................

dco_win: In dco_new_key, document size assumptions for the integer casts

And make all casts explicit so that compiler doesn't complain.

Change-Id: I612bf3b1c56d70a89fc04fad6fe36fd9fadfd258
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: MaxF <ma...@ma...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1221
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33229.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_win.c
1 file changed, 5 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/1221/2

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 9e52859..7dd43d6 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -525,11 +525,6 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 int
 dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, dco_key_slot_t slot,
             const uint8_t *encrypt_key, const uint8_t *encrypt_iv, const uint8_t *decrypt_key,
@@ -540,21 +535,23 @@
 
     const int nonce_len = 8;
     size_t key_len = cipher_kt_key_size(ciphername);
+    ASSERT(key_len <= 32);
 
     OVPN_CRYPTO_DATA crypto_data;
     ZeroMemory(&crypto_data, sizeof(crypto_data));
 
     crypto_data.CipherAlg = dco_get_cipher(ciphername);
-    crypto_data.KeyId = keyid;
+    ASSERT(keyid > 0 && keyid <= UCHAR_MAX);
+    crypto_data.KeyId = (unsigned char)keyid;
     crypto_data.PeerId = peerid;
     crypto_data.KeySlot = slot;
 
     CopyMemory(crypto_data.Encrypt.Key, encrypt_key, key_len);
-    crypto_data.Encrypt.KeyLen = (char)key_len;
+    crypto_data.Encrypt.KeyLen = (unsigned char)key_len;
     CopyMemory(crypto_data.Encrypt.NonceTail, encrypt_iv, nonce_len);
 
     CopyMemory(crypto_data.Decrypt.Key, decrypt_key, key_len);
-    crypto_data.Decrypt.KeyLen = (char)key_len;
+    crypto_data.Decrypt.KeyLen = (unsigned char)key_len;
     CopyMemory(crypto_data.Decrypt.NonceTail, decrypt_iv, nonce_len);
 
     ASSERT(crypto_data.CipherAlg > 0);
@@ -570,10 +567,6 @@
     return 0;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 int
 dco_del_key(dco_context_t *dco, unsigned int peerid, dco_key_slot_t slot)
 {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1221?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I612bf3b1c56d70a89fc04fad6fe36fd9fadfd258
Gerrit-Change-Number: 1221
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: MaxF <ma...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: dco_win: In dco_new_key, document size assumptions for the integer casts

[Gert D. <ge...@gr...>]

Looks reasonable... especially since there already was a (char) cast for
both KeyId and KeyLen, it was just missing the right signedness...

Your patch has been applied to the master branch.

commit e77c34370dbe0f894a2a927e18eb9f50a5820954
Author: Frank Lichtenheld
Date:   Fri Sep 26 18:51:46 2025 +0200

     dco_win: In dco_new_key, document size assumptions for the integer casts

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: MaxF <ma...@ma...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1221
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg33229.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering