wifictl dbus_nm: serve AccessPoint interfaces at returned paths; fix active index
Two defects from the 5-lane review of commit b6ac916a2c:
1. AP paths unserved (MAJOR)
get_access_points() returned Vec<OwnedObjectPath> but no AP interface
objects were exported at those paths. Qt and other NM clients would
see the device but no APs at all.
Fix: add AccessPointInterface (#[interface(name =
"org.freedesktop.NetworkManager.AccessPoint")]) with the standard AP
properties (Flags, WpaFlags, RsnFlags, Ssid, Frequency, Mode,
MaxBitrate, Strength, HwAddress, LastSeen). serve_on_thread now
builds a (path, interface) pair for each AP and calls
serve_at() for each, so every returned path refers to a real D-Bus
object.
2. ActiveAccessPoint wrong-index bug (MAJOR)
When the matching active SSID was at AP index 1 or later,
active_access_point_inner() returned path index 0. Now iterates
over access_points to find the index where ap.ssid == active_ssid
and returns that path. No-match returns '/' sentinel.
Verified: 49/49 tests pass (47 unit + 2 cli_transport; was 41 + 2
in the round-2 commit, +6 new AP interface + active-index tests).
This commit is contained in:
@@ -230,7 +230,7 @@ impl NmAccessPoint {
|
|||||||
mod nm_iface {
|
mod nm_iface {
|
||||||
use std::sync::{Arc, Mutex};
|
use std::sync::{Arc, Mutex};
|
||||||
|
|
||||||
use super::{NmState, NmWifiDevice};
|
use super::{NmAccessPoint, NmState, NmWifiDevice};
|
||||||
use zbus::{
|
use zbus::{
|
||||||
blocking::connection::Builder as ConnectionBuilder, fdo, interface,
|
blocking::connection::Builder as ConnectionBuilder, fdo, interface,
|
||||||
zvariant::{ObjectPath, OwnedObjectPath},
|
zvariant::{ObjectPath, OwnedObjectPath},
|
||||||
@@ -374,19 +374,39 @@ mod nm_iface {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Build the `(path, interface)` pair for every access point in the
|
||||||
|
/// current snapshot, so `serve_on_thread` can export each path as a
|
||||||
|
/// real `org.freedesktop.NetworkManager.AccessPoint` object.
|
||||||
|
pub(crate) fn access_point_interfaces(
|
||||||
|
&self,
|
||||||
|
) -> Vec<(String, AccessPointInterface)> {
|
||||||
|
self.snap()
|
||||||
|
.access_points
|
||||||
|
.iter()
|
||||||
|
.enumerate()
|
||||||
|
.map(|(index, ap)| {
|
||||||
|
(
|
||||||
|
access_point_path(index),
|
||||||
|
AccessPointInterface::new(ap.clone()),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) fn active_access_point_inner(&self) -> fdo::Result<OwnedObjectPath> {
|
pub(crate) fn active_access_point_inner(&self) -> fdo::Result<OwnedObjectPath> {
|
||||||
let snap = self.snap();
|
let snap = self.snap();
|
||||||
let has_active = !snap.active_ssid.is_empty()
|
// Per the NetworkManager spec, `ActiveAccessPoint` is the root
|
||||||
&& snap
|
// path "/" when no access point is associated.
|
||||||
.access_points
|
if snap.active_ssid.is_empty() {
|
||||||
.iter()
|
return try_path("/");
|
||||||
.any(|ap| ap.ssid == snap.active_ssid);
|
}
|
||||||
if has_active {
|
match snap
|
||||||
try_path(&access_point_path(0))
|
.access_points
|
||||||
} else {
|
.iter()
|
||||||
// Per the NetworkManager spec, `ActiveAccessPoint` is the root
|
.position(|ap| ap.ssid == snap.active_ssid)
|
||||||
// path "/" when no access point is associated.
|
{
|
||||||
try_path("/")
|
Some(index) => try_path(&access_point_path(index)),
|
||||||
|
None => try_path("/"),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -488,6 +508,158 @@ mod nm_iface {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
// Access point interface: org.freedesktop.NetworkManager.AccessPoint
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
|
||||||
|
/// Subset of NetworkManager's `NM80211ApSecurityFlags` bitmask that the
|
||||||
|
/// daemon's coarse `security` string can represent.
|
||||||
|
mod nm_ap_sec {
|
||||||
|
/// No security.
|
||||||
|
pub const NONE: u32 = 0;
|
||||||
|
/// Pairwise CCMP (AES) cipher.
|
||||||
|
pub const PAIR_CCMP: u32 = 0x8;
|
||||||
|
/// PSK key management (WPA-/WPA2-Personal).
|
||||||
|
pub const KEY_MGMT_PSK: u32 = 0x100;
|
||||||
|
/// SAE key management (WPA3-Personal).
|
||||||
|
pub const KEY_MGMT_SAE: u32 = 0x400;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Map the access point's `security` label to `(wpa_flags, rsn_flags)`:
|
||||||
|
///
|
||||||
|
/// | label | wpa_flags | rsn_flags |
|
||||||
|
/// |---------------------|------------|------------|
|
||||||
|
/// | `open`/`wep`/other | none | none |
|
||||||
|
/// | `wpa` | CCMP + PSK | none |
|
||||||
|
/// | `wpa2` | CCMP + PSK | CCMP + PSK |
|
||||||
|
/// | `wpa3`/`sae` | none | CCMP + SAE |
|
||||||
|
fn derive_security_flags(security: &str) -> (u32, u32) {
|
||||||
|
let wpa2 = (nm_ap_sec::PAIR_CCMP | nm_ap_sec::KEY_MGMT_PSK,
|
||||||
|
nm_ap_sec::PAIR_CCMP | nm_ap_sec::KEY_MGMT_PSK);
|
||||||
|
match security.to_ascii_lowercase().as_str() {
|
||||||
|
"wpa2" | "wpa3-personal" => wpa2,
|
||||||
|
"wpa" => (nm_ap_sec::PAIR_CCMP | nm_ap_sec::KEY_MGMT_PSK, nm_ap_sec::NONE),
|
||||||
|
"wpa3" | "sae" | "wpa3-sae" => {
|
||||||
|
(nm_ap_sec::NONE, nm_ap_sec::PAIR_CCMP | nm_ap_sec::KEY_MGMT_SAE)
|
||||||
|
}
|
||||||
|
_ => (nm_ap_sec::NONE, nm_ap_sec::NONE),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// D-Bus interface for a single access point, served at
|
||||||
|
/// `{DEVICE_PATH}/AccessPoints/{index}`. Wraps an [`NmAccessPoint`]
|
||||||
|
/// snapshot and exposes the standard
|
||||||
|
/// `org.freedesktop.NetworkManager.AccessPoint` properties.
|
||||||
|
#[derive(Clone)]
|
||||||
|
pub(crate) struct AccessPointInterface {
|
||||||
|
ap: NmAccessPoint,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl AccessPointInterface {
|
||||||
|
pub(crate) fn new(ap: NmAccessPoint) -> Self {
|
||||||
|
Self { ap }
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn ssid_bytes(&self) -> Vec<u8> {
|
||||||
|
self.ap.ssid.as_bytes().to_vec()
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn frequency_value(&self) -> u32 {
|
||||||
|
self.ap.frequency
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn strength_value(&self) -> u8 {
|
||||||
|
self.ap.strength
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn wpa_flags_value(&self) -> u32 {
|
||||||
|
derive_security_flags(&self.ap.security).0
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn rsn_flags_value(&self) -> u32 {
|
||||||
|
derive_security_flags(&self.ap.security).1
|
||||||
|
}
|
||||||
|
|
||||||
|
/// NM 80211Mode: `1` = infrastructure. The daemon does not yet
|
||||||
|
/// distinguish adhoc/ap.
|
||||||
|
pub(crate) fn mode_value(&self) -> u32 {
|
||||||
|
1
|
||||||
|
}
|
||||||
|
|
||||||
|
/// NM 80211ApFlags: `0` = none advertised.
|
||||||
|
pub(crate) fn flags_value(&self) -> u32 {
|
||||||
|
0
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Max bitrate in kbit/s; `0` when not yet determined.
|
||||||
|
pub(crate) fn max_bitrate_value(&self) -> u32 {
|
||||||
|
0
|
||||||
|
}
|
||||||
|
|
||||||
|
/// MAC address; empty until the daemon records one.
|
||||||
|
pub(crate) fn hw_address_value(&self) -> String {
|
||||||
|
String::new()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Wall-clock timestamp of the last beacon seen; `0` when unknown.
|
||||||
|
pub(crate) fn last_seen_value(&self) -> i32 {
|
||||||
|
0
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[interface(name = "org.freedesktop.NetworkManager.AccessPoint")]
|
||||||
|
impl AccessPointInterface {
|
||||||
|
#[zbus(property)]
|
||||||
|
fn flags(&self) -> u32 {
|
||||||
|
self.flags_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn wpa_flags(&self) -> u32 {
|
||||||
|
self.wpa_flags_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn rsn_flags(&self) -> u32 {
|
||||||
|
self.rsn_flags_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn ssid(&self) -> Vec<u8> {
|
||||||
|
self.ssid_bytes()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn frequency(&self) -> u32 {
|
||||||
|
self.frequency_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn mode(&self) -> u32 {
|
||||||
|
self.mode_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn max_bitrate(&self) -> u32 {
|
||||||
|
self.max_bitrate_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn strength(&self) -> u8 {
|
||||||
|
self.strength_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn hw_address(&self) -> String {
|
||||||
|
self.hw_address_value()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[zbus(property)]
|
||||||
|
fn last_seen(&self) -> i32 {
|
||||||
|
self.last_seen_value()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
// Registration
|
// Registration
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
@@ -519,13 +691,25 @@ mod nm_iface {
|
|||||||
let device_path: ObjectPath<'_> = DEVICE_PATH.try_into()?;
|
let device_path: ObjectPath<'_> = DEVICE_PATH.try_into()?;
|
||||||
|
|
||||||
let root = NmRoot::new(Arc::clone(&shared));
|
let root = NmRoot::new(Arc::clone(&shared));
|
||||||
let wireless = NmWirelessDevice::new(shared);
|
let wireless = NmWirelessDevice::new(Arc::clone(&shared));
|
||||||
|
// Collect AP interfaces before `wireless` is moved into serve_at.
|
||||||
|
let ap_interfaces = wireless.access_point_interfaces();
|
||||||
|
|
||||||
let _connection = ConnectionBuilder::session()?
|
let mut builder = ConnectionBuilder::session()?
|
||||||
.name(BUS_NAME)?
|
.name(BUS_NAME)?
|
||||||
.serve_at(root_path, root)?
|
.serve_at(root_path, root)?
|
||||||
.serve_at(device_path, wireless)?
|
.serve_at(device_path, wireless)?;
|
||||||
.build()?;
|
|
||||||
|
// Export each access point at its object path so the paths
|
||||||
|
// returned by GetAccessPoints refer to real D-Bus objects
|
||||||
|
// implementing org.freedesktop.NetworkManager.AccessPoint.
|
||||||
|
for (path_str, iface) in ap_interfaces {
|
||||||
|
let ap_path: ObjectPath<'static> =
|
||||||
|
path_str.as_str().to_owned().try_into()?;
|
||||||
|
builder = builder.serve_at(ap_path, iface)?;
|
||||||
|
}
|
||||||
|
|
||||||
|
let _connection = builder.build()?;
|
||||||
|
|
||||||
log::info!(
|
log::info!(
|
||||||
"wifictl: D-Bus NetworkManager interface registered on the \
|
"wifictl: D-Bus NetworkManager interface registered on the \
|
||||||
@@ -756,6 +940,7 @@ mod tests {
|
|||||||
|
|
||||||
use super::super::nm_iface::{
|
use super::super::nm_iface::{
|
||||||
DEVICE_PATH, NmRoot, NmWirelessDevice, access_point_path, shared_device,
|
DEVICE_PATH, NmRoot, NmWirelessDevice, access_point_path, shared_device,
|
||||||
|
AccessPointInterface,
|
||||||
};
|
};
|
||||||
use super::{NmAccessPoint, NmDeviceState, NmState, NmWifiDevice};
|
use super::{NmAccessPoint, NmDeviceState, NmState, NmWifiDevice};
|
||||||
|
|
||||||
@@ -825,6 +1010,74 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn wireless_device_active_access_point_resolves_to_matching_index() {
|
||||||
|
// "Guest" is the second access point (index 1). The old code
|
||||||
|
// returned index 0 unconditionally; this locks in the real index.
|
||||||
|
let mut device = device_with(NmDeviceState::Activated);
|
||||||
|
device.active_ssid = "Guest".to_string();
|
||||||
|
device.active_strength = 40;
|
||||||
|
let dev = NmWirelessDevice::new(shared_device(&device));
|
||||||
|
assert_eq!(
|
||||||
|
dev.active_access_point_inner().unwrap().as_str(),
|
||||||
|
access_point_path(1),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn wireless_device_active_access_point_empty_ssid_is_root() {
|
||||||
|
let mut device = device_with(NmDeviceState::Disconnected);
|
||||||
|
device.active_ssid.clear();
|
||||||
|
let dev = NmWirelessDevice::new(shared_device(&device));
|
||||||
|
assert_eq!(dev.active_access_point_inner().unwrap().as_str(), "/");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn access_point_interfaces_match_paths_and_data() {
|
||||||
|
let dev = NmWirelessDevice::new(shared_device(&device_with(
|
||||||
|
NmDeviceState::Disconnected,
|
||||||
|
)));
|
||||||
|
let ifaces = dev.access_point_interfaces();
|
||||||
|
assert_eq!(ifaces.len(), 2);
|
||||||
|
assert_eq!(ifaces[0].0, access_point_path(0));
|
||||||
|
assert_eq!(ifaces[0].1.ssid_bytes(), b"RedBear");
|
||||||
|
assert_eq!(ifaces[0].1.frequency_value(), 2412);
|
||||||
|
assert_eq!(ifaces[0].1.strength_value(), 87);
|
||||||
|
assert_eq!(ifaces[1].0, access_point_path(1));
|
||||||
|
assert_eq!(ifaces[1].1.ssid_bytes(), b"Guest");
|
||||||
|
assert_eq!(ifaces[1].1.frequency_value(), 5180);
|
||||||
|
assert_eq!(ifaces[1].1.strength_value(), 40);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn access_point_interfaces_is_empty_when_no_aps() {
|
||||||
|
let dev =
|
||||||
|
NmWirelessDevice::new(shared_device(&NmWifiDevice::default()));
|
||||||
|
assert!(dev.access_point_interfaces().is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn access_point_interface_exposes_wpa2_security_flags() {
|
||||||
|
let iface = AccessPointInterface::new(NmAccessPoint::new(
|
||||||
|
"RedBear", 87, "wpa2", 2412,
|
||||||
|
));
|
||||||
|
assert!(iface.wpa_flags_value() != 0);
|
||||||
|
assert_eq!(iface.wpa_flags_value(), iface.rsn_flags_value());
|
||||||
|
assert_eq!(iface.mode_value(), 1);
|
||||||
|
assert_eq!(iface.flags_value(), 0);
|
||||||
|
assert_eq!(iface.max_bitrate_value(), 0);
|
||||||
|
assert!(iface.hw_address_value().is_empty());
|
||||||
|
assert_eq!(iface.last_seen_value(), 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn access_point_interface_open_security_has_no_flags() {
|
||||||
|
let iface =
|
||||||
|
AccessPointInterface::new(NmAccessPoint::new("Guest", 40, "open", 5180));
|
||||||
|
assert_eq!(iface.wpa_flags_value(), 0);
|
||||||
|
assert_eq!(iface.rsn_flags_value(), 0);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn wireless_device_active_access_point_is_root_when_no_match() {
|
fn wireless_device_active_access_point_is_root_when_no_match() {
|
||||||
let mut device = device_with(NmDeviceState::Disconnected);
|
let mut device = device_with(NmDeviceState::Disconnected);
|
||||||
|
|||||||
Reference in New Issue
Block a user