with R-B/T-B and the following follow-up:
[PATCH qemu-server] gen_rand_chars: handle errors properly
should not really happen on modern systems, but random_bytes just
returns false if it fails to generate random bytes, in which case we
want to die instead of returning an empty 'random' string.
Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
---
PVE/API2/Qemu.pm | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index dcb364d..3965c26 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1597,8 +1597,12 @@ my $gen_rand_chars = sub {
die "invalid length $length" if $length < 1;
my $min = ord('!'); # first printable ascii
- my @rand_bytes = split '', Crypt::OpenSSL::Random::random_bytes($length);
- my $str = join('', map { chr((ord($_) & 0x3F) + $min) } @rand_bytes);
+
+ my $rand_bytes = Crypt::OpenSSL::Random::random_bytes($length);
+ die "failed to generate random bytes!\n"
+ if !$rand_bytes;
+
+ my $str = join('', map { chr((ord($_) & 0x3F) + $min) } split('',
$rand_bytes));
return $str;
};
On June 18, 2020 6:20 pm, Thomas Lamprecht wrote:
> We used the VNC API $ticket as password for VNC, but QEMU limits the
> password to the first 8 chars and ignores the rest[0].
> As our tickets start with a static string (e.g., "PVE") the entropy
> was a bit limited.
>
> For Proxmox VE this does not matters much as the noVNC viewer
> provided by has to go always over the API call, and so a valid
> ticket and correct permissions for the requested VM are enforced
> anyway.
>
> This patch helps external users, which often use NoVNC-Websockify,
> circumventing the API and relying solely on the VNC password to avoid
> snooping on VNC sessions.
>
> A 'generate-password' parameter is added, if set a password from good
> entropy (using libopenssl) is generated.
>
> For simplicity of mapping random bits to ranges we extract 6 bit of
> entropy per character and add the integer value of '!' (first
> printable ASCII char) to that. This way we get 64^8 possibilities,
> which even with millions of guesses per second one would need years
> of guessing and mostly just DDOS the server with websocket upgrade
> requests.
>
> Signed-off-by: Thomas Lamprecht <t.lampre...@proxmox.com>
> ---
>
> We could also extract the last 8 chars of the ticket, but as that ticket is
> only secure as a whole, as seen with this issue, I'd like to avoid that
>
> Initially I had a variant with using perls core rand and all 95 printable
> ascii
> chars, but that wasn't crypthographical secure
>
> PVE/API2/Qemu.pm | 39 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 3e0d59f..dcb364d 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -7,6 +7,7 @@ use Net::SSLeay;
> use POSIX;
> use IO::Socket::IP;
> use URI::Escape;
> +use Crypt::OpenSSL::Random;
>
> use PVE::Cluster qw (cfs_read_file cfs_write_file);;
> use PVE::RRD;
> @@ -1589,6 +1590,19 @@ __PACKAGE__->register_method({
> return undef;
> }});
>
> +# uses good entropy, each char is limited to 6 bit to get printable chars
> simply
> +my $gen_rand_chars = sub {
> + my ($length) = @_;
> +
> + die "invalid length $length" if $length < 1;
> +
> + my $min = ord('!'); # first printable ascii
> + my @rand_bytes = split '', Crypt::OpenSSL::Random::random_bytes($length);
> + my $str = join('', map { chr((ord($_) & 0x3F) + $min) } @rand_bytes);
> +
> + return $str;
> +};
> +
> my $sslcert;
>
> __PACKAGE__->register_method({
> @@ -1610,6 +1624,12 @@ __PACKAGE__->register_method({
> type => 'boolean',
> description => "starts websockify instead of vncproxy",
> },
> + 'generate-password' => {
> + optional => 1,
> + type => 'boolean',
> + default => 0,
> + description => "Generates a random password to be used as
> ticket instead of the API ticket.",
> + },
> },
> },
> returns => {
> @@ -1617,6 +1637,12 @@ __PACKAGE__->register_method({
> properties => {
> user => { type => 'string' },
> ticket => { type => 'string' },
> + password => {
> + optional => 1,
> + description => "Returned if requested with 'generate-password'
> param."
> + ." Consists of printable ASCII characters ('!' .. '~').",
> + type => 'string',
> + },
> cert => { type => 'string' },
> port => { type => 'integer' },
> upid => { type => 'string' },
> @@ -1644,6 +1670,10 @@ __PACKAGE__->register_method({
> my $authpath = "/vms/$vmid";
>
> my $ticket = PVE::AccessControl::assemble_vnc_ticket($authuser,
> $authpath);
> + my $password = $ticket;
> + if ($param->{'generate-password'}) {
> + $password = $gen_rand_chars->(8);
> + }
>
> $sslcert = PVE::Tools::file_get_contents("/etc/pve/pve-root-ca.pem",
> 8192)
> if !$sslcert;
> @@ -1680,7 +1710,7 @@ __PACKAGE__->register_method({
> '-perm', 'Sys.Console'];
>
> if ($param->{websocket}) {
> - $ENV{PVE_VNC_TICKET} = $ticket; # pass ticket to vncterm
> + $ENV{PVE_VNC_TICKET} = $password; # pass ticket to vncterm
> push @$cmd, '-notls', '-listen', 'localhost';
> }
>
> @@ -1690,7 +1720,7 @@ __PACKAGE__->register_method({
>
> } else {
>
> - $ENV{LC_PVE_TICKET} = $ticket if $websocket; # set ticket with
> "qm vncproxy"
> + $ENV{LC_PVE_TICKET} = $password if $websocket; # set ticket
> with "qm vncproxy"
>
> $cmd = [@$remcmd, "/usr/sbin/qm", 'vncproxy', $vmid];
>
> @@ -1725,13 +1755,16 @@ __PACKAGE__->register_method({
>
> PVE::Tools::wait_for_vnc_port($port);
>
> - return {
> + my $res = {
> user => $authuser,
> ticket => $ticket,
> port => $port,
> upid => $upid,
> cert => $sslcert,
> };
> + $res->{password} = $password if $param->{'generate-password'};
> +
> + return $res;
> }});
>
> __PACKAGE__->register_method({
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
