public inbox for pve-devel@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; 5+ 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] 5+ 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; 5+ 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] 5+ 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-03-31 10:03 ` [pve-devel] [PATCH common 2/2] encrypt_pw: check return value matches expected format Fabian Grünbichler
  2 siblings, 1 reply; 5+ 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] 5+ 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
  2 siblings, 0 replies; 5+ 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] 5+ 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
  0 siblings, 0 replies; 5+ 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] 5+ messages in thread

end of thread, other threads:[~2025-03-31 10:42 UTC | newest]

Thread overview: 5+ 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-03-31 10:03 ` [pve-devel] [PATCH common 2/2] encrypt_pw: check return value matches expected format Fabian Grünbichler

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