[pve-devel] [PATCH installer/{pve, pmg}-docs/proxmox-backup v2 0/8] raise minimum root password length to 8 characters

[pve-devel] [PATCH installer/{pve, pmg}-docs/proxmox-backup v2 0/8] raise minimum root password length to 8 characters

[Christoph Heiss]

This idea came to light while talking with Shannon about #5756 [0].

It is 2024, so raising the minimum length for the root password as
entered during the installation from 5 to 8 characters seems very
sensible. NIST also recommends a minimum length of 8 characters for
passwords [1].

See also the respective patches for PVE/PMG [2] and PBS [3] - both
series have been already applied.

I've sent the documentation patches all together with this series, since
a) they are trivial, b) not to clutter the other mailing list and c) to
keep everything together for review.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=5756
[1] https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver
[2] https://lore.proxmox.com/pve-devel/20241004133205.258755-1-s.sterz@proxmox.com/
[3] https://lore.proxmox.com/pbs-devel/20241004134054.263913-1-s.sterz@proxmox.com/

History
=======

v1: https://lore.proxmox.com/pve-devel/20241007092218.723938-1-c.heiss@proxmox.com/

Notable changes v1 -> v2:
  * rebased on latest master
  * new patch #3; converting `InstallRootPassword` to an enum 
  * new patch #5; negative tests
  * new patches #6 through #8: respective documentation updates

Diffstat
========

pve-installer:

Christoph Heiss (5):
  proxinstall: raise minimum root password length to 8 characters
  tui: raise minimum root password length to 8 characters
  common: convert `InstallRootPassword` into an enum
  auto: raise minimum root password length to 8 characters
  auto: add negative tests for root password option

 Proxmox/Sys.pm                                |  5 +++
 html/pbs/passwd.htm                           |  2 +-
 html/pmg/passwd.htm                           |  2 +-
 html/pve/passwd.htm                           |  2 +-
 proxinstall                                   |  7 +++-
 proxmox-auto-installer/src/utils.rs           | 37 +++++++++++++------
 proxmox-auto-installer/tests/parse-answer.rs  | 22 +++++++++--
 .../both-password-and-hashed-set.fail.json    |  3 ++
 .../both-password-and-hashed-set.fail.toml    | 15 ++++++++
 .../tests/resources/parse_answer/btrfs.json   |  2 +-
 .../tests/resources/parse_answer/btrfs.toml   |  2 +-
 .../resources/parse_answer/disk_match.json    |  2 +-
 .../resources/parse_answer/disk_match.toml    |  2 +-
 .../parse_answer/disk_match_all.json          |  2 +-
 .../parse_answer/disk_match_all.toml          |  2 +-
 .../parse_answer/disk_match_any.json          |  2 +-
 .../parse_answer/disk_match_any.toml          |  2 +-
 .../resources/parse_answer/first-boot.json    |  2 +-
 .../resources/parse_answer/first-boot.toml    |  2 +-
 .../tests/resources/parse_answer/minimal.json |  2 +-
 .../tests/resources/parse_answer/minimal.toml |  2 +-
 .../resources/parse_answer/nic_matching.json  |  2 +-
 .../resources/parse_answer/nic_matching.toml  |  2 +-
 .../no-root-password-set.fail.json            |  3 ++
 .../no-root-password-set.fail.toml            | 13 +++++++
 .../parse_answer/short-password.fail.json     |  3 ++
 .../parse_answer/short-password.fail.toml     | 14 +++++++
 .../resources/parse_answer/specific_nic.json  |  2 +-
 .../resources/parse_answer/specific_nic.toml  |  2 +-
 .../tests/resources/parse_answer/zfs.json     |  2 +-
 .../tests/resources/parse_answer/zfs.toml     |  2 +-
 proxmox-installer-common/src/lib.rs           |  3 ++
 proxmox-installer-common/src/setup.rs         | 15 ++++----
 proxmox-tui-installer/src/main.rs             | 10 +++--
 proxmox-tui-installer/src/setup.rs            |  5 +--
 35 files changed, 145 insertions(+), 52 deletions(-)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.toml
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.toml
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.toml

pve-docs:

Christoph Heiss (1):
  installation: adapt to raised root password length requirement

 pve-installation.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

pmg-docs:

Christoph Heiss (1):
  installation: adapt to raised root password length requirement

 pmg-installation.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

proxmox-backup:

Christoph Heiss (1):
  using-the-installer: adapt to raised root password length requirement

 docs/using-the-installer.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.46.0



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

