pfSense » pfSense Packages

When a second user database is configured for the VPN, for example ldap, then the client export utility will no longer list the normal users. This is due to the 'authmode' becoming a comma-separated list of values, and it will not exactly match 'Local Database' anymore. Changing the strict comparison to a str_contains fixes this issue.

Furthermore, if a user is given access to the openVPN client export page, they can impersonate any other user on the VPN by downloading their certificate if certificate login only is used. If privileged data is present in a client config (for example, via a client override) then this too can be accessed. This is unexpected functionality.

Besides cluttering up the page, this could be a security issue if no password is used to authenticate to the VPN.

The diff below will make the following changes to client-export:

1. Forbid access to non 'edit-advanced' users for user certificates and client configs other than their own.
2. Forbid access to non 'edit-advanced' users to certificates and client configs from external databases.
3. Fix the bug that prevents vpn export from working when the authmode contains more than one value.

diff --git a/vpn_openvpn_export_old.php b/vpn_openvpn_export.php
index 541ad3e..9cd578e 100644
--- a/vpn_openvpn_export_old.php
+++ b/vpn_openvpn_export.php
@@ -58,6 +58,7 @@ if (!is_array($config['cert'])) {
 $a_cert = $config['cert'];

 $ras_server = array();
+$user_can_edit_advanced = (isAdminUID($_SESSION['Username']) || userHasPrivilege($user_entry, "page-openvpn-server-advanced") || userHasPrivilege($user_entry, "page-all"));
 foreach ($a_server as $server) {
     if (isset($server['disable'])) {
         continue;
@@ -78,9 +79,11 @@ foreach ($a_server as $server) {
             $ecdsagood[] = $cert['refid'];
         }
     }
-
-    if (($server['mode'] == "server_tls_user") && ($server['authmode'] == "Local Database")) {
+    if (($server['mode'] == "server_tls_user") && str_contains($server['authmode'],"Local Database")) {
         foreach ($a_user as $uindex => $user) {
+                        if(!$user_can_edit_advanced && $user['name'] != $_SESSION['Username']) {
+                                continue;
+                        }
             if (!is_array($user['cert'])) {
                 continue;
             }
@@ -773,6 +776,7 @@ servers[<?=$sindex?>][1][<?=$c?>][3] = '<?=str_replace("'", "\\'", $user['certna
         endif;
     endforeach;
     $c=0;
+    if($user_can_edit_advanced): 
     foreach ($server['certs'] as $cert): ?>
 <?php
         if (!$server['crlref'] || !is_cert_revoked($config['cert'][$cert['cindex']], $server['crlref'])): ?>
@@ -783,6 +787,7 @@ servers[<?=$sindex?>][3][<?=$c?>][1] = '<?=str_replace("'", "\\'", $cert['certna
             $c++;
         endif;
     endforeach;
+    endif;
 endforeach;
 ?>