A radio leaves zone 0 with one station, whatever netdevs the last net zone left on it

build/guest-tests/zones-check.sh

@@ -557,6 +557,31 @@ else
     fail "reattach-egress" "untrusted (init ${upid:-none}): gateway before ${out_before}; eth0 gone while down ${eth0_gone}; gateway while down ${out_down}; gateway after ${out_after}; $(tail -2 "$LOG/untrusted-bg.out" | tr '\n' ' ')"
 fi
 "$KD" stop untrusted >/dev/null 2>&1; wait "$UBG" 2>/dev/null
+# A net zone can rename its NIC, which comes back to zone 0 under that name:
+# a leading dash would reach the next zone's dhcpcd as an option, and a quote
+# would end the name in its nft set. The next start renames it nic<N> first.
+ninit="$(cut -d' ' -f1 /run/kryptik/zones/net/init.pid 2>/dev/null)"
+named="$(nsenter -t "${ninit:-0}" -n sh -c 'ip link set dev eth0 down && ip link set dev eth0 name "-x\"y" && echo NAMED' 2>&1)"
+rn_before="$(ready_count)"
+s6-svc -d /run/service/net-zone
+back=""
+for _ in $(seq 1 20); do back="$(physical)"; [[ -n "$back" ]] && break; sleep 0.5; done
+s6-svc -u /run/service/net-zone
+took=""
+for _ in $(seq 1 60); do [[ "$(ready_count)" -gt "$rn_before" ]] && { took="$(last_ready)"; break; }; sleep 1; done
+plain_uplink='uplinks=nic[0-9]+( |$)'
+if [[ "$named" == *NAMED* && "$back" == '-x"y ' && "$took" =~ $plain_uplink && -z "$(physical)" ]]; then
+    pass "uplink-renamed-plain" "the net zone named its NIC -x\"y, it came back to zone 0 so named, and the next start took it as ${took##*uplinks=}"
+else
+    fail "uplink-renamed-plain" "renamed: ${named:-nothing}; back in zone 0: ${back:-nothing}; READY: ${took:-none}; $(netzone_said 'NOT READY' | tail -1); zone 0 holds: $(physical)"
+fi
+# eth0 again, for what follows: the NIC is back in zone 0, down, while the net zone is.
+s6-svc -d /run/service/net-zone
+for _ in $(seq 1 20); do [[ -n "$(physical)" ]] && break; sleep 0.5; done
+for d in /sys/class/net/*; do [[ -e "$d/device" ]] && ip link set dev "${d##*/}" name eth0; done
+rn_before="$(ready_count)"
+s6-svc -u /run/service/net-zone
+for _ in $(seq 1 60); do [[ "$(ready_count)" -gt "$rn_before" ]] && break; sleep 1; done
 
 # --- zones: the net zone over a radio -----------------------------------------------
 # QEMU has no radio, so mac80211_hwsim makes two. phy1 goes into a network
@@ -659,6 +684,39 @@ if [[ "$ZOUT" == *AP-REACHED* && "${sta_seen:-0}" -ge 1 ]]; then
 else
     fail "wifi-egress" "$ZOUT; stations at the access point: ${sta_seen:-0}; $(tail -2 "$LOG/untrusted.err" | tr '\n' ' ')"
 fi
