[pve-devel] [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex

[pve-devel] [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex

[Stoiko Ivanov]

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

[pve-devel] [PATCH installer 1/2] sys: net: use literal pve-iface regular expression for validation

[Stoiko Ivanov]

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

[pve-devel] [PATCH installer 2/2] common: pinning: use pve-iface regular expression for validation

[Stoiko Ivanov]

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

[pve-devel] applied: [PATCH installer 0/2] sync allowed nic-names with pve-common by using the same regex

[Thomas Lamprecht]

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

Re: [pve-devel] [PATCH installer 2/2] common: pinning: use pve-iface regular expression for validation

[Fiona Ebner]

Am 26.11.25 um 7:08 PM schrieb Stoiko Ivanov:
> +            static RE: OnceLock<Regex> = OnceLock::new();
> +            let re = RE.get_or_init(|| {
> +                RegexBuilder::new(r"^[a-z][a-z0-9_]{1,20}([:\.]\d+)?$")

Should the later part with a colon really be allowed? On the new test
ISO, a name like 'nicat:3' will be accepted, but won't actually work
later when booting:
"Interface name is not valid or too long, ignoring assignment: nicat:3"

I guess the dot can make sense if there are multiple NICs, so you could
have e.g. 'nic0' and 'nic0.3'?

> +                    .case_insensitive(true)
> +                    .build()
> +                    .unwrap()
> +            });
> +


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

Re: [pve-devel] [PATCH installer 2/2] common: pinning: use pve-iface regular expression for validation

[Stoiko Ivanov]

Nice catch - Thanks!

On Mon, 1 Dec 2025 11:53:53 +0100
Fiona Ebner <f.ebner@proxmox.com> wrote:

> Am 26.11.25 um 7:08 PM schrieb Stoiko Ivanov:
> > +            static RE: OnceLock<Regex> = OnceLock::new();
> > +            let re = RE.get_or_init(|| {
> > +                RegexBuilder::new(r"^[a-z][a-z0-9_]{1,20}([:\.]\d+)?$")  
> 
> Should the later part with a colon really be allowed? On the new test
> ISO, a name like 'nicat:3' will be accepted, but won't actually work
> later when booting:
given the back and forth [0] of trying to get this in sync with
pve-common's JSONSchema.pm I did use the literal regex from there on
purpose...

> "Interface name is not valid or too long, ignoring assignment: nicat:3"
...not considering that these are interface-names for the kernel, while
`prefix:\d` and `prefix\.\d` are syntax for /etc/network/interfaces
(the former for aliaseses (having a second ip/network on one interface),
the latter for VLAN tags).

It would probably make sense to add a separate validation for kernel iface
names somewhere centrally and use/reference that (everywhere), but I'd like
to avoid to deviate here from the one thing we (afaict?) always reference
when we deal with "NIC names".
Apart from disallowing the `([:\.]\d+)?` trailer - this would also
restrict the length to 15 characters fwict.

In practical terms I don't think keeping it as is has a large potential
for regression (not expecting many users to touch the advanced mappings,
and even if they add :\d there they'd notice after rebooting the first
time).

> 
> I guess the dot can make sense if there are multiple NICs, so you could
> have e.g. 'nic0' and 'nic0.3'?
see above - afaict in Debian/ifupdown terms these are used for
vlan-interfaces (and might lead to unexpected results if used for plain
nic-names (did not test this though))



[0] e.g.
https://lore.proxmox.com/all/20251113135023.1038305-1-c.heiss@proxmox.com/
https://lore.proxmox.com/all/20251118151532.592423-1-s.ivanov@proxmox.com/
https://lore.proxmox.com/all/20251126180819.817240-1-s.ivanov@proxmox.com/


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