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