Feature #12522
closedMore GUI options for OpenVPN Client-Specific Overrides
100%
Description
I setup an OpenVPN server, let's say 10.10.10.0/24, which works properly
I setup some custom exceptions for a specific user under "Client Specific Overrides"
Issues:
- If option "Prevent this client from receiving any server-defined client settings." is checked:
+ even specifying "IPv4 Tunnel Network" option in client overrides, will always advertise net30 topology on the client (most clients default to net30)
This breaks "OpenVPN Connect" which fails to connect and causes a warning on clients like "OpenVPN for Android"
+ gateway route is not pushed from the server and we cannot define it in client overrides. Open VPN Connect client will fail to provide a gateway route and internet connection from client fails. Other clients make assumption of the gateway and default to 10.10.10.1
+ all other server defined options are lost
- If option "Prevent this client from receiving any server-defined client settings." is unchecked:
+ the options from server are still pushed to the client even if they are defined in "Client Specific Overrides"
This is the case for all "dhcp-option dns" entries from the server still pushed to the client
Current possible fix:
We must set these 2 options in Advanced section under "Client Specific Overrides"
- push "route-gateway 10.10.10.1": else all internet traffic will be denied with OpenVPN Connect
- push "topology subnet": else, OpenVPN connect will fail. OpenVPN For Android client will warn that the topology is net30 but the domain is subnet, and will assume subnet, so it can connect
Optionally, all other options in server need to be set manually too if we want to keep them:
- push "block-outside-dns"
- push "register-dns"
- push "redirect-gateway ipv6":
- push "ping 10"
- push "ping-restart 60"
Finally, the help doc specifies that for setting a fixed IP we need this option:
- ifconfig-push 10.10.10.250 255.255.255.0
This is not true because completing the "IPv4 Tunnel Network: 10.10.10.250/24" automatically adds the ifconfig-push ip to the client
Changes suggested:
- always push the proper server topology when we complete the "IPv4 Tunnel Network" field
- always push the server gateway route when setting client overrides
- optionally: give the ability to set all client exceptions from GUI like the client options in server and in this case
The two first suggestions are a bug fix because they break the connection
Files
Updated by Phil Wardt about 3 years ago
Notes:
Maybe one option would be to add an option "Client setting override server defined client options"
This option would do a "push-remove" for any override defined option, mainly the DNS options because there is currently no way o override them without a "push-reset" triggered by "Prevent this client from receiving any server-defined client settings."
The relevant code is here:
https://github.com/pfsense/pfsense/blob/master/src/usr/local/www/vpn_openvpn_csc.php#L458
Or like I wrote above, after the triggered push-reset, properly push the topology and gateway route
Also, maybe when we check "Prevent this client from receiving any server-defined routes without removing any other options.", it seems to cause a "push-remove route":
https://github.com/pfsense/pfsense/blob/master/src/usr/local/www/vpn_openvpn_csc.php#L465
We should either warn that the gateway is unspecified or better offer a field to specify the gateway
Updated by Jim Pingle almost 3 years ago
- Tracker changed from Bug to Feature
- Subject changed from OpenVPN Client Specific Overrides: "Prevent this client from receiving any server-defined client settings." should preserve topology and gateway to More flexible Client-Specific Override options for controlling options pushed to clients
- Priority changed from Normal to Very Low
- Affected Version deleted (
2.5.2) - Affected Architecture deleted (
All)
It's doing exactly what it says. Normally the client configuration would include the topology rather than having it pushed. People who use that option now typically want what it does exactly, they don't want anything pushed from the server and want to manage it all themselves, which is what it currently does.
It sounds like what you want is more like pull-filter
on the clients.
This could still be a potential new feature in the future (or set of new features) but it's not a bug since it's operating as designed.
Updated by Phil Wardt almost 3 years ago
Jim Pingle wrote in #note-2:
It's doing exactly what it says. Normally the client configuration would include the topology rather than having it pushed. People who use that option now typically want what it does exactly, they don't want anything pushed from the server and want to manage it all themselves, which is what it currently does.
It sounds like what you want is more like
pull-filter
on the clients.This could still be a potential new feature in the future (or set of new features) but it's not a bug since it's operating as designed.
The bug part is this:
When that option is checked, the DNS Settings and any custom available option we set in the Client-Specific Override page is properly pushed to the client when we set it in the GUI, except the topology !
Basically, here are some of the GUI options pushed properly from Client Specific Overrides:
+ IPv4 Tunnel Network: ifconfig-push client_ip mask
+ Redirect gateway: push "redirect-gateway def1"
+ DNS server: push "dhcp-option DNS ip"
+ NTP server: push "dhcp-option NTP ip"
However, the "IPv4 Tunnel Network" help tip in GUI says:
The virtual IPv4 network used for private communications between this client and the server expressed using CIDR (e.g. 10.0.8.5/24).
With subnet topology, enter the client IP address and the subnet mask must match the IPv4 Tunnel Network on the server.
With net30 topology, the first network address of the /30 is assumed to be the server address and the second network address will be assigned to the client.
We first understand that setting IP/24 should push topology subnet and setting IP/30 should push the net30 topology, which is not the case.
The overrides pages is full of options except two of the mandatory ones to make the connection work: push topology and push route-gateway
Maybe it would be more functional and user friendly to:
- add the gateway option in overrides page
- push the proper topology as the help tip states when we set the "IPv4 Tunnel Network" field
- or add the topology field in the overrides if you want to separate it from "IPv4 Tunnel Network"
Updated by Phil Wardt almost 3 years ago
A last note if the features are revised added/once:
The title of the tab is "Client-Specific Override". I never expected when defining some DNS entries that they are pushed at the bottom of the dns list after the dns servers were pushed. I expected the dns entries here properly override the server dns entries.
Maybe the help tips should get improved, or adding a check box "override server options" that will do a push-remove for the specified overrides in this tab
Also, adding a tip to specify what the existing checkboxes actually do: push-reset, push-remove...
Thank you for looking at it and best regards
Updated by Jim Pingle almost 3 years ago
Phil Wardt wrote in #note-3:
Jim Pingle wrote in #note-2:
The bug part is this:
When that option is checked, the DNS Settings and any custom available option we set in the Client-Specific Override page is properly pushed to the client when we set it in the GUI, except the topology !
Yes, that's exactly expected. When you check it, nothing from the server is pushed, only the settings from the override. There is no setting on the override for toplogy.
However, the "IPv4 Tunnel Network" help tip in GUI says:
[...]
We first understand that setting IP/24 should push topology subnet and setting IP/30 should push the net30 topology, which is not the case.
You have that backward, it's saying your value here must match the topology used on the server. It does not control the topology sent to the client.
The overrides pages is full of options except two of the mandatory ones to make the connection work: push topology and push route-gateway
Maybe it would be more functional and user friendly to:
- add the gateway option in overrides page
- push the proper topology as the help tip states when we set the "IPv4 Tunnel Network" field
- or add the topology field in the overrides if you want to separate it from "IPv4 Tunnel Network"
Adding new fields for those two is possible, but it's not a bug, it's a feature request. I would not automatically push a topology to a client by guessing based on the subnet mask, it should be a drop-down selection. If it were to copy the value from the server there would need to be additional validation forcing the user to make the override active only on a single server instance, which would add more complexity and room for error.
Updated by Phil Wardt almost 3 years ago
Jim Pingle wrote in #note-5:
Yes, that's exactly expected. When you check it, nothing from the server is pushed, only the settings from the override. There is no setting on the override for toplogy.
You have that backward, it's saying your value here must match the topology used on the server. It does not control the topology sent to the client.
Adding new fields for those two is possible, but it's not a bug, it's a feature request.
I get your point now that I understood it. But at first, it took me a while to figure it out and look at the client debug code to find the pushed options. I thiught it was a bug somewhere because it works on OpenVPN for Android out of the box, but fails on OpenVPN Connect client which is more strict regarding topology and gateway
Well, let's hope someone looks at the code there once, or maybe just enhancing the help man or the tool tips
Thank you
Updated by Phil Wardt over 2 years ago
Jim Pingle wrote in #note-5:
Yes, that's exactly expected. When you check it, nothing from the server is pushed, only the settings from the override. There is no setting on the override for toplogy.
Adding new fields for those two is possible, but it's not a bug, it's a feature request.
I took a little time and adapted the GUI as below:
https://philz-cwm6.github.io/pfsense_preview/vpn_openvpn_csc_v1dot3_gui.html
GUI Changes:
- add individual server overrides for the optional client settings (local routes/gateway, remote routes, DNS, NTP, Netbios...)
- add option to override topology when selecting "Remove All Server Options" (push-reset)
- add option to set gateways (useful when removing routes)
- add option to override Redirect IPv6 Gateway in addition to existing IPv4 option
- move all server override options to the "Override Configuration" group
- hide override options with a "Select Server Overrides" toggle
- disable un-needed overrides when "Remove All Server Options" (push-reset) is selected
- when "Redirect IPv4/6 Gateway" is toggled, the Local Routes fields are hidden/shown correspondingly like for the server edit part
- fix bug when "0" is entered in DNS/Domain... fields (using !empty() discards 0)
- fix GUI toggle for NTP and DNS options causes inverted results when repeating clicks too quickly
Please use the provided html link to test the GUI only part. I tried to make it openvpn noob friendly.
https://philz-cwm6.github.io/pfsense_preview/vpn_openvpn_csc_v1dot3_gui.html
If it seems fine to you, I will push the source and start adapting code in openvpn.inc file. That should be the simpler part, the harder was the GUI to find something friendly.
Updated by Phil Wardt over 2 years ago
Updated by Phil Wardt over 2 years ago
Had to reset the repo, sorry, updated links and a fixed patch
I pushed the GUI changes code:
https://github.com/pfsense/pfsense/pull/4570
Preview of page result:
https://philz-cwm6.github.io/pfsense_preview/vpn_openvpn_csc_v1dot3_gui.html
Fixed Patch for v2.6.0: attached
Updated by Phil Wardt over 2 years ago
- File vpn_openvpn_csc-v1.3.2-master.patch vpn_openvpn_csc-v1.3.2-master.patch added
- File vpn_openvpn_csc-v1.3.2-pfsense2.6.patch vpn_openvpn_csc-v1.3.2-pfsense2.6.patch added
Attached patch for both current master branch , and for release 2.6.0
It includes last upstream 0/empty() fix
Updated by Phil Wardt over 2 years ago
- File openvpn_enhanced_overrides_master.patch openvpn_enhanced_overrides_master.patch added
- File openvpn_enhanced_overrides_pfsense2.6.patch openvpn_enhanced_overrides_pfsense2.6.patch added
I pushed the full changes with the actions set in openvpn.inc. That was the easier part as most of them were already there and just needed the settings implementation through GUI
A clickable preview of the page is here:
https://philz-cwm6.github.io/pfsense_preview/vpn_openvpn_csc-v1dot5.html
I fully tested all the functions and client side logs on connection.
The config shown in preview page for example does a `push-reset` followed by all needed actions for a working connection.
This simplifies the user interaction as it is not longer mandatory to enter custom options.
We can also remove selectively server-defined routes or DNS servers and push them to the custom config without a whole push-reset
Attached patches for current master branch and pfsense v2.6 release
Note: The previous v2.6 GUI patches had a bug with the tunnel not being saved. The bug was only in the patch, and not in github commits. It was due to the changes between current master branch and 2.6 release
Updated by Phil Wardt over 2 years ago
A last note: the changes are very conservative and follow the code / layout of vpn_openvpn_server.php code
- One of the commits also changes deprecated variables from legacy to new which seems to be safe in my tests but needs a closer look:
https://github.com/pfsense/pfsense/pull/4570/commits/9ef7329d210adbd2e377a97e3bda03bd4f547799
- Also, I commented the ipv6 gateway code I added (same as server current implementation)
https://github.com/pfsense/pfsense/pull/4570/commits/9876e6fb58320d3f3bbc9bb11279e46cd0607cfd
Updated by Phil Wardt about 1 year ago
I pushed a clean version for 2.7
Hope it can be reviewed
https://github.com/pfsense/pfsense/pull/4570
Preview link:
https://philz-cwm6.github.io/pfsense_preview/vpn_openvpn_csc_v1dot3_gui.html
Patches for 2.7 attached
Updated by Phil Wardt about 1 year ago
Updated by Marcos M about 1 year ago
- Subject changed from More flexible Client-Specific Override options for controlling options pushed to clients to Make Client-Specific Override options more flexible
- Status changed from New to Pull Request Review
- Assignee set to Marcos M
- Priority changed from Very Low to Low
Updated by Phil Wardt 12 months ago
I updated the commit as you suggested
https://github.com/pfsense/pfsense/pull/4570
Updated by Phil Wardt 12 months ago
- File openvpn_cso.patch openvpn_cso.patch added
and a patch file for current master branch
Updated by Marcos M 4 months ago
- Subject changed from Make Client-Specific Override options more flexible to Expose additional OpenVPN CSC options to the GUI
- Status changed from Pull Request Review to Feedback
- Target version set to 2.8.0
- % Done changed from 0 to 100
- Plus Target Version set to 24.08
The following new override options are now available in the GUI:
- Reset individual server options
- IPv4 Gateway
- IPv6 Gateway
- Redirect IPv6 Gateway (IPv4 already exists)
- Ping Interval
- Ping Action
- Block Outside DNS
- Force DNS cache update
Updated by Phil Wardt 4 months ago
Thank you for taking the lead with this as I did not have much time left
I checked the code and applied a patch for testing. I see many issues:
1- All below functions do nothing as they are not implemented in openvpn.inc:remove_iroute
remove_dnsdomain
remove_dnsservers
remove_dnsservers
remove_ntpservers
remove_ntpservers
remove_netbios_ntype
remove_netbios_scope
remove_wins
2- The ping options from server cannot be removed here, so they will always add to the ones configured here (pushed twice in logs)
3- When we select Reset all options
, we still end up with topology always removed and needing to be set as custom option. In my version, when selected, user gets a GUI option to set the topology
4- This code can never be true currently:
https://github.com/pfsense/pfsense/blob/2203dde032b983efc4ea173f2403366d9df2acc5/src/etc/inc/openvpn.inc#L1695if ($keep_minimal && ($option == 'remove_route')) {
$auto_config_gateway4 = true;
$auto_config_gateway6 = true;
}
So, when user selects to remove route, the gateway will be removed anyway. Maybe add a note for it like (override routes and gateways)
We can define them in teh GUI, so it is ok at the end
Updated by Marcos M 4 months ago
Thanks! We don't want to remove the ability to reset all options (including topology). The "keep minimal" option is there for the case where options should be reset while keeping the required client configuration (topology/gateway), in which case we determine it automatically per server config. I've pushed a fix for the mentioned issues and included a new option "Inactivity Timeout" to remove/override. See 3ec783537d1b32d30132b51aa8bfd7cd44f314fd
Updated by Phil Wardt 4 months ago
Phil Wardt wrote in #note-23:
The new commit looks fine to me except a few small points:
1- can you change the GUI entryRemove Options -> Local Routes
toRemove Options -> Local Routes and Gateways
so that it is clear that the gateways are removed here
2-Keep minimal options
could display afterRemove Options
so that it is clear that when selectingRemove Options -> Local Routes and Gateways
it will properly keep the server Gateways. Else users can wonder if it overrides previous option
3- I notice that there is no way to remove server defined optionsRedirect IPv4 Gateway
andRedirect IPv6 Gateway
. That was also the case in my code as I missed it too. Can you add an optionRemove Options -> Redirect Gateways
that will do apush-remove redirect-gateway
?I think it will be really a friendlier GUI now :-)
Updated by Phil Wardt 4 months ago
- File patch_openvpn_csc.patch patch_openvpn_csc.patch added
Updated by Phil Wardt 4 months ago
I am testing deepling and options do not work as intended
Please hold and I will give deeper feedback
Updated by Phil Wardt 4 months ago
Ok, there were issues with your current patch:
- the state of keep_minimal was not saved
- the push-remove options were not processed properly
I pushed a new fix that I properly tested through a VPN connection and for all scenarios.
https://github.com/pfsense/pfsense/pull/4691
1- Options are properly removed in remote client when selected
2- keep_minimal
is properly saved
3- added options to remove Block Outside DNS
and Redirect Gateways
4- Changed description Remove Options -> Local Routes
to Remove Options -> Local Routes and Gateways
5- Keep minimal
section moved after Remove Options
Atteched a patch for pfsense 2.7.2 release
Updated by Phil Wardt 4 months ago
Your last commit https://github.com/pfsense/pfsense/commit/012e36e6b87cf63743e644179a15e67f38d94f77
still has issues. As I said, the overrides are not properly processed:
- when you select push_reset
all server options are still pushed to the client, include topology. The reset command is never pushed
- when you select push_reset
, the option keep_minimal
won't have any effect as server options are always pushed
- when we enable and save keep_minimal
, if we select Keep all server options
and save config, the keep_minimal
flag remains enabled in config unlike you want based on your last github comment. You can verify it by selecting again push_reset
and the flag will still be checked
This patch fixes mentioned issues:
https://github.com/pfsense/pfsense/pull/4692
It also ensures no flags are set in config if not specified
It also ensures keep_minimal
is not preserved when we select remove_specified
while not selecting any remove_options
I again tested it in OpenVPN client logs and checked saved config changes for each scenario
Attached a patch for pfsense v2.7.2 release
Updated by Phil Wardt 4 months ago
Attached a new patch for pfsense 2.7.2 release
It implements the latest proposed changed on github commit
https://github.com/pfsense/pfsense/pull/4692
Note that previous v1.7 attached patch also works properly without bugs or missing functions
Updated by Phil Wardt 4 months ago
- File openvpn_csc_patch_v1.10-pfsense-v2.7.2.patch openvpn_csc_patch_v1.10-pfsense-v2.7.2.patch added
For testing, I added a new patch for current pfsense 2.7.2 release
Mainly a code cleanup, no fixes as of previous patch
https://github.com/pfsense/pfsense/pull/4692
Updated by Jim Pingle about 1 month ago
- Subject changed from Expose additional OpenVPN CSC options to the GUI to More GUI options for OpenVPN Client-Specific Overrides
Updated by Jim Pingle about 1 month ago
- Plus Target Version changed from 24.08 to 24.11