Because Windows, because Windows. Now it upgrades correctly from 1.0.1, including automatic driver update from NDIS5 to NDIS6. Also a bit more robust on creating new ports, just in case.

3 files changed, 88 insertions, 54 deletions

diff --git a/ext/installfiles/windows/ZeroTier One.aip b/ext/installfiles/windows/ZeroTier One.aip
index 7ca1270e..c0a6d91b 100644
--- a/ext/installfiles/windows/ZeroTier One.aip
+++ b/ext/installfiles/windows/ZeroTier One.aip
@@ -23,7 +23,7 @@
     <ROW Property="CTRLS" Value="2"/>
     <ROW Property="MSIFASTINSTALL" MultiBuildValue="DefaultBuild:2"/>
     <ROW Property="Manufacturer" Value="ZeroTier, Inc."/>
-    <ROW Property="ProductCode" Value="1033:{B29840D5-AC05-4F09-970F-34071C06912C} " Type="16"/>
+    <ROW Property="ProductCode" Value="1033:{539E07E1-8687-4B88-BA01-91ED2DD5E162} " Type="16"/>
     <ROW Property="ProductLanguage" Value="1033"/>
     <ROW Property="ProductName" Value="ZeroTier One"/>
     <ROW Property="ProductVersion" Value="1.0.4" Type="32"/>
@@ -101,8 +101,8 @@
     <ATTRIBUTE name="Enable" value="false"/>
   </COMPONENT>
   <COMPONENT cid="caphyon.advinst.msicomp.ChainedPackageComponent">
-    <ROW ChainedPackage="ZeroTierOne_NDIS6_x64.msi" Order="1" Options="108" InstallCondition="((NOT Installed) AND (VersionNT64))" RemoveCondition="((REMOVE=&quot;ALL&quot;) AND (NOT UPGRADINGPRODUCTCODE) AND (VersionNT64))"/>
-    <ROW ChainedPackage="ZeroTierOne_NDIS6_x86.msi" Order="2" Options="108" InstallCondition="((NOT Installed) AND (NOT VersionNT64))" RemoveCondition="((REMOVE=&quot;ALL&quot;) AND (NOT UPGRADINGPRODUCTCODE) AND (NOT VersionNT64))"/>
+    <ROW ChainedPackage="ZeroTierOne_NDIS6_x64.msi" Order="1" Options="108" InstallCondition="(VersionNT64)" RemoveCondition="((REMOVE=&quot;ALL&quot;) AND (NOT UPGRADINGPRODUCTCODE) AND (VersionNT64))"/>
+    <ROW ChainedPackage="ZeroTierOne_NDIS6_x86.msi" Order="2" Options="108" InstallCondition="(NOT VersionNT64)" RemoveCondition="((REMOVE=&quot;ALL&quot;) AND (NOT UPGRADINGPRODUCTCODE) AND (NOT VersionNT64))"/>
   </COMPONENT>
   <COMPONENT cid="caphyon.advinst.msicomp.ChainedPackageFileComponent">
     <ROW FileId="ZeroTierOne_NDIS6_x64.msi" ChainedPackage="ZeroTierOne_NDIS6_x64.msi" Options="1" TargetPath="ZeroTierOne_NDIS6_x64.msi" Content="..\..\bin\tap-windows-ndis6\x64\ZeroTierOne_NDIS6_x64.msi"/>
diff --git a/osdep/WindowsEthernetTap.cpp b/osdep/WindowsEthernetTap.cpp
index 2c338516..c6b434e1 100644
--- a/osdep/WindowsEthernetTap.cpp
+++ b/osdep/WindowsEthernetTap.cpp
@@ -234,10 +234,19 @@ std::string WindowsEthernetTap::addNewPersistentTapDevice(const char *pathToInf)
 		return std::string("SetupDiCallClassInstaller(DIF_REGISTERDEVICE) failed");
 	}
 
-	BOOL rebootRequired = FALSE;
-	if (!WINENV.UpdateDriverForPlugAndPlayDevicesA((HWND)0,WINENV.tapDriverName.c_str(),pathToInf,INSTALLFLAG_FORCE|INSTALLFLAG_NONINTERACTIVE,&rebootRequired)) {
+	// HACK: During upgrades, this can fail while the installer is still running. So make 60 attempts
+	// with a 1s delay between each attempt.
+	bool driverInstalled = false;
+	for(int retryCounter=0;retryCounter<60;++retryCounter) {
+		BOOL rebootRequired = FALSE;
+		if (WINENV.UpdateDriverForPlugAndPlayDevicesA((HWND)0,WINENV.tapDriverName.c_str(),pathToInf,INSTALLFLAG_FORCE|INSTALLFLAG_NONINTERACTIVE,&rebootRequired)) {
+			driverInstalled = true;
+			break;
+		} else Sleep(1000);
+	}
+	if (!driverInstalled) {
 		WINENV.SetupDiDestroyDeviceInfoList(deviceInfoSet);
-		return std::string("UpdateDriverForPlugAndPlayDevices() failed -- unable to install driver on device");
+		return std::string("UpdateDriverForPlugAndPlayDevices() failed (made 60 attempts)");
 	}
 
 	WINENV.SetupDiDestroyDeviceInfoList(deviceInfoSet);
@@ -285,13 +294,16 @@ std::string WindowsEthernetTap::destroyAllLegacyPersistentTapDevices()
 		RegCloseKey(nwAdapters);
 	}
 
+	std::string errlist;
 	for(std::set<std::string>::iterator iidp(instanceIdPathsToRemove.begin());iidp!=instanceIdPathsToRemove.end();++iidp) {
 		std::string err = deletePersistentTapDevice(iidp->c_str());
-		if (err.length() > 0)
-			return err;
+		if (err.length() > 0) {
+			if (errlist.length() > 0)
+				errlist.push_back(',');
+			errlist.append(err);
+		}
 	}
-
-	return std::string();
+	return errlist;
 }
 
 std::string WindowsEthernetTap::destroyAllPersistentTapDevices()
@@ -334,13 +346,16 @@ std::string WindowsEthernetTap::destroyAllPersistentTapDevices()
 		RegCloseKey(nwAdapters);
 	}
 
+	std::string errlist;
 	for(std::set<std::string>::iterator iidp(instanceIdPathsToRemove.begin());iidp!=instanceIdPathsToRemove.end();++iidp) {
 		std::string err = deletePersistentTapDevice(iidp->c_str());
-		if (err.length() > 0)
-			return err;
+		if (err.length() > 0) {
+			if (errlist.length() > 0)
+				errlist.push_back(',');
+			errlist.append(err);
+		}
 	}
-
-	return std::string();
+	return errlist;
 }
 
 std::string WindowsEthernetTap::deletePersistentTapDevice(const char *instanceId)
@@ -455,7 +470,6 @@ WindowsEthernetTap::WindowsEthernetTap(
 	char subkeyClass[4096];
 	char data[4096];
 	char tag[24];
-	std::set<std::string> existingDeviceInstances;
 	std::string mySubkeyName;
 
 	if (mtu > 2800)
@@ -487,10 +501,8 @@ WindowsEthernetTap::WindowsEthernetTap(
 					std::string instanceId;
 					type = 0;
 					dataLen = sizeof(data);
-					if (RegGetValueA(nwAdapters,subkeyName,"NetCfgInstanceId",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS) {
+					if (RegGetValueA(nwAdapters,subkeyName,"NetCfgInstanceId",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS)
 						instanceId.assign(data,dataLen);
-						existingDeviceInstances.insert(instanceId);
-					}
 
 					std::string instanceIdPath;
 					type = 0;
@@ -520,50 +532,61 @@ WindowsEthernetTap::WindowsEthernetTap(
 	// If there is no device, try to create one
 	bool creatingNewDevice = (_netCfgInstanceId.length() == 0);
 	if (creatingNewDevice) {
-		std::string errm = addNewPersistentTapDevice((std::string(_pathToHelpers) + WINENV.tapDriverPath).c_str());
-		if (errm.length() != 0)
-			throw std::runtime_error(errm);
-
-		// Scan for the new instance by simply looking for taps that weren't originally there...
-		for(DWORD subkeyIndex=0;;++subkeyIndex) {
-			DWORD type;
-			DWORD dataLen;
-			DWORD subkeyNameLen = sizeof(subkeyName);
-			DWORD subkeyClassLen = sizeof(subkeyClass);
-			FILETIME lastWriteTime;
-			if (RegEnumKeyExA(nwAdapters,subkeyIndex,subkeyName,&subkeyNameLen,(DWORD *)0,subkeyClass,&subkeyClassLen,&lastWriteTime) == ERROR_SUCCESS) {
-				type = 0;
-				dataLen = sizeof(data);
-				if (RegGetValueA(nwAdapters,subkeyName,"ComponentId",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS) {
-					data[dataLen] = '\0';
+		for(int getNewAttemptCounter=0;getNewAttemptCounter<2;++getNewAttemptCounter) {
+			for(DWORD subkeyIndex=0;;++subkeyIndex) {
+				DWORD type;
+				DWORD dataLen;
+				DWORD subkeyNameLen = sizeof(subkeyName);
+				DWORD subkeyClassLen = sizeof(subkeyClass);
+				FILETIME lastWriteTime;
+				if (RegEnumKeyExA(nwAdapters,subkeyIndex,subkeyName,&subkeyNameLen,(DWORD *)0,subkeyClass,&subkeyClassLen,&lastWriteTime) == ERROR_SUCCESS) {
+					type = 0;
+					dataLen = sizeof(data);
+					if (RegGetValueA(nwAdapters,subkeyName,"ComponentId",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS) {
+						data[dataLen] = '\0';
 
-					if (WINENV.tapDriverName == data) {
-						type = 0;
-						dataLen = sizeof(data);
-						if (RegGetValueA(nwAdapters,subkeyName,"NetCfgInstanceId",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS) {
-							if (existingDeviceInstances.count(std::string(data,dataLen)) == 0) {
-								RegSetKeyValueA(nwAdapters,subkeyName,"_ZeroTierTapIdentifier",REG_SZ,tag,(DWORD)(strlen(tag)+1));
-								_netCfgInstanceId.assign(data,dataLen);
+						if (WINENV.tapDriverName == data) {
+							type = 0;
+							dataLen = sizeof(data);
+							if ((RegGetValueA(nwAdapters,subkeyName,"_ZeroTierTapIdentifier",RRF_RT_ANY,&type,(PVOID)data,&dataLen) != ERROR_SUCCESS)||(dataLen <= 0)) {
 								type = 0;
 								dataLen = sizeof(data);
-								if (RegGetValueA(nwAdapters,subkeyName,"DeviceInstanceID",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS)
-									_deviceInstanceId.assign(data,dataLen);
-								mySubkeyName = subkeyName;
+								if (RegGetValueA(nwAdapters,subkeyName,"NetCfgInstanceId",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS) {
+									RegSetKeyValueA(nwAdapters,subkeyName,"_ZeroTierTapIdentifier",REG_SZ,tag,(DWORD)(strlen(tag)+1));
 
-								// Disable DHCP by default on newly created devices
-								HKEY tcpIpInterfaces;
-								if (RegOpenKeyExA(HKEY_LOCAL_MACHINE,"SYSTEM\\CurrentControlSet\\services\\Tcpip\\Parameters\\Interfaces",0,KEY_READ|KEY_WRITE,&tcpIpInterfaces) == ERROR_SUCCESS) {
-									DWORD enable = 0;
-									RegSetKeyValueA(tcpIpInterfaces,_netCfgInstanceId.c_str(),"EnableDHCP",REG_DWORD,&enable,sizeof(enable));
-									RegCloseKey(tcpIpInterfaces);
-								}
+									_netCfgInstanceId.assign(data,dataLen);
 
-								break; // found it!
+									type = 0;
+									dataLen = sizeof(data);
+									if (RegGetValueA(nwAdapters,subkeyName,"DeviceInstanceID",RRF_RT_ANY,&type,(PVOID)data,&dataLen) == ERROR_SUCCESS)
+										_deviceInstanceId.assign(data,dataLen);
+
+									mySubkeyName = subkeyName;
+
+									// Disable DHCP by default on new devices
+									HKEY tcpIpInterfaces;
+									if (RegOpenKeyExA(HKEY_LOCAL_MACHINE,"SYSTEM\\CurrentControlSet\\services\\Tcpip\\Parameters\\Interfaces",0,KEY_READ|KEY_WRITE,&tcpIpInterfaces) == ERROR_SUCCESS) {
+										DWORD enable = 0;
+										RegSetKeyValueA(tcpIpInterfaces,_netCfgInstanceId.c_str(),"EnableDHCP",REG_DWORD,&enable,sizeof(enable));
+										RegCloseKey(tcpIpInterfaces);
+									}
+
+									break; // found an unused zttap device
+								}
 							}
 						}
 					}
-				}
-			} else break; // no more keys or error occurred
+				} else break; // no more keys or error occurred
+			}
+
+			if (_netCfgInstanceId.length() > 0) {
+				break; // found an unused zttap device
+			} else {
+				// no unused zttap devices, so create one
+				std::string errm = addNewPersistentTapDevice((std::string(_pathToHelpers) + WINENV.tapDriverPath).c_str());
+				if (errm.length() > 0)
+					throw std::runtime_error(std::string("unable to create new device instance: ")+errm);
+			}
 		}
 	}
 
diff --git a/service/OneService.cpp b/service/OneService.cpp
index 4ee473f8..670d5641 100644
--- a/service/OneService.cpp
+++ b/service/OneService.cpp
@@ -943,6 +943,17 @@ public:
 							friendlyName,
 							StapFrameHandler,
 							(void *)this))).first;
+					} catch (std::exception &exc) {
+#ifdef __WINDOWS__
+						FILE *tapFailLog = fopen((_homePath + ZT_PATH_SEPARATOR_S"port_error_log.txt").c_str(),"a");
+						if (tapFailLog) {
+							fprintf(tapFailLog,"%.16llx: %s"ZT_EOL_S,(unsigned long long)nwid,exc.what());
+							fclose(tapFailLog);
+						}
+#else
+						fprintf(stderr,"ERROR: unable to configure virtual network port: %s"ZT_EOL_S,exc.what());
+#endif
+						return -999;
 					} catch ( ... ) {
 						return -999; // tap init failed
 					}