[pve-devel] [PATCH installer v2 1/5] proxinstall: raise minimum root password length to 8 characters

[Christoph Heiss]

.. in accordance with current NIST recommendations [0].

It's 2024; so reasonable to expect an 8-character-password at the
minimum.

[0] https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * rebased on latest master
  * adapted html templates

 Proxmox/Sys.pm      | 5 +++++
 html/pbs/passwd.htm | 2 +-
 html/pmg/passwd.htm | 2 +-
 html/pve/passwd.htm | 2 +-
 proxinstall         | 7 +++++--
 5 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Proxmox/Sys.pm b/Proxmox/Sys.pm
index afc6780..b10deb7 100644
--- a/Proxmox/Sys.pm
+++ b/Proxmox/Sys.pm
@@ -7,3 +7,8 @@ use warnings;
 # https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
 # Using that /should/ cover all possible cases that are encountered in the wild.
 our $EMAIL_RE = '^[a-zA-Z0-9.!#$%&\'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$';
+
+# Minimum password length for the root account.
+# See also https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver for the
+# recommendation.
+our $ROOT_PASSWORD_MIN_LENGTH = 8;
diff --git a/html/pbs/passwd.htm b/html/pbs/passwd.htm
index d331b28..f275a6f 100644
--- a/html/pbs/passwd.htm
+++ b/html/pbs/passwd.htm
@@ -27,7 +27,7 @@
       <tr>
         <td valign="top" width="30"><img src="plus.png"></td>
         <td valign="top"><b>Password:</b>
-          Please use a strong password. It should be at least 8 characters long,
+          Please use a strong password. It must be at least 8 characters long,
           and contain a combination of letters, numbers, and symbols.
           <br><br>
         </td>
diff --git a/html/pmg/passwd.htm b/html/pmg/passwd.htm
index 3e94480..32b74f9 100644
--- a/html/pmg/passwd.htm
+++ b/html/pmg/passwd.htm
@@ -27,7 +27,7 @@
       <tr>
 	<td valign="top" width="30"><img src="plus.png"></td>
 	<td valign="top"><b>Password:</b>
-	  Please use a strong password. It should have 8 or more characters.
+	  Please use a strong password. It must have 8 or more characters.
 	  Also combine letters, numbers, and symbols.
 	  <br><br>
 	</td>
diff --git a/html/pve/passwd.htm b/html/pve/passwd.htm
index 1c7d2fc..b70e37c 100644
--- a/html/pve/passwd.htm
+++ b/html/pve/passwd.htm
@@ -27,7 +27,7 @@
       <tr>
 	<td valign="top" width="30"><img src="plus.png"></td>
 	<td valign="top"><b>Password:</b>
-	  Please use a strong password. It should be at least 8 characters long,
+	  Please use a strong password. It must be at least 8 characters long,
 	  and contain a combination of letters, numbers, and symbols.
 	  <br><br>
 	</td>
