Restore missing data to the OpenVPN Connection Status page

[benyamin-codez]

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

• [x] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
• [x] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Is your feature request related to a problem? Please describe.

Some Client information is missing from the VPN: OpenVPN: Connection Status page, perhaps since 23.1.4.

Several previous closed issues could be referenced, namely those involved in refactoring for MVC work, but @fichtner requested this feature request [1].

Describe the solution you like

In respect of clients connecting to remote hosts, in the VPN: OpenVPN: Connection Status page:

1. Restore data in the "Real Address", "Bytes Sent" and "Bytes Received" columns in the table on the Sessions tab;
2. Add data to the "Common Name" column in the table on the Sessions tab; &
3. Add data to the relevant table on the Routes tab.

Historically, opnsense has made use of the OpenVPN Management Interface [2] state and status commands (via a UNIX socket). The status 3 command will only supply traffic data for clients or point-to-point instances (herein lies one problem), for multi-client server instances the list returned includes Common Name, Real Address, Bytes Received, Bytes Sent, Connected Since, Virtual Address, Virtual IPv6 Address, Username, Client ID, Peer ID, and Data Channel Cipher (as of version 2.5) plus a list of connected clients and a routing table.

Between 23.1.3 and 23.1.4, MVC refactoring work was completed on the OpenVPN plugin [3]. This included removal of the functions _openvpn_get_activeclients and _openvpn_get_clientstatus from src/etc/inc/plugins.inc.d/openvpn.inc.

The functionality seems to have been migrated to the new file src/opnsense/scripts/openvpn/ovpn_status.py. The _opnstatus function appears to only consider local servers and their client connections, and not client connections to remote hosts (this is where the problem likely lies). The _ovpnstate function is client/server agnostic. The main function does differentiate between client and server, but the _opnstatus function appears to supply defective data for clients.

The "Common Name" is a new field that has been recently added - at least for clients. I am unsure as to the best way to get this information. The technique for multi-client server instances will not work. Processing the relevant certificates might yield some information but it may not be accurate as the information might come from the remote host during verification. Parsing of the log stream will likely yield multiple results which could be stored. The results would likely be a DNS name for the remote host (depth=0), a user name/description or issuing/root CA (depth=1) depending on the authentication scheme and perhaps a root CA (depth=2). Displaying all of these might be of some utility.

Route information is also missing (in the Routes tab on the same page), but I am unsure whether that applies to client sessions terminating on remote hosts. Information on this tab may only relate to clients connecting to a local OpenVPN server. Route information may or may not be a new feature; I haven't noticed it before. In any case, the technique used for multi-client server instances will not work. I am unaware of any technique to gather this information, save for passing the log stream for any route add commands (which may fail), or parsing the routing table proper and programmatically determining which routes apply to which clients.

Describe alternatives you considered

Use of a status file rather than a socket but that will not work as the same limitations apply.

Additional context

[1] https://forum.opnsense.org/index.php?topic=33314.msg161231#msg161231 [2] https://openvpn.net/community-resources/management-interface/ [3] https://github.com/opnsense/core/commit/7272b4bb036

[AdSchellevis]

At a first glance /usr/local/opnsense/scripts/openvpn/ovpn_status.py --options client does deliver the proper byte counters, but the fieldnames are different. Common name is likely empty as OpenVPN's status probably doesn't return one for clients (in which case empty is fine).

You probably have to compare output between versions in order to know if routes did show something different, in my understanding the route information is only relevant/provided for severs, previous code:

https://github.com/opnsense/core/blob/2a970b568a2839f2a73f04fbab2f36a532bcc2d2/src/www/status_openvpn.php#L190-L224

So long store short, the counters are likely easy to fix, other concerns look a bit off-topic in relation to the previous status page.

[benyamin-codez]

I think that's perhaps one reason Franco suggested a feature request.

Route info and the CN are only provided for multi-client server instances via the status 3 command. Route info could be useful depending on client config, e.g. some of my clients have explicit routes added, some are from me, some are from the provider. CN could be useful especially if several connections are made to the same remote host with different users who are assigned random VIPs via DHCP.

