[pve-devel] [PATCH qemu-server] config: add system and service credentials support

[Thomas Lamprecht]

Am 03.04.25 um 10:34 schrieb Maximiliano Sandoval:
> Thomas Lamprecht <t.lamprecht at proxmox.com> writes:
> 
>> Am 02.04.25 um 16:36 schrieb Maximiliano Sandoval:
>>> Allows to pass system and service credentials to a VM. See [1] for a
>>> description of credentials. This can be potentially used to provision a
>>> VM as per [2]. Values can be passed either as plain text or as a base64
>>> encoded string when the base64 flag is set.
>>
>> Would this also make sense for Containers?
> 
> Absolutely, this would be a followup. The mechanism to pass them down to
> containers is not via smbios 11 though.

yeah sure, the transport is just a virtualization-specific
implementation detail.

> 
>> If it's something we can expose for all guests, we could also (later) look
>> into implementing some simple registry (like a mappings type) fulfilling
>> what the snippets approach would  provide one while having it nicely
>> integrated into our access control system.
> 
> As per systemd-exec's man page, in total one can pass up to 1MB in
> system credentials. A VM config file is certainly not the vehicle for
> such an amount of data and I am also not fully comfortable with putting
> potentially sensitive data as plain-text inside config files or the
> cluster filesystem. I am not fully sure how to approach this long term.
> 
> 
> There is also the more-secure possibility to pass down system
> credentials from the host to the guest (e.g. ImportCredential= or
> LoadCredential=) but that would have the drawback that there is no
> mechanism to sync them acros a cluster.

A mapping could abstract most of that away and also use a flag to denote
if a credential is confidential and then safe it in the root-only
/etc/pve/priv path, IIRC we do something similar for notifications
targets like webhooks. 

anyway, the more I think about this the surer I am that having this
centralized would be much nicer compared to inlining this for every VM
directly in the config; also with the 16 being rather low limit from my
other response.

>>> +my $systemd_cred_fmt = {
>>> +    value => {
>>> +	description => 'The credential\'s value. This is a Base64 encoded string.'
>>
>> But it isn't if the base64 flag is not set?
> 
> I discussed this off-list a bit with Wolfgang and Fabian and I decided
> for the time being to only accept the character set of base64, if a user
> needs more control, they can encode the string as base64. But yes, the
> description should be improved.
> 
> An alternative would be to require the string to be either a base64
> string or match a hand-picked format, e.g. [a-zA-Z0-9\-\.], but that
> would require studying what are the possible use-cases.

Then it might be best to drop the base64 flag for now and enforce that
everywhere. If we want to support anything else in the future we can add
an encoding sub-property enum that defaults to base64 once its actually
required, or deemed useful.

>>> @@ -1955,6 +1991,16 @@ sub parse_guest_agent {
>>>      return $res;
>>>  }
>>>  
>>> +sub parse_systemd_credential {
>>> +    my ($value) = @_;
>>> +
>>> +    return {} if !$value;
>>> +
>>> +    my $res = eval { parse_property_string($systemd_cred_fmt, $value) };
>>> +    warn $@ if $@;
>> Should we really just ignore the error and warn here in a generic parser?
>> Isn't that the callers job to decide?
> 
> Here I just copied what the other parsers in this file do. I am not sure
> how to implement something else.
> 

Alright, so an existing deficiency then; so lets keep that approach for
now.