Regression #12884
closedOpenVPN status display for TAP mode services shows peer-to-peer instead of client list in certain cases
100%
Description
Problem: The webConfigurator OpenVPN status shows our TAP-mode "Remote Access (SSL/TLS + User Auth)" VPNs as peer-to-peer, when it should be displaying the list of connected clients. This affects both dashboard widget and Status|OpenVPN. Appears to have been introduced between pfSense Plus 21.05.2 and 22.01. Observed on Netgate 1541 hardware.
Expected behaviour (observed up to 21.05.2): As per attached screenshot from a pfSense Plus 21.02.2 device (different device but same VPN config approach), the TAP-mode service appears as a separate table, showing connected usernames and assigned IPs. We also see this behaviour on 21.05.2.
New behaviour (observed in 22.01): As per attached screenshot from a pfSense Plus 22.01 device, the section "Peer to Peer Server Instance Statistics" is present instead of the expected list of usernames/IPs.
Example sanitised config.xml extract attached as well. This was first observed when upgrading a device to 22.01 without changing the VPN config; I've just grabbed an extra screenshot from an older device to illustrate the previous behaviour.
Other observations:
Having looked at other bug reports etc., I think it's probably been introduced as part of this change:
https://redmine.pfsense.org/issues/12232
https://redmine.pfsense.org/projects/pfsense/repository/1/revisions/6c3bfb7322105ea0ab6f0fa30a8f63787afbb76e/diff/src/etc/inc/openvpn.inc
The OpenVPN management socket still correctly reports connected users if queried directly through a shell connection (this using the logic from the openvpn_get_server_status function in that openvpn.inc file):
[22.01-RELEASE][admin@hostname]/root: nc -U /var/etc/openvpn/server4/sock >INFO:OpenVPN Management Interface Version 3 -- type 'help' for more info status 2 TITLE,OpenVPN 2.5.4 amd64-portbld-freebsd12.3 [SSL (OpenSSL)] [LZO] [LZ4] [MH/RECVDA] [AEAD] built on Jan 13 2022 TIME,2022-02-26 15:28:10,1645849690 HEADER,CLIENT_LIST,Common Name,Real Address,Virtual Address,Virtual IPv6 Address,Bytes Received,Bytes Sent,Connected Since,Connected Since (time_t),Username,Client ID,Peer ID,Data Channel Cipher CLIENT_LIST,username,192.0.2.103:44116,203.0.113.10,,6218,8916,2022-02-26 15:27:46,1645849666,username,2,0,AES-128-GCM HEADER,ROUTING_TABLE,Virtual Address,Common Name,Real Address,Last Ref,Last Ref (time_t) ROUTING_TABLE,aa:bb:cc:dd:ee:ff@0,username,192.0.2.103:44116,2022-02-26 15:28:01,1645849681 GLOBAL_STATS,Max bcast/mcast queue length,1 END exit
Also just to explain the reasoning behind the configuration: We're using the TAP-mode service to bridge clients into a specific VLAN; the pfSense device has a physical connection but no assigned IP address. Clients get assigned an IP address by the OpenVPN service, but need to use a different device for the gateway after they're bridged into the VLAN. We need a custom server-bridge command to assign the custom gateway. We originally tried with an IP address assigned on the pfSense interface and using more of the built-in server bridge options, but that approach always assigned the pfSense interface IP as the gateway for clients.
Files
Related issues
Updated by Viktor Gurov almost 3 years ago
- Tracker changed from Bug to Regression
- Project changed from pfSense Plus to pfSense
- Category changed from OpenVPN to OpenVPN
- Affected Plus Version deleted (
21.02) - Affected Version set to 2.6.0
Updated by Viktor Gurov almost 3 years ago
- Related to Bug #12232: OpenVPN status incorrect for TAP servers without a defined tunnel network added
Updated by Viktor Gurov almost 3 years ago
- Assignee set to Viktor Gurov
Updated by Jim Pingle almost 3 years ago
- Status changed from New to Not a Bug
The status logic relies on the settings in the GUI fields to determine how to query the OpenVPN management interface. If you are using custom options for certain aspects of the configuration which conflict with what it shown in the GUI settings, the status code has no way to properly determine which method to use when making a query to the management interface.
You could use the system patches package to back out the other change (or apply a different one) to accommodate your customizations, but I don't think changing our code in a way that would break other valid modes to make this kind of custom setup work is a viable path forward.
If the existing server bridge GUI options are not working for your use case a better fix would be to implement changes that would allow you to make the configuration you need using the GUI. This would be fixed as a side effect. That is a fundamentally different feature request though, not a bug.
Updated by Evan Pearce almost 3 years ago
I've been able to reproduce it with a configuration that only uses the GUI options and no custom options, attached.
I don't think the GUI needs to interpret custom options, it just needs to realise that <mode>server_tls_user</mode> (in the config.xml settings) means that multiple users can connect and it should report the list of users. That corresponds to "Server mode: Remote Access (SSL/TLS + User Auth)" in the GUI.
(Looking back at #12232, the problematic config in that case had a mode of p2p_tls, which would correctly be interpreted as peer-to-peer. But the change made for that bug sweeps up any tap-mode service including user remote access.)
Updated by Marcos M almost 3 years ago
- Status changed from Not a Bug to Feedback
Feel free to test the following patch and let us know if it resolves your issue:
diff --git a/src/etc/inc/openvpn.inc b/src/etc/inc/openvpn.inc index 73cf7b4ab7..fce552f5f1 100644 --- a/src/etc/inc/openvpn.inc +++ b/src/etc/inc/openvpn.inc @@ -1889,12 +1889,11 @@ function openvpn_get_active_servers($type="multipoint") { list($tn, $sm) = openvpn_gen_tunnel_network($settings['tunnel_network']); if ((($server['mode'] == "p2p_shared_key") || - (($settings['dev_mode'] == 'tap') && empty($settings['tunnel_network'])) || + (($settings['dev_mode'] == 'tap') && empty($settings['tunnel_network']) && ($server['mode'] == 'p2p_tls')) || ($sm >= 30)) && ($type == "p2p")) { $servers[] = openvpn_get_client_status($server, $socket); } elseif (($server['mode'] != "p2p_shared_key") && - !(($settings['dev_mode'] == 'tap') && empty($settings['tunnel_network'])) && ($type == "multipoint") && ($sm < 30)) { $servers[] = openvpn_get_server_status($server, $socket);
https://docs.netgate.com/pfsense/en/latest/development/system-patches.html
Updated by Jim Pingle almost 3 years ago
- Status changed from Feedback to Pull Request Review
- Target version set to 2.7.0
- Plus Target Version set to 22.05
Updated by Viktor Gurov almost 3 years ago
- Status changed from Pull Request Review to Feedback
- % Done changed from 0 to 100
Applied in changeset 5f3aa9464e9b9b8062faa47e7552552ff3841d92.
Updated by Evan Pearce almost 3 years ago
- File 02-patched-p2p-appears-twice.png 02-patched-p2p-appears-twice.png added
- File 03-alternative-patch.png 03-alternative-patch.png added
The patch above resolves my issue -- once applied, the user remote access service displays client connections.
However, I think it regresses #12232 slightly. The patch makes the p2p_tls service from #12232 appear twice, as both an empty Client Connections box and the Peer-to-Peer Instance details. Refer 02-patched-p2p-appears-twice.png.
I get what I think is more correct output if the elseif clause keeps a negative version of the if clause, i.e. applying this instead of the patch above gives the result in 03-alternative-patch.png. The only difference is the extra added line:
--- a/src/etc/inc/openvpn.inc +++ b/src/etc/inc/openvpn.inc @@ -1889,12 +1889,12 @@ function openvpn_get_active_servers($type="multipoint") { list($tn, $sm) = openvpn_gen_tunnel_network($settings['tunnel_network']); if ((($server['mode'] == "p2p_shared_key") || - (($settings['dev_mode'] == 'tap') && empty($settings['tunnel_network'])) || + (($settings['dev_mode'] == 'tap') && empty($settings['tunnel_network']) && ($server['mode'] == 'p2p_tls')) || ($sm >= 30)) && ($type == "p2p")) { $servers[] = openvpn_get_client_status($server, $socket); } elseif (($server['mode'] != "p2p_shared_key") && - !(($settings['dev_mode'] == 'tap') && empty($settings['tunnel_network'])) && + !(($settings['dev_mode'] == 'tap') && empty($settings['tunnel_network']) && ($server['mode'] == 'p2p_tls')) && ($type == "multipoint") && ($sm < 30)) { $servers[] = openvpn_get_server_status($server, $socket);
But strictly referring to the server_tls_user service in this bug #12884, either version of the patch works for me.
Updated by Viktor Gurov almost 3 years ago
- Status changed from Feedback to New
Evan Pearce wrote in #note-9:
The patch above resolves my issue -- once applied, the user remote access service displays client connections.
However, I think it regresses #12232 slightly. The patch makes the p2p_tls service from #12232 appear twice, as both an empty Client Connections box and the Peer-to-Peer Instance details. Refer 02-patched-p2p-appears-twice.png.
I get what I think is more correct output if the elseif clause keeps a negative version of the if clause, i.e. applying this instead of the patch above gives the result in 03-alternative-patch.png. The only difference is the extra added line:
https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/665
Updated by Jim Pingle almost 3 years ago
- Status changed from New to Pull Request Review
Updated by Viktor Gurov almost 3 years ago
- Status changed from Pull Request Review to Feedback
Updated by Evan Pearce almost 3 years ago
Thanks, the combination of 5f3aa9464e9b9b8062faa47e7552552ff3841d92
then 9be20fdf57fe9c9c17aa16542189854dbf1cbebd
works for me. Tested on both:
2.6.0-RELEASE (amd64)
built on Mon Jan 31 19:57:53 UTC 2022
FreeBSD 12.3-STABLE
22.01-RELEASE (amd64)
built on Mon Feb 07 16:37:59 UTC 2022
FreeBSD 12.3-STABLE
Updated by Jim Pingle over 2 years ago
- Subject changed from webConfigurator OpenVPN status display for TAP mode services shows peer-to-peer instead of clients following Plus 22.01 upgrade to OpenVPN status display for TAP mode services shows peer-to-peer instead of client list in certain cases
Updating subject for release notes.
Updated by Jim Pingle over 2 years ago
- Status changed from Feedback to Resolved