[pve-devel] Allow external vnc access via vncwebsocket

Dominik Csapak d.csapak at proxmox.com
Fri Feb 2 11:50:44 CET 2018


Hi,

a few things to this patch

this is not a proper git patch, so please see 
https://pve.proxmox.com/wiki/Developer_Documentation for how to send 
proper patches
(also see the notes about indentation etc.)

the commit message/subject is also not very meaningful, better would be 
something like:
'do not require ticket on vncwebsocket call'

with a message why this is good and necessary or which bug this fixes

--- (end of formal review, start of review of the content)

i get what you want to do, but would it not be possible to create
a vnc user and use that cookie on your client?

the way you did it can never work if the vncproxy/termproxy/etc. call is 
not made with the vnc at pve user anyway (and even then it will not work 
with the host console)

also, hardcoding a user which might already exist in an existing 
installation is not good

further comment inline

On 02/02/2018 09:45 AM, Entwickler (centron GmbH) wrote:
> This allows Access to novnc from an external Webinterface to user vnc.
> 
> 
> --- /usr/share/perl5/PVE/HTTPServer.pm          2018-01-17 09:18:26.000000000 +0100
> @@ -76,11 +76,16 @@
>       if ($require_auth) {
> -              die "No ticket\n" if !$ticket;
> -
> -              ($username, $age) = PVE::AccessControl::verify_ticket($ticket);
> -
> -              $rpcenv->set_user($username);
> +             if ($rel_uri =~ /vncwebsocket/ && $method eq 'GET' && !$ticket)

the regex is wrong, e.g. if a node is named 'vncwebsocket' (i know it is 
unlikely, but still) all GET requests there will work unauthenticated

> +             {
> +                             $rpcenv->set_user("vnc\@pve");
> +                             $username = "vnc\@pve";

a perl tip: if you use single quotes, you do not need to escape @/$/etc.

var $name = 'name at realm';

works

> +                             $age = 60;
> +             } else {
> +                             die "No ticket\n" if !$ticket;
> +                             ($username, $age) = PVE::AccessControl::verify_ticket($ticket);
> +                             $rpcenv->set_user($username);
> +             }
>                  if ($method eq 'POST' && $rel_uri =~ m|^/nodes/([^/]+)/storage/([^/]+)/upload$|) {
>                     my ($node, $storeid) = ($1, $2);
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 





More information about the pve-devel mailing list