Project

General

Profile

Todo #9511

OpenVPN server/client/override advanced settings privilege separation

Added by Jim Pingle 5 months ago. Updated 5 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
OpenVPN
Target version:
Start date:
05/09/2019
Due date:
% Done:

100%

Estimated time:

Description

This issue needs some additional thought and debate.

Due to advanced directives in OpenVPN it is possible for users to gain elevated privileges since they can run commands or scripts (#9510). Many users rely on these commands to make OpenVPN behave as they want, so we can't just filter them out.

We could create a new privilege to (dis)allow editing the advanced settings on clients, servers, and overrides. Though splitting the advanced editing privilege might be OK, it poses its own set of tricky questions:

  • If a user without privileges to edit the box edits an entry with advanced settings, are they preserved or cleared? (preserved seems best)
  • If a user with access to delete OpenVPN instances doesn't also have access to edit the advanced settings, should they be allowed to delete an instance with an advanced configuration set? If so, they could remove advanced settings by deleting the instances and creating a new one without advanced settings. While not technically a path to elevated privileges, it could lead to unintended network access or behavior if they were able to remove custom routing commands.
  • Do we add that new privilege to all users with access to edit those pages on upgrade, or do leave it up to the user to discover that they can no longer edit the advanced section? Either way is bound to cause lots of support questions. Using a negative form such as "deny advanced edit access" is bound to be even more trouble, given the issues users have understanding "Deny config write".

A new privilege would look like this, in src/etc/inc/priv/user.priv.inc

$priv_list['page-openvpn-server-advanced'] = array();
$priv_list['page-openvpn-server-advanced']['name'] = gettext("WebCfg - OpenVPN: Edit Clients Advanced");
$priv_list['page-openvpn-server-advanced']['descr'] = gettext("Allow edit access to the 'OpenVPN: Clients' Advanced settings field.");
$priv_list['page-openvpn-server-advanced']['warn'] = "standard-warning-root";

$priv_list['page-openvpn-csc-advanced'] = array();
$priv_list['page-openvpn-csc-advanced']['name'] = gettext("WebCfg - OpenVPN: Client Specific Override Advanced");
$priv_list['page-openvpn-csc-advanced']['descr'] = gettext("Allow edit access to the 'OpenVPN: Client Specific Override' advanced settings field.");
$priv_list['page-openvpn-csc-advanced']['warn'] = "standard-warning-root";

$priv_list['page-openvpn-client-advanced'] = array();
$priv_list['page-openvpn-client-advanced']['name'] = gettext("WebCfg - OpenVPN: Edit Servers Advanced");
$priv_list['page-openvpn-client-advanced']['descr'] = gettext("Allow edit access to the 'OpenVPN: Servers' Advanced settings field.");
$priv_list['page-openvpn-client-advanced']['warn'] = "standard-warning-root";

And then on vpn_openvpn_client.php, vpn_openvpn_csc.php, and vpn_openvpn_server.php the code would need to test for this privilege before allowing changes to Advanced settings.

I'd like to see this in 2.5.0 but if we can't reach a viable consensus then it can be nudged forward to the next release.

Associated revisions

Revision b8ca6554 (diff)
Added by Jim Pingle 5 months ago

Remove Advanced box from OpenVPN Wizard. Issue #9511

Revision 4a1841a1 (diff)
Added by Jim Pingle 5 months ago

Implement new OpenVPN advanced options privilege. Fixes #9511

Revision 0dd99de7 (diff)
Added by Jim Pingle 5 months ago

Remove Advanced box from OpenVPN Wizard. Issue #9511

(cherry picked from commit b8ca6554d022e99921835a2fdb35103f41a7302e)

Revision 92d5396f (diff)
Added by Jim Pingle 5 months ago

Implement new OpenVPN advanced options privilege. Fixes #9511

(cherry picked from commit 4a1841a1fabcba0100f6a4f505fc1e132c29da20)

History

#1 Updated by Jim Pingle 5 months ago

If or when this is implemented, the warnings added for #9510 can be removed.

#2 Updated by Jim Pingle 5 months ago

  • Parent task changed from #9398 to #9515

#3 Updated by Jim Pingle 5 months ago

  • Target version changed from 2.5.0 to 2.4.4-p3

#4 Updated by Jim Pingle 5 months ago

  • Removed Advanced options from the OpenVPN wizard. If a user has privileges for it, they can add the settings later.
  • Added new privileges for OpenVPN servers, clients, and overrides which allows for editing advanced options
  • These new privileges are NOT added to users automatically -- erring on the side of security
  • Admin users (the admin user or users with page-all) and users with the new privilege(s) can always alter advanced options
  • Added input validation to warn the user they can't change advanced options if they try, which means:
    • A user without the new privilege can edit existing entries so long as they do not alter the advanced options. The form field is disabled, and if the user submits a form with the value changed, input validation rejects the change
    • A user without the new privilege may create new entries so long as they do not contain advanced options
    • A user without the new privilege may NOT delete an instance if it contains advanced options which they are not allowed to change

#6 Updated by Jim Pingle 5 months ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100

#7 Updated by Jim Pingle 5 months ago

  • Status changed from Feedback to Resolved
  • Private changed from Yes to No

Also available in: Atom PDF