Project

General

Profile

Actions
Copy link

Bug #5693

closed

Hide/Mask stored passwords when presenting GUI forms

Added by Jim Pingle almost 9 years ago. Updated almost 9 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Jim Pingle
Category:
Web Interface
Target version:
2.3
Start date:
12/24/2015
Due date:
% Done:

50%

Estimated time:
Plus Target Version:
Release Notes:
Affected Version:
2.3
Affected Architecture:

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)
Actions
Copy link
 #1

Updated by Jim Thompson almost 9 years ago

  • Assignee set to Jim Pingle
Actions
Copy link
 #2

Updated by Jim Pingle almost 9 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.

Actions
Copy link
 #3

Updated by Anonymous almost 9 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.

Actions
Copy link
 #4

Updated by Anonymous almost 9 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100
Actions
Copy link
 #5

Updated by Anonymous almost 9 years ago

  • % Done changed from 100 to 50
Actions
Copy link
 #6

Updated by Anonymous almost 9 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.

Actions
Copy link
 #7

Updated by Chris Buechler almost 9 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.

Actions
Copy link
 #8

Updated by Anonymous almost 9 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.

Actions
Copy link
 #9

Updated by Anonymous almost 9 years ago

  • Status changed from Feedback to Resolved
Actions
Copy link

Also available in: Atom PDF