OpenVPN / tap-windows6

Windows TAP driver (NDIS 6)
Other
798 stars 237 forks source link

MSM: Incomplete old driver removal #129

Closed becm closed 4 years ago

becm commented 4 years ago

When updating to new version, obsolete TAP drivers are not cleaned up correctly. This affects any update of OpenVPN on Windows (NSIS → MSI and MSI → MSI) when TAP driver changes.

For each existing entry in C:\Windows\System32\DriverStore\FileRepository, a ! prefixed entry is created in the Owners key (might be multiple) at registry paths HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\tap0901 and HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Setup\PnpLockdownFiles\%SystemRoot%/system32/DRIVERS/tap0901.sys

Correct operation (assumed for clean system, valid for Service entry after manual cleanup):

Invalid behaviour:

Effect: minor: Warnings in C:\Windows\INF\setupapi.dev.log minor: service entry in registry not removed unknown: PnP locks on \SystemRoot\System32\drivers\tap0901.sys (file not removed)

Transitions not affected (same TAP version): OpenVPN 2.5_beta22.5_rc1

Entrys in setupapi.dev.log service entry remnants → "Service 'tap0901' still in use by 1 source" PnP entry remnants → "File 'C:\Windows\System32\drivers\tap0901.sys' still in use by 1 source."

Entry in Services\tap0901 can be removed manually (for multiple trials → different count on uninstall). Access to PnpLockdownFiles for manual cleanup is denied.

mattock commented 4 years ago

Thoughts @lstipakov, @selvanair or @rozmansi?

becm commented 4 years ago

@lstipakov Strong indication adapter holding references to old driver during update is the culprit.

Had a chance to test on a (seemingly) clean system.

in comparison to:

As mentioned before, only one try per clean system. Unless somebody knows a way to clean up PnpLockdownFiles appart from the or yearly Win10H1 upgrade.

lstipakov commented 4 years ago

I managed to clean PnpLockdownFiles by running regedit from SYSTEM command prompt (psutil -i -s cmd).

lstipakov commented 4 years ago

I can confirm that - indeed after NSIS->MSI upgrade there are registry entries in "Services" and "PnpLockdownFiles" with zombie "!oemvista" driver.

Those entries also exist after performing NSIS->NSIS upgrade, but they refer to oemX.inf / oemY.inf (when upgrading to a new tap driver) and when drivers are removed with pnputil, registry entries are also gone. This cannot be done for "!oemvista" zombie driver which pnputil doesn's show.

becm commented 4 years ago

It might help to first install the new driver and take additional steps to keep it on the later removal of old instances. NSIS approach demonstrates multiple (TAP) driver instances installed at the same time is not a problem.

Using Wintun MSM with identical steps in OpenVPN context (with persistent adapters) will likely display similar problems on update. In a quick test, Wireguard installer seems to not create adapters (?) and might not be affected.

rozmansi commented 4 years ago
  1. WireGuard always creates TUN adapters on demand and deletes them after use.
  2. WireGuard installer has a custom action to close all tunnels before the upgrade.
  3. Wintun driver/installer communicate to force-close all TUN adapter open handles before the upgrade, allowing the TUN adapter to get disabled and driver unloaded.

Each of those three features allows Windows to gracefully unload wintun.sys driver, allowing a clean upgrade.

TAP-Windows MSM is a fork of Wintun installer without any of the above features:

In the OpenVPN world, the TUN/TAP adapter is always present and enabled. OpenVPN doesn't manipulate adapter (tapctl.exe does); it just manipulates the IP interface of this adapter. This causes Windows to keep the tap0901.sys loaded all the time (wintun.sys as well if you are using it with OpenVPN too.).

Installing the new driver and calling UpdateDriverForPlugAndPlayDevices() will not cause the adapters to switch to the new driver right away. Only after all adapters are disabled (i.e. stopped), and the old tap0901.sys is unloaded. Usually a reboot. Only then the old driver may be uninstalled.

There are two challenges in the former paragraph: a) How to make Windows unload tap0901.sys programatically? b) How to make MSM to continue upgrade process after a reboot?

Either should be solved reliably. a) provides better UX. Let's take a look: an adapter cannot be disabled if any process has open handles with that adapter. This means the TAP-Windows6 installer should kill all openvpn.exe, openvpnserv.exe, tunsafe.exe and whatnot processes. Only after the last TAP-Windows6 adapter is disabled, the Windows kernel would unload old tap0901.sys, releasing the driver files to be gracefully deleted.

I personally believe the most robust way of upgrading the driver would be to:

  1. Try to remove whatever TAP-Windows6 driver from the store it can. This would uninstall only the old unused TAP drivers: versions other or before the one currently being used. Should one driver fail to be uninstalled for being busy, set a "reboot pending" flag.
  2. Add the new driver to the store, call UpdateDriverForPlugAndPlayDevices() - this one also suggests a reboot if required.
  3. If the "reboot pending" flag is set, suggest user to reboot to start using the new driver.

Rationale behind this proposal is:

Note: you cannot "upgrade" to the older driver this way. But that's not the direction the evolution goes. :)

lstipakov commented 4 years ago

This looks reasonable. I want to familiarize myself better with SetupAPI, let me implement this and submit PR.

becm commented 4 years ago

To have this work for both drivers, @zx2c4 will have to adopt relevant changes in Wintun MSM:

From what I can tell, OpenVPN MSI also closes connections and the TAP MSM tries to disable adapters. This either fails or is not sufficent to remove the driver lock (at least when no alternatives are installed yet).

becm commented 4 years ago

Tests with Wireguard installer 0.0.28/0.1.1 display same problem if a wintun adapter is created with tapctl.exe. Seems pure existence of a persistent adapter breaks the current tap-windows6 and wintun MSM update approach.

lstipakov commented 4 years ago

@becm would you be able to test installer with this fix included? https://lestisoftware.fi/OpenVPN-2.5-rc2-I601-amd64.msi

I have tested:

There are no !oemvista values in registry anymore. After uninstalling, HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\tap0901 remains but without Owners key, and PnpLockdownFiles key is deleted.