[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-08-11 Thread cron2 (Code Review) 
cron2 has uploaded a new patch set (#4) to the change originally created by 
stipa. ( http://gerrit.openvpn.net/c/openvpn/+/321?usp=email )

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


Change subject: Set WINS servers via interactice service
..

Set WINS servers via interactice service

At the moments WINS servers are set either:

 - via DHCP, which works only for tap-windows6 driver
 - via netsh when running without interactice service

This means that in 2.6 default setup (interactive service and dco)
WINS is silently ignored.

Add WINS support for non-DHCP drivers (like dco) by passing
WINS settings to interactive service and set them there with
netsh call, similar approach as we use for setting DNS.

Fixes https://github.com/OpenVPN/openvpn/issues/373

Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Signed-off-by: Lev Stipakov 
Acked-by: Frank Lichtenheld 
Message-Id: <20230728131246.694-1-lstipa...@gmail.com>
URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26903.html
Signed-off-by: Gert Doering 
(cherry picked from commit 18826de5737789cb74b48fc40a9ff5cb37d38d98)
---
M include/openvpn-msg.h
M src/openvpn/tun.c
M src/openvpnserv/interactive.c
3 files changed, 226 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/321/4

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 8cd2631..a1464cd 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -40,7 +40,9 @@
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
-msg_set_mtu
+msg_set_mtu,
+msg_add_wins_cfg,
+msg_del_wins_cfg
 } message_type_t;

 typedef struct {
@@ -89,6 +91,13 @@
 typedef struct {
 message_header_t header;
 interface_t iface;
+int addr_len;
+inet_address_t addr[4]; /* support up to 4 dns addresses */
+} wins_cfg_message_t;
+
+typedef struct {
+message_header_t header;
+interface_t iface;
 int disable_nbt;
 int nbt_type;
 char scope_id[256];
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index af959bb..1f2539d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -282,6 +282,72 @@
 }

 static bool
+do_wins_service(bool add, const struct tuntap *tt)
+{
+bool ret = false;
+ack_message_t ack;
+struct gc_arena gc = gc_new();
+HANDLE pipe = tt->options.msg_channel;
+int len = tt->options.wins_len;
+int addr_len = add ? len : 0;
+
+if (addr_len == 0 && add) /* no addresses to add */
+{
+return true;
+}
+
+wins_cfg_message_t wins = {
+.header = {
+(add ? msg_add_wins_cfg : msg_del_wins_cfg),
+sizeof(wins_cfg_message_t),
+0
+},
+.iface = {.index = tt->adapter_index, .name = "" },
+.addr_len = addr_len
+};
+
+/* interface name is required */
+strncpy(wins.iface.name, tt->actual_name, sizeof(wins.iface.name));
+wins.iface.name[sizeof(wins.iface.name) - 1] = '\0';
+
+if (addr_len > _countof(wins.addr))
+{
+addr_len = _countof(wins.addr);
+wins.addr_len = addr_len;
+msg(M_WARN, "Number of WINS addresses sent to service truncated to %d",
+addr_len);
+}
+
+for (int i = 0; i < addr_len; ++i)
+{
+wins.addr[i].ipv4.s_addr = htonl(tt->options.wins[i]);
+}
+
+msg(D_LOW, "%s WINS servers on '%s' (if_index = %d) using service",
+(add ? "Setting" : "Deleting"), wins.iface.name, wins.iface.index);
+
+if (!send_msg_iservice(pipe, &wins, sizeof(wins), &ack, "TUN"))
+{
+goto out;
+}
+
+if (ack.error_number != NO_ERROR)
+{
+msg(M_WARN, "TUN: %s WINS failed using service: %s [status=%u 
if_name=%s]",
+(add ? "adding" : "deleting"), strerror_win32(ack.error_number, 
&gc),
+ack.error_number, wins.iface.name);
+goto out;
+}
+
+msg(M_INFO, "WINS servers %s using service", (add ? "set" : "deleted"));
+ret = true;
+
+out:
+gc_free(&gc);
+return ret;
+}
+
+static bool
 do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
 {
 bool ret = false;
@@ -1557,6 +1623,7 @@
 do_address_service(true, AF_INET, tt);
 do_dns_service(true, AF_INET, tt);
 do_dns_domain_service(true, tt);
+do_wins_service(true, tt);
 }
 else
 {
@@ -6979,6 +7046,7 @@
 }
 else if (tt->options.msg_channel)
 {
+do_wins_service(false, tt);
 do_dns_domain_service(false, tt);
 do_dns_service(false, AF_INET, tt);
 do_address_service(false, AF_INET, tt);
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index d73cef0..a47db8a 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -93,6 +93,7 @@
 undo_dns6,
 undo_domain,
 undo

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-08-11 Thread cron2 (Code Review) 
cron2 has submitted this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/321?usp=email )

Change subject: Set WINS servers via interactice service
..

Set WINS servers via interactice service

At the moments WINS servers are set either:

 - via DHCP, which works only for tap-windows6 driver
 - via netsh when running without interactice service

This means that in 2.6 default setup (interactive service and dco)
WINS is silently ignored.

Add WINS support for non-DHCP drivers (like dco) by passing
WINS settings to interactive service and set them there with
netsh call, similar approach as we use for setting DNS.

Fixes https://github.com/OpenVPN/openvpn/issues/373

Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Signed-off-by: Lev Stipakov 
Acked-by: Frank Lichtenheld 
Message-Id: <20230728131246.694-1-lstipa...@gmail.com>
URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26903.html
Signed-off-by: Gert Doering 
(cherry picked from commit 18826de5737789cb74b48fc40a9ff5cb37d38d98)
---
M include/openvpn-msg.h
M src/openvpn/tun.c
M src/openvpnserv/interactive.c
3 files changed, 226 insertions(+), 1 deletion(-)




diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 8cd2631..a1464cd 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -40,7 +40,9 @@
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
-msg_set_mtu
+msg_set_mtu,
+msg_add_wins_cfg,
+msg_del_wins_cfg
 } message_type_t;

 typedef struct {
@@ -89,6 +91,13 @@
 typedef struct {
 message_header_t header;
 interface_t iface;
+int addr_len;
+inet_address_t addr[4]; /* support up to 4 dns addresses */
+} wins_cfg_message_t;
+
+typedef struct {
+message_header_t header;
+interface_t iface;
 int disable_nbt;
 int nbt_type;
 char scope_id[256];
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index af959bb..1f2539d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -282,6 +282,72 @@
 }

 static bool
+do_wins_service(bool add, const struct tuntap *tt)
+{
+bool ret = false;
+ack_message_t ack;
+struct gc_arena gc = gc_new();
+HANDLE pipe = tt->options.msg_channel;
+int len = tt->options.wins_len;
+int addr_len = add ? len : 0;
+
+if (addr_len == 0 && add) /* no addresses to add */
+{
+return true;
+}
+
+wins_cfg_message_t wins = {
+.header = {
+(add ? msg_add_wins_cfg : msg_del_wins_cfg),
+sizeof(wins_cfg_message_t),
+0
+},
+.iface = {.index = tt->adapter_index, .name = "" },
+.addr_len = addr_len
+};
+
+/* interface name is required */
+strncpy(wins.iface.name, tt->actual_name, sizeof(wins.iface.name));
+wins.iface.name[sizeof(wins.iface.name) - 1] = '\0';
+
+if (addr_len > _countof(wins.addr))
+{
+addr_len = _countof(wins.addr);
+wins.addr_len = addr_len;
+msg(M_WARN, "Number of WINS addresses sent to service truncated to %d",
+addr_len);
+}
+
+for (int i = 0; i < addr_len; ++i)
+{
+wins.addr[i].ipv4.s_addr = htonl(tt->options.wins[i]);
+}
+
+msg(D_LOW, "%s WINS servers on '%s' (if_index = %d) using service",
+(add ? "Setting" : "Deleting"), wins.iface.name, wins.iface.index);
+
+if (!send_msg_iservice(pipe, &wins, sizeof(wins), &ack, "TUN"))
+{
+goto out;
+}
+
+if (ack.error_number != NO_ERROR)
+{
+msg(M_WARN, "TUN: %s WINS failed using service: %s [status=%u 
if_name=%s]",
+(add ? "adding" : "deleting"), strerror_win32(ack.error_number, 
&gc),
+ack.error_number, wins.iface.name);
+goto out;
+}
+
+msg(M_INFO, "WINS servers %s using service", (add ? "set" : "deleted"));
+ret = true;
+
+out:
+gc_free(&gc);
+return ret;
+}
+
+static bool
 do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
 {
 bool ret = false;
@@ -1557,6 +1623,7 @@
 do_address_service(true, AF_INET, tt);
 do_dns_service(true, AF_INET, tt);
 do_dns_domain_service(true, tt);
+do_wins_service(true, tt);
 }
 else
 {
@@ -6979,6 +7046,7 @@
 }
 else if (tt->options.msg_channel)
 {
+do_wins_service(false, tt);
 do_dns_domain_service(false, tt);
 do_dns_service(false, AF_INET, tt);
 do_address_service(false, AF_INET, tt);
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index d73cef0..a47db8a 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -93,6 +93,7 @@
 undo_dns6,
 undo_domain,
 undo_ring_buffer,
+undo_wins,
 _undo_type_max
 } undo_type_t;
 typedef list_item_t *undo_lists_t[_undo_type_max];
@@ -1084,6 +1085,63 @@
 }

 /**
+ * Run the command: netsh interface ip $action wins $if_n

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-08-07 Thread flichtenheld (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos, selvanair, stipa.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3:

(1 comment)

Patchset:

PS3:
Acked-By: Frank Lichtenheld 



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: ordex 
Gerrit-Attention: selvanair 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Mon, 07 Aug 2023 09:03:49 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-08-07 Thread flichtenheld (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos, selvanair, stipa.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: ordex 
Gerrit-Attention: selvanair 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Mon, 07 Aug 2023 09:03:20 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread stipa (Code Review) 
Attention is currently required from: cron2, flichtenheld, ordex, plaisthos, 
selvanair.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3:

(1 comment)

File include/openvpn-msg.h:

http://gerrit.openvpn.net/c/openvpn/+/321/comment/14635399_26d4dd36 :
PS3, Line 45: msg_del_wins_cfg
> A general comment/question: wasn't the existing (unused) "msg_add_nbt_cfg" 
> etc meant for adding wins […]
Not sure. There is also unused nbt_cfg_message_t but at present it cannot be 
used to pass more than two addresses and has other members which will be unused 
if we are to implement passing WINS addresses only.

I was thinking to remove those unused pieces but probably not it this commit.



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: flichtenheld 
Gerrit-Attention: ordex 
Gerrit-Attention: selvanair 
Gerrit-Comment-Date: Tue, 01 Aug 2023 06:26:29 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: selvanair 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread selvanair (Code Review) 
Attention is currently required from: cron2, flichtenheld, ordex, plaisthos, 
stipa.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3:

(1 comment)

File src/openvpnserv/interactive.c:

http://gerrit.openvpn.net/c/openvpn/+/321/comment/d18799fd_594b4a55 :
PS1, Line 1406: if (addr_len > 0 || msg->header.type == msg_del_wins_cfg)
> Acknowledged
Though the process is unprivileged, its started by the privileged service which 
ensures that the process is a trusted executable installed in a particular 
location etc. So messages received via the msg-channel pipe can be trusted. 
Note that this pipe is created by the service with a single instance 
restriction and the handle passed on to openvpn.exe. Without this guarantee 
iservice would have been far less safe and secure.

Only the client process (like the GUI) is untrusted.



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: flichtenheld 
Gerrit-Attention: ordex 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Mon, 31 Jul 2023 16:37:32 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: stipa 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread selvanair (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos, stipa.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3:

(1 comment)

File include/openvpn-msg.h:

http://gerrit.openvpn.net/c/openvpn/+/321/comment/e3485767_6d04ba47 :
PS3, Line 45: msg_del_wins_cfg
A general comment/question: wasn't the existing (unused) "msg_add_nbt_cfg" etc 
meant for adding wins support?



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: ordex 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Mon, 31 Jul 2023 15:16:32 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread flichtenheld (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos, selvanair, stipa.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3:

(1 comment)

File src/openvpnserv/interactive.c:

http://gerrit.openvpn.net/c/openvpn/+/321/comment/71ba4ee8_026f50da :
PS1, Line 1406: if (addr_len > 0 || msg->header.type == msg_del_wins_cfg)
> Normally no, but this request comes from unprivileged userspace process, 
> which we probably do not wa […]
Acknowledged



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: ordex 
Gerrit-Attention: selvanair 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Mon, 31 Jul 2023 11:43:19 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: stipa 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread stipa (Code Review) 
Attention is currently required from: cron2, flichtenheld, ordex, plaisthos, 
selvanair.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3:

(3 comments)

File src/openvpnserv/interactive.c:

http://gerrit.openvpn.net/c/openvpn/+/321/comment/0b804f50_4d9c1022 :
PS1, Line 1089:  * @param  action  "delete" or "add"
> Right. I copypasted it from DNS version which uses add.
Done


http://gerrit.openvpn.net/c/openvpn/+/321/comment/8542b98a_e7eb2982 :
PS1, Line 1094:  * if action = "set" then "static" is added before $addr
> Good point. […]
Done


http://gerrit.openvpn.net/c/openvpn/+/321/comment/7183cc1d_59f0330f :
PS1, Line 1436: if (msg->addr_len > 0)
> Should not matter here, copypaste from similar DNS function. But I can change 
> it.
Done



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: flichtenheld 
Gerrit-Attention: ordex 
Gerrit-Attention: selvanair 
Gerrit-Comment-Date: Mon, 31 Jul 2023 11:30:27 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: stipa 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread stipa (Code Review) 
Attention is currently required from: cron2, flichtenheld, ordex, plaisthos, 
selvanair.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 3:

(4 comments)

File src/openvpnserv/interactive.c:

http://gerrit.openvpn.net/c/openvpn/+/321/comment/6605e8bb_4222a8d1 :
PS1, Line 1089:  * @param  action  "delete" or "add"
> This says "add" but the rest of the function seems to use "set". […]
Right. I copypasted it from DNS version which uses add.


http://gerrit.openvpn.net/c/openvpn/+/321/comment/b9b566c1_c7e3b460 :
PS1, Line 1094:  * if action = "set" then "static" is added before $addr
> Why are you using "set" here? The documentation (https://learn.microsoft. […]
Good point. The reason I use "set" is because I checked the logs of current 
implementation which uses "set". Idea of multiple WINS servers slipped from my 
mind. I checked the code and we use "set + static" for the first server and 
"set" for the rest, if any. Will to the same here.


http://gerrit.openvpn.net/c/openvpn/+/321/comment/bf8912f9_5365fe44 :
PS1, Line 1406: if (addr_len > 0 || msg->header.type == msg_del_wins_cfg)
> Is there ever a case where this would be false?
Normally no, but this request comes from unprivileged userspace process, which 
we probably do not want to trust.


http://gerrit.openvpn.net/c/openvpn/+/321/comment/0b1bcdb1_cc02227e :
PS1, Line 1436: if (msg->addr_len > 0)
> Nitpick: Should be addr_len, not msg->addr_len
Should not matter here, copypaste from similar DNS function. But I can change 
it.



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 3
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: flichtenheld 
Gerrit-Attention: ordex 
Gerrit-Attention: selvanair 
Gerrit-Comment-Date: Mon, 31 Jul 2023 11:28:31 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread stipa (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos, selvanair, stipa.

Hello cron2, flichtenheld, ordex, plaisthos, selvanair,

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

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

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


Change subject: Set WINS servers via interactice service
..

Set WINS servers via interactice service

At the moments WINS servers are set either:

 - via DHCP, which works only for tap-windows6 driver
 - via netsh when running without interactice service

This means that in 2.6 default setup (interactive service and dco)
WINS is silently ignored.

Add WINS support for non-DHCP drivers (like dco) by passing
WINS settings to interactive service and set them there with
netsh call, similar approach as we use for setting DNS.

Fixes https://github.com/OpenVPN/openvpn/issues/373

Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Signed-off-by: Lev Stipakov 
---
M include/openvpn-msg.h
M src/openvpn/tun.c
M src/openvpnserv/interactive.c
3 files changed, 226 insertions(+), 1 deletion(-)


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

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 8cd2631..a1464cd 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -40,7 +40,9 @@
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
-msg_set_mtu
+msg_set_mtu,
+msg_add_wins_cfg,
+msg_del_wins_cfg
 } message_type_t;

 typedef struct {
@@ -89,6 +91,13 @@
 typedef struct {
 message_header_t header;
 interface_t iface;
+int addr_len;
+inet_address_t addr[4]; /* support up to 4 dns addresses */
+} wins_cfg_message_t;
+
+typedef struct {
+message_header_t header;
+interface_t iface;
 int disable_nbt;
 int nbt_type;
 char scope_id[256];
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 4ef390a..f5c698c 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -282,6 +282,72 @@
 }

 static bool
+do_wins_service(bool add, const struct tuntap *tt)
+{
+bool ret = false;
+ack_message_t ack;
+struct gc_arena gc = gc_new();
+HANDLE pipe = tt->options.msg_channel;
+int len = tt->options.wins_len;
+int addr_len = add ? len : 0;
+
+if (addr_len == 0 && add) /* no addresses to add */
+{
+return true;
+}
+
+wins_cfg_message_t wins = {
+.header = {
+(add ? msg_add_wins_cfg : msg_del_wins_cfg),
+sizeof(wins_cfg_message_t),
+0
+},
+.iface = {.index = tt->adapter_index, .name = "" },
+.addr_len = addr_len
+};
+
+/* interface name is required */
+strncpy(wins.iface.name, tt->actual_name, sizeof(wins.iface.name));
+wins.iface.name[sizeof(wins.iface.name) - 1] = '\0';
+
+if (addr_len > _countof(wins.addr))
+{
+addr_len = _countof(wins.addr);
+wins.addr_len = addr_len;
+msg(M_WARN, "Number of WINS addresses sent to service truncated to %d",
+addr_len);
+}
+
+for (int i = 0; i < addr_len; ++i)
+{
+wins.addr[i].ipv4.s_addr = htonl(tt->options.wins[i]);
+}
+
+msg(D_LOW, "%s WINS servers on '%s' (if_index = %d) using service",
+(add ? "Setting" : "Deleting"), wins.iface.name, wins.iface.index);
+
+if (!send_msg_iservice(pipe, &wins, sizeof(wins), &ack, "TUN"))
+{
+goto out;
+}
+
+if (ack.error_number != NO_ERROR)
+{
+msg(M_WARN, "TUN: %s WINS failed using service: %s [status=%u 
if_name=%s]",
+(add ? "adding" : "deleting"), strerror_win32(ack.error_number, 
&gc),
+ack.error_number, wins.iface.name);
+goto out;
+}
+
+msg(M_INFO, "WINS servers %s using service", (add ? "set" : "deleted"));
+ret = true;
+
+out:
+gc_free(&gc);
+return ret;
+}
+
+static bool
 do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
 {
 bool ret = false;
@@ -1557,6 +1623,7 @@
 do_address_service(true, AF_INET, tt);
 do_dns_service(true, AF_INET, tt);
 do_dns_domain_service(true, tt);
+do_wins_service(true, tt);
 }
 else
 {
@@ -6979,6 +7046,7 @@
 }
 else if (tt->options.msg_channel)
 {
+do_wins_service(false, tt);
 do_dns_domain_service(false, tt);
 do_dns_service(false, AF_INET, tt);
 do_address_service(false, AF_INET, tt);
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index d73cef0..a47db8a 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -93,6 +93,7 @@
 undo_dns6,
 undo_domain,
 undo_ring_buffer,
+undo_wins,
 _undo_type_max
 } undo_type_t;
 typedef list_item_t *undo_lists_t[_undo_type_max];
@@ -1084,6 +1085,63 @@
 }

 /**
+ * Run the command: netsh interface ip $action wins $if_name

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-31 Thread flichtenheld (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos, selvanair, stipa.

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

Change subject: Set WINS servers via interactice service
..


Patch Set 1:

(4 comments)

File src/openvpnserv/interactive.c:

http://gerrit.openvpn.net/c/openvpn/+/321/comment/e7e6ec3b_6554de56 :
PS1, Line 1089:  * @param  action  "delete" or "add"
This says "add" but the rest of the function seems to use "set". But also see 
below for "add" vs "set".


http://gerrit.openvpn.net/c/openvpn/+/321/comment/9fdd7e73_de5a661e :
PS1, Line 1094:  * if action = "set" then "static" is added before $addr
Why are you using "set" here? The documentation 
(https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-r2-and-2008/cc731521(v=ws.10)#set-winsserver)
 seems to suggest that that can only set one WINS server ("If the interface is 
already statically configured, the static parameter replaces the existing WINS 
server address list with the one specified in the set winsserver command.") 
Shouldn't you use "add" instead?


http://gerrit.openvpn.net/c/openvpn/+/321/comment/a93a828d_867cc63f :
PS1, Line 1406: if (addr_len > 0 || msg->header.type == msg_del_wins_cfg)
Is there ever a case where this would be false?


http://gerrit.openvpn.net/c/openvpn/+/321/comment/f983786a_f046e4f1 :
PS1, Line 1436: if (msg->addr_len > 0)
Nitpick: Should be addr_len, not msg->addr_len



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

Gerrit-Project: openvpn
Gerrit-Branch: release/2.6
Gerrit-Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Gerrit-Change-Number: 321
Gerrit-PatchSet: 1
Gerrit-Owner: stipa 
Gerrit-Reviewer: cron2
Gerrit-Reviewer: flichtenheld 
Gerrit-Reviewer: ordex 
Gerrit-Reviewer: plaisthos 
Gerrit-Reviewer: selvanair 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2
Gerrit-Attention: ordex 
Gerrit-Attention: selvanair 
Gerrit-Attention: stipa 
Gerrit-Comment-Date: Mon, 31 Jul 2023 09:29:57 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[release/2.6]: Set WINS servers via interactice service

2023-07-28 Thread stipa (Code Review) 
Attention is currently required from: flichtenheld.

Hello flichtenheld,

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

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

to review the following change.


Change subject: Set WINS servers via interactice service
..

Set WINS servers via interactice service

At the moments WINS servers are set either:

 - via DHCP, which works only for tap-windows6 driver
 - via netsh when running without interactice service

This means that in 2.6 default setup (interactive service and dco)
WINS is silently ignored.

Add WINS support for non-DHCP drivers (like dco) by passing
WINS settings to interactive service and set them there with
netsh call, similar approach as we use for setting DNS.

Fixes https://github.com/OpenVPN/openvpn/issues/373

Change-Id: I47c22dcb728011dcedaae47cd03a57219e9c7607
Signed-off-by: Lev Stipakov 
---
M include/openvpn-msg.h
M src/openvpn/tun.c
M src/openvpnserv/interactive.c
3 files changed, 240 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/321/1

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 8cd2631..a1464cd 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -40,7 +40,9 @@
 msg_register_dns,
 msg_enable_dhcp,
 msg_register_ring_buffers,
-msg_set_mtu
+msg_set_mtu,
+msg_add_wins_cfg,
+msg_del_wins_cfg
 } message_type_t;

 typedef struct {
@@ -89,6 +91,13 @@
 typedef struct {
 message_header_t header;
 interface_t iface;
+int addr_len;
+inet_address_t addr[4]; /* support up to 4 dns addresses */
+} wins_cfg_message_t;
+
+typedef struct {
+message_header_t header;
+interface_t iface;
 int disable_nbt;
 int nbt_type;
 char scope_id[256];
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index af959bb..1f2539d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -282,6 +282,72 @@
 }

 static bool
