OpenVPN / tap-windows6

Windows TAP driver (NDIS 6)
Other
785 stars 237 forks source link

Manual MAC address no longer works TAP adaptor 9.24.2 #97

Open TinCanTech opened 4 years ago

TinCanTech commented 4 years ago

While assigning a MAC address via the Windows device manager advance dialogue is still possible, the driver no longer uses that value and always falls back to the value assigned at install. Tested on OpenVPN 2.4.8 Win7 and 10. https://forums.openvpn.net/viewtopic.php?f=1&t=29152

cron2 commented 4 years ago

Hi,

On Thu, Nov 07, 2019 at 03:08:21PM -0800, Richard Bonhomme wrote:

While Assigning a MAC address via the Windows device manager advance dialogue is still possible, the driver no longer uses that value and always falls back to the value assigned at install. Tested on Win7 and 10.

Indeed there were changes in regard to MAC address handling (due to it failing WLHK tests related to MAC address changing)... maybe related to

commit f4f6464cace83ecf6241dbcda0bc8bb62e8a7794 Author: Stephen Stair sgstair@akkit.org Date: Mon Dec 17 21:52:08 2018 -0800

Fix support for changing MAC Address

need to look closer. But not this weekend.

gert

-- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

rednaxes commented 4 years ago

In the last openvpn-install-2.4.8-I602-Win7 package, the problem persists. The driver is installed with the version 9.24.2.601. Reinstalled several times. Removed the driver from the system.

rednaxes commented 4 years ago

In 2.4.9 too.

lygstate commented 3 years ago

how to fixes this issue?

cron2 commented 3 years ago

Hi,

On Mon, Sep 21, 2020 at 07:15:49AM -0700, Yonggang Luo wrote:

how to fixes this issue?

Find someone who understands device driver programming, has an interest in this feature, and can see what broke in the fairly large change set between 2.4.7 and 2.4.8 - which was due to MS testing requirements for driver signing, not because we had fun doing so.

For normal OpenVPN usage, TAP driver MAC address doesn't need to be settable, so it's not of high prio for the OpenVPN developers to fix this.

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

lygstate commented 3 years ago

Where are the tags 2.4.7 and 2.4.8? I can not found the tags in git repository

mattock commented 3 years ago

@lygstate you can check what tap-windows6 version OpenVPN 2.4.x releases have used by checking the history of this file: https://github.com/OpenVPN/openvpn-build/blob/master/windows-nsis/build-complete.vars

lygstate commented 3 years ago

What's the intension of the following change:

 src/OemVista.inf.in | 2 +-
 src/constants.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/OemVista.inf.in b/src/OemVista.inf.in
index c7fd653..ea36f0d 100644
--- a/src/OemVista.inf.in
+++ b/src/OemVista.inf.in
@@ -89,7 +89,7 @@
    AddReg          = @PRODUCT_TAP_WIN_COMPONENT_ID@.reg
    AddReg          = @PRODUCT_TAP_WIN_COMPONENT_ID@.params.reg
    Characteristics = @PRODUCT_TAP_WIN_CHARACTERISTICS@
-   *IfType            = 53  ; IF_TYPE_PROP_VIRTUAL
+   *IfType            = 0x6 ; IF_TYPE_ETHERNET_CSMACD
    *MediaType         = 0x0 ; NdisMedium802_3
    *PhysicalMediaType = 0   ; NdisPhysicalMediumUnspecified

diff --git a/src/constants.h b/src/constants.h
index c862f5b..524d1b7 100644
--- a/src/constants.h
+++ b/src/constants.h
@@ -101,7 +101,7 @@
 #define TAP_CONNECTION_TYPE                NET_IF_CONNECTION_DEDICATED

 // This value must match the *IfType in the driver .inf file
-#define TAP_IFTYPE                         IF_TYPE_PROP_VIRTUAL
+#define TAP_IFTYPE                         IF_TYPE_ETHERNET_CSMACD

 //
 // This is a virtual device, so it can tolerate surprise removal and
lygstate commented 3 years ago

Oh this is the reverse direction, I am trying to revert it

selvanair commented 3 years ago

Hi, On Mon, Sep 21, 2020 at 07:15:49AM -0700, Yonggang Luo wrote: how to fixes this issue? Find someone who understands device driver programming, has an interest in this feature, and can see what broke in the fairly large change set between 2.4.7 and 2.4.8 - which was due to MS testing requirements for driver signing, not because we had fun doing so. For normal OpenVPN usage, TAP driver MAC address doesn't need to be settable, so it's not of high prio for the OpenVPN developers to fix this.

