openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: Fix wrong byte order of --dns server

[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/+/1125?usp=email

to review the following change.


Change subject: Fix wrong byte order of --dns server
......................................................................

Fix wrong byte order of --dns server

We already fixed it in commit 82fd89a04f, but
the fix got lost in 2dfc4f8.

tuntap_options require host byte order, and we store
dns servers in struct dns_server in network byte order,
what we got from getaddrinfo().

Fix (again) by converting to the host byte order.

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



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/25/1125/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3ceada0..ff9addd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3573,7 +3573,7 @@
             {
                 if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
                 {
-                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                    tt->dns[tt->dns_len++] = ntohl(s->addr[i].in.a4.s_addr);
                 }
                 else if (tt->dns6_len + 1 < N_DHCP_ADDR)
                 {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1125?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: I37ca672f05e9d77ec586fa28dadc6dc752e574fb
Gerrit-Change-Number: 1125
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] [XS] Change in openvpn[master]: Fix wrong byte order of --dns server

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

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

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

Change subject: Fix wrong byte order of --dns server
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1125?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: I37ca672f05e9d77ec586fa28dadc6dc752e574fb
Gerrit-Change-Number: 1125
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
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-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Thu, 31 Jul 2025 15:28:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [PATCH v1] Fix wrong byte order of --dns server

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

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

We already fixed it in commit 82fd89a04f, but
the fix got lost in 2dfc4f8.

tuntap_options require host byte order, and we store
dns servers in struct dns_server in network byte order,
what we got from getaddrinfo().

Fix (again) by converting to the host byte order.

Change-Id: I37ca672f05e9d77ec586fa28dadc6dc752e574fb
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
---

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

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3ceada0..ff9addd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3573,7 +3573,7 @@
             {
                 if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
                 {
-                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                    tt->dns[tt->dns_len++] = ntohl(s->addr[i].in.a4.s_addr);
                 }
                 else if (tt->dns6_len + 1 < N_DHCP_ADDR)
                 {

[Openvpn-devel] [PATCH applied] Re: Fix wrong byte order of --dns server

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

Yeah, git rebase accident in the "offending" patch (patch was written,
bug was fixed in the "to-be-moved" code, git rebase adjusted the "old"
side, but had no way to fix this for the "new" side and nobody noticed).

Let's try to get better DNS coverage into the automated Windows test rig.

As for the test, just stared at the code and did a windows compile test -
as Lev wrote, we have fixed this bug before (doubly annoying).

Your patch has been applied to the master branch.

commit 63fed92c96c23e061a64683cab431db1269e3f73
Author: Lev Stipakov
Date:   Thu Jul 31 17:29:06 2025 +0200

     Fix wrong byte order of --dns server

     Signed-off-by: Lev Stipakov <le...@op...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32460.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: Fix wrong byte order of --dns server

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

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

Change subject: Fix wrong byte order of --dns server
......................................................................

Fix wrong byte order of --dns server

We already fixed it in commit 82fd89a04f, but
the fix got lost in 2dfc4f8.

tuntap_options require host byte order, and we store
dns servers in struct dns_server in network byte order,
what we got from getaddrinfo().

Fix (again) by converting to the host byte order.

Change-Id: I37ca672f05e9d77ec586fa28dadc6dc752e574fb
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32460.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
1 file changed, 1 insertion(+), 1 deletion(-)




diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3ceada0..ff9addd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3573,7 +3573,7 @@
             {
                 if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
                 {
-                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                    tt->dns[tt->dns_len++] = ntohl(s->addr[i].in.a4.s_addr);
                 }
                 else if (tt->dns6_len + 1 < N_DHCP_ADDR)
                 {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1125?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: I37ca672f05e9d77ec586fa28dadc6dc752e574fb
Gerrit-Change-Number: 1125
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
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]: Fix wrong byte order of --dns server

[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/+/1125?usp=email )

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


Change subject: Fix wrong byte order of --dns server
......................................................................

Fix wrong byte order of --dns server

We already fixed it in commit 82fd89a04f, but
the fix got lost in 2dfc4f8.

tuntap_options require host byte order, and we store
dns servers in struct dns_server in network byte order,
what we got from getaddrinfo().

Fix (again) by converting to the host byte order.

Change-Id: I37ca672f05e9d77ec586fa28dadc6dc752e574fb
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32460.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/25/1125/2

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3ceada0..ff9addd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3573,7 +3573,7 @@
             {
                 if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
                 {
-                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                    tt->dns[tt->dns_len++] = ntohl(s->addr[i].in.a4.s_addr);
                 }
                 else if (tt->dns6_len + 1 < N_DHCP_ADDR)
                 {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1125?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: I37ca672f05e9d77ec586fa28dadc6dc752e574fb
Gerrit-Change-Number: 1125
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset