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>
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>
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>
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>
This makes the API parallel:
Wintun*Allocate*SendPacket -> WintunSendPacket
WintunReceivePacket -> Wintun*Release*ReceivePacket
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
- Return pointer to ring buffer with packet data allowing clients to
read/write directly. This eliminates one memcpy().
- Make sending/receiving packets thread-safe.
Signed-off-by: Simon Rozman <simon@rozman.si>
We must not use the process heap, as it is changeable. Client may change
it causing our HeapFree() to use wrong heap.
Signed-off-by: Simon Rozman <simon@rozman.si>
Rather than every client reinvent the art of using the Wintun and its
ring buffers, we offer helper structs and functions to unify and
simplify Wintun usage.
Signed-off-by: Simon Rozman <simon@rozman.si>