Just a suggestion, but would "Not Applicable" be better than an empty CN field? Would a similar disclaimer for a missing Routes table be appropriate, e.g. "Not applicable for clients connecting to remote hosts" when no route data is available? I see some utility in having real data for these, but at the moment it comes off as a bit broken.

Regarding "Real Server": if that is supposed to be the remote host IP, it is available from the state command and the ovpn_status.py script, but it is not being displayed. Is that also a case of the field name being different? It does seem these are the ones that could be easily fixed and (re)implemented.

[AdSchellevis]

I think that's perhaps one reason Franco suggested a feature request.

Probably, the counters are a bug, other questions are feature requests indeed. Better to split both for cleaner tickets, I can close this one as a bug with a patch presenting the counters.

Route info and the CN are only provided for multi-client server instances via the status 3 command. Route info could be useful depending on client config, e.g. some of my clients have explicit routes added, some are from me, some are from the provider. CN could be useful especially if several connections are made to the same remote host with different users who are assigned random VIPs via DHCP.

I just don't think the scope of the openvpn status is to present something that lives elsewhere, if you want to find the routes attached (not being tracked by OpenVPN), the route status page is the place to look, as it shows all routes. The reason for OpenVPN offering more fine grained routes is that within the same (server) device routes are treated differently. When it was only about the routes that netstat reported, I would have dropped the view in this case.

We should however restructure the documentation on our end and explain the use of the status page, I don't think there currently is (much) documentation on this subject at the moment.

Just a suggestion, but would "Not Applicable" be better than an empty CN field? Would a similar disclaimer for a missing Routes table be appropriate, e.g. "Not applicable for clients connecting to remote hosts" when no route data is available? I see some utility in having real data for these, but at the moment it comes off as a bit broken.

I don't think so, no, throughout the product fields are irrelevant for different scenario's, marking them "Not Applicable" suggests the systems knows about the intent (which it often doesn't), there's a thin line between misconfigured and not relevant, extra code often doesn't help in these cases (more boilerplate, easier to masquerade bugs). Also one of the reasons why we're not "translating" status codes anymore in this code, makes it harder for people to track issues.

Regarding "Real Server": if that is supposed to be the remote host IP, it is available from the state command and the ovpn_status.py script, but it is not being displayed. Is that also a case of the field name being different? It does seem these are the ones that could be easily fixed and (re)implemented.

I have to check, but when returned and not displayed, I would expect it's. a bug indeed.

[benyamin-codez]

Thanks Ad :thumbsup: :pray: I'm curious to see where the field names where different... :eyes:

[AdSchellevis]

@benyamin-codez this https://github.com/opnsense/core/commit/a5c4de07b0512518d07aee2624c4ff314d09b778 should do the trick, we just read what's being offered by OpenVPN, the original mapping was in the openvpn_get_client_status() function.

https://github.com/opnsense/core/blob/2a970b568a2839f2a73f04fbab2f36a532bcc2d2/src/etc/inc/plugins.inc.d/openvpn.inc#L1350-L1355

we parse tcp/udp stats here (now with mapping):

https://github.com/opnsense/core/blob/a5c4de07b0512518d07aee2624c4ff314d09b778/src/opnsense/scripts/openvpn/ovpn_status.py#L69-L75

[benyamin-codez]

Thanks Ad, elegant solution.

We just need to update /src/www/widgets/widgets/openvpn.widget.php, Line 123, to avoid a regression in the widget:

---         <td><?=$client['remote_host'];?><br/><?=$client['virtual_address'];?></td>
+++         <td><?=$client['real_address'];?><br/><?=$client['virtual_address'];?></td>

Also note: https://github.com/opnsense/core/commit/a0c8016b2

[benyamin-codez]

iirc, the stats were also human readable with a unit suffix,KB/MB/GB, or was it KiB/MiB/GiB...

Maybe using something like this: https://gist.github.com/liunian/9338301?permalink_comment_id=2183132#gistcomment-2183132

Maybe call as humanize() or humanise()... :fire:

[AdSchellevis]

this should fix the last flitches https://github.com/opnsense/core/commit/3066c875a2f61bde48a230ebb3fc150f52cf3db1

[benyamin-codez]

In the .volt where it belongs. Nice! Thanks Ad.