public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH cluster/guest-common/manager/qemu-server v3 00/11] fix #5657: allow configuring RNG device as non-root user
Date: Tue, 11 Feb 2025 13:34:53 +0100	[thread overview]
Message-ID: <1739276885.8xye6t1gw6.astroid@yuna.none> (raw)
In-Reply-To: <20250210153734.103381-1-f.schauer@proxmox.com>

On February 10, 2025 4:37 pm, Filip Schauer wrote:
> Allow users with the VM.Config.HWType privilege to configure VirtIO RNG
> devices on VMs with either /dev/urandom or /dev/random as the entropy
> source.
> 
> Further introduce hardware RNG device mapping to be able to selectively
> allow non-root users with the Mapping.Use privilege to configure
> hardware RNG devices as entropy sources.

some high level questions here:

- this series allows direct access to /dev/urandom and /dev/random, but
  also allows setting up mappings to them (the mapping seems unnecessary)
- the only other (restricted) value is /dev/hwrng

wouldn't it be easier to just define an ACL path for /dev/hwrng, and
skip all the mapping setup if the only sensible mapping you can set up
is a single one for /dev/hwrng?

do we expect other hardware RNG device paths in the future?

else the only benefit of the full-fledged mapping is that you can
"alias" and describe RNG sources and limit them to certain nodes, and I
am not sure that is worth all this machinery and an extra config file ;)
aliasing makes very little sense if there is only three valid choices
that have descriptive names anyway. I also expect that most people using
this would want to give the VM access to the hwrng on all nodes..

other than this the series looks good to me, just a few nits (see
individual patches)

> 
> Changes since v2:
> * Restrict RNG device format to enum of
> * Add descriptive commit message
> * Code style fixes
> * Remove outdated remarks about entropy stravation of /dev/random
> * Split helpers for VirtIO RNG command line arguments into its own
>   commit
> * Add explicit "use PVE::QemuServer::RNG;" statement to PVE/API2/Qemu.pm
> * Fix "map: type check ('array') failed" error when adding a mapping in
>   the UI
> * ui: split resource mapping types into tabbed views
> 
> Changes since v1:
> * Restrict use of /dev/hwrng to the root user
> * introduce hardware RNG mapping
> 
> pve-guest-common:
> 
> Filip Schauer (1):
>   mapping: add a hardware RNG mapping config
> 
>  src/Makefile             |   1 +
>  src/PVE/Mapping/HWRNG.pm | 147 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 148 insertions(+)
>  create mode 100644 src/PVE/Mapping/HWRNG.pm
> 
> 
> pve-cluster:
> 
> Filip Schauer (1):
>   cfs: add 'mapping/hwrng.cfg' to observed files
> 
>  src/PVE/Cluster.pm  | 1 +
>  src/pmxcfs/status.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> 
> pve-manager:
> 
> Filip Schauer (5):
>   introduce hardware rng mapping api
>   introduce hardware rng scanning api
>   ui: add hardware RNG resource mapping
>   ui: allow use of mapped hardware RNGs as entropy sources for VMs
>   ui: split resource mapping types into tabbed views
> 
>  PVE/API2/Cluster/Mapping.pm                   |   7 +
>  PVE/API2/Cluster/Mapping/HWRNG.pm             | 286 ++++++++++++++++++
>  PVE/API2/Cluster/Mapping/Makefile             |   5 +-
>  PVE/API2/Hardware.pm                          |   7 +
>  PVE/API2/Hardware/HWRNG.pm                    |  47 +++
>  PVE/API2/Hardware/Makefile                    |   1 +
>  www/manager6/Makefile                         |  12 +-
>  www/manager6/data/PermPathStore.js            |   1 +
>  www/manager6/dc/Config.js                     |  41 +--
>  www/manager6/form/HWRNGMapSelector.js         |  99 ++++++
>  www/manager6/qemu/HardwareView.js             |   9 +-
>  www/manager6/qemu/RNGEdit.js                  |  79 +++--
>  www/manager6/resource-map/HWRNGMapEdit.js     | 149 +++++++++
>  www/manager6/resource-map/HWRNGMapView.js     |  76 +++++
>  .../{window => resource-map}/PCIMapEdit.js    |   2 +-
>  .../{dc => resource-map}/PCIMapView.js        |   4 +-
>  www/manager6/resource-map/ResourceMapView.js  |  23 ++
>  .../{window => resource-map}/USBMapEdit.js    |   2 +-
>  .../{dc => resource-map}/USBMapView.js        |   4 +-
>  19 files changed, 778 insertions(+), 76 deletions(-)
>  create mode 100644 PVE/API2/Cluster/Mapping/HWRNG.pm
>  create mode 100644 PVE/API2/Hardware/HWRNG.pm
>  create mode 100644 www/manager6/form/HWRNGMapSelector.js
>  create mode 100644 www/manager6/resource-map/HWRNGMapEdit.js
>  create mode 100644 www/manager6/resource-map/HWRNGMapView.js
>  rename www/manager6/{window => resource-map}/PCIMapEdit.js (99%)
>  rename www/manager6/{dc => resource-map}/PCIMapView.js (96%)
>  create mode 100644 www/manager6/resource-map/ResourceMapView.js
>  rename www/manager6/{window => resource-map}/USBMapEdit.js (99%)
>  rename www/manager6/{dc => resource-map}/USBMapView.js (95%)
> 
> 
> qemu-server:
> 
> Filip Schauer (4):
>   refactor: move rng related code into its own module
>   add helpers for VirtIO RNG command line arguments
>   allow non-root users to set /dev/u?random as an RNG source
>   let VirtIO RNG devices source entropy from mapped HWRNGs
> 
>  PVE/API2/Qemu.pm        |  48 +++++++++++++
>  PVE/QemuServer.pm       |  97 ++++++-------------------
>  PVE/QemuServer/Makefile |   1 +
>  PVE/QemuServer/RNG.pm   | 153 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 224 insertions(+), 75 deletions(-)
>  create mode 100644 PVE/QemuServer/RNG.pm
> 
> 
> Summary over all repositories:
>   27 files changed, 1152 insertions(+), 151 deletions(-)
> 
> -- 
> Generated by git-murpp 0.6.0
> 
> 
> _______________________________________________
> 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


  parent reply	other threads:[~2025-02-11 12:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-10 15:37 Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH guest-common v3 01/11] mapping: add a hardware RNG mapping config Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH cluster v3 02/11] cfs: add 'mapping/hwrng.cfg' to observed files Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH manager v3 03/11] introduce hardware rng mapping api Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH manager v3 04/11] introduce hardware rng scanning api Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH manager v3 05/11] ui: add hardware RNG resource mapping Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH manager v3 06/11] ui: allow use of mapped hardware RNGs as entropy sources for VMs Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH manager v3 07/11] ui: split resource mapping types into tabbed views Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH qemu-server v3 08/11] refactor: move rng related code into its own module Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH qemu-server v3 09/11] add helpers for VirtIO RNG command line arguments Filip Schauer
2025-02-10 15:37 ` [pve-devel] [PATCH qemu-server v3 10/11] allow non-root users to set /dev/u?random as an RNG source Filip Schauer
2025-02-11 12:34   ` Fabian Grünbichler
2025-02-10 15:37 ` [pve-devel] [PATCH qemu-server v3 11/11] let VirtIO RNG devices source entropy from mapped HWRNGs Filip Schauer
2025-02-11 12:34   ` Fabian Grünbichler
2025-02-11 12:34 ` Fabian Grünbichler [this message]
2025-02-18 11:17   ` [pve-devel] [PATCH cluster/guest-common/manager/qemu-server v3 00/11] fix #5657: allow configuring RNG device as non-root user Filip Schauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1739276885.8xye6t1gw6.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal