Message ID | 20181016102627.18676-4-simon@rozman.si |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel,1/5] Set output name to libopenvpnmsica.dll in MSVC builds too | expand |
Hi Simon,
This is painful to read, and I bet it was even more painful to write. I am
sorry it came to this, though it does look like it should(TM) work.
From what I gather, the OS version should be checked in the
CustomAction's MSI execution conditions instead of in the CustomAction
itself. The appropriate information can then be passed in based on that:
https://blogs.msdn.microsoft.com/cjacks/2009/05/06/why-custom-actions-get-a-windows-vista-version-lie-on-windows-7/
It might be somewhat more convenient to add the PID to the debug
MessageBox call, but it is probably MUCH more convenient to use the
CustomAction debugging facility built into the MSI service itself:
https://docs.microsoft.com/en-us/windows/desktop/Msi/debugging-custom-actions
Thanks,
Jon
-----Original Message-----
From: Simon Rozman <simon@rozman.si>
Sent: Tuesday, October 16, 2018 3:26 AM
To: openvpn-devel@lists.sourceforge.net
Subject: [Openvpn-devel] [PATCH 4/5] Add MSI custom action for reliable Windows 10 detection
This patch introduces a `FindSystemInfo()` MSI custom action to reliably
detect Windows 10. The MSI built-in properties for Windows version
detection depend on bootstrapper's manifest. We could provide our own
Windows 10 compatible EXE bootstrapper, but that would cover the
Windows 10 detection in the `InstallUISequence` only. The
`InstallExecuteSequence` is launched by msiexec.exe which we cannot
tamper with would still report `VersionNT` as Windows 8 (603).
---
src/openvpnmsica/Makefile.am | 2 +-
src/openvpnmsica/openvpnmsica.c | 124 ++++++++++++++++++++++++++++++--
src/openvpnmsica/openvpnmsica.h | 15 ++++
3 files changed, 136 insertions(+), 5 deletions(-)
diff --git a/src/openvpnmsica/Makefile.am b/src/openvpnmsica/Makefile.am
index d46170b4..ecca74bc 100644
--- a/src/openvpnmsica/Makefile.am
+++ b/src/openvpnmsica/Makefile.am
@@ -41,7 +41,7 @@ libopenvpnmsica_la_CFLAGS = \
-municode -D_UNICODE \
-UNTDDI_VERSION -U_WIN32_WINNT \
-D_WIN32_WINNT=_WIN32_WINNT_VISTA
-libopenvpnmsica_la_LDFLAGS = -ladvapi32 -lole32 -lmsi -lsetupapi -lshlwapi -no-undefined -avoid-version
+libopenvpnmsica_la_LDFLAGS = -ladvapi32 -lole32 -lmsi -lsetupapi -lshlwapi -lversion -no-undefined -avoid-version
endif
libopenvpnmsica_la_SOURCES = \
diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 3b90ce05..d1642d6a 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -36,13 +36,15 @@
#include <memory.h>
#include <msiquery.h>
#include <shlwapi.h>
-#ifdef _MSC_VER
-#pragma comment(lib, "shlwapi.lib")
-#endif
#include <stdbool.h>
#include <stdlib.h>
#include <tchar.h>
+#ifdef _MSC_VER
+#pragma comment(lib, "shlwapi.lib")
+#pragma comment(lib, "version.lib")
+#endif
+
/**
* Local constants
@@ -119,7 +121,7 @@ openvpnmsica_setup_sequence_filename(
{
size_t len_action_name_z = _tcslen(openvpnmsica_cleanup_action_seqs[i].szName) + 1;
TCHAR *szPropertyEx = (TCHAR*)malloc((len_property_name + len_action_name_z) * sizeof(TCHAR));
- memcpy(szPropertyEx , szProperty , len_property_name * sizeof(TCHAR));
+ memcpy(szPropertyEx , szProperty , len_property_name * sizeof(TCHAR));
memcpy(szPropertyEx + len_property_name, openvpnmsica_cleanup_action_seqs[i].szName, len_action_name_z * sizeof(TCHAR));
_stprintf_s(
szFilenameEx, _countof(szFilenameEx),
@@ -142,6 +144,120 @@ openvpnmsica_setup_sequence_filename(
}
+UINT __stdcall
+FindSystemInfo(_In_ MSIHANDLE hInstall)
+{
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
+#ifdef _DEBUG
+ MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v") TEXT(PACKAGE_VERSION), MB_OK);
+#endif
+
+ UINT uiResult;
+ BOOL bIsCoInitialized = SUCCEEDED(CoInitialize(NULL));
+
+ /* Set MSI session handle in TLS. */
+ struct openvpnmsica_tls_data *s = (struct openvpnmsica_tls_data *)TlsGetValue(openvpnmsica_tlsidx_session);
+ s->hInstall = hInstall;
+
+ // Get Windows version.
+ OSVERSIONINFOEX ver_info = { .dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX) };
+ if (!GetVersionEx((LPOSVERSIONINFO)&ver_info)) {
+ uiResult = GetLastError();
+ msg(M_NONFATAL | M_ERRNO, "%s: GetVersionEx() failed", __FUNCTION__);
+ goto cleanup_CoInitialize;
+ }
+
+ // The Windows version is usually spoofed, check using RtlGetVersion().
+ TCHAR szDllPath[0x1000];
+ ExpandEnvironmentStrings(TEXT("%SystemRoot%\\System32\\ntdll.dll"), szDllPath,
+#ifdef UNICODE
+ _countof(szDllPath)
+#else
+ _countof(szDllPath) - 1
+#endif
+ );
+ HMODULE hNtDllModule = LoadLibrary(szDllPath);
+ if (hNtDllModule)
+ {
+ typedef NTSTATUS (WINAPI* fnRtlGetVersion)(PRTL_OSVERSIONINFOW);
+ fnRtlGetVersion RtlGetVersion = (fnRtlGetVersion)GetProcAddress(hNtDllModule, "RtlGetVersion");
+ if (RtlGetVersion)
+ {
+ RTL_OSVERSIONINFOW rtl_ver_info = { .dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOW) };
+ if (RtlGetVersion(&rtl_ver_info) == 0)
+ if (
+ rtl_ver_info.dwMajorVersion > ver_info.dwMajorVersion ||
+ rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && rtl_ver_info.dwMinorVersion > ver_info.dwMinorVersion ||
+ rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && rtl_ver_info.dwMinorVersion == ver_info.dwMinorVersion && rtl_ver_info.dwBuildNumber > ver_info.dwBuildNumber)
+ {
+ // We got RtlGetVersion() and it reported newer version than GetVersionEx().
+ ver_info.dwMajorVersion = rtl_ver_info.dwMajorVersion;
+ ver_info.dwMinorVersion = rtl_ver_info.dwMinorVersion;
+ ver_info.dwBuildNumber = rtl_ver_info.dwBuildNumber;
+ ver_info.dwPlatformId = rtl_ver_info.dwPlatformId;
+ }
+ }
+
+ FreeLibrary(hNtDllModule);
+ }
+
+ // We don't trust RtlGetVersion() either. Check the version resource of kernel32.dll.
+ ExpandEnvironmentStrings(TEXT("%SystemRoot%\\System32\\kernel32.dll"), szDllPath,
+#ifdef UNICODE
+ _countof(szDllPath)
+#else
+ _countof(szDllPath)-1
+#endif
+ );
+
+ DWORD dwHandle;
+ DWORD dwVerInfoSize = GetFileVersionInfoSize(szDllPath, &dwHandle);
+ if (dwVerInfoSize)
+ {
+ LPVOID pVersionInfo = malloc(dwVerInfoSize);
+ if (pVersionInfo)
+ {
+ // Read version info.
+ if (GetFileVersionInfo(szDllPath, dwHandle, dwVerInfoSize, pVersionInfo))
+ {
+ // Get the value for the root block.
+ UINT uiSize = 0;
+ VS_FIXEDFILEINFO *pVSFixedFileInfo = NULL;
+ if (VerQueryValue(pVersionInfo, TEXT("\\"), &pVSFixedFileInfo, &uiSize) && uiSize && pVSFixedFileInfo)
+ if (HIWORD(pVSFixedFileInfo->dwProductVersionMS) > ver_info.dwMajorVersion ||
+ HIWORD(pVSFixedFileInfo->dwProductVersionMS) == ver_info.dwMajorVersion && LOWORD(pVSFixedFileInfo->dwProductVersionMS) > ver_info.dwMinorVersion ||
+ HIWORD(pVSFixedFileInfo->dwProductVersionMS) == ver_info.dwMajorVersion && LOWORD(pVSFixedFileInfo->dwProductVersionMS) == ver_info.dwMinorVersion && HIWORD(pVSFixedFileInfo->dwProductVersionLS) > ver_info.dwBuildNumber)
+ {
+ // We got kernel32.dll version and it is newer.
+ ver_info.dwMajorVersion = HIWORD(pVSFixedFileInfo->dwProductVersionMS);
+ ver_info.dwMinorVersion = LOWORD(pVSFixedFileInfo->dwProductVersionMS);
+ ver_info.dwBuildNumber = HIWORD(pVSFixedFileInfo->dwProductVersionLS);
+ }
+ }
+
+ free(pVersionInfo);
+ }
+ }
+
+ uiResult = MsiSetProperty(hInstall, TEXT("DriverCertification"), ver_info.dwMajorVersion >= 10 ? ver_info.wProductType > VER_NT_WORKSTATION ? TEXT("whql") : TEXT("attsgn") : TEXT(""));
+ if (uiResult != ERROR_SUCCESS)
+ {
+ SetLastError(uiResult); /* MSDN does not mention MsiSetProperty() to set GetLastError(). But we do have an error code. Set last error manually. */
+ msg(M_NONFATAL | M_ERRNO, "%s: MsiSetProperty(\"TAPINTERFACES\") failed", __FUNCTION__);
+ goto cleanup_CoInitialize;
+ }
+
+ uiResult = ERROR_SUCCESS;
+
+cleanup_CoInitialize:
+ if (bIsCoInitialized) CoUninitialize();
+ return uiResult;
+}
+
+
UINT __stdcall
FindTAPInterfaces(_In_ MSIHANDLE hInstall)
{
diff --git a/src/openvpnmsica/openvpnmsica.h b/src/openvpnmsica/openvpnmsica.h
index bb8e28ec..da145062 100644
--- a/src/openvpnmsica/openvpnmsica.h
+++ b/src/openvpnmsica/openvpnmsica.h
@@ -63,6 +63,21 @@ extern "C" {
#endif
+/**
+ * Determines Windows information:
+ * - Sets `DriverCertification` MSI property to "", "attsgn" or "whql"
+ * according to the driver certification required by the running version of
+ * Windows.
+ *
+ * @param hInstall Handle to the installation provided to the DLL custom action
+ *
+ * @return ERROR_SUCCESS on success; An error code otherwise
+ * See: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmsdn.microsoft.com%2Fen-us%2Flibrary%2Fwindows%2Fdesktop%2Faa368072.aspx&data=02%7C01%7Cjkunkee%40microsoft.com%7Cd520a31814634dced1dc08d63351eb58%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636752824275192379&sdata=2DLeMsL5JHIFmh07Fh6zqZTGt117xy%2Bu2TY3acNpaH8%3D&reserved=0
+ */
+DLLEXP_DECL UINT __stdcall
+FindSystemInfo(_In_ MSIHANDLE hInstall);
+
+
/**
* Find existing TAP interfaces and set TAPINTERFACES property with semicolon delimited list
* of installed TAP interface GUIDs.
Hi, > This is painful to read, and I bet it was even more painful to write. I > am sorry it came to this, though it does look like it should(TM) work. I don't blame Microsoft for this mess. Version lies actually solve a lot of problems with legacy software. > From what I gather, the OS version should be checked in the > CustomAction's MSI execution conditions instead of in the CustomAction > itself. The appropriate information can then be passed in based on that: > > https://blogs.msdn.microsoft.com/cjacks/2009/05/06/why-custom-actions- > get-a-windows-vista-version-lie-on-windows-7/ The post on this link is true for Windows 7 (May 6, 2009). It's 2018 now and MSI's property VersionNT got stuck on Windows 8 (602 if I recall correctly). Even when run on Windows 10. Each time Microsoft ceils the version mechanism, they should have introduced a new one: e.g. Version9X >> VersionNT >> Version81. This would keep the legacy MSI packages that are using VersionNT think they're on Windows 8, and allow the new packages to test for Windows 10 in a clean way. It would have been a lot easier to just have an MSI condition (e.g. install WHQL flavour of driver if Version81 AND Version81>=1000 AND MsiNTProductType>=2). I am open to suggestions, how to make a "regular/attestation signed/WHQL" selection logic in the MSI. MSI packages have up to three driver flavours packed and only one must be installed: - x86 MSI has regular+attestation signed - x64 has regular+attestation+WHQL - ARM64 will have attestation signed only > It might be somewhat more convenient to add the PID to the debug > MessageBox call, but it is probably MUCH more convenient to use the > CustomAction debugging facility built into the MSI service itself: > > https://docs.microsoft.com/en-us/windows/desktop/Msi/debugging-custom- > actions Many thanks for this. I will test the MsiBreak method shortly and remove the MessageBox calls completely for a cleaner code then. Regards, Simon
>> From what I gather, the OS version should be checked in the >> CustomAction's MSI execution conditions instead of in the CustomAction >> itself. The appropriate information can then be passed in based on that: >> >> https://blogs.msdn.microsoft.com/cjacks/2009/05/06/why-custom-actions- >> get-a-windows-vista-version-lie-on-windows-7/ > The post on this link is true for Windows 7 (May 6, 2009). It's 2018 now and > MSI's property VersionNT got stuck on Windows 8 (602 if I recall correctly). Quite right. I just talked to someone familiar with this here, and, as I understand it, MSI will never offer a way to do this Right on Windows 10. I'm not sure what the general messaging is from Microsoft around MSI, but my personal perception is that it is not moving forward anymore. > I am open to suggestions, how to make a "regular/attestation signed/WHQL" > selection logic in the MSI. MSI packages have up to three driver flavours > packed and only one must be installed: > - x86 MSI has regular+attestation signed > - x64 has regular+attestation+WHQL > - ARM64 will have attestation signed only Fortunately, I *do* have a suggestion here...I think. First, though, let's make sure I'm thinking straight: In your new system, there is an EXE that runs, detects the architecture, unpacks the right MSI, and runs the right MSI, right? *If* I have that right, the EXE could be manifested as Windows-10-aware: Manifest contents: https://docs.microsoft.com/en-us/windows/desktop/SysInfo/targeting-your-application-at-windows-8-1 VS workflow: https://docs.microsoft.com/en-us/cpp/build/how-to-embed-a-manifest-inside-a-c-cpp-application Basically, you write an XML file and pack it in the EXE as a resource. Visual Studio will do this for you if the file is in your VS project source file list. I'm not sure how you'd do this with mingw_w64, but you'd probably invoke the equivalent of mt.exe: https://stackoverflow.com/questions/1423492/how-do-i-add-a-manifest-to-an-executable-using-mt-exe The parent installer could then call GetVersionEx without being lied to and pass it in to your CustomAction DLL through msiexec: https://www.codeproject.com/articles/16767/how-to-pass-command-line-arguments-to-msi-installe Let me know if that doesn't make sense or won't work for what you're doing. Thanks, Jon
Hi, > > The post on this link is true for Windows 7 (May 6, 2009). It's 2018 > > now and MSI's property VersionNT got stuck on Windows 8 (602 if I > recall correctly). > > Quite right. I just talked to someone familiar with this here, and, as I > understand it, MSI will never offer a way to do this Right on Windows > 10. I'm not sure what the general messaging is from Microsoft around > MSI, but my personal perception is that it is not moving forward > anymore. :( I agree it's an art to make an MSI, but being a known and documented standard, supporting commit/rollback, allowing sys-admins endless customizations of packages... (If only Microsoft offered more stock actions.) Well, I guess Microsoft is coming up with some awesome replacement in due time. :) Note a side take-home message: msiexec.exe will never be Windows 10 aware. > Fortunately, I *do* have a suggestion here...I think. > > First, though, let's make sure I'm thinking straight: In your new > system, there is an EXE that runs, detects the architecture, unpacks the > right MSI, and runs the right MSI, right? > > *If* I have that right, the EXE could be manifested as Windows-10-aware: > > Manifest contents: > https://docs.microsoft.com/en-us/windows/desktop/SysInfo/targeting-your- > application-at-windows-8-1 > VS workflow: > https://docs.microsoft.com/en-us/cpp/build/how-to-embed-a-manifest- > inside-a-c-cpp-application > > Basically, you write an XML file and pack it in the EXE as a resource. > Visual Studio will do this for you if the file is in your VS project > source file list. I'm not sure how you'd do this with mingw_w64, but > you'd probably invoke the equivalent of mt.exe: > > https://stackoverflow.com/questions/1423492/how-do-i-add-a-manifest-to- > an-executable-using-mt-exe > > The parent installer could then call GetVersionEx without being lied to > and pass it in to your CustomAction DLL through msiexec: > > https://www.codeproject.com/articles/16767/how-to-pass-command-line- > arguments-to-msi-installe > > Let me know if that doesn't make sense or won't work for what you're > doing. I am familiar with EXE and manifests, thanks. Nevertheless, I really appreciate your time to extensively research and provide useful references in the debate. In my professional career I make EXE (or WSH) "parent installers" only because end-users usually have no clue what platform is their Windows. You can't do one-MSI-fits-all-platforms for native apps, so the parent installer EXE takes platform choice away from users. All other decision logic is inside MSI packages making them fully autonomous - once you pick the right one for your platform of course. Even if we do make a Windows 10 aware "parent installer" to inject DriverCertification property to the MSI, we get complications in one considerable use case... <SideNote> Actually, I wouldn't be on this boat at all if I wasn't interested into Group Policy deployment of OpenVPN professionally. I work for a SOHO-sized company with zero budget to run anything more than Group Policy MSI deploys, and I am totally fed with running from one employee to another to assist them with keeping OpenVPN up-to-date, since OpenVPN only has an EXE installer for the time being. Therefore, I was thinking of repackaging OpenVPN into MSI for my own needs, but later learned that OpenVPN community is interested in it too. </SideNote> When the MSI package is installed by Group Policy Client, the MSI is launched directly by msiexec.exe. There is no parent installer EXE in between. Therefore, the MSI command line parameter which TAP driver to install would need to be authored by sys-admins. This would make Group Policy deployment of OpenVPN more complicated on sys-admins as it should be. Hence my design choice to select the TAP driver within MSI itself. I would have used the VersionNT property, but it says "I am Windows 8" on Windows 10. I would have used a custom action and MSDN recommended Win32 API for Windows version detection, but when launched by Group Policy Client the parent process is msiexec.exe (which doesn't manifest as Windows 10 aware, remember take-home message above) thus the API says "I am Windows 8" on Windows 10 again. Hence such a complex workaround to detect the MSI has been launched on Windows 10. Microsoft made me do it. ;) Please, note one last thing... I totally agree with everybody saying "you shall not test for OS version, but on feature presence instead". That's why this custom action *does not* return the Windows version - not to tempt anybody else to use it for OS version detection. The FindSystemInfo custom action's only output is the "DriverCertification" property that is set to "", "attsgn", or "whql". The name "FindSystemInfo" was picked generic on purpose, to allow us to add other detections missing in MSI should we need any down the road. After all this, I realized I mistitled the patch. It should have read "Add MSI custom action for reliable driver flavour selection". Best regards, Simon
Hi, > > It might be somewhat more convenient to add the PID to the debug > > MessageBox call, but it is probably MUCH more convenient to use the > > CustomAction debugging facility built into the MSI service itself: > > > > https://docs.microsoft.com/en-us/windows/desktop/Msi/debugging-custom- > > actions > > Many thanks for this. I will test the MsiBreak method shortly and remove > the MessageBox calls completely for a cleaner code then. I gave the MsiBreak method a quick try, but I couldn't make it work. I have set MsiBreak as a system environment variable, restarted Windows Installer service, restarted the shell window from where I invoke msiexec calls to make sure the environment is updated. But it doesn't pop-up any prompt to attach debugger as advertised. I haven't installed the Debugging Tools for Windows, as I run Visual Studio elevated to debug processes. I haven't restarted my computer: restart of a working machine with stage set to debug something is a royal PITA: the libopenvpnmsica.dll has more than one custom property to debug, restarting each time to switch MsiBreak to a different custom action is not viable. Therefore, I'd prefer to keep own MessageBox() call in the beginning of each custom action. It works 100%. Adding PID to the message is an option. I personally never needed it. When running Visual Studio elevated, you press Ctrl+Alt+P, followed by "msi" keystrokes to focus on "msiexec.exe" processes, then observe the MessageBox()'s title (which is deliberately set to function name) in the "Title" column of the available process list. Way faster than searching process by PID. Anyway, I have extended the debug pop-up dialogs to be more informative and include PID. Patch follows... Best regards, Simon
> Note a side take-home message: msiexec.exe will never be Windows 10 aware. Unfortunately, yes. > > Let me know if that doesn't make sense or won't work for what you're > > doing. > I am familiar with EXE and manifests, thanks. Nevertheless, I really > appreciate your time to extensively research and provide useful references > in the debate. It looks like I erred widely on the side of overinforming. I appreciate your patience with that. > In my professional career I make EXE (or WSH) "parent installers" only > because end-users usually have no clue what platform is their Windows. You > can't do one-MSI-fits-all-platforms for native apps, so the parent installer > EXE takes platform choice away from users. All other decision logic is > inside MSI packages making them fully autonomous - once you pick the right > one for your platform of course. Makes sense. Fortunately x86 EXEs run on ARM64, so this even works there. :) > When the MSI package is installed by Group Policy Client, the MSI is > launched directly by msiexec.exe. Ah, I see. That puts a bit of a damper in bolting intelligence on during the msiexec invocation, doesn't it? I'm no expert with GP, and what little I know suggests that any methods for deploying custom EXEs through it are either much more complex than is feasible for you to support or otherwise not worth using. You're the expert between the two of us. Without this ability and with the compat shimming of the MSI infrastructure, there's not much else to be done. > Hence such a complex workaround to detect the MSI has been launched on > Windows 10. Microsoft made me do it. ;) :) > Please, note one last thing... I totally agree with everybody saying "you > That's why this custom action *does not* return the Windows version - not to > tempt anybody else to use it for OS version detection. > The name "FindSystemInfo" was picked generic on > purpose, to allow us to add other detections missing in MSI should we need > any down the road. Smart! I hadn't followed the function to its callers, so I missed this. Samuli, LGTM. Thanks, Jon
> I gave the MsiBreak method a quick try, but I couldn't make it work. I have > set MsiBreak as a system environment variable, restarted Windows Installer > service, restarted the shell window from where I invoke msiexec calls to > make sure the environment is updated. But it doesn't pop-up any prompt to > attach debugger as advertised. That's sad to hear. I'd toss out ideas to try, but... > I haven't installed the Debugging Tools for Windows, as I run Visual Studio > elevated to debug processes. I haven't restarted my computer: restart of a > working machine with stage set to debug something is a royal PITA: the > libopenvpnmsica.dll has more than one custom property to debug, restarting > each time to switch MsiBreak to a different custom action is not viable. I've debugged using MsiBreak set in the admin CMD prompt I used to launch the MSI, so restarts aren't required. The article goes into some very nearly useful details about when this is, but... > Therefore, I'd prefer to keep own MessageBox() call in the beginning of each > custom action. It works 100%. ...not only that, but the article even suggests this: "To start debugging without MsiBreak, put a temporary message box at the beginning of the action's code. When the message box appears during the installation, attach the debugger to the process owning the message box." If you ever need to debug say, the DLL loading, then MsiBreak is the way to go. As it stands, it's *REALLY* hard to beat 100%. :) > Adding PID to the message is an option. I personally never needed it. When > running Visual Studio elevated, you press Ctrl+Alt+P, followed by "msi" > keystrokes to focus on "msiexec.exe" processes, then observe the > MessageBox()'s title (which is deliberately set to function name) in the > "Title" column of the available process list. Way faster than searching > process by PID. I've been wrong before about how easy different options are. :) > Anyway, I have extended the debug pop-up dialogs to be more informative and > include PID. Patch follows... I took a look and it looks good to me, though I agree it's not strictly necessary. Thanks, Jon
Your patch has been applied to the master branch. "LGTM" from Jon after quite some discussion. I'm not trying to understand the intricacies of the code, just some basic sanity checking ("no chance this will scribble over a registry entry or execute unwanted binaries by means of an uninitialized buffer"). commit e2faae87b49379ce8bcffb2f0ae67b0671996f64 Author: Simon Rozman Date: Tue Oct 16 12:26:26 2018 +0200 Add MSI custom action for reliable Windows 10 detection Acked-by: Jon Kunkee <jkunkee@microsoft.com> Message-Id: <20181016102627.18676-4-simon@rozman.si> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17763.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpnmsica/Makefile.am b/src/openvpnmsica/Makefile.am index d46170b4..ecca74bc 100644 --- a/src/openvpnmsica/Makefile.am +++ b/src/openvpnmsica/Makefile.am @@ -41,7 +41,7 @@ libopenvpnmsica_la_CFLAGS = \ -municode -D_UNICODE \ -UNTDDI_VERSION -U_WIN32_WINNT \ -D_WIN32_WINNT=_WIN32_WINNT_VISTA -libopenvpnmsica_la_LDFLAGS = -ladvapi32 -lole32 -lmsi -lsetupapi -lshlwapi -no-undefined -avoid-version +libopenvpnmsica_la_LDFLAGS = -ladvapi32 -lole32 -lmsi -lsetupapi -lshlwapi -lversion -no-undefined -avoid-version endif libopenvpnmsica_la_SOURCES = \ diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c index 3b90ce05..d1642d6a 100644 --- a/src/openvpnmsica/openvpnmsica.c +++ b/src/openvpnmsica/openvpnmsica.c @@ -36,13 +36,15 @@ #include <memory.h> #include <msiquery.h> #include <shlwapi.h> -#ifdef _MSC_VER -#pragma comment(lib, "shlwapi.lib") -#endif #include <stdbool.h> #include <stdlib.h> #include <tchar.h> +#ifdef _MSC_VER +#pragma comment(lib, "shlwapi.lib") +#pragma comment(lib, "version.lib") +#endif + /** * Local constants @@ -119,7 +121,7 @@ openvpnmsica_setup_sequence_filename( { size_t len_action_name_z = _tcslen(openvpnmsica_cleanup_action_seqs[i].szName) + 1; TCHAR *szPropertyEx = (TCHAR*)malloc((len_property_name + len_action_name_z) * sizeof(TCHAR)); - memcpy(szPropertyEx , szProperty , len_property_name * sizeof(TCHAR)); + memcpy(szPropertyEx , szProperty , len_property_name * sizeof(TCHAR)); memcpy(szPropertyEx + len_property_name, openvpnmsica_cleanup_action_seqs[i].szName, len_action_name_z * sizeof(TCHAR)); _stprintf_s( szFilenameEx, _countof(szFilenameEx), @@ -142,6 +144,120 @@ openvpnmsica_setup_sequence_filename( } +UINT __stdcall +FindSystemInfo(_In_ MSIHANDLE hInstall) +{ +#ifdef _MSC_VER +#pragma comment(linker, DLLEXP_EXPORT) +#endif + +#ifdef _DEBUG + MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v") TEXT(PACKAGE_VERSION), MB_OK); +#endif + + UINT uiResult; + BOOL bIsCoInitialized = SUCCEEDED(CoInitialize(NULL)); + + /* Set MSI session handle in TLS. */ + struct openvpnmsica_tls_data *s = (struct openvpnmsica_tls_data *)TlsGetValue(openvpnmsica_tlsidx_session); + s->hInstall = hInstall; + + // Get Windows version. + OSVERSIONINFOEX ver_info = { .dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX) }; + if (!GetVersionEx((LPOSVERSIONINFO)&ver_info)) { + uiResult = GetLastError(); + msg(M_NONFATAL | M_ERRNO, "%s: GetVersionEx() failed", __FUNCTION__); + goto cleanup_CoInitialize; + } + + // The Windows version is usually spoofed, check using RtlGetVersion(). + TCHAR szDllPath[0x1000]; + ExpandEnvironmentStrings(TEXT("%SystemRoot%\\System32\\ntdll.dll"), szDllPath, +#ifdef UNICODE + _countof(szDllPath) +#else + _countof(szDllPath) - 1 +#endif + ); + HMODULE hNtDllModule = LoadLibrary(szDllPath); + if (hNtDllModule) + { + typedef NTSTATUS (WINAPI* fnRtlGetVersion)(PRTL_OSVERSIONINFOW); + fnRtlGetVersion RtlGetVersion = (fnRtlGetVersion)GetProcAddress(hNtDllModule, "RtlGetVersion"); + if (RtlGetVersion) + { + RTL_OSVERSIONINFOW rtl_ver_info = { .dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOW) }; + if (RtlGetVersion(&rtl_ver_info) == 0) + if ( + rtl_ver_info.dwMajorVersion > ver_info.dwMajorVersion || + rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && rtl_ver_info.dwMinorVersion > ver_info.dwMinorVersion || + rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && rtl_ver_info.dwMinorVersion == ver_info.dwMinorVersion && rtl_ver_info.dwBuildNumber > ver_info.dwBuildNumber) + { + // We got RtlGetVersion() and it reported newer version than GetVersionEx(). + ver_info.dwMajorVersion = rtl_ver_info.dwMajorVersion; + ver_info.dwMinorVersion = rtl_ver_info.dwMinorVersion; + ver_info.dwBuildNumber = rtl_ver_info.dwBuildNumber; + ver_info.dwPlatformId = rtl_ver_info.dwPlatformId; + } + } + + FreeLibrary(hNtDllModule); + } + + // We don't trust RtlGetVersion() either. Check the version resource of kernel32.dll. + ExpandEnvironmentStrings(TEXT("%SystemRoot%\\System32\\kernel32.dll"), szDllPath, +#ifdef UNICODE + _countof(szDllPath) +#else + _countof(szDllPath)-1 +#endif + ); + + DWORD dwHandle; + DWORD dwVerInfoSize = GetFileVersionInfoSize(szDllPath, &dwHandle); + if (dwVerInfoSize) + { + LPVOID pVersionInfo = malloc(dwVerInfoSize); + if (pVersionInfo) + { + // Read version info. + if (GetFileVersionInfo(szDllPath, dwHandle, dwVerInfoSize, pVersionInfo)) + { + // Get the value for the root block. + UINT uiSize = 0; + VS_FIXEDFILEINFO *pVSFixedFileInfo = NULL; + if (VerQueryValue(pVersionInfo, TEXT("\\"), &pVSFixedFileInfo, &uiSize) && uiSize && pVSFixedFileInfo) + if (HIWORD(pVSFixedFileInfo->dwProductVersionMS) > ver_info.dwMajorVersion || + HIWORD(pVSFixedFileInfo->dwProductVersionMS) == ver_info.dwMajorVersion && LOWORD(pVSFixedFileInfo->dwProductVersionMS) > ver_info.dwMinorVersion || + HIWORD(pVSFixedFileInfo->dwProductVersionMS) == ver_info.dwMajorVersion && LOWORD(pVSFixedFileInfo->dwProductVersionMS) == ver_info.dwMinorVersion && HIWORD(pVSFixedFileInfo->dwProductVersionLS) > ver_info.dwBuildNumber) + { + // We got kernel32.dll version and it is newer. + ver_info.dwMajorVersion = HIWORD(pVSFixedFileInfo->dwProductVersionMS); + ver_info.dwMinorVersion = LOWORD(pVSFixedFileInfo->dwProductVersionMS); + ver_info.dwBuildNumber = HIWORD(pVSFixedFileInfo->dwProductVersionLS); + } + } + + free(pVersionInfo); + } + } + + uiResult = MsiSetProperty(hInstall, TEXT("DriverCertification"), ver_info.dwMajorVersion >= 10 ? ver_info.wProductType > VER_NT_WORKSTATION ? TEXT("whql") : TEXT("attsgn") : TEXT("")); + if (uiResult != ERROR_SUCCESS) + { + SetLastError(uiResult); /* MSDN does not mention MsiSetProperty() to set GetLastError(). But we do have an error code. Set last error manually. */ + msg(M_NONFATAL | M_ERRNO, "%s: MsiSetProperty(\"TAPINTERFACES\") failed", __FUNCTION__); + goto cleanup_CoInitialize; + } + + uiResult = ERROR_SUCCESS; + +cleanup_CoInitialize: + if (bIsCoInitialized) CoUninitialize(); + return uiResult; +} + + UINT __stdcall FindTAPInterfaces(_In_ MSIHANDLE hInstall) { diff --git a/src/openvpnmsica/openvpnmsica.h b/src/openvpnmsica/openvpnmsica.h index bb8e28ec..da145062 100644 --- a/src/openvpnmsica/openvpnmsica.h +++ b/src/openvpnmsica/openvpnmsica.h @@ -63,6 +63,21 @@ extern "C" { #endif +/** + * Determines Windows information: + * - Sets `DriverCertification` MSI property to "", "attsgn" or "whql" + * according to the driver certification required by the running version of + * Windows. + * + * @param hInstall Handle to the installation provided to the DLL custom action + * + * @return ERROR_SUCCESS on success; An error code otherwise + * See: https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx + */ +DLLEXP_DECL UINT __stdcall +FindSystemInfo(_In_ MSIHANDLE hInstall); + + /** * Find existing TAP interfaces and set TAPINTERFACES property with semicolon delimited list * of installed TAP interface GUIDs.