Bug #5693
closed
Hide/Mask stored passwords when presenting GUI forms
50%
Description
Currently there are a number of places in the GUI for the user to enter passwords used by additional or external systems (Proxies, Notifications, auth servers, HA Sync, etc) and when a password is stored, it is presented back to the user in the form. Ideally this should not be the case. Even though the passwords are stored plain in config.xml if a user has limited access they couldn't see the raw config, but they could potentially get a stored password from the GUI.
A couple ideas (not necessarily the best way, suggestions only):- Come up with a "standard" password entry method that has both a password and confirm password box
- Don't present the real password to the user, but some fake/pre-determined value that the code will recognize and ignore so it doesn't clobber the real password when saving, it would only save the entered value if both the password and confirm password match.
- On pages where the password is optional, retain the ability to leave the password blank/have no password
It would be extra nice if we could find some way to prevent browsers from auto-fill spamming everything with the GUI password if the user has it stored as well, which is another common source of misery in this area.
Mostly complete list of affected pages/fields:- system_hasync.php: HA sync password
- services_captiveportal_vouchers.php: Voucher sync settings
- system_advanced_notifications.php: Growl password, SMTP password
- services_captiveportal.php: RADIUS shared secrets, RADIUS MAC auth password
- system_authservers.php: RADIUS shared secret, LDAP bind user credentials
- wizards/setup_wizard.xml: PPPoE/L2TP/PPTP WAN passwords
- services_pppoe_edit.php: PPPoE user password, RADIUS shared secrets
- interfaces_ppps_edit.php: PPPoE/L2TP/PPTP WAN passwords
- interfaces.php: PPPoE/L2TP/PPTP WAN passwords, 802.1x RADIUS shared secrets, DHCPv6 advanced keyinfo statement secret (maybe?)
- services_dyndns_edit.php: DynDNS service account password
- system_advanced_misc.php: Proxy password
- vpn_l2tp_users_edit.php: L2TP user password
- firewall_virtual_ip_edit.php: CARP VIP password
- vpn_openvpn_client.php: Proxy password, connection user password
- wizards/openvpn_wizard.xml: Wizard stores RADIUS/LDAP password/secret, might be good to just blank that when finishing the wizard
- services_dhcpv6.php: DNS domain key secret (maybe?)
- vpn_ipsec_keys.php: User PSK/Passwords (might be overkill to hide those)
Updated by
Updated by Jim Pingle over 8 years ago
- Assignee changed from Jim Pingle to Anonymous
Nudging this over toward Steve Beaver, he told me last week he had an idea to propose for a fix.
Updated by Anonymous over 8 years ago
If we take a simple example like system_advanced_notifications.php, one solution that would seem to fill the bill is this:
On displaying the password control, we always display some fixed text, such as:
define(DMYPWD, "********");
$section->addInput(new Form_Input(
'smtppassword',
'Email password',
'password',
DMYPWD,
['placeholder' => 'Redacted']
))->setHelp('Enter your password here');
Then in the $_POST handler, we just need:
if ($_POST[smtppassword'] != DMYPWD) {
$config['notifications']['smtp']['password'] = $_POST['smtppassword'];
}
We never display the password. If the user edits it, it is updated. If not it remains unchanged.
Not every page is this simple, and there are places we would need to retrieve the existing password from the config file so we can write it back, but it should be fairly easy to generalize.
Updated by Anonymous over 8 years ago
- Status changed from New to Feedback
- % Done changed from 0 to 100
Applied in changeset 5ea90990e6ebcdbeb78fadf8b88aedef66428c66.
Updated by Anonymous over 8 years ago
- Assignee changed from Anonymous to Jim Pingle
New addPassword() method and password validation added where appropriate.
I have not addressed the wizard XML pages.
Updated by Chris Buechler over 8 years ago
on services_dyndns_edit.php at least, and probably others, you can no longer edit entries without entering the password again. Otherwise fails with "The field Password is required." Which ideally is a check we want if it's a new entry, but not upon edit of existing.
Updated by Anonymous over 8 years ago
services_dyndns_edit.php corrected.
If $_POSTed password == DMYPWD, the old password must be retrieved and re-saved.
This may (or may not) be the case with certain other pages. Checking now.