[pve-devel] [PATCH installer v2 0/6] auto-installer: add option for providing hashed root password

[pve-devel] [PATCH installer v2 0/6] auto-installer: add option for providing hashed root password

[Christoph Heiss]

This series adds a new answer option `global.root_password_hashed`
for the auto-installer, enabling administrators to specify the root
password of the new installation in a hashed format - as generated by
e.g. mkpasswd(1) - instead of plain-text.

Administrators/users might want to avoid passing along a plain-text
password with the different answer-fetching methods supported by the
auto-installer, for obvious reasons.

While this of course does not provide full security, sending a hashed
password might still be preferred by administrators over plain text.

Tested by installing using the GUI and TUI (to ensure no regressions
can happen) and using the auto-installer, once with `root_password` set
(again testing for potential regressions) and once with
`global.root_password_hashed` set instead, testing the new
functionality.

First two patches are small cleanups and may be applied independently.

v1: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063949.html

Notable changes v1 -> v2:
  * rebased on latest master
  * fixed rebase mistake
  * merged previous patch #4/#5 for consistency across crates
  * improved validation in auto-installer

Christoph Heiss (6):
  common: move `PasswordOptions` type to tui crate
  tui-installer: remove `Debug` implementation for password options
  low-level: change root password option to contain either plaintext or
    hash
  {auto,tui}-installer: adapt to new `root_password` plain/hashed setup
    option
  auto-installer: add new `global.root_password_hashed` answer option
  auto-installer: add test for hashed root password option

 Proxmox/Install.pm                            | 25 ++++++++++++++++---
 Proxmox/Install/Config.pm                     | 20 ++++++++++++---
 proxinstall                                   |  4 +--
 proxmox-auto-installer/src/answer.rs          |  3 ++-
 proxmox-auto-installer/src/utils.rs           | 21 ++++++++++++++--
 .../resources/parse_answer/disk_match.json    |  2 +-
 .../parse_answer/disk_match_all.json          |  2 +-
 .../parse_answer/disk_match_any.json          |  2 +-
 .../parse_answer/hashed_root_password.json    | 20 +++++++++++++++
 .../parse_answer/hashed_root_password.toml    | 14 +++++++++++
 .../tests/resources/parse_answer/minimal.json |  2 +-
 .../resources/parse_answer/nic_matching.json  |  2 +-
 .../resources/parse_answer/specific_nic.json  |  2 +-
 .../tests/resources/parse_answer/zfs.json     |  2 +-
 proxmox-installer-common/src/options.rs       | 15 -----------
 proxmox-installer-common/src/setup.rs         | 12 +++++++--
 proxmox-tui-installer/src/main.rs             |  4 +--
 proxmox-tui-installer/src/options.rs          | 20 ++++++++++++---
 proxmox-tui-installer/src/setup.rs            | 10 ++++++--
 19 files changed, 140 insertions(+), 42 deletions(-)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml

-- 
2.44.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/6] common: move `PasswordOptions` type to tui crate

[Christoph Heiss]

It's only used internally there anyway, so make it slightly less
confusing.

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

 proxmox-installer-common/src/options.rs | 15 ---------------
 proxmox-tui-installer/src/main.rs       |  4 ++--
 proxmox-tui-installer/src/options.rs    | 18 ++++++++++++++++--
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index e77914b..9375ded 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -327,21 +327,6 @@ impl TimezoneOptions {
     }
 }
 
