* [pve-devel] [PATCH installer 1/1] assistant: validate: add verify-password option
@ 2025-08-29 18:26 Peter via pve-devel
0 siblings, 0 replies; 3+ messages in thread
From: Peter via pve-devel @ 2025-08-29 18:26 UTC (permalink / raw)
To: pve-devel; +Cc: Peter
[-- Attachment #1: Type: message/rfc822, Size: 10629 bytes --]
From: Peter <pjcreath+proxmox@gmail.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH installer 1/1] assistant: validate: add verify-password option
Date: Fri, 29 Aug 2025 14:26:03 -0400
Message-ID: <20250829182603.46493-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>
---
In preparing an answer file for auto-installation, I somehow mangled
the hashed root password, which I only discovered after performing
the automated installation.
This patch adds an option to the auto-install assistant that lets
the user verify the hash in the answer file by interactively typing
in the expected password and checking it against the hash.
I don't love that I had to add an unsafe call to crypt(), but there
isn't a Rust implementation of yescrypt. To minimize the impact
I wrapped the unsafe call in its own function.
This is my first submission to this mailing list. I've tried to
follow all of the guidelines in the developer documentation, so
please forgive any oversights and let me know if there's anything
I should do differently.
proxmox-auto-install-assistant/Cargo.toml | 2 +
proxmox-auto-install-assistant/src/main.rs | 51 +++++++++++++++++++++-
2 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml
index 9b4a9c4..8af7d9d 100644
--- a/proxmox-auto-install-assistant/Cargo.toml
+++ b/proxmox-auto-install-assistant/Cargo.toml
@@ -17,4 +17,6 @@ proxmox-installer-common = { workspace = true, features = [ "cli" ] }
serde_json.workspace = true
toml.workspace = true
+crypt-sys = "0.1"
glob = "0.3"
+rpassword = "7.2"
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index 5d6c1d5..88d0032 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -2,10 +2,13 @@
//! Additional uses are to validate the format of an answer file or to test match filters and print
//! information on the properties to match against for the current hardware.
-#![forbid(unsafe_code)]
+//#![deny(unsafe_code)]
use anyhow::{Context, Result, bail, format_err};
+use crypt_sys::crypt;
use glob::Pattern;
+use rpassword::prompt_password;
+use std::ffi::{CStr, CString};
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-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-password Interactively verify the hashed root password.
-h, --help Print this help
-V, --version Print version
"#,
@@ -545,6 +552,42 @@ fn validate_answer_file_keys(path: impl AsRef<Path> + fmt::Debug) -> Result<bool
}
}
+/// Wrapper around the unsafe system `crypt` library
+#[allow(unsafe_code)]
+fn system_crypt(password: &str, hash: &str) -> Result<bool> {
+ let password_c = CString::new(password.as_bytes()).unwrap();
+ let hash_c = CString::new(hash.as_bytes()).unwrap();
+
+ let hashed_attempt = unsafe {
+ let result_ptr = crypt(password_c.as_ptr(), hash_c.as_ptr());
+ if result_ptr.is_null() {
+ bail!("crypt() call failed");
+ }
+ // result_ptr is a statically allocated block in the library,
+ // so it doesn't need to be freed.
+ CStr::from_ptr(result_ptr).to_string_lossy().into_owned()
+ };
+
+ Ok(hashed_attempt == hash)
+}
+
+fn verify_hashed_password_interactive(answer: &Answer) -> Result<()> {
+ if let Some(hashed) = &answer.global.root_password_hashed {
+ println!("Verifying hashed root password.");
+ let password = prompt_password("Enter root password to verify: ")
+ .context("Failed to read password")?;
+
+ if system_crypt(&password, hashed)? {
+ println!("Password matches hashed password.");
+ Ok(())
+ } else {
+ bail!("Password does not match hashed password.");
+ }
+ } 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 +596,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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH installer 1/1] assistant: validate: add verify-password option
2025-09-01 10:09 ` Christoph Heiss
@ 2025-09-03 13:41 ` Peter via pve-devel
0 siblings, 0 replies; 3+ messages in thread
From: Peter via pve-devel @ 2025-09-03 13:41 UTC (permalink / raw)
To: pve-devel; +Cc: Peter
[-- Attachment #1: Type: message/rfc822, Size: 10438 bytes --]
From: Peter <pjcreath+proxmox@gmail.com>
To: pve-devel@lists.proxmox.com
Subject: Re: [PATCH installer 1/1] assistant: validate: add verify-password option
Date: Wed, 3 Sep 2025 09:41:28 -0400
Message-ID: <CADbnQj468=prtr7k3vsNjqdMwC4qybKOBV0XjBis2erq=r8m+w@mail.gmail.com>
Yes, I've signed the CLA. Thank you for your feedback, I'll work on a
revised patch!
On Mon, Sep 1, 2025 at 6:09 AM Christoph Heiss <c.heiss@proxmox.com> wrote:
> On Fri Aug 29, 2025 at 8:26 PM CEST, Peter wrote:
> > Adds an option to interactively verify the hashed root password in
> > the answer file, so that mistakes can be caught before installation.
>
> Sounds like a useful option to me!
>
> >
> > Signed-off-by: Peter <pjcreath+proxmox@gmail.com>
> > ---
> >
> > In preparing an answer file for auto-installation, I somehow mangled
> > the hashed root password, which I only discovered after performing
> > the automated installation.
> >
> > This patch adds an option to the auto-install assistant that lets
> > the user verify the hash in the answer file by interactively typing
> > in the expected password and checking it against the hash.
> >
> > I don't love that I had to add an unsafe call to crypt(), but there
> > isn't a Rust implementation of yescrypt. To minimize the impact
> > I wrapped the unsafe call in its own function.
> >
> > This is my first submission to this mailing list. I've tried to
> > follow all of the guidelines in the developer documentation, so
> > please forgive any oversights and let me know if there's anything
> > I should do differently.
>
> Did you sign our CLA [0]? Just to be sure we can accept it :)
>
> [0]
> https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright
>
> >
> > proxmox-auto-install-assistant/Cargo.toml | 2 +
> > proxmox-auto-install-assistant/src/main.rs | 51 +++++++++++++++++++++-
> > 2 files changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/proxmox-auto-install-assistant/Cargo.toml
> b/proxmox-auto-install-assistant/Cargo.toml
> > index 9b4a9c4..8af7d9d 100644
> > --- a/proxmox-auto-install-assistant/Cargo.toml
> > +++ b/proxmox-auto-install-assistant/Cargo.toml
> > @@ -17,4 +17,6 @@ proxmox-installer-common = { workspace = true,
> features = [ "cli" ] }
> > serde_json.workspace = true
> > toml.workspace = true
> >
> > +crypt-sys = "0.1"
>
> That crate is (currently) not packaged for Debian, so that would have to
> be done first.
>
> But fortunately we got that already properly & safely wrapped in our
> `proxmox-sys` [1] crate, including a function for verifying passwords.
> You can find it in proxmox-sys/src/crypt.rs, including some example
> usages in the unit tests.
>
> For the proxmox-sys crate you need to pull in our `devel` repository, as
> described in [2].
>
> [1] https://git.proxmox.com/?p=proxmox.git;a=tree;f=proxmox-sys
> [2]
> https://pve.proxmox.com/wiki/Developer_Documentation#Development_Package_Repository
>
> > glob = "0.3"
> > +rpassword = "7.2"
>
> For this we also already got some functionality in our `proxmox-sys`
> crate, see below for an example.
>
> > diff --git a/proxmox-auto-install-assistant/src/main.rs
> b/proxmox-auto-install-assistant/src/main.rs
> > index 5d6c1d5..88d0032 100644
> > --- a/proxmox-auto-install-assistant/src/main.rs
> > +++ b/proxmox-auto-install-assistant/src/main.rs
> [..]
> >
> > 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-password"),
>
> This should probably throw an error if stdin (at least) is not connected
> to an interactive terminal, this can be checked with:
>
> std::io::stdin().is_terminal()
>
> Also, IMO the option should be named `--verify-root-password`, to make
> its name a bit more precise.
>
> > // 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-password Interactively verify the hashed root password.
> > -h, --help Print this help
> > -V, --version Print version
> > "#,
> > @@ -545,6 +552,42 @@ 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 = prompt_password("Enter root password to verify:
> ")
> > + .context("Failed to read password")?;
>
> So this could be something like:
>
> use proxmox_sys::linux::tty;
> let password = tty::read_readpassword("Enter root password to verify: ")
> .context("Failed to read password")?;
>
> > +
> > + if system_crypt(&password, hashed)? {
> > + println!("Password matches hashed password.");
> > + Ok(())
> > + } else {
> > + bail!("Password does not match hashed password.");
> > + }
> > + } else {
> > + bail!("'root-password-hashed' not set in answer file, cannot
> verify.");
> > + }
> > +}
>
>
[-- 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH installer 1/1] assistant: validate: add verify-password option
[not found] <20250829182603.46493-1-pjcreath+proxmox@gmail.com>
@ 2025-09-01 10:09 ` Christoph Heiss
2025-09-03 13:41 ` Peter via pve-devel
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Heiss @ 2025-09-01 10:09 UTC (permalink / raw)
To: Peter; +Cc: pve-devel
On Fri Aug 29, 2025 at 8:26 PM CEST, Peter wrote:
> Adds an option to interactively verify the hashed root password in
> the answer file, so that mistakes can be caught before installation.
Sounds like a useful option to me!
>
> Signed-off-by: Peter <pjcreath+proxmox@gmail.com>
> ---
>
> In preparing an answer file for auto-installation, I somehow mangled
> the hashed root password, which I only discovered after performing
> the automated installation.
>
> This patch adds an option to the auto-install assistant that lets
> the user verify the hash in the answer file by interactively typing
> in the expected password and checking it against the hash.
>
> I don't love that I had to add an unsafe call to crypt(), but there
> isn't a Rust implementation of yescrypt. To minimize the impact
> I wrapped the unsafe call in its own function.
>
> This is my first submission to this mailing list. I've tried to
> follow all of the guidelines in the developer documentation, so
> please forgive any oversights and let me know if there's anything
> I should do differently.
Did you sign our CLA [0]? Just to be sure we can accept it :)
[0] https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright
>
> proxmox-auto-install-assistant/Cargo.toml | 2 +
> proxmox-auto-install-assistant/src/main.rs | 51 +++++++++++++++++++++-
> 2 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml
> index 9b4a9c4..8af7d9d 100644
> --- a/proxmox-auto-install-assistant/Cargo.toml
> +++ b/proxmox-auto-install-assistant/Cargo.toml
> @@ -17,4 +17,6 @@ proxmox-installer-common = { workspace = true, features = [ "cli" ] }
> serde_json.workspace = true
> toml.workspace = true
>
> +crypt-sys = "0.1"
That crate is (currently) not packaged for Debian, so that would have to
be done first.
But fortunately we got that already properly & safely wrapped in our
`proxmox-sys` [1] crate, including a function for verifying passwords.
You can find it in proxmox-sys/src/crypt.rs, including some example
usages in the unit tests.
For the proxmox-sys crate you need to pull in our `devel` repository, as
described in [2].
[1] https://git.proxmox.com/?p=proxmox.git;a=tree;f=proxmox-sys
[2] https://pve.proxmox.com/wiki/Developer_Documentation#Development_Package_Repository
> glob = "0.3"
> +rpassword = "7.2"
For this we also already got some functionality in our `proxmox-sys`
crate, see below for an example.
> diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
> index 5d6c1d5..88d0032 100644
> --- a/proxmox-auto-install-assistant/src/main.rs
> +++ b/proxmox-auto-install-assistant/src/main.rs
[..]
>
> 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-password"),
This should probably throw an error if stdin (at least) is not connected
to an interactive terminal, this can be checked with:
std::io::stdin().is_terminal()
Also, IMO the option should be named `--verify-root-password`, to make
its name a bit more precise.
> // 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-password Interactively verify the hashed root password.
> -h, --help Print this help
> -V, --version Print version
> "#,
> @@ -545,6 +552,42 @@ 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 = prompt_password("Enter root password to verify: ")
> + .context("Failed to read password")?;
So this could be something like:
use proxmox_sys::linux::tty;
let password = tty::read_readpassword("Enter root password to verify: ")
.context("Failed to read password")?;
> +
> + if system_crypt(&password, hashed)? {
> + println!("Password matches hashed password.");
> + Ok(())
> + } else {
> + bail!("Password does not match hashed password.");
> + }
> + } 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-03 13:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-29 18:26 [pve-devel] [PATCH installer 1/1] assistant: validate: add verify-password option Peter via pve-devel
[not found] <20250829182603.46493-1-pjcreath+proxmox@gmail.com>
2025-09-01 10:09 ` Christoph Heiss
2025-09-03 13:41 ` Peter via pve-devel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox