* [pve-devel] [PATCH installer 0/3] raise minimum root password length to 8 characters
@ 2024-10-07 9:22 Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 1/3] proxinstall: " Christoph Heiss
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Christoph Heiss @ 2024-10-07 9:22 UTC (permalink / raw)
To: pve-devel
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].
In the GUI installer, we already recommend passwords of length 8 or
greater during the respective installation step.
See also the respective patches for PVE [2] and PBS [3].
[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/
Christoph Heiss (3):
proxinstall: raise minimum root password length to 8 characters
tui: raise minimum root password length to 8 characters
auto-installer: raise minimum root password length to 8 characters
Proxmox/Makefile | 1 +
Proxmox/Sys.pm | 6 ++++++
proxinstall | 8 ++++++--
proxmox-auto-installer/src/utils.rs | 5 +++++
.../tests/resources/parse_answer/disk_match.json | 2 +-
.../tests/resources/parse_answer/disk_match.toml | 2 +-
.../tests/resources/parse_answer/disk_match_all.json | 2 +-
.../tests/resources/parse_answer/disk_match_all.toml | 2 +-
.../tests/resources/parse_answer/disk_match_any.json | 2 +-
.../tests/resources/parse_answer/disk_match_any.toml | 2 +-
.../tests/resources/parse_answer/minimal.json | 2 +-
.../tests/resources/parse_answer/minimal.toml | 2 +-
.../tests/resources/parse_answer/nic_matching.json | 2 +-
.../tests/resources/parse_answer/nic_matching.toml | 2 +-
.../tests/resources/parse_answer/specific_nic.json | 2 +-
.../tests/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-tui-installer/src/main.rs | 11 ++++++-----
20 files changed, 41 insertions(+), 21 deletions(-)
create mode 100644 Proxmox/Sys.pm
--
2.46.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH installer 1/3] proxinstall: raise minimum root password length to 8 characters
2024-10-07 9:22 [pve-devel] [PATCH installer 0/3] raise minimum root password length to 8 characters Christoph Heiss
@ 2024-10-07 9:22 ` Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 2/3] tui: " Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 3/3] auto-installer: " Christoph Heiss
2 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2024-10-07 9:22 UTC (permalink / raw)
To: pve-devel
.. 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>
---
Proxmox/Makefile | 1 +
Proxmox/Sys.pm | 6 ++++++
proxinstall | 8 ++++++--
3 files changed, 13 insertions(+), 2 deletions(-)
create mode 100644 Proxmox/Sys.pm
diff --git a/Proxmox/Makefile b/Proxmox/Makefile
index 035626b..edcabc1 100644
--- a/Proxmox/Makefile
+++ b/Proxmox/Makefile
@@ -12,6 +12,7 @@ PERL_MODULES=\
Install/RunEnv.pm \
Install/StorageConfig.pm \
Log.pm \
+ Sys.pm \
Sys/Block.pm \
Sys/Command.pm \
Sys/File.pm \
diff --git a/Proxmox/Sys.pm b/Proxmox/Sys.pm
new file mode 100644
index 0000000..41e5316
--- /dev/null
+++ b/Proxmox/Sys.pm
@@ -0,0 +1,6 @@
+package Proxmox::Sys;
+
+use strict;
+use warnings;
+
+our $ROOT_PASSWORD_MIN_LENGTH = 8;
diff --git a/proxinstall b/proxinstall
index 12f3eaa..a4ed20b 100755
--- a/proxinstall
+++ b/proxinstall
@@ -33,6 +33,7 @@ my $iso_env = Proxmox::Install::ISOEnv::get();
use Proxmox::Install;
use Proxmox::Install::Config;
+use Proxmox::Sys;
use Proxmox::Sys::Block qw(get_cached_disks);
use Proxmox::Sys::Command qw(syscmd);
use Proxmox::Sys::File qw(file_read_all file_write_all);
@@ -720,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.46.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH installer 2/3] tui: raise minimum root password length to 8 characters
2024-10-07 9:22 [pve-devel] [PATCH installer 0/3] raise minimum root password length to 8 characters Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 1/3] proxinstall: " Christoph Heiss
@ 2024-10-07 9:22 ` Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 3/3] auto-installer: " Christoph Heiss
2 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2024-10-07 9:22 UTC (permalink / raw)
To: pve-devel
.. 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>
---
proxmox-installer-common/src/lib.rs | 3 +++
proxmox-tui-installer/src/main.rs | 11 ++++++-----
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index 850e825..70dffab 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -4,3 +4,6 @@ pub mod setup;
pub mod utils;
pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
+
+/// 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 3fb87a7..c5befce 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -22,6 +22,7 @@ use proxmox_installer_common::{
options::{BootdiskOptions, NetworkOptions, TimezoneOptions},
setup::{installer_setup, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo},
utils::Fqdn,
+ ROOT_PASSWORD_MIN_LENGTH,
};
mod setup;
@@ -452,14 +453,14 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
Regex::new(r"^[\w\+\-\~]+(\.[\w\+\-\~]+)*@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*$")
.unwrap();
- if root_password.len() < 5 {
- Err("password too short, must be at least 5 characters long")
+ 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")
+ Err("passwords do not match".to_owned())
} else if email == "mail@example.invalid" {
- Err("invalid email address")
+ Err("invalid email address".to_owned())
} else if !email_regex.is_match(&email) {
- Err("Email does not look like a valid address (user@domain.tld)")
+ Err("Email does not look like a valid address (user@domain.tld)".to_owned())
} else {
Ok(PasswordOptions {
root_password,
--
2.46.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
2024-10-07 9:22 [pve-devel] [PATCH installer 0/3] raise minimum root password length to 8 characters Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 1/3] proxinstall: " Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 2/3] tui: " Christoph Heiss
@ 2024-10-07 9:22 ` Christoph Heiss
2024-10-07 9:49 ` Stefan Hanreich
2 siblings, 1 reply; 9+ messages in thread
From: Christoph Heiss @ 2024-10-07 9:22 UTC (permalink / raw)
To: pve-devel
.. 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>
---
proxmox-auto-installer/src/utils.rs | 5 +++++
.../tests/resources/parse_answer/disk_match.json | 2 +-
.../tests/resources/parse_answer/disk_match.toml | 2 +-
.../tests/resources/parse_answer/disk_match_all.json | 2 +-
.../tests/resources/parse_answer/disk_match_all.toml | 2 +-
.../tests/resources/parse_answer/disk_match_any.json | 2 +-
.../tests/resources/parse_answer/disk_match_any.toml | 2 +-
.../tests/resources/parse_answer/minimal.json | 2 +-
.../tests/resources/parse_answer/minimal.toml | 2 +-
.../tests/resources/parse_answer/nic_matching.json | 2 +-
.../tests/resources/parse_answer/nic_matching.toml | 2 +-
.../tests/resources/parse_answer/specific_nic.json | 2 +-
.../tests/resources/parse_answer/specific_nic.toml | 2 +-
proxmox-auto-installer/tests/resources/parse_answer/zfs.json | 2 +-
proxmox-auto-installer/tests/resources/parse_answer/zfs.toml | 2 +-
15 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 45ad222..e0dd2ae 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -13,6 +13,7 @@ use proxmox_installer_common::{
setup::{
InstallConfig, InstallRootPassword, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo,
},
+ ROOT_PASSWORD_MIN_LENGTH,
};
use serde::{Deserialize, Serialize};
@@ -309,6 +310,10 @@ fn verify_root_password_settings(answer: &Answer) -> Result<()> {
} 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 if answer.global.root_password.is_some()
+ && answer.global.root_password.as_ref().map(|s| s.len()) < Some(ROOT_PASSWORD_MIN_LENGTH)
+ {
+ bail!("`global.root_password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long");
} else {
Ok(())
}
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 6c8d6d9..a2f8b6f 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 2d2e94e..a6567c2 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 1f3b2eb..f9d29f7 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/minimal.json b/proxmox-auto-installer/tests/resources/parse_answer/minimal.json
index 9fe9150..1db23e6 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"
}
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 610060e..ba0a47f 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"
}
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 5f456bb..fcf26fb 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"
}
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 025dd8f..710b4d6 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.46.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
2024-10-07 9:22 ` [pve-devel] [PATCH installer 3/3] auto-installer: " Christoph Heiss
@ 2024-10-07 9:49 ` Stefan Hanreich
2024-10-07 9:52 ` Christoph Heiss
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Hanreich @ 2024-10-07 9:49 UTC (permalink / raw)
To: pve-devel
On 10/7/24 11:22, Christoph Heiss wrote:
> .. 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>
> ---
> proxmox-auto-installer/src/utils.rs | 5 +++++
> .../tests/resources/parse_answer/disk_match.json | 2 +-
> .../tests/resources/parse_answer/disk_match.toml | 2 +-
> .../tests/resources/parse_answer/disk_match_all.json | 2 +-
> .../tests/resources/parse_answer/disk_match_all.toml | 2 +-
> .../tests/resources/parse_answer/disk_match_any.json | 2 +-
> .../tests/resources/parse_answer/disk_match_any.toml | 2 +-
> .../tests/resources/parse_answer/minimal.json | 2 +-
> .../tests/resources/parse_answer/minimal.toml | 2 +-
> .../tests/resources/parse_answer/nic_matching.json | 2 +-
> .../tests/resources/parse_answer/nic_matching.toml | 2 +-
> .../tests/resources/parse_answer/specific_nic.json | 2 +-
> .../tests/resources/parse_answer/specific_nic.toml | 2 +-
> proxmox-auto-installer/tests/resources/parse_answer/zfs.json | 2 +-
> proxmox-auto-installer/tests/resources/parse_answer/zfs.toml | 2 +-
> 15 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
> index 45ad222..e0dd2ae 100644
> --- a/proxmox-auto-installer/src/utils.rs
> +++ b/proxmox-auto-installer/src/utils.rs
> @@ -13,6 +13,7 @@ use proxmox_installer_common::{
> setup::{
> InstallConfig, InstallRootPassword, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo,
> },
> + ROOT_PASSWORD_MIN_LENGTH,
> };
> use serde::{Deserialize, Serialize};
>
> @@ -309,6 +310,10 @@ fn verify_root_password_settings(answer: &Answer) -> Result<()> {
> } 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 if answer.global.root_password.is_some()
> + && answer.global.root_password.as_ref().map(|s| s.len()) < Some(ROOT_PASSWORD_MIN_LENGTH)
> + {
> + bail!("`global.root_password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long");
> } else {
> Ok(())
> }
maybe match is better at this point?
Something like
match (answer.global.root_password, answer.global.root_password_hashed) {
[..]
(Some(password), _) if password.len() < ROOT_PASSWORD_MIN_LENGTH,
[..]
}
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
2024-10-07 9:49 ` Stefan Hanreich
@ 2024-10-07 9:52 ` Christoph Heiss
2024-10-07 9:55 ` Stefan Hanreich
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Heiss @ 2024-10-07 9:52 UTC (permalink / raw)
To: Stefan Hanreich; +Cc: Proxmox VE development discussion
Thanks for the review!
On Mon, Oct 07, 2024 at 11:49:02AM GMT, Stefan Hanreich wrote:
> On 10/7/24 11:22, Christoph Heiss wrote:
[..]
> > @@ -309,6 +310,10 @@ fn verify_root_password_settings(answer: &Answer) -> Result<()> {
> > } 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 if answer.global.root_password.is_some()
> > + && answer.global.root_password.as_ref().map(|s| s.len()) < Some(ROOT_PASSWORD_MIN_LENGTH)
> > + {
> > + bail!("`global.root_password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long");
> > } else {
> > Ok(())
> > }
>
> maybe match is better at this point?
>
> Something like
>
> match (answer.global.root_password, answer.global.root_password_hashed) {
> [..]
> (Some(password), _) if password.len() < ROOT_PASSWORD_MIN_LENGTH,
> [..]
> }
>
I'll rework it a bit, match definitely seems like the better fit here!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
2024-10-07 9:52 ` Christoph Heiss
@ 2024-10-07 9:55 ` Stefan Hanreich
2024-10-07 10:02 ` Christoph Heiss
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Hanreich @ 2024-10-07 9:55 UTC (permalink / raw)
To: Christoph Heiss; +Cc: Proxmox VE development discussion
On 10/7/24 11:52, Christoph Heiss wrote:
> Thanks for the review!
>
> On Mon, Oct 07, 2024 at 11:49:02AM GMT, Stefan Hanreich wrote:
>> On 10/7/24 11:22, Christoph Heiss wrote:
> [..]
>>> @@ -309,6 +310,10 @@ fn verify_root_password_settings(answer: &Answer) -> Result<()> {
>>> } 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 if answer.global.root_password.is_some()
>>> + && answer.global.root_password.as_ref().map(|s| s.len()) < Some(ROOT_PASSWORD_MIN_LENGTH)
>>> + {
>>> + bail!("`global.root_password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long");
>>> } else {
>>> Ok(())
>>> }
>>
>> maybe match is better at this point?
>>
>> Something like
>>
>> match (answer.global.root_password, answer.global.root_password_hashed) {
>> [..]
>> (Some(password), _) if password.len() < ROOT_PASSWORD_MIN_LENGTH,
>> [..]
>> }
>>
>
> I'll rework it a bit, match definitely seems like the better fit here!
I also thought about suggesting an Enum here like
enum AnswerPassword {
Raw(...)
Hashed(...)
}
But that's probably a bit more involved and idk how it would work with
the answer file format, so hard for me to tell if feasible with the
current state of things
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
2024-10-07 9:55 ` Stefan Hanreich
@ 2024-10-07 10:02 ` Christoph Heiss
2024-10-07 10:39 ` Stefan Hanreich
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Heiss @ 2024-10-07 10:02 UTC (permalink / raw)
To: Stefan Hanreich; +Cc: Proxmox VE development discussion
On Mon, Oct 07, 2024 at 11:55:19AM GMT, Stefan Hanreich wrote:
> On 10/7/24 11:52, Christoph Heiss wrote:
> > Thanks for the review!
> >
> > On Mon, Oct 07, 2024 at 11:49:02AM GMT, Stefan Hanreich wrote:
> >> On 10/7/24 11:22, Christoph Heiss wrote:
> > [..]
> >>> @@ -309,6 +310,10 @@ fn verify_root_password_settings(answer: &Answer) -> Result<()> {
> >>> } 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 if answer.global.root_password.is_some()
> >>> + && answer.global.root_password.as_ref().map(|s| s.len()) < Some(ROOT_PASSWORD_MIN_LENGTH)
> >>> + {
> >>> + bail!("`global.root_password` must be at least {ROOT_PASSWORD_MIN_LENGTH} characters long");
> >>> } else {
> >>> Ok(())
> >>> }
> >>
> >> maybe match is better at this point?
> >>
> >> Something like
> >>
> >> match (answer.global.root_password, answer.global.root_password_hashed) {
> >> [..]
> >> (Some(password), _) if password.len() < ROOT_PASSWORD_MIN_LENGTH,
> >> [..]
> >> }
> >>
> >
> > I'll rework it a bit, match definitely seems like the better fit here!
>
> I also thought about suggesting an Enum here like
>
> enum AnswerPassword {
> Raw(...)
> Hashed(...)
> }
>
> But that's probably a bit more involved and idk how it would work with
> the answer file format, so hard for me to tell if feasible with the
> current state of things
Sounds like a good idea tho. Shouldn't be that much work, other than
maybe a custom (small) de-/serializer for the answer file and config
output.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
2024-10-07 10:02 ` Christoph Heiss
@ 2024-10-07 10:39 ` Stefan Hanreich
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Hanreich @ 2024-10-07 10:39 UTC (permalink / raw)
To: Christoph Heiss; +Cc: Proxmox VE development discussion
On 10/7/24 12:02, Christoph Heiss wrote:
> Sounds like a good idea tho. Shouldn't be that much work, other than
> maybe a custom (small) de-/serializer for the answer file and config
> output.
Possibly even untagged and rename serde directive could suffice
(together with a FromStr impl maybe?)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-10-07 10:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-07 9:22 [pve-devel] [PATCH installer 0/3] raise minimum root password length to 8 characters Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 1/3] proxinstall: " Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 2/3] tui: " Christoph Heiss
2024-10-07 9:22 ` [pve-devel] [PATCH installer 3/3] auto-installer: " Christoph Heiss
2024-10-07 9:49 ` Stefan Hanreich
2024-10-07 9:52 ` Christoph Heiss
2024-10-07 9:55 ` Stefan Hanreich
2024-10-07 10:02 ` Christoph Heiss
2024-10-07 10:39 ` Stefan Hanreich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox