public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex
@ 2025-11-26 18:07 Stoiko Ivanov
  2025-11-26 18:07 ` [pve-devel] [PATCH installer 1/2] sys: net: use literal pve-iface regular expression for validation Stoiko Ivanov
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stoiko Ivanov @ 2025-11-26 18:07 UTC (permalink / raw)
  To: pve-devel

This patchset tries to address the discrepancies between the validation
for nic-names used in our JSONSchema.pm in pve-common:
https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/JSONSchema.pm;h=827889950b9165dd22e7b12e934d08fd8b21e855;hb=HEAD#l680
and here in our installer. Thx @Thomas for noticing this:
https://lore.proxmox.com/pve-devel/5482904f-efa5-40ef-b8e6-f1ba935f2431@proxmox.com/

Here I simply copied the regex from pve-common and used that for
validation.

minimally tested in a VM with (the now permitted) 'ABC' 'Nic0' and 'B_' as
nicnames.

Stoiko Ivanov (2):
  sys: net: use literal pve-iface regular expression for validation
  common: pinning: use pve-iface regular expression for validation

 Proxmox/Sys/Net.pm                      | 15 ++------
 proxmox-installer-common/src/options.rs | 51 +++++++++----------------
 test/validate-link-pin-map.pl           | 12 ++++--
 3 files changed, 29 insertions(+), 49 deletions(-)

-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH installer 1/2] sys: net: use literal pve-iface regular expression for validation
  2025-11-26 18:07 [pve-devel] [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex Stoiko Ivanov
@ 2025-11-26 18:07 ` Stoiko Ivanov
  2025-11-26 18:07 ` [pve-devel] [PATCH installer 2/2] common: pinning: use " Stoiko Ivanov
  2025-11-27  7:18 ` [pve-devel] applied: [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Stoiko Ivanov @ 2025-11-26 18:07 UTC (permalink / raw)
  To: pve-devel

Instead of trying to do partial checks to validate the interface name
in accordance with `pve-iface` in pve-common, just copy the regular
expression. This comes at the expense of a longer and unified
error-message for different errors, but I'd consider this acceptable.

The length of {1,20} is also copied from the pve-iface validation
regex to make this a literal copy. The kernel restricts it to 15 bytes
(16 with terminating \0) - but we check for the actual length
boundaries before anyways.

Fixes: fc9e720 ("sys: net: fix error-message for interface names first character")
Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 Proxmox/Sys/Net.pm            | 15 +++------------
 test/validate-link-pin-map.pl | 12 ++++++++----
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 37feee1..e3b3b56 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -335,18 +335,9 @@ sub validate_link_pin_map : prototype($) {
                 . " characters\n";
         }
 
-        if ($name =~ m/^[0-9]+$/) {
-            die "interface name '$name' for '$mac' is invalid: "
-                . "name must not be fully numeric\n";
-        }
-
-        if ($name !~ m/^[a-z]/) {
-            die "interface name '$name' for '$mac' is invalid: name must start with a lower-case letter\n";
-        }
-
-        if ($name !~ m/^[a-zA-Z_][a-zA-Z0-9_]*$/) {
-            die "interface name '$name' for '$mac' is invalid: "
-                . "name must only consist of alphanumeric characters and underscores\n";
+        if ($name !~ m/^[a-z][a-z0-9_]{1,20}([:\.]\d+)?$/i) {
+            die "interface name '$name' for '$mac' is invalid: name must start with a letter and "
+                . "contain only ascii characters, digits and underscores\n";
         }
 
         if ($reverse_mapping->{$name}) {
diff --git a/test/validate-link-pin-map.pl b/test/validate-link-pin-map.pl
index 3fbd0e4..2cb8a90 100755
--- a/test/validate-link-pin-map.pl
+++ b/test/validate-link-pin-map.pl
@@ -38,28 +38,32 @@ like($@, qr/12:34:56:ab:cd:ef/, "duplicate error message contains expected mac a
 eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => 'nic-' }) };
 is(
     $@,
-    "interface name 'nic-' for 'ab:cd:ef:12:34:56' is invalid: name must only consist of alphanumeric characters and underscores\n",
+    "interface name 'nic-' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and "
+        . "contain only ascii characters, digits and underscores\n",
     "name with dash is rejected",
 );
 
 eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '0nic' }) };
 is(
     $@,
-    "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must start with a lower-case letter\n",
+    "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and "
+        . "contain only ascii characters, digits and underscores\n",
     "name starting with number is rejected",
 );
 
 eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '_a' }) };
 is(
     $@,
-    "interface name '_a' for 'ab:cd:ef:12:34:56' is invalid: name must start with a lower-case letter\n",
+    "interface name '_a' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and "
+        . "contain only ascii characters, digits and underscores\n",
     "name starting with underscore is rejected",
 );
 
 eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '12345' }) };
 is(
     $@,
-    "interface name '12345' for 'ab:cd:ef:12:34:56' is invalid: name must not be fully numeric\n",
+    "interface name '12345' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and "
+        . "contain only ascii characters, digits and underscores\n",
     "fully numeric name is rejected",
 );
 
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH installer 2/2] common: pinning: use pve-iface regular expression for validation
  2025-11-26 18:07 [pve-devel] [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex Stoiko Ivanov
  2025-11-26 18:07 ` [pve-devel] [PATCH installer 1/2] sys: net: use literal pve-iface regular expression for validation Stoiko Ivanov
@ 2025-11-26 18:07 ` Stoiko Ivanov
  2025-11-27  7:18 ` [pve-devel] applied: [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Stoiko Ivanov @ 2025-11-26 18:07 UTC (permalink / raw)
  To: pve-devel

the previous attempts at mimicking the `pve-iface` validation in
pve-common mis-read the validation, or missed some corner-cases.

So instead of trying to rebuild a regex validation, simply copy the
regular expression from pve-common. Following the recommendation at
https://docs.rs/regex/latest/regex/#overview by using RegexBuilder for
case-insensitive matching.

I kept the length-check at {1,20}, to have this as close to the
original as possible, as we restrict the length to 15 in the checks
before anyways.

Fixes:  47700a5 ("common: pinning: require first character to be lower-case ascii")
Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 proxmox-installer-common/src/options.rs | 51 +++++++++----------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 34d0725..fbc0207 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,5 +1,5 @@
 use anyhow::{Result, bail};
-use regex::Regex;
+use regex::{Regex, RegexBuilder};
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::net::{IpAddr, Ipv4Addr};
@@ -517,25 +517,18 @@ impl NetworkInterfacePinningOptions {
                 );
             }
 
-            if name.chars().all(char::is_numeric) {
-                bail!(
-                    "interface name '{name}' for '{mac}' is invalid: name must not be fully numeric"
-                );
-            }
-
             // Mimicking the `pve-iface` schema verification
-            if !name.starts_with(|c: char| c.is_ascii_lowercase()) {
-                bail!(
-                    "interface name '{name}' for '{mac}' is invalid: name must start with a lowercase letter"
-                );
-            }
-
-            if !name
-                .chars()
-                .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_')
-            {
+            static RE: OnceLock<Regex> = OnceLock::new();
+            let re = RE.get_or_init(|| {
+                RegexBuilder::new(r"^[a-z][a-z0-9_]{1,20}([:\.]\d+)?$")
+                    .case_insensitive(true)
+                    .build()
+                    .unwrap()
+            });
+
+            if !re.is_match(name) {
                 bail!(
-                    "interface name '{name}' for '{mac}' is invalid: name must only consist of alphanumeric lowercase characters and underscores"
+                    "interface name '{name}' for '{mac}' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
                 );
             }
 
@@ -1022,7 +1015,7 @@ mod tests {
         assert!(res.is_err());
         assert_eq!(
             res.unwrap_err().to_string(),
-            "interface name 'nic-' for 'ab:cd:ef:12:34:56' is invalid: name must only consist of alphanumeric lowercase characters and underscores"
+            "interface name 'nic-' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
         )
     }
 
@@ -1037,7 +1030,7 @@ mod tests {
         assert!(res.is_err());
         assert_eq!(
             res.unwrap_err().to_string(),
-            "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must start with a lowercase letter"
+            "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
         );
 
         options
@@ -1048,34 +1041,26 @@ mod tests {
         assert!(res.is_err());
         assert_eq!(
             res.unwrap_err().to_string(),
-            "interface name '_a' for 'ab:cd:ef:12:34:56' is invalid: name must start with a lowercase letter"
+            "interface name '_a' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
         );
     }
 
     #[test]
-    fn network_interface_pinning_options_fail_on_uppercase_char() {
+    fn network_interface_pinning_options_pass_on_uppercase_char() {
         let mut options = NetworkInterfacePinningOptions::default();
         options
             .mapping
             .insert("ab:cd:ef:12:34:56".to_owned(), "Nic0".to_owned());
 
         let res = options.verify();
-        assert!(res.is_err());
-        assert_eq!(
-            res.unwrap_err().to_string(),
-            "interface name 'Nic0' for 'ab:cd:ef:12:34:56' is invalid: name must start with a lowercase letter"
-        );
+        assert!(res.is_ok());
 
         options
             .mapping
             .insert("ab:cd:ef:12:34:56".to_owned(), "nIc0".to_owned());
 
         let res = options.verify();
-        assert!(res.is_err());
-        assert_eq!(
-            res.unwrap_err().to_string(),
-            "interface name 'nIc0' for 'ab:cd:ef:12:34:56' is invalid: name must only consist of alphanumeric lowercase characters and underscores"
-        );
+        assert!(res.is_ok());
 
         options
             .mapping
@@ -1096,7 +1081,7 @@ mod tests {
         assert!(res.is_err());
         assert_eq!(
             res.unwrap_err().to_string(),
-            "interface name '12345' for 'ab:cd:ef:12:34:56' is invalid: name must not be fully numeric"
+            "interface name '12345' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
         )
     }
 }
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] applied: [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex
  2025-11-26 18:07 [pve-devel] [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex Stoiko Ivanov
  2025-11-26 18:07 ` [pve-devel] [PATCH installer 1/2] sys: net: use literal pve-iface regular expression for validation Stoiko Ivanov
  2025-11-26 18:07 ` [pve-devel] [PATCH installer 2/2] common: pinning: use " Stoiko Ivanov
@ 2025-11-27  7:18 ` Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2025-11-27  7:18 UTC (permalink / raw)
  To: pve-devel, Stoiko Ivanov

On Wed, 26 Nov 2025 19:07:14 +0100, Stoiko Ivanov wrote:
> This patchset tries to address the discrepancies between the validation
> for nic-names used in our JSONSchema.pm in pve-common:
> https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/JSONSchema.pm;h=827889950b9165dd22e7b12e934d08fd8b21e855;hb=HEAD#l680
> and here in our installer. Thx @Thomas for noticing this:
> https://lore.proxmox.com/pve-devel/5482904f-efa5-40ef-b8e6-f1ba935f2431@proxmox.com/
> 
> Here I simply copied the regex from pve-common and used that for
> validation.
> 
> [...]

Applied, thanks!

[1/2] sys: net: use literal pve-iface regular expression for validation
      commit: 854eef5f55152618dd86be61bb2bc4afdbd93477
[2/2] common: pinning: use pve-iface regular expression for validation
      commit: 035594b94f947e3c2c336ab562174886d002b400


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-11-27  7:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-26 18:07 [pve-devel] [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex Stoiko Ivanov
2025-11-26 18:07 ` [pve-devel] [PATCH installer 1/2] sys: net: use literal pve-iface regular expression for validation Stoiko Ivanov
2025-11-26 18:07 ` [pve-devel] [PATCH installer 2/2] common: pinning: use " Stoiko Ivanov
2025-11-27  7:18 ` [pve-devel] applied: [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal