[pve-devel] [PATCH installer v2 1/1] assistant: validate: add verify-password option
Christoph Heiss
c.heiss at proxmox.com
Tue Sep 9 13:56:20 CEST 2025
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.");
> + }
> +}
> +
More information about the pve-devel
mailing list