openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: dco linux: avoid sending local port to ovpn

[ralf_lici (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/+/1068?usp=email

to review the following change.


Change subject: dco linux: avoid sending local port to ovpn
......................................................................

dco linux: avoid sending local port to ovpn

When sending an OVPN_CMD_NEW_PEER netlink message to ovpn, we currently
attempt to include the local port along with the local address. However,
`dco_multi_get_localaddr()` does not record the port, so we end up
sending a zero value. This zero is rejected by ovpn's netlink policy,
leading to an error and aborted connection.

Since openvpn does not actually need to send the local port because the
module retrieves it directly from the socket, this commit ensures that
only the local address is sent.

Change-Id: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Signed-off-by: Ralf Lici <ra...@ma...>
---
M src/openvpn/dco_linux.c
1 file changed, 0 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/68/1068/1

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 0345413..22a445a 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -265,13 +265,11 @@
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV4, sizeof(struct in_addr),
                     &((struct sockaddr_in *)localaddr)->sin_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in *)localaddr)->sin_port);
         }
         else if (localaddr->sa_family == AF_INET6)
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV6, sizeof(struct in6_addr),
                     &((struct sockaddr_in6 *)localaddr)->sin6_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in6 *)localaddr)->sin6_port);
         }
     }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1068?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: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Gerrit-Change-Number: 1068
Gerrit-PatchSet: 1
Gerrit-Owner: ralf_lici <ra...@ma...>
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] [XS] Change in openvpn[master]: dco linux: avoid sending local port to ovpn

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

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

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

Change subject: dco linux: avoid sending local port to ovpn
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

Patchset:

PS1: 
thanks for spotting this! indeed this part of the code wasn't well tested.
My may even drop the LOCAL_PORT from the API entirely as it doesn't make much sense (any socket is already bound to some port)



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1068?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: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Gerrit-Change-Number: 1068
Gerrit-PatchSet: 1
Gerrit-Owner: ralf_lici <ra...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ralf_lici <ra...@ma...>
Gerrit-Comment-Date: Wed, 25 Jun 2025 12:57:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [PATCH v1] dco linux: avoid sending local port to ovpn

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

From: Ralf Lici <ra...@ma...>

When sending an OVPN_CMD_NEW_PEER netlink message to ovpn, we currently
attempt to include the local port along with the local address. However,
`dco_multi_get_localaddr()` does not record the port, so we end up
sending a zero value. This zero is rejected by ovpn's netlink policy,
leading to an error and aborted connection.

Since openvpn does not actually need to send the local port because the
module retrieves it directly from the socket, this commit ensures that
only the local address is sent.

Change-Id: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Antonio Quartulli <an...@ma...>
---

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

Acked-by according to Gerrit (reflected above):
Antonio Quartulli <an...@ma...>

        
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 0345413..22a445a 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -265,13 +265,11 @@
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV4, sizeof(struct in_addr),
                     &((struct sockaddr_in *)localaddr)->sin_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in *)localaddr)->sin_port);
         }
         else if (localaddr->sa_family == AF_INET6)
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV6, sizeof(struct in6_addr),
                     &((struct sockaddr_in6 *)localaddr)->sin6_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in6 *)localaddr)->sin6_port);
         }
     }
 

[Openvpn-devel] [PATCH applied] Re: dco linux: avoid sending local port to ovpn

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

I have tested this on Linux + DCO "client side" (which should not excercise
this code path at all, so "no change") and have set up a server instance
that has "--multihome" in the config - and as expected, the current code
fails

2025-06-25 18:37:44 us=736543 freebsd-74-amd64/udp6:194.97.140.3:51620 peer-id=0 Cannot add peer to DCO for freebsd-74-amd64/udp6:194.97.140.3:51620 peer-id=0: Numerical result out of range (-34)

.. and the fixed code succeeds.  Well spotted... (I do have a --multihome
server instance somewhere, but not "with DCO", meh - now I have one).

Your patch has been applied to the master branch.

commit 6c2bd6be4f8ac4f0b25aa05e2d5eb9bf6b736cd1
Author: Ralf Lici
Date:   Wed Jun 25 18:26:31 2025 +0200

     dco linux: avoid sending local port to ovpn

     Signed-off-by: Ralf Lici <ra...@ma...>
     Acked-by: Antonio Quartulli <an...@ma...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31971.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: dco linux: avoid sending local port to ovpn

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

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

Change subject: dco linux: avoid sending local port to ovpn
......................................................................

dco linux: avoid sending local port to ovpn

When sending an OVPN_CMD_NEW_PEER netlink message to ovpn, we currently
attempt to include the local port along with the local address. However,
`dco_multi_get_localaddr()` does not record the port, so we end up
sending a zero value. This zero is rejected by ovpn's netlink policy,
leading to an error and aborted connection.

Since openvpn does not actually need to send the local port because the
module retrieves it directly from the socket, this commit ensures that
only the local address is sent.

Change-Id: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Antonio Quartulli <an...@ma...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31971.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
1 file changed, 0 insertions(+), 2 deletions(-)




diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 0345413..22a445a 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -265,13 +265,11 @@
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV4, sizeof(struct in_addr),
                     &((struct sockaddr_in *)localaddr)->sin_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in *)localaddr)->sin_port);
         }
         else if (localaddr->sa_family == AF_INET6)
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV6, sizeof(struct in6_addr),
                     &((struct sockaddr_in6 *)localaddr)->sin6_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in6 *)localaddr)->sin6_port);
         }
     }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1068?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: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Gerrit-Change-Number: 1068
Gerrit-PatchSet: 2
Gerrit-Owner: ralf_lici <ra...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [XS] Change in openvpn[master]: dco linux: avoid sending local port to ovpn

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

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

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


Change subject: dco linux: avoid sending local port to ovpn
......................................................................

dco linux: avoid sending local port to ovpn

When sending an OVPN_CMD_NEW_PEER netlink message to ovpn, we currently
attempt to include the local port along with the local address. However,
`dco_multi_get_localaddr()` does not record the port, so we end up
sending a zero value. This zero is rejected by ovpn's netlink policy,
leading to an error and aborted connection.

Since openvpn does not actually need to send the local port because the
module retrieves it directly from the socket, this commit ensures that
only the local address is sent.

Change-Id: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Antonio Quartulli <an...@ma...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31971.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_linux.c
1 file changed, 0 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/68/1068/2

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 0345413..22a445a 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -265,13 +265,11 @@
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV4, sizeof(struct in_addr),
                     &((struct sockaddr_in *)localaddr)->sin_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in *)localaddr)->sin_port);
         }
         else if (localaddr->sa_family == AF_INET6)
         {
             NLA_PUT(nl_msg, OVPN_A_PEER_LOCAL_IPV6, sizeof(struct in6_addr),
                     &((struct sockaddr_in6 *)localaddr)->sin6_addr);
-            NLA_PUT_U16(nl_msg, OVPN_A_PEER_LOCAL_PORT, ((struct sockaddr_in6 *)localaddr)->sin6_port);
         }
     }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1068?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: I5d9535d46e5a5488f4a2b637a6fcb99aad668fee
Gerrit-Change-Number: 1068
Gerrit-PatchSet: 2
Gerrit-Owner: ralf_lici <ra...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset