Message ID | 1538510474-27602-3-git-send-email-selva.nair@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel] Enable dhcp on tap adapter using interactive service | expand |
Built and tested on VS, works as expected. ACK. ti 2. lokak. 2018 klo 23.02 selva.nair@gmail.com kirjoitti: > From: Selva Nair <selva.nair@gmail.com> > > Move writing the message buffer to the interactive service pipe and > reading acknowledgement to a function. > > A minor bug in open_tun where the ack data could be read even after > a communication error is fixed. > > Signed-off-by: Selva Nair <selva.nair@gmail.com> > --- > src/openvpn/route.c | 6 +----- > src/openvpn/tun.c | 34 +++++++++------------------------- > src/openvpn/win32.c | 27 ++++++++++++++++++++++----- > src/openvpn/win32.h | 9 +++++++++ > 4 files changed, 41 insertions(+), 35 deletions(-) > > diff --git a/src/openvpn/route.c b/src/openvpn/route.c > index ff39230..8a3e8b4 100644 > --- a/src/openvpn/route.c > +++ b/src/openvpn/route.c > @@ -2991,16 +2991,12 @@ del_route_ipapi(const struct route_ipv4 *r, const > struct tuntap *tt) > static bool > do_route_service(const bool add, const route_message_t *rt, const size_t > size, HANDLE pipe) > { > - DWORD len; > bool ret = false; > ack_message_t ack; > struct gc_arena gc = gc_new(); > > - if (!WriteFile(pipe, rt, size, &len, NULL) > - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) > + if (!send_msg_iservice(pipe, rt, size, &ack, "ROUTE")) > { > - msg(M_WARN, "ROUTE: could not talk to service: %s [%lu]", > - strerror_win32(GetLastError(), &gc), GetLastError()); > goto out; > } > > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index a2d5315..948fd17 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -82,7 +82,6 @@ static DWORD get_adapter_index_flexible(const char > *name); > static bool > do_address_service(const bool add, const short family, const struct > tuntap *tt) > { > - DWORD len; > bool ret = false; > ack_message_t ack; > struct gc_arena gc = gc_new(); > @@ -115,11 +114,8 @@ do_address_service(const bool add, const short > family, const struct tuntap *tt) > addr.prefix_len = tt->netbits_ipv6; > } > > - if (!WriteFile(pipe, &addr, sizeof(addr), &len, NULL) > - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) > + if (!send_msg_iservice(pipe, &addr, sizeof(addr), &ack, "TUN")) > { > - msg(M_WARN, "TUN: could not talk to service: %s [%lu]", > - strerror_win32(GetLastError(), &gc), GetLastError()); > goto out; > } > > @@ -141,7 +137,6 @@ out: > static bool > do_dns6_service(bool add, const struct tuntap *tt) > { > - DWORD len; > bool ret = false; > ack_message_t ack; > struct gc_arena gc = gc_new(); > @@ -185,11 +180,8 @@ do_dns6_service(bool add, const struct tuntap *tt) > msg(D_LOW, "%s IPv6 dns servers on '%s' (if_index = %d) using > service", > (add ? "Setting" : "Deleting"), dns.iface.name, dns.iface.index); > > - if (!WriteFile(pipe, &dns, sizeof(dns), &len, NULL) > - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) > + if (!send_msg_iservice(pipe, &dns, sizeof(dns), &ack, "TUN")) > { > - msg(M_WARN, "TUN: could not talk to service: %s [%lu]", > - strerror_win32(GetLastError(), &gc), GetLastError()); > goto out; > } > > @@ -5222,11 +5214,8 @@ service_enable_dhcp(const struct tuntap *tt) > .iface = { .index = tt->adapter_index, .name = "" } > }; > > - if (!WriteFile(pipe, &dhcp, sizeof(dhcp), &len, NULL) > - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) > + if (!send_msg_iservice(pipe, &dhcp, sizeof(dhcp), &ack, > "Enable_dhcp")) > { > - msg(M_WARN, "Enable_dhcp: could not talk to service: %s [%lu]", > - strerror_win32(GetLastError(), &gc), GetLastError()); > goto out; > } > > @@ -5461,18 +5450,16 @@ fork_dhcp_action(struct tuntap *tt) > static void > register_dns_service(const struct tuntap *tt) > { > - DWORD len; > HANDLE msg_channel = tt->options.msg_channel; > ack_message_t ack; > struct gc_arena gc = gc_new(); > > message_header_t rdns = { msg_register_dns, sizeof(message_header_t), > 0 }; > > - if (!WriteFile(msg_channel, &rdns, sizeof(rdns), &len, NULL) > - || !ReadFile(msg_channel, &ack, sizeof(ack), &len, NULL)) > + if (!send_msg_iservice(msg_channel, &rdns, sizeof(rdns), &ack, > "Register_dns")) > { > - msg(M_WARN, "Register_dns: could not talk to service: %s > [status=0x%lx]", > - strerror_win32(GetLastError(), &gc), GetLastError()); > + gc_free(&gc); > + return; > } > > else if (ack.error_number != NO_ERROR) > @@ -5936,14 +5923,11 @@ open_tun(const char *dev, const char *dev_type, > const char *dev_node, struct tun > .iface = { .index = index, .name = "" } > }; > > - if (!WriteFile(tt->options.msg_channel, &msg, > sizeof(msg), &len, NULL) > - || !ReadFile(tt->options.msg_channel, &ack, > sizeof(ack), &len, NULL)) > + if (send_msg_iservice(tt->options.msg_channel, &msg, > sizeof(msg), > + &ack, "TUN")) > { > - msg(M_WARN, "TUN: could not talk to service: %s > [%lu]", > - strerror_win32(GetLastError(), &gc), > GetLastError()); > + status = ack.error_number; > } > - > - status = ack.error_number; > } > else > { > diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c > index 3905524..e43296e 100644 > --- a/src/openvpn/win32.c > +++ b/src/openvpn/win32.c > @@ -1264,7 +1264,6 @@ win_get_tempdir(void) > static bool > win_block_dns_service(bool add, int index, const HANDLE pipe) > { > - DWORD len; > bool ret = false; > ack_message_t ack; > struct gc_arena gc = gc_new(); > @@ -1278,11 +1277,8 @@ win_block_dns_service(bool add, int index, const > HANDLE pipe) > .iface = { .index = index, .name = "" } > }; > > - if (!WriteFile(pipe, &data, sizeof(data), &len, NULL) > - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) > + if (!send_msg_iservice(pipe, &data, sizeof(data), &ack, "Block_DNS")) > { > - msg(M_WARN, "Block_DNS: could not talk to service: %s [%lu]", > - strerror_win32(GetLastError(), &gc), GetLastError()); > goto out; > } > > @@ -1473,4 +1469,25 @@ win32_version_string(struct gc_arena *gc, bool > add_name) > return (const char *)out.data; > } > > +bool > +send_msg_iservice(HANDLE pipe, const void *data, size_t size, > + ack_message_t *ack, const char *context) > +{ > + struct gc_arena gc = gc_new(); > + DWORD len; > + bool ret = true; > + > + if (!WriteFile(pipe, data, size, &len, NULL) > + || !ReadFile(pipe, ack, sizeof(*ack), &len, NULL)) > + { > + msg(M_WARN, "%s: could not talk to service: %s [%lu]", > + context? context : "Unknown", > + strerror_win32(GetLastError(), &gc), GetLastError()); > + ret = false; > + } > + > + gc_free(&gc); > + return ret; > +} > + > #endif /* ifdef _WIN32 */ > diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h > index 4b99a5e..b5cbe25 100644 > --- a/src/openvpn/win32.h > +++ b/src/openvpn/win32.h > @@ -26,6 +26,7 @@ > #define OPENVPN_WIN32_H > > #include "mtu.h" > +#include "openvpn-msg.h" > > /* location of executables */ > #define SYS_PATH_ENV_VAR_NAME "SystemRoot" /* environmental variable > name that normally contains the system path */ > @@ -307,5 +308,13 @@ int win32_version_info(void); > */ > const char *win32_version_string(struct gc_arena *gc, bool add_name); > > +/* > + * Send the |size| bytes in buffer |data| to the interactive service > |pipe| > + * and read the result in |ack|. Returns false on communication error. > + * The string in |context| is used to prefix error messages. > + */ > +bool send_msg_iservice(HANDLE pipe, const void *data, size_t size, > + ack_message_t *ack, const char *context); > + > #endif /* ifndef OPENVPN_WIN32_H */ > #endif /* ifdef _WIN32 */ > -- > 2.1.4 > > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel >
Acked-by: Gert Doering <gert@greenie.muc.de> Patch looks good. This one I have actually tested :-) - test build on ubuntu 16.04/mingw, test run on win7. This test also includes setting of DHCP mode from an unprivileged user (which works) Fri Oct 05 16:26:52 2018 DHCP enabled on interface 15 using service Your patch has been applied to the master branch. Only merged to master because "refactoring". commit 717c19300e27db8127553800cf8e0d81bd5bd5a8 Author: Selva Nair Date: Tue Oct 2 16:01:14 2018 -0400 Refactor sending commands to interactive service Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <1538510474-27602-3-git-send-email-selva.nair@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17519.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/route.c b/src/openvpn/route.c index ff39230..8a3e8b4 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -2991,16 +2991,12 @@ del_route_ipapi(const struct route_ipv4 *r, const struct tuntap *tt) static bool do_route_service(const bool add, const route_message_t *rt, const size_t size, HANDLE pipe) { - DWORD len; bool ret = false; ack_message_t ack; struct gc_arena gc = gc_new(); - if (!WriteFile(pipe, rt, size, &len, NULL) - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) + if (!send_msg_iservice(pipe, rt, size, &ack, "ROUTE")) { - msg(M_WARN, "ROUTE: could not talk to service: %s [%lu]", - strerror_win32(GetLastError(), &gc), GetLastError()); goto out; } diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index a2d5315..948fd17 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -82,7 +82,6 @@ static DWORD get_adapter_index_flexible(const char *name); static bool do_address_service(const bool add, const short family, const struct tuntap *tt) { - DWORD len; bool ret = false; ack_message_t ack; struct gc_arena gc = gc_new(); @@ -115,11 +114,8 @@ do_address_service(const bool add, const short family, const struct tuntap *tt) addr.prefix_len = tt->netbits_ipv6; } - if (!WriteFile(pipe, &addr, sizeof(addr), &len, NULL) - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) + if (!send_msg_iservice(pipe, &addr, sizeof(addr), &ack, "TUN")) { - msg(M_WARN, "TUN: could not talk to service: %s [%lu]", - strerror_win32(GetLastError(), &gc), GetLastError()); goto out; } @@ -141,7 +137,6 @@ out: static bool do_dns6_service(bool add, const struct tuntap *tt) { - DWORD len; bool ret = false; ack_message_t ack; struct gc_arena gc = gc_new(); @@ -185,11 +180,8 @@ do_dns6_service(bool add, const struct tuntap *tt) msg(D_LOW, "%s IPv6 dns servers on '%s' (if_index = %d) using service", (add ? "Setting" : "Deleting"), dns.iface.name, dns.iface.index); - if (!WriteFile(pipe, &dns, sizeof(dns), &len, NULL) - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) + if (!send_msg_iservice(pipe, &dns, sizeof(dns), &ack, "TUN")) { - msg(M_WARN, "TUN: could not talk to service: %s [%lu]", - strerror_win32(GetLastError(), &gc), GetLastError()); goto out; } @@ -5222,11 +5214,8 @@ service_enable_dhcp(const struct tuntap *tt) .iface = { .index = tt->adapter_index, .name = "" } }; - if (!WriteFile(pipe, &dhcp, sizeof(dhcp), &len, NULL) - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) + if (!send_msg_iservice(pipe, &dhcp, sizeof(dhcp), &ack, "Enable_dhcp")) { - msg(M_WARN, "Enable_dhcp: could not talk to service: %s [%lu]", - strerror_win32(GetLastError(), &gc), GetLastError()); goto out; } @@ -5461,18 +5450,16 @@ fork_dhcp_action(struct tuntap *tt) static void register_dns_service(const struct tuntap *tt) { - DWORD len; HANDLE msg_channel = tt->options.msg_channel; ack_message_t ack; struct gc_arena gc = gc_new(); message_header_t rdns = { msg_register_dns, sizeof(message_header_t), 0 }; - if (!WriteFile(msg_channel, &rdns, sizeof(rdns), &len, NULL) - || !ReadFile(msg_channel, &ack, sizeof(ack), &len, NULL)) + if (!send_msg_iservice(msg_channel, &rdns, sizeof(rdns), &ack, "Register_dns")) { - msg(M_WARN, "Register_dns: could not talk to service: %s [status=0x%lx]", - strerror_win32(GetLastError(), &gc), GetLastError()); + gc_free(&gc); + return; } else if (ack.error_number != NO_ERROR) @@ -5936,14 +5923,11 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun .iface = { .index = index, .name = "" } }; - if (!WriteFile(tt->options.msg_channel, &msg, sizeof(msg), &len, NULL) - || !ReadFile(tt->options.msg_channel, &ack, sizeof(ack), &len, NULL)) + if (send_msg_iservice(tt->options.msg_channel, &msg, sizeof(msg), + &ack, "TUN")) { - msg(M_WARN, "TUN: could not talk to service: %s [%lu]", - strerror_win32(GetLastError(), &gc), GetLastError()); + status = ack.error_number; } - - status = ack.error_number; } else { diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c index 3905524..e43296e 100644 --- a/src/openvpn/win32.c +++ b/src/openvpn/win32.c @@ -1264,7 +1264,6 @@ win_get_tempdir(void) static bool win_block_dns_service(bool add, int index, const HANDLE pipe) { - DWORD len; bool ret = false; ack_message_t ack; struct gc_arena gc = gc_new(); @@ -1278,11 +1277,8 @@ win_block_dns_service(bool add, int index, const HANDLE pipe) .iface = { .index = index, .name = "" } }; - if (!WriteFile(pipe, &data, sizeof(data), &len, NULL) - || !ReadFile(pipe, &ack, sizeof(ack), &len, NULL)) + if (!send_msg_iservice(pipe, &data, sizeof(data), &ack, "Block_DNS")) { - msg(M_WARN, "Block_DNS: could not talk to service: %s [%lu]", - strerror_win32(GetLastError(), &gc), GetLastError()); goto out; } @@ -1473,4 +1469,25 @@ win32_version_string(struct gc_arena *gc, bool add_name) return (const char *)out.data; } +bool +send_msg_iservice(HANDLE pipe, const void *data, size_t size, + ack_message_t *ack, const char *context) +{ + struct gc_arena gc = gc_new(); + DWORD len; + bool ret = true; + + if (!WriteFile(pipe, data, size, &len, NULL) + || !ReadFile(pipe, ack, sizeof(*ack), &len, NULL)) + { + msg(M_WARN, "%s: could not talk to service: %s [%lu]", + context? context : "Unknown", + strerror_win32(GetLastError(), &gc), GetLastError()); + ret = false; + } + + gc_free(&gc); + return ret; +} + #endif /* ifdef _WIN32 */ diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h index 4b99a5e..b5cbe25 100644 --- a/src/openvpn/win32.h +++ b/src/openvpn/win32.h @@ -26,6 +26,7 @@ #define OPENVPN_WIN32_H #include "mtu.h" +#include "openvpn-msg.h" /* location of executables */ #define SYS_PATH_ENV_VAR_NAME "SystemRoot" /* environmental variable name that normally contains the system path */ @@ -307,5 +308,13 @@ int win32_version_info(void); */ const char *win32_version_string(struct gc_arena *gc, bool add_name); +/* + * Send the |size| bytes in buffer |data| to the interactive service |pipe| + * and read the result in |ack|. Returns false on communication error. + * The string in |context| is used to prefix error messages. + */ +bool send_msg_iservice(HANDLE pipe, const void *data, size_t size, + ack_message_t *ack, const char *context); + #endif /* ifndef OPENVPN_WIN32_H */ #endif /* ifdef _WIN32 */