[Openvpn-devel] [PATCH 6/7] wintun: stop sending TAP-Windows6 ioctls to NDIS device

[Simon Rozman]

Wintun doesn't have its own I/O device. Rather, it taps on existing
Windows-provided NDIS device. Sending TAP-Windows6 IOCTL requests to it
is risky, as TAP-Windows6 is using one of the well-known device types
(FILE_DEVICE_UNKNOWN) with function IDs as 1, 2, 3 etc. raising a chance
of collision as NDIS might react to one of these IOCTLs.

Signed-off-by: Simon Rozman 
---
 src/openvpn/error.c   | 5 -
 src/openvpn/forward.c | 2 +-
 src/openvpn/sig.c | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/error.c b/src/openvpn/error.c
index b2492f2b..8d91a131 100644
--- a/src/openvpn/error.c
+++ b/src/openvpn/error.c
@@ -688,7 +688,10 @@ x_check_status(int status,
 }
 #elif defined(_WIN32)
 /* get possible driver error from TAP-Windows driver */
-extended_msg = tap_win_getinfo(tt, &gc);
+if (!tt->wintun)
+{
+extended_msg = tap_win_getinfo(tt, &gc);
+}
 #endif
 if (!ignore_sys_error(my_errno))
 {
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 6b823613..2bc9d871 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1956,7 +1956,7 @@ pre_select(struct context *c)
 if (check_debug_level(D_TAP_WIN_DEBUG))
 {
 c->c2.timeval.tv_sec = 1;
-if (tuntap_defined(c->c1.tuntap))
+if (tuntap_defined(c->c1.tuntap) && !c->c1.tuntap->wintun)
 {
 tun_show_debug(c->c1.tuntap);
 }
diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index d7f2abb8..f02aa57c 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -315,7 +315,7 @@ print_status(const struct context *c, struct status_output 
*so)
 status_printf(so, "Post-decrypt truncations," counter_format, 
c->c2.n_trunc_post_decrypt);
 #endif
 #ifdef _WIN32
-if (tuntap_defined(c->c1.tuntap))
+if (tuntap_defined(c->c1.tuntap) && !c->c1.tuntap->wintun)
 {
 status_printf(so, "TAP-WIN32 driver status,\"%s\"",
   tap_win_getinfo(c->c1.tuntap, &gc));
-- 
2.24.1.windows.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 4/7] wintun: register ring buffers when iterating adapters

[Simon Rozman]

Wintun adapters may be considered available if ring buffer registration
succeeded. Therefore, we must attempt to register ring buffers when
iterating adapters and continue on failure.

Signed-off-by: Simon Rozman 
---
 src/openvpn/tun.c | 110 +++---
 1 file changed, 64 insertions(+), 46 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 220dee87..9dc9b3a2 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -6129,11 +6129,71 @@ tuntap_dhcp_mask(const struct tuntap *tt, const char 
*device_guid)
 gc_free(&gc);
 }
 
+static bool
+tun_try_open_device(struct tuntap *tt, const char *device_guid, const struct 
device_instance_id_interface *device_instance_id_interface)
+{
+const char *path = NULL;
+char tuntap_device_path[256];
+
+if (tt->wintun)
+{
+const struct device_instance_id_interface *dev_if;
+
+/* Open Wintun adapter */
+for (dev_if = device_instance_id_interface; dev_if != NULL; dev_if = 
dev_if->next)
+{
+if (strcmp(dev_if->net_cfg_instance_id, device_guid) == 0)
+{
+path = dev_if->device_interface_list;
+break;
+}
+}
+if (path == NULL)
+{
+return false;
+}
+}
+else
+{
+/* Open TAP-Windows adapter */
+openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), 
"%s%s%s",
+USERMODEDEVICEDIR,
+device_guid,
+TAP_WIN_SUFFIX);
+path = tuntap_device_path;
+}
+
+tt->hand = CreateFile(path,
+  GENERIC_READ | GENERIC_WRITE,
+  0,/* was: FILE_SHARE_READ */
+  0,
+  OPEN_EXISTING,
+  FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
+  0);
+
+if (tt->hand == INVALID_HANDLE_VALUE)
+{
+msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s", tt->wintun ? 
"Wintun" : "TAP-Windows", path);
+return false;
+}
+
+if (tt->wintun)
+{
+/* Wintun adapter may be considered "open" after ring buffers are 
successfuly registered. */
+if (!wintun_register_ring_buffer(tt))
+{
+msg(D_TUNTAP_INFO, "Failed to register %s adapter ring buffers", 
device_guid);
+return false;
+}
+}
+
+return true;
+}
+
 static void
 tun_open_device(struct tuntap *tt, const char *dev_node, const char 
**device_guid)
 {
 struct gc_arena gc = gc_new();
-char *path = NULL;
 char tuntap_device_path[256];
 const struct tap_reg* tap_reg = get_tap_reg(&gc);
 const struct panel_reg* panel_reg = get_panel_reg(&gc);
@@ -6197,27 +6257,11 @@ tun_open_device(struct tuntap *tt, const char 
*dev_node, const char **device_gui
 
 if (tt->wintun)
 {
-const struct device_instance_id_interface* dev_if;
-
 if (!is_picked_device_wintun)
 {
 /* wintun driver specified but picked adapter is not 
wintun, proceed to next one */
 goto next;
 }
-
-path = NULL;
-for (dev_if = device_instance_id_interface; dev_if != NULL; 
dev_if = dev_if->next)
-{
-if (strcmp(dev_if->net_cfg_instance_id, *device_guid) == 0)
-{
-path = (char*)dev_if->device_interface_list;
-break;
-}
-}
-if (path == NULL)
-{
-goto next;
-}
 }
 else
 {
@@ -6226,28 +6270,9 @@ tun_open_device(struct tuntap *tt, const char *dev_node, 
const char **device_gui
 /* tap-windows6 driver specified but picked adapter is 
wintun, proceed to next one */
 goto next;
 }
-
-/* Open Windows TAP-Windows adapter */
-openvpn_snprintf(tuntap_device_path, 
sizeof(tuntap_device_path), "%s%s%s",
- USERMODEDEVICEDIR,
- *device_guid,
- TAP_WIN_SUFFIX);
-path = tuntap_device_path;
 }
 
-tt->hand = CreateFile(path,
-  GENERIC_READ | GENERIC_WRITE,
-  0,/* was: FILE_SHARE_READ */
-  0,
-  OPEN_EXISTING,
-  FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
-  0);
-
-if (tt->hand == INVALID_HANDLE_VALUE)
-{
-msg(D_TUNTAP_INFO, "CreateFile failed on %s device: %s", 

[Openvpn-devel] [PATCH 2/7] tun.c: upgrade get_device_guid() to return the Windows driver type

[Simon Rozman]

Signed-off-by: Simon Rozman 
---
 src/openvpn/tun.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 053a8232..623ed37b 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -4110,6 +4110,7 @@ static const char *
 get_device_guid(const char *name,
 char *actual_name,
 int actual_name_size,
+bool *wintun,
 const struct tap_reg *tap_reg,
 const struct panel_reg *panel_reg,
 struct gc_arena *gc)
@@ -4145,6 +4146,10 @@ get_device_guid(const char *name,
 {
 buf_printf(&actual, "%s", name);
 }
+if (wintun)
+{
+*wintun = tr->wintun;
+}
 return BSTR(&ret);
 }
 
@@ -4154,6 +4159,10 @@ get_device_guid(const char *name,
 if (tr)
 {
 buf_printf(&actual, "%s", name);
+if (wintun)
+{
+*wintun = tr->wintun;
+}
 buf_printf(&ret, "%s", tr->guid);
 return BSTR(&ret);
 }
@@ -4838,7 +4847,7 @@ tap_allow_nonadmin_access(const char *dev_node)
 if (dev_node)
 {
 /* Get the device GUID for the device specified with --dev-node. */
-device_guid = get_device_guid(dev_node, actual_buffer, 
sizeof(actual_buffer), tap_reg, panel_reg, &gc);
+device_guid = get_device_guid(dev_node, actual_buffer, 
sizeof(actual_buffer), NULL, tap_reg, panel_reg, &gc);
 
 if (!device_guid)
 {
@@ -5412,7 +5421,7 @@ netsh_get_id(const char *dev_node, struct gc_arena *gc)
 
 if (dev_node)
 {
-guid = get_device_guid(dev_node, BPTR(&actual), BCAP(&actual), 
tap_reg, panel_reg, gc);
+guid = get_device_guid(dev_node, BPTR(&actual), BCAP(&actual), NULL, 
tap_reg, panel_reg, gc);
 }
 else
 {
@@ -6132,7 +6141,7 @@ tun_open_device(struct tuntap *tt, const char *dev_node, 
const char **device_gui
 if (dev_node)
 {
 /* Get the device GUID for the device specified with --dev-node. */
-*device_guid = get_device_guid(dev_node, actual_buffer, 
sizeof(actual_buffer), tap_reg, panel_reg, &gc);
+*device_guid = get_device_guid(dev_node, actual_buffer, 
sizeof(actual_buffer), NULL, tap_reg, panel_reg, &gc);
 
 if (!*device_guid)
 {
-- 
2.24.1.windows.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 5/7] wintun: add support for --dev-node

[Simon Rozman]

Signed-off-by: Simon Rozman 
---
 src/openvpn/tun.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 9dc9b3a2..8508b9c0 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -6194,7 +6194,6 @@ static void
 tun_open_device(struct tuntap *tt, const char *dev_node, const char 
**device_guid)
 {
 struct gc_arena gc = gc_new();
-char tuntap_device_path[256];
 const struct tap_reg* tap_reg = get_tap_reg(&gc);
 const struct panel_reg* panel_reg = get_panel_reg(&gc);
 const struct device_instance_id_interface* device_instance_id_interface = 
get_device_instance_id_interface(&gc);
@@ -6207,31 +6206,34 @@ tun_open_device(struct tuntap *tt, const char 
*dev_node, const char **device_gui
  */
 if (dev_node)
 {
+bool is_picked_device_wintun = false;
+
 /* Get the device GUID for the device specified with --dev-node. */
-*device_guid = get_device_guid(dev_node, actual_buffer, 
sizeof(actual_buffer), NULL, tap_reg, panel_reg, &gc);
+*device_guid = get_device_guid(dev_node, actual_buffer, 
sizeof(actual_buffer), &is_picked_device_wintun, tap_reg, panel_reg, &gc);
 
 if (!*device_guid)
 {
-msg(M_FATAL, "TAP-Windows adapter '%s' not found", dev_node);
+msg(M_FATAL, "Adapter '%s' not found", dev_node);
 }
 
-/* Open Windows TAP-Windows adapter */
-openvpn_snprintf(tuntap_device_path, sizeof(tuntap_device_path), 
"%s%s%s",
- USERMODEDEVICEDIR,
- *device_guid,
- TAP_WIN_SUFFIX);
-
-tt->hand = CreateFile(tuntap_device_path,
-  GENERIC_READ | GENERIC_WRITE,
-  0,/* was: FILE_SHARE_READ */
-  0,
-  OPEN_EXISTING,
-  FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED,
-  0);
+if (tt->wintun)
+{
+if (!is_picked_device_wintun)
+{
+msg(M_FATAL, "Adapter '%s' is TAP-Windows, Wintun expected", 
dev_node);
+}
+}
+else
+{
+if (is_picked_device_wintun)
+{
+msg(M_FATAL, "Adapter '%s' is Wintun, TAP-Windows expected", 
dev_node);
+}
+}
 
-if (tt->hand == INVALID_HANDLE_VALUE)
+if (!tun_try_open_device(tt, *device_guid, 
device_instance_id_interface))
 {
-msg(M_ERR, "CreateFile failed on TAP device: %s", 
tuntap_device_path);
+msg(M_FATAL, "Failed to open %s adapter: %s", tt->wintun ? 
"Wintun" : "TAP-Windows", dev_node);
 }
 }
 else
-- 
2.24.1.windows.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/7] tun.c: make wintun_register_ring_buffer() non-fatal on failures

[Simon Rozman]

Wintun allows multiple handles to be opened on it's NDIS device pipe.
Just by succeeding to open the pipe does not warrant the adapter is
unused.

When iterating for available Wintun adapter, we will need to try
registering ring buffers with each one to actually determine which one
is used and which one is not.

Therefore, a failure to register ring buffers should be detectable, but
not M_FATAL.

Signed-off-by: Simon Rozman 
---
 src/openvpn/tun.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 623ed37b..220dee87 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5647,11 +5647,12 @@ register_dns_service(const struct tuntap *tt)
 gc_free(&gc);
 }
 
-static void
+static bool
 service_register_ring_buffers(const struct tuntap *tt)
 {
 HANDLE msg_channel = tt->options.msg_channel;
 ack_message_t ack;
+bool ret = true;
 struct gc_arena gc = gc_new();
 
 register_ring_buffers_message_t msg = {
@@ -5669,13 +5670,13 @@ service_register_ring_buffers(const struct tuntap *tt)
 
 if (!send_msg_iservice(msg_channel, &msg, sizeof(msg), &ack, "Register 
ring buffers"))
 {
-gc_free(&gc);
-return;
+ret = false;
 }
 else if (ack.error_number != NO_ERROR)
 {
-msg(M_FATAL, "Register ring buffers failed using service: %s 
[status=0x%x]",
+msg(M_NONFATAL, "Register ring buffers failed using service: %s 
[status=0x%x]",
 strerror_win32(ack.error_number, &gc), ack.error_number);
+ret = false;
 }
 else
 {
@@ -5683,6 +5684,7 @@ service_register_ring_buffers(const struct tuntap *tt)
 }
 
 gc_free(&gc);
+return ret;
 }
 
 void
@@ -5922,9 +5924,11 @@ tuntap_set_ip_addr(struct tuntap *tt,
 gc_free(&gc);
 }
 
-static void
+static bool
 wintun_register_ring_buffer(struct tuntap *tt)
 {
+bool ret = true;
+
 tt->wintun_send_ring = (struct tun_ring 
*)MapViewOfFile(tt->wintun_send_ring_handle,
 
FILE_MAP_ALL_ACCESS,
 0,
@@ -5939,7 +5943,7 @@ wintun_register_ring_buffer(struct tuntap *tt)
 
 if (tt->options.msg_channel)
 {
-service_register_ring_buffers(tt);
+ret = service_register_ring_buffers(tt);
 }
 else
 {
@@ -5953,13 +5957,16 @@ wintun_register_ring_buffer(struct tuntap *tt)
 tt->rw_handle.read,
 tt->rw_handle.write))
 {
-msg(M_FATAL, "ERROR:  Failed to register ring buffers: %lu", 
GetLastError());
+msg(M_NONFATAL, "Failed to register ring buffers: %lu", 
GetLastError());
+ret = false;
 }
 if (!RevertToSelf())
 {
 msg(M_FATAL, "ERROR:  RevertToSelf error: %lu", GetLastError());
 }
 }
+
+return ret;
 }
 
 static void
@@ -6367,7 +6374,10 @@ open_tun(const char *dev, const char *dev_type, const 
char *dev_node, struct tun
 
 if (tt->wintun)
 {
-wintun_register_ring_buffer(tt);
+if (!wintun_register_ring_buffer(tt))
+{
+msg(M_FATAL, "Failed to register ring buffers");
+}
 }
 else
 {
-- 
2.24.1.windows.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 7/7] tun.c: reword the at_least_one_tap_win() error

[Simon Rozman]

Signed-off-by: Simon Rozman 
---
 src/openvpn/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 8508b9c0..14ff0259 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -4031,7 +4031,7 @@ at_least_one_tap_win(const struct tap_reg *tap_reg)
 {
 if (!tap_reg)
 {
-msg(M_FATAL, "There are no TAP-Windows adapters on this system.  You 
should be able to create a TAP-Windows adapter by going to Start -> All 
Programs -> TAP-Windows -> Utilities -> Add a new TAP-Windows virtual ethernet 
adapter.");
+msg(M_FATAL, "There are no TAP-Windows nor Wintun adapters on this 
system.  You should be able to create an adapter by using tapctl.exe utility.");
 }
 }
 
-- 
2.24.1.windows.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/7] tun.c: make Windows device lookup functions more general

[Simon Rozman]

Since the introduction of Wintun, not all network devices in Windows are
TAP-Windows6. Rather than returning a simple true/false answer, a couple
of functions were reworked to return a corresponding struct tap_reg *
or NULL instead.

As it would make the code `tr = is_tap_win(...)` a bit awkward those
functions (both static) were renamed to better reflect their nature.

Signed-off-by: Simon Rozman 
---
 src/openvpn/tun.c | 39 ++-
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index ad497a71..053a8232 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -3976,10 +3976,10 @@ show_tap_win_adapters(int msglev, int warnlev)
 }
 
 /*
- * Confirm that GUID is a TAP-Windows adapter.
+ * Lookup a TAP-Windows or Wintun adapter by GUID.
  */
-static bool
-is_tap_win(const char *guid, const struct tap_reg *tap_reg)
+static const struct tap_reg *
+get_tap_by_guid(const char *guid, const struct tap_reg *tap_reg)
 {
 const struct tap_reg *tr;
 
@@ -3987,11 +3987,11 @@ is_tap_win(const char *guid, const struct tap_reg 
*tap_reg)
 {
 if (guid && !strcmp(tr->guid, guid))
 {
-return true;
+return tr;
 }
 }
 
-return false;
+return NULL;
 }
 
 static const char *
@@ -4010,16 +4010,16 @@ guid_to_name(const char *guid, const struct panel_reg 
*panel_reg)
 return NULL;
 }
 
-static const char *
-name_to_guid(const char *name, const struct tap_reg *tap_reg, const struct 
panel_reg *panel_reg)
+static const struct tap_reg *
+get_tap_by_name(const char *name, const struct tap_reg *tap_reg, const struct 
panel_reg *panel_reg)
 {
 const struct panel_reg *pr;
 
 for (pr = panel_reg; pr != NULL; pr = pr->next)
 {
-if (name && !strcmp(pr->name, name) && is_tap_win(pr->guid, tap_reg))
+if (name && !strcmp(pr->name, name))
 {
-return pr->guid;
+return get_tap_by_guid(pr->guid, tap_reg);
 }
 }
 
@@ -4116,6 +4116,7 @@ get_device_guid(const char *name,
 {
 struct buffer ret = alloc_buf_gc(256, gc);
 struct buffer actual = clear_buf();
+const struct tap_reg *tr;
 
 /* Make sure we have at least one TAP adapter */
 if (!tap_reg)
@@ -4131,7 +4132,8 @@ get_device_guid(const char *name,
 }
 
 /* Check if GUID was explicitly specified as --dev-node parameter */
-if (is_tap_win(name, tap_reg))
+tr = get_tap_by_guid(name, tap_reg);
+if (tr)
 {
 const char *act = guid_to_name(name, panel_reg);
 buf_printf(&ret, "%s", name);
@@ -4148,11 +4150,11 @@ get_device_guid(const char *name,
 
 /* Lookup TAP adapter in network connections list */
 {
-const char *guid = name_to_guid(name, tap_reg, panel_reg);
-if (guid)
+tr = get_tap_by_name(name, tap_reg, panel_reg);
+if (tr)
 {
 buf_printf(&actual, "%s", name);
-buf_printf(&ret, "%s", guid);
+buf_printf(&ret, "%s", tr->guid);
 return BSTR(&ret);
 }
 }
@@ -4696,11 +4698,14 @@ get_adapter_index_flexible(const char *name)  /* actual 
name or GUID */
 {
 const struct tap_reg *tap_reg = get_tap_reg(&gc);
 const struct panel_reg *panel_reg = get_panel_reg(&gc);
-const char *guid = name_to_guid(name, tap_reg, panel_reg);
-index = get_adapter_index_method_1(guid);
-if (index == TUN_ADAPTER_INDEX_INVALID)
+const struct tap_reg *tr = get_tap_by_name(name, tap_reg, panel_reg);
+if (tr)
 {
-index = get_adapter_index_method_2(guid);
+index = get_adapter_index_method_1(tr->guid);
+if (index == TUN_ADAPTER_INDEX_INVALID)
+{
+index = get_adapter_index_method_2(tr->guid);
+}
 }
 }
 if (index == TUN_ADAPTER_INDEX_INVALID)
-- 
2.24.1.windows.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] Summary of the community meeting (19th December 2019)

[Samuli Seppänen]

Hi,

Here's the summary of the IRC meeting.

---

COMMUNITY MEETING

Place: #openvpn-meeting on irc.freenode.net
Date: Thurday 19th December 2019
Time: 20:00 CET (19:00 UTC)

Planned meeting topics for this meeting were here:



Your local meeting time is easy to check from services such as



SUMMARY

cron2, lev, mattock and rozmansi participated in this meeting.

---

Discussed the status of wintun patches. The original patches have been
merged, with some improvements like support for multiple tunnels. What
remains is refactoring and improvements.

The OpenVPN 2.5 MSI installer code needs to be updated to include wintun
support. Rozmansi is working on it.

--

Discussed moving from netsh to ipapi on Windows. And, by default, using
ipapi via iService. While netsh is easier to debug, using ipapi is much
faster, which has a big effect with a large number of routes. Also noted
that netsh set/add can use "validate=no" to speed up DNS provisioning.

--

Agreed that in the short term having a thin command-line launcher that
allows running OpenVPN with iService from the command-line. The iService
is required to use Wintun with OpenVPN.

--

Noted that IPv6 on forums was still unreachable. Cron2 and mattock
debugged and fixed this during the meeting.

--

Full chatlog attached

(20:54:28) mattock: almost time
(20:55:23) rozmansi [sid334387@gateway/web/irccloud.com/x-wjeamthsozplmfwu] è 
entrato nella stanza.
(20:55:35) rozmansi: hi
(20:58:24) mattock: hi!
(20:58:33) lev__: hello
(20:58:57) mattock: selvanair said he won't make it, but promised to check the 
chatlog
(20:59:04) mattock: cron2 said he'd be a bit late
(20:59:19) mattock: maybe we can start with wintun, if there is anything left 
to be discussed there :)
(21:00:43) lev__: original wintun patch series is merged
(21:00:59) lev__: next we have refactorings and improvements
(21:01:08) rozmansi: exactly
(21:01:17) lev__: some are acked, some are sent but not acked, some are not yet 
sent
(21:01:29) rozmansi: nothing drastical. just bits and odds (I suppose)
(21:01:46) lev__: IMO the most important is support of multiple tunnels
(21:01:58) lev__: functionality-wise
(21:02:02) rozmansi: oh, I can ack one right now. Tested it on my hardware 
today before I left for home...
(21:02:44) lev__: that's your patch :) 
(21:02:58) mattock: btw. "official" topic list is here: 
https://community.openvpn.net/openvpn/wiki/Topics-2019-12-19
(21:03:00) vpnHelper: Title: Topics-2019-12-19 – OpenVPN Community (at 
community.openvpn.net)
(21:03:04) lev__: the one with --dev-node support
(21:03:20) cron2: eww
(21:03:21) cron2: I'm here
(21:04:45) rozmansi: hi
(21:05:06) lev__: about installer - so far wintun support is in patch to 
openvpn-build, but that's NSIS installer 
(21:05:20) mattock: hi cron2!
(21:05:21) lev__: is it so that we plan to use MSI for 2,5
(21:05:27) mattock: yes
(21:05:29) lev__: guten aben
(21:05:33) rozmansi: lev__: yes
(21:06:09) lev__: so maybe we need wintun support there, too
(21:06:45) rozmansi: I have a PR to add MSI packaging on openvpn-build open a 
loong time now. But it's dusty and a lot of things changed in the meanwhile. I 
plan to get it in shape for 2.5
(21:07:41) rozmansi: That PR dates long before Wintun was created. So it has 
TAP-Windows6 driver install only.
(21:07:47) cron2: we want all the goodness in 2.5 :-)
(21:08:24) rozmansi: Adding Wintun to the MSI is easy, as Wintun is shipped as 
ready-to-use MSI merge module...
(21:09:24) rozmansi: What needs more work is to make something similar for the 
TAP-Windows6. The Wix's plugin used to install drivers (Difx) didn't perform 
very well on upgrades the last time I tested it.
(21:09:40) lev__: I've sent a patch today with removes 5s route-delay for 
non-dhcp IP set method. By default we use DHCP (expected wintun) and I was 
wondering, cannot we switch to IPAPI as a default to make connection 5 seconds 
faster
(21:10:18) rozmansi: Wintun should work with ipapi - WireGuard is using 
winipcfg to configure it.
(21:10:36) cron2: yes, we should be using ipapi via iservice by default
(21:10:41) lev__: as I mentioned in -devel, openvpn3 uses netsh and doesn't 
have that delay
(21:10:47) cron2: Selva has been talking about this for a long time
(21:11:06) cron2: netsh is easier to debug than ipapi ("because you can run the 
commands by hand") but ipapi is faster
(21:11:17) cron2: which makes a difference if you're isntalling like 10.000 
routes
(21:11:24) cron2: valdikss does this
(21:12:03) lev__: but that is relevant when you don't use iservice
(21:12:09) rozmansi: another delay, I fixed (but not sent a patch yet) with 
netsh is that netsh ipv4 dns set/add should have validate=no (as per IPv6). 
This speeds DNS provisioning considerably. I recon that's because routes are 
not set _before_ the DNS.
(21:12:25) cron2: we can use IPAPI without iservice
(21:12:52) cron2: and ACK to rozmansi :)
(21:1

[Openvpn-devel] [PATCH applied] Re: tun.c: do not add/remove routes on tun open/close

[Gert Doering]

Your patch has been applied to the master branch.

As discussed on the IRC meeting, I've amended the commit message to make
it more clear what happened.  See below.

(Test built on ubuntu 1604, and also stared-at-code and asked confused
questions :-) )

commit 4f2289893d9d1d448e9654b00d18e13f8fb0936c
Author: Lev Stipakov
Date:   Thu Dec 19 13:18:21 2019 +0200

 tun.c: do not add/remove on-link IPv4 route on tun open/close

 Commit 1c4a47f added route manipulation to open/close tun functions
 (for the IPv4 on-link network), but it turns out to be not needed
 at all.

 Signed-off-by: Lev Stipakov 
 Acked-by: Simon Rozman 
 Message-Id: <20191219111821.313-1-lstipa...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19256.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH applied] Re: tun.c: refactor open_tun() implementation

[Gert Doering]

Your patch has been applied to the master branch.

I have test built on Ubuntu 1604/mingw (works).  No further tests.

Thanks for the refactoring.  It's a good basis for further cleanup.

I have *not* tried to follow this in full detail, trusting Simon 
on this.   What I did do is verify it's all inside WIN32, so it
won't affect other platforms.

What we *will* need to do when you're done with your tun.c-massacre is 
to run a full set of IPv4/IPv6/dual-stack test with all then-supported 
--ip-win32 methods for tap6 and wintun to see if we broke something
in an unexpected way - like "ipv6-only no longer working for default
configs", etc. - this will be an interesting test matrix for Samuli's
windows test rig to set up...  IPv4/IPv6/dual-stack, topology p2p vs.
topology subnet, tap6/tun vs. tap6/tap vs. wintun (= 18 combinations
for each --ip-win32 method).  Yay.

Also, it might be a good excercise to give the resulting tun.c a more
thorough review (when done) - independent of the individual patches
that move "bits from here to there", try to grok the logical flow for
each method, and see if it's logical at all...


Code-wise, I think we should factor out the "flush_neighbors_message"
bit of tuntap_set_ip_addr() - all other iservice functions are really
isolated "fill the structure, do the message handling", not integrated
into a larger function.  This is not new code, but reading through it,
I found it causes itching :-)


commit 509c45f6e6885f4c2d413aa8e0189513098b3dcc
Author: Lev Stipakov
Date:   Wed Nov 13 12:42:16 2019 +0200

 tun.c: refactor open_tun() implementation

 Signed-off-by: Lev Stipakov 
 Acked-by: Simon Rozman 
 Message-Id: <20191113104216.1545-1-lstipa...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19137.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] tun.c: do not add/remove routes on tun open/close

[Simon Rozman]

Makes sense. I am pretty sure the interface network route is not required to
be added explicitly.

Acked-by: Simon Rozman 

Best regards,
Simon

> -Original Message-
> From: Lev Stipakov 
> Sent: Thursday, December 19, 2019 12:18 PM
> To: openvpn-devel@lists.sourceforge.net
> Cc: Lev Stipakov 
> Subject: [Openvpn-devel] [PATCH] tun.c: do not add/remove routes on tun
> open/close
> 
> From: Lev Stipakov 
> 
> Commit 1c4a47f added route manipulation to open/close tun functions,
> which is not really needed:
> 
>  - routes are added in check_add_routes(), triggered by timer
> 
>  - routes are removed in delete_routes(), called by do_close_tun()
> 
> Signed-off-by: Lev Stipakov 
> ---
>  src/openvpn/route.c |  2 +-
>  src/openvpn/route.h |  3 ---
>  src/openvpn/tun.c   | 17 -
>  3 files changed, 1 insertion(+), 21 deletions(-)
> 
> diff --git a/src/openvpn/route.c b/src/openvpn/route.c index
> cc6d5519..97e90e56 100644
> --- a/src/openvpn/route.c
> +++ b/src/openvpn/route.c
> @@ -3019,7 +3019,7 @@ out:
>  return ret;
>  }
> 
> -bool
> +static bool
>  do_route_ipv4_service(const bool add, const struct route_ipv4 *r, const
> struct tuntap *tt)  {
>  DWORD if_index = windows_route_find_if_index(r, tt); diff --git
> a/src/openvpn/route.h b/src/openvpn/route.h index 27b652cd..7dd96091
> 100644
> --- a/src/openvpn/route.h
> +++ b/src/openvpn/route.h
> @@ -321,9 +321,6 @@ void setenv_routes(struct env_set *es, const struct
> route_list *rl);
> 
>  void setenv_routes_ipv6(struct env_set *es, const struct
> route_ipv6_list *rl6);
> 
> -bool do_route_ipv4_service(const bool add, const struct route_ipv4 *r,
> -   const struct tuntap *tt);
> -
>  bool is_special_addr(const char *addr_str);
> 
>  void get_default_gateway(struct route_gateway_info *rgi, diff --git
> a/src/openvpn/tun.c b/src/openvpn/tun.c index 8d87ac41..ad497a71 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -860,21 +860,6 @@ delete_route_connected_v6_net(struct tuntap *tt,  }
> #endif /* if defined(_WIN32) || defined(TARGET_DARWIN) ||
> defined(TARGET_NETBSD) || defined(TARGET_OPENBSD) */
> 
> -#if defined(_WIN32)
> -void
> -do_route_ipv4_service_tun(bool add, const struct tuntap *tt) -{
> -struct route_ipv4 r4;
> -CLEAR(r4);
> -r4.network = tt->local & tt->remote_netmask;
> -r4.netmask = tt->remote_netmask;
> -r4.gateway = tt->local;
> -r4.metric = 0; /* connected route */
> -r4.flags = RT_DEFINED | RT_METRIC_DEFINED;
> -do_route_ipv4_service(add, &r4, tt);
> -}
> -#endif
> -
>  #if defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)  \
>  || defined(TARGET_NETBSD) || defined(TARGET_OPENBSD)
>  /* we can't use true subnet mode on tun on all platforms, as that @@ -
> 1406,7 +1391,6 @@ do_ifconfig_ipv4(struct tuntap *tt, const char
> *ifname, int tun_mtu,
>  if (tt->options.msg_channel && tt->wintun)
>  {
>  do_address_service(true, AF_INET, tt);
> -do_route_ipv4_service_tun(true, tt);
>  do_dns_service(true, AF_INET, tt);
>  }
>  else
> @@ -6489,7 +6473,6 @@ close_tun(struct tuntap *tt, openvpn_net_ctx_t
> *ctx)
>  {
>  if (tt->options.msg_channel)
>  {
> -do_route_ipv4_service_tun(false, tt);
>  do_address_service(false, AF_INET, tt);
>  do_dns_service(false, AF_INET, tt);
>  }
> --
> 2.17.1
> 
> 
> 
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] options.c: do not force route delay when not using DHCP

[Lev Stipakov]

From: Lev Stipakov 

Route delay may be only necessary when we perform
DHCP handshake. When we use IPAPI / netsh / manual,
no delay needed.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/options.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index cebcbb07..a6f40e10 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3001,9 +3001,18 @@ options_postprocess_mutate_invariant(struct options 
*options)
 }
 
 #ifdef _WIN32
+/* when using wintun, kernel doesn't send DHCP requests, so use netsh to 
set IP address and netmask */
+if (options->wintun)
+{
+options->tuntap_options.ip_win32_type = IPW32_SET_NETSH;
+}
+
 if ((dev == DEV_TYPE_TUN || dev == DEV_TYPE_TAP) && 
!options->route_delay_defined)
 {
-if (options->mode == MODE_POINT_TO_POINT)
+/* delay may only be necessary when we perform DHCP handshake */
+const bool dhcp = (options->tuntap_options.ip_win32_type == 
IPW32_SET_DHCP_MASQ)
+  || (options->tuntap_options.ip_win32_type == 
IPW32_SET_ADAPTIVE);
+if ((options->mode == MODE_POINT_TO_POINT) && dhcp)
 {
 options->route_delay_defined = true;
 options->route_delay = 5; /* Vista sometimes has a race without 
this */
@@ -3016,14 +3025,8 @@ options_postprocess_mutate_invariant(struct options 
*options)
 options->ifconfig_noexec = false;
 }
 
-/* for wintun kernel doesn't send DHCP requests, so use netsh to set IP 
address and netmask */
-if (options->wintun)
-{
-options->tuntap_options.ip_win32_type = IPW32_SET_NETSH;
-}
-
 remap_redirect_gateway_flags(options);
-#endif
+#endif /* ifdef _WIN32 */
 
 #if P2MP_SERVER
 /*
-- 
2.17.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] tun.c: do not add/remove routes on tun open/close

[Lev Stipakov]

From: Lev Stipakov 

Commit 1c4a47f added route manipulation to open/close tun functions,
which is not really needed:

 - routes are added in check_add_routes(), triggered by timer

 - routes are removed in delete_routes(), called by do_close_tun()

Signed-off-by: Lev Stipakov 
---
 src/openvpn/route.c |  2 +-
 src/openvpn/route.h |  3 ---
 src/openvpn/tun.c   | 17 -
 3 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index cc6d5519..97e90e56 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -3019,7 +3019,7 @@ out:
 return ret;
 }
 
-bool
+static bool
 do_route_ipv4_service(const bool add, const struct route_ipv4 *r, const struct 
tuntap *tt)
 {
 DWORD if_index = windows_route_find_if_index(r, tt);
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index 27b652cd..7dd96091 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -321,9 +321,6 @@ void setenv_routes(struct env_set *es, const struct 
route_list *rl);
 
 void setenv_routes_ipv6(struct env_set *es, const struct route_ipv6_list *rl6);
 
-bool do_route_ipv4_service(const bool add, const struct route_ipv4 *r,
-   const struct tuntap *tt);
-
 bool is_special_addr(const char *addr_str);
 
 void get_default_gateway(struct route_gateway_info *rgi,
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 8d87ac41..ad497a71 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -860,21 +860,6 @@ delete_route_connected_v6_net(struct tuntap *tt,
 }
 #endif /* if defined(_WIN32) || defined(TARGET_DARWIN) || 
defined(TARGET_NETBSD) || defined(TARGET_OPENBSD) */
 
-#if defined(_WIN32)
-void
-do_route_ipv4_service_tun(bool add, const struct tuntap *tt)
-{
-struct route_ipv4 r4;
-CLEAR(r4);
-r4.network = tt->local & tt->remote_netmask;
-r4.netmask = tt->remote_netmask;
-r4.gateway = tt->local;
-r4.metric = 0; /* connected route */
-r4.flags = RT_DEFINED | RT_METRIC_DEFINED;
-do_route_ipv4_service(add, &r4, tt);
-}
-#endif
-
 #if defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)  \
 || defined(TARGET_NETBSD) || defined(TARGET_OPENBSD)
 /* we can't use true subnet mode on tun on all platforms, as that
@@ -1406,7 +1391,6 @@ do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, 
int tun_mtu,
 if (tt->options.msg_channel && tt->wintun)
 {
 do_address_service(true, AF_INET, tt);
-do_route_ipv4_service_tun(true, tt);
 do_dns_service(true, AF_INET, tt);
 }
 else
@@ -6489,7 +6473,6 @@ close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx)
 {
 if (tt->options.msg_channel)
 {
-do_route_ipv4_service_tun(false, tt);
 do_address_service(false, AF_INET, tt);
 do_dns_service(false, AF_INET, tt);
 }
-- 
2.17.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel