OpenVPN / openvpn

OpenVPN is an open source VPN daemon
http://openvpn.net
Other
10.25k stars 2.92k forks source link

windows client and proxy SOCKS: DeviceIoControl(OVPN_IOCTL_START_VPN) failed: Invalid descriptor(errno=6) #522

Closed githubrn closed 3 months ago

githubrn commented 3 months ago

Describe the bug There is no connection via proxy. Error in the log. Exiting due to fatal error.

To Reproduce using a proxy SOCKS setting without an option windows-driver wintun, ERROR:

DeviceIoControl(OVPN_IOCTL_START_VPN) failed: Invalid descriptor.   (errno=6)
Exiting due to fatal error

Expected behavior connection via proxy SOCKS without error and without modification of settings

Version information (please complete the following information):

Additional context use config:

client
dev tun
proto udp
remote x.x.x.x 5413
resolv-retry infinite
nobind
persist-key
persist-tun
;comp-lzo
verb 9
<ca>
-----BEGIN CERTIFICATE-----
skip
-----END CERTIFICATE-----
</ca>
<cert>
-----BEGIN CERTIFICATE-----
skip
-----END CERTIFICATE-----
</cert>
<key>
-----BEGIN PRIVATE KEY-----
skip
-----END PRIVATE KEY-----
</key>

image logs: 2-6-6-err.txt 2-6-9-err.txt

if you add the option windows-driver wintun connection is ok, there are no errors

client
dev tun
proto udp
remote x.x.x.x 5413
resolv-retry infinite
nobind
persist-key
persist-tun
;comp-lzo
verb 9
windows-driver wintun       <<<<--------------added

log: 2-6-9-drv-wintun-connect-OK.txt

cron2 commented 3 months ago

Mmmmh. This is interesting.

First of all, the problem is that the default driver is now win-dco, which, from the looks of it, is not compatible with using a SOCKS proxy. The error message is not very polite, nor very helpful - this should be discovered earlier, and errored-out.

Using wintun (or --disable-dco or windows-driver tap6) makes packet processing happen in openvpn.exe, which can deal with SOCKS proxy.

So, the actual bug here is "if using win-dco, refuse socks proxy setting or turn off dco automatically". It works if the socks-proxy setting is in the config file, but in your case it's coming in via management

2024-03-13 16:14:54 us=828000 MANAGEMENT: CMD 'proxy SOCKS 127.0.0.1 10808'

... and it seems we do not detect this case correctly.

selvanair commented 3 months ago

So, the actual bug here is "if using win-dco, refuse socks proxy setting or turn off dco automatically". It works if the socks-proxy setting is in the config file, but in your case it's coming in via management

2024-03-13 16:14:54 us=828000 MANAGEMENT: CMD 'proxy SOCKS 127.0.0.1 10808'

If this is from the GUI, may be we could automatically add --disable-dco to the command line when proxy is set via the GUI? This wont be enough in general, but could help.

cron2 commented 3 months ago

Sending --disable-dco down from the GUI would work, but I'm not sure if that is the best possible way, duplicate handling of all (today) DCO-incompatible options in the GUI.

OTOH, is there anything besides proxy setting (which are all not DCO compatible) which can be set in the GUI?

selvanair commented 3 months ago

Apart from --config and some IV variables (IV_GUI_VER and IV_SSO), the only options that the GUI sets are --log --service --auth-retry interact --management --management-query-passwords --management-hold --pull-filter ignore route-method and, optionally, --management-query-proxy.

This is the fist time we find one of those options could cause a conflict.

cron2 commented 3 months ago

I think the only problematic one here is --management-query-proxy as it could lead to setting a "conflicting" proxy later on.

So maybe if proxies are involved make it always send --disable-dco together with --management-query-proxy?

@lstipakov what would you recommend?

selvanair commented 3 months ago

On further thought, the same could be achieved more generally by disabling dco in the core when --management-query-proxy is in the options. We already have dco_check_option() where this could be added. Looks much easier than handling this when proxy command is received from the management.

flichtenheld commented 3 months ago

On further thought, the same could be achieved more generally by disabling dco in the core when --management-query-proxy is in the options. We already have dco_check_option() where this could be added. Looks much easier than handling this when proxy command is received from the management.

Makes sense to me. Can you send a patch or would you prefer someone else take care of it?

selvanair commented 3 months ago

On further thought, the same could be achieved more generally by disabling dco in the core when --management-query-proxy is in the options. We already have dco_check_option() where this could be added. Looks much easier than handling this when proxy command is received from the management.

Makes sense to me. Can you send a patch or would you prefer someone else take care of it?

But then I found this (in 2.6.1 onwards)

commit 42cda5ad9e8542a48385eb2e0b7807773aa341f1
Author: Lev Stipakov <lev@openvpn.net>
Date:   Mon Feb 20 11:06:01 2023 +0200

    Disable DCO if proxy is set via management

    DCO doesn't support proxy and we already disable DCO
    is proxy is set in profile.

    Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Antonio Quartulli <a@unstable.cc>     :)

And indeed the error log has:

2024-03-13 16:14:54 us=828000 MANAGEMENT: CMD 'proxy SOCKS 127.0.0.1 10808'
2024-03-13 16:14:54 us=828000 Proxy set via management, disabling Data Channel Offload.

It seems dco_start_tun() still gets called as tt->windows_driver remains set to WINDOWS_DRIVER_DCO. In tun.c, init_tun_post()

    if (tt->windows_driver == WINDOWS_DRIVER_DCO)
    {
        dco_start_tun(tt);
        return;
    }

This is called from init() without any checks for dco_enabled().

@lstipakov ?

ordex commented 3 months ago

I presume this comes from the fact that the logic in windows is not boolean (DCO on / off), but we have this windows_driver attribute leading the way.

@lstipakov maybe on windows, next to disabling the DCO flag we should turn windows_driver to something else?

lstipakov commented 3 months ago

Just reproduced the problem. Looking into it.

lstipakov commented 3 months ago

Not sure how I tested that commit on Windows - it is a bit too late at that point to disable DCO at least on Windows since we've already done certain DCO-specific adjustments like ip_win32_type etc. The man page says that proxy management command requires --management-query-proxy to be set. I agree with @selvanair proposal to add one more check to dco_check_startup_option(). Since this supersedes the previous fix (which is also not working al least on Window) I'll also remote it.