push-reset should not reset topology and route-gateway from global config

[Emmanuel Bretelle]

When using push-reset, all "push" option are reset (this also include topology and route-gateway).

Considering this server conf:

route 192.168.34.0 255.255.255.0
server 10.9.0.0 255.255.255.0

Considering a client with this part of the conf in his ccd's file:

push-reset
route 192.168.33.0 255.255.255.0
route 192.168.32.0 255.255.255.0

I expect client to get an IP in 10.9.0.0/24 range and get route added for 192.168.33.0/24 and 192.168.32.0/24
(but not 192.168.34.0/24)

In topology net30 (default topology) it works perfectly, but does not in topology subnet.
The reason is that in topology subnet, the following options MUST to be pushed:

• topology subnet
• route-gateway 10.9.0.1

Whithout these, the client will fail to set itself up with the following message:

Wed Jul 21 21:00:01 2010 Data Channel Decrypt: Using 160 bit message hash 'SHA1' for HMAC authentication
Wed Jul 21 21:00:01 2010 Control Channel: TLSv1, cipher TLSv1/SSLv3 DHE-RSA-AES256-SHA, 1024 bit RSA
Wed Jul 21 21:00:01 2010 [server] Peer Connection Initiated with [AF_INET]192.168.51.128:1195
Wed Jul 21 21:00:03 2010 SENT CONTROL [server]: 'PUSH_REQUEST' (status=1)
Wed Jul 21 21:00:03 2010 PUSH: Received control message: 'PUSH_REPLY,route 192.168.33.0 255.255.255.0,route 192.168.32.0 255.255.255.0,ifconfig 10.9.0.2 255.255.255.0'
Wed Jul 21 21:00:03 2010 OPTIONS IMPORT: --ifconfig/up options modified
Wed Jul 21 21:00:03 2010 OPTIONS IMPORT: route options modified
Wed Jul 21 21:00:03 2010 WARNING: Since you are using --dev tun with a point-to-point topology, the second argument to --ifconfig must be an IP address.  You are using something (255.255.255.0) that looks more like a netmask. (silence this warning with --ifconfig-nowarn)
Wed Jul 21 21:00:03 2010 ROUTE default_gateway=192.168.2.1
Wed Jul 21 21:00:03 2010 TUN/TAP device tun0 opened
Wed Jul 21 21:00:03 2010 TUN/TAP TX queue length set to 100
Wed Jul 21 21:00:03 2010 /sbin/ifconfig tun0 10.9.0.2 pointopoint 255.255.255.0 mtu 1500
SIOCSIFDSTADDR: Invalid argument

It seems to me that overriding topology and route-gateway from ccd can only break things and make the connection unusable.
Also they become compulsary and redundant when using push-rest in topology subnet .

Even though it can be worked-around by adding them to the ccd's file, it become a bit more complexed to handle these when the push options of the user are store remotely as this would involve to have a specific route-gateway set up for each openvpn server that will get users config from that remote backend.

What do you guys think? should those opeions (topology/route-gateway) be reset from push list ? or just overriden when provided by ccd's file.

Tks

[Emmanuel Bretelle]

Just to clarify. Here is the complete server config:

First a working server config (in topology net30)

tmp-dir /dev/shm
port 1195
proto udp
dev tun
ca keys/ca.crt
cert keys/server.crt
key keys/server.key  # This file should be kept secret
dh keys/dh1024.pem
server 10.9.0.0 255.255.255.0
ifconfig-pool-persist ipp.txt
push "route 192.168.34.0 255.255.255.0"
username-as-common-name
duplicate-cn
keepalive 10 120
tls-auth keys/ta.key 0 # This file is secret
cipher DES-EDE3-CBC  # Triple-DES
comp-lzo
client-cert-not-required
user nobody
group nogroup
persist-key
persist-tun
status openvpn-status.log
verb 4
plugin /home/chantra/openvpn-ldap-auth/src/.libs/libopenvpn-ldap-auth.so -c /etc/openvpn/ldap-auth/ldap-auth.conf

and user ccd's:

push-reset
push "route 192.168.33.0 255.255.255.0"
push "route 192.168.32.0 255.255.255.0"

This work perfectly.
Now, adding to the server config

topology subnet

will prevent the client to established the tunnel with the error provided in previous comment. The reasons being that no "topology" is pushed to client, neither route-gateway is.

A workaround would be to add to client's ccd file:

push "topology subnet"
push "route-gateway 10.9.0.1"

