Re: [pve-devel] [PATCH installer v2 1/1] assistant: validate: add verify-password option

Re: [pve-devel] [PATCH installer v2 1/1] assistant: validate: add verify-password option

[Christoph Heiss]

Looks good overall, just some small nits inline :)

On Thu Sep 4, 2025 at 1:18 AM CEST, Peter wrote:
> [..]
> @@ -17,4 +17,5 @@ proxmox-installer-common = { workspace = true, features = [ "cli" ] }
>  serde_json.workspace = true
>  toml.workspace = true
>
> +proxmox-sys = { version = "1.0.0", features = [ "crypt" ] }

Forgot to mention on v1, but new dependencies must also be recorded in
debian/control.

You can use the command

  debcargo deb-dependencies proxmox-auto-install-assistant/Cargo.toml

to automatically generate that list (`debcargo` is available through the
normal Debian repositories) and afterwards

  wrap-and-sort -tkn

to sort that list.

>  glob = "0.3"
> diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
> index 5d6c1d5..98b4f23 100644
> --- a/proxmox-auto-install-assistant/src/main.rs
> +++ b/proxmox-auto-install-assistant/src/main.rs
> @@ -6,6 +6,9 @@
>
>  use anyhow::{Context, Result, bail, format_err};
>  use glob::Pattern;
> +use proxmox_sys::linux::tty::read_password;
> +use proxmox_sys::crypt::verify_crypt_pw;

These two lines should be alphabetically sorted - you can just run

  cargo fmt

before sending a patch, that will take care of all of that.

Personally I'd combine them:

use proxmox_sys::{linux::tty::read_password, crypt::verify_crypt_pw};

> [..]
>  impl cli::Subcommand for CommandValidateAnswerArgs {
>      fn parse(args: &mut cli::Arguments) -> Result<Self> {
>          Ok(Self {
>              debug: args.contains(["-d", "--debug"]),
> +            verify_password: args.contains("--verify-root-password"),
>              // Needs to be last
>              path: args.free_from_str()?,
>          })
> @@ -176,6 +182,7 @@ ARGUMENTS:
>
>  OPTIONS:
>    -d, --debug        Also show the full answer as parsed.
> +      --verify-root-password  Interactively verify the hashed root password.
>    -h, --help         Print this help
>    -V, --version      Print version

Please align all the descriptions.

> [..]
> @@ -545,6 +556,20 @@ fn validate_answer_file_keys(path: impl AsRef<Path> + fmt::Debug) -> Result<bool
>      }
>  }
>
> +fn verify_hashed_password_interactive(answer: &Answer) -> Result<()> {
> +    if let Some(hashed) = &answer.global.root_password_hashed {
> +        println!("Verifying hashed root password.");
> +
> +        let password = String::from_utf8(read_password("Enter root password to verify: ")?)?;
> +        verify_crypt_pw(&password, hashed)?;

verify_crypt_pw(&password, hashed).context("Failed to verify hashed root password")?;

Makes the output just a bit nicer when verification fails.

> +
> +        println!("Password matches hashed password.");

println!("Password matches hashed root password.");

For consistency with the other messages.

> +        Ok(())
> +    } else {
> +        bail!("'root-password-hashed' not set in answer file, cannot verify.");
> +    }
> +}
> +


_______________________________________________
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/1] assistant: validate: add verify-password option

[Peter via pve-devel]

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

From: Peter <pjcreath+proxmox@gmail.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH installer v2 1/1] assistant: validate: add verify-password option
Date: Wed,  3 Sep 2025 19:18:28 -0400
Message-ID: <20250903231828.53459-1-pjcreath+proxmox@gmail.com>

Adds an option to interactively verify the hashed root password in
the answer file, so that mistakes can be caught before installation.

Signed-off-by: Peter <pjcreath+proxmox@gmail.com>
---

 changes since v1:
 * revised code to depend on proxmox crates instead of external crypt and rpassword
 * changed flag to --verify-root-password
 * added a check that stdin is a tty before prompting for password

 proxmox-auto-install-assistant/Cargo.toml  |  1 +
 proxmox-auto-install-assistant/src/main.rs | 31 ++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml
index 9b4a9c4..eeba42f 100644
--- a/proxmox-auto-install-assistant/Cargo.toml
+++ b/proxmox-auto-install-assistant/Cargo.toml
@@ -17,4 +17,5 @@ proxmox-installer-common = { workspace = true, features = [ "cli" ] }
 serde_json.workspace = true
 toml.workspace = true
 
+proxmox-sys = { version = "1.0.0", features = [ "crypt" ] }
 glob = "0.3"
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index 5d6c1d5..98b4f23 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -6,6 +6,9 @@
 
 use anyhow::{Context, Result, bail, format_err};
 use glob::Pattern;
+use proxmox_sys::linux::tty::read_password;
+use proxmox_sys::crypt::verify_crypt_pw;
+use std::io::IsTerminal;
 use std::{
     collections::BTreeMap,
     fmt, fs,
@@ -153,12 +156,15 @@ struct CommandValidateAnswerArgs {
     path: PathBuf,
     /// Whether to also show the full answer as parsed.
     debug: bool,
+    /// Interactively verify the hashed root password.
+    verify_password: bool,
 }
 
 impl cli::Subcommand for CommandValidateAnswerArgs {
     fn parse(args: &mut cli::Arguments) -> Result<Self> {
         Ok(Self {
             debug: args.contains(["-d", "--debug"]),
+            verify_password: args.contains("--verify-root-password"),
             // Needs to be last
             path: args.free_from_str()?,
         })
@@ -176,6 +182,7 @@ ARGUMENTS:
 
 OPTIONS:
   -d, --debug        Also show the full answer as parsed.
+      --verify-root-password  Interactively verify the hashed root password.
   -h, --help         Print this help
   -V, --version      Print version
     "#,
@@ -184,6 +191,10 @@ OPTIONS:
     }
 
     fn run(&self) -> Result<()> {
+        if self.verify_password && !std::io::stdin().is_terminal() {
+            Self::print_usage();
+            bail!("Verifying the root password requires an interactive terminal.");
+        }
         validate_answer(self)
     }
 }
@@ -545,6 +556,20 @@ fn validate_answer_file_keys(path: impl AsRef<Path> + fmt::Debug) -> Result<bool
     }
 }
 
+fn verify_hashed_password_interactive(answer: &Answer) -> Result<()> {
+    if let Some(hashed) = &answer.global.root_password_hashed {
+        println!("Verifying hashed root password.");
+
+        let password = String::from_utf8(read_password("Enter root password to verify: ")?)?;
+        verify_crypt_pw(&password, hashed)?;
+
+        println!("Password matches hashed password.");
+        Ok(())
+    } else {
+        bail!("'root-password-hashed' not set in answer file, cannot verify.");
+    }
+}
+
 fn validate_answer(args: &CommandValidateAnswerArgs) -> Result<()> {
     let mut valid = validate_answer_file_keys(&args.path)?;
 
@@ -553,6 +578,12 @@ fn validate_answer(args: &CommandValidateAnswerArgs) -> Result<()> {
             if args.debug {
                 println!("Parsed data from answer file:\n{:#?}", answer);
             }
+            if args.verify_password {
+                if let Err(err) = verify_hashed_password_interactive(&answer) {
+                    eprintln!("{err:#}");
+                    valid = false;
+                }
+            }
         }
         Err(err) => {
             eprintln!("{err:#}");
-- 
2.47.2



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