-#[derive(Clone, Debug)]
-pub struct PasswordOptions {
-    pub email: String,
-    pub root_password: String,
-}
-
-impl Default for PasswordOptions {
-    fn default() -> Self {
-        Self {
-            email: "mail@example.invalid".to_string(),
-            root_password: String::new(),
-        }
-    }
-}
-
 #[derive(Clone, Debug, PartialEq)]
 pub struct NetworkOptions {
     pub ifname: String,
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 4fb7afd..dd600c6 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -16,10 +16,10 @@ use cursive::{
 use regex::Regex;
 
 mod options;
-use options::InstallerOptions;
+use options::{InstallerOptions, PasswordOptions};
 
 use proxmox_installer_common::{
-    options::{BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions},
+    options::{BootdiskOptions, NetworkOptions, TimezoneOptions},
     setup::{installer_setup, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo},
     utils::Fqdn,
 };
diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs
index 73fbf2a..6335762 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -2,8 +2,7 @@ use crate::SummaryOption;
 
 use proxmox_installer_common::{
     options::{
-        BootdiskOptions, BtrfsRaidLevel, FsType, NetworkOptions, PasswordOptions, TimezoneOptions,
-        ZfsRaidLevel,
+        BootdiskOptions, BtrfsRaidLevel, FsType, NetworkOptions, TimezoneOptions, ZfsRaidLevel,
     },
     setup::LocaleInfo,
 };
@@ -25,6 +24,21 @@ pub const FS_TYPES: &[FsType] = {
     ]
 };
 
+#[derive(Clone, Debug)]
+pub struct PasswordOptions {
+    pub email: String,
+    pub root_password: String,
+}
+
+impl Default for PasswordOptions {
+    fn default() -> Self {
+        Self {
+            email: "mail@example.invalid".to_string(),
+            root_password: String::new(),
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct InstallerOptions {
     pub bootdisk: BootdiskOptions,
-- 
2.45.1



_______________________________________________
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/6] tui-installer: remove `Debug` implementation for password options

[Christoph Heiss]

So we never accidentally show/log the password somewhere. Need to drop
it from `InstallerOptions` in turn too, but it's never used currently
anyway.

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

 proxmox-tui-installer/src/options.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs
index 6335762..19992ca 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -24,7 +24,7 @@ pub const FS_TYPES: &[FsType] = {
     ]
 };
 
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct PasswordOptions {
     pub email: String,
     pub root_password: String,
@@ -39,7 +39,7 @@ impl Default for PasswordOptions {
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct InstallerOptions {
     pub bootdisk: BootdiskOptions,
     pub timezone: TimezoneOptions,
-- 
2.45.1



_______________________________________________
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/6] low-level: change root password option to contain either plaintext or hash

[Christoph Heiss]

A hashed password can be created e.g. using the `mkpasswd(1)`.
This then will allow the auto-installer to pass along a
already-hashed password from the user, instead of simple plaintext.

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

 Proxmox/Install.pm        | 25 ++++++++++++++++++++++---
 Proxmox/Install/Config.pm | 20 +++++++++++++++++---
 proxinstall               |  4 ++--
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index c0f8955..bcf8ba7 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -621,6 +621,27 @@ sub prepare_grub_efi_boot_esp {
     die "failed to prepare EFI boot using Grub on '$espdev': $err" if $err;
 }
 
+my sub setup_root_password {
+    my ($targetdir) = @_;
+
+    my $plain = Proxmox::Install::Config::get_root_password('plain');
+    my $hashed = Proxmox::Install::Config::get_root_password('hashed');
+
+    die "root password must be set!\n"
+	if !defined($plain) && !defined($hashed);
+
+    die "plain and hashed root password cannot be set at the same time!\n"
+	if defined($plain) && defined($hashed);
+
+    if (defined($plain)) {
+	my $octets = encode("utf-8", $plain);
+	run_command("chroot $targetdir /usr/sbin/chpasswd", undef, "root:$octets\n");
+    } elsif (defined($hashed)) {
+	my $octets = encode("utf-8", $hashed);
+	run_command("chroot $targetdir /usr/sbin/chpasswd --encrypted", undef, "root:$octets\n");
+    }
+}
+
 sub extract_data {
     my $iso_env = Proxmox::Install::ISOEnv::get();
     my $run_env = Proxmox::Install::RunEnv::get();
@@ -1269,9 +1290,7 @@ _EOD
 
 	diversion_remove($targetdir, "/sbin/start-stop-daemon");
 
-	# set root password
-	my $octets = encode("utf-8", Proxmox::Install::Config::get_password());
-	run_command("chroot $targetdir /usr/sbin/chpasswd", undef, "root:$octets\n");
+	setup_root_password($targetdir);
 
 	# set root ssh keys
 	my $ssh_keys = Proxmox::Install::Config::get_root_ssh_keys();
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index ecd8a74..0313fd9 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -90,7 +90,7 @@ my sub init_cfg {
 	keymap => 'en-us',
 
 	# root credentials & details
-	password => undef,
+	root_password => undef,
 	mailto => 'mail@example.invalid',
 	root_ssh_keys => [],
 
@@ -196,8 +196,22 @@ sub get_timezone { return get('timezone'); }
 sub set_keymap { set_key('keymap', $_[0]); }
 sub get_keymap { return get('keymap'); }
 
-sub set_password { set_key('password', $_[0]); }
-sub get_password { return get('password'); }
+sub set_root_password {
+    my ($key) = @_;
+    croak "unknown root password option '$key'"
+	if $key ne 'plain' && $key ne 'hashed';
+
+    set_key('root_password', { $_[0] => $_[1] });
+}
+
+sub get_root_password {
+    my ($key) = @_;
+    croak "unknown root password option '$key'"
+	if $key ne 'plain' && $key ne 'hashed';
+
+    my $password = get('root_password');
+    return defined($password->{$key}) ? $password->{$key} : undef;
+}
 
 sub set_mailto { set_key('mailto', $_[0]); }
 sub get_mailto { return get('mailto'); }
diff --git a/proxinstall b/proxinstall
index a6a4cfb..12f3eaa 100755
--- a/proxinstall
+++ b/proxinstall
@@ -674,7 +674,7 @@ sub create_password_view {
 
     cleanup_view();
 
-    my $password = Proxmox::Install::Config::get_password();
+    my $password = Proxmox::Install::Config::get_root_password('plain');
 
     my $grid = &$create_basic_grid();
     $gtk_state->{inbox}->pack_start($grid, 0, 0, 0);
@@ -745,7 +745,7 @@ sub create_password_view {
 	    return;
 	}
 
-	Proxmox::Install::Config::set_password($t1);
+	Proxmox::Install::Config::set_root_password('plain', $t1);
 	Proxmox::Install::Config::set_mailto($t3);
 
 	$step_number++;
-- 
2.45.1



_______________________________________________
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/6] {auto, tui}-installer: adapt to new `root_password` plain/hashed setup option

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * previously split up into two separate patches between auto- and tui-
    installer, now merged together for consistency

 proxmox-auto-installer/src/utils.rs                  |  9 +++++++--
 .../tests/resources/parse_answer/disk_match.json     |  2 +-
 .../tests/resources/parse_answer/disk_match_all.json |  2 +-
 .../tests/resources/parse_answer/disk_match_any.json |  2 +-
 .../tests/resources/parse_answer/minimal.json        |  2 +-
 .../tests/resources/parse_answer/nic_matching.json   |  2 +-
 .../tests/resources/parse_answer/specific_nic.json   |  2 +-
 .../tests/resources/parse_answer/zfs.json            |  2 +-
 proxmox-installer-common/src/setup.rs                | 12 ++++++++++--
 proxmox-tui-installer/src/setup.rs                   | 10 ++++++++--
 10 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 202ad41..229b7e2 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -10,7 +10,9 @@ use crate::{
 };
 use proxmox_installer_common::{
     options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption},
-    setup::{InstallConfig, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo},
+    setup::{
+        InstallConfig, InstallRootPassword, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo,
+    },
 };
 use serde::{Deserialize, Serialize};
 
@@ -334,7 +336,10 @@ pub fn parse_answer(
         timezone: answer.global.timezone.clone(),
         keymap: answer.global.keyboard.to_string(),
 
-        password: answer.global.root_password.clone(),
+        root_password: InstallRootPassword {
+            plain: Some(answer.global.root_password.clone()),
+            hashed: None,
+        },
         mailto: answer.global.mailto.clone(),
         root_ssh_keys: answer.global.root_ssh_keys.clone(),
 
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 3a117b6..a0942cc 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",
-  "password": "123456",
+  "root_password": { "plain": "123456" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
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 5325fc3..a7324f1 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",
-  "password": "123456",
+  "root_password": { "plain": "123456" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
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 18e22d1..8e13496 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",
-  "password": "123456",
+  "root_password": { "plain": "123456" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/minimal.json b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
index bb72713..7a6bfd5 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",
-  "password": "123456",
+  "root_password": { "plain": "123456" },
   "target_hd": "/dev/sda",
   "timezone": "Europe/Vienna"
 }
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 de94165..e1c9d12 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",
-  "password": "123456",
+  "root_password": { "plain": "123456" },
   "target_hd": "/dev/sda",
   "timezone": "Europe/Vienna"
 }
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 5b4fcfc..1187eb4 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",
-  "password": "123456",
+  "root_password": { "plain": "123456" },
   "target_hd": "/dev/sda",
   "timezone": "Europe/Vienna"
 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/zfs.json b/proxmox-auto-installer/tests/resources/parse_answer/zfs.json
index 65724a8..8229a4b 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",
-  "password": "123456",
+  "root_password": { "plain": "123456" },
   "timezone": "Europe/Vienna",
   "zfs_opts": {
       "arc_max": 2048,
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 64d05af..ef92eb7 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -424,6 +424,14 @@ 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>,
+}
+
 pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child> {
     let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) = if test_mode {
         (
@@ -444,7 +452,7 @@ pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child>
 }
 
 /// See Proxmox::Install::Config
-#[derive(Debug, Deserialize, Serialize)]
+#[derive(Deserialize, Serialize)]
 pub struct InstallConfig {
     pub autoreboot: usize,
 
@@ -485,7 +493,7 @@ pub struct InstallConfig {
     pub timezone: String,
     pub keymap: String,
 
-    pub password: String,
+    pub root_password: InstallRootPassword,
     pub mailto: String,
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub root_ssh_keys: Vec<String>,
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index 8c01e42..ee6e65c 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -1,7 +1,10 @@
 use std::collections::BTreeMap;
 
 use crate::options::InstallerOptions;
-use proxmox_installer_common::{options::AdvancedBootdiskOptions, setup::InstallConfig};
+use proxmox_installer_common::{
+    options::AdvancedBootdiskOptions,
+    setup::{InstallConfig, InstallRootPassword},
+};
 
 impl From<InstallerOptions> for InstallConfig {
     fn from(options: InstallerOptions) -> Self {
@@ -23,7 +26,10 @@ impl From<InstallerOptions> for InstallConfig {
             timezone: options.timezone.timezone,
             keymap: options.timezone.kb_layout,
 
-            password: options.password.root_password,
+            root_password: InstallRootPassword {
+                plain: Some(options.password.root_password),
+                hashed: None,
+            },
             mailto: options.password.email,
             root_ssh_keys: vec![],
 
-- 
2.45.1



_______________________________________________
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/6] auto-installer: add new `global.root_password_hashed` answer option

[Christoph Heiss]

This allows user to specify the root password in a hashed format,
generated using e.g. mkpasswd(1), instead of plaintext.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * move root password setting validation into own function
  * explicitly check for case for both are unset

 proxmox-auto-installer/src/answer.rs |  3 ++-
 proxmox-auto-installer/src/utils.rs  | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index aab7198..d691da1 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -26,7 +26,8 @@ pub struct Global {
     pub keyboard: KeyboardLayout,
     pub mailto: String,
     pub timezone: String,
-    pub root_password: String,
+    pub root_password: Option<String>,
+    pub root_password_hashed: Option<String>,
     #[serde(default)]
     pub reboot_on_error: bool,
     #[serde(default)]
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 229b7e2..2500f43 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -303,6 +303,17 @@ pub fn verify_locale_settings(answer: &Answer, locales: &LocaleInfo) -> Result<(
     Ok(())
 }
 
+fn verify_root_password_settings(answer: &Answer) -> Result<()> {
+    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(())
+    }
+}
+
 pub fn parse_answer(
     answer: &Answer,
     udev_info: &UdevInfo,
@@ -318,6 +329,7 @@ pub fn parse_answer(
     let network_settings = get_network_settings(answer, udev_info, runtime_info, setup_info)?;
 
     verify_locale_settings(answer, locales)?;
+    verify_root_password_settings(answer)?;
 
     let mut config = InstallConfig {
         autoreboot: 1_usize,
@@ -337,8 +349,8 @@ pub fn parse_answer(
         keymap: answer.global.keyboard.to_string(),
 
         root_password: InstallRootPassword {
-            plain: Some(answer.global.root_password.clone()),
-            hashed: None,
+            plain: answer.global.root_password.clone(),
+            hashed: answer.global.root_password_hashed.clone(),
         },
         mailto: answer.global.mailto.clone(),
         root_ssh_keys: answer.global.root_ssh_keys.clone(),
-- 
2.45.1



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

[pve-devel] [PATCH installer v2 6/6] auto-installer: add test for hashed root password option

[Christoph Heiss]

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

 .../parse_answer/hashed_root_password.json    | 20 +++++++++++++++++++
 .../parse_answer/hashed_root_password.toml    | 14 +++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml

diff --git a/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json
new file mode 100644
index 0000000..19c51ba
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json
@@ -0,0 +1,20 @@
+{
+  "autoreboot": 1,
+  "cidr": "192.168.1.114/24",
+  "country": "at",
+  "dns": "192.168.1.254",
+  "domain": "testinstall",
+  "filesys": "ext4",
+  "gateway": "192.168.1.1",
+  "hdsize": 223.57088470458984,
+  "lvm_auto_rename": 1,
+  "hostname": "pveauto",
+  "keymap": "de",
+  "mailto": "mail@no.invalid",
+  "mngmt_nic": "eno1",
+  "root_password": {
+    "hashed": "$y$j9T$VgMv8lsz/TEvzesCZU3xD.$SK.h4QW51Jr/EmjuaTz5Bt4kYiX2Iezz6omzoqVEwj9"
+  },
+  "target_hd": "/dev/sda",
+  "timezone": "Europe/Vienna"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml
new file mode 100644
index 0000000..ed4d2e5
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml
@@ -0,0 +1,14 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root_password_hashed = "$y$j9T$VgMv8lsz/TEvzesCZU3xD.$SK.h4QW51Jr/EmjuaTz5Bt4kYiX2Iezz6omzoqVEwj9"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "ext4"
+disk_list = ["sda"]
-- 
2.45.1



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

Re: [pve-devel] [PATCH installer v2 0/6] auto-installer: add option for providing hashed root password

[Theodor Fumics via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 8892 bytes --]

From: Theodor Fumics <theodor.fumics@gmx.net>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>, Christoph Heiss <c.heiss@proxmox.com>
Subject: Re: [pve-devel] [PATCH installer v2 0/6] auto-installer: add option for providing hashed root password
Date: Tue, 16 Jul 2024 15:48:51 +0200
Message-ID: <d34b72ff-9e55-4ca4-8a05-36aaaf07b692@gmx.net>

I have setup the new functionality according to the instructions from
[1] and [2], and tested various hashed and non-hashed passwords. The
only potential improvement would be to check if the provided hash is
valid because passing an invalid hash makes it rather impossible to
login after the installation. While it's not possible to fully verify
the validity of a hash, you could verify if it has the correct length
and only consists of hexadecimal characters (0-9, A-F).

Other than that it works great.


[1] https://wiki.intra.proxmox.com/index.php/Testing_Installer_Changes

[2] https://pve.proxmox.com/wiki/Automated_Installation

On 7/15/24 09:56, Christoph Heiss wrote:
> This series adds a new answer option `global.root_password_hashed`
> for the auto-installer, enabling administrators to specify the root
> password of the new installation in a hashed format - as generated by
> e.g. mkpasswd(1) - instead of plain-text.
>
> Administrators/users might want to avoid passing along a plain-text
> password with the different answer-fetching methods supported by the
> auto-installer, for obvious reasons.
>
> While this of course does not provide full security, sending a hashed
> password might still be preferred by administrators over plain text.
>
> Tested by installing using the GUI and TUI (to ensure no regressions
> can happen) and using the auto-installer, once with `root_password` set
> (again testing for potential regressions) and once with
> `global.root_password_hashed` set instead, testing the new
> functionality.
>
> First two patches are small cleanups and may be applied independently.
>
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063949.html
>
> Notable changes v1 -> v2:
>    * rebased on latest master
>    * fixed rebase mistake
>    * merged previous patch #4/#5 for consistency across crates
>    * improved validation in auto-installer
>
> Christoph Heiss (6):
>    common: move `PasswordOptions` type to tui crate
>    tui-installer: remove `Debug` implementation for password options
>    low-level: change root password option to contain either plaintext or
>      hash
>    {auto,tui}-installer: adapt to new `root_password` plain/hashed setup
>      option
>    auto-installer: add new `global.root_password_hashed` answer option
>    auto-installer: add test for hashed root password option
>
>   Proxmox/Install.pm                            | 25 ++++++++++++++++---
>   Proxmox/Install/Config.pm                     | 20 ++++++++++++---
>   proxinstall                                   |  4 +--
>   proxmox-auto-installer/src/answer.rs          |  3 ++-
>   proxmox-auto-installer/src/utils.rs           | 21 ++++++++++++++--
>   .../resources/parse_answer/disk_match.json    |  2 +-
>   .../parse_answer/disk_match_all.json          |  2 +-
>   .../parse_answer/disk_match_any.json          |  2 +-
>   .../parse_answer/hashed_root_password.json    | 20 +++++++++++++++
>   .../parse_answer/hashed_root_password.toml    | 14 +++++++++++
>   .../tests/resources/parse_answer/minimal.json |  2 +-
>   .../resources/parse_answer/nic_matching.json  |  2 +-
>   .../resources/parse_answer/specific_nic.json  |  2 +-
>   .../tests/resources/parse_answer/zfs.json     |  2 +-
>   proxmox-installer-common/src/options.rs       | 15 -----------
>   proxmox-installer-common/src/setup.rs         | 12 +++++++--
>   proxmox-tui-installer/src/main.rs             |  4 +--
>   proxmox-tui-installer/src/options.rs          | 20 ++++++++++++---
>   proxmox-tui-installer/src/setup.rs            | 10 ++++++--
>   19 files changed, 140 insertions(+), 42 deletions(-)
>   create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json
>   create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml
>


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[pve-devel] applied-series: [PATCH installer v2 0/6] auto-installer: add option for providing hashed root password

[Thomas Lamprecht]

Am 15/07/2024 um 09:56 schrieb Christoph Heiss:
> This series adds a new answer option `global.root_password_hashed`
> for the auto-installer, enabling administrators to specify the root
> password of the new installation in a hashed format - as generated by
> e.g. mkpasswd(1) - instead of plain-text.
> 
> Administrators/users might want to avoid passing along a plain-text
> password with the different answer-fetching methods supported by the
> auto-installer, for obvious reasons.
> 
> While this of course does not provide full security, sending a hashed
> password might still be preferred by administrators over plain text.
> 
> Tested by installing using the GUI and TUI (to ensure no regressions
> can happen) and using the auto-installer, once with `root_password` set
> (again testing for potential regressions) and once with
> `global.root_password_hashed` set instead, testing the new
> functionality.
> 
> First two patches are small cleanups and may be applied independently.
> 
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-May/063949.html
> 
> Notable changes v1 -> v2:
>   * rebased on latest master
>   * fixed rebase mistake
>   * merged previous patch #4/#5 for consistency across crates
>   * improved validation in auto-installer
> 
> Christoph Heiss (6):
>   common: move `PasswordOptions` type to tui crate
>   tui-installer: remove `Debug` implementation for password options
>   low-level: change root password option to contain either plaintext or
>     hash
>   {auto,tui}-installer: adapt to new `root_password` plain/hashed setup
>     option
>   auto-installer: add new `global.root_password_hashed` answer option
>   auto-installer: add test for hashed root password option
> 
>  Proxmox/Install.pm                            | 25 ++++++++++++++++---
>  Proxmox/Install/Config.pm                     | 20 ++++++++++++---
>  proxinstall                                   |  4 +--
>  proxmox-auto-installer/src/answer.rs          |  3 ++-
>  proxmox-auto-installer/src/utils.rs           | 21 ++++++++++++++--
>  .../resources/parse_answer/disk_match.json    |  2 +-
>  .../parse_answer/disk_match_all.json          |  2 +-
>  .../parse_answer/disk_match_any.json          |  2 +-
>  .../parse_answer/hashed_root_password.json    | 20 +++++++++++++++
>  .../parse_answer/hashed_root_password.toml    | 14 +++++++++++
>  .../tests/resources/parse_answer/minimal.json |  2 +-
>  .../resources/parse_answer/nic_matching.json  |  2 +-
>  .../resources/parse_answer/specific_nic.json  |  2 +-
>  .../tests/resources/parse_answer/zfs.json     |  2 +-
>  proxmox-installer-common/src/options.rs       | 15 -----------
>  proxmox-installer-common/src/setup.rs         | 12 +++++++--
>  proxmox-tui-installer/src/main.rs             |  4 +--
>  proxmox-tui-installer/src/options.rs          | 20 ++++++++++++---
>  proxmox-tui-installer/src/setup.rs            | 10 ++++++--
>  19 files changed, 140 insertions(+), 42 deletions(-)
>  create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.json
>  create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/hashed_root_password.toml
> 


applied series while taking the freedom to record Theodor's feedback
in form of a T-b tag, thanks!


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