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, 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 +++-
d
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 applied] Re: Add CRL extractor script for --crl-verify dir mode
Thanks! Need to say, implemented "run an openssl binary" internal method is a bit faster than python-native crl parsing, according our tests and usage experience. -- Best Regards, Vladislav Grishenko > -Original Message- > From: Gert Doering > Sent: Thursday, May 6, 2021 1:12 AM > To: Vladislav Grishenko > Cc: openvpn-devel@lists.sourceforge.net > Subject: [PATCH applied] Re: Add CRL extractor script for --crl-verify dir mode > > Acked-by: Gert Doering > > We discussed this in the community meeting today, and came to the conclusion > that this is a nice and helpful addition. It could be argued that python is > somewhat heavy to "run an openssl binary and do something with the result", > but it's neither the first nor last python script in our repo, and "have a script" is > better than "I could write one in shell" :-) > > I have not actually tested the script. David has taken a look and said it looks > reasonable. Rules for contrib/ are not as strict as for "openvpn main source", so > that's good enough. > > Thanks. > > Your patch has been applied to the master branch. > > commit 4c2549ba5d8b1b449acc62a46692345710965647 > Author: Vladislav Grishenko > Date: Sat Oct 3 02:51:46 2020 +0500 > > Add CRL extractor script for --crl-verify dir mode > > Signed-off-by: Vladislav Grishenko > Acked-by: Gert Doering > Message-Id: <20201002215146.31324-1-themi...@yandex-team.ru> > URL: https://www.mail-archive.com/openvpn- > de...@lists.sourceforge.net/msg21154.html > Signed-off-by: Gert Doering > > > -- > kind regards, > > Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v12] 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. 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 | 8 +- src/openvpn/options.c | 281 +++-- src/openvpn/options.h | 4 + src/openvpn/socket.c| 867 +++- src/openvpn/socket.h| 65 ++- src/openvpn/syshead.h | 5 + 13 files changed, 1368 insertions(+), 78 deletions(-) diff --git a/configure.ac b/configure.ac index f05faf99..da5e1723 100644 --- a/configure.ac +++ b/configure.ac @@ -455,7 +455,7 @@ SOCKET_INCLUDES=" " AC_CHECK_HEADERS( - [net/if.h netinet/ip.h resolv.h sys/un.h net/if_u
Re: [Openvpn-devel] [PATCH v3 1/2] Fix IPv4 default gateway with multiple route tables
Hi,
> However, to prevent the next casual reader from asking the same question,
> wouldn't it make sense to change this comparison with:
>
> if (res->table != RT_TABLE_UNSPEC && res->table != table)
I don’t think it's necessary for following reasons:
1. Readability. "if (res->table)" reads as "if filter table is set ". Changing
it to "if (res->table != RT_TABLE_UNSPEC)" gives the same reading imho.
2. RT_TABLE_UNSPEC was and will be always zero, because zillion of software
incl. iproute2 depends on zeroed structs with 0 == UNSPEC
3. Comparing with RT_TABLE_UNSPEC here will require additional code to
explicitely init res.table with RT_TABLE_UNSPEC, which in fact is will be noop
after CLEAR(res) and have little sense, since there's no chance to have
RT_TABLE_UNSPEC != 0.
Anyway, if you still thinks it's required, I can do change.
> Another question: why restricting the research to the MAIN table only for IPv4
> and only when searching a default route?
For real dst, incl ::/128 we do the right thing - perform "ip route get" that
will get real gateway for specified dst no matter what table or anything else
is.
And we performs "ip route show" dump only to get gateway for "0.0.0.0"
destination by a reason.
Getting default gateway for "0.0.0.0/0" destination is not logically possible
at all, for example what is default gateway for this case:
0.0.0.0/1 via a.a.a.a
128.0.0.0/1 via b.b.b.b
Therefore for 0.0.0.0/0 case heuristic is used, if there's a default route, if
it's in main table and so on, that’s why only dumping needs it.
Any other - don't.
> Otherwise we are fixing this problem only for the ipv4/default search, but the
> function remains "buggy" for the other cases.
Buggy here is searching default gateway for 0.0.0.0/0 itself. Other cases are
right from the scratch :)
--
Best Regards, Vladislav Grishenko
> -Original Message-
> From: Antonio Quartulli
> Sent: Friday, April 16, 2021 7:01 PM
> To: Vladislav Grishenko ; openvpn-
> de...@lists.sourceforge.net
> Subject: Re: [Openvpn-devel] [PATCH v3 1/2] Fix IPv4 default gateway with
> multiple route tables
>
> Hi,
>
> On 16/04/2021 14:07, Vladislav Grishenko wrote:
> > Current default gateway selection for zero destination address just
> > dumps and parses all the routing tables. If any of non-main table with
> > default route comes first, wrong default gateway can be picked.
> > Since adding/removing routes currently handles only main table, let's
> > stick to RT_TABLE_MAIN while selecting default route too.
> >
> > v2: keep gateway address unchanged on lookup error
> > v3: reduce ammout of gateway address copying
> >
> > Reported-by: Donald Sharp
> > Signed-off-by: Vladislav Grishenko
> > ---
> > src/openvpn/networking_sitnl.c | 26 --
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/openvpn/networking_sitnl.c
> > b/src/openvpn/networking_sitnl.c index 2bc70a50..ea1621ed 100644
> > --- a/src/openvpn/networking_sitnl.c
> > +++ b/src/openvpn/networking_sitnl.c
> > @@ -426,6 +426,7 @@ typedef struct {
> > inet_address_t gw;
> > char iface[IFNAMSIZ];
> > bool default_only;
> > +unsigned int table;
> > } route_res_t;
> >
> > static int
> > @@ -435,7 +436,8 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
> > struct rtmsg *r = NLMSG_DATA(n);
> > struct rtattr *rta = RTM_RTA(r);
> > int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
> > -unsigned int ifindex = 0;
> > +unsigned int table, ifindex = 0;
> > +void *gw = NULL;
> >
> > /* filter-out non-zero dst prefixes */
> > if (res->default_only && r->rtm_dst_len != 0) @@ -443,6 +445,9 @@
> > sitnl_route_save(struct nlmsghdr *n, void *arg)
> > return 1;
> > }
> >
> > +/* route table, ignored with RTA_TABLE */
> > +table = r->rtm_table;
> > +
> > while (RTA_OK(rta, len))
> > {
> > switch (rta->rta_type)
> > @@ -458,13 +463,24 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
> >
> > /* GW for the route */
> > case RTA_GATEWAY:
> > -memcpy(>gw, RTA_DATA(rta), res->addr_size);
> > +gw = RTA_DATA(rta);
> > +break;
> > +
> > +/* route table */
> > +case RTA_TABLE:
> > +table = *(unsigned int *)RTA_DATA(rta);
> > break;
> > }
> >
[Openvpn-devel] [PATCH v3 2/2] Add basic support for multipath gateway
Load balancing setup over multiple upstreams may include multipath
gateway route, which is not not supported by OpenVPN.
Let's add basic support for that for selecting best route for zero
destination address - use any one of nexthop addresses as a gateway,
weights are not handled.
Setup example:
ip route add default \
nexthop via 192.168.1.1 dev eth1 weight 1 \
nexthop via 192.168.2.1 dev eth2 weight 1
v2: keep gateway address unchanged on lookup error
v3: reduce ammout of gateway address copying
Reported-by: Donald Sharp
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index ea1621ed..aa35f5f5 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -450,6 +450,9 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
while (RTA_OK(rta, len))
{
+struct rtnexthop *nh;
+int nhlen;
+
switch (rta->rta_type)
{
/* route interface */
@@ -470,6 +473,37 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
case RTA_TABLE:
table = *(unsigned int *)RTA_DATA(rta);
break;
+
+/* multipath nexthops */
+case RTA_MULTIPATH:
+nh = RTA_DATA(rta);
+nhlen = RTA_PAYLOAD(rta);
+
+while (RTNH_OK(nh, nhlen))
+{
+struct rtattr *nha = RTNH_DATA(nh);
+int nhalen = nh->rtnh_len - sizeof(*nh);
+
+/* init route interface & gateway */
+ifindex = nh->rtnh_ifindex;
+gw = NULL;
+
+while (RTA_OK(nha, nhalen))
+{
+switch (nha->rta_type)
+{
+/* GW for the route */
+case RTA_GATEWAY:
+gw = RTA_DATA(nha);
+break;
+}
+
+nha = RTA_NEXT(nha, nhalen);
+}
+
+nh = RTNH_NEXT(nh);
+}
+break;
}
rta = RTA_NEXT(rta, len);
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3 1/2] Fix IPv4 default gateway with multiple route tables
Current default gateway selection for zero destination address just
dumps and parses all the routing tables. If any of non-main table
with default route comes first, wrong default gateway can be picked.
Since adding/removing routes currently handles only main table,
let's stick to RT_TABLE_MAIN while selecting default route too.
v2: keep gateway address unchanged on lookup error
v3: reduce ammout of gateway address copying
Reported-by: Donald Sharp
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 26 --
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 2bc70a50..ea1621ed 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -426,6 +426,7 @@ typedef struct {
inet_address_t gw;
char iface[IFNAMSIZ];
bool default_only;
+unsigned int table;
} route_res_t;
static int
@@ -435,7 +436,8 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
struct rtmsg *r = NLMSG_DATA(n);
struct rtattr *rta = RTM_RTA(r);
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
-unsigned int ifindex = 0;
+unsigned int table, ifindex = 0;
+void *gw = NULL;
/* filter-out non-zero dst prefixes */
if (res->default_only && r->rtm_dst_len != 0)
@@ -443,6 +445,9 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
return 1;
}
+/* route table, ignored with RTA_TABLE */
+table = r->rtm_table;
+
while (RTA_OK(rta, len))
{
switch (rta->rta_type)
@@ -458,13 +463,24 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
/* GW for the route */
case RTA_GATEWAY:
-memcpy(>gw, RTA_DATA(rta), res->addr_size);
+gw = RTA_DATA(rta);
+break;
+
+/* route table */
+case RTA_TABLE:
+table = *(unsigned int *)RTA_DATA(rta);
break;
}
rta = RTA_NEXT(rta, len);
}
+/* filter out any route not coming from the selected table */
+if (res->table && res->table != table)
+{
+return 1;
+}
+
if (!if_indextoname(ifindex, res->iface))
{
msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifname for index %d",
@@ -472,6 +488,11 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
return -1;
}
+if (gw)
+{
+memcpy(>gw, gw, res->addr_size);
+}
+
return 0;
}
@@ -507,6 +528,7 @@ sitnl_route_best_gw(sa_family_t af_family, const
inet_address_t *dst,
{
req.n.nlmsg_flags |= NLM_F_DUMP;
res.default_only = true;
+res.table = RT_TABLE_MAIN;
}
else
{
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 1/2] Fix IPv4 default gateway with multiple route tables
Current default gateway selection for zero destination address just
dumps and parses all the routing tables. If any of non-main table
with default route comes first, wrong default gateway can be picked.
Since adding/removing routes currently handles only main table,
let's stick to RT_TABLE_MAIN while selecting default route too.
Reported-By: Donald Sharp
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 2bc70a50..402d3303 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -426,6 +426,7 @@ typedef struct {
inet_address_t gw;
char iface[IFNAMSIZ];
bool default_only;
+unsigned int table;
} route_res_t;
static int
@@ -435,7 +436,8 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
struct rtmsg *r = NLMSG_DATA(n);
struct rtattr *rta = RTM_RTA(r);
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
-unsigned int ifindex = 0;
+unsigned int table, ifindex = 0;
+inet_address_t gw;
/* filter-out non-zero dst prefixes */
if (res->default_only && r->rtm_dst_len != 0)
@@ -443,6 +445,11 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
return 1;
}
+/* route table, ignored with RTA_TABLE */
+table = r->rtm_table;
+/* initial route gateway */
+gw = res->gw;
+
while (RTA_OK(rta, len))
{
switch (rta->rta_type)
@@ -458,19 +465,31 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
/* GW for the route */
case RTA_GATEWAY:
-memcpy(>gw, RTA_DATA(rta), res->addr_size);
+memcpy(, RTA_DATA(rta), res->addr_size);
+break;
+
+/* route table */
+case RTA_TABLE:
+table = *(unsigned int *)RTA_DATA(rta);
break;
}
rta = RTA_NEXT(rta, len);
}
+/* filter out any route not coming from the selected table */
+if (res->table && res->table != table)
+{
+return 1;
+}
+
if (!if_indextoname(ifindex, res->iface))
{
msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifname for index %d",
__func__, ifindex);
return -1;
}
+res->gw = gw;
return 0;
}
@@ -507,6 +526,7 @@ sitnl_route_best_gw(sa_family_t af_family, const
inet_address_t *dst,
{
req.n.nlmsg_flags |= NLM_F_DUMP;
res.default_only = true;
+res.table = RT_TABLE_MAIN;
}
else
{
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 2/2] Add basic support for multipath gateway
Load balancing setup over multiple upstreams may include multipath
gateway route, which is not not supported by OpenVPN.
Let's add basic support for that for selecting best route for zero
destination address - use any one of nexthop addresses as a gateway,
weights are not handled.
Setup example:
ip route add default \
nexthop via 192.168.1.1 dev eth1 weight 1 \
nexthop via 192.168.2.1 dev eth2 weight 1
Reported-By: Donald Sharp
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 35 ++
1 file changed, 35 insertions(+)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 402d3303..02c34d6b 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -452,6 +452,9 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
while (RTA_OK(rta, len))
{
+struct rtnexthop *nh;
+int nhlen;
+
switch (rta->rta_type)
{
/* route interface */
@@ -472,6 +475,38 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
case RTA_TABLE:
table = *(unsigned int *)RTA_DATA(rta);
break;
+
+/* multipath nexthops */
+case RTA_MULTIPATH:
+nh = RTA_DATA(rta);
+nhlen = RTA_PAYLOAD(rta);
+
+while (RTNH_OK(nh, nhlen))
+{
+struct rtattr *nha = RTNH_DATA(nh);
+int nhalen = nh->rtnh_len - sizeof(*nh);
+
+/* route interface */
+ifindex = nh->rtnh_ifindex;
+/* initial route gateway */
+gw = res->gw;
+
+while (RTA_OK(nha, nhalen))
+{
+switch (nha->rta_type)
+{
+/* GW for the route */
+case RTA_GATEWAY:
+memcpy(, RTA_DATA(nha), res->addr_size);
+break;
+}
+
+nha = RTA_NEXT(nha, nhalen);
+}
+
+nh = RTNH_NEXT(nh);
+}
+break;
}
rta = RTA_NEXT(rta, len);
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/2] Fix IPv4 default gateway with multiple route tables
Current default gateway selection for zero destignation address just
dumps and parses all the routing tables. If any of non-main table
with default route comes first, wrong default gateway can be picked.
Since adding/removing routes currently handles only main table,
let's stick to RT_TABLE_MAIN while selecting default route too.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 2bc70a50..56543648 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -426,6 +426,7 @@ typedef struct {
inet_address_t gw;
char iface[IFNAMSIZ];
bool default_only;
+unsigned int table;
} route_res_t;
static int
@@ -435,7 +436,7 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
struct rtmsg *r = NLMSG_DATA(n);
struct rtattr *rta = RTM_RTA(r);
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
-unsigned int ifindex = 0;
+unsigned int table, ifindex = 0;
/* filter-out non-zero dst prefixes */
if (res->default_only && r->rtm_dst_len != 0)
@@ -443,6 +444,9 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
return 1;
}
+/* route table, ignored with RTA_TABLE */
+table = r->rtm_table;
+
while (RTA_OK(rta, len))
{
switch (rta->rta_type)
@@ -460,11 +464,22 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
case RTA_GATEWAY:
memcpy(>gw, RTA_DATA(rta), res->addr_size);
break;
+
+/* route table */
+case RTA_TABLE:
+table = *(unsigned int *)RTA_DATA(rta);
+break;
}
rta = RTA_NEXT(rta, len);
}
+/* filter-out zero dns prefixes from other tables */
+if (res->table && res->table != table)
+{
+return 1;
+}
+
if (!if_indextoname(ifindex, res->iface))
{
msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifname for index %d",
@@ -507,6 +522,7 @@ sitnl_route_best_gw(sa_family_t af_family, const
inet_address_t *dst,
{
req.n.nlmsg_flags |= NLM_F_DUMP;
res.default_only = true;
+res.table = RT_TABLE_MAIN;
}
else
{
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/2] Add basic support for multipath gateway
Load balancing setup over multiple upstreams may include multipath
gateway route, which is not not supported by OpenVPN.
Let's add basic support for that for selecting best route for zero
destignation address - use any one of nexthop addresses as a gateway,
weights are not handled.
Setup example:
ip route add default \
nexthop via 192.168.1.1 dev eth1 weight 1 \
nexthop via 192.168.2.1 dev eth2 weight 1
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 37 ++
1 file changed, 37 insertions(+)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 56543648..8f084687 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -449,6 +449,11 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
while (RTA_OK(rta, len))
{
+#ifdef RTA_MULTIPATH
+struct rtnexthop *nh;
+int nhlen;
+#endif
+
switch (rta->rta_type)
{
/* route interface */
@@ -469,6 +474,38 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
case RTA_TABLE:
table = *(unsigned int *)RTA_DATA(rta);
break;
+
+#ifdef RTA_MULTIPATH
+/* multipath nexthops */
+case RTA_MULTIPATH:
+nh = RTA_DATA(rta);
+nhlen = RTA_PAYLOAD(rta);
+
+while (RTNH_OK(nh, nhlen))
+{
+struct rtattr *nha = RTNH_DATA(nh);
+int nhalen = nh->rtnh_len - sizeof(*nh);
+
+/* route interface */
+ifindex = nh->rtnh_ifindex;
+
+while (RTA_OK(nha, nhalen))
+{
+switch (nha->rta_type)
+{
+/* GW for the route */
+case RTA_GATEWAY:
+memcpy(>gw, RTA_DATA(nha),
res->addr_size);
+break;
+}
+
+nha = RTA_NEXT(nha, nhalen);
+}
+
+nh = RTNH_NEXT(nh);
+}
+break;
+#endif
}
rta = RTA_NEXT(rta, len);
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v11] Add DNS SRV remote host discovery support
Hi, As discussed, rebased against git master with one change vs v10 - dead inetd code removal. Tested on Windows 10 and Linux, FreeBSD should be fine as well since previous v10. We talked about the OpenVPN usage experience at NextHop 2020 conference last fall and got interested external feedback regarding the SRV feature. Would be great, if you could kindly suggest when re-review/merge can be scheduled if no issue found. -- Best Regards, Vladislav Grishenko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v11] 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.
v11:
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
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 | 8 +-
src/openvpn/options.c | 281 +++--
src/openvpn/options.h | 4 +
src/openvpn/socket.c| 875 +++-
src/openvpn/socket.h| 54 ++
src/openvpn/syshead.h | 5 +
13 files changed, 1371 insertions(+), 72 deletions(-)
diff --git a/configure.ac b/configure.ac
index 8e31dcb2..dcb0db0d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -474,7 +474,7 @@ SOCKET_INCLUDES="
"
AC_CHECK_HEADERS(
- [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h
sys/kern_co
Re: [Openvpn-devel] [PATCH v10] Add DNS SRV remote host discovery support
Hello and happy holidays, Is there a chance to get back to this patch since v9 was acked and minor fix for undefined EAI_NODATA on FreeBSD was applied? -- Best Regards, Vladislav Grishenko > -Original Message- > From: Vladislav Grishenko > Sent: Friday, December 4, 2020 9:15 PM > To: openvpn-devel@lists.sourceforge.net > Cc: g...@greenie.muc.de > Subject: [Openvpn-devel] [PATCH v10] 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. > > v10: > 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 > > Signed-off-by: Vladislav Grishenko > --- > configure.ac| 2 +- > doc/man-sections/client-options.rst | 121 +++- > doc/m
Re: [Openvpn-devel] [PATCH v9] Add DNS SRV remote host discovery support
Hi, Gert Indeed, early platform-related code was tested on FreeBSD w/o EAI_NODATA return results used. Since that changes were not related to low-level resolving code, and therefore was tested on Linux with _GNU and Windows only. Unfortunately at some time point usage of EAI_NODATA return result was introduced and I've missed FreeBSD testing thought it was still not necessary, sorry. Please refer v10 version sent and additional separate v9->v10 patch to see exact changes. It address all the places with EAI_NODATA and keeps windows logic consistent as well. > I have no idea *why* FreeBSD considers this "obsoleted", or what this define is used for - have not yet investigated in any way https://lists.freebsd.org/pipermail/freebsd-ports/2003-October/005757.html > It was depricated in RFC3493 (aka RFC2553bis). Now, getaddrinfo(3) returns EAI_NONAME instead of EAI_NODATA. -- Best Regards, Vladislav Grishenko > -Original Message- > From: Gert Doering > Sent: Friday, December 4, 2020 7:02 PM > To: Vladislav Grishenko > Cc: openvpn-devel@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH v9] Add DNS SRV remote host discovery > support > > Hi, > > On Mon, Nov 16, 2020 at 03:08:57AM +0500, Vladislav Grishenko wrote: > > 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. > > So, Arne has ACKed it, and I went work on it today. One problem showed up on > FreeBSD - the code uses EAI_NODATA, which is found in on FreeBSD, > but with this comment: > > #if 0 > /* obsoleted */ > #define EAI_NODATA 7 /* no address associated with hostname */ > #endif > > I'm not sure what to make out of this, but as it stands, the patchset does not > work. > > If I just add > > #define EAI_NODATA 7 > > to socket.c, it compiles and passes my regular client tests (no SRV tests yet), but > I do not think this is the correct solution. > > (I have no idea *why* FreeBSD considers this "obsoleted", or what this define is > used for - have not yet investigated in any way) > > > If you need to test on FreeBSD, openvpn-vagrant has boxes that get installed > with all prerequisites (or I can give you access to one of my buildslaves). > > 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
[Openvpn-devel] [PATCH v10] 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. v10: 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 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 | 8 +- src/openvpn/options.c | 286 +++-- src/openvpn/options.h | 4 + src/openvpn/socket.c| 875 +++- src/openvpn/socket.h| 54 ++ src/openvpn/syshead.h | 5 + 13 files changed, 1374 insertions(+), 74 deletions(-) diff --git a/configure.ac b/configure.ac index 1ab8fe59..d110ba98 100644 --- a/configure.ac +++ b/configure.ac @@ -470,7 +470,7 @@ SOCKET_INCLUDES=" " AC_CHECK_HEADERS( - [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h sys/kern_control.h], + [net/if.
[Openvpn-devel] [PATCH] 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. v10: 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 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 | 8 +- src/openvpn/options.c | 286 +++-- src/openvpn/options.h | 4 + src/openvpn/socket.c| 875 +++- src/openvpn/socket.h| 54 ++ src/openvpn/syshead.h | 5 + 13 files changed, 1374 insertions(+), 74 deletions(-) diff --git a/configure.ac b/configure.ac index 1ab8fe59..d110ba98 100644 --- a/configure.ac +++ b/configure.ac @@ -470,7 +470,7 @@ SOCKET_INCLUDES=" " AC_CHECK_HEADERS( - [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h sys/kern_control.h], + [net/if.
[Openvpn-devel] [PATCH] Drop EAI_NODATA, absent on FreeBSD and obsoleted by RFC3493
---
src/openvpn/socket.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index bd085e8f..31e8fe9a 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -625,10 +625,8 @@ query_servinfo(const char *domain, int proto,
break;
case DNS_ERROR_RCODE_NAME_ERROR:
-return EAI_NONAME; /* HOST_NOT_FOUND */
-
case DNS_INFO_NO_RECORDS:
-return EAI_NODATA; /* NO_DATA */
+return EAI_NONAME; /* HOST_NOT_FOUND */
case DNS_ERROR_NO_DNS_SERVERS:
case DNS_ERROR_RCODE_FORMAT_ERROR:
@@ -642,7 +640,7 @@ query_servinfo(const char *domain, int proto,
return EAI_AGAIN; /* TRY_AGAIN */
default:
-return EAI_NODATA;
+return EAI_FAIL;
}
struct servinfo *list = NULL, *first = NULL;
@@ -682,7 +680,7 @@ query_servinfo(const char *domain, int proto,
}
else
{
-status = EAI_NODATA;
+status = EAI_FAIL;
}
done:
@@ -717,13 +715,11 @@ query_servinfo(const char *domain, int proto,
switch (h_errno)
{
case HOST_NOT_FOUND:
-return EAI_NONAME;
-
case NO_ADDRESS:
#if NO_ADDRESS != NO_DATA
case NO_DATA:
#endif
-return EAI_NODATA;
+return EAI_NONAME;
case NO_RECOVERY:
return EAI_FAIL;
@@ -782,7 +778,7 @@ query_servinfo(const char *domain, int proto,
}
else
{
-status = EAI_NODATA;
+status = EAI_FAIL;
}
done:
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v8] Add DNS SRV remote host discovery support
Hi Arne, Thank you for the review and please refer v9 where all the mentioned parts are handled. -- Best Regards, Vladislav Grishenko > -Original Message- > From: Arne Schwabe > Sent: Tuesday, October 20, 2020 11:58 AM > To: Vladislav Grishenko ; openvpn- > de...@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH v8] Add DNS SRV remote host discovery > support > > Am 05.10.20 um 00:36 schrieb 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: > > remote server1.example.net 1194 udp > > remote server2.example.net 1194 udp > > remote server3.example.net 1194 udp > > remote server4.example.net 443 tcp > > > > now it's possible to specify just one --remote-srv: > > remote-srv example.net > > > > and configure following DNS SRV records: > > name prio weight port target > > _openvpn._udp.example.net IN SRV 10 60 1194 server1.example.net > > _openvpn._udp.example.net IN SRV 10 40 1194 server2.example.net > > _openvpn._udp.example.net IN SRV 10 0 1194 server3.example.net > > _openvpn._tcp.example.net IN SRV 20 0 443 server4.example.net > > Missing . at the end at least if it should be in a bind zone file: > > > _openvpn._udp.example.net. IN SRV 10 60 1194 server1.example.net. > > or > > _openvpn._udp IN SRV 10 60 1194 server1.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"
[Openvpn-devel] [PATCH v9] 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. v9: 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 man 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 | 8 +- src/openvpn/options.c | 286 +++-- src/openvpn/options.h | 4 + src/openvpn/socket.c| 879 +++- src/openvpn/socket.h| 54 ++ src/openvpn/syshead.h | 5 + 13 files changed, 1378 insertions(+), 74 deletions(-) diff --git a/configure.ac b/configure.ac index 1ab8fe59..d110ba98 100644 --- a/configure.ac +++ b/configure.ac @@ -470,7 +470,7 @@ SOCKET_INCLUDES=" " AC_CHECK_HEADERS( - [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h sys/kern_control.h], + [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h net/
Re: [Openvpn-devel] [PATCH applied] Re: Speedup TCP remote hosts connections
Hi Gert, > "--tcp-server" Yep, mean it, even poll doesn't used there. Have no any prio about it tho, just related thoughts. -- Best Regards, Vladislav Grishenko > -Original Message- > From: Gert Doering > Sent: Monday, October 5, 2020 10:28 PM > To: Vladislav Grishenko > Cc: 'Gert Doering' ; openvpn- > de...@lists.sourceforge.net > Subject: Re: [PATCH applied] Re: Speedup TCP remote hosts connections > > Hi, > > On Mon, Oct 05, 2020 at 10:22:43PM +0500, Vladislav Grishenko wrote: > > Perhaps same approach can be applied to server's tcp listening, would > > require testing of more management cases. > > There's two code paths here > > --tcp-server > > and > > --mode server + --tcp > > I think the "mode server" code path is already very quick (= if I test with your fast > TCP client, against a "git master" server, I get 0.16s connection setup time). This > is what people use on "more than one client" servers, so it's already fine. > > The "--tcp-server" code path is "point to point". Not sure we currently test this > at all (I have UDP p2p instance, but no TCP yet... seems I need to add one). This > might indeed be slow, I had a look at the > accept() path in socket.c recently and was wondering "how can this work at all?" > - but that's "socket.c" vs. "mtcp.c", I think. > > > Since this is somewhat of a niche case, I'd put that into the "2.6" bin :-) > - the other one, TCP on the client, is much more heavily used, so 2.5 > for that was appropriate. > > 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
Re: [Openvpn-devel] [PATCH applied] Re: Speedup TCP remote hosts connections
Hi Gert, Thanks for that. Perhaps same approach can be applied to server's tcp listening, would require testing of more management cases. -- Best Regards, Vladislav Grishenko > -Original Message- > From: Gert Doering > Sent: Sunday, October 4, 2020 5:19 PM > To: Vladislav Grishenko > Cc: openvpn-devel@lists.sourceforge.net > Subject: [PATCH applied] Re: Speedup TCP remote hosts connections > > Your patch has been applied to the master branch. > > I have run a number of TCP client connect tests, to a very close server (speedup > very noticeable!), to distant server (still some difference), and to a non- > reachable IP address (because I was worried that this would block out > management, but it doesn't - it just shifts the "delay 1 s" to openvpn_connect(), > and makes it a "max 1s" instead of a hard sleep). > > I have not looked into manage.c as closely, but from a quick glance, this also > looks reasonable. > > Since we need a RC3 anyway (due to the redirect-gateway bug), and Arne's > motto for 2.5 is "MAKE THINGS FAST!", I think this still fits into 2.5. > > commit b68aa00603332357e6c711e91c5b4ba04d78294b (master) commit > d08b66f39cf80733a51e1607386dc4993faef09f (release/2.5) > Author: Vladislav Grishenko > Date: Fri Oct 2 03:53:19 2020 +0500 > > Speedup TCP remote hosts connections > > Signed-off-by: Vladislav Grishenko > Acked-by: Arne Schwabe > Message-Id: <20201001225319.25125-1-themi...@yandex-team.ru> > URL: https://www.mail-archive.com/openvpn- > de...@lists.sourceforge.net/msg21139.html > Signed-off-by: Gert Doering > > > -- > kind regards, > > Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: Support X509 field list to be username
Hi Gert,
Thank you.
--
Best Regards, Vladislav Grishenko
> -Original Message-
> From: Gert Doering
> Sent: Monday, October 5, 2020 3:36 PM
> To: Vladislav Grishenko
> Cc: openvpn-devel@lists.sourceforge.net
> Subject: [PATCH applied] Re: Support X509 field list to be username
>
> Your patch has been applied to the master branch.
>
> Client-side tested, and lightly stared at the code.
>
> The whitespace dragon made me add that blank after comma (but not the
other
> one in code not already touched by this patch) - IRC discussion points to
> "sp_after_comma", which we indeed do not set today. We need to discuss
this
> with the dragons :-)
>
> After quite some deliberation I've decided that this is "new feature"
> and "too late for 2.5", so not applied to release/2.5 - it was not an easy
> decision, as this could indeed be a very useful feature for server
operators
> running "distribution provided" binaries, which usually is "2.5". OTOH,
at some
> point we want to get 2.5.0 *out*, and if we keep adding features, we need
more
> RCs, and then we'll never get there. OTOH, again, this is a fairly
isolated change
> - so if enough users yell at me that THEY MUST HAVE THIS, we could put it
in
> 2.5.1 or 2.5.2 ("small and isolated new features can be done in
small-numbered
> subreleases").
>
> Also, we do want 2.6.0 to happen "soonish" this time, like "in one year,
not in
> four years". Let's see...
>
> commit 3b04c34de140355b5b1d4ed85f7ccf1db9b0135d
> Author: Vladislav Grishenko
> Date: Mon Oct 5 05:51:14 2020 +0500
>
> Support X509 field list to be username
>
> Signed-off-by: Vladislav Grishenko
> Acked-by: Arne Schwabe
> Message-Id: <20201005005114.13619-1-themi...@yandex-team.ru>
> URL: https://www.mail-archive.com/openvpn-
> de...@lists.sourceforge.net/msg21168.html
> Signed-off-by: Gert Doering
>
>
> --
> kind regards,
>
> Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v5] Support X509 field list to be username
Hi Arne,
> From: Arne Schwabe
> Sent: Monday, October 5, 2020 1:26 PM
> Am 05.10.20 um 02:51 schrieb Vladislav Grishenko:
> > OpenVPN has the ability to choose different X509 field in case "CN"
> > can not be use used to be unique connected username since commit
> > 935c62be9c0c8a256112df818bfb8470586a23b6 "Choose a different field in
> > X509 to be username".
> > Unfortunately it's not enough in case when client has multiple and
> > valid certificates from PKI for different devices (ex. laptop, mobile,
> > etc) with the same CN/UID.
> >
> > Having --duplicate-cn as a workaround helps only partially: clients
> > can be connected, but it breaks coexistance with
> > --ifconfig-pool-persist, --client-config-dir and opens doors to DoS
> > possibility since same client device (with the same cert) being
> > reconnected no more replaces previously connected session, so it can
> > exhaust server resources (ex. address pool) and can prevent other clients
> > to be
> connected.
> >
> > With this patch, multiple X509 fields incl. "serialNumber" can be
> > chosen to be username with --x509-username-field parameters, they will
> > be concatened into the one username using '_' separator. As long as
> > the resulting username is unique, --duplicate-cn will not be required.
> > Default field is preserved as "CN".
> >
> > Openssl backend is the only supported, since so far MbedTLS has no
> > --x509-username-field support at all.
> >
> > v2: conform C99, man update, fix typos
> > v3: reuse buffer methods, drop delimiter define, use memcpy
> > v4: man update, change separator "_" to avoid path issues on windows
> > v5: mention collision possibility with "_" separator in man
> > capitalize hex serialNumber value
>
> > diff --git a/src/openvpn/ssl_verify_openssl.c
> > b/src/openvpn/ssl_verify_openssl.c
> > index 454efeec..34e1de01 100644
> > --- a/src/openvpn/ssl_verify_openssl.c
> > +++ b/src/openvpn/ssl_verify_openssl.c
> > @@ -269,6 +269,21 @@ backend_x509_get_username(char *common_name,
> int cn_len,
> > return FAILURE;
> > }
> > }
> > +else if (strcmp(LN_serialNumber,x509_username_field) == 0)
> > +{
>
> Whitespace error after ,
Style copy-paste from the line above, will fix both places in v6 version. Thanks
https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/ssl_verify_openssl.c#L265
>
> Otherwise the patch looks good now. (We had informal reviews on IRC that
> prompted the newer versions)
>
> Acked-By: Arne Schwabe
--
Best Regards, Vladislav Grishenko
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v5] Support X509 field list to be username
OpenVPN has the ability to choose different X509 field in case "CN" can
not be use used to be unique connected username since commit
935c62be9c0c8a256112df818bfb8470586a23b6 "Choose a different field in
X509 to be username".
Unfortunately it's not enough in case when client has multiple and
valid certificates from PKI for different devices (ex. laptop,
mobile, etc) with the same CN/UID.
Having --duplicate-cn as a workaround helps only partially: clients can
be connected, but it breaks coexistance with --ifconfig-pool-persist,
--client-config-dir and opens doors to DoS possibility since same client
device (with the same cert) being reconnected no more replaces previously
connected session, so it can exhaust server resources (ex. address pool)
and can prevent other clients to be connected.
With this patch, multiple X509 fields incl. "serialNumber" can be chosen
to be username with --x509-username-field parameters, they will be
concatened into the one username using '_' separator. As long as the
resulting username is unique, --duplicate-cn will not be required.
Default field is preserved as "CN".
Openssl backend is the only supported, since so far MbedTLS has no
--x509-username-field support at all.
v2: conform C99, man update, fix typos
v3: reuse buffer methods, drop delimiter define, use memcpy
v4: man update, change separator "_" to avoid path issues on windows
v5: mention collision possibility with "_" separator in man
capitalize hex serialNumber value
Signed-off-by: Vladislav Grishenko
---
doc/man-sections/tls-options.rst | 21 ++
src/openvpn/init.c | 6 ++--
src/openvpn/options.c| 49 +---
src/openvpn/options.h| 4 +--
src/openvpn/ssl_common.h | 6 +++-
src/openvpn/ssl_verify.c | 49 +---
src/openvpn/ssl_verify_openssl.c | 15 ++
7 files changed, 104 insertions(+), 46 deletions(-)
diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index 8c2db7cd..5dd20013 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -632,20 +632,23 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
options can be defined to track multiple attributes.
--x509-username-field args
- Field in the X.509 certificate subject to be used as the username
- (default :code:`CN`).
+ Fields in the X.509 certificate subject to be used as the username
+ (default :code:`CN`). If multiple fields are specified their values
+ will be concatenated into the one username using :code:`_` symbol
+ as a separator.
Valid syntax:
::
- x509-username-field [ext:]fieldname
+ x509-username-field [ext:]fieldname [[ext:]fieldname...]
- Typically, this option is specified with **fieldname** as
+ Typically, this option is specified with **fieldname** arguments as
either of the following:
::
x509-username-field emailAddress
x509-username-field ext:subjectAltName
+ x509-username-field CN serialNumber
The first example uses the value of the :code:`emailAddress` attribute
in the certificate's Subject field as the username. The second example
@@ -653,16 +656,22 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
``fieldname`` :code:`subjectAltName` be searched for an rfc822Name
(email) field to be used as the username. In cases where there are
multiple email addresses in :code:`ext:fieldname`, the last occurrence
- is chosen.
+ is chosen. The last example uses the value of the :code:`CN` attribute
+ in the Subject field, combined with the :code:`_` separator and the
+ hexadecimal representation of the certificate's :code:`serialNumber`.
When this option is used, the ``--verify-x509-name`` option will match
against the chosen ``fieldname`` instead of the Common Name.
Only the :code:`subjectAltName` and :code:`issuerAltName` X.509
- extensions are supported.
+ extensions and :code:`serialNumber` X.509 attribute are supported.
**Please note:** This option has a feature which will convert an
all-lowercase ``fieldname`` to uppercase characters, e.g.,
:code:`ou` -> :code:`OU`. A mixed-case ``fieldname`` or one having the
:code:`ext:` prefix will be left as-is. This automatic upcasing feature is
deprecated and will be removed in a future release.
+
+ Non-compliant symbols are being replaced with the :code:`_` symbol, same as
+ the field separator, so concatenating multiple fields with such or :code:`_`
+ symbols can potentially lead to username collisions.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 31ecadcc..f87c11e7 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2916,14 +2916,14 @@ do_init_crypto_tls(struct context *c, const unsigned
int flags)
to.crl_file_inline = options->crl_file_inline;
to.ssl_flags = options->
[Openvpn-devel] [PATCH v8] Add DNS SRV remote host discovery support
d for historical
reasons.
- man update
v5: rebase against upstream with connection advancing fix
- allow management skip/accept for remote service hosts as for --remote
- improve compatibility with a way "--persist-remote-ip" is handled
- ensure max line length is 80
v6:
- pick out code-style conformant changes into separate patch
- add more options checks and comments
v7:
- prefer line breaks before long string parameters
- use win32/posix suffixes for query_servinfo
v8:
- rework compatibility with --preresolve and --persist-remote-ip
- fix dns data structures leak on wine/win32
- add priority and weight logging
Signed-off-by: Vladislav Grishenko
---
configure.ac| 2 +-
doc/man-sections/client-options.rst | 117 +++-
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 | 8 +-
src/openvpn/options.c | 268 +++--
src/openvpn/options.h | 4 +
src/openvpn/socket.c| 854 +++-
src/openvpn/socket.h| 58 +-
src/openvpn/syshead.h | 5 +
13 files changed, 1328 insertions(+), 81 deletions(-)
diff --git a/configure.ac b/configure.ac
index ebb32204..22088aaf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -477,7 +477,7 @@ SOCKET_INCLUDES="
"
AC_CHECK_HEADERS(
- [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
+ [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
,
,
[[${SOCKET_INCLUDES}]]
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index af21fbcd..2ed9655d 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -307,10 +307,117 @@ configuration.
specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
addresses, in the order getaddrinfo() returns them.
+--remote-srv args
+ Remote DNS SRV domain, service name and protocol.
+
+ Valid syntaxes:
+ ::
+
+ remote-srv domain
+ remote-srv domain service
+ remote-srv domain service proto
+
+ 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 `RFC 2782 <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.
+
+ The ``service`` argument indicates the symbolic name of the desired
+ service. By default it is :code:`openvpn` registered service name.
+
+ The ``proto`` argument indicates the symbolic name of the desired
+ protocol and the protocol to use when connecting with the remote, and
+ may either be :code:`tcp`, :code:`udp` or special value :code:`auto`
+ used by default to try both. In this case all the discovered remote
+ hosts will be ordered by server selection mechanism regardless their
+ protocol. To enforce IPv4 or IPv6 connections add a :code:`4` or
+ :code:`6` suffix; like :code:`udp4` / :code:`udp6` / :code:`tcp4` /
+ :code:`tcp6` / :code:`auto4` / :code:`auto6`.
+
+ On the client, multiple ``--remote-srv`` options may be specified for
+ redundancy, each referring to a different DNS SRV record name, in the
+ order specified by the list of ``--remote-srv`` options. Specifying
+ multiple ``--remote-srv`` options for this purpose is a special case
+ of the more general connection-profile feature. See the
+ documentation below.
+
+ The client will move on to the next DNS SRV record in the ordered list,
+ in the event of connection failure. Note that at any given time, the
+ OpenVPN client will at most be connected to one server.
+
+ If particular DNS SRV record next resolves to multiple IP addresses,
+ OpenVPN will try them in the order that the system getaddrinfo()
+ presents them, so prioritization and DNS randomization is done
[Openvpn-devel] [PATCH v7] Add DNS SRV remote host discovery support
cted, keep related code disabled for historical reasons.
man update
v5: rebase against upstream with connection advancing fix
allow management skip/accept for exact remote service hosts as for --remote
improve compatibility with a way "--persist-remote-ip" is handled
ensure max line length is 80
v6: pick out code-style conformant changes into separate patch
add more options checks and comments
fix typos
v7: prefer line breaks before long string parameters
use win32/posix suffixes for query_servinfo
Signed-off-by: Vladislav Grishenko
---
configure.ac| 2 +-
doc/man-sections/client-options.rst | 117 +++-
doc/management-notes.txt| 6 +
src/openvpn/Makefile.am | 2 +-
src/openvpn/buffer.h| 5 -
src/openvpn/errlevel.h | 1 +
src/openvpn/init.c | 66 ++-
src/openvpn/openvpn.vcxproj | 8 +-
src/openvpn/options.c | 268 +++--
src/openvpn/options.h | 4 +
src/openvpn/socket.c| 859 +++-
src/openvpn/socket.h| 56 +-
src/openvpn/syshead.h | 5 +
13 files changed, 1320 insertions(+), 79 deletions(-)
diff --git a/configure.ac b/configure.ac
index ebb32204..22088aaf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -477,7 +477,7 @@ SOCKET_INCLUDES="
"
AC_CHECK_HEADERS(
- [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
+ [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
,
,
[[${SOCKET_INCLUDES}]]
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index af21fbcd..2ed9655d 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -307,10 +307,117 @@ configuration.
specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
addresses, in the order getaddrinfo() returns them.
+--remote-srv args
+ Remote DNS SRV domain, service name and protocol.
+
+ Valid syntaxes:
+ ::
+
+ remote-srv domain
+ remote-srv domain service
+ remote-srv domain service proto
+
+ 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 `RFC 2782 <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.
+
+ The ``service`` argument indicates the symbolic name of the desired
+ service. By default it is :code:`openvpn` registered service name.
+
+ The ``proto`` argument indicates the symbolic name of the desired
+ protocol and the protocol to use when connecting with the remote, and
+ may either be :code:`tcp`, :code:`udp` or special value :code:`auto`
+ used by default to try both. In this case all the discovered remote
+ hosts will be ordered by server selection mechanism regardless their
+ protocol. To enforce IPv4 or IPv6 connections add a :code:`4` or
+ :code:`6` suffix; like :code:`udp4` / :code:`udp6` / :code:`tcp4` /
+ :code:`tcp6` / :code:`auto4` / :code:`auto6`.
+
+ On the client, multiple ``--remote-srv`` options may be specified for
+ redundancy, each referring to a different DNS SRV record name, in the
+ order specified by the list of ``--remote-srv`` options. Specifying
+ multiple ``--remote-srv`` options for this purpose is a special case
+ of the more general connection-profile feature. See the
+ documentation below.
+
+ The client will move on to the next DNS SRV record in the ordered list,
+ in the event of connection failure. Note that at any given time, the
+ OpenVPN client will at most be connected to one server.
+
+ If particular DNS SRV record next resolves to multiple IP addresses,
+ OpenVPN will try them in the order that the system getaddrinfo()
+ presents them, so prioritization and DNS randomization is done by the
+ system library. Unless an IP version is forced by the protocol
+ specification (4
Re: [Openvpn-devel] [PATCH applied] Re: Selectively reformat too long lines
Gert, thank you > We need to cover this in the style guide, to make clear which variant is preferred > (if the text fits into one line if putting it onto its own line, this is a different > story). Indeed, it needs to be written down, including what to do with existing code on changes near around. In this case I had to follow Antonio suggestion about the breaks, previous version w/o them hasn't pass review. As for blame, most of git ui tools allows to traverse blame in depth, incl. tig - console git shell, anyway any refactoring brings the same issue. -- Best Regards, Vladislav Grishenko > -Original Message- > From: Gert Doering > Sent: Saturday, October 3, 2020 12:33 AM > To: Vladislav Grishenko > Cc: openvpn-devel@lists.sourceforge.net > Subject: [PATCH applied] Re: Selectively reformat too long lines > > Your patch has been applied to the master and release/2.5 branch. > > I am not particularily happy about merging this sort of "cleanup" > patch so late before 2.5 release - *but* I have very carefully checked that it's > really all just re-wrapping long function calls, or long text strings, so the danger > to 2.5 stability is minimal. OTOH, not merging this to release/2.5 means more > work for all future bugfixes that happen to be close enough to one of these > changes so the "diff context" is changed. So, in it goes. > > As a side note - but too late to stop it, as Arne and Antonio have spoken - I do > not think this sort of change is actually making the code *more* readable: > > -msg(M_WARN, "DEPRECATED OPTION: --inetd mode is deprecated " > -"and will be removed in OpenVPN 2.6"); > +msg(M_WARN, > +"DEPRECATED OPTION: --inetd mode is deprecated and will be removed > " > +"in OpenVPN 2.6"); > > it's basically making 3 lines out of two, while the text still needs to be wrapped. > So instead of "two lengthy" lines, we now have "one very short line, one very > long line, and one short line again" - I find that *less* readable, and since the old > code conformed to the 80 character limit, it's commit noise which makes "git > blame" harder to follow, with questionable benefit. > > We need to cover this in the style guide, to make clear which variant is preferred > (if the text fits into one line if putting it onto its own line, this is a different > story). > > > commit a5409c0d34bf02cacdee61d61ba7b3e1f72e132f (master) commit > c055e28654f340c617a2b0eece7fa28c79c1a9fc (release/2.5) > Author: Vladislav Grishenko > Date: Thu Sep 24 14:10:04 2020 +0500 > > Selectively reformat too long lines > > Signed-off-by: Vladislav Grishenko > Acked-by: Arne Schwabe > Message-Id: <20200924091004.29065-1-themi...@yandex-team.ru> > URL: https://www.mail-archive.com/openvpn- > de...@lists.sourceforge.net/msg21083.html > Signed-off-by: Gert Doering > > > -- > kind regards, > > Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Add CRL extractor script for --crl-verify dir mode
When --crl-verify is enabled, specified CRL file gets reloaded on
every client connection. With huge CRL files it may take a significant
amount of time - seconds and tens of seconds, during which OpenVPN is
blocked and can't serve existing and/or incoming connections due its
singlethread nature.
In alternative mode --crl-verify option takes directory containing
files named as decimal serial numbers of the revoked certificates and
'dir' flag, revoked certificate check is being done by checking the
presence of client's certificate number in that directory.
This script allow to perform incremental extraction of revoked serial
numbers from CRL by adding absent ones and removing excess ones.
Usage example:
extractcrl.py -f pem /path/to/crl.pem /path/to/outdir
extractcrl.py -f der /path/to/crl.crl /path/to/outdir
cat /path/to/crl.pem | extractcrl.py -f pem - /path/to/outdir
cat /path/to/crl.crl | extractcrl.py -f der - /path/to/outdir
Output example:
Loaded: 309797 revoked certs in 4.136s
Scanned: 312006 files in 0.61s
Created: 475 files in 0.05s
Removed: 2684 files in 0.116s
Signed-off-by: Vladislav Grishenko
---
contrib/extract-crl/extractcrl.py | 138 ++
1 file changed, 138 insertions(+)
create mode 100755 contrib/extract-crl/extractcrl.py
diff --git a/contrib/extract-crl/extractcrl.py
b/contrib/extract-crl/extractcrl.py
new file mode 100755
index ..441464e9
--- /dev/null
+++ b/contrib/extract-crl/extractcrl.py
@@ -0,0 +1,138 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+'''
+Helper script for CRL (certificate revocation list) file extraction
+to a directory containing files named as decimal serial numbers of
+the revoked certificates, to be used with OpenVPN CRL directory
+verify mode. To enable this mode, directory and 'dir' flag needs to
+be specified as parameters of '--crl-verify' option.
+For more information refer OpenVPN tls-options.rst.
+
+Usage example:
+extractcrl.py -f pem /path/to/crl.pem /path/to/outdir
+extractcrl.py -f der /path/to/crl.crl /path/to/outdir
+cat /path/to/crl.pem | extractcrl.py -f pem - /path/to/outdir
+cat /path/to/crl.crl | extractcrl.py -f der - /path/to/outdir
+
+Output example:
+Loaded: 309797 revoked certs in 4.136s
+Scanned: 312006 files in 0.61s
+Created: 475 files in 0.05s
+Removed: 2684 files in 0.116s
+'''
+
+import argparse
+import os
+import sys
+import time
+from subprocess import check_output
+
+FILETYPE_PEM = 'PEM'
+FILETYPE_DER = 'DER'
+
+def measure_time(method):
+def elapsed(*args, **kwargs):
+start = time.time()
+result = method(*args, **kwargs)
+return result, round(time.time() - start, 3)
+return elapsed
+
+@measure_time
+def load_crl(filename, format):
+
+def try_openssl_module(filename, format):
+from OpenSSL import crypto
+types = {
+FILETYPE_PEM: crypto.FILETYPE_PEM,
+FILETYPE_DER: crypto.FILETYPE_ASN1
+}
+if filename == '-':
+crl = crypto.load_crl(types[format], sys.stdin.buffer.read())
+else:
+with open(filename, 'rb') as f:
+crl = crypto.load_crl(types[format], f.read())
+return set(int(r.get_serial(), 16) for r in crl.get_revoked())
+
+def try_openssl_exec(filename, format):
+args = ['openssl', 'crl', '-inform', format, '-text']
+if filename != '-':
+args += ['-in', filename]
+serials = set()
+for line in check_output(args, universal_newlines=True).splitlines():
+_, _, serial = line.partition('Serial Number:')
+if serial:
+serials.add(int(serial.strip(), 16))
+return serials
+
+try:
+return try_openssl_module(filename, format)
+except ImportError:
+return try_openssl_exec(filename, format)
+
+@measure_time
+def scan_dir(dirname):
+_, _, files = next(os.walk(dirname))
+return set(int(f) for f in files if f.isdigit())
+
+@measure_time
+def create_new_files(dirname, newset, oldset):
+addset = newset.difference(oldset)
+for serial in addset:
+try:
+with open(os.path.join(dirname, str(serial)), 'xb'): pass
+except FileExistsError:
+pass
+return addset
+
+@measure_time
+def remove_old_files(dirname, newset, oldset):
+delset = oldset.difference(newset)
+for serial in delset:
+try:
+os.remove(os.path.join(dirname, str(serial)))
+except FileNotFoundError:
+pass
+return delset
+
+def check_crlfile(arg):
+if arg == '-' or os.path.isfile(arg):
+return arg
+raise argparse.ArgumentTypeError('No such file "{}"'.format(arg))
+
+def check_outdir(arg):
+if os.path.isdir(arg):
+return arg
+raise argparse.ArgumentTypeError('No such directory: "{}"'.format(arg))
+
+def main():
+parser = argparse.ArgumentParser(desc
[Openvpn-devel] [PATCH v3] Speedup TCP remote hosts connections
For non-blocking TCP/Unix connection, OpenVPN checks was it established in
loop and if not - sleeps or handles management for next one second. Since
the first check is made right after the connection attempt, it will likely
be always unsuccessful, causing redundant wait for one or more seconds:
00:00:00.667607 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.667713 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now in
progress)
00:00:00.667832 poll([{fd=5, events=POLLOUT}], 1, 0) = 0 (Timeout)
00:00:00.667954 nanosleep({tv_sec=1, tv_nsec=0}, 0x7fff52450270) = 0
00:00:01.668608 poll([{fd=5, events=POLLOUT}], 1, 0) = 1 ([{fd=5,
revents=POLLOUT}])
After this patch openvpn_connect() will perform blocking wait for connection
establishment (if possible) and just check for management events once in one
second (if management enabled) w/o sleep. This speedups TCP/Unix connection
establishment and provides almost real connection time that can be used for
detection of the fastest remote server in subsequent patches:
00:00:00.790510 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.790616 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now in
progress)
00:00:00.790877 poll([{fd=5, events=POLLOUT}], 1, 1000) = 0 (Timeout)
00:00:01.792880 poll([{fd=5, events=POLLOUT}], 1, 1000) = 1 ([{fd=5,
revents=POLLOUT}])
Or, with management interface enabled:
00:00:00.906421 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.906527 connect(6, {...}, 16) = -1 EINPROGRESS (Operation now in
progress)
00:00:00.906779 poll([{fd=6, events=POLLOUT}], 1, 1000) = 0 (Timeout)
00:00:01.910418 poll([{fd=3, events=POLLIN|POLLPRI}], 1, 0) = 0 (Timeout)
00:00:01.911365 poll([{fd=6, events=POLLOUT}], 1, 1000) = 0 ([{fd=6,
revents=POLLOUT}])
v2: cosmetics, decrease connection_timeout to avoid wait more than it
v3: teach management_sleep() to handle zero timeout and reject negative
use 1s timeout for connection and 0s timeout for management events
Signed-off-by: Vladislav Grishenko
---
src/openvpn/manage.c | 30 +++---
src/openvpn/socket.c | 6 +++---
2 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 898cb3b3..ac142177 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -3310,12 +3310,17 @@ man_block(struct management *man, volatile int
*signal_received, const time_t ex
if (man_standalone_ok(man))
{
+/* expire time can be already overdue, for this case init zero
+ * timeout to avoid waiting first time and exit loop early with
+ * either obtained event or timeout.
+ */
+tv.tv_usec = 0;
+tv.tv_sec = 0;
+
while (true)
{
event_reset(man->connection.es);
management_socket_set(man, man->connection.es, NULL, NULL);
-tv.tv_usec = 0;
-tv.tv_sec = 1;
if (man_check_for_signals(signal_received))
{
status = -1;
@@ -3343,6 +3348,10 @@ man_block(struct management *man, volatile int
*signal_received, const time_t ex
}
break;
}
+
+/* wait one second more */
+tv.tv_sec = 1;
+tv.tv_usec = 0;
}
}
return status;
@@ -3444,7 +3453,7 @@ management_event_loop_n_seconds(struct management *man,
int sec)
/* set expire time */
update_time();
-if (sec)
+if (sec >= 0)
{
expire = now + sec;
}
@@ -3474,7 +3483,7 @@ management_event_loop_n_seconds(struct management *man,
int sec)
/* revert state */
man->persist.standalone_disabled = standalone_disabled_save;
}
-else
+else if (sec > 0)
{
sleep(sec);
}
@@ -4117,11 +4126,15 @@ log_history_ref(const struct log_history *h, const int
index)
void
management_sleep(const int n)
{
-if (management)
+if (n < 0)
+{
+return;
+}
+else if (management)
{
management_event_loop_n_seconds(management, n);
}
-else
+else if (n > 0)
{
sleep(n);
}
@@ -4132,7 +4145,10 @@ management_sleep(const int n)
void
management_sleep(const int n)
{
-sleep(n);
+if (n > 0)
+{
+sleep(n);
+}
}
#endif /* ENABLE_MANAGEMENT */
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 76bdbfc5..155780e3 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1470,14 +1470,14 @@ openvpn_connect(socket_descriptor_t sd,
struct pollfd fds[1];
fds[0].fd = sd;
fds[0].events = POLLOUT;
-status = poll(fds, 1, 0);
+status = poll(fds, 1, (connect_timeout > 0) ? 1000 : 0);
#else
fd_set writes;
struct timeval tv;
FD_ZERO();
openvpn_fd_set(sd, );
Re: [Openvpn-devel] [PATCH] Speedup TCP remote hosts connections
Hi Arne, > Okay, I looked at the code and you are right. That code is a mess. It > basically > calls select/pool with a timeout of 0, then actively does select/pool in > management for 1s and does this in a loop. That this totally wrong indeed. > > I think we should be more radical here. Always do a 1s poll/select timeout and > do a 0s management sleep/poll. That will also make the code a bit simpler. Othe thing, management_sleep(0) results in infinite wait, so I;ve changed api in subsequent patch. Now, management_sleep() can handle only non-negative value and managemet_event_loop_n_seconds() can take negative values to have infinite wait, if necessary. Since there were no negative or zero parameter for management_sleep() users, no side effect behavior change is expected. Seems, a bit simpler can't be achieved :) -- Best Regards, Vladislav Grishenko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Speedup TCP remote hosts connections
For non-blocking TCP/Unix connection, OpenVPN checks was it established in
loop and if not - sleeps or handles management for next one second. Since
the first check is made right after the connection attempt, it will likely
be always unsuccessful, resulting in redundant wait for one or more seconds:
00:00:00.667607 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.667713 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now in
progress)
00:00:00.667832 poll([{fd=5, events=POLLOUT}], 1, 0) = 0 (Timeout)
00:00:00.667954 nanosleep({tv_sec=1, tv_nsec=0}, 0x7fff52450270) = 0
00:00:01.668608 poll([{fd=5, events=POLLOUT}], 1, 0) = 1 ([{fd=5,
revents=POLLOUT}])
With this patch openvpn_connect() will wait for connection establishment for
the first second (if possible) and will return success immediately: this
speedups connection and provides real connection time that can be used for
detection of the fastest remote server in subsequent patches:
00:00:00.019133 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.019265 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now in
progress)
00:00:00.019388 poll([{fd=5, events=POLLOUT}], 1, 1000) = 1 ([{fd=5,
revents=POLLOUT}])
If connection still can't established - this should be treated as either too
slow/far or non-responding server, so imprecise connection checks every next
one second in loop will be performed as usual.
v2: cosmetics, decrease connection_timeout to avoid wait more than it in total
Signed-off-by: Vladislav Grishenko
---
src/openvpn/socket.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 76bdbfc5..049216ff 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1464,13 +1464,27 @@ openvpn_connect(socket_descriptor_t sd,
#endif
)
{
+/* Non-blocking connection is in the progress and can be likely
+ * established within the first second with no need for imprecise
+ * sleeping in loop.
+ */
+bool connect_wait = (connect_timeout > 0);
while (true)
{
#if POLL
+int timeout = 0;
struct pollfd fds[1];
+
fds[0].fd = sd;
fds[0].events = POLLOUT;
-status = poll(fds, 1, 0);
+if (connect_wait)
+{
+connect_wait = false;
+connect_timeout--;
+timeout = 1000;
+}
+
+status = poll(fds, 1, timeout);
#else
fd_set writes;
struct timeval tv;
@@ -1479,6 +1493,12 @@ openvpn_connect(socket_descriptor_t sd,
openvpn_fd_set(sd, );
tv.tv_sec = 0;
tv.tv_usec = 0;
+if (connect_wait)
+{
+connect_wait = false;
+connect_timeout--;
+tv.tv_sec = 1;
+}
status = select(sd + 1, NULL, , NULL, );
#endif
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Speedup TCP remote hosts connections
For non-blocking TCP/Unix connection, OpenVPN checks was it established in
loop and if not - sleeps or handles management for next one second. Since
the first check is made right after the connection attempt, it will likely
be always unsuccessful, resulting in redundant wait for one or more seconds:
00:00:00.667607 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.667713 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now in
progress)
00:00:00.667832 poll([{fd=5, events=POLLOUT}], 1, 0) = 0 (Timeout)
00:00:00.667954 nanosleep({tv_sec=1, tv_nsec=0}, 0x7fff52450270) = 0
00:00:01.668608 poll([{fd=5, events=POLLOUT}], 1, 0) = 1 ([{fd=5,
revents=POLLOUT}])
With this patch openvpn_connect() will wait for connection establishment for
the first second (if possible) and will return success immediately: this
speedups connection and provides real connection time that can be used for
detection of the fastest remote server in subsequent patches:
00:00:00.019133 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.019265 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now in
progress)
00:00:00.019388 poll([{fd=5, events=POLLOUT}], 1, 1000) = 1 ([{fd=5,
revents=POLLOUT}])
If connection still can't established - this should be treated as either too
slow/far or non-responding server, so imprecise connection checks every next
one second in loop will be performed as usual.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/socket.c | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 76bdbfc5..66b1e4e2 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1464,13 +1464,26 @@ openvpn_connect(socket_descriptor_t sd,
#endif
)
{
+/* Non-blocking connection is in the progress and can be likely
+ * established within the first second with no need for imprecise
+ * sleeping in loop.
+ */
+bool connect_wait = (connect_timeout > 0);
while (true)
{
#if POLL
+int timeout = 0;
struct pollfd fds[1];
+
fds[0].fd = sd;
fds[0].events = POLLOUT;
-status = poll(fds, 1, 0);
+if (connect_wait)
+{
+connect_wait = false;
+timeout = 1000;
+}
+
+status = poll(fds, 1, timeout);
#else
fd_set writes;
struct timeval tv;
@@ -1479,6 +1492,11 @@ openvpn_connect(socket_descriptor_t sd,
openvpn_fd_set(sd, );
tv.tv_sec = 0;
tv.tv_usec = 0;
+if (connect_wait)
+{
+connect_wait = false;
+tv.tv_sec = 1;
+}
status = select(sd + 1, NULL, , NULL, );
#endif
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v6 1/2] Selectively reformat too long lines
Hi Antonio,
Here's I have aligned the last line to add next new proto, already aligned.
Yes, you’re right “UDPv6” also needs to be aligned, and space needs to be added
for all lines, thank you.
V7 is sent
--
Best Regards, Vladislav Grishenko
> -Original Message-
> From: Antonio Quartulli
> Sent: Thursday, September 24, 2020 1:12 PM
> To: Vladislav Grishenko ; openvpn-
> de...@lists.sourceforge.net
> Subject: Re: [Openvpn-devel] [PATCH v6 1/2] Selectively reformat too long
> lines
>
> Hi,
>
> On 20/09/2020 22:57, Vladislav Grishenko wrote:
> > @@ -3170,7 +3179,7 @@ static const struct proto_names proto_names[] = {
> > {"udp6","UDPv6", AF_INET6, PROTO_UDP},
> > {"tcp6-server","TCPv6_SERVER", AF_INET6, PROTO_TCP_SERVER},
> > {"tcp6-client","TCPv6_CLIENT", AF_INET6, PROTO_TCP_CLIENT},
> > -{"tcp6","TCPv6", AF_INET6, PROTO_TCP},
> > +{"tcp6", "TCPv6", AF_INET6, PROTO_TCP},
>
> What are you actually fixing here? Adding a tab?
> I feel there should be a space after each ',', but that is probably out of
> the scope
> of this patch?
>
>
> --
> Antonio Quartulli
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v7] Selectively reformat too long lines
Per https://community.openvpn.net/openvpn/wiki/CodeStyle the maximum line
length is 80 characters. This patch allows to split upcoming changes into
CodeStyle-conformant (w/o real code change) and more feature-specific.
Upcoming changes adds new PROTO_AUTO, so existing proto_names array is
reformatted as well.
v7: prefer line breaks before long string parameters
reformat proto_names array
Signed-off-by: Vladislav Grishenko
---
src/openvpn/init.c| 3 +-
src/openvpn/options.c | 80 +--
src/openvpn/socket.c | 52 +---
3 files changed, 89 insertions(+), 46 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d1ad5c8f..31ecadcc 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3646,7 +3646,8 @@ do_close_link_socket(struct context *c)
&& ( (c->options.persist_remote_ip)
||
( c->sig->source != SIG_SOURCE_HARD
- && ((c->c1.link_socket_addr.current_remote &&
c->c1.link_socket_addr.current_remote->ai_next)
+ && ((c->c1.link_socket_addr.current_remote
+ && c->c1.link_socket_addr.current_remote->ai_next)
|| c->options.no_advance))
)))
{
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 4b22d3d9..92f446e7 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1983,7 +1983,8 @@ connection_entry_load_re(struct connection_entry *ce,
const struct remote_entry
}
static void
-options_postprocess_verify_ce(const struct options *options, const struct
connection_entry *ce)
+options_postprocess_verify_ce(const struct options *options,
+ const struct connection_entry *ce)
{
struct options defaults;
int dev = DEV_TYPE_UNDEF;
@@ -2011,7 +2012,9 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
*/
if (ce->proto == PROTO_TCP)
{
-msg(M_USAGE, "--proto tcp is ambiguous in this context. Please
specify --proto tcp-server or --proto tcp-client");
+msg(M_USAGE,
+"--proto tcp is ambiguous in this context. Please specify "
+"--proto tcp-server or --proto tcp-client");
}
/*
@@ -2051,8 +2054,9 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
if (options->inetd)
{
-msg(M_WARN, "DEPRECATED OPTION: --inetd mode is deprecated "
-"and will be removed in OpenVPN 2.6");
+msg(M_WARN,
+"DEPRECATED OPTION: --inetd mode is deprecated and will be removed
"
+"in OpenVPN 2.6");
}
if (options->lladdr && dev != DEV_TYPE_TAP)
@@ -2065,7 +2069,9 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
*/
if (options->ce.tun_mtu_defined && options->ce.link_mtu_defined)
{
-msg(M_USAGE, "only one of --tun-mtu or --link-mtu may be defined (note
that --ifconfig implies --link-mtu %d)", LINK_MTU_DEFAULT);
+msg(M_USAGE,
+"only one of --tun-mtu or --link-mtu may be defined (note that "
+"--ifconfig implies --link-mtu %d)", LINK_MTU_DEFAULT);
}
if (!proto_is_udp(ce->proto) && options->mtu_test)
@@ -2092,18 +2098,23 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
if (string_defined_equal(ce->remote, options->ifconfig_local)
|| string_defined_equal(ce->remote, options->ifconfig_remote_netmask))
{
-msg(M_USAGE, "--local and --remote addresses must be distinct from
--ifconfig addresses");
+msg(M_USAGE,
+"--local and --remote addresses must be distinct from --ifconfig "
+"addresses");
}
if (string_defined_equal(ce->local, options->ifconfig_local)
|| string_defined_equal(ce->local, options->ifconfig_remote_netmask))
{
-msg(M_USAGE, "--local addresses must be distinct from --ifconfig
addresses");
+msg(M_USAGE,
+"--local addresses must be distinct from --ifconfig addresses");
}
-if (string_defined_equal(options->ifconfig_local,
options->ifconfig_remote_netmask))
+if (string_defined_equal(options->ifconfig_local,
+ options->ifconfig_remote_netmask))
{
-msg(M_USAGE, "local and remote/netmask --ifconfig addresses must be
different");
+msg(M_USAGE,
+"local and remote/netmask --ifconfig addresses must be different");
}
if (ce->bind_defined && !ce->bind_local)
@@ -2113,12 +2124,1
[Openvpn-devel] [PATCH v3] Fix update_time() and openvpn_gettimeofday() coexistence
With TIME_BACKTRACK_PROTECTION defined, openvpn_gettimeofday() uses and
updates global variable "now_usec" along with "now" only if current time
is ahead of the previsouly stored, taking nanoseconds into account.
But, update_time() function updates only "now" leaving "now_usec" as
is with any previously value stored.
This breaks openvpn_gettimeofday() results and leads to time jumps in the
future within one second, can affect shaper and user timers.
Example:
100.900 openvpn_gettimeofday():
now set to 100s, now_usec set to 900ns, stored time is 100.900
101.300 update_time():
now set to 101s, but now_usec is not updated and still 900ns, stored
time jumps to the future 101.900
101.600 openvpn_gettimeofday():
current time 101.600 is in the past relatively stored time 101.900,
now & now_usec variables are not updated, returned time 101.900 is
still and again incorrect
102.100 openvpn_gettimeofday():
current time 102.100 is no longer in the past relatively stored time
101.900, so now & now_usec get updated with wrong time delta from
previous openvpn_gettimeofday() call or now/now_usec math
Since update_time() and openvpn_gettimeofday() calls are mixed in runtime,
there're several options to fix the things:
1. Allow update_time() to reset "now_usec" value backward to 0, since it's
used directly only in time ajusting and always invalidate it in
openvpn_gettimeofday() unless time has drifted backwards.
Quick solution that only fixes openvpn_gettimeofday() and keeps current
level of time performance and backward-protection handling way.
2. Switch update_time() to gettimeofday() not only for windows, but for all
platforms: "now_usec" will be updated accordingly. As a disadvantage,
gettimeofday() may have performance penalty on older or platforms w/o VDSO
where expensive kernel syscall will be made. And it will still need time
adjusting code, doubt it's feasible.
3. Switch update_time() and openvpn_gettimeofday() to clock_gettime() on
Linux/BSD platforms with CLOCK_REALTIME_FAST/CLOCK_REALTIME_COARSE
clock sources. According tests it'll be faster with VDSO than gettimeofday()
or CLOCK_REALTIME/CLOCK_REALTIME_PRECISE, but still may require adjusting
code to protect from time jumps on devices with no RTC (ex. routers) where
NTP is the only way to get correct time after boot. Since not every *libc
have clock_gettime() and corresponding CLOCK_* defines and/or running
kernel may have no VDSO/corresponding CLOCK_* support - related autotools
checks and fallback code can still be necessary.
4. Switch update_time() and openvpn_gettimeofday() to clock_gettime() on
Linux/BSD platforms with CLOCK_MONOTONIC_FAST/CLOCK_MONOTONIC_COARSE
clock sources. This may allow to get rid of time adjusting code at all
with the acceptable performance on modern systems, but may still require
to fallback to gettimeofday() with adj friends on older platforms (most
likely to be Linux CPE/routers). From the effort point of view, splitting
the whole OpenVPN code into realtime/monotonic is most significant and
desired task among the above, several parts still needs to use realtime
due API or storage or output reasons.
This patch implements the first stage only.
v2: move from gettimeofday() (1st way) back to time(), don't check previous
value of "now_usec" in update_usec() instead
v3: recover "now_usec" checks against time jumps within one second, zero it
in update_time() calls instead to pass the check.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/otime.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index a6f7ec25..78d20ba8 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -84,6 +84,7 @@ update_time(void)
openvpn_gettimeofday(, NULL);
#else
update_now(time(NULL));
+now_usec = 0;
#endif
}
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Fix update_time() and openvpn_gettimeofday() coexistance
With TIME_BACKTRACK_PROTECTION defined, openvpn_gettimeofday() uses and
updates global variable "now_usec" along with "now" only if current time
is ahead of the previsouly stored, taking nanoseconds into account.
But, update_time() function updates only "now" leaving "now_usec" as
is with any previously value stored.
This breaks both update_time() and openvpn_gettimeofday() results and
leads to time jumps in the future within one second, can affect shaper
and user timers.
100.900 openvpn_gettimeofday():
now set to 100s, now_usec set to 900ns, stored time is 100.900
101.300 update_time():
now set to 101s, but now_usec is not updated and still 900ns, stored
time jumps to the future 101.900
101.600 openvpn_gettimeofday():
current time 101.600 is in the past relatively stored time 101.900,
now & now_usec variables are not updated, returned time 101.900 is
still and again incorrect
102.100 openvpn_gettimeofday():
current time 102.100 is no longer in the past relatively stored time
101.900, so now & now_usec get updated with wrong time delta from
previous openvpn_gettimeofday() call or now/now_usec math
Since update_time() and openvpn_gettimeofday() calls are mixed in runtime,
there're two ways to fix their coexistance:
* either fix update_time() to always update "now_usec" for example by
moving update_time() to gettimeofday(). Unfortunately, it may bring
performance penalty depending on Arch/OS and vuruallized/hw environmant
according the tests on various Linux, FreeBSD and MacOS versions.
* or, for now, allow update_time() to break "now_usec" value, since it's
used directly only in time ajusting and always invalidate it in
openvpn_gettimeofday() w/o checking the previous and possibly obsolete
value with no performance changes against the current implementation.
This patch implements the second way.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/otime.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index 640168a9..d27d1f9d 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -73,7 +73,7 @@ update_now_usec(struct timeval *tv)
{
const time_t last = now;
update_now(tv->tv_sec);
-if (now > last || (now == last && tv->tv_usec > now_usec))
+if (now >= last)
{
now_usec = tv->tv_usec;
}
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Fix update_time() and openvpn_gettimeofday()
Hi Arne,
> But that also
> implies it is the other way round on other platforms. The update_time
> function is
> called in multiple critical places. Do we have any idea about this?
On Linux with glibc's implementation both time() and gettimeofday() results in
the same clock_gettime() call over VDSO
(https://man7.org/linux/man-pages/man7/vdso.7.html).
On FreeBSD (and so), same if VDSO is used both time() and gettimeofday() will
use clock_gettime too with even additional "tv->tv_usec = 0" for time().
So, on most of architectures there will be no difference after replacing time()
to gettimeofday() except. asm commands for moving 32/64bit tv_usec, it'll be
not visible.
On some acrhs/systems where VDSO is not supported - same syscall will happen.
> I remember
> vaguely that the default gettimeofday in FreeBSD is more accurate than the
> Linux variant (involves a kernel call) but you can request a version similar
> to the
> Linux one by some flag (reading from a shared readonly memory page that is
> read only).
As for clock drifting, it maybe feasible to use clock_gettime() with
CLOCK_MONOTONIC and get rid of homegrown time adj code at all -> returned time
will always be monotonic by design.
At least on supported platforms (!_WIN32).
--
Best Regards, Vladislav Grishenko
> -Original Message-
> From: Arne Schwabe
> Sent: Tuesday, September 22, 2020 1:41 PM
> To: Vladislav Grishenko ; openvpn-
> de...@lists.sourceforge.net
> Subject: Re: [Openvpn-devel] [PATCH] Fix update_time() and
> openvpn_gettimeofday()
>
>
> > --- a/src/openvpn/otime.h
> > +++ b/src/openvpn/otime.h
> > @@ -78,13 +78,9 @@ openvpn_gettimeofday(struct timeval *tv, void *tz)
> > static inline void
> > update_time(void)
> > {
> > -#ifdef _WIN32
> > -/* on _WIN32, gettimeofday is faster than time(NULL) */
> > +/* can't use time(NULL), now_usec needs to be updated */
> > struct timeval tv;
> > openvpn_gettimeofday(, NULL);
> > -#else
> > -update_now(time(NULL));
> > -#endif
> > }
> >
> > #else /* !TIME_BACKTRACK_PROTECTION */
> >
>
> I am hesitant about this change, it mentions that gettimeofday is faster on
> Windows than time(NULL) and we prefer it there for this reason. But that also
> implies it is the other way round on other platforms. The update_time
> function is
> called in multiple critical places. Do we have any idea about this? I remember
> vaguely that the default gettimeofday in FreeBSD is more accurate than the
> Linux variant (involves a kernel call) but you can request a version similar
> to the
> Linux one by some flag (reading from a shared readonly memory page that is
> read only).
>
> Arne
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Fix update_time() and openvpn_gettimeofday()
With TIME_BACKTRACK_PROTECTION defined, openvpn_gettimeofday() uses and
updates global variable "now_usec" along with "now" only if current time
is ahead of the previsouly stored, taking nanoseconds into account.
But, update_time() function updates only "now" leaving "now_usec" as
is with any previously value stored.
This breaks both update_time() and openvpn_gettimeofday() results and
leads to time jumps in the future within one second, can affect shaper
and user timers.
100.900 openvpn_gettimeofday():
now set to 100s, now_usec set to 100ns, stored time is 100.900
101.300 update_time():
now set to 101s, but now_usec is not updated and still 900ns, stored
time jumps to the future 101.900
101.600 openvpn_gettimeofday():
current time 101.600 is in the past relatively stored time 101.900,
now & now_usec variables are not updated, returned time 101.900 is
still and again incorrect
102.100 openvpn_gettimeofday():
current time 102.100 is no longer in the past relatively stored time
101.900, so now & now_usec get updated with wrong time delta from
previous openvpn_gettimeofday() call or now/now_usec math
Since update_time() and openvpn_gettimeofday() calls are mixed in runtime,
to fix their coexistance update_time() must update "now_usec" as well,
calling just update_now() is not enough.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/otime.h | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index a6f7ec25..fab4575c 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -78,13 +78,9 @@ openvpn_gettimeofday(struct timeval *tv, void *tz)
static inline void
update_time(void)
{
-#ifdef _WIN32
-/* on _WIN32, gettimeofday is faster than time(NULL) */
+/* can't use time(NULL), now_usec needs to be updated */
struct timeval tv;
openvpn_gettimeofday(, NULL);
-#else
-update_now(time(NULL));
-#endif
}
#else /* !TIME_BACKTRACK_PROTECTION */
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v6 2/2] Add DNS SRV remote host discovery support
weights greater than 0
were all selected, keep related code disabled for historical reasons.
man update
v5: rebase against upstream with connection advancing fix
allow management skip/accept for exact remote service hosts as for --remote
improve compatibility with a way "--persist-remote-ip" is handled
ensure max line length is 80
v6: pick out code-style conformant changes into separate patch
add more options checks and comments
fix typos
Signed-off-by: Vladislav Grishenko
---
configure.ac| 2 +-
doc/man-sections/client-options.rst | 117 +++-
doc/management-notes.txt| 6 +
src/openvpn/Makefile.am | 2 +-
src/openvpn/buffer.h| 5 -
src/openvpn/errlevel.h | 1 +
src/openvpn/init.c | 66 ++-
src/openvpn/openvpn.vcxproj | 8 +-
src/openvpn/options.c | 262 +++--
src/openvpn/options.h | 4 +
src/openvpn/socket.c| 857 +++-
src/openvpn/socket.h| 56 +-
src/openvpn/syshead.h | 5 +
13 files changed, 1312 insertions(+), 79 deletions(-)
diff --git a/configure.ac b/configure.ac
index ebb32204..22088aaf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -477,7 +477,7 @@ SOCKET_INCLUDES="
"
AC_CHECK_HEADERS(
- [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
+ [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
,
,
[[${SOCKET_INCLUDES}]]
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index af21fbcd..4445b4a8 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -307,10 +307,117 @@ configuration.
specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
addresses, in the order getaddrinfo() returns them.
+--remote-srv args
+ Remote DNS SRV domain, service name and protocol.
+
+ Valid syntaxes:
+ ::
+
+ remote-srv domain
+ remote-srv domain service
+ remote-srv domain service proto
+
+ 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 `RFC 2782 <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.
+
+ The ``service`` argument indicates the symbolic name of the desired
+ service. By default it is :code:`openvpn` registered service name.
+
+ The ``proto`` argument indicates the symbolic name of the desired
+ protocol and the protocol to use when connecting with the remote, and
+ may either be :code:`tcp`, :code:`udp` or special value :code:`auto`
+ used by default to try both. In this case all the discovered remote
+ hosts will be ordered by server selection mechanism regardless their
+ protocol. To enforce IPv4 or IPv6 connections add a :code:`4` or
+ :code:`6` suffix; like :code:`udp4` / :code:`udp6` / :code:`tcp4` /
+ :code:`tcp6` / :code:`auto4` / :code:`auto6`.
+
+ On the client, multiple ``--remote-srv`` options may be specified for
+ redundancy, each referring to a different DNS SRV record name, in the
+ order specified by the list of ``--remote-srv`` options. Specifying
+ multiple ``--remote-srv`` options for this purpose is a special case
+ of the more general connection-profile feature. See the
+ documentation below.
+
+ The client will move on to the next DNS SRV record in the ordered list,
+ in the event of connection failure. Note that at any given time, the
+ OpenVPN client will at most be connected to one server.
+
+ If particular DNS SRV record next resolves to multiple IP addresses,
+ OpenVPN will try them in the order that the system getaddrinfo()
+ presents them, so prioritization and DNS randomization is done by the
+ system library. Unless an IP version is forced by the protocol
+ specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
+ addresses, in the order getaddrinfo() r
[Openvpn-devel] [PATCH v6 1/2] Selectively reformat too long lines
Per https://community.openvpn.net/openvpn/wiki/CodeStyle the maximum line
length is 80 characters. This patch allows to split upcoming changes into
CodeStyle-conformant (w/o real code change) and more feature-specific.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/init.c| 3 ++-
src/openvpn/options.c | 44 +--
src/openvpn/socket.c | 26 +
3 files changed, 50 insertions(+), 23 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d1ad5c8f..31ecadcc 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3646,7 +3646,8 @@ do_close_link_socket(struct context *c)
&& ( (c->options.persist_remote_ip)
||
( c->sig->source != SIG_SOURCE_HARD
- && ((c->c1.link_socket_addr.current_remote &&
c->c1.link_socket_addr.current_remote->ai_next)
+ && ((c->c1.link_socket_addr.current_remote
+ && c->c1.link_socket_addr.current_remote->ai_next)
|| c->options.no_advance))
)))
{
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 4b22d3d9..ed4229c0 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1983,7 +1983,8 @@ connection_entry_load_re(struct connection_entry *ce,
const struct remote_entry
}
static void
-options_postprocess_verify_ce(const struct options *options, const struct
connection_entry *ce)
+options_postprocess_verify_ce(const struct options *options,
+ const struct connection_entry *ce)
{
struct options defaults;
int dev = DEV_TYPE_UNDEF;
@@ -2011,7 +2012,8 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
*/
if (ce->proto == PROTO_TCP)
{
-msg(M_USAGE, "--proto tcp is ambiguous in this context. Please
specify --proto tcp-server or --proto tcp-client");
+msg(M_USAGE, "--proto tcp is ambiguous in this context. "
+"Please specify --proto tcp-server or --proto tcp-client");
}
/*
@@ -2065,7 +2067,8 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
*/
if (options->ce.tun_mtu_defined && options->ce.link_mtu_defined)
{
-msg(M_USAGE, "only one of --tun-mtu or --link-mtu may be defined (note
that --ifconfig implies --link-mtu %d)", LINK_MTU_DEFAULT);
+msg(M_USAGE, "only one of --tun-mtu or --link-mtu may be defined "
+"(note that --ifconfig implies --link-mtu %d)", LINK_MTU_DEFAULT);
}
if (!proto_is_udp(ce->proto) && options->mtu_test)
@@ -2092,18 +2095,22 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
if (string_defined_equal(ce->remote, options->ifconfig_local)
|| string_defined_equal(ce->remote, options->ifconfig_remote_netmask))
{
-msg(M_USAGE, "--local and --remote addresses must be distinct from
--ifconfig addresses");
+msg(M_USAGE, "--local and --remote addresses must be distinct from "
+"--ifconfig addresses");
}
if (string_defined_equal(ce->local, options->ifconfig_local)
|| string_defined_equal(ce->local, options->ifconfig_remote_netmask))
{
-msg(M_USAGE, "--local addresses must be distinct from --ifconfig
addresses");
+msg(M_USAGE, "--local addresses must be distinct from "
+"--ifconfig addresses");
}
-if (string_defined_equal(options->ifconfig_local,
options->ifconfig_remote_netmask))
+if (string_defined_equal(options->ifconfig_local,
+ options->ifconfig_remote_netmask))
{
-msg(M_USAGE, "local and remote/netmask --ifconfig addresses must be
different");
+msg(M_USAGE, "local and remote/netmask --ifconfig addresses "
+"must be different");
}
if (ce->bind_defined && !ce->bind_local)
@@ -2217,11 +2224,14 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
if ((ce->http_proxy_options) && ce->proto != PROTO_TCP_CLIENT)
{
-msg(M_USAGE, "--http-proxy MUST be used in TCP Client mode (i.e.
--proto tcp-client)");
+msg(M_USAGE, "--http-proxy MUST be used in TCP Client mode "
+"(i.e. --proto tcp-client)");
}
+
if ((ce->http_proxy_options) && !ce->http_proxy_options->server)
{
-msg(M_USAGE, "--http-proxy not specified but other http proxy options
present");
+msg(M_USAGE, "--http-proxy not specified but "
+
[Openvpn-devel] [PATCH v5] Add DNS SRV remote host discovery support
weights greater than 0
were all selected, keep related code disabled for historical reasons.
man update
v5: rebase against upstream with connection advancing fix
allow management skip/accept for exact remote service hosts as for --remote
improve compability with a way "--persist-remote-ip" is handled
ensure max line length is 80
Signed-off-by: Vladislav Grishenko
---
configure.ac| 2 +-
doc/man-sections/client-options.rst | 117 +++-
doc/management-notes.txt| 6 +
src/openvpn/Makefile.am | 2 +-
src/openvpn/buffer.h| 5 -
src/openvpn/errlevel.h | 1 +
src/openvpn/init.c | 69 ++-
src/openvpn/openvpn.vcxproj | 8 +-
src/openvpn/options.c | 252 ++--
src/openvpn/options.h | 4 +
src/openvpn/socket.c| 881 +++-
src/openvpn/socket.h| 56 +-
src/openvpn/syshead.h | 5 +
13 files changed, 1324 insertions(+), 84 deletions(-)
diff --git a/configure.ac b/configure.ac
index ebb32204..22088aaf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -477,7 +477,7 @@ SOCKET_INCLUDES="
"
AC_CHECK_HEADERS(
- [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
+ [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
,
,
[[${SOCKET_INCLUDES}]]
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index af21fbcd..ca2f27a5 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -307,10 +307,117 @@ configuration.
specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
addresses, in the order getaddrinfo() returns them.
+--remote-srv args
+ Remote DNS SRV domain, service name and protocol.
+
+ Valid syntaxes:
+ ::
+
+ remote-srv domain
+ remote-srv domain service
+ remote-srv domain service proto
+
+ 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 `RFC 2782 <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.
+
+ The ``service`` argument indicates the symbolic name of the desired
+ service. By default it is :code:`openvpn` registered service name.
+
+ The ``proto`` argument indicates the symbolic name of the desired
+ protocol and the protocol to use when connecting with the remote, and
+ may either be :code:`tcp`, :code:`udp` or special value :code:`auto`
+ used by default to try both. In this case all the discovered remote
+ hosts will be ordered by server selection mechanism regardless their
+ protocol. To enforce IPv4 or IPv6 connections add a :code:`4` or
+ :code:`6` suffix; like :code:`udp4` / :code:`udp6` / :code:`tcp4` /
+ :code:`tcp6` / :code:`auto4` / :code:`auto6`.
+
+ On the client, multiple ``--remote-srv`` options may be specified for
+ redundancy, each referring to a different DNS SRV record name, in the
+ order specified by the list of ``--remote-srv`` options. Specifying
+ multiple ``--remote-srv`` options for this purpose is a special case
+ of the more general connection-profile feature. See the
+ documentation below.
+
+ The client will move on to the next DNS SRV record in the ordered list,
+ in the event of connection failure. Note that at any given time, the
+ OpenVPN client will at most be connected to one server.
+
+ If partucular DNS SRV record next resolves to multiple IP addresses,
+ OpenVPN will try them in the order that the system getaddrinfo()
+ presents them, so priorization and DNS randomization is done by the
+ system library. Unless an IP version is forced by the protocol
+ specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
+ addresses, in the order getaddrinfo() returns them.
+
+ Examples:
+ ::
+
+ remote-srv example.net
+ remote-srv example.net openvpn
+ remote-srv exampl
Re: [Openvpn-devel] [PATCH applied] Re: Fix fatal error at switching remotes (#629)
Hi, Gert > > That "fix for real" is about persist_remote_ip option as far as I > > understand, not directly related to this fatal assert fix. > > Well, the whole preresolve / connection entry "complex" is old and has been > extended and updated a few times, and your SVR patch also builds on top of > that. That's true, I hit this assert for SRV initially, 'coz same advancing logic was used, v5 version is upcoming following this commit. > At some point, refactoring is needed... > (We have some other thing to consider which is even more intrusive - when we > reconnect to a different IP address, and that new IP address is currently routed > into the tunnel, we need to set up new /32 host routes before moving to a new > server can work... openvpn3, as I understand, sets up "all host routes!" right at > the start, but that might or might not be the best solution either) New address is being handled in disconnected (yet) state, so tunnel routes should not be active, since 2.x supports at most one tunnel active. While this is preserved, /32 host route can be made anytime between resolved state and connection attempt, sounds not so intrusive, if I got you right. Meanwhile, "--persist-remote-ip" documented as "Preserve most recently authenticated remote IP address and port number across SIGUSR1 and --ping-restart". Current implementation doesn't follow it precisely, instead it does "Preserve most recently authenticated remote host name and port...", if that remote name resolves into multiple addresses - they will be still iterated. Guess, this is what was meant by "fix by real" > > 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
Re: [Openvpn-devel] [PATCH applied] Re: Fix fatal error at switching remotes (#629)
Thank you a lot, That "fix for real" is about persist_remote_ip option as far as I understand, not directly related to this fatal assert fix. -- Best Regards, Vladislav Grishenko > -Original Message- > From: Gert Doering > Sent: Thursday, September 17, 2020 1:46 PM > To: Vladislav Grishenko > Cc: openvpn-devel@lists.sourceforge.net > Subject: [PATCH applied] Re: Fix fatal error at switching remotes (#629) > > Your patch has been applied to the master, release/2.5 and release/2.4 branch > (bugfix). > > I have fixed a few "addinfo" occurances and re-wrapped the comment slightly. > Not checked the actual code, just ran a t_client test on > 2.4 "to be sure". > > As Arne wrote there is a "fix for real" dangling here... :-) > > commit 3ad86c2534a92af137809b6d446d570193e6d01f (master) commit > 6554025a422d3d7e5465bcbfad34fa3e196b53b0 (release/2.5) commit > 7fdcd286a15fb4f64e979c4fdbf52223d4bdede0 (release/2.4) > Author: Vladislav Grishenko > Date: Wed Sep 16 19:17:55 2020 +0500 > > Fix fatal error at switching remotes (#629) > > Signed-off-by: Vladislav Grishenko > Acked-by: Lev Stipakov > Message-Id: <20200916141755.1923-1-themi...@yandex-team.ru> > URL: https://www.mail-archive.com/openvpn- > de...@lists.sourceforge.net/msg21019.html > Signed-off-by: Gert Doering > > > -- > kind regards, > > Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Fix fatal error at switching remotes (#629)
If remote server has been resolved to multiple addresses, at
least one connection attemt has been made and connection to
the last address was skipped by management - resolved earlier
link socket addrinfo objects will not be cleared neither on
instance close nor in the next connection entry loop.
This causes fatal error assert:
>REMOTE:openvpn.net,1194,udp
remote ACCEPT
SUCCESS: remote command succeeded
>REMOTE:openvpn.net,1194,udp
remote SKIP
SUCCESS: remote command succeeded
>FATAL:Assertion failed at init.c:504
(c->c1.link_socket_addr.current_remote == NULL)
Fix this behaviour by cleaning stale addinfo objects.
v2: better comment placement and too long length fix
Signed-off-by: Vladislav Grishenko
---
src/openvpn/init.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index a785934a..8fc79638 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -500,6 +500,17 @@ next_connection_entry(struct context *c)
*/
if (!c->options.persist_remote_ip)
{
+/* Connection entry addinfo 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
+ */
+if (c->c1.link_socket_addr.remote_list)
+{
+clear_remote_addrlist(>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);
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Fix fatal error at switching remotes (#629)
If remote server has been resolved to multiple addresses, at
least one connection attemt has been made and connection to
the last address was skipped by management - resolved earlier
link socket addrinfo objects will not be cleared neither on
instance close nor in the next connection entry loop.
This causes fatal error assert:
>REMOTE:openvpn.net,1194,udp
remote ACCEPT
SUCCESS: remote command succeeded
>REMOTE:openvpn.net,1194,udp
remote SKIP
SUCCESS: remote command succeeded
>FATAL:Assertion failed at init.c:504
(c->c1.link_socket_addr.current_remote == NULL)
Fix this behaviour by cleaning stale addinfo objects.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/init.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index a785934a..508270a7 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -500,6 +500,16 @@ next_connection_entry(struct context *c)
*/
if (!c->options.persist_remote_ip)
{
+if (c->c1.link_socket_addr.remote_list)
+{
+/* Connection entry addinfo objects might have been
resolved
+ * earlier but the entry itself might have been
skipped by
+ * management on the previous loop. if so, need to
clear the
+ * addrinfo objects as close_instance does */
+clear_remote_addrlist(>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);
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4] 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, useful in case of huge 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: remote server1.example.net 1194 udp remote server2.example.net 1194 udp remote server3.example.net 1194 udp remote server4.example.net 443 tcp now it's possible to specify just one --remote-srv: remote-srv example.net and configure following DNS SRV records: name prio weight port target _openvpn._udp.example.net IN SRV 10 60 1194 server1.example.net _openvpn._udp.example.net IN SRV 10 40 1194 server2.example.net _openvpn._udp.example.net IN SRV 10 0 1194 server3.example.net _openvpn._tcp.example.net 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. Tested on Linux/Glibc & Windows 10. v2: use DNS SRV for feature naming change option name to --remote-service-discovery add --remote-service-discovery [service] optional parameter support rewrite man, add algo/prio/weight description and examples fix random weight selection was not implicit remove pulled REMOTE_DISCOVERY support, not needed split windows/unix-specific parts into extra functions rename functions into servinfo scope, add doxygen comments when appropriate remove addrinfo hack, use servinfo containers of addrinfo list instead better proxy support (tcp mode not supported so far) log discovery attempts and results, if enabled v3: complete logic rewrite use separate --remote-srv [service] [proto] option remove fallback, same is achieved with additiona --remote/--remote-srv add "auto" protocol support to allow both udp & tcp hosts be discovered add support for tcp / http proxy (natively) man update v4: due RFC 2782 ambiguity, prefer to use all resolved DNS SRV records, even ones with weight 0 after the records containing weights gr
[Openvpn-devel] [PATCH v3] Add DNS SRV remote host discovery support
DNS SRV 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, useful in case of huge ammount
of clients & servers deployed.
Patch introduces "--remote-srv domain [service] [proto]" option, which
enables DNS SRV discovery and optionally allows to change default "openvpn"
service and default "auto" protocol to the specified values.
Client will try to resolve "_service._proto.domain" name via DNS and use
returned DNS SRV records as OpenVPN remote server list ordered by server
selection mechanism defined in RFC 2782:
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.
Example: instead of multiple remotes in order:
remote s1.example.net 1194 udp
remote s2.example.net 1194 udp
remote s3.example.net 1194 udp
remote s4.example.net 1194 tcp
now it's possible to specify just one remote-srv:
remote-srv example.net
and configure following DNS SRV records:
name prio weight port host
_openvpn._udp.example.net IN SRV 10 60 1194 s1.example.net
_openvpn._udp.example.net IN SRV 10 40 1194 s2.example.net
_openvpn._udp.example.net IN SRV 10 0 1194 s3.example.net
_openvpn._tcp.example.net IN SRV 20 0 443 s4.example.net
Client will first try to resolve "_openvpn._udp.example.net" and
"_openvpn._tcp.example.net" names.
Next, "s1.example.net", "s2.example.net" and "s3.example.net" will be
ordered before "s4.example.net" as their priority 10 is smaller than 20.
Next, "s1.example.net", "s2.example.net" and "s3.example.net" will be
randomly ordered with weight probability - first will be either
"s1.example.net" with probability 60% or "s2.example.net" with 40% or
"s3.example.net" with almost zero probability.
Next, if "s1.example.net" is picked as first, second server will be either
"s2.example.net" with almost 100% probability or "s3.example.net" instead.
Next, if "s2.example.net" is picked as second, "s3.example.net" will be
skipped due zero weight.
"s4.example.net" will be the last one as the only server with priority 20.
Finally, servers will be resolved and accessed in this order.
If "_openvpn._udp.example.net" and "_openvpn._udp.example.net" can't be
resolved or no valid records were returned, client will move on to the
next connection entry.
Tested on Linux & Windows 10.
v2: use DNS SRV for feature naming
change option name to --remote-service-discovery
add --remote-service-discovery [service] optional parameter support
rewrite man, add algo/prio/weight description and examples
fix random weight selection was not implicit
remove pulled REMOTE_DISCOVERY support, not needed
split windows/unix-specific parts into extra functions
rename functions into servinfo scope, add doxygen comments when appropriate
remove addrinfo hack, use servinfo containers of addrinfo list instead
better proxy support (tcp mode not supported so far)
log discovery attempts and results, if enabled
v3: complete logic rewrite
use separate --remote-srv [service] [proto] option
remove fallback, same is achieved with additiona --remote/--remote-srv
add "auto" protocol support to allow both udp & tcp hosts be discovered
add support for tcp / http proxy (natively)
man update
Signed-off-by: Vladislav Grishenko
---
configure.ac| 2 +-
doc/man-sections/client-options.rst | 88 ++-
doc/management-notes.txt| 6 +
src/openvpn/Makefile.am | 2 +-
src/openvpn/buffer.h| 5 -
src/openvpn/errlevel.h | 1 +
src/openvpn/init.c | 57 +-
src/openvpn/openvpn.vcxproj | 8 +-
src/openvpn/options.c | 238 ++--
src/openvpn/options.h | 4 +
src/openvpn/socket.c| 836 +++-
src/openvpn/socket.h| 56 +-
src/openvpn/syshead.h | 5 +
13 files
[Openvpn-devel] [PATCH v4] Support X509 field list to be username
OpenVPN has the ability to choose different X509 field in case "CN" can't be use used to be unique connected username since commit 935c62be9c0c8a256112df818bfb8470586a23b6. Unfortunately it's not enough in case client has multiple and valid certificates from PKI for different devices (ex. laptop, mobile, etc) with the same CN/UID. Having --duplicate-cn as a workaround helps only partially: clients can be connected, but it breaks coexistance with --ifconfig-pool-persist, --client-config-dir and opens doors to DoS possibility since same client device (with the same cert) being reconnected no more replaces previously connected session, so it can exhaust server resources (ex. address pool) and can prevent other clients to be connected. With this patch, multiple X509 fields incl. "serialNumber" can be chosen to be username with --x509-username-field space-separated parameters. Multiple fields will be concatened into one username using '_' delimeter. As long as resulting username is unique, --duplicate-cn will not be required. Default value is preserved as "CN" only. Openssl backend is the only supported at the moment, since so far MbedTLS has no --x509-username-field support at all. v2: conform C99, man update, fix typos v3: reuse buffer methods, drop delimiter define, use memcpy v4: man update, change delimeter "_" to avoid path issues on windows Signed-off-by: Vladislav Grishenko --- doc/man-sections/tls-options.rst | 14 + src/openvpn/init.c | 6 ++-- src/openvpn/options.c| 49 +--- src/openvpn/options.h| 4 +-- src/openvpn/ssl_common.h | 2 +- src/openvpn/ssl_verify.c | 49 +--- src/openvpn/ssl_verify_openssl.c | 14 + 7 files changed, 93 insertions(+), 45 deletions(-) diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst index 8c2db7cd..a670578d 100644 --- a/doc/man-sections/tls-options.rst +++ b/doc/man-sections/tls-options.rst @@ -632,13 +632,14 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa options can be defined to track multiple attributes. --x509-username-field args - Field in the X.509 certificate subject to be used as the username - (default :code:`CN`). + Fields in the X.509 certificate subject to be used as the username + (default :code:`CN`). If multiple fields are specified their values + will be concatenated with :code:`_` delimeter. Valid syntax: :: - x509-username-field [ext:]fieldname + x509-username-field [ext:]fieldname [[ext:]fieldname...] Typically, this option is specified with **fieldname** as either of the following: @@ -646,6 +647,7 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa x509-username-field emailAddress x509-username-field ext:subjectAltName + x509-username-field CN serialNumber The first example uses the value of the :code:`emailAddress` attribute in the certificate's Subject field as the username. The second example @@ -653,13 +655,15 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa ``fieldname`` :code:`subjectAltName` be searched for an rfc822Name (email) field to be used as the username. In cases where there are multiple email addresses in :code:`ext:fieldname`, the last occurrence - is chosen. + is chosen. The last example uses the value of :code:`CN` attribute in + the Subject field concatened with :code:`_` delimeter and with the + certificate's :code:`serialNumber` as the username. When this option is used, the ``--verify-x509-name`` option will match against the chosen ``fieldname`` instead of the Common Name. Only the :code:`subjectAltName` and :code:`issuerAltName` X.509 - extensions are supported. + extensions and :code:`serialNumber` X.509 attribute are supported. **Please note:** This option has a feature which will convert an all-lowercase ``fieldname`` to uppercase characters, e.g., diff --git a/src/openvpn/init.c b/src/openvpn/init.c index a785934a..fb19fac3 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2905,14 +2905,14 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.crl_file_inline = options->crl_file_inline; to.ssl_flags = options->ssl_flags; to.ns_cert_type = options->ns_cert_type; -memmove(to.remote_cert_ku, options->remote_cert_ku, sizeof(to.remote_cert_ku)); +memcpy(to.remote_cert_ku, options->remote_cert_ku, sizeof(to.remote_cert_ku)); to.remote_cert_eku = options->remote_cert_eku; to.verify_hash = options->verify_hash; to.verify_hash_algo = options->verify_hash_algo; #ifdef ENABLE_X509ALTUSERNAME -to.x509_username_field = (char *) options->x509_username_field; +memcpy(to.x509_username_field, options->x509_username_field, sizeof(to.x509_username_field)); #else -to.x509_usern
Re: [Openvpn-devel] [PATCH applied] Re: Fix best gateway selection over netlink
Hi Gert, Great, many thanks -- Best Regards, Vladislav Grishenko > -Original Message- > From: Gert Doering > Sent: Thursday, September 10, 2020 2:23 PM > To: Vladislav Grishenko > Cc: openvpn-devel@lists.sourceforge.net > Subject: [PATCH applied] Re: Fix best gateway selection over netlink > > Thanks a lot. > > Your patch has been applied to the master and release/2.5 branch. > > (I have lightly tested this on Linux 4.19.9 and stared a bit at the code. Antonio is > the master of this code and if he says it's good, it is :-) ) > > commit 505d5ad8fadcdc56bae07f4b95c05acd93a47c24 (master) commit > 7a9fefb6e2ea28967a09956eeb041b687ca0e335 (release/2.5) > Author: Vladislav Grishenko > Date: Tue Sep 8 17:36:25 2020 +0500 > > Fix best gateway selection over netlink > > Signed-off-by: Vladislav Grishenko > Acked-by: Antonio Quartulli > Message-Id: <20200908123625.23179-1-themi...@yandex-team.ru> > URL: https://www.mail-archive.com/openvpn- > de...@lists.sourceforge.net/msg20900.html > Signed-off-by: Gert Doering > > > -- > kind regards, > > Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Fix best gateway selection over netlink
Hi, Antonio Thank you for review -- Best Regards, Vladislav Grishenko > -Original Message- > From: Antonio Quartulli > Sent: Thursday, September 10, 2020 2:02 PM > To: Vladislav Grishenko ; openvpn- > de...@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH v3] Fix best gateway selection over > netlink > > Hi, > > On 08/09/2020 14:36, Vladislav Grishenko wrote: > > Netlink route request with NLM_F_DUMP flag set means to return all > > entries matching criteria passed in message content - matching > > supplied family & dst address in our case. > > So, gateway from the first ipv4 route was always used. > > > > On kernels earlier than 2.6.38 default routes are the last ones, so > > arbitrary host/net route w/o gateway is likely be returned as first, > > causing gateway to be invalid or empty. > > After refactoring in 2.6.38 kernel default routes are on top, so the > > problem with older kernels was hidden. > > > > Fix this behavior by selecting first 0.0.0.0/0 if dst was not set or > > empty. For IPv6, no behavior is changed - request ::/128 route, so > > just clarify the sizes via netlink route api. > > > > Tested on 5.4.0, 4.1.51, 2.6.36 and 2.6.22 kernels. > > > > Signed-off-by: Vladislav Grishenko > > Thanks for taking care of this issue and for digging into the sitnl code. > > The change is really contained and and easy to review. > Tested a bit and it works as expected. > > Acked-by: Antonio Quartulli > > -- > Antonio Quartulli > ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Fix --remote protocol can't be set without port argument
Ok, thank you for clarification -- Best Regards, Vladislav Grishenko > -Original Message- > From: David Sommerseth > Sent: Wednesday, September 9, 2020 10:49 PM > To: Vladislav Grishenko ; openvpn- > de...@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH] Fix --remote protocol can't be set > without > port argument > > On 08/09/2020 21:01, Vladislav Grishenko wrote: > > Hi David, > > > >> -Original Message- > >> From: David Sommerseth > >> Sent: Tuesday, September 8, 2020 6:23 PM > >> To: Vladislav Grishenko ; 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/f500c49c8e0a77ce665b11f6a > dbea4029cf3b85f> > > 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 > > > remote vpn1.example.com > > > remote vpn2.example.com > proto tcp > > --- > > 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 > ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] Add DNS SRV host discovery support
> From: Arne Schwabe > Sent: Wednesday, September 9, 2020 5:15 PM > > I see your point, thank you. > > Fallback is done just to conform RFC 2782 suggestion, personally I feel it's > hardly be used with controllable client profiles when it's known that > domain.tld > does dns srv. > > Yes. And see the point in that RFC. I would rather document that for out > fallback > the user has to manually specify that because I don't see a good "works for > all" > fallback. Sure, I saw. RFC point of fallback was pretty valid for combined --remote for seamless migration dns->srv dns. With new --remote-srv this will be not necessary anymore, no migration here. Moreover, facing the coexisting with usual --remote, fallback becomes pretty useless additional complexity. > > > Additional --remote-srv indeed has benefits: > > * opens a non-conflicting syntax way to have multiple udp/tcp dns srv > requests, as you wrote in earlier mails. This will allow to sort them all by > prio/weight. > > * cleaner syntax for specifying custom dns srv service name, with > > default "openvpn" and protocol (if need to limit discovery to only udp or > > tcp) > So, in my mind syntax looks like: > > --remote-srv [service] [protocol], where both args are optional, by > > default service==openvpn, protocol==any > > Sounds good to me! Great, doing this way. > > > And, important thing, multiple --remote-srv still needs to be supported for: > > * custom order of protocol > > * different service names > > * allow to reuse --remote-random with --random-srv as well to achieve > > dns-srv-based lb > > > > Is it good enough in this form? > > I don't really see the need for that but it doesn't break the normal case of > just > one remote-srv, so fine with me. > > Arne > -- 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] Fix --remote protocol can't be set without port argument
Hi David, > -Original Message- > From: David Sommerseth > Sent: Tuesday, September 8, 2020 6:23 PM > To: Vladislav Grishenko ; 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. Man pages still has all of them since that time: remote server.exmaple.net remote server.exmaple.net 1194 remote server.exmaple.net tcp 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? > The OpenVPN 2.4 and prior releases has this line: > >--remote host [port] [proto] > > But this syntax was not supported by rst2man, so it was replaced with "args" > and the examples coming below; which carried an error. > > > -- > kind regards, > > David Sommerseth > OpenVPN Inc > ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3] Fix best gateway selection over netlink
Netlink route request with NLM_F_DUMP flag set means to return
all entries matching criteria passed in message content -
matching supplied family & dst address in our case.
So, gateway from the first ipv4 route was always used.
On kernels earlier than 2.6.38 default routes are the last ones,
so arbitrary host/net route w/o gateway is likely be returned as
first, causing gateway to be invalid or empty.
After refactoring in 2.6.38 kernel default routes are on top, so
the problem with older kernels was hidden.
Fix this behavior by selecting first 0.0.0.0/0 if dst was not set
or empty. For IPv6, no behavior is changed - request ::/128 route,
so just clarify the sizes via netlink route api.
Tested on 5.4.0, 4.1.51, 2.6.36 and 2.6.22 kernels.
Signed-off-by: Vladislav Grishenko
---
doc/man-sections/advanced-options.rst | 7 +++--
src/openvpn/networking_sitnl.c| 44 ---
2 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/doc/man-sections/advanced-options.rst
b/doc/man-sections/advanced-options.rst
index 9b96e406..bedc8841 100644
--- a/doc/man-sections/advanced-options.rst
+++ b/doc/man-sections/advanced-options.rst
@@ -11,8 +11,11 @@ Standalone Debug Options
--show-gateway
--show-gateway IPv6-target
- If an IPv6 target address is passed as argument, the IPv6 route for this
- host is reported.
+ For IPv6 this queries the route towards ::/128, or the specified IPv6
+ target address if passed as argument.
+ For IPv4 on Linux, Windows, MacOS and BSD it looks for a 0.0.0.0/0 route.
+ If there are more specific routes, the result will not always be matching
+ the route of the IPv4 packets to the VPN gateway.
Advanced Expert Options
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 713a213a..2bc70a50 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -345,6 +345,13 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned
int groups,
* continue;
* }
*/
+
+if (h->nlmsg_type == NLMSG_DONE)
+{
+ret = 0;
+goto out;
+}
+
if (h->nlmsg_type == NLMSG_ERROR)
{
err = (struct nlmsgerr *)NLMSG_DATA(h);
@@ -360,7 +367,11 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned
int groups,
ret = 0;
if (cb)
{
-ret = cb(h, arg_cb);
+int r = cb(h, arg_cb);
+if (r <= 0)
+{
+ret = r;
+}
}
}
else
@@ -375,8 +386,12 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned
int groups,
if (cb)
{
-ret = cb(h, arg_cb);
-goto out;
+int r = cb(h, arg_cb);
+if (r <= 0)
+{
+ret = r;
+goto out;
+}
}
else
{
@@ -410,6 +425,7 @@ typedef struct {
int addr_size;
inet_address_t gw;
char iface[IFNAMSIZ];
+bool default_only;
} route_res_t;
static int
@@ -421,6 +437,12 @@ sitnl_route_save(struct nlmsghdr *n, void *arg)
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
unsigned int ifindex = 0;
+/* filter-out non-zero dst prefixes */
+if (res->default_only && r->rtm_dst_len != 0)
+{
+return 1;
+}
+
while (RTA_OK(rta, len))
{
switch (rta->rta_type)
@@ -477,11 +499,25 @@ sitnl_route_best_gw(sa_family_t af_family, const
inet_address_t *dst,
{
case AF_INET:
res.addr_size = sizeof(in_addr_t);
-req.n.nlmsg_flags |= NLM_F_DUMP;
+/*
+ * kernel can't return 0.0.0.0/8 host route, dump all
+ * the routes and filter for 0.0.0.0/0 in cb()
+ */
+if (!dst || !dst->ipv4)
+{
+req.n.nlmsg_flags |= NLM_F_DUMP;
+res.default_only = true;
+}
+else
+{
+req.r.rtm_dst_len = 32;
+}
break;
case AF_INET6:
res.addr_size = sizeof(struct in6_addr);
+/* kernel can return ::/128 host route */
+req.r.rtm_dst_len = 128;
break;
default:
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] Fix best gateway selection over netlink
Sorry, comment typo:
- /* kernel cat return 0.0.0.0/128 host route */
+ /* kernel can return ::/128 host route */
--
Best Regards, Vladislav Grishenko
> -Original Message-
> From: Vladislav Grishenko
> Sent: Tuesday, September 8, 2020 7:54 AM
> To: openvpn-devel@lists.sourceforge.net
> Subject: [Openvpn-devel] [PATCH v2] Fix best gateway selection over
netlink
>
> Netlink route request with NLM_F_DUMP flag set means to return all entries
> matching criteria passed in message content - matching supplied family &
dst
> address in our case.
> So, gateway from the first ipv4 route was always used.
>
> On kernels earlier than 2.6.38 default routes are the last ones, so some
host/net
> route w/o gateway is likely be returned as first, causing gateway to be
invalid or
> empty.
> After refactoring in 2.6.38 kernel first routes are the default (or more
specific
> 0.0.0.0/n), so it's gateway is usually valid, hiding the problem.
>
> Fix this behavior by requesting exact route with corresponding full prefix
size,
> and filter dumped routes against default route /0 if dst was not set.
Empty dst is
> not valid address, so it's handled as default route request too.
>
> If there's several default routes dumped with different metrics, the first
one will
> be less-numbered, so we can stop w/o additional iteration for metric
> comparison.
>
> Tested on 5.4.0, 4.1.51 and 2.6.36 kernels.
>
> Signed-off-by: Vladislav Grishenko
> ---
> src/openvpn/networking_sitnl.c | 47 +-
> 1 file changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/src/openvpn/networking_sitnl.c
b/src/openvpn/networking_sitnl.c
> index 713a213a..81d52710 100644
> --- a/src/openvpn/networking_sitnl.c
> +++ b/src/openvpn/networking_sitnl.c
> @@ -90,7 +90,7 @@ struct sitnl_route_req {
> char buf[256];
> };
>
> -typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *msg, void *arg);
> +typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *req, struct
> +nlmsghdr *msg, void *arg);
>
> /**
> * Object returned by route request operation @@ -345,6 +345,13 @@
> sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned int groups,
> * continue;
> * }
> */
> +
> +if (h->nlmsg_type == NLMSG_DONE)
> +{
> +ret = 0;
> +goto out;
> +}
> +
> if (h->nlmsg_type == NLMSG_ERROR)
> {
> err = (struct nlmsgerr *)NLMSG_DATA(h); @@ -360,7 +367,11
@@
> sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned int groups,
> ret = 0;
> if (cb)
> {
> -ret = cb(h, arg_cb);
> +int r = cb(payload, h, arg_cb);
> +if (r <= 0)
> +{
> +ret = r;
> +}
> }
> }
> else
> @@ -375,8 +386,12 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer,
> unsigned int groups,
>
> if (cb)
> {
> -ret = cb(h, arg_cb);
> -goto out;
> +int r = cb(payload, h, arg_cb);
> +if (r <= 0)
> +{
> +ret = r;
> +goto out;
> +}
> }
> else
> {
> @@ -413,14 +428,21 @@ typedef struct {
> } route_res_t;
>
> static int
> -sitnl_route_save(struct nlmsghdr *n, void *arg)
> +sitnl_route_save(struct nlmsghdr *req, struct nlmsghdr *n, void *arg)
> {
> route_res_t *res = arg;
> +struct rtmsg *q = NLMSG_DATA(req);
> struct rtmsg *r = NLMSG_DATA(n);
> struct rtattr *rta = RTM_RTA(r);
> int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
> unsigned int ifindex = 0;
>
> +/* if dumped, filter out non-default routes */
> +if (q->rtm_dst_len == 0 && r->rtm_dst_len)
> +{
> +return 1;
> +}
> +
> while (RTA_OK(rta, len))
> {
> switch (rta->rta_type)
> @@ -477,11 +499,24 @@ sitnl_route_best_gw(sa_family_t af_family, const
> inet_address_t *dst,
> {
> case AF_INET:
> res.addr_size = sizeof(in_addr_t);
> -req.n.nlmsg_flags |= NLM_F_DUMP;
> +/*
> + * kernel can't return 0.0.0.0/32 host route, therefore
> + * need to dump all the routes and filter them in cb()
> + */
>
[Openvpn-devel] [PATCH v2] Fix best gateway selection over netlink
Netlink route request with NLM_F_DUMP flag set means to return
all entries matching criteria passed in message content -
matching supplied family & dst address in our case.
So, gateway from the first ipv4 route was always used.
On kernels earlier than 2.6.38 default routes are the last ones,
so some host/net route w/o gateway is likely be returned as first,
causing gateway to be invalid or empty.
After refactoring in 2.6.38 kernel first routes are the default
(or more specific 0.0.0.0/n), so it's gateway is usually valid,
hiding the problem.
Fix this behavior by requesting exact route with corresponding
full prefix size, and filter dumped routes against default
route /0 if dst was not set. Empty dst is not valid address,
so it's handled as default route request too.
If there's several default routes dumped with different
metrics, the first one will be less-numbered, so we can stop
w/o additional iteration for metric comparison.
Tested on 5.4.0, 4.1.51 and 2.6.36 kernels.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 47 +-
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 713a213a..81d52710 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -90,7 +90,7 @@ struct sitnl_route_req {
char buf[256];
};
-typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *msg, void *arg);
+typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *req, struct nlmsghdr
*msg, void *arg);
/**
* Object returned by route request operation
@@ -345,6 +345,13 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned
int groups,
* continue;
* }
*/
+
+if (h->nlmsg_type == NLMSG_DONE)
+{
+ret = 0;
+goto out;
+}
+
if (h->nlmsg_type == NLMSG_ERROR)
{
err = (struct nlmsgerr *)NLMSG_DATA(h);
@@ -360,7 +367,11 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned
int groups,
ret = 0;
if (cb)
{
-ret = cb(h, arg_cb);
+int r = cb(payload, h, arg_cb);
+if (r <= 0)
+{
+ret = r;
+}
}
}
else
@@ -375,8 +386,12 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned
int groups,
if (cb)
{
-ret = cb(h, arg_cb);
-goto out;
+int r = cb(payload, h, arg_cb);
+if (r <= 0)
+{
+ret = r;
+goto out;
+}
}
else
{
@@ -413,14 +428,21 @@ typedef struct {
} route_res_t;
static int
-sitnl_route_save(struct nlmsghdr *n, void *arg)
+sitnl_route_save(struct nlmsghdr *req, struct nlmsghdr *n, void *arg)
{
route_res_t *res = arg;
+struct rtmsg *q = NLMSG_DATA(req);
struct rtmsg *r = NLMSG_DATA(n);
struct rtattr *rta = RTM_RTA(r);
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
unsigned int ifindex = 0;
+/* if dumped, filter out non-default routes */
+if (q->rtm_dst_len == 0 && r->rtm_dst_len)
+{
+return 1;
+}
+
while (RTA_OK(rta, len))
{
switch (rta->rta_type)
@@ -477,11 +499,24 @@ sitnl_route_best_gw(sa_family_t af_family, const
inet_address_t *dst,
{
case AF_INET:
res.addr_size = sizeof(in_addr_t);
-req.n.nlmsg_flags |= NLM_F_DUMP;
+/*
+ * kernel can't return 0.0.0.0/32 host route, therefore
+ * need to dump all the routes and filter them in cb()
+ */
+if (!dst || dst->ipv4 == htonl(INADDR_ANY))
+{
+req.n.nlmsg_flags |= NLM_F_DUMP;
+}
+else
+{
+req.r.rtm_dst_len = 32;
+}
break;
case AF_INET6:
res.addr_size = sizeof(struct in6_addr);
+/* kernel cat return 0.0.0.0/128 host route */
+req.r.rtm_dst_len = 128;
break;
default:
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Fix best gateway selection over netlink
Netlink route request with NLM_F_DUMP flag set means to
return all entries matching criteria passed in message
content - matching supplied dst address in our case.
So, gateway from the first returned route was always used
even there were more specific routes present.
By a chance, after route refactoring in ~2.6.38 first route
is the default route, so default gateway was always used,
hiding the problem.
On earlier kernels default route is the last one, so
route w/o gateway is likely be returned as first causes
gateway always to be 0.0.0.0.
Fix this behavior by requesting exact route, not dump along
with specifying correct dst perfix size.
Tested on 5.4.0, 4.1.51 and 2.6.36 kernels.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/networking_sitnl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 713a213a..150dfa5c 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -477,11 +477,12 @@ sitnl_route_best_gw(sa_family_t af_family, const
inet_address_t *dst,
{
case AF_INET:
res.addr_size = sizeof(in_addr_t);
-req.n.nlmsg_flags |= NLM_F_DUMP;
+req.r.rtm_dst_len = 32;
break;
case AF_INET6:
res.addr_size = sizeof(struct in6_addr);
+req.r.rtm_dst_len = 128;
break;
default:
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Fix --remote protocol can't be set without port argument
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.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/options.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 8bf82c57..02ac08d8 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5682,16 +5682,26 @@ add_option(struct options *options,
re.remote = p[1];
if (p[2])
{
-re.remote_port = p[2];
-if (p[3])
+/* Since port is optional, second parameter can be a protocol */
+int proto = ascii2proto(p[2]);
+sa_family_t af = ascii2af(p[2]);
+if (proto < 0)
{
-const int proto = ascii2proto(p[3]);
-const sa_family_t af = ascii2af(p[3]);
-if (proto < 0)
+/* Second is not proto, port then. Protocol should be third */
+re.remote_port = p[2];
+if (p[3])
{
-msg(msglevel, "remote: bad protocol associated with host
%s: '%s'", p[1], p[3]);
-goto err;
+proto = ascii2proto(p[3]);
+af = ascii2af(p[3]);
+if (proto < 0)
+{
+msg(msglevel, "remote: bad protocol associated with
host %s: '%s'", p[1], p[3]);
+goto err;
+}
}
+}
+if (proto >= 0)
+{
re.proto = proto;
re.af = af;
}
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Add DNS SRV host discovery support
DNS SRV 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, useful in case of huge ammounts of client & servers deployed. Patch introduces "--remote-service-discovery [service]" option, which enables discovery and optionally allows to change "openvpn" default service to user specified. Client will try to resolve ``_service._proto.host`` name via DNS and use returned DNS SRV records as OpenVPN remote server list ordered by server selection mechanism defined in RFC 2782: 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. Example: instead of multiple remotes on order in profile: remote s1.example.com 1194 udp remote s2.example.com 1194 udp remote s3.example.com 1194 udp remote s4.example.com 1194 udp remote example.com 1194 udp now it's possible to specify one remote with discovery: remote example.net 1194 udp remote-service-discovery and addconfigure following DNS SRV records, for examle: _openvpn._udp.example.net IN SRV 10 60 1194 s1.example.net _openvpn._udp.example.net IN SRV 10 40 1194 s2.example.net _openvpn._udp.example.net IN SRV 10 0 1194 s3.example.net _openvpn._udp.example.net IN SRV 20 0 1194 s4.example.net Client will try to resolve "_openvpn._udp.example.net". Next, "s1.example.net", "s2.example.net" and "s3.example.net" will be ordered before "s4.example.net" as their priority 10 is smaller than 20. Next, "s1.example.net", "s2.example.net" and "s3.example.net" will be randomly ordered with weight probability - first will be either "s1.example.net" with probability 60% or "s2.example.net" with 40% or "s3.example.net" with almost zero probability. Next, if "s1.example.net" is picked as first, second server will be either "s2.example.net" with almost 100% probability or "s3.example.net" instead. Next, if "s2.example.net" is picked as second, "s3.example.net" will be skipped due zero weight. "s4.example.net" will be the last one as the only server with priority 20. Finally, servers will be resolved and accessed in this order. If _openvpn._udp.example.net" can't be resolved or no valid records are returned or they can't be resolved, client will try to access "example.net" host at port 1194 as if no discovery is enabled. v2: use DNS SRV for feature naming change option name to --remote-service-discovery add --remote-service-discovery [service] optional parameter support rewrite man, add algo/prio/weight description and examples fix random weight selection was not implicit remove pulled REMOTE_DISCOVERY support, not needed split windows/unix-specific parts into extra functions rename functions into servinfo scope, add doxygen comments when appropriate remove addrinfo hack, use servinfo containers of addrinfo list instead better proxy support (tcp mode not supported so far) log discovery attempts and results, if enabled Signed-off-by: Vladislav Grishenko --- configure.ac| 2 +- doc/man-sections/client-options.rst | 63 +++ src/openvpn/Makefile.am | 2 +- src/openvpn/buffer.h| 5 - src/openvpn/errlevel.h | 1 + src/openvpn/init.c | 19 +- src/openvpn/openvpn.vcxproj | 8 +- src/openvpn/options.c | 8 + src/openvpn/options.h | 1 + src/openvpn/socket.c| 773 +--- src/openvpn/socket.h| 93 +++- src/openvpn/syshead.h | 5 + src/openvpn/tun.c | 19 +- src/openvpn/tun.h | 3 +- 14 files changed, 904 insertions(+), 98 deletions(-) diff --git a/configure.ac b/configure.ac index f8279924..67251bed 100644 --- a/configure.ac +++ b/configure.ac @@ -477,7 +477,7 @@ SOCKET_INCLUDES=" " AC_CHECK_HEADERS( - [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h sys/kern_control.h], + [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h n
Re: [Openvpn-devel] [PATCH] Add DNS SRV host discovery support
Hi, Arne Many thanks the review, please refer comments inline -- Best Regards, Vladislav Grishenko > -Original Message- > From: Arne Schwabe > Sent: Tuesday, August 25, 2020 2:10 PM > Am 25.08.20 um 00:15 schrieb Vladislav Grishenko: > > DNS SRV (rfc2782) support allows to use several OpenVPN servers for a > > single domain w/o explicit profile enumerating, to move services from > > host to host with little fuss, and to designate some hosts as primary > > servers for a service and others as backups. > > OpenVPN client ask for a specific service/protocol for a specific > > domain and get back the names & ports of available servers to connect > > to, ordered by priority and relative weight. Note, implemented server > > selection is intended to express static information such as "this > > server has a faster CPU than that one" or "this server has a much > > better network connection than that one" at the moment client asks for a > records. > > Feature has been asked several times already, can be useful in case of > > for huge ammounts of client & servers deployed. > > Hu? You are talking here about loading balancing by a weight, right? Well, yes at some point, from RFC 2052: Weight Load balancing mechanism. When selecting a target host among the those that have the same priority, the chance of trying this one first SHOULD be proportional to its weight. The range of this number is 1-65535. Domain administrators are urged to use Weight 0 when there isn't any load balancing to do, to make the RR easier to read for humans (less noisy). RFC 2782 has renamed "load balancing" into "server selection": Weight A server selection mechanism. 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. The range of this number is 0-65535. This is a 16 bit unsigned integer in network byte order. Domain administrators SHOULD use Weight 0 when there isn't any server selection to do, to make the RR easier to read for humans (less noisy). In the presence of records containing weights greater than 0, records with weight 0 should have a very small chance of being selected. > > > For example, instead of multiple remotes in profile: > > remote server.example.com 1194 udp > > remote server1.example.com 1195 udp > > remote server2.example.org 1196 udp > > remote server.example.com 443 tcp > > remote server1.example.tld 8443 tcp > > > > Now it's possible to specify just static domain(s) and enable discovery: > > remote server.example.com 1194 udp > > remote server.example.com 443 tcp > > remote-discovery > > > > When discovery is enabled, > > I wonder if we should enable this by default. But this still fixes the order > of UDP > and TCP in the config. Ideally I want just to say: > > remote vpn.mycorp.com and then it should resolve to all UDP+TCP options. > And it should be possible to point the client to try > udp/1194/primary.vpn.corp.com and then 443/tcp/primary.vpn.corp.com and > only after that try 1194/backup.vpn.corp.com. With this syntax it looks like > that > is not possible. I'm afraid here we have chicken vs egg issue here. Service resolution is protocol-based, same as getaddrinfo() uses SOCK_DGRAM/SOCK_STREAM to give correct servname (same service name can have different ports on different proto, afair netbios, that's why originally), so logically we need to know our protocol before asking for service on it. Reflecting it, currently OpenVPN has no "unknown protocol" state for one connection entry and defaults to UDP with no options set to have it in consistent state for subsequent socket/framing initialization and remote resolving. Same as "remote domain.tld" will be udp-only, service discovery will happen over _udp.domain.tld. Making OpenVPN service discovery protocol agnostic will mean to allow dynamic protocol change within current connection entry/remote, I'm not sure if openvpn is ready for this. If this can be implemented as looping over udp/tcp if not explicitly set - like looping over remote_list addresses, this can give a base for "full" discovery. Let's imagine it's done and working - cross-protocol server selection rules are not defined by RFC, ambiguity arises: should tcp and udp records share prio numbers? If yes, should they share relative weight algo or weighted selection should stay within proto scope? Sure, it can be documented in OpenVPN docs, but one who setup that can have general experience with other DNS SRV
[Openvpn-devel] [PATCH] Add DNS SRV host discovery support
DNS SRV (rfc2782) support allows to use several OpenVPN servers for a single
domain w/o explicit profile enumerating, to move services from host to host
with little fuss, and to designate some hosts as primary servers for a service
and others as backups.
OpenVPN client ask for a specific service/protocol for a specific domain and
get back the names & ports of available servers to connect to, ordered by
priority and relative weight. Note, implemented server selection is intended
to express static information such as "this server has a faster CPU than that
one" or "this server has a much better network connection than that one" at
the moment client asks for a records.
Feature has been asked several times already, can be useful in case of for huge
ammounts of client & servers deployed.
For example, instead of multiple remotes in profile:
remote server.example.com 1194 udp
remote server1.example.com 1195 udp
remote server2.example.org 1196 udp
remote server.example.com 443 tcp
remote server1.example.tld 8443 tcp
Now it's possible to specify just static domain(s) and enable discovery:
remote server.example.com 1194 udp
remote server.example.com 443 tcp
remote-discovery
When discovery is enabled, OpenVPN will first try to resolve
_openvpn._udp.server.example.com SRV records and use resolved targets (see
example below) as servers for the subsequent connection attempts. If there's
no records or resolve error, OpenVPN will fallback to direct connection
using specified server.example.com host and 1194 port. Next connection
attempt will be done with next configures connection/remote entry - using
_openvpn._tcp.server.example.com records (if any) and server.example.com:443
as last resort.
DNS zone can look like below (arbitrary prio & weight values):
_openvpn._udp.server.example.com IN SRV 10 70 1194 server.example.com
_openvpn._udp.server.example.com IN SRV 10 30 1195 server1.example.com
_openvpn._udp.server.example.com IN SRV 20 0 1196 server2.example.org
_openvpn._tcp.server.example.com IN SRV 10 10 443 server.example.com
_openvpn._tcp.server.example.com IN SRV 10 200 443 server.example.com
Currently only "openvpn" service is supported, usage of per-connection
service names is up to syntax decision (either intead of fallback port, or
as remote-discovery argument, etc), almost all the required mechanics
is implemented for that.
References:
https://tools.ietf.org/html/rfc2782
https://en.wikipedia.org/wiki/SRV_record
https://sourceforge.net/p/openvpn/mailman/message/34364911/
https://forums.openvpn.net/viewtopic.php?f=10=13660
Signed-off-by: Vladislav Grishenko
---
configure.ac| 2 +-
doc/man-sections/client-options.rst | 26 ++
src/openvpn/Makefile.am | 2 +-
src/openvpn/buffer.h| 5 -
src/openvpn/init.c | 9 +-
src/openvpn/manage.c| 2 +-
src/openvpn/openvpn.vcxproj | 8 +-
src/openvpn/options.c | 11 +
src/openvpn/options.h | 1 +
src/openvpn/ps.c| 2 +-
src/openvpn/socket.c| 530 ++--
src/openvpn/socket.h| 15 +
src/openvpn/syshead.h | 5 +
13 files changed, 580 insertions(+), 38 deletions(-)
diff --git a/configure.ac b/configure.ac
index f8279924..67251bed 100644
--- a/configure.ac
+++ b/configure.ac
@@ -477,7 +477,7 @@ SOCKET_INCLUDES="
"
AC_CHECK_HEADERS(
- [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
+ [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h net/if_utun.h
sys/kern_control.h],
,
,
[[${SOCKET_INCLUDES}]]
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index ec1e3b11..53e6580e 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -299,6 +299,32 @@ configuration.
specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
addresses, in the order getaddrinfo() returns them.
+--remote-discovery
+ Perform rfc2782 remote host discovery using specified ``host`` and
+ ``proto`` values.
+ OpenVPN will try to resolve `` _openvpn._proto.host`` name via DNS
+ and use returned DNS SRV target and port records as OpenVPN servers
+ addresses in the order specified by Priority and Weight of that
+ records. If one record resolves to multiple IP addresses, OpenVPN
+ will try them in the order that the system getaddrinfo() presents
+ them. If discovery is failed, usual ``host`` and ``port`` connection
+ will be tried as a fallback.
+
+ Example:
+ ::
+
+ remote example.net 1194 udp
+ remote example.net 443 tcp
+ remote-discovery
+
+ DNS zone:
+ ::
+
+ _openvpn._udp.example.net IN SRV 10 60 1194 server1.example.net
+ _openvpn._udp.example.net IN SRV 10 40 1194 server2.example.n
[Openvpn-devel] [PATCH v3] Support x509 field list to be username
OpenVPN has the ability to choose different x509 field in case "CN" can't be use used to be unique connected username since commit 935c62be9c0c8a256112df818bfb8470586a23b6. Unfortunately it's not enough in case client has multiple and valid certificates from PKI for different devices (ex. laptop, mobile, etc) with the same CN/UID. Having --duplicate-cn as a workaround helps only partially: clients can be connected, but it breaks coexistance with --ifconfig-pool-persist, --client-config-dir and opens doors to DoS possibility since same client device (with the same cert) being reconnected no more replaces previously connected session, so it can exhaust server resources (ex. address pool) and can prevent other clients to be connected. With this patch, multiple x509 fields incl. "serialNumber" can be chosen to be username as --x509-username-field space-separated parameters. Multiple fields will be joined into one username using '/' delimeter for consistency with CN/addr logging and preserving ability for hierarchical ccd. As long as resulting username is unique, --duplicate-cn will not be required. Default value is preserved as "CN" only. Openssl backend is the only supported at the moment, since so far MbedTLS has no alt user name support at all. v2: conform C99, man update, fix typos v3: reuse buffer methods, drop delimiter define, use memcpy Signed-off-by: Vladislav Grishenko --- doc/man-sections/tls-options.rst | 9 -- src/openvpn/init.c | 6 ++-- src/openvpn/options.c| 49 +--- src/openvpn/options.h| 4 +-- src/openvpn/ssl_common.h | 8 +- src/openvpn/ssl_verify.c | 49 +--- src/openvpn/ssl_verify_openssl.c | 14 + 7 files changed, 96 insertions(+), 43 deletions(-) diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst index 8c2db7cd..8449f5dc 100644 --- a/doc/man-sections/tls-options.rst +++ b/doc/man-sections/tls-options.rst @@ -632,13 +632,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa options can be defined to track multiple attributes. --x509-username-field args - Field in the X.509 certificate subject to be used as the username + Field list in the X.509 certificate subject to be used as the username (default :code:`CN`). Valid syntax: :: - x509-username-field [ext:]fieldname + x509-username-field [ext:]fieldname [[ext:]fieldname...] Typically, this option is specified with **fieldname** as either of the following: @@ -646,6 +646,7 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa x509-username-field emailAddress x509-username-field ext:subjectAltName + x509-username-field CN serialNumber The first example uses the value of the :code:`emailAddress` attribute in the certificate's Subject field as the username. The second example @@ -653,7 +654,9 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa ``fieldname`` :code:`subjectAltName` be searched for an rfc822Name (email) field to be used as the username. In cases where there are multiple email addresses in :code:`ext:fieldname`, the last occurrence - is chosen. + is chosen. The last example uses the value of :code:`CN` attribute in + the Subject field and the certificate's :code:`serialNumber` field in + hex delimited by slash symbol as the resulting username. When this option is used, the ``--verify-x509-name`` option will match against the chosen ``fieldname`` instead of the Common Name. diff --git a/src/openvpn/init.c b/src/openvpn/init.c index dfa045b0..b748d985 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2905,14 +2905,14 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.crl_file_inline = options->crl_file_inline; to.ssl_flags = options->ssl_flags; to.ns_cert_type = options->ns_cert_type; -memmove(to.remote_cert_ku, options->remote_cert_ku, sizeof(to.remote_cert_ku)); +memcpy(to.remote_cert_ku, options->remote_cert_ku, sizeof(to.remote_cert_ku)); to.remote_cert_eku = options->remote_cert_eku; to.verify_hash = options->verify_hash; to.verify_hash_algo = options->verify_hash_algo; #ifdef ENABLE_X509ALTUSERNAME -to.x509_username_field = (char *) options->x509_username_field; +memcpy(to.x509_username_field, options->x509_username_field, sizeof(to.x509_username_field)); #else -to.x509_username_field = X509_USERNAME_FIELD_DEFAULT; +to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT; #endif to.es = c->c2.es; to.net_ctx = >net_ctx; diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 8bf82c57..f7e76d3e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -876,7 +876,7 @@ init_options(struct options *o, const bool init_gc) o->tls_cert_profile = N
Re: [Openvpn-devel] [PATCH 1/2] Support multiple x509 field list to be username
Hi,
>> #ifdef ENABLE_X509ALTUSERNAME
>> -to.x509_username_field = (char *) options->x509_username_field;
>> +memmove(to.x509_username_field, options->x509_username_field,
>> sizeof(to.x509_username_field));
>> #else
>> -to.x509_username_field = X509_USERNAME_FIELD_DEFAULT;
>> +to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
>> #endif
>
> Can't we use the same code in both cases? And why memmove instead memcpy?
Can't, options struct contains x509_username_field only with
ENABLE_X509ALTUSERNAME.
Memmove is used to be consistent with remote_cert_ku copying several lines
above, although I think memcpy is enough for here and above - tls and options
mem regions can't be crossed.
>> /* Default field in X509 to be username */
>> #define X509_USERNAME_FIELD_DEFAULT "CN"
>> +#define X509_USERNAME_FIELD_DELIMITER '/'
>
> This seems to be used nowhere.
It's used down below as a field delimiter for possible easy change:
> +{
> +*buf = X509_USERNAME_FIELD_DELIMITER;
> }
I'll drop it with just "/%s" in v3.
>> +#define TLS_USERNAME_LEN 128
>
> NAK on this one. The X509 CN name length is max 64 characeters. So you
> either have stick to that limit in your patch or have to untagle the CN
> name length vs the field you are using, which might end up in a seperate
> patch.
> ...
> Not really wrong but we normally do stuff like appending etc to a string
> with our buf method/ classes. I would prefer we do it the same here
> instead of doing it completely different (espeically the !! is nowhere
> else found in the code base).
Good point, 64 limit can be kept for each backend_x509_get_username() call,
subsequent appending will be done via buffer methods - this way buffer size
will be untied from TLS_USERNAME_LEN.
> C89 style instead C99. The !!i feels weird. It is the same as max(i, 1)
> but less readable.
Yes, sure.
--
Best Regards, Vladislav Grishenko
-Original Message-
From: Arne Schwabe
Sent: Friday, August 14, 2020 6:56 PM
To: Vladislav Grishenko ;
openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH 1/2] Support multiple x509 field list to be
username
Am 28.07.20 um 00:13 schrieb Vladislav Grishenko:
> OpenVPN has the ability to choose different x509 field in case "CN"
> can't be use used to be unique connected username since commit
> 935c62be9c0c8a256112df818bfb8470586a23b6.
> Unfortunately it's not enough in case client has multiple and valid
> certificates from PKI for different devices (ex. laptop, mobile, etc)
> with the same CN/UID.
>
> Having --duplicate-cn as a workaround helps only partially - clients
> can be connected, but it breaks coexistance with --ifconfig-pool-persist,
> --client-config-dir and opens doors to DoS possibility since same client
> device (with the same cert) being reconnected doesn't replace previously
> connected session no more, so can exhaust server ressources (ex. address
> pool) and can prevent other clients to be connected.
>
> With this patch, multiple x509 fields incl. "serialNumber" can be chosen
> to be username as --x509-username-files space-separated parameters.
> Multiple fields will be joined into one username using '/' delimeter for
> consistency with CN/addr logging and preserving ability for hierarchical
> ccd. As long as resulting username is unique, --duplicate-cn will not
> be required. Default value is preserved as "CN" only.
>
> Openssl backend is the only supported at the moment, since so far MbedTLS
> has no alt user name support at all.
> ---
> doc/man-sections/tls-options.rst | 9 ---
> src/openvpn/init.c | 4 +--
> src/openvpn/options.c| 46 ++--
> src/openvpn/options.h| 4 +--
> src/openvpn/ssl.h| 1 +
> src/openvpn/ssl_common.h | 8 +-
> src/openvpn/ssl_verify.c | 35
> src/openvpn/ssl_verify_openssl.c | 15 +++
> 8 files changed, 83 insertions(+), 39 deletions(-)
>
> diff --git a/doc/man-sections/tls-options.rst
> b/doc/man-sections/tls-options.rst
> index 8c2db7cd..301f8be4 100644
> --- a/doc/man-sections/tls-options.rst
> +++ b/doc/man-sections/tls-options.rst
> @@ -632,13 +632,13 @@ certificates and keys:
> https://github.com/OpenVPN/easy-rsa
>options can be defined to track multiple attributes.
>
> --x509-username-field args
> - Field in the X.509 certificate subject to be used as the username
> + Field list in the X.509 certificate subject to be used as the username
>(default :code:`CN`).
>
>Valid syntax:
>::
>
> - x509-usern
Re: [Openvpn-devel] [PATCH v2] Allow management to kill client instances by CN wildcard
Hi, Yes, killing a client with cn ending in * will also lead to killing all the clients whose cn starts with that prefix. Use other char would no-intuitive (ex. +). What about optional "prefix" mode word for explicit mode (can be also enhanced one day with suffix/regexp/etc). kill cn [mode]: Kill the client instance(s) having common name cn. -- Best Regards, Vladislav Grishenko -Original Message- From: Selva Nair Sent: Friday, August 14, 2020 11:22 PM To: openvpn-devel Subject: Re: [Openvpn-devel] [PATCH v2] Allow management to kill client instances by CN wildcard Hi On Fri, Aug 14, 2020 at 1:36 PM Arne Schwabe wrote: > > Am 14.08.20 um 19:12 schrieb Vladislav Grishenko: > > In case of some permanent part of common name (ex. domain) and/or > > long complex common name consisting of multiple x509 fields, it's > > handly to kill client instances via management interface with just > > prefix of common name, not by exact match only. > > > > Patch allows to use asterisk as wildcard placeholder in the last > > trailing symbol of kill command parameter. > > Single asterisk - empty prefix would be too greedy and can be too > > harmful, therefore not allowed. Wildcards in the middle of parameter > > string are not supported to keep the the things simple at the moment. > > > > v2: fine tune comments > > > > Thanks for v2, > > Acked-By; Arne Schwabe '*' is an allowed character in x509 common name unless we explicitly forbid it. So killing a client with name ending in * would get tricky if not impossible without side effects. Selva ___ 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
[Openvpn-devel] [PATCH v2] Allow management to kill client instances by CN wildcard
In case of some permanent part of common name (ex. domain) and/or
long complex common name consisting of multiple x509 fields, it's
handly to kill client instances via management interface with just
prefix of common name, not by exact match only.
Patch allows to use asterisk as wildcard placeholder in the last
trailing symbol of kill command parameter.
Single asterisk - empty prefix would be too greedy and can be too
harmful, therefore not allowed. Wildcards in the middle of
parameter string are not supported to keep the the things simple at the moment.
v2: fine tune comments
Signed-off-by: Vladislav Grishenko
---
doc/management-notes.txt | 2 ++
src/openvpn/multi.c | 15 ++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 61daaf07..91073693 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -195,6 +195,8 @@ Command examples:
kill Test-Client -- kill the client instance having a
common name of "Test-Client".
+ kill Test-Cli* -- kill the client instances having a
+ common name starting with "Test-Cli".
kill 1.2.3.4:4000 -- kill the client instance having a
source address and port of 1.2.3.4:4000
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 13738180..36be5de2 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3820,6 +3820,19 @@ management_callback_kill_by_cn(void *arg, const char
*del_cn)
struct hash_element *he;
int count = 0;
+/* Check passed string for non-empty prefix with trailing asterisk */
+size_t len = strlen(del_cn);
+if (len > 1 && del_cn[len - 1] == '*')
+{
+/* Exclude trailing asterisk from string comparison */
+len--;
+}
+else
+{
+/* Include terminating NUL char to perform exact string comparison */
+len++;
+}
+
hash_iterator_init(m->iter, );
while ((he = hash_iterator_next()))
{
@@ -3827,7 +3840,7 @@ management_callback_kill_by_cn(void *arg, const char
*del_cn)
if (!mi->halt)
{
const char *cn = tls_common_name(mi->context.c2.tls_multi, false);
-if (cn && !strcmp(cn, del_cn))
+if (cn && !strncmp(cn, del_cn, len))
{
multi_signal_instance(m, mi, SIGTERM);
++count;
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 02/17] Cleanup tls_pre_decrypt_lite and tls_pre_encrypt
Tested-By: Vladislav Grishenko Read-checked with --ignore-space-change, build & tested with sample server/client profile. -- 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 01/17] Refactor/Reformat tls_pre_decrypt
Tested-By: Vladislav Grishenko Read-checked with --ignore-space-change, build & tested with sample server/client profile. -- 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 applied] Re: Log serial number of revoked certificate
Hi, Gert Thank you. I'd appreciate if patch could be applied to release/2.4 too, no changes are required - related code is the same, just hunks offset in ssl_verify.c and ssl_verify_openssl.c I've tested 2.4.9 [git:release/2.4/7c428ca19a8df6b9+] with patch on sample certificates, please refer log is below: OpenSSL, --crl-verify sample-keys/ca.crl Wed Aug 5 17:18:49 2020 127.0.0.1:16001 VERIFY ERROR: depth=0, error=certificate revoked: C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3 Wed Aug 5 17:18:49 2020 127.0.0.1:16001 OpenSSL: error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed Wed Aug 5 17:18:49 2020 127.0.0.1:16001 TLS_ERROR: BIO read tls_read_plaintext error mbedTLS, --crl-verify sample-keys/ca.crl Wed Aug 5 17:25:28 2020 127.0.0.1:16001 VERIFY OK: depth=1, C=KG, ST=NA, L=BISHKEK, O=OpenVPN-TEST, emailAddress=me@myhost.mydomain Wed Aug 5 17:25:28 2020 127.0.0.1:16001 VERIFY ERROR: depth=0, subject=C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3: The certificate has been revoked (is on a CRL) Wed Aug 5 17:25:28 2020 127.0.0.1:16001 TLS_ERROR: read tls_read_plaintext error: X509 - Certificate verification failed, e.g. CRL, CA or signature check failed touch sample-keys/3, --crl-verify sample-keys/ dir Wed Aug 5 17:18:12 2020 127.0.0.1:16001 VERIFY OK: depth=1, C=KG, ST=NA, L=BISHKEK, O=OpenVPN-TEST, emailAddress=me@myhost.mydomain Wed Aug 5 17:18:12 2020 127.0.0.1:16001 VERIFY CRL: depth=0, C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3 is revoked Wed Aug 5 17:18:12 2020 127.0.0.1:16001 OpenSSL: error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed Wed Aug 5 17:18:12 2020 127.0.0.1:16001 TLS_ERROR: BIO read tls_read_plaintext error -- Best Regards, Vladislav Grishenko -Original Message- From: Gert Doering Sent: Wednesday, August 5, 2020 4:55 PM To: Vladislav Grishenko Cc: openvpn-devel@lists.sourceforge.net Subject: [PATCH applied] Re: Log serial number of revoked certificate Your patch has been applied to the master branch. I have not done much testing, just a test run "make check" on an OpenSSL and mbedTLS build. I have not looked into applying it to "release/2.4" - if you think it's needed, let me know (or if it needs more work because the code has diverged too much, send a 2.4 patch) - thanks. commit 992e9cec40539a155afa9eae10502aa62f617965 Author: Vladislav Grishenko Date: Wed Aug 5 15:23:33 2020 +0500 Log serial number of revoked certificate Signed-off-by: Vladislav Grishenko Acked-by: Lev Stipakov Message-Id: <20200805102333.3109-1-themi...@yandex-team.ru> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20642.ht ml Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Log serial number of revoked certificate
As it appears commit 767e4c56becbfeea525e4695a810593f373883cd "Log
serial number of revoked certificate" hasn't survive refactoring
of CRL handling.
In most of situations admin of OpenVPN server needs to know which
particular certificate is used by client.
In the case when certificate is valid, environment variable can be
used for that but once it is revoked, no user scripts are invoked
so there is no way to get serial number, only subject is logged.
Let's log certificate serial in case it is revoked and additionally
log certificate depth & subject in crl-verify "dir" mode for better
consistency with crl file (non-dir) mode.
v2: log if serial is not availble, require it in crl-verify dir mode
Signed-off-by: Vladislav Grishenko
---
src/openvpn/ssl_verify.c | 14 +++---
src/openvpn/ssl_verify_mbedtls.c | 5 +++--
src/openvpn/ssl_verify_openssl.c | 5 +++--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 844bc57d..97ccb93b 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -599,7 +599,8 @@ cleanup:
* check peer cert against CRL directory
*/
static result_t
-verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert)
+verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert,
+ const char *subject, int cert_depth)
{
result_t ret = FAILURE;
char fn[256];
@@ -607,6 +608,12 @@ verify_check_crl_dir(const char *crl_dir,
openvpn_x509_cert_t *cert)
struct gc_arena gc = gc_new();
char *serial = backend_x509_get_serial(cert, );
+if (!serial)
+{
+msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial number is not
available",
+cert_depth, subject);
+goto cleanup;
+}
if (!openvpn_snprintf(fn, sizeof(fn), "%s%c%s", crl_dir,
OS_SPECIFIC_DIRSEP, serial))
{
@@ -616,7 +623,8 @@ verify_check_crl_dir(const char *crl_dir,
openvpn_x509_cert_t *cert)
fd = platform_open(fn, O_RDONLY, 0);
if (fd >= 0)
{
-msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is
revoked", serial);
+msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked",
+cert_depth, subject, serial);
goto cleanup;
}
@@ -758,7 +766,7 @@ verify_cert(struct tls_session *session,
openvpn_x509_cert_t *cert, int cert_dep
{
if (opt->ssl_flags & SSLF_CRL_VERIFY_DIR)
{
-if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert))
+if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert, subject,
cert_depth))
{
goto cleanup;
}
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index fd31bbbd..93891038 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -68,6 +68,7 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert,
int cert_depth,
int ret = 0;
char errstr[512] = { 0 };
char *subject = x509_get_subject(cert, );
+char *serial = backend_x509_get_serial(cert, );
ret = mbedtls_x509_crt_verify_info(errstr, sizeof(errstr)-1, "",
*flags);
if (ret <= 0 && !openvpn_snprintf(errstr, sizeof(errstr),
@@ -82,8 +83,8 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert,
int cert_depth,
if (subject)
{
-msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s: %s",
-cert_depth, subject, errstr);
+msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, serial=%s:
%s",
+cert_depth, subject, serial ? serial : "",
errstr);
}
else
{
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index ff14db23..454efeec 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -71,6 +71,7 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
{
/* get the X509 name */
char *subject = x509_get_subject(current_cert, );
+char *serial = backend_x509_get_serial(current_cert, );
if (!subject)
{
@@ -89,10 +90,10 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
}
/* Remote site specified a certificate, but it's not correct */
-msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s",
+msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, serial=%s",
X509_STORE_CTX_get_error_depth(ctx),
X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)),
-subject);
+subject, serial ? serial : "");
ERR_clear_error();
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Log serial number of revoked certificate
Hi, Lev Thanks for review, I'll make improvements in V2. -- Best Regards, Vladislav Grishenko -Original Message- From: Lev Stipakov Sent: Wednesday, August 5, 2020 1:29 PM To: Vladislav Grishenko Cc: openvpn-devel Subject: Re: [Openvpn-devel] [PATCH] Log serial number of revoked certificate Hi, Compiled and tested on Ubuntu 20.04, looks good. A few nit-picks: > +verify_check_crl_dir(const char *crl_dir, int cert_depth, > +openvpn_x509_cert_t *cert, char *subject) The last parameter could benefit from const to indicate that function is not going to modify it. > -msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is > revoked", serial); > +msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked", > +cert_depth, subject, serial); Since you are modifying this line, could you add a NULL check to serial to and pass something like "" in this case? > +msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, > serial=%s: %s", > +cert_depth, subject, serial ? serial : "", errstr); I would use "" in NULL case, otherwise the error message becomes funny. > +msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, > + serial=%s", > X509_STORE_CTX_get_error_depth(ctx), > X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)), > -subject); > +subject, serial ? serial : ""); Same as above. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Support x509 field list to be username
OpenVPN has the ability to choose different x509 field in case "CN" can't be use used to be unique connected username since commit 935c62be9c0c8a256112df818bfb8470586a23b6. Unfortunately it's not enough in case client has multiple and valid certificates from PKI for different devices (ex. laptop, mobile, etc) with the same CN/UID. Having --duplicate-cn as a workaround helps only partially: clients can be connected, but it breaks coexistance with --ifconfig-pool-persist, --client-config-dir and opens doors to DoS possibility since same client device (with the same cert) being reconnected no more replaces previously connected session, so it can exhaust server resources (ex. address pool) and can prevent other clients to be connected. With this patch, multiple x509 fields incl. "serialNumber" can be chosen to be username as --x509-username-field space-separated parameters. Multiple fields will be joined into one username using '/' delimeter for consistency with CN/addr logging and preserving ability for hierarchical ccd. As long as resulting username is unique, --duplicate-cn will not be required. Default value is preserved as "CN" only. Openssl backend is the only supported at the moment, since so far MbedTLS has no alt user name support at all. v2: conform C99, man update, fix typos Signed-off-by: Vladislav Grishenko --- doc/man-sections/tls-options.rst | 9 -- src/openvpn/init.c | 4 +-- src/openvpn/options.c| 49 +--- src/openvpn/options.h| 4 +-- src/openvpn/ssl.h| 1 + src/openvpn/ssl_common.h | 8 +- src/openvpn/ssl_verify.c | 35 --- src/openvpn/ssl_verify_openssl.c | 14 + 8 files changed, 82 insertions(+), 42 deletions(-) diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst index 8c2db7cd..8449f5dc 100644 --- a/doc/man-sections/tls-options.rst +++ b/doc/man-sections/tls-options.rst @@ -632,13 +632,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa options can be defined to track multiple attributes. --x509-username-field args - Field in the X.509 certificate subject to be used as the username + Field list in the X.509 certificate subject to be used as the username (default :code:`CN`). Valid syntax: :: - x509-username-field [ext:]fieldname + x509-username-field [ext:]fieldname [[ext:]fieldname...] Typically, this option is specified with **fieldname** as either of the following: @@ -646,6 +646,7 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa x509-username-field emailAddress x509-username-field ext:subjectAltName + x509-username-field CN serialNumber The first example uses the value of the :code:`emailAddress` attribute in the certificate's Subject field as the username. The second example @@ -653,7 +654,9 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa ``fieldname`` :code:`subjectAltName` be searched for an rfc822Name (email) field to be used as the username. In cases where there are multiple email addresses in :code:`ext:fieldname`, the last occurrence - is chosen. + is chosen. The last example uses the value of :code:`CN` attribute in + the Subject field and the certificate's :code:`serialNumber` field in + hex delimited by slash symbol as the resulting username. When this option is used, the ``--verify-x509-name`` option will match against the chosen ``fieldname`` instead of the Common Name. diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 1ea4735d..11b417a8 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2912,9 +2912,9 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.verify_hash = options->verify_hash; to.verify_hash_algo = options->verify_hash_algo; #ifdef ENABLE_X509ALTUSERNAME -to.x509_username_field = (char *) options->x509_username_field; +memmove(to.x509_username_field, options->x509_username_field, sizeof(to.x509_username_field)); #else -to.x509_username_field = X509_USERNAME_FIELD_DEFAULT; +to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT; #endif to.es = c->c2.es; to.net_ctx = >net_ctx; diff --git a/src/openvpn/options.c b/src/openvpn/options.c index bc256b18..6fa17b3d 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -877,7 +877,7 @@ init_options(struct options *o, const bool init_gc) o->tls_cert_profile = NULL; o->ecdh_curve = NULL; #ifdef ENABLE_X509ALTUSERNAME -o->x509_username_field = X509_USERNAME_FIELD_DEFAULT; +o->x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT; #endif #ifdef ENABLE_PKCS11 o->pkcs11_pin_cache_period = -1; @@ -8434,41 +8434,44 @@ add_option(struct options *options, x509_track_add(>x509_track, p[1], msglevel, >gc); } #ifdef ENABLE_X509ALTUSER
[Openvpn-devel] [PATCH 2/2] Allow killing of client instances by cn with wildcards
In case of some permanent part of common name (ex. domain) and/or
long complex common name consisting of multiple x509 fields, it's
handly to kill client instances via management interface with just
part of common name, not by exact match only.
Patch allows to use wildcard placeholder '*' as the last trailing
symbol of kill command parameter.
Single '*' wildcard would be too greedy and can be too harmful,
therefore not allowed. Wildcards in the middle of parameter string
are not supported to keep the the things simple at the moment.
Signed-off-by: Vladislav Grishenko
---
doc/management-notes.txt | 2 ++
src/openvpn/multi.c | 6 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 61daaf07..91073693 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -195,6 +195,8 @@ Command examples:
kill Test-Client -- kill the client instance having a
common name of "Test-Client".
+ kill Test-Cli* -- kill the client instances having a
+ common name starting with "Test-Cli".
kill 1.2.3.4:4000 -- kill the client instance having a
source address and port of 1.2.3.4:4000
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 9bda38b0..8952658a 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3772,6 +3772,10 @@ management_callback_kill_by_cn(void *arg, const char
*del_cn)
struct hash_element *he;
int count = 0;
+/* Allow trailing wildcard */
+int len = strlen(del_cn);
+len += (len > 1 && del_cn[len-1] == '*') ? -1 : 1;
+
hash_iterator_init(m->iter, );
while ((he = hash_iterator_next()))
{
@@ -3779,7 +3783,7 @@ management_callback_kill_by_cn(void *arg, const char
*del_cn)
if (!mi->halt)
{
const char *cn = tls_common_name(mi->context.c2.tls_multi, false);
-if (cn && !strcmp(cn, del_cn))
+if (cn && !strncmp(cn, del_cn, len))
{
multi_signal_instance(m, mi, SIGTERM);
++count;
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/2] Support multiple x509 field list to be username
OpenVPN has the ability to choose different x509 field in case "CN"
can't be use used to be unique connected username since commit
935c62be9c0c8a256112df818bfb8470586a23b6.
Unfortunately it's not enough in case client has multiple and valid
certificates from PKI for different devices (ex. laptop, mobile, etc)
with the same CN/UID.
Having --duplicate-cn as a workaround helps only partially - clients
can be connected, but it breaks coexistance with --ifconfig-pool-persist,
--client-config-dir and opens doors to DoS possibility since same client
device (with the same cert) being reconnected doesn't replace previously
connected session no more, so can exhaust server ressources (ex. address
pool) and can prevent other clients to be connected.
With this patch, multiple x509 fields incl. "serialNumber" can be chosen
to be username as --x509-username-files space-separated parameters.
Multiple fields will be joined into one username using '/' delimeter for
consistency with CN/addr logging and preserving ability for hierarchical
ccd. As long as resulting username is unique, --duplicate-cn will not
be required. Default value is preserved as "CN" only.
Openssl backend is the only supported at the moment, since so far MbedTLS
has no alt user name support at all.
---
doc/man-sections/tls-options.rst | 9 ---
src/openvpn/init.c | 4 +--
src/openvpn/options.c| 46 ++--
src/openvpn/options.h| 4 +--
src/openvpn/ssl.h| 1 +
src/openvpn/ssl_common.h | 8 +-
src/openvpn/ssl_verify.c | 35
src/openvpn/ssl_verify_openssl.c | 15 +++
8 files changed, 83 insertions(+), 39 deletions(-)
diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index 8c2db7cd..301f8be4 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -632,13 +632,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
options can be defined to track multiple attributes.
--x509-username-field args
- Field in the X.509 certificate subject to be used as the username
+ Field list in the X.509 certificate subject to be used as the username
(default :code:`CN`).
Valid syntax:
::
- x509-username-field [ext:]fieldname
+ x509-username-field [ext:]fieldname [[ext:]fieldname...]
Typically, this option is specified with **fieldname** as
either of the following:
@@ -646,6 +646,7 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
x509-username-field emailAddress
x509-username-field ext:subjectAltName
+ x509-username-field CN serialNumber
The first example uses the value of the :code:`emailAddress` attribute
in the certificate's Subject field as the username. The second example
@@ -653,7 +654,9 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
``fieldname`` :code:`subjectAltName` be searched for an rfc822Name
(email) field to be used as the username. In cases where there are
multiple email addresses in :code:`ext:fieldname`, the last occurrence
- is chosen.
+ is chosen. The last example uses the value of :code:`CN` attribute in
+ the Subject field and the hex representation of certificate's serial
+ number delimited by slash symbol as the resulting username.
When this option is used, the ``--verify-x509-name`` option will match
against the chosen ``fieldname`` instead of the Common Name.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 1ea4735d..11b417a8 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2912,9 +2912,9 @@ do_init_crypto_tls(struct context *c, const unsigned int
flags)
to.verify_hash = options->verify_hash;
to.verify_hash_algo = options->verify_hash_algo;
#ifdef ENABLE_X509ALTUSERNAME
-to.x509_username_field = (char *) options->x509_username_field;
+memmove(to.x509_username_field, options->x509_username_field,
sizeof(to.x509_username_field));
#else
-to.x509_username_field = X509_USERNAME_FIELD_DEFAULT;
+to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
#endif
to.es = c->c2.es;
to.net_ctx = >net_ctx;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index bc256b18..a51038dd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -877,7 +877,7 @@ init_options(struct options *o, const bool init_gc)
o->tls_cert_profile = NULL;
o->ecdh_curve = NULL;
#ifdef ENABLE_X509ALTUSERNAME
-o->x509_username_field = X509_USERNAME_FIELD_DEFAULT;
+o->x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
#endif
#ifdef ENABLE_PKCS11
o->pkcs11_pin_cache_period = -1;
@@ -8434,7 +8434,7 @@ add_option(struct options *options,
x509_track_add(>x509_track, p[1], msglevel, >gc);
}
#ifdef ENABLE_X509ALTUSERNAME
-else if (streq(p[0], "x509-username-field") && p[1] && !p[2])
+else if (streq(p[0], "x509-username-field") && p[1])
{
[Openvpn-devel] [PATCH] Log serial number of revoked certificate
As it appears commit 767e4c56becbfeea525e4695a810593f373883cd "Log
serial number of revoked certificate" hasn't survive refactoring
of CRL handling.
In most of situations admin of OpenVPN server needs to know which
particular certificate is used by client.
In the case when certificate is valid, environment variable can be
used for that but once it is revoked, no user scripts are invoked
so there is no way to get serial number, only subject is logged.
Let's log certificate serial in case it is revoked and additionally
log certificate depth & subject in crl-verify "dir" mode for better
consistency with crl file (non-dir) mode.
Signed-off-by: Vladislav Grishenko
---
src/openvpn/ssl_verify.c | 7 ---
src/openvpn/ssl_verify_mbedtls.c | 5 +++--
src/openvpn/ssl_verify_openssl.c | 5 +++--
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 844bc57d..07745514 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -599,7 +599,7 @@ cleanup:
* check peer cert against CRL directory
*/
static result_t
-verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert)
+verify_check_crl_dir(const char *crl_dir, int cert_depth, openvpn_x509_cert_t
*cert, char *subject)
{
result_t ret = FAILURE;
char fn[256];
@@ -616,7 +616,8 @@ verify_check_crl_dir(const char *crl_dir,
openvpn_x509_cert_t *cert)
fd = platform_open(fn, O_RDONLY, 0);
if (fd >= 0)
{
-msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is
revoked", serial);
+msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked",
+cert_depth, subject, serial);
goto cleanup;
}
@@ -758,7 +759,7 @@ verify_cert(struct tls_session *session,
openvpn_x509_cert_t *cert, int cert_dep
{
if (opt->ssl_flags & SSLF_CRL_VERIFY_DIR)
{
-if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert))
+if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert_depth,
cert, subject))
{
goto cleanup;
}
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index fd31bbbd..e9982e41 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -68,6 +68,7 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert,
int cert_depth,
int ret = 0;
char errstr[512] = { 0 };
char *subject = x509_get_subject(cert, );
+char *serial = backend_x509_get_serial(cert, );
ret = mbedtls_x509_crt_verify_info(errstr, sizeof(errstr)-1, "",
*flags);
if (ret <= 0 && !openvpn_snprintf(errstr, sizeof(errstr),
@@ -82,8 +83,8 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert,
int cert_depth,
if (subject)
{
-msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s: %s",
-cert_depth, subject, errstr);
+msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, serial=%s:
%s",
+cert_depth, subject, serial ? serial : "", errstr);
}
else
{
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index ff14db23..20095cf7 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -71,6 +71,7 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
{
/* get the X509 name */
char *subject = x509_get_subject(current_cert, );
+char *serial = backend_x509_get_serial(current_cert, );
if (!subject)
{
@@ -89,10 +90,10 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
}
/* Remote site specified a certificate, but it's not correct */
-msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s",
+msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, serial=%s",
X509_STORE_CTX_get_error_depth(ctx),
X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)),
-subject);
+subject, serial ? serial : "");
ERR_clear_error();
--
2.17.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
