Re: [Openvpn-devel] [PATCH v14] Add DNS SRV remote host discovery support
Hi, Frank Observing behavior is not desired, indeed. I'll look into -- Best Regards, Vladislav Grishenko > -Original Message- > From: Frank Lichtenheld > Sent: Thursday, December 1, 2022 6:37 PM > To: Gert Doering > Cc: openvpn-devel@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH v14] Add DNS SRV remote host discovery > support > > I have several nitpicks with this patch which I can enumerate later, but there is at > least one critical issue which prevents me from ACKing this: > > # src/openvpn/openvpn --client --tls-cert-profile insecure --ca ../ca.crt --cert > ../t_client.c\ > rt --key ../t_client.key--remote-cert-tls server --comp-lzo --verb 3 --dev tun -- > proto tcp4 --r\ > emote-srv lichtenheld.net --writepid ../tests/t_client-flichtenheld-TUXEDO- > InfinityBook-S-15-17-Gen7\ > -20221201-141818/openvpn-1.pid --setenv TESTNUM 1 --setenv TOP_BUILDDIR > .. --script-security 2 --up \ ./update_t_client_ips.sh > 2022-12-01 14:18:20 WARNING: Compression for receiving enabled. > Compression has been used in the pas\ t to break encryption. Sent packets are > not compressed unless "allow-compression yes" is also set. > 2022-12-01 14:18:20 Note: --cipher is not set. OpenVPN versions before 2.5 > defaulted to BF-CBC as fa\ llback when cipher negotiation failed in this case. If > you need this fallback please add '--data-cip\ hers-fallback BF-CBC' to your > configuration and/or add BF-CBC to --data-ciphers. > 2022-12-01 14:18:20 OpenVPN 2.6_git [git:master/c98fe8b90271df5c] x86_64- > pc-linux-gnu [SSL (OpenSSL)\ ] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] [AEAD] built > on Dec 1 2022 > 2022-12-01 14:18:20 library versions: OpenSSL 3.0.2 15 Mar 2022, LZO 2.10 > 2022-12-01 14:18:21 Resolved remote service host: conn-test- > server.openvpn.org:51194,udp4 prio 0 wei\ ght 0 > 2022-12-01 14:18:21 Resolved remote service host: conn-test- > server.openvpn.org:51194,tcp4-client pri\ o 0 weight 0 > 2022-12-01 14:18:21 NOTE: the current --script-security setting may allow this > configuration to call\ user-defined scripts > 2022-12-01 14:18:21 TCP/UDP: Preserving recently used remote address: > [AF_INET]199.102.77.82:51194 > 2022-12-01 14:18:21 Socket Buffers: R=[212992->212992] S=[212992->212992] > 2022-12-01 14:18:21 UDPv4 link local: (not bound) > 2022-12-01 14:18:21 UDPv4 link remote: [AF_INET]199.102.77.82:51194 > > As you can see it ignores the "--proto tcp4" if no proto was specified in -- > remote-srv. > This is inconsistent with how --remote works. I don't think this can be the > desired behaviour. > > Regards, > -- > Frank Lichtenheld > > > ___ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v14] Add DNS SRV remote host discovery support
Hi Frank,
> > +"--remote-srv domain [service] : Perform DNS SRV remote host
> discovery.\n"
>
> Missing [proto]
Sure, will add
> > -if (!proto_is_udp(ce->proto) && options->mtu_test)
> > +/* Defer validation for --remote-srv with auto protocol */
> > +if (!proto_is_udp(ce->proto) && options->mtu_test
> > +&& !(ce->remote_srv && ce->proto == PROTO_AUTO))
> > {
> > msg(M_USAGE, "--mtu-test only makes sense with --proto udp");
>
> You disable this test here, but you don't add this in any of the
> later checks. So it seems this test is just completely removed when
> using remote-srv?
Right, this check should be moved into options_postprocess_verify_ce_proto()
and seems forgotten, will add it too. Thank you
> > + * get_cached_srv_entry return 0 on success and -1
>
> "returns"
Ok, sure
--
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 v14] Add DNS SRV remote host discovery support
Smaller nitpicks:
On Wed, Nov 30, 2022 at 09:57:18PM +0100, Gert Doering wrote:
> From: Vladislav Grishenko
[...]
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index c2154b8d..3a70748e 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -350,7 +350,12 @@ management_callback_remote_cmd(void *arg, const char **p)
> }
> else if (!strcmp(p[1], "MOD") && p[2] && p[3])
> {
> -if (strlen(p[2]) < RH_HOST_LEN && strlen(p[3]) < RH_PORT_LEN)
> +if (ce->remote_srv && ce->proto == PROTO_AUTO)
> +{
> +/* can't mutate --remote-srv into --remote without protocol
> */
> +ret = false;
> +}
> +else if (strlen(p[2]) < RH_HOST_LEN && strlen(p[3]) <
> RH_PORT_LEN)
> {
> struct remote_host_store *rhs = c->options.rh_store;
> if (!rhs)
> @@ -363,6 +368,7 @@ management_callback_remote_cmd(void *arg, const char **p)
>
> ce->remote = rhs->host;
> ce->remote_port = rhs->port;
> +ce->remote_srv = false;
> flags = CE_MAN_QUERY_REMOTE_MOD;
> ret = true;
> }
> @@ -462,6 +468,23 @@ clear_remote_addrlist(struct link_socket_addr *lsa, bool
> free)
> lsa->current_remote = NULL;
> }
>
> +/*
> + * Clear the remote service list
> + */
> +static void
> +clear_remote_servlist(struct link_socket_addr *lsa, bool free)
> +{
> +if (lsa->service_list && free)
> +{
> +freeservinfo(lsa->service_list);
> +}
> +lsa->service_list = NULL;
> +lsa->current_service = NULL;
> +
> +/* clear addrinfo objects as well */
> +clear_remote_addrlist(lsa, free);
> +}
> +
> /*
> * Increment to next connection entry
> */
> @@ -491,6 +514,24 @@ next_connection_entry(struct context *c)
> c->c1.link_socket_addr.current_remote =
> c->c1.link_socket_addr.current_remote->ai_next;
> }
> +/* Check if there is another resolved service to try for
> + * the current connection unless persist-remote-ip was
> + * requested and current service already has an address */
> +else if (c->c1.link_socket_addr.current_service
> + && c->c1.link_socket_addr.current_service->next
> + && !(c->options.persist_remote_ip
> + && c->c1.link_socket_addr.remote_list))
> +{
> +c->c1.link_socket_addr.current_service =
> +c->c1.link_socket_addr.current_service->next;
> +
> +/* Clear addrinfo object of the previous service */
> +if (c->c1.link_socket_addr.remote_list)
> +{
> +clear_remote_addrlist(>c1.link_socket_addr,
> + !c->options.resolve_in_advance);
> +}
> +}
> else
> {
> c->options.advance_next_remote = false;
> @@ -500,20 +541,24 @@ next_connection_entry(struct context *c)
> */
> if (!c->options.persist_remote_ip)
> {
> -/* Connection entry addrinfo objects might have been
> +/* Connection entry addr/servinfo objects might have been
> * resolved earlier but the entry itself might have been
> - * skipped by management on the previous loop.
> - * If so, clear the addrinfo objects as close_instance
> does
> + * skipped on the previous loop either by management or
> + * due inappropriate service protocol.
> + * Clear the addr/servinfo objects as close_instance
> does.
> */
> -if (c->c1.link_socket_addr.remote_list)
> +if (c->c1.link_socket_addr.remote_list
> +|| c->c1.link_socket_addr.service_list)
> {
> -clear_remote_addrlist(>c1.link_socket_addr,
> +clear_remote_servlist(>c1.link_socket_addr,
>
> !c->options.resolve_in_advance);
> }
>
> /* close_instance should have cleared the addrinfo
> objects */
> ASSERT(c->c1.link_socket_addr.current_remote == NULL);
> ASSERT(c->c1.link_socket_addr.remote_list == NULL);
> +ASSERT(c->c1.link_socket_addr.current_service == NULL);
> +ASSERT(c->c1.link_socket_addr.service_list == NULL);
> }
> else
> {
> @@ -549,6 +594,12 @@ next_connection_entry(struct context *c)
> }
>
> c->options.ce = *ce;
> +if (ce_defined &&
Re: [Openvpn-devel] [PATCH v14] Add DNS SRV remote host discovery support
I have several nitpicks with this patch which I can enumerate later, but there is at least one critical issue which prevents me from ACKing this: # src/openvpn/openvpn --client --tls-cert-profile insecure --ca ../ca.crt --cert ../t_client.c\ rt --key ../t_client.key--remote-cert-tls server --comp-lzo --verb 3 --dev tun --proto tcp4 --r\ emote-srv lichtenheld.net --writepid ../tests/t_client-flichtenheld-TUXEDO-InfinityBook-S-15-17-Gen7\ -20221201-141818/openvpn-1.pid --setenv TESTNUM 1 --setenv TOP_BUILDDIR .. --script-security 2 --up \ ./update_t_client_ips.sh 2022-12-01 14:18:20 WARNING: Compression for receiving enabled. Compression has been used in the pas\ t to break encryption. Sent packets are not compressed unless "allow-compression yes" is also set. 2022-12-01 14:18:20 Note: --cipher is not set. OpenVPN versions before 2.5 defaulted to BF-CBC as fa\ llback when cipher negotiation failed in this case. If you need this fallback please add '--data-cip\ hers-fallback BF-CBC' to your configuration and/or add BF-CBC to --data-ciphers. 2022-12-01 14:18:20 OpenVPN 2.6_git [git:master/c98fe8b90271df5c] x86_64-pc-linux-gnu [SSL (OpenSSL)\ ] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] [AEAD] built on Dec 1 2022 2022-12-01 14:18:20 library versions: OpenSSL 3.0.2 15 Mar 2022, LZO 2.10 2022-12-01 14:18:21 Resolved remote service host: conn-test-server.openvpn.org:51194,udp4 prio 0 wei\ ght 0 2022-12-01 14:18:21 Resolved remote service host: conn-test-server.openvpn.org:51194,tcp4-client pri\ o 0 weight 0 2022-12-01 14:18:21 NOTE: the current --script-security setting may allow this configuration to call\ user-defined scripts 2022-12-01 14:18:21 TCP/UDP: Preserving recently used remote address: [AF_INET]199.102.77.82:51194 2022-12-01 14:18:21 Socket Buffers: R=[212992->212992] S=[212992->212992] 2022-12-01 14:18:21 UDPv4 link local: (not bound) 2022-12-01 14:18:21 UDPv4 link remote: [AF_INET]199.102.77.82:51194 As you can see it ignores the "--proto tcp4" if no proto was specified in --remote-srv. This is inconsistent with how --remote works. I don't think this can be the desired behaviour. Regards, -- Frank Lichtenheld ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v14] Add DNS SRV remote host discovery support
From: Vladislav Grishenko 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. 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 | 15 +- src/openvpn/options.c | 258 ++-- src/openvpn/options.h | 4 + src/openvpn/socket.c| 875 +++- src/openvpn/socket.h| 65 ++- src/openvpn/syshead.h | 5 + 13 files changed, 1375 insertions(+), 63 deletions(-) diff --git a/configure.ac