diff --git a/proxinstall b/proxinstall
index 8168529..2c399d2 100755
--- a/proxinstall
+++ b/proxinstall
@@ -721,8 +721,11 @@ sub create_password_view {
 	my $t1 = $pwe1->get_text;
 	my $t2 = $pwe2->get_text;
 
-	if (length ($t1) < 5) {
-	    Proxmox::UI::message("Password is too short.");
+	if (length ($t1) < $Proxmox::Sys::ROOT_PASSWORD_MIN_LENGTH) {
+	    Proxmox::UI::message(
+		"Password too short, must be at least " .
+		"$Proxmox::Sys::ROOT_PASSWORD_MIN_LENGTH characters long"
+	    );
 	    $pwe1->grab_focus();
 	    return;
 	}
-- 
2.47.0



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

[pve-devel] [PATCH installer v2 2/5] tui: raise minimum root password length to 8 characters

[Christoph Heiss]

.. in accordance with current NIST recommendations [0].

It's 2024; so reasonable to expect an 8-character-password at the
minimum.

[0] https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * rebased on latest master
  * added note about requirement in the UI

 proxmox-installer-common/src/lib.rs |  3 +++
 proxmox-tui-installer/src/main.rs   | 10 +++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index 13acb89..3dc3bfb 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -17,3 +17,6 @@ pub const FIRST_BOOT_EXEC_NAME: &str = "proxmox-first-boot";
 
 /// Maximum file size for the first-boot hook executable.
 pub const FIRST_BOOT_EXEC_MAX_SIZE: usize = 1024 * 1024; // 1 MiB
+
+/// Minimum length for the root password
+pub const ROOT_PASSWORD_MIN_LENGTH: usize = 8;
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 4385713..b33b6f7 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -20,6 +20,7 @@ use proxmox_installer_common::{
     options::{email_validate, BootdiskOptions, NetworkOptions, TimezoneOptions},
     setup::{installer_setup, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo},
     utils::Fqdn,
+    ROOT_PASSWORD_MIN_LENGTH,
 };
 
 mod setup;
@@ -422,7 +423,10 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
     let options = &state.options.password;
 
     let inner = FormView::new()
-        .child("Root password", EditView::new().secret())
+        .child(
+            "Root password [at least 8 characters]",
+            EditView::new().secret(),
+        )
         .child("Confirm root password", EditView::new().secret())
         .child(
             "Administrator email",
@@ -447,8 +451,8 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
                     .get_value::<EditView, _>(2)
                     .ok_or("failed to retrieve email")?;
 
-                if root_password.len() < 5 {
-                    Err("password too short, must be at least 5 characters long".to_owned())
+                if root_password.len() < ROOT_PASSWORD_MIN_LENGTH {
+                    Err(format!("password too short, must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long"))
                 } else if root_password != confirm_password {
                     Err("passwords do not match".to_owned())
                 } else if let Err(err) = email_validate(&email) {
-- 
2.47.0



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

[pve-devel] [PATCH installer v2 3/5] common: convert `InstallRootPassword` into an enum

[Christoph Heiss]

It's more appropriate for that type of data, since only one of both
variants is ever allowed to be set. Makes it also a bit more ergonomic
to handle.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * new patch

 proxmox-auto-installer/src/utils.rs   | 15 +++++++++++----
 proxmox-installer-common/src/setup.rs |  9 ++++-----
 proxmox-tui-installer/src/setup.rs    |  5 +----
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 27bbc3b..3378bdd 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -368,6 +368,16 @@ pub fn parse_answer(
     verify_email_and_root_password_settings(answer)?;
     verify_first_boot_settings(answer)?;
 
+    let root_password = match (
+        &answer.global.root_password,
+        &answer.global.root_password_hashed,
+    ) {
+        (Some(password), None) => InstallRootPassword::Plain(password.to_owned()),
+        (None, Some(hashed)) => InstallRootPassword::Hashed(hashed.to_owned()),
+        // Make the compiler happy, won't be reached anyway due to above checks
+        _ => bail!("invalid root password setting"),
+    };
+
     let mut config = InstallConfig {
         autoreboot: 1_usize,
         filesys: filesystem,
@@ -386,10 +396,7 @@ pub fn parse_answer(
         timezone: answer.global.timezone.clone(),
         keymap: answer.global.keyboard.to_string(),
 
-        root_password: InstallRootPassword {
-            plain: answer.global.root_password.clone(),
-            hashed: answer.global.root_password_hashed.clone(),
-        },
+        root_password,
         mailto: answer.global.mailto.clone(),
         root_ssh_keys: answer.global.root_ssh_keys.clone(),
 
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 4b4920e..4adb168 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -464,11 +464,10 @@ impl Interface {
 }
 
 #[derive(Clone, Deserialize, Serialize)]
-pub struct InstallRootPassword {
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub plain: Option<String>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub hashed: Option<String>,
+#[serde(rename_all = "lowercase")]
+pub enum InstallRootPassword {
+    Plain(String),
+    Hashed(String),
 }
 
 #[derive(Clone, Default, Deserialize, Serialize)]
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index b2a3511..b90c7dc 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -27,10 +27,7 @@ impl From<InstallerOptions> for InstallConfig {
             timezone: options.timezone.timezone,
             keymap: options.timezone.kb_layout,
 
-            root_password: InstallRootPassword {
-                plain: Some(options.password.root_password),
-                hashed: None,
-            },
+            root_password: InstallRootPassword::Plain(options.password.root_password),
             mailto: options.password.email,
             root_ssh_keys: vec![],
 
-- 
2.47.0



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

[pve-devel] [PATCH installer v2 4/5] auto: raise minimum root password length to 8 characters

[Christoph Heiss]

.. in accordance with current NIST recommendations [0].

It's 2024; so reasonable to expect an 8-character-password at the
minimum.

While at it, refactor the `InstallRootPassword` struct into an enum, as
suggested by Stefan.

[0] https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * reworked check to use a `match` instead, as suggested by Stefan

 proxmox-auto-installer/src/utils.rs           | 22 +++++++++++++------
 .../tests/resources/parse_answer/btrfs.json   |  2 +-
 .../tests/resources/parse_answer/btrfs.toml   |  2 +-
 .../resources/parse_answer/disk_match.json    |  2 +-
 .../resources/parse_answer/disk_match.toml    |  2 +-
 .../parse_answer/disk_match_all.json          |  2 +-
 .../parse_answer/disk_match_all.toml          |  2 +-
 .../parse_answer/disk_match_any.json          |  2 +-
 .../parse_answer/disk_match_any.toml          |  2 +-
 .../resources/parse_answer/first-boot.json    |  2 +-
 .../resources/parse_answer/first-boot.toml    |  2 +-
 .../tests/resources/parse_answer/minimal.json |  2 +-
 .../tests/resources/parse_answer/minimal.toml |  2 +-
 .../resources/parse_answer/nic_matching.json  |  2 +-
 .../resources/parse_answer/nic_matching.toml  |  2 +-
 .../resources/parse_answer/specific_nic.json  |  2 +-
 .../resources/parse_answer/specific_nic.toml  |  2 +-
 .../tests/resources/parse_answer/zfs.json     |  2 +-
 .../tests/resources/parse_answer/zfs.toml     |  2 +-
 19 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 3378bdd..5dc7979 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -14,6 +14,7 @@ use proxmox_installer_common::{
         InstallBtrfsOption, InstallConfig, InstallFirstBootSetup, InstallRootPassword,
         InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo,
     },
+    ROOT_PASSWORD_MIN_LENGTH,
 };
 use serde::{Deserialize, Serialize};
 
@@ -325,13 +326,20 @@ fn verify_email_and_root_password_settings(answer: &Answer) -> Result<()> {
 
     email_validate(&answer.global.mailto).with_context(|| answer.global.mailto.clone())?;
 
-    if answer.global.root_password.is_some() && answer.global.root_password_hashed.is_some() {
-        bail!("`global.root_password` and `global.root_password_hashed` cannot be set at the same time");
-    } else if answer.global.root_password.is_none() && answer.global.root_password_hashed.is_none()
-    {
-        bail!("One of `global.root_password` or `global.root_password_hashed` must be set");
-    } else {
-        Ok(())
+    match (
+        &answer.global.root_password,
+        &answer.global.root_password_hashed,
+    ) {
+        (Some(_), Some(_)) => {
+            bail!("`global.root_password` and `global.root_password_hashed` cannot be set at the same time");
+        }
+        (None, None) => {
+            bail!("One of `global.root_password` or `global.root_password_hashed` must be set");
+        }
+        (Some(password), None) if password.len() < ROOT_PASSWORD_MIN_LENGTH => {
+            bail!("`global.root_password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long");
+        }
+        _ => Ok(()),
     }
 }
 
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json
index de4c6e5..0c1f032 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.json
@@ -16,7 +16,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "eno1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "timezone": "Europe/Vienna",
   "btrfs_opts": {
     "compress": "zlib"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
index 8fcd27d..9071f7f 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/btrfs.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-dhcp"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json
index 48a82e6..d5ffddd 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.json
@@ -18,7 +18,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "eno1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml
index 68676ac..5177eb2 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-dhcp"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json
index f012eb1..78a5e0c 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json
@@ -15,7 +15,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "eno1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml
index f20a4fe..60daa54 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-dhcp"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json
index ad3e304..2e65fce 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json
@@ -22,7 +22,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "eno1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml
index e1f33c9..6e45c5b 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-dhcp"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/first-boot.json b/proxmox-auto-installer/tests/resources/parse_answer/first-boot.json
index ff3f859..fafde51 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/first-boot.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/first-boot.json
@@ -12,7 +12,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "eno1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "target_hd": "/dev/sda",
   "timezone": "Europe/Vienna",
   "first_boot": { "enabled": 1, "ordering_target": "network-pre" }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/first-boot.toml b/proxmox-auto-installer/tests/resources/parse_answer/first-boot.toml
index 75c6a5d..720cd9c 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/first-boot.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/first-boot.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [first-boot]
 source = "from-iso"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/minimal.json b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
index 62b45c9..0339dbc 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
@@ -12,7 +12,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "eno1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "target_hd": "/dev/sda",
   "timezone": "Europe/Vienna",
   "first_boot": { "enabled": 0 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml b/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml
index db8fec4..16f355c 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/minimal.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-dhcp"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json
index e8b5424..5d707c4 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json
@@ -12,7 +12,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "enp65s0f0",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "target_hd": "/dev/sda",
   "timezone": "Europe/Vienna",
   "first_boot": { "enabled": 0 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml
index 087c37f..eb6130a 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-answer"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json
index a5a4e0b..49240b4 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json
@@ -12,7 +12,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "enp129s0f1np1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "target_hd": "/dev/sda",
   "timezone": "Europe/Vienna",
   "first_boot": { "enabled": 0 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml
index 60f7f14..4ea49bc 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-answer"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/zfs.json b/proxmox-auto-installer/tests/resources/parse_answer/zfs.json
index 090b58d..622f6d6 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/zfs.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer/zfs.json
@@ -16,7 +16,7 @@
   "keymap": "de",
   "mailto": "mail@no.invalid",
   "mngmt_nic": "eno1",
-  "root_password": { "plain": "123456" },
+  "root_password": { "plain": "12345678" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml b/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml
index 4d48998..369fd63 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/zfs.toml
@@ -4,7 +4,7 @@ country = "at"
 fqdn = "pveauto.testinstall"
 mailto = "mail@no.invalid"
 timezone = "Europe/Vienna"
-root_password = "123456"
+root_password = "12345678"
 
 [network]
 source = "from-dhcp"
-- 
2.47.0



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

[pve-devel] [PATCH installer v2 5/5] auto: add negative tests for root password option

[Christoph Heiss]

Extends our "test runner" for the parse-answer tests such that if a test
file ends with ".fail.toml", it is considered a negative test and
expected to fail. The expected error message is stored in the
accompanying <name>.fail.json file.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * new patch

 proxmox-auto-installer/tests/parse-answer.rs  | 22 ++++++++++++++++---
 .../both-password-and-hashed-set.fail.json    |  3 +++
 .../both-password-and-hashed-set.fail.toml    | 15 +++++++++++++
 .../no-root-password-set.fail.json            |  3 +++
 .../no-root-password-set.fail.toml            | 13 +++++++++++
 .../parse_answer/short-password.fail.json     |  3 +++
 .../parse_answer/short-password.fail.toml     | 14 ++++++++++++
 proxmox-installer-common/src/setup.rs         |  6 ++---
 8 files changed, 73 insertions(+), 6 deletions(-)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.toml
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.toml
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.toml

diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 65f8c1e..57436a2 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -64,11 +64,27 @@ fn test_parse_answers() {
         let extension = p.extension().unwrap().to_str().unwrap();
         if extension == "toml" {
             println!("Test: {name}");
+
             let answer = get_answer(p.clone()).unwrap();
-            let config =
-                &parse_answer(&answer, &udev_info, &runtime_info, &locales, &setup_info).unwrap();
+            let config = parse_answer(&answer, &udev_info, &runtime_info, &locales, &setup_info);
+
+            let config = if name.ends_with(".fail") {
+                let json_path = tests_path.join(format!("{name}.json"));
+                let json_raw = std::fs::read_to_string(json_path).unwrap();
+                let err_json: Value = serde_json::from_str(&json_raw).unwrap();
+
+                assert!(config.is_err());
+                assert_eq!(
+                    config.unwrap_err().to_string(),
+                    err_json.get("error").unwrap().as_str().unwrap()
+                );
+                continue;
+            } else {
+                config.unwrap()
+            };
+
             println!("Selected disks: {:#?}", &config.disk_selection);
-            let config_json = serde_json::to_string(config);
+            let config_json = serde_json::to_string(&config);
             let config: Value = serde_json::from_str(config_json.unwrap().as_str()).unwrap();
             let mut path = tests_path.clone();
             path.push(format!("{name}.json"));
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.json b/proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.json
new file mode 100644
index 0000000..fd1213e
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.json
@@ -0,0 +1,3 @@
+{
+  "error": "`global.root_password` and `global.root_password_hashed` cannot be set at the same time"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.toml b/proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.toml
new file mode 100644
index 0000000..0a56fc9
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/both-password-and-hashed-set.fail.toml
@@ -0,0 +1,15 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "both-password-and-hashed-set.fail.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root_password = "12345678"
+root_password_hashed = "$y$j9T$343s9MNhV4xZhW1Be6J6H1$rIxofnXWmp0FQGGIPO3BRwb1jK4ZXWaxT7OjhHJmum0"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "ext4"
+disk_list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.json b/proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.json
new file mode 100644
index 0000000..6d75755
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.json
@@ -0,0 +1,3 @@
+{
+  "error": "One of `global.root_password` or `global.root_password_hashed` must be set"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.toml b/proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.toml
new file mode 100644
index 0000000..454e0b6
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/no-root-password-set.fail.toml
@@ -0,0 +1,13 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "no-root-password-set.fail.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "ext4"
+disk_list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.json b/proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.json
new file mode 100644
index 0000000..c424b0b
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.json
@@ -0,0 +1,3 @@
+{
+    "error": "`global.root_password` must be at least 8 characters long"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.toml b/proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.toml
new file mode 100644
index 0000000..a0eb1ec
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/short-password.fail.toml
@@ -0,0 +1,14 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "short-password.fail.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root_password = "12345"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "ext4"
+disk_list = ["sda"]
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 4adb168..c8fc118 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -463,14 +463,14 @@ impl Interface {
     }
 }
 
-#[derive(Clone, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize)]
 #[serde(rename_all = "lowercase")]
 pub enum InstallRootPassword {
     Plain(String),
     Hashed(String),
 }
 
-#[derive(Clone, Default, Deserialize, Serialize)]
+#[derive(Clone, Debug, Default, Deserialize, Serialize)]
 pub struct InstallFirstBootSetup {
     #[serde(
         serialize_with = "serialize_bool_as_u32",
@@ -501,7 +501,7 @@ pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child>
 }
 
 /// See Proxmox::Install::Config
-#[derive(Deserialize, Serialize)]
+#[derive(Debug, Deserialize, Serialize)]
 pub struct InstallConfig {
     pub autoreboot: usize,
 
-- 
2.47.0



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

[pve-devel] [PATCH pve-docs v2 6/8] installation: adapt to raised root password length requirement

[Christoph Heiss]

It's been raised in the installer across the board, so adapt it here
too.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * new patch

 pve-installation.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pve-installation.adoc b/pve-installation.adoc
index 869a879..0ffe3ae 100644
--- a/pve-installation.adoc
+++ b/pve-installation.adoc
@@ -175,7 +175,7 @@ not commonly used in your country.
 [thumbnail="screenshot/pve-set-password.png", float="left"]
 
 Next the password of the superuser (`root`) and an email address needs to be
-specified. The password must consist of at least 5 characters. It's highly
+specified. The password must consist of at least 8 characters. It's highly
 recommended to use a stronger password. Some guidelines are:
 
 - Use a minimum password length of at least 12 characters.
-- 
2.47.0



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

[pve-devel] [PATCH pmg-docs v2 7/8] installation: adapt to raised root password length requirement

[Christoph Heiss]

It's been raised in the installer across the board, so adapt it here
too.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * new patch

 pmg-installation.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pmg-installation.adoc b/pmg-installation.adoc
index 4209784..484b4ac 100644
--- a/pmg-installation.adoc
+++ b/pmg-installation.adoc
@@ -150,7 +150,7 @@ use a keyboard layout not commonly used in your country.
 [thumbnail="installer/pmg-set-password.png", float="left"]
 
 You then need to specify an email address and the superuser (root)
-password. The password must have at least 5 characters, but we highly
+password. The password must have at least 8 characters, but we highly
 recommend to use stronger passwords - here are some guidelines:
 
 - Use a minimum password length of at least 12 characters.
-- 
2.47.0



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

[pve-devel] [PATCH proxmox-backup v2 8/8] using-the-installer: adapt to raised root password length requirement

[Christoph Heiss]

It's been raised in the installer across the board, so adapt it here
too.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * new patch

 docs/using-the-installer.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/using-the-installer.rst b/docs/using-the-installer.rst
index 85d7c75b..ae9ae515 100644
--- a/docs/using-the-installer.rst
+++ b/docs/using-the-installer.rst
@@ -152,7 +152,7 @@ not commonly used in your country.
   :alt: Proxmox Backup Server Installer - Password and email configuration
 
 Next the password of the superuser (``root``) and an email address needs to be
-specified. The password must consist of at least 5 characters. It's highly
+specified. The password must consist of at least 8 characters. It's highly
 recommended to use a stronger password. Some guidelines are:
 
 |
-- 
2.47.0



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

Re: [pve-devel] [PATCH installer/{pve, pmg}-docs/proxmox-backup v2 0/8] raise minimum root password length to 8 characters

[Christoph Heiss]

v2: https://lore.proxmox.com/pve-devel/20241216094114.476756-1-c.heiss@proxmox.com/

On Fri Nov 29, 2024 at 1:21 PM CET, Christoph Heiss wrote:
> [..]


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