Delete old-ID Mullvad TAP on updates

5 files changed, 54 insertions, 76 deletions

diff --git a/dist-assets/windows/installer.nsh b/dist-assets/windows/installer.nsh
index 864350a7a4..9cfbf30aa5 100644
--- a/dist-assets/windows/installer.nsh
+++ b/dist-assets/windows/installer.nsh
@@ -13,6 +13,7 @@
 #
 
 # TAP device hardware ID
+!define DEPRECATED_TAP_HARDWARE_ID "tap0901"
 !define TAP_HARDWARE_ID "tapmullvad0901"
 
 # "sc" exit code
@@ -168,54 +169,58 @@
 #
 # RemoveTap
 #
-# Try to remove the Mullvad TAP adapter
-# and driver if there are no other TAPs available.
+# Try to remove the Mullvad TAP adapter driver
 #
 !macro RemoveTap
 	Push $0
 	Push $1
 
-	driverlogic::Initialize
-
+	nsExec::ExecToStack '"$TEMP\driver\tapinstall.exe" remove ${TAP_HARDWARE_ID}'
 	Pop $0
 	Pop $1
 
-	${If} $0 != ${MULLVAD_SUCCESS}
-		Goto RemoveTap_return_only
-	${EndIf}
+	Pop $1
+	Pop $0
+
+!macroend
+
+!define RemoveTap '!insertmacro "RemoveTap"'
+
+#
+# RemoveOldIdTap
+#
+# Try to remove the old Mullvad TAP adapter (with a non-unique hardware ID),
+# and uninstall the driver if it's not in use.
+#
+!macro RemoveOldIdTap
+	Push $0
+	Push $1
 
-	driverlogic::RemoveMullvadTap
+	driverlogic::RemoveOldMullvadTap
 
 	Pop $0
 	Pop $1
 
 	${If} $0 == ${RMT_GENERAL_ERROR}
-		Goto RemoveTap_return
+		Goto RemoveOldIdTap_return
 	${EndIf}
 
 	${If} $0 == ${RMT_NO_REMAINING_ADAPTERS}
 		# Remove the driver altogether
-		nsExec::ExecToStack '"$TEMP\driver\tapinstall.exe" remove ${TAP_HARDWARE_ID}'
+		nsExec::ExecToStack '"$TEMP\driver\tapinstall.exe" remove ${DEPRECATED_TAP_HARDWARE_ID}'
 
 		Pop $0
 		Pop $1
 	${EndIf}
 	
-	RemoveTap_return:
-
-	driverlogic::Deinitialize
-	
-	Pop $0
-	Pop $1
-
-	RemoveTap_return_only:
+	RemoveOldIdTap_return:
 
 	Pop $1
 	Pop $0
 
 !macroend
 
-!define RemoveTap '!insertmacro "RemoveTap"'
+!define RemoveOldIdTap '!insertmacro "RemoveOldIdTap"'
 
 #
 # InstallDriver
@@ -244,7 +249,15 @@
 		log::Log $R0
 		Goto InstallDriver_return_only
 	${EndIf}
-	
+
+	#
+	# Remove the old-ID Mullvad TAP, if it exists
+	#
+	${RemoveOldIdTap}
+
+	#
+	# Reinstall the TAP driver if it is already installed. Updating may cause issues.
+	#
 	log::Log "Calling on plugin to enumerate network adapters"
 	driverlogic::EstablishBaseline
 
@@ -278,44 +291,8 @@
 
 	IntCmp $InstallDriver_BaselineStatus ${EB_NO_TAP_ADAPTERS_PRESENT} InstallDriver_install_driver
 
-	#
-	# Driver is already installed and there are one or several virtual adapters present.
-	# Update driver.
-	#
-	log::Log "TAP driver is already installed - Updating to latest version"
-	nsExec::ExecToStack '"$TEMP\driver\tapinstall.exe" update "$TEMP\driver\OemVista.inf" ${TAP_HARDWARE_ID}'
-
-	Pop $0
-	Pop $1
-
-	${If} $0 != ${DEVCON_EXIT_OK}
-	${AndIf} $0 != ${DEVCON_EXIT_REBOOT}
-		StrCpy $R0 "Failed to update TAP driver: error $0"
-		log::LogWithDetails $R0 $1
-		Goto InstallDriver_return
-	${EndIf}
-
-	#
-	# Driver updates will replace the GUIDs and names
-	# of our adapters, so let's restore them.
-	#
-	log::Log "Restoring any changed TAP adapter aliases"
-	driverlogic::RollbackTapAliases
-
-	Pop $0
-	Pop $1
-
-	${If} $0 != ${MULLVAD_SUCCESS}
-		StrCpy $R0 "Failed to roll back TAP adapter aliases: error $0"
-		log::LogWithDetails $R0 $1
-		Goto InstallDriver_return
-	${EndIf}
-
-	${If} $InstallDriver_BaselineStatus == ${EB_MULLVAD_ADAPTER_PRESENT}
-		log::Log "Virtual adapter with custom name already present on system"
-
-		Goto InstallDriver_return_success
-	${EndIf}
+	log::Log "Removing existing TAP driver"
+	${RemoveTap}
 
 	InstallDriver_install_driver:
 
