[pve-devel] [PATCH manager] fix #3389: Skip CSRF token check also for API tokens on file upload

[Lorenz Stechauner]

On file upload, the check for CSRF tokens was already skipped
when performing user authentication.This now happens for API
tokens also.

Signed-off-by: Lorenz Stechauner <l.stechau...@proxmox.com>
---
 PVE/HTTPServer.pm | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm
index 64976c7c..63b8583e 100755
--- a/PVE/HTTPServer.pm
+++ b/PVE/HTTPServer.pm
@@ -79,8 +79,8 @@ sub auth_handler {
 
     if ($require_auth) {
        if ($api_token) {
+           # returns tokenid actually
            $username = PVE::AccessControl::verify_token($api_token);
-           $rpcenv->set_user($username); #actually tokenid in this case
        } else {
            die "No ticket\n" if !$ticket;
 
@@ -94,25 +94,25 @@ sub auth_handler {
                die "No ticket\n"
                    if ($rel_uri ne '/access/tfa' || $method ne 'POST');
            }
+       }
 
-           $rpcenv->set_user($username);
-
-           if ($method eq 'POST' && $rel_uri =~ 
m|^/nodes/([^/]+)/storage/([^/]+)/upload$|) {
-               my ($node, $storeid) = ($1, $2);
-               # we disable CSRF checks if $isUpload is set,
-               # to improve security we check user upload permission here
-               my $perm = { check => ['perm', "/storage/$storeid", 
['Datastore.AllocateTemplate']] };
-               $rpcenv->check_api2_permissions($perm, $username, {});
-               $isUpload = 1;
-           }
+       $rpcenv->set_user($username);
 
-           # we skip CSRF check for file upload, because it is
-           # difficult to pass CSRF HTTP headers with native html forms,
-           # and it should not be necessary at all.
-           my $euid = $>;
-           PVE::AccessControl::verify_csrf_prevention_token($username, $token)
-               if !$isUpload && ($euid != 0) && ($method ne 'GET');
+       if ($method eq 'POST' && $rel_uri =~ 
m|^/nodes/([^/]+)/storage/([^/]+)/upload$|) {
+           my ($node, $storeid) = ($1, $2);
+           # we disable CSRF checks if $isUpload is set,
+           # to improve security we check user upload permission here
+           my $perm = { check => ['perm', "/storage/$storeid", 
['Datastore.AllocateTemplate']] };
+           $rpcenv->check_api2_permissions($perm, $username, {});
+           $isUpload = 1;
        }
+
+       # we skip CSRF check for file upload, because it is
+       # difficult to pass CSRF HTTP headers with native html forms,
+       # and it should not be necessary at all.
+       my $euid = $>;
+       PVE::AccessControl::verify_csrf_prevention_token($username, $token)
+           if !$isUpload && ($euid != 0) && ($method ne 'GET');
     }
 
     return {
-- 
2.20.1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel