Non exposed openvpn dhcp-option NBDD option

phil80

Hi,
While working to add more options to the OpenVPN Client Specific Overrides (redmine_12522), I found that the code to specify an NBDD server address (NetBIOS over TCP/IP Datagram Distribution Server) was properly implemented in all relevant code vpn_openvpn_server.php, vpn_openvpn_csc.php and openvpn.inc

Only the GUI menu is not present in both vpn_openvpn_server.php and vpn_openvpn_csc.php files

Is it on purpose ? Or should the code be either discarded or the menu option exposed ?
https://github.com/pfsense/pfsense/blob/master/src/etc/inc/openvpn.inc#L789

Note that there is a small error in code:
https://github.com/pfsense/pfsense/blob/master/src/usr/local/www/vpn_openvpn_server.php#L833

if ($pconfig['dns_server_enable']) {
	$server['nbdd_server1'] = $pconfig['nbdd_server1'];
}

// should be
if ($pconfig['nbdd_server_enable']) {
	$server['nbdd_server1'] = $pconfig['nbdd_server1'];
}

Should I submit a separate PR ?

jimp

I don't see anything that touched code specifically mentioning nbdd in over a decade so it probably was lost whenever the GUI was last overhauled (~2.3) or even older.

NetBIOS has mostly died in general, though, so even if it is fixed I'm not sure how useful it will be. MS deprecated it in Windows 2000, though it's still hanging on by a thread in some areas.

phil80

@jimp said in Non exposed openvpn dhcp-option NBDD option:

I don't see anything that touched code specifically mentioning nbdd in over a decade so it probably was lost whenever the GUI was last overhauled (~2.3) or even older.

NetBIOS has mostly died in general, though, so even if it is fixed I'm not sure how useful it will be. MS deprecated it in Windows 2000, though it's still hanging on by a thread in some areas.

Should I send a PR to remove the dead code ? Or rather expose the GUI option ?

jimp

Fixing the GUI option seems better at the moment, since it's partway to working. There may be someone out there who still needs it and OpenVPN itself hasn't deprecated it yet.

phil80

@jimp said in Non exposed openvpn dhcp-option NBDD option:

Fixing the GUI option seems better at the moment, since it's partway to working. There may be someone out there who still needs it and OpenVPN itself hasn't deprecated it yet.

Please look at this PR
https://redmine.pfsense.org/issues/13085?next_issue_id=13084

I am just not comfortable with pfsense using !(empty() for testing text/numeric form fields. This causes the value of 0 to be actually saved in settings, but it will not be exposed in GUI for example under NTP and WINS servers

I implemented the proper way for php to validate this type of checks and added it to the CSC page only. But the problem is probably across all GUI pages. Using empty() is probably safe under most circumstances, but it is not very clean in the way it is implemented.

I posted a gist with the php samples:
https://gist.github.com/PhilZ-cwm6/4676368b77fc01d478f42b5c94f26a66