[pve-devel] [PATCH openid 0/1] Make OIDC userinfo endpoint optional

2024-09-02 Thread Thomas Skinner
In the OpenID Connect documentation 
(https://openid.net/specs/openid-connect-core-1_0.html), the
protocol abstract defined in 1.3 states in step 4 that "The RP can send a 
request with the Access 
Token to the UserInfo Endpoint", which would imply that getting information 
from the UserInfo
endpoint is not a requirement for the protocol. Some OpenID Providers (e.g. 
ADFS) do not support
retrieving any additional claims in the UserInfo endpoint.

This patch changes the userinfo claims to be optional instead of required. If 
the claims can be
retrieved successfully from the userinfo endpoint, they are returned as 
retrieved. If the claims
cannot be retrieved successfully, the claims are returned as None.

While this patch does not explicitly add an option as requested in bug #4234, 
it does fix issue of
the userinfo endpoint not providing claims properly.

It would be nice to have some log output when claims cannot be retrieved for 
troubleshooting
purposes, but I'm not sure how the PVE team would prefer that be handled.

Thomas Skinner (1):
  fix #4234: openid: make userinfo request optional

 proxmox-openid/src/lib.rs | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.39.2


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



[pve-devel] [PATCH openid 1/1] fix #4234: openid: make userinfo request optional

2024-09-02 Thread Thomas Skinner
Signed-off-by: Thomas Skinner 
---
 proxmox-openid/src/lib.rs | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
index fe65fded..7cef06e0 100644
--- a/proxmox-openid/src/lib.rs
+++ b/proxmox-openid/src/lib.rs
@@ -195,7 +195,7 @@ impl OpenIdAuthenticator {
 &self,
 code: &str,
 private_auth_state: &PrivateAuthState,
-) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
+) -> Result<(CoreIdTokenClaims, Option), Error> {
 let code = AuthorizationCode::new(code.to_string());
 // Exchange the code with a token.
 let token_response = self
@@ -213,11 +213,14 @@ impl OpenIdAuthenticator {
 .claims(&id_token_verifier, &private_auth_state.nonce)
 .map_err(|err| format_err!("Failed to verify ID token: {}", err))?;
 
-let userinfo_claims: GenericUserInfoClaims = self
+let userinfo_claims: Option = match self
 .client
 .user_info(token_response.access_token().to_owned(), None)?
 .request(http_client)
-.map_err(|err| format_err!("Failed to contact userinfo endpoint: 
{}", err))?;
+{
+Ok(claims) => Some(claims),
+Err(..) => None,
+};
 
 Ok((id_token_claims.clone(), userinfo_claims))
 }
-- 
2.39.2


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



[pve-devel] [PATCH SERIES openid/access-control/docs/manager] fix #4411: add support for openid groups

2024-09-02 Thread Thomas Skinner
This patch series adds support for groups for OpenID logins. 

The following options are implemented:
  - Configurable claim for retrieving a list of groups and adding them to the 
user in PVE
  - Allowing "synchronization" of groups on login by overriding groups assigned
to the user in PVE (this option is off by default)
  - Replacing invalid characters in group names with a configurable valid 
characters
(by default, this is an underscore '_')

The logic implemented by this patch expects the groups claim in the ID 
token/userinfo
endpoint to send a list of groups.

This patch also adds support for using additional claims from the OpenID 
provider
by exposing all additional claims and their values from the ID token (previously
only available for the userinfo endpoint). This is necessary for OpenID 
providers
that do not support additional information in the userinfo endpoint.


proxmox/proxmox-openid:

Thomas Skinner (1):
  fix #4411: openid: add library code for openid groups support

 proxmox-openid/src/lib.rs | 55 +--
 1 file changed, 47 insertions(+), 8 deletions(-)


pve-access-control:

Thomas Skinner (1):
  fix #4411: openid: add logic for openid groups support

 src/PVE/API2/OpenId.pm | 32 
 src/PVE/Auth/OpenId.pm | 21 +
 2 files changed, 53 insertions(+)


pve-docs:

Thomas Skinner (1):
  fix #4411: openid: add docs for openid groups support

 api-viewer/apidata.js | 40 
 pveum.adoc| 32 
 2 files changed, 72 insertions(+)


pve-manager:

Thomas Skinner (1):
  fix #4411: openid: add ui config for openid groups support

 www/manager6/dc/AuthEditOpenId.js | 35 ---
 1 file changed, 32 insertions(+), 3 deletions(-)

-- 
2.39.2


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



[pve-devel] [PATCH docs 1/1] fix #4411: openid: add docs for openid groups support

2024-09-02 Thread Thomas Skinner
Signed-off-by: Thomas Skinner 
---
 api-viewer/apidata.js | 40 
 pveum.adoc| 32 
 2 files changed, 72 insertions(+)

diff --git a/api-viewer/apidata.js b/api-viewer/apidata.js
index 8ba94e4..0edafd7 100644
--- a/api-viewer/apidata.js
+++ b/api-viewer/apidata.js
@@ -53895,6 +53895,26 @@ const apiSchema = [
  "type" : "string",
  "typetext" : ""
   },
+  "groups-claim" : {
+ "description" : "OpenID claim used to 
retrieve groups with.",
+ "optional" : 1,
+ "type" : "string",
+ "typetext" : ""
+  },
+  "groups-overwrite" : {
+ "default" : 0,
+ "description" : "All groups will be 
overwritten for the user on login.",
+ "optional" : 1,
+ "type" : "boolean",
+ "typetext" : ""
+  },
+  "groups-replace-character" : {
+ "default" : "_",
+ "description" : "Character used to replace 
any invalid characters in groups from provider.",
+ "optional" : 1,
+ "pattern" : "^[A-Za-z0-9\\.\\-_]$",
+ "type" : "string"
+  },
   "issuer-url" : {
  "description" : "OpenID Issuer Url",
  "maxLength" : 256,
@@ -54233,6 +54253,26 @@ const apiSchema = [
"type" : "string",
"typetext" : ""
 },
+"groups-claim" : {
+   "description" : "OpenID claim used to retrieve 
groups with.",
+   "optional" : 1,
+   "type" : "string",
+   "typetext" : ""
+},
+"groups-overwrite" : {
+   "default" : 0,
+   "description" : "All groups will be overwritten for 
the user on login.",
+   "optional" : 1,
+   "type" : "boolean",
+   "typetext" : ""
+},
+"groups-replace-character" : {
+   "default" : "_",
+   "description" : "Character used to replace any 
invalid characters in groups from provider.",
+   "optional" : 1,
+   "pattern" : "^[A-Za-z0-9\\.\\-_]$",
+   "type" : "string"
+},
 "issuer-url" : {
"description" : "OpenID Issuer Url",
"maxLength" : 256,
diff --git a/pveum.adoc b/pveum.adoc
index c115866..0a031cc 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -456,6 +456,16 @@ use the `autocreate` option to automatically add new users.
 * `Username Claim` (`username-claim`): OpenID claim used to generate the unique
 username (`subject`, `username` or `email`).
 
+* `Groups Claim` (`groups-claim`): OpenID claim used to retrieve the groups 
from
+the ID token or userinfo endpoint.
+
+* `Overwrite Groups` (`groups-overwrite`): Overwrite all groups assigned to 
user
+instead of appending to existing groups (default behavior).
+
+* `Groups Replacement Character` (`groups-replace-character`): Character to
+replace invalid characters in groups names from the OpenID provider. Default
+behavior is to replace each invalid character with an underscore (`'_'`).
+
 Username mapping
 
 
@@ -479,6 +489,28 @@ Another option is to use `email`, which also yields human 
readable
 usernames. Again, only use this setting if the server guarantees the
 uniqueness of this attribute.
 
+Groups mapping
+^^
+
+Specifying the `groups-claim` setting in the OpenID configuration enables group
+mapping functionality. The data provided in the `groups-claim` should be
+a list of strings that correspond to groups that a user should be a member of 
in
+{pve}. Any groups reported by the OpenID provider that do not exist in {pve} 
are
+ignored.
+
+By default, groups are appended to the user's existing groups. It may be 
+desirable to overwrite any groups that the user is already a member in {pve}
+with those from the OpenID provider. Enabling the `groups-overwrite` setting
+removes all groups from the user in {pve} before adding the groups reported by
+the OpenID provider.
+

[pve-devel] [PATCH manager 1/1] fix #4411: openid: add ui config for openid groups support

2024-09-02 Thread Thomas Skinner
Signed-off-by: Thomas Skinner 
---
 www/manager6/dc/AuthEditOpenId.js | 35 ---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/www/manager6/dc/AuthEditOpenId.js 
b/www/manager6/dc/AuthEditOpenId.js
index 544c0de5..30ee050a 100644
--- a/www/manager6/dc/AuthEditOpenId.js
+++ b/www/manager6/dc/AuthEditOpenId.js
@@ -40,6 +40,16 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
},
name: 'client-key',
},
+   {
+   xtype: 'proxmoxtextfield',
+   name: 'scopes',
+   fieldLabel: gettext('Scopes'),
+   emptyText: `${Proxmox.Utils.defaultText} (email profile)`,
+   submitEmpty: false,
+   cbind: {
+   deleteEmpty: '{!isCreate}',
+   },
+   },
 ],
 
 column2: [
@@ -74,14 +84,23 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
},
{
xtype: 'proxmoxtextfield',
-   name: 'scopes',
-   fieldLabel: gettext('Scopes'),
-   emptyText: `${Proxmox.Utils.defaultText} (email profile)`,
+   name: 'groups-claim',
+   fieldLabel: gettext('Groups Claim'),
+   emptyText: `${Proxmox.Utils.defaultText} ${gettext('(none)')}`,
submitEmpty: false,
cbind: {
deleteEmpty: '{!isCreate}',
},
},
+   {
+   xtype: 'proxmoxcheckbox',
+   fieldLabel: gettext('Overwrite Groups'),
+   name: 'groups-overwrite',
+   value: 0,
+   cbind: {
+   deleteEmpty: '{!isCreate}',
+   },
+   },
{
xtype: 'proxmoxKVComboBox',
name: 'prompt',
@@ -111,6 +130,16 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
deleteEmpty: '{!isCreate}',
},
},
+   {
+   xtype: 'proxmoxtextfield',
+   name: 'groups-replace-character',
+   fieldLabel: gettext('Groups Replacement Character'),
+   emptyText: `${Proxmox.Utils.defaultText} '_'`,
+   submitEmpty: false,
+   cbind: {
+   deleteEmpty: '{!isCreate}',
+   },
+   },
 ],
 
 initComponent: function() {
-- 
2.39.2


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



[pve-devel] [PATCH openid 1/1] fix #4411: openid: add library code for openid groups support

2024-09-02 Thread Thomas Skinner
Signed-off-by: Thomas Skinner 
---
 proxmox-openid/src/lib.rs | 55 +--
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
index fe65fded..bf8c650b 100644
--- a/proxmox-openid/src/lib.rs
+++ b/proxmox-openid/src/lib.rs
@@ -15,8 +15,11 @@ pub use auth_state::*;
 use openidconnect::{
 //curl::http_client,
 core::{
-CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, CoreClient, 
CoreGenderClaim,
-CoreIdTokenClaims, CoreIdTokenVerifier, CoreProviderMetadata,
+CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, 
CoreErrorResponseType,
+CoreGenderClaim, CoreIdTokenVerifier, CoreJsonWebKey, 
CoreJsonWebKeyType,
+CoreJsonWebKeyUse, CoreJweContentEncryptionAlgorithm, 
CoreJwsSigningAlgorithm,
+CoreProviderMetadata, CoreRevocableToken, CoreRevocationErrorResponse,
+CoreTokenIntrospectionResponse, CoreTokenType,
 },
 AdditionalClaims,
 AuthenticationContextClass,
@@ -24,6 +27,9 @@ use openidconnect::{
 ClientId,
 ClientSecret,
 CsrfToken,
+EmptyExtraTokenFields,
+IdTokenClaims,
+IdTokenFields,
 IssuerUrl,
 Nonce,
 OAuth2TokenResponse,
@@ -31,15 +37,47 @@ use openidconnect::{
 PkceCodeVerifier,
 RedirectUrl,
 Scope,
+StandardErrorResponse,
+StandardTokenResponse,
 UserInfoClaims,
 };
 
 /// Stores Additional Claims into a serde_json::Value;
-#[derive(Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
 pub struct GenericClaims(Value);
 impl AdditionalClaims for GenericClaims {}
 
 pub type GenericUserInfoClaims = UserInfoClaims;
+pub type GenericIdTokenClaims = IdTokenClaims;
+
+pub type GenericIdTokenFields = IdTokenFields<
+GenericClaims,
+EmptyExtraTokenFields,
+CoreGenderClaim,
+CoreJweContentEncryptionAlgorithm,
+CoreJwsSigningAlgorithm,
+CoreJsonWebKeyType,
+>;
+
+pub type GenericTokenResponse = StandardTokenResponse;
+
+pub type GenericClient = openidconnect::Client<
+GenericClaims,
+CoreAuthDisplay,
+CoreGenderClaim,
+CoreJweContentEncryptionAlgorithm,
+CoreJwsSigningAlgorithm,
+CoreJsonWebKeyType,
+CoreJsonWebKeyUse,
+CoreJsonWebKey,
+CoreAuthPrompt,
+StandardErrorResponse,
+GenericTokenResponse,
+CoreTokenType,
+CoreTokenIntrospectionResponse,
+CoreRevocableToken,
+CoreRevocationErrorResponse,
+>;
 
 #[derive(Debug, Deserialize, Serialize, Clone)]
 pub struct OpenIdConfig {
@@ -56,7 +94,7 @@ pub struct OpenIdConfig {
 }
 
 pub struct OpenIdAuthenticator {
-client: CoreClient,
+client: GenericClient,
 config: OpenIdConfig,
 }
 
@@ -120,8 +158,9 @@ impl OpenIdAuthenticator {
 
 let provider_metadata = CoreProviderMetadata::discover(&issuer_url, 
http_client)?;
 
-let client = CoreClient::from_provider_metadata(provider_metadata, 
client_id, client_key)
-.set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?);
+let client =
+GenericClient::from_provider_metadata(provider_metadata, 
client_id, client_key)
+
.set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?);
 
 Ok(Self {
 client,
@@ -195,7 +234,7 @@ impl OpenIdAuthenticator {
 &self,
 code: &str,
 private_auth_state: &PrivateAuthState,
-) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
+) -> Result<(GenericIdTokenClaims, GenericUserInfoClaims), Error> {
 let code = AuthorizationCode::new(code.to_string());
 // Exchange the code with a token.
 let token_response = self
@@ -206,7 +245,7 @@ impl OpenIdAuthenticator {
 .map_err(|err| format_err!("Failed to contact token endpoint: {}", 
err))?;
 
 let id_token_verifier: CoreIdTokenVerifier = 
self.client.id_token_verifier();
-let id_token_claims: &CoreIdTokenClaims = token_response
+let id_token_claims: &GenericIdTokenClaims = token_response
 .extra_fields()
 .id_token()
 .expect("Server did not return an ID token")
-- 
2.39.2


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



[pve-devel] [PATCH access-control 1/1] fix #4411: openid: add logic for openid groups support

2024-09-02 Thread Thomas Skinner
Signed-off-by: Thomas Skinner 
---
 src/PVE/API2/OpenId.pm | 32 
 src/PVE/Auth/OpenId.pm | 21 +
 2 files changed, 53 insertions(+)

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index 77410e6..22a2188 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -220,6 +220,38 @@ __PACKAGE__->register_method ({
$rpcenv->check_user_enabled($username);
}
 
+   if (defined(my $groups_claim = $config->{'groups-claim'})) {
+   if (defined(my $groups_list = $info->{$groups_claim})) {
+   if (UNIVERSAL::isa($groups_list, 'ARRAY')) {
+   
PVE::AccessControl::lock_user_config(sub {
+   my $usercfg = 
cfs_read_file("user.cfg");
+   
+   # if groups should be 
overwritten, delete them first
+   if ( 
$config->{'groups-overwrite'}) {
+   
PVE::AccessControl::delete_user_group($username, $usercfg);
+   }
+   
+   # replace any invalid 
characters with
+   my $replace_character = 
$config->{'groups-replace-character'};
+   my @oidc_groups_list = map { $_ 
=~ s/[^A-Za-z0-9\.\-_]/$replace_character/gr } @{ $groups_list };
+   
+   # only populate groups that are 
in the oidc list and exist in pve
+   my @existing_groups_list = keys 
%{$usercfg->{groups}};
+   my @groups_intersect = grep { 
my $g = $_; grep $_ eq $g, @oidc_groups_list } @existing_groups_list;
+
+   # ensure user is a member of 
these groups
+   map { 
PVE::AccessControl::add_user_group($username, $usercfg, $_) } @groups_intersect;
+
+   cfs_write_file("user.cfg", 
$usercfg);
+   }, "openid group mapping failed");
+   } else {
+   syslog('err', "groups list is not an 
array; groups will not be updated");
+   }
+   } else {
+   syslog('err', "groups claim '$groups_claim' is 
not found in claims");
+   }
+   }
+
my $ticket = PVE::AccessControl::assemble_ticket($username);
my $csrftoken = 
PVE::AccessControl::assemble_csrf_prevention_token($username);
my $cap = $rpcenv->compute_api_permission($username);
diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
index c8e4db9..0e3fdc4 100755
--- a/src/PVE/Auth/OpenId.pm
+++ b/src/PVE/Auth/OpenId.pm
@@ -42,6 +42,24 @@ sub properties {
type => 'string',
optional => 1,
},
+   "groups-claim" => {
+   description => "OpenID claim used to retrieve groups with.",
+   type => 'string',
+   optional => 1,
+   },
+   "groups-overwrite" => {
+   description => "All groups will be overwritten for the user on 
login.",
+   type => 'boolean',
+   default => 0,
+   optional => 1,
+   },
+   "groups-replace-character" => {
+   description => "Character used to replace any invalid characters in 
groups from provider.",
+   type => 'string',
+   pattern => '^[A-Za-z0-9\.\-_]$',
+   default => '_',
+   optional => 1,
+   },
prompt => {
description => "Specifies whether the Authorization Server prompts 
the End-User for"
." reauthentication and consent.",
@@ -73,6 +91,9 @@ sub options {
"client-key" => { optional => 1 },
autocreate => { optional => 1 },
"username-claim" => { optional => 1, fixed => 1 },
+   "groups-claim" => { optional => 1 },
+   "groups-overwrite" => { optional => 1 },
+   "groups-replace-character" => { optional => 1},
prompt => { optional => 1 },
scopes => { optional => 1 },
"acr-values" => { optional => 1 },
-- 
2.39.2


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



Re: [pve-devel] [PATCH qemu-server] fix #5284: diallow moving vm disks to storages not meant for images

2024-09-02 Thread Filip Schauer

I tried to move a VM disk from a directory storarge to another directory
storage that does not support the content type 'images'.

```
$ qm disk move 103 efidisk0 local2
400 Parameter verification failed.
storage: storage 'local2' does not support vm images
qm disk move   [] [OPTIONS]
```

As expected, it was prevented.

Trying to move a VM disk to a Proxmox Backup Server now returns a more
fitting error message.

Before the patch:

```
$ qm disk move 103 efidisk0 store1 --delete
create full clone of drive efidisk0 (local:103/vm-103-disk-0.qcow2)
storage migration failed: can't allocate space in pbs storage
```

After the patch:

```
$ qm disk move 103 efidisk0 store1 --delete
400 Parameter verification failed.
storage: storage 'store1' does not support vm images
qm disk move   [] [OPTIONS]
```

PS: The title has a typo: "diallow"

Tested-By: Filip Schauer 

On 29/08/2024 16:26, Daniel Kral wrote:

Adds a check if the target storage of a VM disk move between two
different storages supports the content type 'images'. Without the
check, it will move the disk image to storages which do not support VM
images, which causes the VM to fail at startup (for any non-unused
disks).

Signed-off-by: Daniel Kral 
---
There is a content type check for any used volume (e.g. 'scsi0', ...) at
PVE/QemuServer.pm:3964 in the subroutine `config_to_command` which will
(at least) make the VM fail to start unless the volumes are moved to a
storage that supports images again.

Also, just as a note, moving the disk to storages that do not support
the `vdisk_alloc` method in the storage plugin (like PBS) also
rightfully fail before and after this change.

  PVE/API2/Qemu.pm | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index d25a79fe..b6ba9d21 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4409,6 +4409,9 @@ __PACKAGE__->register_method({
);
} elsif ($storeid) {
$rpcenv->check($authuser, "/storage/$storeid", 
['Datastore.AllocateSpace']);
+   my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+   raise_param_exc({ storage => "storage '$storeid' does not support vm 
images" })
+   if !$scfg->{content}->{images};
  
  	$load_and_check_move->(); # early checks before forking/locking
  



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



Re: [pve-devel] Continuing on making the VM ID suggestion strategy configurable

2024-09-02 Thread Daniel Krambrock via pve-devel
--- Begin Message ---

Hi everyone,

please excuse me for leaving this issue lying around. I am very pleased 
that Severen is interested in continuing this patch and takes over the work.


Am 30.08.24 um 01:08 schrieb Severen Redwood:

There's a patch series [2] from a few months ago which addresses this by
making the VM ID suggestion strategy configurable with the following
options:

1. Use the smallest ID that is not currently in use (current behaviour).
2. Use one greater than the largest ID in use.
3. Use the smallest ID that is neither currently nor previously in use.

In particular, option 3 is the one that would best solve the problem for
me.


You are of course right, only option 3 solves the problem as also 
Shannon pointed out.



However, the patches are stuck on some unresolved feedback and the
author (CC'd in) seems to have either paused or abandoned work on the
feature. For this reason, I'm interested in picking up where they left
off to get the feature to an acceptable state. Is this OK?


For me this is more then OK, thank you. In case it's still important: I 
have already signed the Harmony CLA.


Yours sincerely,
Daniel

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


Re: [pve-devel] [PATCH qemu-server] fix #5657: allow configuring RNG device as non-root user

2024-09-02 Thread Fabian Grünbichler
On August 26, 2024 1:08 pm, Filip Schauer wrote:
> Allow any user with the VM.Config.HWType permission to add or remove a
> VirtIO RNG device on a VM. This is in line with the behaviour of cloning
> a VM and restoring a VM backup as defined in
> PVE::QemuServer::check_mapping_access.

IIRC this was intentional, since passing in the hardware RNG can starve
the host of entropy rather quickly. is this no longer the case, or
handled by some other check? if so, please include these details here.
if not, then I don't think we want to go with this patch - but maybe we
want to tighten some other code paths instead ;)

> 
> Signed-off-by: Filip Schauer 
> ---
>  PVE/API2/Qemu.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index d25a79f..5ab65f9 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -592,6 +592,7 @@ my $hwtypeoptions = {
>  'vga' => 1,
>  'watchdog' => 1,
>  'audio0' => 1,
> +'rng0' => 1,
>  };
>  
>  my $generaloptions = {
> -- 
> 2.39.2
> 
> 
> 
> ___
> 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



[pve-devel] [PATCH container] fix #5674: add missing 'proxyto' for LXC interfaces API

2024-09-02 Thread Fabian Grünbichler
else this API endpoint would only work when connected to the node where the
container is currently running.

Signed-off-by: Fabian Grünbichler 
---
 src/PVE/API2/LXC.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 741f33c..d9f1c0a 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -2523,6 +2523,7 @@ __PACKAGE__->register_method({
 path => '{vmid}/interfaces',
 method => 'GET',
 protected => 1,
+proxyto => 'node',
 permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
 },
-- 
2.39.2



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


[pve-devel] [PATCH storage] base plugin: do not decode the empty string

2024-09-02 Thread Maximiliano Sandoval
If the json was empty, for example if the qemu-img command times out, a
message

warn "could not parse qemu-img info command output for '$filename' - 
$err\n";

would have been printed.

This message could lead one to think the issue lies in the contents of
the json, even if the previous warning said that there was a timeout.

Signed-off-by: Maximiliano Sandoval 
---
 src/PVE/Storage/Plugin.pm | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 6444390f..8cc693c7 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -974,6 +974,10 @@ sub file_size_info {
# otherwise we warn about it and try to parse the json
warn $err_output;
 }
+if (!$json) {
+   # skip decoding if there was no output, e.g. if there was a timeout.
+   return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef;
+}
 
 my $info = eval { decode_json($json) };
 if (my $err = $@) {
-- 
2.39.2



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



Re: [pve-devel] [PATCH storage v4] fix #4272: btrfs: add rename feature

2024-09-02 Thread Maximiliano Sandoval


Maximiliano Sandoval  writes:

> Ping.

Ping.

-- 
Maximiliano


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