Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
Hi, On Wed, Jan 11, 2023 at 01:42:31AM +0500, Vladislav Grishenko wrote: > Hi, sure, will do. > Yes, I???ve noticed undesired code dup in v14 and have fixed everything found > in v15 rebase, same will be rechecked in v16 of course. Did you find time to have a look into this? 2.6.0 is out, but since there is demand for the SRV patch, we've agreed to do an exceptional feature merge for 2.6.1 for SRV and dynamic tls-crypt - but that means "we need to have a fully reviewed v16"... 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 g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
Hi, sure, will do.
Yes, I’ve noticed undesired code dup in v14 and have fixed everything found
in v15 rebase, same will be rechecked in v16 of course.
Thanks!
Ср, 11 янв. 2023 г. в 01:05, Gert Doering :
> Hi,
>
> On Thu, Dec 29, 2022 at 12:27:46PM +0500, Vladislav Grishenko wrote:
> > client will move on to the next connection entry.
> >
> > v15:
> > rebase to master (Dec 2022)
> > add optional port argument to --remote and --remote-srv usage message
> > fix --proto option coexisting with --remote-srv
> > fix --nobind option coexisting with --remote-srv
> > fix options postprocess mutation lost in v13/v14
> > recover --mtu-test handling with --remote-srv
> > use explicit srv resolve stub for openbsd for the future
> > fix comments
>
> Getting close... but, alas, we need another rebase - the signal handling
> fixes from Selva cause conflicts. Can you do a v16, please?
>
> Also, please have a very close look at the code now - it looks like
> the previous rebase is now creating quite a bit of (undesired!) code
> duplication. For example, this new hunk:
>
> +static bool
> +options_postprocess_verify_ce_proto(const struct options *options,
> +const struct connection_entry *ce)
> +{
> +int msglevel = M_WARN|M_NOPREFIX|M_OPTERR;
> +
> +/*
> + * Sanity check on --local, --remote, and --ifconfig
> + */
> +
> +if (proto_is_net(ce->proto)
> +&& string_defined_equal(ce->local, ce->remote)
> +&& string_defined_equal(ce->local_port, ce->remote_port))
> +{
> +msg(msglevel, "--remote and --local addresses are the same");
> +return false;
> +}
> +
> +if (string_defined_equal(ce->remote, options->ifconfig_local)
> +|| string_defined_equal(ce->remote,
> options->ifconfig_remote_netmask))
> +{
> +msg(msglevel,
> +"--local and --remote addresses must be distinct from
> --ifconfig "
> +"addresses");
> +return false;
> +}
>
>
> ... these checks are existing today, in options_postprocess_verify_ce(),
> but your patch is not *moving* them to the new function, but *duplicating*
> them. This can happen if "git --rebase" gets confused by too many
> unrelated changes, but is not the desired end state.
>
> 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
> g...@greenie.muc.de
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
--
Best regards, Vladislav Grishenko
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
Hi,
On Thu, Dec 29, 2022 at 12:27:46PM +0500, Vladislav Grishenko wrote:
> client will move on to the next connection entry.
>
> v15:
> rebase to master (Dec 2022)
> add optional port argument to --remote and --remote-srv usage message
> fix --proto option coexisting with --remote-srv
> fix --nobind option coexisting with --remote-srv
> fix options postprocess mutation lost in v13/v14
> recover --mtu-test handling with --remote-srv
> use explicit srv resolve stub for openbsd for the future
> fix comments
Getting close... but, alas, we need another rebase - the signal handling
fixes from Selva cause conflicts. Can you do a v16, please?
Also, please have a very close look at the code now - it looks like
the previous rebase is now creating quite a bit of (undesired!) code
duplication. For example, this new hunk:
+static bool
+options_postprocess_verify_ce_proto(const struct options *options,
+const struct connection_entry *ce)
+{
+int msglevel = M_WARN|M_NOPREFIX|M_OPTERR;
+
+/*
+ * Sanity check on --local, --remote, and --ifconfig
+ */
+
+if (proto_is_net(ce->proto)
+&& string_defined_equal(ce->local, ce->remote)
+&& string_defined_equal(ce->local_port, ce->remote_port))
+{
+msg(msglevel, "--remote and --local addresses are the same");
+return false;
+}
+
+if (string_defined_equal(ce->remote, options->ifconfig_local)
+|| string_defined_equal(ce->remote, options->ifconfig_remote_netmask))
+{
+msg(msglevel,
+"--local and --remote addresses must be distinct from --ifconfig "
+"addresses");
+return false;
+}
... these checks are existing today, in options_postprocess_verify_ce(),
but your patch is not *moving* them to the new function, but *duplicating*
them. This can happen if "git --rebase" gets confused by too many
unrelated changes, but is not the desired end state.
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 g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
On Thu, Dec 29, 2022 at 11:29:29AM +0100, Frank Lichtenheld wrote: > On Thu, Dec 29, 2022 at 12:27:46PM +0500, Vladislav Grishenko wrote: > [...] > > v15: > > rebase to master (Dec 2022) > > add optional port argument to --remote and --remote-srv usage message > > fix --proto option coexisting with --remote-srv > > fix --nobind option coexisting with --remote-srv > > fix options postprocess mutation lost in v13/v14 > > recover --mtu-test handling with --remote-srv > > use explicit srv resolve stub for openbsd for the future > > fix comments > > I don't really have time today to review this completely, but a quick test > seems to confirm the issue I identified last time is indeed fixed. Tested > by replacing --remote in t_client tests 1 and 2 with --remote-srv > lichtenheld.net > > SRV setup for lichtenheld.net: > _openvpn._tcp 60 IN SRV 0 0 51194 conn-test-server.openvpn.org. > _openvpn._udp 60 IN SRV 0 0 51194 conn-test-server.openvpn.org. Also put this through the buildbot tests. No build/test failures. Regards, -- Frank Lichtenheld ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
On Thu, Dec 29, 2022 at 12:27:46PM +0500, Vladislav Grishenko wrote: [...] > v15: > rebase to master (Dec 2022) > add optional port argument to --remote and --remote-srv usage message > fix --proto option coexisting with --remote-srv > fix --nobind option coexisting with --remote-srv > fix options postprocess mutation lost in v13/v14 > recover --mtu-test handling with --remote-srv > use explicit srv resolve stub for openbsd for the future > fix comments I don't really have time today to review this completely, but a quick test seems to confirm the issue I identified last time is indeed fixed. Tested by replacing --remote in t_client tests 1 and 2 with --remote-srv lichtenheld.net SRV setup for lichtenheld.net: _openvpn._tcp 60 IN SRV 0 0 51194 conn-test-server.openvpn.org. _openvpn._udp 60 IN SRV 0 0 51194 conn-test-server.openvpn.org. Regards, -- Frank Lichtenheld ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
Hi, please refer diff against v14 https://pastebin.com/XA0dWiih -- Best Regards, Vladislav Grishenko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
DNS SRV remote host discovery allows to have multiple OpenVPN servers for a single domain w/o explicit profile enumeration, to move services from host to host with little fuss, and to designate hosts as primary servers for a service and others as backups. Feature has been asked several times already, should be useful in case of substantial number of clients & servers deployed. Patch introduces "--remote-srv domain [service] [proto]" option. The "service" and "proto" arguments are optional. Client will try to resolve DNS SRV record "_service._proto.domain" and use returned DNS SRV records as remote server list ordered by server selection mechanism defined in RFC2782 (https://tools.ietf.org/html/rfc2782): A client MUST attempt to contact the target host with the lowest-numbered priority field value it can reach, target hosts with the same priority SHOULD be tried in an order defined by the weight field. The weight field specifies a relative weight for entries with the same priority. Larger weights SHOULD be given a proportionately higher probability of being selected. Domain administrators SHOULD use Weight 0 when there isn't any server selection to do. In the presence of records containing weights greater than 0, records with Weight 0 SHOULD have a very small chance of being selected. Note: OpenVPN server selection mechanism implementation indeed will give records with weight of zero a very small chance of being selected first, but never skip them. Example: instead of multiple --remote in order, now it's possible to specify just one --remote-srv and configure DNS SRV records: remote-srv example.net name prio weight port target $ORIGIN example.net. _openvpn._udp IN SRV 10 60 1194 server1.example.net. _openvpn._udp IN SRV 10 40 1194 server2.example.net. _openvpn._udp IN SRV 10 0 1194 server3.example.net. _openvpn._tcp IN SRV 20 0 443 server4.example.net. For "--remote-srv example.net" following will happen in order: 1. The client will first try to resolve "_openvpn._udp.example.net" and "_openvpn._tcp.example.net". 2. Records "server1.example.net:1194", "server2.example.net:1194" and "server3.example.net:1194" will be selected before record "server4.example.net:443" as their priority 10 is smaller than 20. 3. Records "server1.example.net:1194", "server2.example.net:1194" and "server3.example.net:1194" will be randomly selected with weight probability: first will be either "server1.example.net:1194" with 60% probability or "server2.example.net:1194" with 40% or "server3.example.net:1194" with almost zero probability. 4. If "server1.example.net:1194" was selected, the second record will be either "server2.example.net:1194" with almost 100% probability or "server3.example.net:1194" with almost 0%. 5. If "server2.example.net:1194" was selected, the third record will be the only last record of priority 10 - "server3.example.net:1194". 6. Record "server4.example.net:443" will be the last one selected as the only record with priority 20. 7. Each of the resulting "target:port" remote hosts will be resolved and accessed if its protocol has no conflict with the rest of the OpenVPN options. If DNS SRV name can't be resolved or no valid records were returned, client will move on to the next connection entry. v15: rebase to master (Dec 2022) add optional port argument to --remote and --remote-srv usage message fix --proto option coexisting with --remote-srv fix --nobind option coexisting with --remote-srv fix options postprocess mutation lost in v13/v14 recover --mtu-test handling with --remote-srv use explicit srv resolve stub for openbsd for the future fix comments v14: disable SRV handling on OpenBSD due to resolver library differences v13: rebase to master (Nov 2022) apply "'static inline', not 'inline static' style guide" include --explicit-exit-notify logic from commit 7953b07bf56c1df0 formatting changes required by uncrustify replace last occurance of EAI_NODATA with EAI_NONAME OpenBSD does not have ns_c_in/ns_t_srv, need to use C_IN/T_SRV instead v12: add get_cached_srv_entry() for servinfo vs addrinfo cache split add check for mixed --remote and --remote-srv add doxygen dns srv functions comments use query_servinfo() for both unix and windows fix undefined NS_MAXMSG issue on macOS fix undefined EAI_NODATA issue on FreeBSD fix man and msg() indents rebase against master Signed-off-by: Vladislav Grishenko --- configure.ac| 2 +- doc/man-sections/client-options.rst | 121 +++- doc/management-notes.txt| 6 + src/openvpn/Makefile.am | 2 +- src/openvpn/buffer.h| 5 - src/openvpn/errlevel.h | 1 + src/openvpn/init.c | 79 ++- src/openvpn/openvpn.vcxproj