+# A net zone can leave its radio with a second netdev or with none, and the
+# radio comes back to zone 0 so. Each round makes that change in the net
+# zone, stops it, waits for phy0 in zone 0 and starts it: the next zone must
+# get the radio with one fresh station, not one the last zone named (amon0
+# sorts before the station), and join the access point again.
+radio_rounds=""
+radio_netdevs() { nsenter -t "${1:-0}" -n iw dev 2>/dev/null | awk '$1 == "Interface" { n++; s = s " " $2 } END { printf "%d%s", n, s }'; }
+for round in extra none; do
+    ninit="$(cut -d' ' -f1 /run/kryptik/zones/net/init.pid 2>/dev/null)"
+    if [[ "$round" = extra ]]; then
+        # By index: iw finds a phy's name in sysfs, which is zone 0's here and no longer lists it.
+        phy="$(nsenter -t "${ninit:-0}" -n iw dev 2>/dev/null | sed -n 's/^phy#\([0-9]*\)$/\1/p' | head -1)"
+        nsenter -t "${ninit:-0}" -n iw "phy#${phy:-0}" interface add amon0 type monitor > "$LOG/radio-$round.out" 2>&1
+    else
+        for w in $(nsenter -t "${ninit:-0}" -n iw dev 2>/dev/null | awk '$1 == "Interface" { print $2 }'); do
+            nsenter -t "${ninit:-0}" -n iw dev "$w" del >> "$LOG/radio-$round.out" 2>&1
+        done
+    fi
+    left="$(radio_netdevs "$ninit")"
+    rb="$(ready_count)"
+    s6-svc -d /run/service/net-zone
+    for _ in $(seq 1 30); do [[ -e /sys/class/ieee80211/phy0 ]] && break; sleep 0.5; done
+    s6-svc -u /run/service/net-zone
+    line="$(ready_after "$rb" " wifi=$AP_SSID " 90)"
+    joined="did not join"; [[ "$line" == *" wifi=$AP_SSID "* ]] && joined=joined
+    radio_rounds="${radio_rounds}${round}: left ${left}, back with $(radio_netdevs "$(cut -d' ' -f1 /run/kryptik/zones/net/init.pid 2>/dev/null)"), ${joined}; "
+done
+fresh_radio='^extra: left 2 [^,]*, back with 1 nic[0-9]+, joined; none: left 0, back with 1 nic[0-9]+, joined; $'
+if [[ "$radio_rounds" =~ $fresh_radio ]]; then
+    pass "radio-recarried" "netdevs on the radio as the net zone left it and as the next one got it: ${radio_rounds}"
+else
+    fail "radio-recarried" "${radio_rounds}$(uncaught | grep -a 'kryptikd: \(wiphy\|interface\)' | tail -3 | tr '\n' ' ') $(tr '\n' ' ' < "$LOG/radio-extra.out")"
+fi
 # personal's definition does not open the radio's own network: once the net
 # zone has taken the access point as a gateway, its address is refused
 # personal, and what lies past it is not.

compartments/kryptikd/src/netlink.rs

@@ -14,8 +14,14 @@ const GENL_ID_CTRL: u16 = 0x10;
 const CTRL_CMD_GETFAMILY: u8 = 3;
 const CTRL_ATTR_FAMILY_ID: u16 = 1;
 const CTRL_ATTR_FAMILY_NAME: u16 = 2;
+const NL80211_CMD_NEW_INTERFACE: u8 = 7;
+const NL80211_CMD_DEL_INTERFACE: u8 = 8;
 const NL80211_CMD_SET_WIPHY_NETNS: u8 = 49;
 const NL80211_ATTR_WIPHY: u16 = 1;
+const NL80211_ATTR_IFINDEX: u16 = 3;
+const NL80211_ATTR_IFNAME: u16 = 4;
+const NL80211_ATTR_IFTYPE: u16 = 5;
+const NL80211_IFTYPE_STATION: u32 = 2;
 const NL80211_ATTR_NETNS_FD: u16 = 219;
 const NLA_TYPE_MASK: u16 = 0x3fff;
 
@@ -247,6 +253,27 @@ fn check_name(name: &str) -> io::Result<()> {
     Ok(())
 }
 
+/// Rename the interface at `idx`, which must be down, and return its new name: a `%d` in `name`
+/// takes the first number free in this namespace.
+pub fn rename_index(idx: u32, name: &str) -> io::Result<String> {
+    check_name(name)?;
+    let mut m = Msg::new(RTM_NEWLINK, 0, 1);
+    m.ifinfomsg(libc::AF_UNSPEC as u8, idx as i32, 0, 0);
+    m.attr_str(IFLA_IFNAME, name);
+    transact(m.finish(), &format!("rename interface {idx} to {name:?}"))?;
+    name_of(idx)
+}
+
+/// The name of the interface at `idx` in this namespace.
+pub fn name_of(idx: u32) -> io::Result<String> {
+    let mut buf = [0 as libc::c_char; libc::IF_NAMESIZE];
+    // SAFETY: if_indextoname writes at most IF_NAMESIZE bytes, its NUL included.
+    if unsafe { libc::if_indextoname(idx, buf.as_mut_ptr()) }.is_null() {
+        return Err(io::Error::last_os_error());
+    }
+    Ok(unsafe { std::ffi::CStr::from_ptr(buf.as_ptr()) }.to_string_lossy().into_owned())
+}
+
 /// Create a veth pair `a` <-> `b`, `b` born in namespace `peer_ns` and with MAC `peer_mac` when given.
 pub fn create_veth(a: &str, b: &str, peer_ns: Option<RawFd>, peer_mac: Option<[u8; 6]>) -> io::Result<()> {
     check_name(a)?;
@@ -377,6 +404,28 @@ pub fn set_wiphy_netns(phy: u32, ns_fd: RawFd) -> io::Result<()> {
     transact_on(NETLINK_GENERIC, m.finish(), &format!("move wiphy {phy} into namespace")).map(|_| ())
 }
 
+/// `iw phy <phy> interface add <name> type managed`: a station netdev on a wiphy, `%d` in
+/// `name` taking the first free number.
+pub fn new_station(phy: u32, name: &str) -> io::Result<()> {
+    check_name(name)?;
+    let family = genl_family_id("nl80211")?;
+    let mut m = Msg::new(family, 0, 1);
+    m.genlmsghdr(NL80211_CMD_NEW_INTERFACE, 0);
+    m.attr_u32(NL80211_ATTR_WIPHY, phy);
+    m.attr_str(NL80211_ATTR_IFNAME, name);
+    m.attr_u32(NL80211_ATTR_IFTYPE, NL80211_IFTYPE_STATION);
+    transact_on(NETLINK_GENERIC, m.finish(), &format!("add a station to wiphy {phy}")).map(|_| ())
+}
+
+/// `iw dev <dev> del`: remove a radio's netdev, by index.
+pub fn del_interface(idx: u32) -> io::Result<()> {
+    let family = genl_family_id("nl80211")?;
+    let mut m = Msg::new(family, 0, 1);
+    m.genlmsghdr(NL80211_CMD_DEL_INTERFACE, 0);
+    m.attr_u32(NL80211_ATTR_IFINDEX, idx);
+    transact_on(NETLINK_GENERIC, m.finish(), &format!("delete wireless interface {idx}")).map(|_| ())
+}
+
 pub fn add_addr4(dev: &str, addr: [u8; 4], prefix: u8) -> io::Result<()> {
     let idx = index_of(dev)?;
     let mut m = Msg::new(RTM_NEWADDR, NLM_F_CREATE | NLM_F_EXCL, 1);

compartments/kryptikd/src/netlink/tests.rs

@@ -424,3 +424,29 @@ fn isolated_ports_block_zone_to_zone() {
         other => panic!("bridge isolation test failed at step {other}"),
     }
 }
+
+/// A name a net zone could leave on a NIC is replaced by index, the kernel numbering `nic%d`.
+#[test]
+fn rename_by_index_takes_a_template() {
+    let rc = in_userns_netns(|| {
+        let r: Result<(), i32> = (|| {
+            step(1, create_veth("x\"y", "pb", None, None))?;
+            let idx = index_of("x\"y").map_err(|_| 2)?;
+            let now = rename_index(idx, "nic%d").map_err(|e| {
+                eprintln!("rename_index: {e}");
+                3
+            })?;
+            if now != "nic0" || index_of("x\"y").is_ok() || index_of("nic0").ok() != Some(idx) {
+                eprintln!("renamed {now:?}");
+                return Err(4);
+            }
+            Ok(())
+        })();
+        r.err().unwrap_or(0)
+    });
+    match rc {
+        0 => {}
+        77 => eprintln!("no unprivileged user namespace; skipping"),
+        other => panic!("rename test failed at step {other}"),
+    }
+}

compartments/kryptikd/src/netzone.rs

@@ -13,8 +13,9 @@
 //! Runs in the root parent while the zone waits at its handshake; an unprivileged launch gets
 //! loopback only. NAT, forwarding policy and the resolver belong to tools/net/netzone-init.sh.
 
-use std::ffi::CStr;
+use std::ffi::{CStr, OsString};
 use std::io;
+use std::os::unix::ffi::OsStrExt;
 use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd};
 use std::path::Path;
 
