all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [RFC access-control/common 0/3] hash passwords using yescrypt
@ 2025-03-31 10:03 Fabian Grünbichler
  2025-03-31 10:03 ` [pve-devel] [PATCH access-control 1/1] PVE/PAM: switch to yescrypt by default Fabian Grünbichler
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2025-03-31 10:03 UTC (permalink / raw)
  To: pve-devel

Debian switched the default hash algorithm for /etc/shadow to yescrypt
for Bullseye. Our installer uses it for the root password set during
installation. But any PAM/PVE user created over the API, or any password
change triggered afterwards for such users, will fallback to
sha256crypt.

Since the helper in pve-common is also used by cloud-init (which is
stuck not supporting yescrypt for the time being for unrelated reasons),
make the new behaviour opt-in (which might be handy for future
migrations as well).

sending as RFC in case I missed some usage of this, and also to discuss
whether we might just want to move PVE/PAM realms over to a proxmox-sys
perlmod-wrapped helper instead (proxmox-sys and thus PBS defaults to
yescrypt and binds to the C lib interfaces that actually allow
specifying hashing parameters somewhat sanely)

pve-access-control:

Fabian Grünbichler (1):
  PVE/PAM: switch to yescrypt by default

 src/PVE/Auth/PAM.pm | 2 +-
 src/PVE/Auth/PVE.pm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

pve-common:

Fabian Grünbichler (2):
  encrypt_pw: allow yescrypt in addition to sha256
  encrypt_pw: check return value matches expected format

 src/PVE/Tools.pm | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH access-control 1/1] PVE/PAM: switch to yescrypt by default
  2025-03-31 10:03 [pve-devel] [RFC access-control/common 0/3] hash passwords using yescrypt Fabian Grünbichler
@ 2025-03-31 10:03 ` Fabian Grünbichler
  2025-03-31 10:03 ` [pve-devel] [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256 Fabian Grünbichler
  2025-03-31 10:03 ` [pve-devel] [PATCH common 2/2] encrypt_pw: check return value matches expected format Fabian Grünbichler
  2 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2025-03-31 10:03 UTC (permalink / raw)
  To: pve-devel; +Cc: Trent W . Buck

this will hash the password of new users or rehash the password on
password changes using 'yescrypt', which is the default in Debian since
Bullseye[0].

0: https://www.debian.org/releases/bullseye/amd64/release-notes/ch-information.en.html#pam-default-password

Reported-by: Trent W. Buck <trentbuck@gmail.com>
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires corresponding pve-common change, but will fallback to previous default
    sha256 on older pve-common versions by virtue of the new parameter being
    ignored.

 src/PVE/Auth/PAM.pm | 2 +-
 src/PVE/Auth/PVE.pm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Auth/PAM.pm b/src/PVE/Auth/PAM.pm
index feabc0b..85c6d12 100755
--- a/src/PVE/Auth/PAM.pm
+++ b/src/PVE/Auth/PAM.pm
@@ -72,7 +72,7 @@ sub store_password {
 
     my $cmd = ['usermod'];
 
-    my $epw = PVE::Tools::encrypt_pw($password);
+    my $epw = PVE::Tools::encrypt_pw($password, 'y');
 
     push @$cmd, '-p', $epw, $username;
 
diff --git a/src/PVE/Auth/PVE.pm b/src/PVE/Auth/PVE.pm
index de39d35..f17d716 100755
--- a/src/PVE/Auth/PVE.pm
+++ b/src/PVE/Auth/PVE.pm
@@ -95,7 +95,7 @@ sub store_password {
 
     lock_shadow_config(sub {
 	my $shadow_cfg = cfs_read_file($shadowconfigfile);
-	my $epw = PVE::Tools::encrypt_pw($password);
+	my $epw = PVE::Tools::encrypt_pw($password, 'y');
 	$shadow_cfg->{users}->{$username}->{shadow} = $epw;
 	cfs_write_file($shadowconfigfile, $shadow_cfg);
     });
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256
  2025-03-31 10:03 [pve-devel] [RFC access-control/common 0/3] hash passwords using yescrypt Fabian Grünbichler
  2025-03-31 10:03 ` [pve-devel] [PATCH access-control 1/1] PVE/PAM: switch to yescrypt by default Fabian Grünbichler
@ 2025-03-31 10:03 ` Fabian Grünbichler
  2025-03-31 10:42   ` Shannon Sterz
  2025-04-07 19:26   ` [pve-devel] applied: " Thomas Lamprecht
  2025-03-31 10:03 ` [pve-devel] [PATCH common 2/2] encrypt_pw: check return value matches expected format Fabian Grünbichler
  2 siblings, 2 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2025-03-31 10:03 UTC (permalink / raw)
  To: pve-devel

this has been the default for Debian since Bullseye[0].

besides password setting for the PAM/PVE/PMG realms, this is also used
to hash cloud-init passwords for Linux VMs, where only a subset of
prefixes is currently allowed.

'j9T' is the default cost factor for yescrypt.

0: https://www.debian.org/releases/bullseye/amd64/release-notes/ch-information.en.html#pam-default-password

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    instead of opt-in to yescrypt, we could also default to it and opt-into sha256
    in qemu-server for cloud init.. but that means breaking old qemu-server, as
    opposed to the change being completely backwards compatible..

 src/PVE/Tools.pm | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 57eb86c..95cd93c 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1805,7 +1805,7 @@ sub fchownat($$$$$) {
 my $salt_starter = time();
 
 sub encrypt_pw {
-    my ($pw) = @_;
+    my ($pw, $prefix) = @_;
 
     $salt_starter++;
     my $salt = substr(Digest::SHA::sha1_base64(time() + $salt_starter + $$), 0, 8);
@@ -1813,7 +1813,18 @@ sub encrypt_pw {
     # crypt does not want '+' in salt (see 'man crypt')
     $salt =~ s/\+/X/g;
 
-    return crypt(encode("utf8", $pw), "\$5\$$salt\$");
+    $prefix = '5' if !$prefix;
+
+    my $input;
+    if ($prefix eq '5') {
+        $input = "\$5\$$salt\$";
+    } elsif ($prefix eq 'y') {
+        $input = "\$y\$j9T\$$salt\$"
+    } else {
+        die "Cannot hash password, unknown crypt prefix '$prefix'\n";
+    }
+
+    return crypt(encode("utf8", $pw), $input);
 }
 
 # intended usage: convert_size($val, "kb" => "gb")
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH common 2/2] encrypt_pw: check return value matches expected format
  2025-03-31 10:03 [pve-devel] [RFC access-control/common 0/3] hash passwords using yescrypt Fabian Grünbichler
  2025-03-31 10:03 ` [pve-devel] [PATCH access-control 1/1] PVE/PAM: switch to yescrypt by default Fabian Grünbichler
  2025-03-31 10:03 ` [pve-devel] [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256 Fabian Grünbichler
@ 2025-03-31 10:03 ` Fabian Grünbichler
  2025-04-07 19:27   ` [pve-devel] applied: " Thomas Lamprecht
  2 siblings, 1 reply; 7+ messages in thread
From: Fabian Grünbichler @ 2025-03-31 10:03 UTC (permalink / raw)
  To: pve-devel

since this manually constructs the input string for `crypt`, which looks
different depending on used prefix/hashing algorithm, ensure that it was
understood by crypt and that it returned a proper hashed password line.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    alternatively, we could switch to a wrapper around gen_crypt_salt[_..], but a
    quick search didn't find an applicable perl one.. we do have one in
    proxmox-sys ;)

 src/PVE/Tools.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 95cd93c..9792ad6 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1824,7 +1824,12 @@ sub encrypt_pw {
         die "Cannot hash password, unknown crypt prefix '$prefix'\n";
     }
 
-    return crypt(encode("utf8", $pw), $input);
+    my $res = crypt(encode("utf8", $pw), $input);
+    if ($res =~ m/^\$$prefix\$/) {
+        return $res;
+    } else {
+        die "Failed to hash password!\n";
+    }
 }
 
 # intended usage: convert_size($val, "kb" => "gb")
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256
  2025-03-31 10:03 ` [pve-devel] [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256 Fabian Grünbichler
@ 2025-03-31 10:42   ` Shannon Sterz
  2025-04-07 19:26   ` [pve-devel] applied: " Thomas Lamprecht
  1 sibling, 0 replies; 7+ messages in thread
From: Shannon Sterz @ 2025-03-31 10:42 UTC (permalink / raw)
  To: Proxmox VE development discussion; +Cc: pve-devel

On Mon Mar 31, 2025 at 12:03 PM CEST, Fabian Grünbichler wrote:
> this has been the default for Debian since Bullseye[0].
>
> besides password setting for the PAM/PVE/PMG realms, this is also used
> to hash cloud-init passwords for Linux VMs, where only a subset of
> prefixes is currently allowed.
>
> 'j9T' is the default cost factor for yescrypt.
>
> 0: https://www.debian.org/releases/bullseye/amd64/release-notes/ch-information.en.html#pam-default-password
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>
> Notes:
>     instead of opt-in to yescrypt, we could also default to it and opt-into sha256
>     in qemu-server for cloud init.. but that means breaking old qemu-server, as
>     opposed to the change being completely backwards compatible..
>
>  src/PVE/Tools.pm | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 57eb86c..95cd93c 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -1805,7 +1805,7 @@ sub fchownat($$$$$) {
>  my $salt_starter = time();
>
>  sub encrypt_pw {
> -    my ($pw) = @_;
> +    my ($pw, $prefix) = @_;
>
>      $salt_starter++;
>      my $salt = substr(Digest::SHA::sha1_base64(time() + $salt_starter + $$), 0, 8);
> @@ -1813,7 +1813,18 @@ sub encrypt_pw {
>      # crypt does not want '+' in salt (see 'man crypt')
>      $salt =~ s/\+/X/g;
>
> -    return crypt(encode("utf8", $pw), "\$5\$$salt\$");
> +    $prefix = '5' if !$prefix;
> +
> +    my $input;
> +    if ($prefix eq '5') {
> +        $input = "\$5\$$salt\$";
> +    } elsif ($prefix eq 'y') {
> +        $input = "\$y\$j9T\$$salt\$"

since you already mentioned that switching to proxmox-sys via perl mod
might be an option, one thing to point out is that it already also
provides a wrapper for `man 3 crypt_gensalt` for generating the proper
cost factor the same way Debian does instead of hardcoding it as
`j9T` here. it would make it easier to keep that in-sync.

> +    } else {
> +        die "Cannot hash password, unknown crypt prefix '$prefix'\n";
> +    }
> +
> +    return crypt(encode("utf8", $pw), $input);
>  }
>
>  # intended usage: convert_size($val, "kb" => "gb")



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] applied: [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256
  2025-03-31 10:03 ` [pve-devel] [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256 Fabian Grünbichler
  2025-03-31 10:42   ` Shannon Sterz
@ 2025-04-07 19:26   ` Thomas Lamprecht
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2025-04-07 19:26 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 31.03.25 um 12:03 schrieb Fabian Grünbichler:
> this has been the default for Debian since Bullseye[0].
> 
> besides password setting for the PAM/PVE/PMG realms, this is also used
> to hash cloud-init passwords for Linux VMs, where only a subset of
> prefixes is currently allowed.
> 
> 'j9T' is the default cost factor for yescrypt.
> 
> 0: https://www.debian.org/releases/bullseye/amd64/release-notes/ch-information.en.html#pam-default-password
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     instead of opt-in to yescrypt, we could also default to it and opt-into sha256
>     in qemu-server for cloud init.. but that means breaking old qemu-server, as
>     opposed to the change being completely backwards compatible..
> 
>  src/PVE/Tools.pm | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
>

applied this one, thanks!

It should not hurt and let's use already use this then rather
easily and we can still switch the underlying implementation.


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] applied: [PATCH common 2/2] encrypt_pw: check return value matches expected format
  2025-03-31 10:03 ` [pve-devel] [PATCH common 2/2] encrypt_pw: check return value matches expected format Fabian Grünbichler
@ 2025-04-07 19:27   ` Thomas Lamprecht
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2025-04-07 19:27 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 31.03.25 um 12:03 schrieb Fabian Grünbichler:
> since this manually constructs the input string for `crypt`, which looks
> different depending on used prefix/hashing algorithm, ensure that it was
> understood by crypt and that it returned a proper hashed password line.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     alternatively, we could switch to a wrapper around gen_crypt_salt[_..], but a
>     quick search didn't find an applicable perl one.. we do have one in
>     proxmox-sys ;)
> 
>  src/PVE/Tools.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
>

applied this one too, thanks!


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-04-07 19:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-31 10:03 [pve-devel] [RFC access-control/common 0/3] hash passwords using yescrypt Fabian Grünbichler
2025-03-31 10:03 ` [pve-devel] [PATCH access-control 1/1] PVE/PAM: switch to yescrypt by default Fabian Grünbichler
2025-03-31 10:03 ` [pve-devel] [PATCH common 1/2] encrypt_pw: allow yescrypt in addition to sha256 Fabian Grünbichler
2025-03-31 10:42   ` Shannon Sterz
2025-04-07 19:26   ` [pve-devel] applied: " Thomas Lamprecht
2025-03-31 10:03 ` [pve-devel] [PATCH common 2/2] encrypt_pw: check return value matches expected format Fabian Grünbichler
2025-04-07 19:27   ` [pve-devel] applied: " Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal