Minor issue in Status -> OpenVPN page

dhatz

Using latest 2.1-BETA, when navigating to page Status -> OpenVPN, the buttons in the upper-right section reflect the running status / restart / stop of the wrong service, namely dnsmasq instead of openvpn.

jimp

Might check again on the next snapshot after the one you're on. I'm synced to the latest code and it's not doing that for me. I was doing quite a lot of work on that shortcut bar this weekend and it's possible that certain snapshots didn't have a complete set of the code.

dhatz

Just upgraded to 2.1-BETA0 built on Sun Aug 12 15:48:03 EDT 2012 and the issue remains (I also checked the html source just in case)

jimp

Could swear I replied to this but perhaps it was just as something was moved in the DC…

What is the exact URL you're on when this happens? And do you have a screenshot or something similar showing how it's linking to the wrong place?

I still can't reproduce it, and the code definitely isn't pointing the openvpn status page to dnsmasq...

phil.davis

Me too:
2.1-BETA0 (i386)
built on Sun Aug 12 15:48:49 EDT 2012
FreeBSD 8.3-RELEASE-p4

I started on a test system without any OpenVPN defined. At https://192.168.12.1/vpn_openvpn_server.php or status_openvpn.php I hover over the:
Green arrow button - "dnsmasq Service is Running"
Restart button - "Restart dnsmasq Service"
Stop button - "Stop dnsmasq Service"

Then added an OpenVPN shared-key Server, the results are the same.

I noticed that at one stage in the commits for this, the OpenVPN services bit was commented out in shortcuts.inc:

//$shortcuts['openvpn']['service'] = "openvpn";

But it is back in now (and in my snapshot).

I am not really sure what the running/restart/stop buttons are supposed to do in this context - there could be lots of OpenVPN Servers and Clients defined/listed on the page, or none at all. Is the button supposed to restart/stop all of them? Should the button not be there if there are no OpenVPN instances defined?

With the current code, in fbegin.inc:
a) $shortcut_section="openvpn" - good
b) $vpnid = "" - not so good
It calls find_service_by_openvpn_vpnid($vpnid) - the blank parameter makes this fall through to returning the first array (service) that get_services finds. That happens to be "dnsmasq" on many systems
if(isset($config['dnsmasq']['enable'])) {…

It needs $vpnid defined in fbegin.inc - but I can't see a sensible way to define it.

Of course, if I edit a particular OpenVPN Server instance, then the buttons do the sensible things for that instance.

phil.davis

Here is a screenshot of where the problem is - I didn't get it to capture the text when hovering. There are some bits of debugging echo stuff that I added to show the value of $shortcut_section, $vpnid and the array returned as $ssvc.

openvpn-buttons.jpg_thumb

phil.davis

Note that this issue happens on the following pages:
diag_logs_openvpn.php
status_openvpn.php
vpn_openvpn_server.php
vpn_openvpn_client.php
vpn_openvpn_csc.php
diag_logs_auth.php
status_captiveportal.php
services_captiveportal_zones.php

The opening page of each of these has a list of OpenVPN or CaptivePortal objects, or a status or log of zero to many of these objects. So the state/restart/stop buttons don't have a good unique meaning at these places.

jimp

And on my setup, I don't get start/stop controls on any of those pages. They're only supposed to show up when it can determine a proper/unique service for that page.

At first I left the service bit disabled for OpenVPN and captive portal because they needed extra code to determine the right process. Once I finished up that code, I enabled them again.

There must be something about the config that's causing it, or perhaps something needs (re)initialized at the start of the code.

jimp

Try syncing in the two commits I just made and see if it helps. I still can't replicate it here so I'm just guessing…

https://github.com/bsdperimeter/pfsense/commit/9cb6bef1e3f71d4eba30118bb0409a39e589a22b
https://github.com/bsdperimeter/pfsense/commit/d583aa710a765748b46555a388b8cdd70f327b0b

phil.davis

The first commit $ssvc = array(): does not do anything to help - but it will do no harm.
The second commit does the trick - I was about to try something like this, and you have used the real belts and braces approach to make sure to only match the desired entry.
I tested on another system that had cron, pfblocker and OpenVPN client export utility. On that system it would say "cron Service is Running" and so on - cron was the first service entry that it came across. The problem was the not set 'vpnid' array entry returns a NULL which was matching the NULL passed in as the $vpnid parameter.
I have gone to the various OpenVPN and CaptivePortal settings, status and log pages and all seems well.
I am surprised that you don't see this - I get it on 2 systems I have upgraded - 1 with a minimal config and one with a couple of packages installed and a bunch of OpenVPN links etc. Anyway - thems the breaks.

jimp

I'm surprised I didn't see it either, but no matter how much I clicked around on two separate VMs, it was always as it should have been.

I didn't try a fresh VM though. Anyhow, when it does work, it's the best thing since sliced bread. :-)