@@ -221,27 +222,103 @@ fn carry_nic(nic: &str, zone_ns: i32) -> Result<Uplink, NetError> {
 }
 
 /// Interfaces on a bus device (`/sys/class/net/<n>/device`): what `[network] nic = "*"` moves.
+///
+/// A NIC comes back to zone 0 under whatever name the net zone that held it gave it, and the
+/// next zone's script, nft set and dhcpcd would take that name as it is: one that is not plain
+/// becomes `nic<N>` first. It came back down, as a rename needs.
+///
+/// A radio comes back with whatever netdevs that zone left on it, and leaves with one station,
+/// as a boot gives it. One with several loses them all, since which to keep would be that
+/// zone's choice, and moving the wiphy would take an AP, mesh or monitor netdev along; one with
+/// none, then or before, gets a fresh station, which takes the radio's own address.
 pub fn physical_interfaces() -> io::Result<Vec<String>> {
-    physical_interfaces_under(Path::new("/sys/class/net"))
+    let class_net = Path::new("/sys/class/net");
+    let listed = physical_interfaces_under(class_net)?;
+    let mut per_radio = std::collections::BTreeMap::new();
+    for (_, _, phy) in &listed {
+        if let Some(p) = phy {
+            *per_radio.entry(*p).or_insert(0) += 1;
+        }
+    }
+    for (name, idx, phy) in &listed {
+        if phy.is_some_and(|p| per_radio[&p] > 1) {
+            if let Err(e) = netlink::del_interface(*idx) {
+                eprintln!("kryptikd: interface {name:?} on a radio with several could not be deleted: {e}");
+            }
+        }
+    }
+    for phy in bare_radios(Path::new("/sys/class")) {
+        match netlink::new_station(phy, "nic%d") {
+            Ok(()) => eprintln!("kryptikd: wiphy {phy} came back with no interface or several; it has one station now"),
+            Err(e) => eprintln!("kryptikd: wiphy {phy} has no interface, and none could be added: {e}"),
+        }
+    }
+    let mut out = Vec::new();
+    let mut radios = std::collections::BTreeSet::new();
+    for (name, idx, phy) in physical_interfaces_under(class_net)? {
+        // A netdev that could not be deleted goes along with its radio's first.
+        if phy.is_some_and(|p| !radios.insert(p)) {
+            continue;
+        }
+        if plain_name(name.as_bytes()) {
+            out.push(name.to_string_lossy().into_owned());
+            continue;
+        }
+        match netlink::rename_index(idx, "nic%d") {
+            Ok(now) => {
+                eprintln!("kryptikd: interface {name:?} renamed {now:?} before it moves");
+                out.push(now);
+            }
+            Err(e) => eprintln!("kryptikd: interface {name:?} left in zone 0: {e}"),
+        }
+    }
+    out.sort();
+    Ok(out)
+}
+
+/// A name as the kernel and eudev give one: a lower-case letter, then lower-case letters and
+/// digits, so no option, quote, glob or control byte. Not `BRIDGE`, which the start makes in
+/// the zone after the NICs move: a NIC by that name would fail it.
+fn plain_name(name: &[u8]) -> bool {
+    name.len() < 16
+        && name != BRIDGE.as_bytes()
+        && name.first().is_some_and(|b| b.is_ascii_lowercase())
+        && name.iter().all(|b| b.is_ascii_lowercase() || b.is_ascii_digit())
 }
 
