[pve-devel] [RFC pve-installer v3] fix #6508: auto-installer/post-hook: make iface mac address optional

* [pve-devel] [RFC pve-installer v3] fix #6508: auto-installer/post-hook: make iface mac address optional
@ 2025-07-11 15:34 Christian Ebner
  2025-07-14 19:27 ` Thomas Lamprecht
  0 siblings, 1 reply; 3+ messages in thread
From: Christian Ebner @ 2025-07-11 15:34 UTC (permalink / raw)
  To: pve-devel

This is a breaking change as it affects the answer file fetch payload,
the mac address now being optional. The version is therefore bumped.

The installer assumes to have a valid mac address for all interfaces
as provided by the runtime environment json file. Deserialization
will however fail if this is not the case.

In some cases, the interface might not provide a valid MAC address,
for example if not of type ethernet. The address type can be
different based on the low level protocol used by the interface when
queried via sysfs, see `addr_len` [0].

This was encountered for WWAN LTE module without any SIM installed
on some laptops, returning 'link/none' instead.

Therefore, make the interfaces hardware address optional.

It is now not present for network interfaces not having the expected
'link/ether' type, not matching the regex used while gathering the
interfaces in the runtime environment.

Also, rename the internal type from NetdevWithMac to NetdevWithOptMac
to reflect this and perform input validation of the hardware address
when reading it from sysfs.

[0] https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-net

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Note:
Sending this as RFC since not sure about the version bump and since a
second pair of eyes is really waranted given the regression potential.

 proxmox-auto-installer/src/sysinfo.rs         | 26 ++++++++++++++-----
 .../src/fetch_plugins/http.rs                 |  2 +-
 proxmox-installer-common/src/options.rs       |  4 +--
 proxmox-installer-common/src/setup.rs         |  2 +-
 proxmox-post-hook/src/main.rs                 |  3 ++-
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/proxmox-auto-installer/src/sysinfo.rs b/proxmox-auto-installer/src/sysinfo.rs
index fe3a10d..780881b 100644
--- a/proxmox-auto-installer/src/sysinfo.rs
+++ b/proxmox-auto-installer/src/sysinfo.rs
@@ -14,7 +14,7 @@ pub struct SysInfo {
     product: ProductConfig,
     iso: IsoInfo,
     dmi: SystemDMI,
-    network_interfaces: Vec<NetdevWithMac>,
+    network_interfaces: Vec<NetdevWithOptMac>,
 }
 
 impl SysInfo {
@@ -32,7 +32,7 @@ impl SysInfo {
         Ok(Self {
             product: setup_info.config,
             iso: setup_info.iso_info,
-            network_interfaces: NetdevWithMac::get_all()?,
+            network_interfaces: NetdevWithOptMac::get_all()?,
             dmi: SystemDMI::get()?,
         })
     }
@@ -44,21 +44,33 @@ impl SysInfo {
 }
 
 #[derive(Debug, Serialize)]
-struct NetdevWithMac {
+struct NetdevWithOptMac {
     /// The network link name
     pub link: String,
-    /// The MAC address of the network device
-    pub mac: String,
+    /// The MAC address of the network device (optional)
+    pub mac: Option<String>,
 }
 
-impl NetdevWithMac {
+impl NetdevWithOptMac {
     fn get_all() -> Result<Vec<Self>> {
         let mut result: Vec<Self> = Vec::new();
 
         let links = get_nic_list()?;
         for link in links {
             let mac = fs::read_to_string(format!("/sys/class/net/{link}/address"))?;
-            let mac = String::from(mac.trim());
+            let mac = mac.trim();
+
+            // length checks for 12 chars and 5 separator
+            let mac = if mac.len() == 17
+                && mac
+                    .split(':')
+                    .all(|hex| u8::from_str_radix(hex, 16).is_ok())
+            {
+                Some(mac.to_string())
+            } else {
+                None
+            };
+
             result.push(Self { link, mac });
         }
         Ok(result)
diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index 9b3d15c..905d76d 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -44,7 +44,7 @@ struct HttpFetchInfoSchema {
 }
 
 impl HttpFetchInfoSchema {
-    const SCHEMA_VERSION: &str = "1.0";
+    const SCHEMA_VERSION: &str = "2.0";
 }
 
 impl Default for HttpFetchInfoSchema {
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 9cc4ee0..03464f3 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -518,7 +518,7 @@ mod tests {
                 name: "eth0".to_owned(),
                 index: 0,
                 state: InterfaceState::Up,
-                mac: "01:23:45:67:89:ab".to_owned(),
+                mac: Some("01:23:45:67:89:ab".to_owned()),
                 addresses: Some(vec![
                     CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
                 ]),
@@ -645,7 +645,7 @@ mod tests {
                 name: "eth0".to_owned(),
                 index: 0,
                 state: InterfaceState::Up,
-                mac: "01:23:45:67:89:ab".to_owned(),
+                mac: Some("01:23:45:67:89:ab".to_owned()),
                 addresses: None,
             },
         );
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 66cea72..a968484 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -469,7 +469,7 @@ pub struct Interface {
 
     pub index: usize,
 
-    pub mac: String,
+    pub mac: Option<String>,
 
     pub state: InterfaceState,
 
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index bd27121..acaa107 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -77,7 +77,8 @@ struct DiskInfo {
 #[serde(rename_all = "kebab-case")]
 struct NetworkInterfaceInfo {
     /// MAC address of the interface
-    mac: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    mac: Option<String>,
     /// (Designated) IP address of the interface
     #[serde(skip_serializing_if = "Option::is_none")]
     address: Option<CidrAddress>,
-- 
2.47.2



_______________________________________________
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