Unfortunately some commits made that time were incomplete/untested. Looks like the change to the way MAC address is read was incomplete as well.

@lygstate Could you please check whether this one fixes it? https://github.com/OpenVPN/tap-windows6/compare/master...selvanair:custom-mac-address You may have to delete exisitng TAP Windows adapters and recreate (not just update the driver). Note that you must use a MAC starting with 02.

cron2 commented 3 years ago

Hi,

On Mon, Sep 21, 2020 at 10:14:54AM -0700, Selva Nair wrote:

Unfortunately some commits made that time were incomplete/untested. Looks like the change to the way MAC address is read was incomplete as well.

Ugh. I assumed that any "we broke it" issues would be caught by the - fairly extensive - Windows driver testing machinery. But it seems it did not cover these, or we did not follow up.

Apologies to everyone :-( - and thanks that you looked into it.

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

lygstate commented 3 years ago

Hi, On Mon, Sep 21, 2020 at 07:15:49AM -0700, Yonggang Luo wrote: how to fixes this issue? Find someone who understands device driver programming, has an interest in this feature, and can see what broke in the fairly large change set between 2.4.7 and 2.4.8 - which was due to MS testing requirements for driver signing, not because we had fun doing so. For normal OpenVPN usage, TAP driver MAC address doesn't need to be settable, so it's not of high prio for the OpenVPN developers to fix this.

Unfortunately some commits made that time were incomplete/untested. Looks like the change to the way MAC address is read was incomplete as well.

@lygstate Could you please check whether this one fixes it? master...selvanair:custom-mac-address You may have to delete exisitng TAP Windows adapters and recreate (not just update the driver). Note that you must use a MAC starting with 02.

Direct modify this file not working, cauase the driver can not be installed.

selvanair commented 3 years ago

The patch modifies the Inf.in file which is used by the build script to generate the Inf file. And then the driver has to be installed using that Inf.

I know modifying the registry will work, but that doesn't test the patch. If you have a setup to apply the patch, build the driver and install it please test. Such a driver will lack signature --- see README.rst on how to install unsigned drivers.

Otherwise @mattock we'll have to make a test installer.

lygstate commented 3 years ago

Hi, On Mon, Sep 21, 2020 at 07:15:49AM -0700, Yonggang Luo wrote: how to fixes this issue? Find someone who understands device driver programming, has an interest in this feature, and can see what broke in the fairly large change set between 2.4.7 and 2.4.8 - which was due to MS testing requirements for driver signing, not because we had fun doing so. For normal OpenVPN usage, TAP driver MAC address doesn't need to be settable, so it's not of high prio for the OpenVPN developers to fix this.

Unfortunately some commits made that time were incomplete/untested. Looks like the change to the way MAC address is read was incomplete as well.

@lygstate Could you please check whether this one fixes it? master...selvanair:custom-mac-address You may have to delete exisitng TAP Windows adapters and recreate (not just update the driver). Note that you must use a MAC starting with 02.

why mac should starting with 02?

selvanair commented 3 years ago

~I can only guess for the reason behind 02. One has to restrict the MAC to an acceptable "locally administered MAC address". Checking for 02 instead of just the U/L bit appears to be a choice made when it was coded (not by me). In principle we could have allowed more without colliding with any OUIs.~

Ignore all that any with bit 2 of byte 1 equal to 1 should work -- starting with 02 was the only one I had ever used and was writing from confused memory..

lygstate commented 3 years ago

The patch modifies the Inf.in file which is used by the build script to generate the Inf file. And then the driver has to be installed using that Inf.

I know modifying the registry will work, but that doesn't test the patch. If you have a setup to apply the patch, build the driver and install it please test. Such a driver will lack signature --- see README.rst on how to install unsigned drivers.

Otherwise @mattock we'll have to make a test installer.

Can we fix the CI to creating a testing driver automatically

selvanair commented 3 years ago

Ignore the comment about starting with 02 -- any valid "locally administered address" should work once the INF is fixed.

mattock commented 3 years ago

We don't have any CI in this repository, though we probably should. Getting evne unsigned drivers as build artifacts for PRs would be very useful.

According to this article it is quite easy to boot Windows 10 temporarily into mode that allows unsigned drivers to be loaded. I can build unsigned drivers and send a link tomorrow.

The official release/signing process for tap-windows6 (MSM + new OpenVPN installer) takes so much time (~4 hours) that I really can't do it except for official releases.

lygstate commented 3 years ago

@mattock unsigned driver are enough, so user doesn't need to build it to verify things, construct a build env are not that easy

mattock commented 3 years ago

@lygstate ok, I'll have unsigned drivers for you guys tomorrow.

selvanair commented 3 years ago

Although my patch fixes reading the MAC set by user, its not enough. I only tested that the MAC of the adapter changes and we get an IP via dhcp, not that the tunnel really "works"..

Previously we used to overwrite the permanent address by what's set by the user. Now we only set the current address which is good, I suppose. But, ProcessARP() in txpath.c compares the src MAC with the permanent address not the current address. This is not going to work.

Did this pass any of the HLK tests?

In dhcp.c the CurrentAddress is checked which is good.

@mattock: hold off on generating the test driver. We may need more fixes -- this time real code changes :)