-/// `physical_interfaces` over any sysfs `class/net` directory, for tests.
-fn physical_interfaces_under(class_net: &Path) -> io::Result<Vec<String>> {
+/// Each physical interface's name, as the kernel holds it, its index, and its wiphy's if it
+/// is a radio's, under any sysfs `class/net` directory, for tests.
+fn physical_interfaces_under(class_net: &Path) -> io::Result<Vec<(OsString, u32, Option<u32>)>> {
     let mut out = Vec::new();
     for e in std::fs::read_dir(class_net)? {
         let e = e?;
-        let name = e.file_name().to_string_lossy().into_owned();
-        if name == "lo" {
+        let name = e.file_name();
+        if name == "lo" || !e.path().join("device").exists() {
             continue;
         }
-        if e.path().join("device").exists() {
-            out.push(name);
-        }
+        let idx = sysfs_index(&e.path().join("ifindex"))
+            .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "a sysfs ifindex is not a number"))?;
+        out.push((name, idx, sysfs_index(&e.path().join("phy80211/index"))));
     }
     out.sort();
     Ok(out)
 }
 
+/// The wiphys under a sysfs `class` directory that no netdev sits on, by index.
+fn bare_radios(class: &Path) -> Vec<u32> {
+    let entries = |d: &str| std::fs::read_dir(class.join(d)).into_iter().flatten().flatten();
+    let held: std::collections::BTreeSet<u32> =
+        entries("net").filter_map(|e| sysfs_index(&e.path().join("phy80211/index"))).collect();
+    let mut bare: Vec<u32> =
+        entries("ieee80211").filter_map(|e| sysfs_index(&e.path().join("index"))).filter(|p| !held.contains(p)).collect();
+    bare.sort();
+    bare
+}
+
+fn sysfs_index(path: &Path) -> Option<u32> {
+    std::fs::read_to_string(path).ok()?.trim().parse().ok()
+}
+
 /// Plumb the nic zone (uplinks and bridge), then reattach running routed zones.
 pub fn plumb_nic_zone(zone: &Zone, zone_ns: i32, zones_dir: &Path) -> Result<(), NetError> {
     plumb_nic_zone_bridge(zone, zone_ns)?;

compartments/kryptikd/src/netzone/tests.rs

@@ -340,3 +340,32 @@ fn replumb_empty_directory_attempts_nothing() {
     assert!(replumb_routed_zones(&dir, -1).is_empty());
     let _ = std::fs::remove_dir_all(&dir);
 }
+
+#[test]
+fn only_plain_names_pass_on() {
+    for ok in ["eth0", "wlan0", "enp0s31f6", "wlp2s0", "enx001122334455"] {
+        assert!(plain_name(ok.as_bytes()), "{ok}");
+    }
+    // What a net zone could leave behind: an option, a quote that ends nft's string, an escape,
+    // the bridge's own name.
+    for bad in ["-k", "x\"y", "x\",\"kryptik0", "e\u{1b}th0", "kryptik0", "Eth0", "eth0.1", "0eth", "", "abcdefghijklmnop"] {
+        assert!(!plain_name(bad.as_bytes()), "{bad:?}");
+    }
+    assert!(!plain_name(b"\xff"));
+}
+
+#[test]
+fn bare_radios_are_the_ones_no_netdev_holds() {
+    let dir = std::env::temp_dir().join(format!("kryptik-radios-{}", std::process::id()));
+    let put = |path: &str, value: &str| {
+        let f = dir.join(path);
+        std::fs::create_dir_all(f.parent().unwrap()).unwrap();
+        std::fs::write(f, value).unwrap();
+    };
+    put("ieee80211/phy0/index", "0\n");
+    put("ieee80211/phy3/index", "3\n");
+    put("net/wlan0/phy80211/index", "0\n");
+    put("net/eth0/ifindex", "2\n");
+    assert_eq!(bare_radios(&dir), vec![3]);
+    let _ = std::fs::remove_dir_all(&dir);
+}

docs/design/net-zone.md

@@ -249,6 +249,17 @@ configured the zone keeps its IPv4 path and the launcher says so.
   `/sys/class/net/<n>/phy80211` exists. Every interface on the wiphy moves
   with its name, and cfg80211 returns the wiphy to the initial namespace when
   the net zone's namespace dies.
+- **It leaves zone 0 as a boot gives it, with one station.** The net zone
+  can add netdevs to its radio, an AP, mesh or monitor one, or delete the
+  one it has, and the radio comes back so. The next start deletes every
+  netdev of a radio with several (`NL80211_CMD_DEL_INTERFACE`), since which
+  to keep would be that zone's choice, gives a radio with none a fresh
+  station named `nic<N>` (`NL80211_CMD_NEW_INTERFACE`), which takes the
+  radio's own address, and carries each radio once. Otherwise a second
+  netdev failed the start once its radio had moved with the first, and a
+  radio with none stayed in zone 0 until a reboot. A driver that cannot add
+  an interface (some FullMAC ones) leaves such a radio in zone 0, and the
+  log says why.
 - **It associates before it leases.** The script finds radios by their
   `phy80211` link, not by name, and runs one `wpa_supplicant` per radio on
   `/etc/wpa_supplicant.conf`, restarting one that dies; dhcpcd takes the
@@ -289,9 +300,14 @@ as `SIGSYS` in the zone's log and a `wifi=connecting` that never changes.
   started, so one started before any net zone resolves names once attached;
   kryptikd never edits a running zone's sealed root.
 - The kernel returns the physical interface to the initial namespace, down
-  and unaddressed, under its own name (`dev<N>` only if zone 0 has an
-  interface by that name by then); kryptikd leaves it so until the next net
-  zone start, which takes it again whatever its name.
+  and unaddressed, under the name it had there (`dev<N>` only if zone 0 has
+  an interface by that name by then); kryptikd leaves it so until the next
+  net zone start, which takes it again. That name was the net zone's to
+  choose, anything but `/`, `:` and whitespace, so the start first renames
+  one that is not plain (a lower-case letter, then lower-case letters and
+  digits, and not `kryptik0`, the bridge it makes there) `nic<N>`: no option,
+  quote or escape a past net zone chose reaches the next one's script, nft set
+  or dhcpcd, and no name of its own stops it.
 
 ## What this guarantees
 
@@ -332,6 +348,13 @@ as `SIGSYS` in the zone's log and a `wifi=connecting` that never changes.
   (`netlink::tests::wireless_moves_by_wiphy`).
 - `wifi.rs` unit tests and the serve and cli suites cover the credentials
   file and `kryptik wifi`.
+- The zones suite has the net zone rename its NIC `-x"y` and restarts it: the
+  NIC is back in zone 0 under that name, and the next start comes up `READY`
+  with it as `nic<N>`. `netlink::tests::rename_by_index_takes_a_template`
+  renames a veth by its index to `nic%d`. Over the simulated radio it leaves
+  the net zone's radio with a monitor netdev named to sort before the
+  station, then with no netdev at all, and each time the next start carries
+  it with one fresh `nic<N>` station that joins the access point again.
 - `tools/tests/netzone-uplink.sh`: the zones a definition lets through, by the
   address kryptikd derives for each, every host's addresses pinned to its own
   MAC and the MAC the same as `netlink::zone_mac`, the gateway sets as nft is