aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* driver: workaround SDV failure with code analysissr/sdvSimon Rozman2021-07-122-7/+9
| | | | | | | | | | | | | | SDV is using own CL.EXE which returns error code 2 when code analysis is turned on. However, we need code analysis results for DVL. While we could use a new "ReleaseSDV" configuration, we don't really require limited code analysis in Release builds, as long as we address all full code analysis warnings in Debug builds. To make DVL happier, an intermediate Release build was injected with code analysis turned on. Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: cleanup project fileSimon Rozman2021-07-121-155/+132
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: remove excessive media connection reporting on adapter initSimon Rozman2021-07-121-4/+1
| | | | | | | | The initial adapter state (including media connection) is provided by the NDIS_MINIPORT_ADAPTER_GENERAL_ATTRIBUTES. Additional NdisMIndicateStatusEx() call seems excessive. Signed-off-by: Simon Rozman <simon@rozman.si>
* api: use SuggestedInstanceId instead of NetSetupAnticipatedInstanceIdJason A. Donenfeld2021-07-097-466/+158
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All was well with NetSetupAnticipatedInstanceId, until a bug crept into recent Windows builds that caused old GUIDs not to be properly removed, resulting in subsequent adapter creations to fail, because NetSetup AnticipatedInstanceId considers it fatal when the target GUID already exists, even if in diminished form. The initial solution was to detect cruft, and then steal a TrustedInstaller token and sleuth around the registry cleaning things up. The horror! Uncomfortable with this, I reopened IDA and had a look around with fresh eyes, three years after the original discovery of NetSetupAnticipated InstanceId. There, I found some interesting behavior in NetSetupSvcDeviceManager::InstallNetworkInterfaces, which amounts to something like: if (IsSet("RetiredNetCfgInstanceId") { if (IsSet("NetSetupAnticipatedInstanceId") DeleteAdapter(GetValue("RetiredNetCfgInstanceId")); else Set("NetSetupAnticipatedInstanceId", GetValue("RetiredNetCfgInstanceId")); Delete("RetiredNetCfgInstanceId"); } CreateAdapter = TRUE; if (IsSet("NetSetupAnticipatedInstanceId")) { Guid = GetValue("NetSetupAnticipatedInstanceId"); if (AdapterAlreadyExists(Guid)) CreateAdapter = FALSE; else SetGuidOfNewAdapter(Guid); Delete("NetSetupAnticipatedInstanceId"); } else if (IsSet("SuggestedInstanceId")) { Guid = GetValue("SuggestedInstanceId"); if (!AdapterAlreadyExists(Guid)) SetGuidOfNewAdapter(Guid); Delete("SuggestedInstanceId"); } Thus, one appealing strategy would be to set both NetSetupAnticipated InstanceId and RetiredInstanceId to the same value, and let the service handle deleting the old one for us before creating the new one. However, the cleanup of the old adapter winds up being quasi- asynchronous, and thus we still wind up in the CreateAdapter = FALSE case. So, the remaining strategy is to simply use SuggestedInstanceId instead. This has the behavior that if there's an adapter already in use, it'll use a new random GUID. The result is that adapter creation won't fail. That's not great, but the docs have always made it clear that "requested" is a best-effort sort of thing. Plus, hopefully the creation of the new adapter will help nudge the bug a bit and cleanup the old cruft. In some ways, transitioning from our old strategy of "cudgel the registry until we get the GUID we want" to "ask politely and accept no for an answer" is a disappointing regression in functionality. But it also means we don't need to keep crazy token stealing code around, or fish around in the registry dangerously. This probably also increases the likelihood that an adapter will be created during edge cases, which means fewer errors for users, which could be a good thing. On the downside, we have the perpetual tensions caused by a system that now "fails open" instead of "fails closed". But so it goes in Windows land. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: log instance id when object file name is emptyJason A. Donenfeld2021-07-081-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: print correct last error when failingJason A. Donenfeld2021-07-081-1/+1
| | | | | | | Prior to the conversion, LastError is ERROR_SUCCESS, so move the logging to be after the conversion. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* version: bump0.12Jason A. Donenfeld2021-06-251-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: don't auto-elevateJason A. Donenfeld2021-06-258-161/+69
| | | | | | | There's no longer a need to do this for every API call. This only exists now for the pnp guid reuse workaround hack. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: hard code security descriptor bytesJason A. Donenfeld2021-06-251-18/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is compatible with old Windows. Generated by: #include <stdio.h> #include <windows.h> #include <sddl.h> int main(int argc, char *argv[]) { PSECURITY_DESCRIPTOR sd; ULONG sd_len; if (!ConvertStringSecurityDescriptorToSecurityDescriptorA("O:SYD:P(A;;FA;;;SY)(A;;FA;;;BA)S:(ML;;NWNRNX;;;HI)", SDDL_REVISION_1, &sd, &sd_len)) return 1; for (ULONG i = 0; i < sd_len; ++i) printf("0x%02x%s%s", ((unsigned char *)sd)[i], i == sd_len - 1 ? "" : ",", i == sd_len -1 || i % 8 == 7 ? "\n": " "); return 0; } This can be easily checked from kernel space with this ugly snippet: UNICODE_STRING Func; RtlInitUnicodeString(&Func, L"SeConvertSecurityDescriptorToStringSecurityDescriptor"); WCHAR *Str = NULL; ((NTSTATUS(NTAPI *)(PSECURITY_DESCRIPTOR, DWORD, DWORD, WCHAR **, DWORD *))MmGetSystemRoutineAddress(&Func))( TunDispatchSecurityDescriptor, 1, 0x14, &Str, NULL); DbgPrint("Did it work? %ls\n", Str); Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: build security descriptor from sddlJason A. Donenfeld2021-06-253-100/+10
| | | | | | This is a bit easier to read. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: allow admins but require high integrity labelJason A. Donenfeld2021-06-253-8/+70
| | | | | | Might be more reasonable. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: specify pnplockdown in infJason A. Donenfeld2021-06-251-0/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: formatJason A. Donenfeld2021-06-251-5/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: only mark GUID as in-use if Status != NotPresentJason A. Donenfeld2021-05-111-13/+16
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* version: bump0.11Jason A. Donenfeld2021-05-101-2/+2
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: do not assume aligned addresses when allocating MDLsJason A. Donenfeld2021-05-101-2/+4
| | | | | | | | | | | IoAllocateMdl allocates a different size structure depending on the bottom in-page bits of the address. By passing null, it assumes that the address is aligned within the page, which it might not be. Fix this by passing the eventual virtual address to the allocation function so that the right amount is always allocated. Reported-by: Oleksandr Muzychuk <oleksandr.muzychuk@apriorit.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: move init-only functions into INIT segmentSimon Rozman2021-05-101-0/+9
| | | | | Reference: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/writing-a-driverentry-routine Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: fix memory leak on pre-Windows 7Simon Rozman2021-05-101-3/+2
| | | | | | | Should NDIS version check fail the DriverEntry() bailed out without releasing the TunDispatchSecurityDescriptor. Signed-off-by: Simon Rozman <simon@rozman.si>
* driver: cleanup unused DBG defineSimon Rozman2021-05-101-6/+0
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* version: bump0.10.5Jason A. Donenfeld2021-05-101-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: use simpler problem status checkingJason A. Donenfeld2021-05-101-12/+13
| | | | | | | | This reworks commit e51b49604b5d00a641b698e7c40d4d46a06644c9. Link: https://www.reddit.com/r/WireGuard/comments/n6yocf/unable_to_create_wintun_on_windows_7_laptop_with/ Reported-by: Alirz Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: check that GUID is valid before assuming it's in useJason A. Donenfeld2021-05-101-2/+7
| | | | | | | | | ROOT/NET/000X could have been claimed by a different driver, so we want to double check. Link: https://lists.zx2c4.com/pipermail/wireguard/2021-May/006716.html Reported-by: Piotr Sobczak <piotrs@glosol.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: discourage UaF on teardownJason A. Donenfeld2021-05-101-1/+0
| | | | | | | | | | | | | | | | | | | | | | While it does make sense to make readers unblock by setting the read event on teardown, this is something that consumers of the library should do _before_ calling EndSession, not something that makes sense for the library to do itself. The reason is that, in the hypothetical case in which this makes sense, immediately after unblocking the reader via SetEvent, the function goes on to free all of the memory that that reader might want to use. So, rather, the proper shutdown flow is from the application side, and looks like: Closing = true; SetEvent(WintunGetReadWaitEvent()); WaitForReadersToReturn(); WintunEndSession(); Alternatively, rather than using WaitForSingleObject on the read event, consumers can WaitForMultipleObjects and include a shutdown event, which is what the example code does. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* version: bump0.10.4Jason A. Donenfeld2021-05-051-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: skip requested GUID if !win10Jason A. Donenfeld2021-05-051-5/+12
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: clean up NetSetup2 GUIDsJason A. Donenfeld2021-05-055-0/+268
| | | | | | | | Recent versions of Windows fail to tidy up, causing issues when reusing GUIDs. Check to see if a GUID might be orphaned, and forcibly clear out the registry state if so. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: don't pass bogus previous buffer size argumentJason A. Donenfeld2021-05-051-2/+2
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: don't return ERROR_SUCCESS if adapter doesn't come upJason A. Donenfeld2021-05-041-4/+23
| | | | | | | | Otherwise we fail to remove the zombie adapter, and then the problem repeats itself. Note that this is part of a larger issue of clearing out bad GUIDs in NetSetup2 on recent Windows builds Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* project: fix typo in .editorconfigSimon Rozman2021-05-041-1/+1
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* version: bump0.10.3Jason A. Donenfeld2021-04-131-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* vs: put .pdb files in the intermediate foldersSimon Rozman2021-04-131-0/+3
| | | | | | | | | | | | Wondering, why WinDbg is refusing to load symbols for wintun.sys recently... By default, building puts .pdb files to the output folder. Next to the final binaries: wintun.sys, wintun.dll, example.exe... Wait?! But the wintun.pdb from wintun.dll overwrites the wintun.pdb from wintun.sys then. Signed-off-by: Simon Rozman <simon@rozman.si>
* vs: remove api->wintun project dependencySimon Rozman2021-04-131-3/+0
| | | | | | | | | | | | | | | | | | | | The time stamping with Inf2Cat never makes the wintun output really "up- to-date". This keeps triggering wintun project rebuilds over and over again. Including all the projects depending on wintun. Thou, the api project really depends on the driver built by wintun project, those needless driver rebuilds are utterly annoying. To be correct, the amd64 api project also depends on the arm64 api project - a dependency which cannot be described using .sln. So, one way or another, a developer must build projects inside the .sln in a specific order. Another solution would be to split the solution file (pun intended). One .sln for driver, another for the api and example projects. Then open the one the developer is focused on. Signed-off-by: Simon Rozman <simon@rozman.si>
* api: log Windows error message too when creating folder or file failsSimon Rozman2021-04-131-2/+2
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* api: fix fallback log line printf templateSimon Rozman2021-04-101-1/+1
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* Fix © in resourcesSimon Rozman2021-03-192-0/+4
| | | | | | | | The \xa9 is © on Windows-125x code pages. When Wintun was compiled on a Windows computer using UTF-8 as default code page (for "non-Unicode" programs), the Copyright notice in resources was wrong. Signed-off-by: Simon Rozman <simon@rozman.si>
* project: add support for intermediate versioningSimon Rozman2021-03-194-13/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | While the Wintun driver is typically released only at <major>.<minor> milestones, the wintun.dll did see some intermediate releases. To make MSI logic correctly decide to upgrade local wintun.dll file, the version inscribed in wintun.dll file resources should increment in those intermediate releases. MSI ignores file timestamps. One could use REINSTALLMODE=emus Installer property to force copying wintun.dll when its version doesn't change. But REINSTALLMODE applies to all files in the MSI session and would be an additional requirement when authoring MSI packages with wintun.dll. Bumping only the final ZIP filename version is not sufficient. Therefore, a <major>.<minor> or <major>.<minor>.<build> versioning is introduced. Furthermore, we no longer distinguish between WintunVersion and WintunVersionStr. All our releases used strictly numeric <major>.<minor> notation, making WintunVersion and WintunVersionStr always the same. When the driver didn't change, just bump the version in wintun.proj and run `msbuild wintun.proj /t:Zip` to rebuild the wintun.dll and make the new ZIP file. Signed-off-by: Simon Rozman <simon@rozman.si>
* api: make .h filenames lowercase for building with MinGW on LinuxSimon Rozman2021-03-161-2/+2
| | | | | | | | MinGW supplies all Windows header files using lowercase filenames. This makes some of the #include lines in wintun.h fail to resolve the .h files correctly on a case-sensitive filesystem. Signed-off-by: Simon Rozman <simon@rozman.si>
* api: elevate to SYSTEM in WintunEnumAdapters()Simon Rozman2021-03-082-3/+21
| | | | | | | | | | | | | | The WintunEnumAdapters() requires namespace mutex. However, NamespaceTakePoolMutex() works as SYSTEM user only. WireGuard is using the WintunEnumAdapters() in its manager service to cleanup stale adapters. As the WireGuard manager service is running as SYSTEM, this requirement was not apparent before. This commit also extends the example project to list its existing adapters at start. Signed-off-by: Simon Rozman <simon@rozman.si>
* api: upgrade logging0.10.2Simon Rozman2021-02-1610-177/+431
| | | | | | | Log runtime information to quickly check whether the values are sane when analyzing error logs sent in by users. Signed-off-by: Simon Rozman <simon@rozman.si>
* api: tighten function parameter code analysis annotationsSimon Rozman2021-02-041-6/+5
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* api: truncate long log lines with …Simon Rozman2021-02-041-3/+5
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* api: unify NetCfgInstanceId registry retrievalSimon Rozman2021-02-041-26/+22
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* README: document the Windows SDK version requirementSimon Rozman2021-02-041-1/+1
| | | | Signed-off-by: Simon Rozman <simon@rozman.si>
* api: ensure that device object exists before returning from open/create0.10.1Jason A. Donenfeld2021-02-031-13/+62
| | | | | | | | | | | | | | | | | Some users are seeing errors like this after rebooting from Windows Update: 2021-01-28 18:39:45.220197: [TUN] Creating Wintun interface 2021-01-28 18:39:49.420116: [TUN] [Wintun] CreateAdapter: Creating adapter 2021-01-28 18:39:53.704007: [TUN] [Wintun] OpenDeviceObject: Failed to connect to adapter: The system cannot find the path specified. (Code 0x00000003) 2021-01-28 18:39:53.704007: [TUN] [Wintun] WintunStartSession: Failed to open adapter device object 2021-01-28 18:39:54.097037: [TUN] Unable to create Wintun interface: Error starting session: The system cannot find the path specified. It appears that creation of the device object file might happen asynchronously, so this commit polls for it. Reported-by: Artem Kuroptev <artem@kuroptev.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: bump copyrightJason A. Donenfeld2021-01-3030-32/+32
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: use custom devpkey for poolJason A. Donenfeld2021-01-301-2/+28
| | | | | | | | | | It seems like the friendly name is still getting reset sometimes. Rather than swimming upstream, it turns out we can just use a custom devpkey that, according to msdn, is respected. https://docs.microsoft.com/en-us/windows-hardware/drivers/install/creating-custom-device-properties Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* api: close private namespace when unloading DLLJason A. Donenfeld2020-12-171-12/+16
| | | | | | | | | | | | | | Prior, people making calls to LoadLibrary/FreeLibrary would experience a failure to create or open adapters, because the private namespace was already loaded into the process and not cleaned up on DLL detachment. While this pattern is probably a misuse of the library, we should still support cleaning that up. This commit makes the right calls to free the boundary descriptor and close the private namespace. It does not, however, destroy the private namespace using the flag on the second parameter, in case of races with other processes. Reported-by: Brad Spencer <bspencer@blackberry.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* version: bump0.10Jason A. Donenfeld2020-12-161-2/+2
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* project: license prebuilt binaries more permissivelyJason A. Donenfeld2020-12-163-2/+86
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* driver: use IoAllocateMdl without being too cleverJason A. Donenfeld2020-12-151-8/+17
| | | | | | | | | | | | Windows 7 doesn't like our trick of sticking MDLs into the NBL context area. So we do things more traditionally, by allocating an MDL with IoAllocateMdl and freeing it with IoFreeMdl. This means that we have to keep track of the MDL between allocation and free time, and we don't have any more miniport reserved pointers left in the NBL. So instead we walk the MdlChain field of the first NB, and free the one that has an address living inside of the non-partial MDL. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>