mattock commented 3 years ago

@selvanair pretty much all the HLK tests passed. Those few that failed were probably not related, but I can't be sure of that.

If getting HLK to work for tap-windows6 was not such a pain it would be good to use it for regression testing.

selvanair commented 3 years ago

@lygstate ok, I'll have unsigned drivers for you guys tomorrow.

Unsigned driver never worked for me (testsigning is on). Doesn't load unless its signed with some certificate which need not be trusted. I use a code-signing cert issued by my own CA and that works. Loading the CA to the trusted roots in Windows makes the install dialog look more legit, but not required.

cron2 commented 3 years ago

Hi,

On Wed, Sep 23, 2020 at 10:23:41AM -0700, Selva Nair wrote:

Although my patch fixes reading the MAC set by user, its not enough. I only tested that the MAC of the adapter changes and we get an IP via dhcp, not that the tunnel really "works"..

Previously we used to overwrite the permanent address by what's set by the user. Now we only set the current address which is good, I suppose. But, ProcessARP() in txpath.c compares the src MAC with the permanent address not the current address. This is not going to work.

Did this pass any of the HLK tests?

To make the HLK test, we needed to run in TAP mode, with a p2p setup (because the HLK test injects lots of "interesting" packets that it wants to see on the other end, including varying src/dst MAC addresses).

