diff mbox series

[Openvpn-devel] Improve Windows version detection with manifest

Message ID 20200724141445.302-1-lstipakov@gmail.com
State Superseded
Headers
Series [Openvpn-devel] Improve Windows version detection with manifest |

Commit Message

Lev Stipakov July 24, 2020, 4:14 a.m. UTC 
  From: Lev Stipakov <lev@openvpn.net>

Add manifest file to detect Windows versions greater than Windows 8.

Below is an example output on Windows 10.

Before:
        Windows version 6.2 (Windows 8 or greater) 64bit

After:
        Windows version 10.0 (Windows 10 or greater) 64bit

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/compat/compat-versionhelpers.h     |  6 +++++
 src/openvpn/openvpn.manifest           | 33 ++++++++++++++++++++++++++
 src/openvpn/openvpn.vcxproj            | 15 ++++++++++++
 src/openvpn/openvpn.vcxproj.filters    |  5 ++++
 src/openvpn/openvpn_win32_resources.rc |  2 ++
 src/openvpn/win32.c                    | 20 ++++++++++++++--
 src/openvpn/win32.h                    |  8 ++++---
 7 files changed, 84 insertions(+), 5 deletions(-)
 create mode 100644 src/openvpn/openvpn.manifest

Comments

Gert Doering July 24, 2020, 8:40 a.m. UTC | #1 
Hi,

On Fri, Jul 24, 2020 at 05:14:45PM +0300, Lev Stipakov wrote:
> From: Lev Stipakov <lev@openvpn.net>
> 
> Add manifest file to detect Windows versions greater than Windows 8.

NAK, this breaks Ubuntu/MinGW builds.  Actually, I think it breaks all 
windows builds, if you build from tarballs (which the "generic" build does -
first, make dist, then, build from there).

You introduce an include to "openvpn.manifest", but this is not bundled,
so

i686-w64-mingw32-windres -DHAVE_CONFIG_H -I. -I../.. -I../../include  -I../../include -I../../src/compat -DWIN32_LEAN_AND_MEAN -DNTDDI_VERSION=NTDDI_VISTA -D_WIN32_WINNT=_WIN32_WINNT_VISTA -i "openvpn_win32_resources.rc" -o "openvpn_win32_resources.o"
i686-w64-mingw32-windres: can't open file `openvpn.manifest': No such file or directory
Makefile:933: recipe for target 'openvpn_win32_resources.o' failed
make[4]: *** [openvpn_win32_resources.o] Error 1
make[4]: *** Waiting for unfinished jobs....

And this is what breaks MinGW builds:

../../src/compat/compat-versionhelpers.h: In function 'IsWindows10OrGreater':
../../src/compat/compat-versionhelpers.h:101:45: error: '_WIN32_WINNT_WINTHRESHOLD' undeclared (first use in this function); did you mean '_WIN32_WINNT_WINBLUE'?
     return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINTHRESHOLD), LOBYTE(_WIN32_WINNT_WINTHRESHOLD), 0);
                                             ^
../../src/compat/compat-versionhelpers.h:101:45: note: each undeclared identifier is reported only once for each function it appears in


possibly the _WIN32_WINNT_WINTHRESHOLD is missing on MinGW, at least the
version bundled with Ubuntu 18.04 (I've skimmed the headers, and it's
missing).


The rest of the code changes look good, and I have no idea what all the
XML files do - but those are relevant for MSVC, and you are the expert
on that ("if you say this is beneficial, I'll merge").

gert
Lev Stipakov July 24, 2020, 9:34 a.m. UTC | #2 
Hm.. I tested mingw build (windows-nsis) on Ubuntu 19.04 and
everything worked fine.

That's because:

1) Ubuntu 19.04 probably has recent enough mingw with _WIN32_WINNT_WINTHRESHOLD

2) When I tested mingw build, I pointed source url to git repo, not to
tarball - that's why it worked.

That XML is required (no matter msvc or mingw) to tell Windows that
your app is Win10-aware and allows you
to use API which detects Windows >= 8.1

I'll send V2 with above issues fixed.