+do_wins_service(bool add, const struct tuntap *tt)
+{
+bool ret = false;
+ack_message_t ack;
+struct gc_arena gc = gc_new();
+HANDLE pipe = tt->options.msg_channel;
+int len = tt->options.wins_len;
+int addr_len = add ? len : 0;
+
+if (addr_len == 0 && add) /* no addresses to add */
+{
+return true;
+}
+
+wins_cfg_message_t wins = {
+.header = {
+(add ? msg_add_wins_cfg : msg_del_wins_cfg),
+sizeof(wins_cfg_message_t),
+0
+},
+.iface = {.index = tt->adapter_index, .name = "" },
+.addr_len = addr_len
+};
+
+/* interface name is required */
+strncpy(wins.iface.name, tt->actual_name, sizeof(wins.iface.name));
+wins.iface.name[sizeof(wins.iface.name) - 1] = '\0';
+
+if (addr_len > _countof(wins.addr))
+{
+addr_len = _countof(wins.addr);
+wins.addr_len = addr_len;
+msg(M_WARN, "Number of WINS addresses sent to service truncated to %d",
+addr_len);
+}
+
+for (int i = 0; i < addr_len; ++i)
+{
+wins.addr[i].ipv4.s_addr = htonl(tt->options.wins[i]);
+}
+
+msg(D_LOW, "%s WINS servers on '%s' (if_index = %d) using service",
+(add ? "Setting" : "Deleting"), wins.iface.name, wins.iface.index);
+
+if (!send_msg_iservice(pipe, &wins, sizeof(wins), &ack, "TUN"))
+{
+goto out;
+}
+
+if (ack.error_number != NO_ERROR)
+{
+msg(M_WARN, "TUN: %s WINS failed using service: %s [status=%u 
if_name=%s]",
+(add ? "adding" : "deleting"), strerror_win32(ack.error_number, 
&gc),
+ack.error_number, wins.iface.name);
+goto out;
+}
+
+msg(M_INFO, "WINS servers %s using service", (add ? "set" : "deleted"));
+ret = true;
+
+out:
+gc_free(&gc);
+return ret;
+}
+
+static bool
 do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
 {
 bool ret = false;
@@ -1557,6 +1623,7 @@
 do_address_service(true, AF_INET, tt);
 do_dns_service(true, AF_INET, tt);
 do_dns_domain_service(true, tt);
+do_wins_service(true, tt);
 }
 else
 {
@@ -6979,6 +7046,7 @@
 }
 else if (tt->options.msg_channel)
 {
+do_wins_service(false, tt);
 do_dns_domain_service(false, tt);
 do_dns_service(false, AF_INET, tt);
 do_address_service(false, AF_INET, tt);
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index d73cef0..6c54e62 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -93,6 +93,7 @@
 undo_dns6,
 undo_domain,
 undo_ring_buffer,
+undo_wins,
 _undo_type_max
 } undo_type_t;
 typedef list_item_t *undo_lists_t[_undo_type_max];
@@ -1084,6 +1085,63 @@
 }

 /**
+ * Run the command: netsh interface ip $action wins $if_name [static] $addr
+ * @param  action  "delete" or "add"
+ * @param