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

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

[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.

A VM configuration file which, for example, contains:

    systemd-cred0: name=foo,value=bar
    systemd-cred1: name=encoded-foo,value=YmFy,base64=1

will have the following arguments added to its kvm command:

    -smbios 'type=11,value=io.systemd.credential:foo=bar' \
    -smbios 'type=11,value=io.systemd.credential.binary:encoded-foo=YmFy'

On the guest these credentials can be read via:

    dmidecode -t 11

via:

    systemd-creds --system list
    systemd-creds --system cat $CREDENTIAL_NAME

or at:

   /run/credentials/@system/$CREDENTIAL_NAME

[1] https://systemd.io/CREDENTIALS/
[2] https://www.freedesktop.org/software/systemd/man/latest/systemd.system-credentials.html

Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Differences from RFC:
 - Fixed multiple points from the feedback in the RFC
 - Uses "systemd-cred$i" instead of "systemd-credential.$name" as a name
 - The snipped support was dropped
 - More strict format required on credential names and values


 PVE/QemuServer.pm | 72 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ffd5d56b..1f902b8b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -150,6 +150,31 @@ my $watchdog_fmt = {
 };
 PVE::JSONSchema::register_format('pve-qm-watchdog', $watchdog_fmt);
 
+my $MAX_CREDENTIALS = 16;
+
+my $systemd_cred_fmt = {
+    value => {
+	description => 'The credential\'s value. This is a Base64 encoded string.'
+	    .' If the value should contain arbitrary binary data,'
+	    .' then the value can be a Base64 encoded string and the base64=1 flag should be set.',
+	type => 'string',
+	pattern => '[A-Za-z0-9+\/]+={0,2}',
+	typetext => '<string>',
+    },
+    name => {
+	description => 'The credential\'s name. This is a short ASCII string suitable as filename in the filesystem',
+	type => 'string',
+	pattern => '[A-Za-z0-9\-\._]+',
+	typetext => '<string>',
+    },
+    base64 => {
+	description => 'Whether the credential\'s value is base64 encoded.',
+	type => 'boolean',
+	optional => 1,
+	default => 0,
+    },
+};
+
 my $agent_fmt = {
     enabled => {
 	description => "Enable/disable communication with a QEMU Guest Agent (QGA) running in the VM.",
@@ -946,6 +971,13 @@ my $netdesc = {
     description => "Specify network devices.",
 };
 
+my $systemd_cred_desc = {
+    optional => 1,
+    type => 'string',
+    format => $systemd_cred_fmt,
+    description => 'Specify system and service credentials.',
+};
+
 PVE::JSONSchema::register_standard_option("pve-qm-net", $netdesc);
 
 my $ipconfig_fmt = {
@@ -1007,6 +1039,10 @@ for (my $i = 0; $i < $MAX_NETS; $i++)  {
     $confdesc_cloudinit->{"ipconfig$i"} = $ipconfigdesc;
 }
 
+for (my $i = 0; $i < $MAX_CREDENTIALS; $i++)  {
+    $confdesc->{"systemd-cred$i"} = $systemd_cred_desc;
+}
+
 foreach my $key (keys %$confdesc_cloudinit) {
     $confdesc->{$key} = $confdesc_cloudinit->{$key};
 }
@@ -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 $@;
+    return $res;
+}
+
 sub get_qga_key {
     my ($conf, $key) = @_;
     return undef if !defined($conf->{agent});
@@ -3383,6 +3429,17 @@ my sub get_vga_properties {
     return ($vga, $qxlnum);
 }
 
+sub smbios_11_cred_arg {
+    my ($name, $value, $is_encoded) = @_;
+
+    if ($is_encoded) {
+	die "value $value is not base64 encoded\n" if $value !~ /^[A-Za-z0-9+\/]+={0,2}$/;
+	return ('-smbios', "type=11,value=io.systemd.credential.binary:$name=$value");
+    } else {
+	return ('-smbios', "type=11,value=io.systemd.credential:$name=$value");
+    }
+}
+
 sub config_to_command {
     my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
         $live_restore_backing) = @_;
@@ -4015,6 +4072,21 @@ sub config_to_command {
 	push @$cmd, '-snapshot';
     }
 
+    for (my $i = 0; $i < $MAX_CREDENTIALS; $i++)  {
+	my $cred_name = "systemd-cred$i";
+
+	next if !$conf->{$cred_name};
+
+	my $opts = parse_systemd_credential($conf->{$cred_name});
+	my $name = $opts->{'name'};
+	my $is_encoded = $opts->{'base64'};
+	my $value = $opts->{'value'};
+
+	if ($value && $name) {
+	    push @$cmd, smbios_11_cred_arg($name, $value, $is_encoded);
+	}
+    }
+
     # add custom args
     if ($conf->{args}) {
 	my $aa = PVE::Tools::split_args($conf->{args});
-- 
2.39.5



_______________________________________________
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] config: add system and service credentials support

[Thomas Lamprecht]

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?

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.

> 
> A VM configuration file which, for example, contains:
> 
>     systemd-cred0: name=foo,value=bar
>     systemd-cred1: name=encoded-foo,value=YmFy,base64=1

Tangentially related: Moving the VM config fully over to section config
parsing would get us list/array support for free – albeit the move might be
rather costly..


> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index ffd5d56b..1f902b8b 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -150,6 +150,31 @@ my $watchdog_fmt = {
>  };
>  PVE::JSONSchema::register_format('pve-qm-watchdog', $watchdog_fmt);
>  
> +my $MAX_CREDENTIALS = 16;
> +
> +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?

> +	    .' If the value should contain arbitrary binary data,'

nit: why the early line break in the mid of the same sentence?

> +	    .' then the value can be a Base64 encoded string and the base64=1 flag should be set.',

different casing: Base64 here and base64 below for the property
with the same name.

> +	type => 'string',
> +	pattern => '[A-Za-z0-9+\/]+={0,2}',
> +	typetext => '<string>',
> +    },
> +    name => {
> +	description => 'The credential\'s name. This is a short ASCII string suitable as filename in the filesystem',
> +	type => 'string',
> +	pattern => '[A-Za-z0-9\-\._]+',

The dot does not need to be escaped in a [characterset] pattern, and new use-sites
might prefer using a quoted regex via qr/REGEX/ [perlop-regex-quote], which we
already use on a few sites and has some small performance benefit and also
features like having some regex-modifier flags enforced just for that sub
pattern. I.e. the following should be the same: qr/[a-z0-9_.-]/i – not that
you need to use exactly that here, just fyi in case you did not know about qr//.

[perlop-regex-quote]: https://perldoc.perl.org/perlop#Regexp-Quote-Like-Operators

> +	typetext => '<string>',
> +    },
> +    base64 => {
> +	description => 'Whether the credential\'s value is base64 encoded.',
> +	type => 'boolean',
> +	optional => 1,
> +	default => 0,
> +    },
> +};
> +
>  my $agent_fmt = {
>      enabled => {
>  	description => "Enable/disable communication with a QEMU Guest Agent (QGA) running in the VM.",
> @@ -946,6 +971,13 @@ my $netdesc = {
>      description => "Specify network devices.",
>  };
>  
> +my $systemd_cred_desc = {
> +    optional => 1,
> +    type => 'string',
> +    format => $systemd_cred_fmt,
> +    description => 'Specify system and service credentials.',
> +};
> +
>  PVE::JSONSchema::register_standard_option("pve-qm-net", $netdesc);
>  
>  my $ipconfig_fmt = {

> @@ -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? 

> +    return $res;
> +}
> +
>  sub get_qga_key {
>      my ($conf, $key) = @_;
>      return undef if !defined($conf->{agent});
> @@ -3383,6 +3429,17 @@ my sub get_vga_properties {
>      return ($vga, $qxlnum);
>  }
>  
> +sub smbios_11_cred_arg {
> +    my ($name, $value, $is_encoded) = @_;
> +
> +    if ($is_encoded) {
> +	die "value $value is not base64 encoded\n" if $value !~ /^[A-Za-z0-9+\/]+={0,2}$/;

A base64 regex might be good to have in a module wide variable, that then can
also be used in the JSONSchema format above. Can be here in this module for now.

> +	return ('-smbios', "type=11,value=io.systemd.credential.binary:$name=$value");
> +    } else {
> +	return ('-smbios', "type=11,value=io.systemd.credential:$name=$value");
> +    }
> +}
> +
>  sub config_to_command {
>      my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
>          $live_restore_backing) = @_;
> @@ -4015,6 +4072,21 @@ sub config_to_command {
>  	push @$cmd, '-snapshot';
>      }
>  
> +    for (my $i = 0; $i < $MAX_CREDENTIALS; $i++)  {
> +	my $cred_name = "systemd-cred$i";
> +
> +	next if !$conf->{$cred_name}

might be tiny bit nicer if written as:

my $cred = $conf->{"systemd-cred$i"} or next;

;
> +
> +	my $opts = parse_systemd_credential($conf->{$cred_name});
> +	my $name = $opts->{'name'};
> +	my $is_encoded = $opts->{'base64'};
> +	my $value = $opts->{'value'};

I'd avoid those useless intermediate variables, this part is not big enough
to justify them IMO, just go for direct shell access.

> +
> +	if ($value && $name) {
> +	    push @$cmd, smbios_11_cred_arg($name, $value, $is_encoded);
> +	}
> +    }
> +
>      # add custom args
>      if ($conf->{args}) {
>  	my $aa = PVE::Tools::split_args($conf->{args}); 


_______________________________________________
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] config: add system and service credentials support

[Thomas Lamprecht]

Am 03.04.25 um 09:49 schrieb Thomas Lamprecht:
> 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?
> 
> 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.
> 
>>
>> A VM configuration file which, for example, contains:
>>
>>     systemd-cred0: name=foo,value=bar
>>     systemd-cred1: name=encoded-foo,value=YmFy,base64=1
> 
> Tangentially related: Moving the VM config fully over to section config
> parsing would get us list/array support for free – albeit the move might be
> rather costly..
Oh, and one alternative would be to provide just one `systemd-credenentials` key
here that always refers to such a credential mapping, which has then an array
of all the credentials with their name and value.

As I checked [fdo-sc] and there are like 50 supported credentials, some with
a glob to allow sets of credentials per type like for network or udev rules,
so 15 might not cut it quite fast even if the use-case is not _that_ complex.

[fdo-sc] https://www.freedesktop.org/software/systemd/man/latest/systemd.system-credentials.html


_______________________________________________
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] config: add system and service credentials support

[Maximiliano Sandoval]

Thomas Lamprecht <t.lamprecht@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.

> 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 VM configuration file which, for example, contains:
>> 
>>     systemd-cred0: name=foo,value=bar
>>     systemd-cred1: name=encoded-foo,value=YmFy,base64=1
>
> Tangentially related: Moving the VM config fully over to section config
> parsing would get us list/array support for free – albeit the move might be
> rather costly..
>
>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index ffd5d56b..1f902b8b 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -150,6 +150,31 @@ my $watchdog_fmt = {
>>  };
>>  PVE::JSONSchema::register_format('pve-qm-watchdog', $watchdog_fmt);
>>  
>> +my $MAX_CREDENTIALS = 16;
>> +
>> +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.

>> +	    .' If the value should contain arbitrary binary data,'
>
> nit: why the early line break in the mid of the same sentence?
>
>> +	    .' then the value can be a Base64 encoded string and the base64=1 flag should be set.',
>
> different casing: Base64 here and base64 below for the property
> with the same name.
>
>> +	type => 'string',
>> +	pattern => '[A-Za-z0-9+\/]+={0,2}',
>> +	typetext => '<string>',
>> +    },
>> +    name => {
>> +	description => 'The credential\'s name. This is a short ASCII string suitable as filename in the filesystem',
>> +	type => 'string',
>> +	pattern => '[A-Za-z0-9\-\._]+',
>
> The dot does not need to be escaped in a [characterset] pattern, and new use-sites
> might prefer using a quoted regex via qr/REGEX/ [perlop-regex-quote], which we
> already use on a few sites and has some small performance benefit and also
> features like having some regex-modifier flags enforced just for that sub
> pattern. I.e. the following should be the same: qr/[a-z0-9_.-]/i – not that
> you need to use exactly that here, just fyi in case you did not know about qr//.
>
> [perlop-regex-quote]: https://perldoc.perl.org/perlop#Regexp-Quote-Like-Operators
>
>> +	typetext => '<string>',
>> +    },
>> +    base64 => {
>> +	description => 'Whether the credential\'s value is base64 encoded.',
>> +	type => 'boolean',
>> +	optional => 1,
>> +	default => 0,
>> +    },
>> +};
>> +
>>  my $agent_fmt = {
>>      enabled => {
>>  	description => "Enable/disable communication with a QEMU Guest Agent (QGA) running in the VM.",
>> @@ -946,6 +971,13 @@ my $netdesc = {
>>      description => "Specify network devices.",
>>  };
>>  
>> +my $systemd_cred_desc = {
>> +    optional => 1,
>> +    type => 'string',
>> +    format => $systemd_cred_fmt,
>> +    description => 'Specify system and service credentials.',
>> +};
>> +
>>  PVE::JSONSchema::register_standard_option("pve-qm-net", $netdesc);
>>  
>>  my $ipconfig_fmt = {
>
>> @@ -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.

>
>> +    return $res;
>> +}
>> +
>>  sub get_qga_key {
>>      my ($conf, $key) = @_;
>>      return undef if !defined($conf->{agent});
>> @@ -3383,6 +3429,17 @@ my sub get_vga_properties {
>>      return ($vga, $qxlnum);
>>  }
>>  
>> +sub smbios_11_cred_arg {
>> +    my ($name, $value, $is_encoded) = @_;
>> +
>> +    if ($is_encoded) {
>> +	die "value $value is not base64 encoded\n" if $value !~ /^[A-Za-z0-9+\/]+={0,2}$/;
>
> A base64 regex might be good to have in a module wide variable, that then can
> also be used in the JSONSchema format above. Can be here in this module for now.
>
>> +	return ('-smbios', "type=11,value=io.systemd.credential.binary:$name=$value");
>> +    } else {
>> +	return ('-smbios', "type=11,value=io.systemd.credential:$name=$value");
>> +    }
>> +}
>> +
>>  sub config_to_command {
>>      my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
>>          $live_restore_backing) = @_;
>> @@ -4015,6 +4072,21 @@ sub config_to_command {
>>  	push @$cmd, '-snapshot';
>>      }
>>  
>> +    for (my $i = 0; $i < $MAX_CREDENTIALS; $i++)  {
>> +	my $cred_name = "systemd-cred$i";
>> +
>> +	next if !$conf->{$cred_name}
>
> might be tiny bit nicer if written as:
>
> my $cred = $conf->{"systemd-cred$i"} or next;
>
> ;
>> +
>> +	my $opts = parse_systemd_credential($conf->{$cred_name});
>> +	my $name = $opts->{'name'};
>> +	my $is_encoded = $opts->{'base64'};
>> +	my $value = $opts->{'value'};
>
> I'd avoid those useless intermediate variables, this part is not big enough
> to justify them IMO, just go for direct shell access.
>
>> +
>> +	if ($value && $name) {
>> +	    push @$cmd, smbios_11_cred_arg($name, $value, $is_encoded);
>> +	}
>> +    }
>> +
>>      # add custom args
>>      if ($conf->{args}) {
>>  	my $aa = PVE::Tools::split_args($conf->{args}); 



_______________________________________________
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] config: add system and service credentials support

[Maximiliano Sandoval]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> Am 03.04.25 um 09:49 schrieb Thomas Lamprecht:
>> 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?
>> 
>> 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.
>> 
>>>
>>> A VM configuration file which, for example, contains:
>>>
>>>     systemd-cred0: name=foo,value=bar
>>>     systemd-cred1: name=encoded-foo,value=YmFy,base64=1
>> 
>> Tangentially related: Moving the VM config fully over to section config
>> parsing would get us list/array support for free – albeit the move might be
>> rather costly..
> Oh, and one alternative would be to provide just one `systemd-credenentials` key
> here that always refers to such a credential mapping, which has then an array
> of all the credentials with their name and value.
>
> As I checked [fdo-sc] and there are like 50 supported credentials, some with
> a glob to allow sets of credentials per type like for network or udev rules,
> so 15 might not cut it quite fast even if the use-case is not _that_ complex.
>
> [fdo-sc] https://www.freedesktop.org/software/systemd/man/latest/systemd.system-credentials.html

The number 16 was picked at random, we can certainly bump it. The
credentials in [fdo-sc] are just some well-known credentials which are
picked up at boot by systemd and which will then "do something with
them", the user might on top of them also add e.g. the newly introduced
credentials for the backup-client or Apache's or just some random data
they need in their app inside the guest.


_______________________________________________
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] config: add system and service credentials support

[Thomas Lamprecht]

Am 03.04.25 um 10:34 schrieb Maximiliano Sandoval:
> Thomas Lamprecht <t.lamprecht@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.


_______________________________________________
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] config: add system and service credentials support

[Lukas Wagner]

On  2025-04-03 11:04, Thomas Lamprecht wrote:
> Am 03.04.25 um 10:34 schrieb Maximiliano Sandoval:
>>
>> 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. 

For context:

With webhooks, we have 'secrets', which are dedicated key-value pairs which can be
configured via the UI. For instance, you could set up a secret with key 'password'
and value '12345'. In the URL/Body/Headers we support templating syntax that allows to
access secrets via the 'secret' namespace, e.g {{ secret.password }}.
All secrets are stored in /etc/pve/priv/notifications.cfg, which is, as you said,
only readable by root.

-- 
- Lukas



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