-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Windows MSI Packaging #141
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me in general. I added some inline comments and will test this next.
windows-msi/version.m4
Outdated
|
||
dnl The upgrade GUIDs MUST persist for all versions of the same product line. | ||
dnl Please use own upgrade GUIDs when deploying a non-official OpenVPN release. | ||
define([PRODUCT_UPGRADE_GUID_x86], [{1195A47B-A37A-4055-9D34-B7A691F7E97B}]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How were/are these GUIDs created?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uuidgen | tr [a-f] [A-F]
I just picked some random to get started. Any unique hexadecimal number is OK. Old MSI versions required all uppercase GUIDs and I stick to that recommendation.
PRODUCT_UPGRADE_GUID_...
are called upgrade code(s) and are used by MSI to follow the product line across versions, 3rd party products to detect OpenVPN is installed... The upgrade code is rarely changed - it is the product identifier. However, I choose not to hard-code them to allow 3rd party vendors to package their own "OpenVPN" product line.
Use of platform-specific upgrade codes teaches Group Policy Management Editor not to mix 32 and 64-bit packages when updating. Example: MS Office 2016 has something like {00160000-008C-0000-0000-0000000FF1CE}
for 32-bit version and {00160000-008C-0000-1000-0000000FF1CE}
for 64-bit.
If you want some fancy ones to mark the official line of 32 and 64-bit OpenVPN MSI packages, it is time to set them now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know little about MSI or WiX so only randomly looked at a few places -- great job indeed!
It may be hard to get all cross-dependencies and nuances about upgrading from previous NSIS install right in the first round, so we can merge, test, fix, repeat, I guess.
And one more thing: If the service or GUI is running is it stopped to allow for file updates or does the user have to intervene? Quitting the GUI is easy for the user, but at least for the service we should stop and restart as/when required. Couldn't figure how/whether that's done. |
Services are authored to stop on component uninstall and start on component install. (But, I'll double check that.) The applications (i.e. |
That would work for updates or fresh install but not for "upgrade" from NSIS installed version, would it? Note that even if services are not getting updated, openvpnserv2 has to be stopped to terminate any running openvpn.exe. In case of upgrading from nsis, if service is running the install may just fail or lead to a broken setup? I don't yet have a setup to build and test this, so just guessing. Triggering the nsis uninstall script if previous install detected may be an option. As msi installer will be only for 2.5 (?) looking for a pre 2.5 openvpn.exe, and, if found, uninstall first may also be an option? For GUI, prompting the user to stop it or reboot later is fine. |
For a better illustration...
As expected, I get the following prompt. (OpenVPN Deamon process is openvpn.exe) Now I can choose to terminate the VPN connection and close the OpenVPN GUI myself before updating. Or, proceed with the update and restart computer later.
This is the log of the update: OpenVPN-install.log |
Interesting.. I guess it must be moving the original gui exe to a temp location and let it continue running. Renaming files in use is supported by ntfs (though the file explorer UI may not allow it). What executable path and name are shown in the running process properties? Updating from a 2.4.4 or older NSIS install to 2.4.5 or 6 should show this clearly --- the running GUI should report a version of 11.9.0.0 in its settings->About menu but the newly installed file will show 11.10.0.0. The GUI version did not change between 2.4.5 and 2.4.6. |
This confirms @selvanair's assumptions. The update indeed allows the established VPN connections to persist until the processed are recycled. This means very good news for updating remote computers over an OpenVPN connection. Test machine: up-to-date Windows 8.1 Pro. Now I need to use even older version, to test the TAP driver update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means an update of the service will require a reboot (or a manual service restart if the user knows how to do that) for the update to take effect, right? Could lead to inconsistencies if the user just restarts the GUI so that an updated openvpn.exe will get used with an older interactive service.
Does it show a warning that updates will be effective only after a system restart or some such or can we provide an optional service restart at the end of the installation?
Meanwhile, I have tested and confirmed that live updating works on Windows 7 too. Yes, Selva... This currently means that user is responsible for restarting OpenVPN (daemon, interactive service, GUI, and/or We have two opposing design options now:
Initially, I was targeting at option 2. However, after observing that live update is possible, I started leaning towards option 1. My last patch at [openvpn-devel] mailing list combined with the last patch in this PR demonstrate that it is possible to detect live VPN connections in the update process. I am thinking about challenges if we take hybrid approach: When live VPN connections are detected, user is given a choice: "Do you want to preserve established VPN connections? (Note: You will need to restart OpenVPN later to switch to the new version.)" If there are no live connections or user chooses No for the previous question, the MSI does stop-update-start; otherwise update only. |
I think this is okay, but we could be a little more helpful by saying the service also need to be restarted and how to do that. Alternatively just say that a reboot is required for changes to take effect (if service was updated) --- Windows users are used to rebooting anyway :) @mattock may have better suggestions. |
I think the two options suggested by @rozmansi are for different audiences/use-cases:
A hybrid approach would be best of course. If it is difficult to implement then let's think about which one of the above we should choose. There are good arguments for both. That said, enterprise sysadmins are able to figure out ways to work around caveats in option 2, whereas typical end users might be caught badly off-guard by 1. |
The current implementation of MSI packets was the 1st option already. Because it was the simplest to implement. :) I have changed the MSI package to:
One more thing about services: The Microsoft Installer is designed to install, configure and start services in a constant way = update resets everything back to defaults. Stock MSI actions are pretty limited. It is impossible to use a variable for the required service state and startup. Hence, the legacy and the I am desperately trying to find time to add a new set of custom actions to detect which services were running before the update, what startup type they had, and to start and configure them the same after the update is complete. Much like the NSIS installer does now. It's a lot of work and basically means reinventing the wheel (in other words: replacing entire MSI service management by a custom one). Unless we can afford to have the BTW, a transform can be authored for those users to have it applied at installation/update time to set the Or, we could simply author the MSI to start and set for auto-start the Just thinking aloud... |
If it wont be too much work, an option is to check whether the service is running and if so set a registry key named autostart_config_dir (only if absent) to the same value as config_dir. Else leave it blank. If that's easier to do, we can change the service code to auto start configs in that directory and let the service start type to auto by default. This check is needed only for the first move (from NSIS to MSI) --anyway, we are already doing a some housekeeping jobs during install in that case. I never liked openvpn service autostarting all configs in the same central config folder that the GUI also reads. We should have had a separate location for such configs or a way of selecting which configs to autostart. This may be an opportunity to do so. |
This is not too much work and it would provide a seamless update for
In the long term, I think it is cleaner and more consistent we always set Another problem with
I totally agree. I vote for the separate config folder approach:
(Note that using two separate config folders means user needs to move/duplicate all external files referenced in .ovpn by the relative path too. We could keep the same config folder but have a .ovpn and .ovpn-auto config files. Unfortunately, Windows Explorer makes it difficult to change the file extensions by default. We could change the |
👍 to having a separate folder for automatically started config files. I'm also not against having separate config directories for client and server configs, as long as there are no implicit assumptions about auto-starting based on type. For example, my wife has an always-on client connection to her workplace, so autostarting is not just a thing for servers. But it could get tricky if we also need to distinguish between client, server and autostart configs. |
Dividing the config folder into client/server on Windows would only bring confusion. Heck, I don't even see the rationale behind dividing
...comes to my mind. The later would still allow OpenVPN-GUI to run a .ovpn from |
I agree with not distinguishing server and client configs. Very few people run server on Windows and they can fend for themselves. As for config-auto vs config\service, I prefer the former -- the latter where the GUI sees these configs brings us back closer to the current state: users may try to start those configs from the GUI, may even succeed (if there are enough tap adapters) and cause the auto-started one to disconnect or the two to usurp each other back and forth. What I had in mind was something like this (using some key and folder names for concreteness, not saying these are the best names)
Then update the service code to use autostart_config_dir reg key to find the config file locations and ignore if its blank, missing, unreadable, empty etc. Also update NSIS installer to create this reg key if we distribute the updated service with future 2.4.x releases. The only real downside is what @rozmansi mentioned: the change of config location for new installs. That's the price to pay for separating the two kinds of configs. As for users who want to stop the auto-start, they should either move the configs away or rename the folder or the registry key. Stopping the service is not a good method (except as a stopgap measure), as upgrades will bring the service back on. BTW, "auto" may not be the right word to describe these configs -- in the GUI also we can have some configs to "autostart" so the meaning of auto is context-dependent. That said, I don't have a better word -- "service", "daemon", "always-on", "prestarted" ? -- and am ok with "auto". |
On Linux there was solid reasoning behind separating client and server configs, but I've forgotten the details. It probably had to do with init script / systemd unit file ordering on boot, but I could be mistaken. On Windows I think it would just confuse people. For me config-auto seems reasonable. The pseudo-code by @selvanair looks good. |
@rozmansi To avoid your concern about folder name depends on upgrade/fresh-install issue, we could copy the contents of config to config-auto when appropriate: key not found but service running, so we add the key and copy the contents of config. Then the reg key value also stays independent of fresh install or upgrade. Documenting the change becomes easier that way. |
@selvanair Can you do the |
Looking at openvpnserv2 -- it reads both 32 bit and 64 bit registries and starts configs in both (using appropriate exe_path found in each place). Is that what is expected or can we just require 64 bit OpenVPN installation on 64 bit machines? Else the installer doing 64 bit install will have to also look for existing 32 bit install and update its registry, copy config folder etc.. @mattock: any suggestions? |
The MSI packages will install binaries according to the next matrix:
[*] Therefore, we can't just always expect 64-bit OpenVPN installation on 64-bit Windows. We will shoot ourselves in the foot when we release ARM64 version. What we can assume is, no system shall have both 32 and 64-bit versions of binaries installed at once. We don't have MinGW, OpenSSL and other libraries available for ARM64 yet and will have to use x86 user-space native binaries for the time being. However, we will hit a problem when we go out with an all-ARM64 package: updating the ARM64/x86 Frankenstein installations will require migrating config files from |
Okay, let's go with that but leave the current logic of scanning and starting configs based on both normal and Wow6432Node keys. The new version of service will look for autostart_config_dir key (is that the name we settled on?) in both nodes. Makes it very simple. We support cases were Wow6432Node key point to a 64 bit installation (exe_path, config_dir etc.) so there could be some corner cases which admins will have to fix manually. I'm not worried about ARM as there are no existing (NSIS) installations on that platform so we start from a clean slate and the new reg key and config folder will always exist (empty or not). But your point about valid 32 bit installations on 64bit is good to keep in mind. |
|
Agreed. |
This allows to isolate autostarted configs from those accessed through the GUI into separate directories. This also allows setting the service start type to default to Auto (not manual) without unexpectedly starting profiles in config_dir. If the registry value does not exist or is empty it is ignored. Otherwise it must point to a directory that exists. The installer should set up a default value and default directory. See OpenVPN/openvpn-build#141 (comment) and subsequent discussion. Signed-off-by: Selva Nair <selva.nair@gmail.com>
This allows to isolate autostarted configs from those accessed through the GUI into separate directories. This also allows setting the service start type to default to Auto (not manual) without unexpectedly starting profiles in config_dir. If the registry value does not exist or is empty it is ignored. Otherwise it must point to a directory that exists. The installer should set up a default value and default directory. See OpenVPN/openvpn-build#141 (comment) and subsequent discussion. Signed-off-by: Selva Nair <selva.nair@gmail.com>
This allows to isolate autostarted configs from those accessed through the GUI into separate directories. This also allows setting the service start type to default to Auto (not manual) without unexpectedly starting profiles in config_dir. If the registry value does not exist or is empty it is ignored. Otherwise it must point to a directory that exists. The installer should set up a default value and default directory. See OpenVPN/openvpn-build#141 (comment) and subsequent discussion. Signed-off-by: Selva Nair <selva.nair@gmail.com>
See PR #142 and PR 10 of OpenVPN/openvpnserv2 Based on this patch, I update my previous pseudo code to
|
Some distributions of GNU utils for Windows install gunzip as a symlink to gzip.exe (e.g. Git for Windows). Such symlinks are not supported within cmd.exe shell. The building script was upgraded to use `gzip.exe -d` instead. Although Git for Windows installs bunzip2.exe (as a clone of bzip2.exe), an analogous `bzip2.exe -d` approach was adopted. Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
At the time developing the MSI packaging, I am using same Windows computer for digital signing and packaging. I have evolved three batch files to assist me. If somebody else might find them useful, or merely as a source of inspiration, I am including those in the distribution. Signed-off-by: Simon Rozman <simon@rozman.si>
When using BSD tar.exe or tar.exe without GZip/BZip2 support, we must use piping. This batch explicitly instructs tar.exe to use TAR stream from stdin. Note: Only GNU tar.exe is tested which this commit documents. Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
This prevents repeating `msiexec /fu` calls at user logon issue. The Active Setup component version is incremented when product transitions from uninstalled to installed state only now. This means that uninstalling OpenVPN and then reinstalling same version of OpenVPN it will still bump the Active Setup component version and trigger one-time `msiexec /fu` call on first user logon after the reinstallation. Signed-off-by: Simon Rozman <simon@rozman.si>
WScript.Shell's method RegDelete is silly enough to throw an error when a registry value it is trying to delete something that does not exist. This is not really an error condition as the state we are trying to achieve is already achieved. In case there is no StubPath value in the registry, PublishActiveSetup() fails in the uninstall phase preventing OpenVPN uninstallation. Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
When there are no TAP-Windows6 adapters present, the tap0901 driver's service is not (and cannot be) started. This prevents all OpenVPN services from starting. The no-TAP-Windows6/Wintun adapters present situation is checked at run-time. Signed-off-by: Simon Rozman <simon@rozman.si>
The TAP-Windows6 certificate injection will be handled within the oncoming MSM merge module. Signed-off-by: Simon Rozman <simon@rozman.si>
The TAP-Windows6 driver installation will be handled by the oncoming MSM module. Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
The WiX files were updated to adopt changes and renaming that took place in openvpn repository. Signed-off-by: Simon Rozman <simon@rozman.si>
The libopenvpnmsica.dll has been upgraded to support arbitrary hardware ID network adapter creation on install. Signed-off-by: Simon Rozman <simon@rozman.si>
There's no "Ethernet" in Wintun. And there might be no "Ethernet" in TAP-Windows6 either. Signed-off-by: Simon Rozman <simon@rozman.si>
Return msiDoActionStatusSuccess (i.e. "Action completed successfully.") rather than zero default, which MSI interpretes as msiDoActionStatusNoAction (i.e. "Action not executed."). See-also: https://docs.microsoft.com/en-us/windows/win32/msi/return-values-of-jscript-and-vbscript-custom-actions Signed-off-by: Simon Rozman <simon@rozman.si>
Signed-off-by: Simon Rozman <simon@rozman.si>
When MSI detects user is using OpenVPNService (the service is running or set to auto-start), it marks OpenVPN.Service for install. If the config-auto subfolder does not exist yet, it moves all files from config subfolder to config-auto. This is a one-time migration of auto-start config files. Another feature of the MSI installer is a thorough cleanup of all the NSIS files and registry settings before install. So far so good. But (there always is a "but", otherwise you wouldn't be reading this, would you?), both features combined explode! The RemoveFiles removes the NSIS' config/README.txt file first, while the MoveFiles action already has a list of files to move from config to config-auto. With config/README.txt already gone, MoveFiles fails. I have decided not to clean any NSIS leftover files from the config subfolder. The README.txt file is overwriten by the same-named MSI supplied file anyway. And the config subfolder is kind of sacred => it contains user authored files. Moving them to a different folder is bold enough. Signed-off-by: Simon Rozman <simon@rozman.si>
I was able to build MSI packages that use the tap-windows6 MSM. Installation with default settings on a computer with no OpenVPN installations (=uninstalled NSIS beforehand) worked on these platforms:
It failed on Windows 10 arm64 due to tap-windows6 and wintun issues. I ran out of time before being able to debug that further. It could be a problem in my build, or not. I'll fill in more details later. But the "more important" platforms seem to be ok. For some reason Windows Server 2016 informed me that the computer needs a restart to complete the installation. Not sure why. My builds are here: |
The MSI installers were produced and "seem to work" just fine. |
This PR adds MSI packaging side-by-side with NSIS packaging.
Quick Instructions
MSI Packaging
See detailed instructions in https://github.com/rozmansi/openvpn-build/blob/feature/windows-msi/windows-msi/README.rst.
Binaries for Initial Impression
Compiled, signed and MSI packaged OpenVPN is available at https://github.com/rozmansi/openvpn/releases/tag/v2.4.6m. This is for informational purposes, to get a quick overview of setup experience.
Final Note
Please mind that this PR starts with a packaging of OpenVPN 2.4.6 version instead of 2.5._git on purpose. We need to have previous version of the package to test the upgrading process before 2.5 is published.