Upgrade windows crate

[ssrlive]

Hi @TroyNeubauer , I have upgraded the windows crate. In this commit , I add a function verify_wintun_bin which can ensure the wintun.dll has not been tampered with.

[ssrlive]

I think this commit will work. @M0dEx

[TroyNeubauer]

Thanks for putting this together! Overall, I'm not sold on the approach.

I want to keep this crate narrowly focused on wrapping the underlying C library, so this verification functionality would be best left to users's of wintun. Also, it looks like your get_signer_name function is not specific to wintun. Perhaps you could upload a crate with this function so non-wintun users who also need to verify the signer's name can also benefit?

Secondly, I'm confident that windows already verifies driver dll's when they are loaded:

1. Drivers have to be submitted to the Windows Partner Center for certification: https://learn.microsoft.com/en-us/windows-hardware/drivers/dashboard/hardware-submission-create
2. Kernel-mode software must be digitally signed to be loaded on x64-based versions of Windows Vista and later versions of the Windows family of operating systems: https://learn.microsoft.com/en-us/previous-versions/windows/hardware/design/dn653569(v=vs.85)

So its unclear that enforcing that the signer's name is WireGuard LLC adds security on top of what Windows already does.

[TroyNeubauer]

If you still need to bump the version of windows-sys, please submit a seperate PR

[ssrlive]

Mainly because the windows crate changes its api frequently, so I use windows-sys to ensure that our code base will not be changed due to upstream changes in the future. And the windows-sys crate has always been in our dependency library.

If you still need to bump the version of windows-sys, please submit a seperate PR

Is it best for me to delete the subsequent commits? @TroyNeubauer emm, I have finished the job.

[ssrlive]

Can such a situation be considered? The attacker uses the source code of wintun and the signed wintun.sys to create a tampered dll, which can complete the risk attack.

1. Drivers have to be submitted to the Windows Partner Center for certification: https://learn.microsoft.com/en-us/windows-hardware/drivers/dashboard/hardware-submission-create

@TroyNeubauer

[TroyNeubauer]

I'm assuming windows will detect tampered DLL's, as the signature will no longer be valid for a tampered file.

If you can show a proof of concept where a tampered or unsigned dll is loaded successfully through this crate (with a default windows configuration, no bcdedit /set testsigning on or other hacks), then I will consider including verification code in wintun. No need for digging into the machine code and changing the functionality of wintun.dll, modifying a single byte (without corrupting something like a header) should be sufficient for testing windows's signing mechanism.

[ssrlive]

I want to clarify a concept, wintun.dll will be signed twice, The first signature is after the wintun.sys file is compiled. After this file is signed, it will be used as part of the resources of wintun.dll. The second signature is after wintun.dll is compiled and wintun.dll is signed.

Signing of wintun.sys is required, otherwise the driver cannot be loaded. Signing wintun.dll is not necessary because anyone can compile a dll for others to use.

The working process of wintun.dll is as follows: When wintun.dll is loaded, the first thing it does is to release the driver wintun.sys and load the driver. Then do other work.

Since wintun.dll can be used without being signed, an attacker may embed the official wintun.sys file into the wintun.dll file compiled by himself after modifying the source code logic and distribute it.

This is why we need to check the signature of wintun.dll file.

Here is the resources define: https://github.com/WireGuard/wintun/blob/21958630ede7639386f4e69024b937b5834e0e8a/api/resources.rc#L15-L20

[TroyNeubauer]

I see. Thanks for clarifying.

I still think hardcoding hashes is not worth breaking updatability for downstream users. Youre right that the dll could be malicious, however this is a problem faced by anyone who uses dynamic libraries, and therefore a specific solution in this crate is out of scope.

[ssrlive]

Anyway, I hope you merge this PR since I have removed the verify code.

[TroyNeubauer]

Looks good! Can open a new PR to keep the history tidy?