Re: [Openvpn-devel] [PATCH] Detecting Windows version

2015-12-29 Thread Gert Doering 
Hi,

On Tue, Dec 29, 2015 at 06:03:04PM +0200, Lev Stipakov wrote:
> * Use adapter name instead of index on WinXP - sadly XP does not support 
> indexes
> * Write Windows version to log
> * Send it with peer-info as IV_PLAT_VER

Overall, I'm fine with the patch - thanks a lot.  

I've built release/2.3 + patch on ubuntu 14.04 and tested on XP SP3, and
it nicely worked - sending IV_PLAT_VER=5.1 ("WIN_XP"), and using interface
name again.

A few small details (half of them already discussed on IRC):

 - please send IV_PLAT_VER only if the client requested this (push-peer-info)
   (this is slightly more sensitive information, comparable to library
   versions, and when introducing library versions in IV_SSL, we decided
   to not send such information by default)

 - and please send a patch of the windows version bits for master ("all but
   the changes against route.c and tun.c" :) )

 - src/compat/compat-versionhelpers.h needs to be listed "somewhere"
   (src/compat/Makefile.am libcompat_la_SOURCES=... maybe?) so that
   "make dist" will include it into the generated .tar.gz

>  #elif defined (WIN32)
>  
> -  struct buffer out = alloc_buf_gc (64, );
> -  buf_printf (, "interface=%d", tt->adapter_index );
> -  device = buf_bptr();
> +  if (win32_version_info() != WIN_XP)
> +{
> + struct buffer out = alloc_buf_gc (64, );
> + buf_printf (, "interface=%d", tt->adapter_index );
> + device = buf_bptr();
> +}

the indentation on these is off - they are indented with a full 
but our funny 2.3 convention requires *six* spaces here...  (sorry for
being a pain... trying to be consistent)

Same thing for delete_route_ipv6().

Functionally, this all looks perfectly fine (I only tested on XP, but
as the rest is just existing code, it will "obviously" work for the
non-XP case).

[..]
> +int
> +win32_version_info()
> +{
> +if (!IsWindowsXPOrGreater())
> +{
> +msg (M_FATAL, "Error: Windows version must be XP or greater.");
> +}
> +
> +if (!IsWindowsVistaOrGreater())
> +{
> +return WIN_XP;
> +}

I'm not sure if this is the canonical best version to deal with it or
not, but I don't particularily *want* to get myself involved into this.

If someone else thinks this needs improvement (like, working Win10
detection :-) ) I'm all open to accept additional patches on top of it,
but to solve our immediate needs, this is good enough for me and does 
not *need* changes.

thanks, and waiting for v2 :-)

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

[Openvpn-devel] [PATCH] Detecting Windows version

2015-12-29 Thread Lev Stipakov 
* Use adapter name instead of index on WinXP - sadly XP does not support indexes
* Write Windows version to log
* Send it with peer-info as IV_PLAT_VER

Signed-off-by: Lev Stipakov 
---
 config-msvc.h  |  1 +
 configure.ac   |  1 +
 src/compat/compat-versionhelpers.h | 81 ++
 src/openvpn/openvpn.c  |  3 ++
 src/openvpn/options.c  | 12 ++
 src/openvpn/options.h  |  4 ++
 src/openvpn/route.c| 19 ++---
 src/openvpn/ssl.c  |  2 +
 src/openvpn/tun.c  |  2 +-
 src/openvpn/win32.c| 63 +
 src/openvpn/win32.h| 13 ++
 11 files changed, 194 insertions(+), 7 deletions(-)
 create mode 100644 src/compat/compat-versionhelpers.h

diff --git a/config-msvc.h b/config-msvc.h
index fa99384..ae43a5f 100644
--- a/config-msvc.h
+++ b/config-msvc.h
@@ -45,6 +45,7 @@
 #define HAVE_SYS_STAT_H 1
 #define HAVE_LZO_LZO1X_H 1
 #define HAVE_LZO_LZOUTIL_H 1
+#define HAVE_VERSIONHELPERS_H 1

 #define HAVE_ACCESS 1
 #define HAVE_CHDIR 1
diff --git a/configure.ac b/configure.ac
index 87d9116..773cded 100644
--- a/configure.ac
+++ b/configure.ac
@@ -423,6 +423,7 @@ AC_CHECK_HEADERS([ \
netinet/in.h netinet/in_systm.h \
netinet/tcp.h arpa/inet.h netdb.h \
windows.h winsock2.h ws2tcpip.h \
+   versionhelpers.h \
 ])
 AC_CHECK_HEADERS([ \
sys/time.h sys/ioctl.h sys/stat.h \
diff --git a/src/compat/compat-versionhelpers.h 
b/src/compat/compat-versionhelpers.h
new file mode 100644
index 000..f634091
--- /dev/null
+++ b/src/compat/compat-versionhelpers.h
@@ -0,0 +1,81 @@
+/**
+ * This file is part of the mingw-w64 runtime package.
+ * No warranty is given; refer to the file DISCLAIMER within this package.
+ */
+
+#ifndef _INC_VERSIONHELPERS
+#define _INC_VERSIONHELPERS
+
+#include 
+
+#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP) && !defined(__WIDL__)
+
+#ifdef __cplusplus
+#define VERSIONHELPERAPI inline bool
+#else
+#define VERSIONHELPERAPI FORCEINLINE BOOL
+#endif
+
+#define _WIN32_WINNT_WINBLUE0x0603
+
+VERSIONHELPERAPI IsWindowsVersionOrGreater(WORD major, WORD minor, WORD 
servpack)
+{
+OSVERSIONINFOEXW vi = {sizeof(vi),major,minor,0,0,{0},servpack};
+return VerifyVersionInfoW(, 
VER_MAJORVERSION|VER_MINORVERSION|VER_SERVICEPACKMAJOR,
+VerSetConditionMask(VerSetConditionMask(VerSetConditionMask(0,
+VER_MAJORVERSION,VER_GREATER_EQUAL),
+VER_MINORVERSION,VER_GREATER_EQUAL),
+VER_SERVICEPACKMAJOR, VER_GREATER_EQUAL));
+}
+
+VERSIONHELPERAPI IsWindowsXPOrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINXP), 
LOBYTE(_WIN32_WINNT_WINXP), 0);
+}
+
+VERSIONHELPERAPI IsWindowsXPSP1OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINXP), 
LOBYTE(_WIN32_WINNT_WINXP), 1);
+}
+
+VERSIONHELPERAPI IsWindowsXPSP2OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINXP), 
LOBYTE(_WIN32_WINNT_WINXP), 2);
+}
+
+VERSIONHELPERAPI IsWindowsXPSP3OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINXP), 
LOBYTE(_WIN32_WINNT_WINXP), 3);
+}
+
+VERSIONHELPERAPI IsWindowsVistaOrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_VISTA), 
LOBYTE(_WIN32_WINNT_VISTA), 0);
+}
+
+VERSIONHELPERAPI IsWindowsVistaSP1OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_VISTA), 
LOBYTE(_WIN32_WINNT_VISTA), 1);
+}
+
+VERSIONHELPERAPI IsWindowsVistaSP2OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_VISTA), 
LOBYTE(_WIN32_WINNT_VISTA), 2);
+}
+
+VERSIONHELPERAPI IsWindows7OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WIN7), 
LOBYTE(_WIN32_WINNT_WIN7), 0);
+}
+
+VERSIONHELPERAPI IsWindows7SP1OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WIN7), 
LOBYTE(_WIN32_WINNT_WIN7), 1);
+}
+
+VERSIONHELPERAPI IsWindows8OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WIN8), 
LOBYTE(_WIN32_WINNT_WIN8), 0);
+}
+
+VERSIONHELPERAPI IsWindows8Point1OrGreater(void) {
+return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINBLUE), 
LOBYTE(_WIN32_WINNT_WINBLUE), 0);
+}
+
+VERSIONHELPERAPI IsWindowsServer(void) {
+OSVERSIONINFOEXW vi = {sizeof(vi),0,0,0,0,{0},0,0,0,VER_NT_WORKSTATION};
+return !VerifyVersionInfoW(, VER_PRODUCT_TYPE, VerSetConditionMask(0, 
VER_PRODUCT_TYPE, VER_EQUAL));
+}
+
+#endif
+#endif
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index 32e326e..823c3dd 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -220,6 +220,9 @@ openvpn_main (int argc, char *argv[])

  /* print version number */
  msg (M_INFO, "%s", title_string);
+#ifdef WIN32
+ show_windows_version(M_INFO);
+#endif