pe 24. heinäk. 2020 klo 21.40 Gert Doering (gert@greenie.muc.de) kirjoitti:
>
> Hi,
>
> On Fri, Jul 24, 2020 at 05:14:45PM +0300, Lev Stipakov wrote:
> > From: Lev Stipakov <lev@openvpn.net>
> >
> > Add manifest file to detect Windows versions greater than Windows 8.
>
> NAK, this breaks Ubuntu/MinGW builds.  Actually, I think it breaks all
> windows builds, if you build from tarballs (which the "generic" build does -
> first, make dist, then, build from there).
>
> You introduce an include to "openvpn.manifest", but this is not bundled,
> so
>
> i686-w64-mingw32-windres -DHAVE_CONFIG_H -I. -I../.. -I../../include  -I../../include -I../../src/compat -DWIN32_LEAN_AND_MEAN -DNTDDI_VERSION=NTDDI_VISTA -D_WIN32_WINNT=_WIN32_WINNT_VISTA -i "openvpn_win32_resources.rc" -o "openvpn_win32_resources.o"
> i686-w64-mingw32-windres: can't open file `openvpn.manifest': No such file or directory
> Makefile:933: recipe for target 'openvpn_win32_resources.o' failed
> make[4]: *** [openvpn_win32_resources.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
>
> And this is what breaks MinGW builds:
>
> ../../src/compat/compat-versionhelpers.h: In function 'IsWindows10OrGreater':
> ../../src/compat/compat-versionhelpers.h:101:45: error: '_WIN32_WINNT_WINTHRESHOLD' undeclared (first use in this function); did you mean '_WIN32_WINNT_WINBLUE'?
>      return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINTHRESHOLD), LOBYTE(_WIN32_WINNT_WINTHRESHOLD), 0);
>                                              ^
> ../../src/compat/compat-versionhelpers.h:101:45: note: each undeclared identifier is reported only once for each function it appears in
>
>
> possibly the _WIN32_WINNT_WINTHRESHOLD is missing on MinGW, at least the
> version bundled with Ubuntu 18.04 (I've skimmed the headers, and it's
> missing).
>
>
> The rest of the code changes look good, and I have no idea what all the
> XML files do - but those are relevant for MSVC, and you are the expert
> on that ("if you say this is beneficial, I'll merge").
>
> gert
>
> --
> "If was one thing all people took for granted, was conviction that if you
>  feed honest figures into a computer, honest figures come out. Never doubted
>  it myself till I met a computer with a sense of humor."
>                              Robert A. Heinlein, The Moon is a Harsh Mistress
>
> Gert Doering - Munich, Germany                             gert@greenie.muc.de
Gert Doering July 24, 2020, 9:40 a.m. UTC | #3 
Hi,

On Fri, Jul 24, 2020 at 10:34:17PM +0300, Lev Stipakov wrote:
> 2) When I tested mingw build, I pointed source url to git repo, not to
> tarball - that's why it worked.

How do you build?  I use the "build-snapshot" script, and that does
a "make dist" somewhere in between.

> That XML is required (no matter msvc or mingw) to tell Windows that
> your app is Win10-aware and allows you
> to use API which detects Windows >= 8.1

Thanks for explaining.

> I'll send V2 with above issues fixed.

Thanks :)

gert
Lev Stipakov July 24, 2020, 9:49 a.m. UTC | #4 
> How do you build?  I use the "build-snapshot" script, and that does
> a "make dist" somewhere in between.

ubuntu@ip-172-31-38-110:~/openvpn-build/windows-nsis$
OPENVPN_VERSION=fix-remove-impersonation
OPENVPN_URL=https://github.com/lstipakov/openvpn/archive/fix/remove-impersonation.zip
./build-complete --use-depcache
Gert Doering July 24, 2020, 10:23 a.m. UTC | #5 
Hi,

On Fri, Jul 24, 2020 at 10:49:07PM +0300, Lev Stipakov wrote:
> > How do you build?  I use the "build-snapshot" script, and that does
> > a "make dist" somewhere in between.
> 
> ubuntu@ip-172-31-38-110:~/openvpn-build/windows-nsis$
> OPENVPN_VERSION=fix-remove-impersonation
> OPENVPN_URL=https://github.com/lstipakov/openvpn/archive/fix/remove-impersonation.zip
> ./build-complete --use-depcache

Ah, you point it at (effectively) a ready-made tarball.

I use "build-snapshot" which fetches a git repo + branch

Maybe build-complete does the same if "OPENVPN_URL" points to a
git repo... dunno.  I have

OPENVPN_URL="git://github.greenie.net/openvpn-236.git"
OPENVPN_VERSION="${OPENVPN_VERSION:-2.5_git}"; export OPENVPN_VERSION
OPENVPN_BRANCH="${OPENVPN_BRANCH:-master}"

gert
diff mbox series

Patch

diff --git a/src/compat/compat-versionhelpers.h b/src/compat/compat-versionhelpers.h
index 251fb047..c8511d0f 100644
--- a/src/compat/compat-versionhelpers.h
+++ b/src/compat/compat-versionhelpers.h
@@ -95,6 +95,12 @@  IsWindows8Point1OrGreater(void)
     return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINBLUE), LOBYTE(_WIN32_WINNT_WINBLUE), 0);
 }
 
+VERSIONHELPERAPI
+IsWindows10OrGreater()
+{
+    return IsWindowsVersionOrGreater(HIBYTE(_WIN32_WINNT_WINTHRESHOLD), LOBYTE(_WIN32_WINNT_WINTHRESHOLD), 0);
+}
+
 VERSIONHELPERAPI
 IsWindowsServer(void)
 {
diff --git a/src/openvpn/openvpn.manifest b/src/openvpn/openvpn.manifest
new file mode 100644
index 00000000..fa5b3d7f
--- /dev/null
+++ b/src/openvpn/openvpn.manifest
@@ -0,0 +1,33 @@ 
+<?xml version="1.0" encoding="utf-8" standalone="yes"?>
+<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1" xmlns:asmv3="urn:schemas-microsoft-com:asm.v3">
+    <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
+        <application>
+            <!-- Windows 10 -->
+            <supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
+            <!-- Windows 8.1 -->
+            <supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
+            <!-- Windows 8 -->
+            <supportedOS Id="{4a2f28e3-53b9-4441-ba9c-d69d4a4a6e38}"/>
+            <!-- Windows 7 -->
+            <supportedOS Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}"/>
+            <!-- Windows Vista -->
+            <supportedOS Id="{e2011457-1546-43c5-a5fe-008deee3d3f0}"/>
+        </application>
+    </compatibility>
+    <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
+        <security>
+            <requestedPrivileges>
+                <!--
+                  UAC settings:
+                  - app should run at same integrity level as calling process
+                  - app does not need to manipulate windows belonging to
+                    higher-integrity-level processes
+                  -->
+                <requestedExecutionLevel
+                    level="asInvoker"
+                    uiAccess="false"
+                />
+            </requestedPrivileges>
+        </security>
+    </trustInfo>
+</assembly>
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index bd1a5844..5367979d 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -70,6 +70,18 @@ 
   <PropertyGroup>
     <_ProjectFileVersion>10.0.30319.1</_ProjectFileVersion>
   </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">
+    <GenerateManifest>false</GenerateManifest>
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
+    <GenerateManifest>false</GenerateManifest>
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">
+    <GenerateManifest>false</GenerateManifest>
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">
+    <GenerateManifest>false</GenerateManifest>
+  </PropertyGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">
     <ClCompile>
       <AdditionalIncludeDirectories>..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
@@ -311,6 +323,9 @@ 
       <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
     </ProjectReference>
   </ItemGroup>
+  <ItemGroup>
+    <Manifest Include="openvpn.manifest" />
+  </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
diff --git a/src/openvpn/openvpn.vcxproj.filters b/src/openvpn/openvpn.vcxproj.filters
index e0bc7d5e..cf5748c7 100644
--- a/src/openvpn/openvpn.vcxproj.filters
+++ b/src/openvpn/openvpn.vcxproj.filters
@@ -515,4 +515,9 @@ 
       <Filter>Resource Files</Filter>
     </ResourceCompile>
   </ItemGroup>
+  <ItemGroup>
+    <Manifest Include="openvpn.manifest">
+      <Filter>Resource Files</Filter>
+    </Manifest>
+  </ItemGroup>
 </Project>
\ No newline at end of file
diff --git a/src/openvpn/openvpn_win32_resources.rc b/src/openvpn/openvpn_win32_resources.rc
index e4f1ee95..1ea5f878 100644
--- a/src/openvpn/openvpn_win32_resources.rc
+++ b/src/openvpn/openvpn_win32_resources.rc
@@ -7,6 +7,8 @@ 
 
 #pragma code_page(65001) /* UTF8 */
 
+1 RT_MANIFEST "openvpn.manifest"
+
 LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
 
 VS_VERSION_INFO VERSIONINFO
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index eb4c0307..7e913165 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1417,10 +1417,18 @@  win32_version_info(void)
     {
         return WIN_7;
     }
-    else
+
+    if (!IsWindows8Point1OrGreater())
     {
         return WIN_8;
     }
+
+    if (!IsWindows10OrGreater())
+    {
+        return WIN_8_1;
+    }
+
+    return WIN_10;
 }
 
 bool
@@ -1458,7 +1466,15 @@  win32_version_string(struct gc_arena *gc, bool add_name)
             break;
 
         case WIN_8:
-            buf_printf(&out, "6.2%s", add_name ? " (Windows 8 or greater)" : "");
+            buf_printf(&out, "6.2%s", add_name ? " (Windows 8)" : "");
+            break;
+
+        case WIN_8_1:
+            buf_printf(&out, "6.3%s", add_name ? " (Windows 8.1)" : "");
+            break;
+
+        case WIN_10:
+            buf_printf(&out, "10.0%s", add_name ? " (Windows 10 or greater)" : "");
             break;
 
         default:
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index 79504776..da85ed4d 100644
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -298,10 +298,12 @@  bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);
 
 bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);
 
-#define WIN_XP 0
+#define WIN_XP    0
 #define WIN_VISTA 1
-#define WIN_7 2
-#define WIN_8 3
+#define WIN_7     2
+#define WIN_8     3
+#define WIN_8_1   4
+#define WIN_10    5
 
 int win32_version_info(void);