Re: [pve-devel] [PATCH qemu-server v3 11/11] let VirtIO RNG devices source entropy from mapped HWRNGs

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On February 10, 2025 4:37 pm, Filip Schauer wrote:
> This allows a user with the Mapping.Modify privilege on /mapping/hwrng
> to configure a hardware RNG mapping. A less privileged user with the
> Mapping.Use privilege can then pass the mapped hardware RNG device as an
> entropy source to a VirtIO RNG device.
> 
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>  PVE/API2/Qemu.pm      |  5 +++++
>  PVE/QemuServer.pm     |  5 +++++
>  PVE/QemuServer/RNG.pm | 25 +++++++++++++++++++++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 194e6357..33b3625b 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -812,9 +812,14 @@ my sub check_rng_perm {
>  
>      my $device = PVE::JSONSchema::parse_property_string('pve-qm-rng', $value);
>      if ($device->{source}) {
> +	# Backward compatibility for non-mapped /dev/hwrng
>  	if ($device->{source} eq '/dev/hwrng') {
>  	    die "only root can set '$opt' config for a non-mapped Hardware RNG device\n";
>  	}
> +    } elsif ($device->{mapping}) {
> +	$rpcenv->check_full($authuser, "/mapping/hwrng/$device->{mapping}", ['Mapping.Use']);
> +    } else {
> +	die "either 'source' or 'mapping' must be set.\n";
>      }
>  
>      return 1;
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index cc69eeb1..82a6c65d 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6402,10 +6402,15 @@ sub check_mapping_access {
>  	    my $device = PVE::JSONSchema::parse_property_string('pve-qm-rng', $conf->{$opt});
>  
>  	    if ($device->{source}) {
> +		# Backward compatibility for non-mapped /dev/hwrng
>  		if ($device->{source} eq '/dev/hwrng') {
>  		    die "only root can set '$opt' config for a non-mapped Hardware RNG device\n"
>  			if $user ne 'root@pam';
>  		}
> +	    } elsif ($device->{mapping}) {
> +		$rpcenv->check_full($user, "/mapping/hwrng/$device->{mapping}", ['Mapping.Use']);
> +	    } else {
> +		die "either 'source' or 'mapping' must be set.\n";

this is handled here, but if both are set then the parser will silently
drop them (see below).. seems a bit inconsistent, for other mappings the
parser enforces this already as well..

>  	    }
>  	}
>      }
> diff --git a/PVE/QemuServer/RNG.pm b/PVE/QemuServer/RNG.pm
> index ae5b2530..3ee19852 100644
> --- a/PVE/QemuServer/RNG.pm
> +++ b/PVE/QemuServer/RNG.pm
> @@ -4,6 +4,7 @@ use strict;
>  use warnings;
>  
>  use PVE::JSONSchema;
> +use PVE::Mapping::HWRNG;
>  use PVE::Tools qw(file_read_firstline);
>  
>  use PVE::QemuServer::PCI qw(print_pci_addr);
> @@ -22,11 +23,20 @@ my $rng_fmt = {
>  	type => 'string',
>  	enum => ['/dev/urandom', '/dev/random', '/dev/hwrng'],
>  	default_key => 1,
> +	optional => 1,
>  	description => "The file on the host to gather entropy from. Using urandom does *not*"
>  	    ." decrease security in any meaningful way, as it's still seeded from real entropy, and"
>  	    ." the bytes provided will most likely be mixed with real entropy on the guest as well."
>  	    ." '/dev/hwrng' can be used to pass through a hardware RNG from the host.",
>      },
> +    mapping => {
> +	optional => 1,
> +	type => 'string',
> +	format_description => 'mapping-id',
> +	format => 'pve-configid',
> +	description => "The ID of a cluster wide mapping. When specified, entropy is gathered from"
> +	    ." a hardware RNG on the host. Either this or the default-key 'source' must be set.",
> +    },
>      max_bytes => {
>  	type => 'integer',
>  	description => "Maximum bytes of entropy allowed to get injected into the guest every"
> @@ -65,6 +75,11 @@ sub parse_rng {
>      my $res = eval { PVE::JSONSchema::parse_property_string($rng_fmt, $value) };
>      warn $@ if $@;
>  
> +    my $source = $res->{source};
> +    my $mapping = $res->{mapping};
> +
> +    return if $source && $mapping; # not a valid configuration
> +
>      return $res;
>  }
>  
> @@ -89,9 +104,19 @@ sub get_rng_source_path {
>      my ($rng) = @_;
>  
>      my $source = $rng->{source};
> +    my $mapping = $rng->{mapping};
> +
> +    return if $source && $mapping; # not a valid configuration

this cannot really happen, since the parser already dropped this
combination?

>  
>      if (defined($source)) {
>  	return $source;
> +    } elsif (defined($mapping)) {
> +	my $devices = PVE::Mapping::HWRNG::find_on_current_node($mapping);
> +	die "Hardware RNG mapping not found for '$mapping'\n" if !$devices || !scalar($devices->@*);
> +	die "More than one Hardware RNG mapping per host not supported\n"
> +	    if scalar($devices->@*) > 1;
> +
> +	return $devices->[0]->{path};

should we maybe simplify this - and just defined a single static mapping
for now for /dev/hwrng, so that we have an ACL path to refer to?

>      }
>  
>      return;
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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