diff --git a/windows/nsis-plugins/src/driverlogic/context.cpp b/windows/nsis-plugins/src/driverlogic/context.cpp
index 4df6a94b85..1718182768 100644
--- a/windows/nsis-plugins/src/driverlogic/context.cpp
+++ b/windows/nsis-plugins/src/driverlogic/context.cpp
@@ -28,6 +28,7 @@ namespace
 {
 
 const wchar_t TAP_HARDWARE_ID[] = L"tapmullvad0901";
+const wchar_t DEPRECATED_TAP_HARDWARE_ID[] = L"tap0901";
 
 template<typename T>
 void LogAdapters(const std::wstring &description, const T &adapters)
@@ -245,7 +246,7 @@ std::optional<std::wstring> GetDeviceRegistryStringProperty(
 	return { buffer.data() };
 }
 
-std::set<Context::NetworkAdapter> GetTapAdapters()
+std::set<Context::NetworkAdapter> GetTapAdapters(const std::wstring &tapHardwareId)
 {
 	std::set<Context::NetworkAdapter> adapters;
 
@@ -293,7 +294,7 @@ std::set<Context::NetworkAdapter> GetTapAdapters()
 
 		const auto hardwareId = GetDeviceRegistryStringProperty(devInfo, &devInfoData, SPDRP_HARDWAREID);
 		if (!hardwareId.has_value()
-			|| wcscmp(hardwareId.value().c_str(), TAP_HARDWARE_ID) != 0)
+			|| wcscmp(hardwareId.value().c_str(), tapHardwareId.c_str()) != 0)
 		{
 			continue;
 		}
@@ -387,7 +388,7 @@ std::optional<Context::NetworkAdapter> Context::FindMullvadAdapter(const std::se
 
 Context::BaselineStatus Context::establishBaseline()
 {
-	m_baseline = GetTapAdapters();
+	m_baseline = GetTapAdapters(TAP_HARDWARE_ID);
 
 	if (m_baseline.empty())
 	{
@@ -404,7 +405,7 @@ Context::BaselineStatus Context::establishBaseline()
 
 void Context::recordCurrentState()
 {
-	m_currentState = GetTapAdapters();
+	m_currentState = GetTapAdapters(TAP_HARDWARE_ID);
 }
 
 void Context::rollbackTapAliases()
@@ -453,9 +454,9 @@ Context::NetworkAdapter Context::getNewAdapter()
 }
 
 //static
-Context::DeletionResult Context::DeleteMullvadAdapter()
+Context::DeletionResult Context::DeleteOldMullvadAdapter()
 {
-	auto tapAdapters = GetTapAdapters();
+	auto tapAdapters = GetTapAdapters(DEPRECATED_TAP_HARDWARE_ID);
 	std::optional<NetworkAdapter> mullvadAdapter = FindMullvadAdapter(tapAdapters);
 
 	if (!mullvadAdapter.has_value())
@@ -505,7 +506,7 @@ Context::DeletionResult Context::DeleteMullvadAdapter()
 		const auto hardwareId = GetDeviceRegistryStringProperty(devInfo, &devInfoData, SPDRP_HARDWAREID);
 
 		if (hardwareId.has_value()
-			&& wcscmp(TAP_HARDWARE_ID, hardwareId.value().data()) == 0)
+			&& wcscmp(DEPRECATED_TAP_HARDWARE_ID, hardwareId.value().data()) == 0)
 		{
 			if (0 != GetNetCfgInstanceId(devInfo, devInfoData).compare(mullvadGuid))
 			{
diff --git a/windows/nsis-plugins/src/driverlogic/context.h b/windows/nsis-plugins/src/driverlogic/context.h
index 9c0b19d62f..d3b57d4f44 100644
--- a/windows/nsis-plugins/src/driverlogic/context.h
+++ b/windows/nsis-plugins/src/driverlogic/context.h
@@ -60,7 +60,7 @@ public:
 		SOME_REMAINING_TAP_ADAPTERS
 	};
 
-	static DeletionResult DeleteMullvadAdapter();
+	static DeletionResult DeleteOldMullvadAdapter();
 
 private:
 
diff --git a/windows/nsis-plugins/src/driverlogic/driverlogic.cpp b/windows/nsis-plugins/src/driverlogic/driverlogic.cpp
index 4cd54f4ed0..8f00dbcecf 100644
--- a/windows/nsis-plugins/src/driverlogic/driverlogic.cpp
+++ b/windows/nsis-plugins/src/driverlogic/driverlogic.cpp
@@ -150,19 +150,19 @@ void __declspec(dllexport) NSISCALL EstablishBaseline
 }
 
 //
-// RemoveMullvadTap
+// RemoveOldMullvadTap
 //
-// Deletes the Mullvad TAP adapter.
+// Deletes the old Mullvad TAP adapter with ID tap0901.
 //
 //
-enum class RemoveMullvadTapStatus
+enum class RemoveOldMullvadTapStatus
 {
 	GENERAL_ERROR = 0,
 	SUCCESS_NO_REMAINING_TAP_ADAPTERS,
 	SUCCESS_SOME_REMAINING_TAP_ADAPTERS
 };
 
-void __declspec(dllexport) NSISCALL RemoveMullvadTap
+void __declspec(dllexport) NSISCALL RemoveOldMullvadTap
 (
 	HWND hwndParent,
 	int string_size,
@@ -178,17 +178,17 @@ void __declspec(dllexport) NSISCALL RemoveMullvadTap
 	{
 		pushstring(L"");
 		
-		switch (Context::DeleteMullvadAdapter())
+		switch (Context::DeleteOldMullvadAdapter())
 		{
 			case Context::DeletionResult::NO_REMAINING_TAP_ADAPTERS:
 			{
-				pushint(RemoveMullvadTapStatus::SUCCESS_NO_REMAINING_TAP_ADAPTERS);
+				pushint(RemoveOldMullvadTapStatus::SUCCESS_NO_REMAINING_TAP_ADAPTERS);
 				break;
 			}
 
 			case Context::DeletionResult::SOME_REMAINING_TAP_ADAPTERS:
 			{
-				pushint(RemoveMullvadTapStatus::SUCCESS_SOME_REMAINING_TAP_ADAPTERS);
+				pushint(RemoveOldMullvadTapStatus::SUCCESS_SOME_REMAINING_TAP_ADAPTERS);
 				break;
 			}
 
@@ -201,12 +201,12 @@ void __declspec(dllexport) NSISCALL RemoveMullvadTap
 	catch (std::exception &err)
 	{
 		pushstring(common::string::ToWide(err.what()).c_str());
-		pushint(RemoveMullvadTapStatus::GENERAL_ERROR);
+		pushint(RemoveOldMullvadTapStatus::GENERAL_ERROR);
 	}
 	catch (...)
 	{
 		pushstring(L"Unspecified error");
-		pushint(RemoveMullvadTapStatus::GENERAL_ERROR);
+		pushint(RemoveOldMullvadTapStatus::GENERAL_ERROR);
 	}
 }
 
diff --git a/windows/nsis-plugins/src/driverlogic/driverlogic.def b/windows/nsis-plugins/src/driverlogic/driverlogic.def
index f7982c513d..2367487538 100644
--- a/windows/nsis-plugins/src/driverlogic/driverlogic.def
+++ b/windows/nsis-plugins/src/driverlogic/driverlogic.def
@@ -5,6 +5,6 @@ EXPORTS
 Initialize
 EstablishBaseline
 IdentifyNewAdapter
-RemoveMullvadTap
+RemoveOldMullvadTap
 RollbackTapAliases
 Deinitialize