On 08/09/2020 21:01, Vladislav Grishenko wrote: > Hi David, > >> -----Original Message----- >> From: David Sommerseth <open...@sf.lists.topphemmelig.net> >> Sent: Tuesday, September 8, 2020 6:23 PM >> To: Vladislav Grishenko <themi...@yandex-team.ru>; openvpn- >> de...@lists.sourceforge.net >> Subject: Re: [Openvpn-devel] [PATCH] Fix --remote protocol can't be set >> without >> port argument >> >> On 03/09/2020 13:44, Vladislav Grishenko wrote: >>> According client-options.rst additional argumets ``port`` and >>> ``proto`` are both optional and it's allowed to have port absent and >>> protocol >> set: >>> --remote args >>> Examples: >>> remote server.example.net tcp >>> >>> But when protocol is set without preceeding port argument, it is being >>> misparsed as a port with subsequent error: >>> RESOLVE: Cannot resolve host address: server.example.net:tcp >>> (Servname not supported for ai_socktype) >>> >>> Since protocol names are predefined and don't match service names, fix >>> this behavior by checking second argument for valid protocol first. >>> >> Uhm ... I'm leaning towards a NAK to this patch and would rather suggest >> updating the man page to be correct. This is a mistake from my side when >> converting the man page to .rst files. The example should be: >> >> remote server.example.net 1194 tcp >> > > Hum. Initially all variants were supported, by checking numeric port and > taking it as proto, if not numeric. Later port became string servname and > optional logic was lost.
The man page history disagrees ... even though, it doesn't say this order is explicit. 2.4 - <https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage#lbAG> 2.3 - <https://community.openvpn.net/openvpn/wiki/Openvpn23ManPage#lbAG> 2.2 - <https://community.openvpn.net/openvpn/wiki/Openvpn22ManPage> 2.1 - <https://community.openvpn.net/openvpn/wiki/Openvpn21ManPage#lbAG> > Man pages still has all of them since that time: >> remote server.exmaple.net >> remote server.exmaple.net 1194 >> remote server.exmaple.net tcp These lines comes from this commit: <https://github.com/OpenVPN/openvpn/commit/f500c49c8e0a77ce665b11f6adbea4029cf3b85f> Where the last line (missing port number) is incorrect. I also don't see any commit changing the "remote" parsing in options.c in the way you describe. This section has just few changes over the years, but even back to changes 7-12 years ago, the second argument has always been processed as a port number and the third one protocol. Both causing an error if it is not a valid value. So putting protocol in the second argument would trigger an error in 2.1, 2.2, 2.3 and 2.4 as far as I can see. > > At first look there is no need for proto w/o port set, why it can be > important? With support of server host & port discovery (DNS SRV RFC 2782), > port info is not required, only host and protocol make sense. So I though > about this one little step forward toward ~consistent syntax. Does it makes > any sense? I don't see why OpenVPN's --remote parsing needs to comply with DNS SRV RFC standards. How is that related at all? I know we have patches for adding DNS SRV query support, but that doesn't imply passing this information via add_option(), does it? If you use --remote vpn.example.com --proto tcp .... that will already work. What will not work is --remote vpn.example.com tcp Also, this will work in config files: ------------------------------- port 5000 proto udp <connection> remote vpn1.example.com </connection> <connetion> remote vpn2.example.com proto tcp </connection> ------------------------------- All of these use cases makes sense, as it depends on a pre-set value. What does not make sense to me is to muddy a pretty clearly defined argument order which has been the standard since the beginning of OpenVPN. -- kind regards, David Sommerseth OpenVPN Inc
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