The reasons this issue happens seems to be down to ​http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=blob;f=helper.c;h=a9d7fd9fa2936cf5de1318ac7065372a251732b6;hb=refs/heads/bugfix2.1#l147

To solve this:

• either the push_options list should not be totally reset (keeping topology and rute-gateway options at least)
• or these last options should be kept in a separate push_options list that can only be overriden, not reset

[Emmanuel Bretelle]

Some more info can be found here:
​http://thread.gmane.org/gmane.network.openvpn.devel/3852

[Gert Döring]

We should do something about this :-) - now I ran into this myself on some of my setups, and it's stupid and annoying.

No code suggestions yet, just bringing this to people's memory again.

[Gert Döring]

push-reset should ignore certain options and leave them alone.

To be done:

• make a list of directives push-reset should ignore
• bring them to the list
• get agreement

[debbie10t]

push-reset also deletes server keepalive ping ping-restart parameters.

[diegofd]

Is there an ETA to fix this bug?

As workaround you must explicitly push all the needed parameters:

push-reset

push "topology subnet"
push "route-gateway 10.8.0.1"
push "ping 10"
push "ping-restart 120"
push "route 10.8.0.0 255.255.255.0"

[debbie10t]

For me personally, I think --push-reset should remain as is and cancel all server config --push statements. As you add --push-reset to the client CCD file anyway it behooves the administrator to write the CCD file correctly - Although, --topology is a possible exception as the connection will fail without it.

[egroeper]

ACK @debbie10t. At my installation I'm pushing different subnets via client config dir to some clients. Therefore these clients get a different route-gateway (and different routes).

Currently every issue regarding this can be fixed by a correct ccd config. If some setting would be unresettable, people with complex setups will get problems.

Perhaps we should extend the docs to make it more clear, that all server config statements are reset (i.e. notes at --server and --server-bridge directive or an additional note at --push-reset directive).

I don't see a use case for changing topology via ccd. So I agree, that this settings could be an exception.

Please don't make settings like route-gateway unresettable (unchangeable). Being able to override these settings (like suggested by chantra), would be okay, too.

[Gert Döring]

So, small update from the development side of things - a new option has been added (to git master) which will solve the underlying option ("I need to unset certain options for specific clients") without being as radical as --push-reset:

commit 970312f185012341cc5bcc9492ab3e1413c7b3c7
Author: Gert Doering <gert@…>
Date: Mon May 16 12:13:04 2016 +0200

Implement push-remove option to selectively remove pushed options.

With this option, the server can remove individual options from the
set pushed to a client (call from --client-config-dir file, or from
--client-connect script or plugin). Options are removed at parse
time, so it is possible to do stuff like:

push-remove route-ipv6
push "route-ipv6 fd00::/8"

to first remove all IPv6 route options set so far, then add something
specific (what "push-reset" does to all the options).

Arguments to push-remove are strncmp()'ed to option string, so partial
matches like

push-remove "route-ipv6 2001:"

are possible ("remove all IPv6 routes starting with 2001:").

this solves all cases where I had to use --push-reset so far ("do not send IPv6 options to client xyz", "do not send a default route to client abc"), and is much more fine-grained.

Still, the original topic here warrants a closer look at the --push-reset documentation, and possibly at trying to make an exception for --topology (or maybe not - if you really insist on it, you can run a server with --topology X and a client with --topology Y, provided you use --ifconfig-push with the appropriate values for the client's topology setting...)

[Gert Döring]

So. After long and hard deliberation (read: everbody looked the other way for 10 years), I've actually written a documentation update for --push-reset, pointing out the problem, and recommending use of --push-remove instead.

Comments on the list were between "yeah", "let's just deprecate --push-reset!" and "can we not fix push-reset?"... I am closing this ticket now, but if someone wants to send a patch that fixes this in a nice way (like, not 500 lines of code...) we can have a look.

I am not going to write that patch, because I think there's too many config variants that lead to different decisions on "what options are critical?", and I think that --push-reset is enough of an expert tool that documenting the risks is good enough.

Anyway:

commit 5fd66510dfdef628fa95f156c5f9d80af9ae1531 (master)
commit cdeef20bc6ea4c15824427055f2ffeff53651dee (release/2.5)
commit d61cbfcde78bf65ec677d164d5d03e00f092befd (release/2.4)
Author: Gert Doering
Date: Tue Sep 8 13:15:11 2020 +0200

Document that --push-remove is generally more suitable than --push-reset

will be part of 2.4.10 and 2.5.0 release.