OpenVPN / tap-windows6

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

fixed bug when stripping 802.1Q tag #117

Closed svkers closed 4 years ago

svkers commented 4 years ago

PR fixes issue #116.

Note: The code has neither been compiled nor tested.

selvanair commented 4 years ago

This looks right and builds without error but I have no setup to test it. @mattock: can we make a signed binary so that someone may volunteer to do a test?

cron2 commented 4 years ago

Hi,

On Tue, Jul 28, 2020 at 06:00:49AM -0700, Selva Nair wrote:

This looks right and builds without error but I have no setup to test it. @mattock: can we make a signed binary so that someone may volunteer to do a test?

mattock is still on vacation, will be back Monday

(thanks for the review)

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

TinCanTech commented 4 years ago

I can build openvpn.exe with the patch but testing VLANs may take a lot longer ..

cron2 commented 4 years ago

Hi,

On Tue, Jul 28, 2020 at 10:45:13AM -0700, TinCanTech wrote:

I can build openvpn.exe with the patch but testing VLANs may take a lot longer ..

You'd need to build a tap driver.

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

TinCanTech commented 4 years ago

You'd need to build a tap driver

I build the entire Windows installer ;-) Patched TAP driver included.

selvanair commented 4 years ago

Easier to test with a properly signed driver. I couldn't get the unsigned driver to work by enabling test-signing on any of the Windows 10 machines I have access to.

cron2 commented 4 years ago

Hi,

On Tue, Jul 28, 2020 at 02:50:06PM -0700, TinCanTech wrote:

You'd need to build a tap driver I build the entire Windows installer ;-)

The normal build script for the windows installer just downloads the already-signed tap9 bundle from mattock. Doesn't build the driver.

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 4 years ago

Sure, I love building and signing tap-windows6 drivers :smile:. I think I'll do that after 2.5-beta1 and produce a tap-windows6 MSM ("Merge module") instead of the old-school tap-windows6 NSIS installer. I put this on my TODO.

selvanair commented 4 years ago

Sure, I love building and signing tap-windows6 drivers 😄. I think I'll do that after 2.5-beta1 and produce a tap-windows6 MSM ("Merge module") instead of the old-school tap-windows6 NSIS installer. I put this on my TODO.

@mattock Was this included in beta1? If not can we include this in beta2 and get it tested?

mattock commented 4 years ago

@selva no, this was not included in beta1. The question I guess is: is this safe enough to put into a public release without providing a test installer first?

cron2 commented 4 years ago

Hi,

On Tue, Aug 25, 2020 at 09:23:28PM -0700, Samuli SeppÀnen wrote:

@selva no, this was not included in beta1. The question I guess is: is this safe enough to put into a public release without providing a test installer first?

As this is an obviously-correct bugfix for a corner case, yes.

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 4 years ago

I take "obviously correct fix" as ACK.

cron2 commented 4 years ago

Hi,

On Wed, Aug 26, 2020 at 12:06:47PM -0700, Samuli SeppÀnen wrote:

I take "obviously correct fix" as ACK.

It looks very correct, so, yes, but I wanted to get this tested :-)

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