So, no, these bits have never been truly tested it seems. Bah :-(

gert

-- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

selvanair commented 3 years ago

Okay, looks like both proxy arp and IPv6 NA are affected. In tun mode, I can ping the interface address but nothing else as would be expected if proxy arp fails. I'll do some more tests as this looks like something we can fix without needing Windows driver foo.. sigh..

cron2 commented 3 years ago

Hi,

On Wed, Sep 23, 2020 at 11:08:01AM -0700, Selva Nair wrote:

Okay, looks like both proxy arp and IPv6 NA are affected. In tun mode, I can ping the interface address but nothing else as would be expected if proxy arp fails. I'll do some more tests as this looks like something we can fix without needing Windows driver foo.. sigh..

Now this is something I feel more comfortable reviewing - it's just C code looking at packet offsets.

I do not have a test rig to build and test the driver right now, so I can only stare and point...

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

selvanair commented 3 years ago

@mattock unsigned driver are enough, so user doesn't need to build it to verify things, construct a build env are not that easy

@lygstate Its simpler than it looks. Assuming you have python installed, download EWDK iso and mount it as, say, drive E: Then cd to tap-windows6 directory and edit paths.py to have EWDK = E:. That's all what it takes.

To build unsigned drivers, run "python buidtap.py -b"

cron2 commented 3 years ago

Hi,

patch has been merged (thanks, Selva) - @mattock: can you bump the driver version and build a new driver for us, please?

gert

-- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

mattock commented 3 years ago

@cron2 sure. Should we go the "unsigned test driver first" -> "test" -> "release driver second" route? Or are the fixes "obviously correct" and/or tested? :smile:

selvanair commented 3 years ago

@mattock: I've mildly tested using a self-signed driver on a Windows 10, and the two patches merged cant possibly break anything. But always better to get more test reports from users as driver programming is not my thing.

The driver I built is available if there is interest (will have to be installed using tapinstall or devcon) (oops, keyboard gone wild...)

davidelbert commented 3 years ago

Has this fix been merged? How can I get a copy of the updated driver and assign the MAC to a TAP adaptor?

cron2 commented 3 years ago

Hi,

On Wed, May 19, 2021 at 10:32:25AM -0700, David Elbert wrote:

Has this fix been merged? How can I get a copy of the updated driver and assign the MAC to a TAP adaptor?

Looking at the tap-windows6 repo, I see a version bump to 9.24.6.601 happen after the merging of these MAC-related patches.

Actually, 9.24.5.601 should have it, and with 2.5.0, we released 9.24.6.601 I think.

Anyway -> if you have the most recent 2.4.x or 2.5.x OpenVPN installer, you have the latest TAP driver, and this should work.

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany @.***

davidelbert commented 3 years ago

Thanks @cron2. I do have the latest, but am not able to set the MAC address of the tap. Perhaps I'm doing something wrong. I'll dig into that and see if I can figure it out.

TinCanTech commented 3 years ago

I just tested on Win7 with https://build.openvpn.net/downloads/snapshots/OpenVPN-2.6-dco-preview-x86.msi and successfully set the TAP adapter MAC to AA-BB-FF-AA-BB-01 (Must be - not :)

mattock commented 3 years ago

I want to add that 2.4.x has 9.24.2. There has not been a pressing need (e.g. security fix) to rebuild the NSI-based tap-windows installer, which is an effort in itself.

IB-Rahn commented 1 year ago

I am running into some problems regarding setting the MAC Address on the newest version (9.24.6.601) which comes with OpenVPN 2.6.1. I am able to change the MAC address but only with certain values. For example: I want to use 00-1D-60-47-B6-B8 but then it just uses a default address. If I change the first two hex values to 12-1D-60-47-B6-B8 it works. Anyone got a hint why that's happening?

rednaxes commented 11 months ago

The mac address on the tap adapter does not change. I want to set the mac address on the tap adapter the same as on the PC network card. the latest version on which it worked is 2.4.7. At the moment, in version 2.6.6 with the 9.26.0.0 driver, it turned out only after changing the first two bytes of the mac pc. This is not a complete solution.The problem remains.

selvanair commented 11 months ago

Since version around 9.22, only addresses that are unicast and locally administered (i.e, I/G bit = 0 and U/L bit = 1) are accepted. This means the low nibble of the first byte should be 2, 6, A or E. I do not know the reason for restricting to "locally administered" (private) addresses.

What is the use case?

cron2 commented 11 months ago

Hi,

On Sat, Sep 16, 2023 at 07:59:30AM -0700, Selva Nair wrote:

I do not know the reason for restricting to "locally administered" (private) addresses.

Neither do I, but given that this was added in

commit f4f6464cace83ecf6241dbcda0bc8bb62e8a7794 Author: Stephen Stair @.***> Date: Mon Dec 17 21:52:08 2018 -0800

Fix support for changing MAC Address

and Stephen did lots of the work to make the TAP-driver pass the microsoft driver tests - maybe it was necessary for that, or maybe the manual said "it needs to do that way", but nothing in the tests actually checks...

We could ask :-)

gert

selvanair commented 11 months ago

At the point where that change was made, custom MAC address did not work due to some bugs that we fixed later:

commit 52e04b5202a3a5bd78a2bffe09a348cb44dc8265 Author: Selva Nair selva.nair@gmail.com Date: Wed Sep 23 15:56:58 2020 -0400

So, I wonder whether this restriction helped HLK tests at all.

rednaxes commented 11 months ago

I use openvpn to connect remote computers via dev tap. On the tap adapter, I expose the mac the same as on the PC network card. The user decided to take the PC home connects to Openvpn and gets the same ip address as in the office. It is convenient for work. I have not met a network card where you cannot install an arbitrary mac. What is this strange restriction on openvpn?

brandonros commented 9 months ago

At the point where that change was made, custom MAC address did not work due to some bugs that we fixed later:

commit 52e04b5 Author: Selva Nair selva.nair@gmail.com Date: Wed Sep 23 15:56:58 2020 -0400

So, I wonder whether this restriction helped HLK tests at all.

This bug says it was fixed in 2020 but the latest release 9.26.0 from 2023-04-27 has a "Physical Address" (from ipconfig /all) that does not match whatever is changed in Device Manager for "MAC Address"

Is this basically "blocked"/not allowed anymore because of driver signing? I can't tell if you aren't allowed to change the MAC at all, or you are but it has to start with 00-FF or something.

edit: downgrading to 2.4.7 works https://swupdate.openvpn.org/community/releases/openvpn-install-2.4.7-I607-Win10.exe

when you configure the MAC address, you must put - (not : and not just the hex characters without separator)