From 4546993f7dc84f21708a9f92fffe9eebd6be5624 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 9 Feb 2024 15:10:18 +0000 Subject: [PATCH 01/21] Add IAccessPointController::SetSsid function. --- .../microsoft/net/wifi/IAccessPointController.hxx | 10 ++++++++++ src/linux/wifi/core/AccessPointControllerLinux.cxx | 7 +++++++ .../microsoft/net/wifi/AccessPointControllerLinux.hxx | 10 ++++++++++ tests/unit/wifi/helpers/AccessPointControllerTest.cxx | 11 +++++++++++ .../net/wifi/test/AccessPointControllerTest.hxx | 10 ++++++++++ .../microsoft/net/wifi/test/AccessPointTest.hxx | 1 + 6 files changed, 49 insertions(+) diff --git a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx index 0b388b30..a93723cc 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx @@ -93,6 +93,16 @@ struct IAccessPointController */ virtual bool SetFrequencyBands(std::vector frequencyBands) = 0; + + /** + * @brief Set the SSID of the access point. + * + * @param ssid The SSID to be set. + * @return true + * @return false + */ + virtual bool + SetSssid(std::string_view ssid) = 0; }; /** diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx index d03c95fa..1317bc87 100644 --- a/src/linux/wifi/core/AccessPointControllerLinux.cxx +++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx @@ -273,6 +273,13 @@ AccessPointControllerLinux::SetFrequencyBands(std::vector AccessPointControllerLinuxFactory::Create(std::string_view interfaceName) { diff --git a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx index 65930d68..dfec38d9 100644 --- a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx +++ b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx @@ -73,6 +73,16 @@ struct AccessPointControllerLinux : bool SetFrequencyBands(std::vector frequencyBands) override; + /** + * @brief Set the SSID of the access point. + * + * @param ssid The SSID to be set. + * @return true + * @return false + */ + bool + SetSssid(std::string_view ssid) override; + private: Wpa::Hostapd m_hostapd; }; diff --git a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx index 2582ecd0..fa638265 100644 --- a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx +++ b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx @@ -83,6 +83,17 @@ AccessPointControllerTest::SetFrequencyBands(std::vector return true; } +bool +AccessPointControllerTest::SetSssid(std::string_view ssid) +{ + if (AccessPoint == nullptr) { + throw std::runtime_error("AccessPointControllerTest::SetSssid called with null AccessPoint"); + } + + AccessPoint->Ssid = ssid; + return true; +} + AccessPointControllerFactoryTest::AccessPointControllerFactoryTest(AccessPointTest *accessPoint) : AccessPoint(accessPoint) {} diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx index 8ef57133..de77be86 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx @@ -75,6 +75,16 @@ struct AccessPointControllerTest final : */ virtual bool SetFrequencyBands(std::vector frequencyBands) override; + + /** + * @brief Set the SSID of the access point. + * + * @param ssid The SSID to be set. + * @return true + * @return false + */ + bool + SetSssid(std::string_view ssid) override; }; /** diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx index 5e832b3a..ea9e4f06 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointTest.hxx @@ -28,6 +28,7 @@ struct AccessPointTest final : bool IsEnabled{ false }; Microsoft::Net::Wifi::Ieee80211Protocol Protocol; std::vector FrequencyBands; + std::string Ssid; /** * @brief Construct a new AccessPointTest object with the given interface name and default capabilities. From 6b9279964a3d0cc92c046c464f26ac1d352a1cb8 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 9 Feb 2024 15:11:18 +0000 Subject: [PATCH 02/21] Fix poorly worded comment. --- src/linux/wpa-controller/Hostapd.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx index 676492f5..10c5a731 100644 --- a/src/linux/wpa-controller/Hostapd.cxx +++ b/src/linux/wpa-controller/Hostapd.cxx @@ -61,7 +61,7 @@ Hostapd::GetProperty(std::string_view propertyName, std::string& propertyValue) throw HostapdException("Failed to send hostapd 'get' command"); } - // Check Failed() and not IsOk() since the response will indicate failure + // Check Failed() instead of IsOk() since the response will indicate failure // with "FAIL", otherwise, the payload is the property value (not "OK"). if (response->Failed()) { return false; From 2d7e1dd216ac3d2274eb642d9108c30b3b2bf030 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 9 Feb 2024 15:15:12 +0000 Subject: [PATCH 03/21] Historical fixups. --- .../wifi/test/AccessPointControllerTest.hxx | 50 ++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx index de77be86..09289d29 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx @@ -7,6 +7,8 @@ #include #include +#include +#include namespace Microsoft::Net::Wifi::Test { @@ -18,25 +20,37 @@ struct AccessPointTest; * This implementation takes an AccessPointTest object which it uses to implement the IAccessPointController interface. * The owner of this class must ensure that the passes AccessPointTest* remains valid for the lifetime of this object. */ -struct AccessPointControllerTest final : - public Microsoft::Net::Wifi::IAccessPointController +struct AccessPointControllerTest final : public Microsoft::Net::Wifi::IAccessPointController { AccessPointTest *AccessPoint{ nullptr }; + AccessPointControllerTest() = default; + ~AccessPointControllerTest() override = default; + /** * @brief Construct a new AccessPointControllerTest object that uses the specified AccessPointTest to implement the * IAccessPointController interface. * * @param accessPoint The access point to use. */ - AccessPointControllerTest(AccessPointTest *accessPoint); + explicit AccessPointControllerTest(AccessPointTest *accessPoint); + + /** + * Prevent copying and moving of AccessPointControllerTest objects. + */ + AccessPointControllerTest(const AccessPointControllerTest &) = delete; + AccessPointControllerTest & + operator=(const AccessPointControllerTest &) = delete; + AccessPointControllerTest(AccessPointControllerTest &&) = delete; + AccessPointControllerTest & + operator=(AccessPointControllerTest &&) = delete; /** * @brief Get the interface name associated with this controller. * * @return std::string_view */ - virtual std::string_view + std::string_view GetInterfaceName() const override; /** @@ -45,7 +59,7 @@ struct AccessPointControllerTest final : * @return true * @return false */ - virtual bool + bool GetIsEnabled() override; /** @@ -53,7 +67,7 @@ struct AccessPointControllerTest final : * * @return Ieee80211AccessPointCapabilities */ - virtual Ieee80211AccessPointCapabilities + Ieee80211AccessPointCapabilities GetCapabilities() override; /** @@ -63,7 +77,7 @@ struct AccessPointControllerTest final : * @return true * @return false */ - virtual bool + bool SetProtocol(Microsoft::Net::Wifi::Ieee80211Protocol ieeeProtocol) override; /** @@ -73,7 +87,7 @@ struct AccessPointControllerTest final : * @return true * @return false */ - virtual bool + bool SetFrequencyBands(std::vector frequencyBands) override; /** @@ -90,11 +104,12 @@ struct AccessPointControllerTest final : /** * @brief IAccessPointControllerFactory implementation for testing purposes. */ -struct AccessPointControllerFactoryTest final : - public Microsoft::Net::Wifi::IAccessPointControllerFactory +struct AccessPointControllerFactoryTest final : public Microsoft::Net::Wifi::IAccessPointControllerFactory { AccessPointTest *AccessPoint{ nullptr }; + ~AccessPointControllerFactoryTest() override = default; + /** * @brief Construct a new AccessPointControllerFactoryTest object with no AccessPointTest parent/owner. This may * only be used for cases where the constructed object won't actually be used (eg. unit tests for other components @@ -105,11 +120,22 @@ struct AccessPointControllerFactoryTest final : AccessPointControllerFactoryTest() = default; /** - * @brief Construct a new AccessPointControllerFactoryTest object. The owner of this object must ensure that the passed AccessPointTest remains valid for the lifetime of this object. + * @brief Construct a new AccessPointControllerFactoryTest object. The owner of this object must ensure that the + * passed AccessPointTest remains valid for the lifetime of this object. * * @param accessPoint The access point to create controllers for. */ - AccessPointControllerFactoryTest(AccessPointTest *accessPoint); + explicit AccessPointControllerFactoryTest(AccessPointTest *accessPoint); + + /** + * Prevent copying and moving of AccessPointControllerFactoryTest objects. + */ + AccessPointControllerFactoryTest(const AccessPointControllerFactoryTest &) = delete; + AccessPointControllerFactoryTest & + operator=(const AccessPointControllerFactoryTest &) = delete; + AccessPointControllerFactoryTest(AccessPointControllerFactoryTest &&) = delete; + AccessPointControllerFactoryTest & + operator=(AccessPointControllerFactoryTest &&) = delete; /** * @brief Create a new instance that can control the access point. From f84c4199d4e7895fee07d2ba388799362b05f017 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 9 Feb 2024 16:57:54 +0000 Subject: [PATCH 04/21] Add status code conversions. --- .../dot11/adapter/Ieee80211Dot11Adapters.cxx | 44 +++++++++++++++++++ .../net/wifi/Ieee80211Dot11Adapters.hxx | 20 +++++++++ 2 files changed, 64 insertions(+) diff --git a/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx b/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx index 54aa5bcf..eff4e3cd 100644 --- a/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx +++ b/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -14,6 +15,49 @@ namespace Microsoft::Net::Wifi { +using Microsoft::Net::Remote::Wifi::WifiAccessPointOperationStatusCode; +using Microsoft::Net::Wifi::AccessPointOperationStatusCode; + +WifiAccessPointOperationStatusCode +ToDot11AccessPointOperationStatusCode(AccessPointOperationStatusCode& accessPointOperationStatusCode) noexcept +{ + switch (accessPointOperationStatusCode) { + case AccessPointOperationStatusCode::Succeeded: + return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeSucceeded; + case AccessPointOperationStatusCode::InvalidParameter: + return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInvalidParameter; + case AccessPointOperationStatusCode::AccessPointInvalid: + return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeAccessPointInvalid; + case AccessPointOperationStatusCode::AccessPointNotEnabled: + return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeAccessPointNotEnabled; + case AccessPointOperationStatusCode::OperationNotSupported: + return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeOperationNotSupported; + case AccessPointOperationStatusCode::Unknown: + default: + return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeUnknown; + } +} + +AccessPointOperationStatusCode +FromDot11AccessPointOperationStatusCode(WifiAccessPointOperationStatusCode wifiAccessPointOperationStatusCode) noexcept +{ + switch (wifiAccessPointOperationStatusCode) { + case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeSucceeded: + return AccessPointOperationStatusCode::Succeeded; + case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInvalidParameter: + return AccessPointOperationStatusCode::InvalidParameter; + case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeAccessPointInvalid: + return AccessPointOperationStatusCode::AccessPointInvalid; + case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeAccessPointNotEnabled: + return AccessPointOperationStatusCode::AccessPointNotEnabled; + case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeOperationNotSupported: + return AccessPointOperationStatusCode::OperationNotSupported; + case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeUnknown: + default: + return AccessPointOperationStatusCode::Unknown; + } +} + using Microsoft::Net::Wifi::Dot11PhyType; using Microsoft::Net::Wifi::Ieee80211Protocol; diff --git a/src/common/wifi/dot11/adapter/include/microsoft/net/wifi/Ieee80211Dot11Adapters.hxx b/src/common/wifi/dot11/adapter/include/microsoft/net/wifi/Ieee80211Dot11Adapters.hxx index 0722d813..275b7fc0 100644 --- a/src/common/wifi/dot11/adapter/include/microsoft/net/wifi/Ieee80211Dot11Adapters.hxx +++ b/src/common/wifi/dot11/adapter/include/microsoft/net/wifi/Ieee80211Dot11Adapters.hxx @@ -4,12 +4,32 @@ #include +#include #include +#include #include #include namespace Microsoft::Net::Wifi { +/** + * @brief Convert the specified WifiAccessPointOperationStatusCode to the equivalent AccessPointOperationStatus. + * + * @param accessPointOperationStatusCode The WifiAccessPointOperationStatusCode to convert. + * @return Microsoft::Net::Remote::Wifi::WifiAccessPointOperationStatusCode + */ +Microsoft::Net::Remote::Wifi::WifiAccessPointOperationStatusCode +ToDot11AccessPointOperationStatusCode(Microsoft::Net::Wifi::AccessPointOperationStatusCode& accessPointOperationStatusCode) noexcept; + +/** + * @brief Convert the specified AccessPointOperationStatus to the equivalent WifiAccessPointOperationStatusCode. + * + * @param wifiAccessPointOperationStatusCode The AccessPointOperationStatus to convert. + * @return Microsoft::Net::Wifi::AccessPointOperationStatusCode + */ +Microsoft::Net::Wifi::AccessPointOperationStatusCode +FromDot11AccessPointOperationStatusCode(Microsoft::Net::Remote::Wifi::WifiAccessPointOperationStatusCode wifiAccessPointOperationStatusCode) noexcept; + /** * @brief Convert the specified Dot11PhyType to the equivalent IEEE 802.11 protocol. * From efd100cacdecd84d56555d99f34dec533de073ce Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 9 Feb 2024 20:27:51 +0000 Subject: [PATCH 05/21] Return rich status type from SetSsid. --- .../wifi/core/AccessPointOperationStatus.cxx | 16 +++++ src/common/wifi/core/CMakeLists.txt | 2 + .../net/wifi/AccessPointOperationStatus.hxx | 65 +++++++++++++++++++ .../net/wifi/IAccessPointController.hxx | 22 ++++--- .../wifi/core/AccessPointControllerLinux.cxx | 16 +++-- .../net/wifi/AccessPointControllerLinux.hxx | 27 +++++--- .../helpers/AccessPointControllerTest.cxx | 6 +- .../wifi/test/AccessPointControllerTest.hxx | 8 +-- 8 files changed, 132 insertions(+), 30 deletions(-) create mode 100644 src/common/wifi/core/AccessPointOperationStatus.cxx create mode 100644 src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx diff --git a/src/common/wifi/core/AccessPointOperationStatus.cxx b/src/common/wifi/core/AccessPointOperationStatus.cxx new file mode 100644 index 00000000..a0bdc2b9 --- /dev/null +++ b/src/common/wifi/core/AccessPointOperationStatus.cxx @@ -0,0 +1,16 @@ + +#include + +using namespace Microsoft::Net::Wifi; + +/* static */ +AccessPointOperationStatus +AccessPointOperationStatus::MakeSucceeded() noexcept +{ + return AccessPointOperationStatus{ AccessPointOperationStatusCode::Succeeded }; +} + +AccessPointOperationStatus::operator bool() const noexcept +{ + return (Code == AccessPointOperationStatusCode::Succeeded); +} diff --git a/src/common/wifi/core/CMakeLists.txt b/src/common/wifi/core/CMakeLists.txt index cd0d2a97..bbf872ba 100644 --- a/src/common/wifi/core/CMakeLists.txt +++ b/src/common/wifi/core/CMakeLists.txt @@ -10,12 +10,14 @@ target_sources(wifi-core AccessPoint.cxx AccessPointController.cxx AccessPointControllerException.cxx + AccessPointOperationStatus.cxx PUBLIC FILE_SET HEADERS BASE_DIRS ${WIFI_CORE_PUBLIC_INCLUDE} FILES ${WIFI_CORE_PUBLIC_INCLUDE_PREFIX}/AccessPoint.hxx ${WIFI_CORE_PUBLIC_INCLUDE_PREFIX}/AccessPointController.hxx + ${WIFI_CORE_PUBLIC_INCLUDE_PREFIX}/AccessPointOperationStatus.hxx ${WIFI_CORE_PUBLIC_INCLUDE_PREFIX}/IAccessPoint.hxx ${WIFI_CORE_PUBLIC_INCLUDE_PREFIX}/IAccessPointController.hxx ${WIFI_CORE_PUBLIC_INCLUDE_PREFIX}/Ieee80211.hxx diff --git a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx new file mode 100644 index 00000000..3137cbc1 --- /dev/null +++ b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx @@ -0,0 +1,65 @@ + +#ifndef ACCESS_POINT_OPERATION_STATUS_HXX +#define ACCESS_POINT_OPERATION_STATUS_HXX + +#include + +namespace Microsoft::Net::Wifi +{ +/** + * @brief High-level status reported by various access point operations. + */ +enum class AccessPointOperationStatusCode { + Unknown, + Succeeded, + InvalidParameter, + AccessPointInvalid, + AccessPointNotEnabled, + OperationNotSupported, +}; + +/** + * @brief Coarse status of an access point operation. + */ +struct AccessPointOperationStatus +{ + AccessPointOperationStatusCode Code{ AccessPointOperationStatusCode::Unknown }; + std::string Message; + + AccessPointOperationStatus() = default; + virtual ~AccessPointOperationStatus() = default; + + /** + * @brief Create an AccessPointOperationStatus with the given status code. + */ + constexpr explicit AccessPointOperationStatus(AccessPointOperationStatusCode code) noexcept : + Code{ code } + {} + + AccessPointOperationStatus(const AccessPointOperationStatus &) = default; + AccessPointOperationStatus(AccessPointOperationStatus &&) = default; + AccessPointOperationStatus & + operator=(const AccessPointOperationStatus &) = default; + AccessPointOperationStatus & + operator=(AccessPointOperationStatus &&) = default; + + /** + * @brief Create an AccessPointOperationStatus describing an operation that succeeded. + * + * @return AccessPointOperationStatus + */ + static AccessPointOperationStatus + MakeSucceeded() noexcept; + + /** + * @brief Implicit bool operator allowing AccessPointOperationStatus to be used directly in condition statements + * (eg. if (status) // ...). + * + * @return true + * @return false + */ + operator bool() const noexcept; // NOLINT(hicpp-explicit-conversions) +}; +} // namespace Microsoft::Net::Wifi + +#endif // ACCESS_POINT_OPERATION_STATUS_HXX diff --git a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx index a93723cc..15a7eda4 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/IAccessPointController.hxx @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -41,12 +42,14 @@ struct IAccessPointController virtual ~IAccessPointController() = default; /** - * Prevent copying and moving of IAccessPointController objects. + * Prevent copying and moving of IAccessPointController objects. */ IAccessPointController(const IAccessPointController&) = delete; - IAccessPointController& operator=(const IAccessPointController&) = delete; + IAccessPointController& + operator=(const IAccessPointController&) = delete; IAccessPointController(IAccessPointController&&) = delete; - IAccessPointController& operator=(IAccessPointController&&) = delete; + IAccessPointController& + operator=(IAccessPointController&&) = delete; /** * @brief Get the interface name associated with this controller. @@ -98,10 +101,9 @@ struct IAccessPointController * @brief Set the SSID of the access point. * * @param ssid The SSID to be set. - * @return true - * @return false + * @return AccessPointOperationStatus */ - virtual bool + virtual AccessPointOperationStatus SetSssid(std::string_view ssid) = 0; }; @@ -115,12 +117,14 @@ struct IAccessPointControllerFactory virtual ~IAccessPointControllerFactory() = default; /** - * Prevent copying and moving of IAccessPointControllerFactory objects. + * Prevent copying and moving of IAccessPointControllerFactory objects. */ IAccessPointControllerFactory(const IAccessPointControllerFactory&) = delete; - IAccessPointControllerFactory& operator=(const IAccessPointControllerFactory&) = delete; + IAccessPointControllerFactory& + operator=(const IAccessPointControllerFactory&) = delete; IAccessPointControllerFactory(IAccessPointControllerFactory&&) = delete; - IAccessPointControllerFactory& operator=(IAccessPointControllerFactory&&) = delete; + IAccessPointControllerFactory& + operator=(IAccessPointControllerFactory&&) = delete; /** * @brief Create a new IAccessPointController object. diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx index 1317bc87..12645320 100644 --- a/src/linux/wifi/core/AccessPointControllerLinux.cxx +++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -193,7 +194,7 @@ bool AccessPointControllerLinux::SetProtocol(Microsoft::Net::Wifi::Ieee80211Protocol ieeeProtocol) { bool isOk = false; - Wpa::HostapdHwMode hwMode = detail::IeeeProtocolToHostapdHwMode(ieeeProtocol); + const Wpa::HostapdHwMode hwMode = detail::IeeeProtocolToHostapdHwMode(ieeeProtocol); try { // Set the hostapd hw_mode property. @@ -249,7 +250,7 @@ AccessPointControllerLinux::SetFrequencyBands(std::vector diff --git a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx index dfec38d9..3ea6ea44 100644 --- a/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx +++ b/src/linux/wifi/core/include/microsoft/net/wifi/AccessPointControllerLinux.hxx @@ -2,12 +2,16 @@ #ifndef ACCESS_POINT_CONTROLLER_LINUX_HXX #define ACCESS_POINT_CONTROLLER_LINUX_HXX +#include #include #include #include #include +#include +#include #include +#include namespace Microsoft::Net::Wifi { @@ -29,12 +33,14 @@ struct AccessPointControllerLinux : explicit AccessPointControllerLinux(std::string_view interfaceName); /** - * Prevent copying and moving of this object. + * Prevent copying and moving of this object. */ AccessPointControllerLinux(const AccessPointControllerLinux&) = delete; - AccessPointControllerLinux& operator=(const AccessPointControllerLinux&) = delete; + AccessPointControllerLinux& + operator=(const AccessPointControllerLinux&) = delete; AccessPointControllerLinux(AccessPointControllerLinux&&) = delete; - AccessPointControllerLinux& operator=(AccessPointControllerLinux&&) = delete; + AccessPointControllerLinux& + operator=(AccessPointControllerLinux&&) = delete; /** * @brief Get whether the access point is enabled. @@ -75,12 +81,11 @@ struct AccessPointControllerLinux : /** * @brief Set the SSID of the access point. - * + * * @param ssid The SSID to be set. - * @return true - * @return false + * @return AccessPointOperationStatus */ - bool + AccessPointOperationStatus SetSssid(std::string_view ssid) override; private: @@ -98,12 +103,14 @@ struct AccessPointControllerLinuxFactory : ~AccessPointControllerLinuxFactory() override = default; /** - * Prevent copying and moving of this object. + * Prevent copying and moving of this object. */ AccessPointControllerLinuxFactory(const AccessPointControllerLinuxFactory&) = delete; - AccessPointControllerLinuxFactory& operator=(const AccessPointControllerLinuxFactory&) = delete; + AccessPointControllerLinuxFactory& + operator=(const AccessPointControllerLinuxFactory&) = delete; AccessPointControllerLinuxFactory(AccessPointControllerLinuxFactory&&) = delete; - AccessPointControllerLinuxFactory& operator=(AccessPointControllerLinuxFactory&&) = delete; + AccessPointControllerLinuxFactory& + operator=(AccessPointControllerLinuxFactory&&) = delete; /** * @brief Create a new IAccessPointController object. diff --git a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx index fa638265..b1594b10 100644 --- a/tests/unit/wifi/helpers/AccessPointControllerTest.cxx +++ b/tests/unit/wifi/helpers/AccessPointControllerTest.cxx @@ -1,5 +1,4 @@ - #include #include #include @@ -10,6 +9,7 @@ #include #include +#include #include #include #include @@ -83,7 +83,7 @@ AccessPointControllerTest::SetFrequencyBands(std::vector return true; } -bool +AccessPointOperationStatus AccessPointControllerTest::SetSssid(std::string_view ssid) { if (AccessPoint == nullptr) { @@ -91,7 +91,7 @@ AccessPointControllerTest::SetSssid(std::string_view ssid) } AccessPoint->Ssid = ssid; - return true; + return AccessPointOperationStatus::MakeSucceeded(); } AccessPointControllerFactoryTest::AccessPointControllerFactoryTest(AccessPointTest *accessPoint) : diff --git a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx index 09289d29..b47fa305 100644 --- a/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx +++ b/tests/unit/wifi/helpers/include/microsoft/net/wifi/test/AccessPointControllerTest.hxx @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -92,12 +93,11 @@ struct AccessPointControllerTest final : public Microsoft::Net::Wifi::IAccessPoi /** * @brief Set the SSID of the access point. - * + * * @param ssid The SSID to be set. - * @return true - * @return false + * @return AccessPointOperationStatus */ - bool + AccessPointOperationStatus SetSssid(std::string_view ssid) override; }; From 7eae55b500958b5727eb7c290b6bc4a032fa636c Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 9 Feb 2024 20:52:52 +0000 Subject: [PATCH 06/21] Add internal error. --- .../include/microsoft/net/wifi/AccessPointOperationStatus.hxx | 1 + src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx index 3137cbc1..e40445b1 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx @@ -16,6 +16,7 @@ enum class AccessPointOperationStatusCode { AccessPointInvalid, AccessPointNotEnabled, OperationNotSupported, + InternalError, }; /** diff --git a/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx b/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx index eff4e3cd..0c7d9a17 100644 --- a/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx +++ b/src/common/wifi/dot11/adapter/Ieee80211Dot11Adapters.cxx @@ -32,6 +32,8 @@ ToDot11AccessPointOperationStatusCode(AccessPointOperationStatusCode& accessPoin return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeAccessPointNotEnabled; case AccessPointOperationStatusCode::OperationNotSupported: return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeOperationNotSupported; + case AccessPointOperationStatusCode::InternalError: + return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInternalError; case AccessPointOperationStatusCode::Unknown: default: return WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeUnknown; @@ -52,6 +54,8 @@ FromDot11AccessPointOperationStatusCode(WifiAccessPointOperationStatusCode wifiA return AccessPointOperationStatusCode::AccessPointNotEnabled; case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeOperationNotSupported: return AccessPointOperationStatusCode::OperationNotSupported; + case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeInternalError: + return AccessPointOperationStatusCode::InternalError; case WifiAccessPointOperationStatusCode::WifiAccessPointOperationStatusCodeUnknown: default: return AccessPointOperationStatusCode::Unknown; From 52c6f08f2255a4641f1aa0fe08bc7246743af0e8 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 9 Feb 2024 21:11:41 +0000 Subject: [PATCH 07/21] Implement AccessPointControllerLinux::SetSssid. --- .../net/wifi/AccessPointOperationStatus.hxx | 2 +- .../wifi/core/AccessPointControllerLinux.cxx | 44 ++++++++++++++++--- .../include/Wpa/ProtocolHostapd.hxx | 2 + 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx index e40445b1..ec424b66 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/AccessPointOperationStatus.hxx @@ -52,7 +52,7 @@ struct AccessPointOperationStatus static AccessPointOperationStatus MakeSucceeded() noexcept; - /** + /** * @brief Implicit bool operator allowing AccessPointOperationStatus to be used directly in condition statements * (eg. if (status) // ...). * diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx index 12645320..62de7a80 100644 --- a/src/linux/wifi/core/AccessPointControllerLinux.cxx +++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx @@ -275,17 +275,51 @@ AccessPointControllerLinux::SetFrequencyBands(std::vector diff --git a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx index 6b50b9b9..bb6eee1b 100644 --- a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx +++ b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx @@ -201,6 +201,8 @@ struct ProtocolHostapd : static constexpr auto PropertyHwModeValueAD = "ad"; static constexpr auto PropertyHwModeValueAny = "any"; + static constexpr auto PropertyNameSsid = "ssid"; + // Macros below used to avoid repeating the same string literals in multiple places. // NOLINTBEGIN(cppcoreguidelines-macro-usage) #define HOSTAPD_PROPERTY_KEY_VALUE_DELIMETER "=" From 4e1aa96699e8cbc06cb214a8697791aadfef8eb5 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 16 Feb 2024 18:30:55 +0000 Subject: [PATCH 08/21] Fix typos. --- src/linux/wpa-controller/WpaCommandStatus.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/linux/wpa-controller/WpaCommandStatus.cxx b/src/linux/wpa-controller/WpaCommandStatus.cxx index f451f414..e73e0ea9 100644 --- a/src/linux/wpa-controller/WpaCommandStatus.cxx +++ b/src/linux/wpa-controller/WpaCommandStatus.cxx @@ -59,15 +59,15 @@ WpaStatusResponseParser::ParsePayload() } if (!(status.Ieee80211n == 0)) { - // TODO: parse attributes that are preset when this value is set. + // TODO: parse attributes that are present when this value is set. } if (!(status.Ieee80211ac == 0)) { - // TODO: parse attributes that are preset when this value is set. + // TODO: parse attributes that are present when this value is set. } if (!(status.Ieee80211ax == 0)) { - // TODO: parse attributes that are preset when this value is set. + // TODO: parse attributes that are present when this value is set. } return response; From 7a95a9130178985a1a5ec957c0e5a9b010e471a3 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 16 Feb 2024 18:31:23 +0000 Subject: [PATCH 09/21] Fix static member access through variable. --- src/linux/wpa-controller/WpaKeyValuePair.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/linux/wpa-controller/WpaKeyValuePair.cxx b/src/linux/wpa-controller/WpaKeyValuePair.cxx index 3ab5fece..6969e08b 100644 --- a/src/linux/wpa-controller/WpaKeyValuePair.cxx +++ b/src/linux/wpa-controller/WpaKeyValuePair.cxx @@ -16,13 +16,13 @@ WpaKeyValuePair::TryParseValue(std::string_view input) if (!Value.has_value()) { // Find the starting position of the key. const auto keyPosition = input.find(Key); - if (keyPosition != input.npos) { + if (keyPosition != std::string_view::npos) { // Assign the starting position of the value, advancing past the key. input = input.substr(keyPosition + std::size(Key)); // Find the end of the line, marking the end of the value string. const auto eolPosition = input.find('\n'); - if (eolPosition != input.npos) { + if (eolPosition != std::string_view::npos) { Value = input.substr(0, eolPosition); } else { LOGW << std::format("Failed to parse value for key: {} (missing eol)", Key); From 51b1def4022c4e9dde4ab3bf414b2a343425adf3 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Fri, 16 Feb 2024 18:40:43 +0000 Subject: [PATCH 10/21] Add SetSsid helper function. --- src/linux/wpa-controller/Hostapd.cxx | 22 +++++++++++++++++++ .../wpa-controller/include/Wpa/Hostapd.hxx | 10 +++++++++ 2 files changed, 32 insertions(+) diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx index 10c5a731..bd38322a 100644 --- a/src/linux/wpa-controller/Hostapd.cxx +++ b/src/linux/wpa-controller/Hostapd.cxx @@ -11,6 +11,7 @@ #include #include #include +#include using namespace Wpa; @@ -52,6 +53,27 @@ Hostapd::GetStatus() return response->Status; } +bool +Hostapd::SetSsid(std::string_view ssid, bool reload) +{ + const bool ssidWasSet = SetProperty(ProtocolHostapd::PropertyNameSsid, ssid); + if (!ssidWasSet) { + throw HostapdException("Failed to set hostapd 'ssid' property"); + } + + if (!reload) { + LOGW << "Skipping hostapd reload after setting 'ssid' (requested)"; + return true; + } + + const bool configurationReloadSucceeded = Reload(); + if (!configurationReloadSucceeded) { + throw HostapdException("Failed to reload hostapd configuration"); + } + + return true; +} + bool Hostapd::GetProperty(std::string_view propertyName, std::string& propertyValue) { diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx index 239eca4a..ebe3b2b0 100644 --- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx +++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx @@ -73,6 +73,16 @@ struct Hostapd : HostapdStatus GetStatus() override; + /** + * @brief Set the ssid of the access point. + * + * @param ssid The ssid to set. + * @return true + * @return false + */ + bool + SetSsid(std::string_view ssid, bool reload = true); + /** * @brief Get a property value for the interface. * From e4a2a092b6f8a9da3e3e7a79a5bc45979eee099f Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sat, 17 Feb 2024 00:04:10 +0000 Subject: [PATCH 11/21] Perform complete kv string parsing in common wpa parser class. --- .../wpa-controller/WpaResponseParser.cxx | 38 +++++++++++++++++++ .../include/Wpa/ProtocolWpa.hxx | 1 + 2 files changed, 39 insertions(+) diff --git a/src/linux/wpa-controller/WpaResponseParser.cxx b/src/linux/wpa-controller/WpaResponseParser.cxx index 4f4fd46a..803a9eba 100644 --- a/src/linux/wpa-controller/WpaResponseParser.cxx +++ b/src/linux/wpa-controller/WpaResponseParser.cxx @@ -1,10 +1,13 @@ #include #include +#include #include +#include #include #include +#include #include #include #include @@ -53,10 +56,45 @@ WpaResponseParser::GetResponsePayload() const noexcept bool WpaResponseParser::TryParseProperties() { + // Convert a range to a string-view for pre-C++23 where std::string_view doesn't have a range constructor. + constexpr auto toStringView = [](auto&& sv) { + return std::string_view{ std::data(sv), std::size(sv) }; + }; + if (std::empty(m_propertiesToParse)) { return true; } + // Parse the payload into individual lines containing key value pairs. + auto lines = m_responsePayload | std::views::split(ProtocolWpa::KeyValueLineDelimeter) | std::views::transform(toStringView); + + // Parse each line into a key-value pair, and populate a map with them. + std::unordered_map properties; + for (const auto line : lines) { + auto keyValuePair = line | std::views::split(ProtocolWpa::KeyValueDelimiter) | std::views::transform(toStringView); + auto keyValuePairIterator = std::begin(keyValuePair); + + if (keyValuePairIterator == std::end(keyValuePair)) { + LOGV << std::format("Skipping empty key value pair line '{}'", line); + continue; + } + + // Ensure a delimited key and value were found. + const auto numElements = std::ranges::distance(keyValuePairIterator, std::end(keyValuePair)); + if (numElements < 2) { + LOGW << std::format("Skipping key value pair line '{}' with less than 2 elements ({})", line, numElements); + continue; + } + + // Parse out the key and value, ensuring the key is non-empty (empty values are allowed). + const auto key = *std::next(keyValuePairIterator, 0); + const auto value = *std::next(keyValuePairIterator, 1); + if (!std::empty(key)) { + properties[key] = value; + LOGV << std::format("[{}] {}={}", std::size(properties), key, value); + } + } + for (auto propertyToParseIterator = std::begin(m_propertiesToParse); propertyToParseIterator != std::end(m_propertiesToParse);) { auto& propertyToParse = *propertyToParseIterator; auto propertyValue = propertyToParse.TryParseValue(m_responsePayload); diff --git a/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx b/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx index 18e0b990..25a8633c 100644 --- a/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx +++ b/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx @@ -13,6 +13,7 @@ struct ProtocolWpa { // The delimeter used to separate key-value pairs in the WPA control protocol. static constexpr auto KeyValueDelimiter = '='; + static constexpr auto KeyValueLineDelimeter = '\n'; // Command payloads. static constexpr auto CommandPayloadPing = "PING"; From 5f81635f55f445ffd397950a173dde8e1825c7fb Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sat, 17 Feb 2024 00:05:53 +0000 Subject: [PATCH 12/21] Add properties for hostapd status bss array. --- .../include/Wpa/ProtocolHostapd.hxx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx index bb6eee1b..fac2ad94 100644 --- a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx +++ b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx @@ -212,20 +212,31 @@ struct ProtocolHostapd : #define HOSTAPD_PROPERTY_NAME_DISABLE11AC "disable_11ac" #define HOSTAPD_PROPERTY_NAME_IEEE80211AX "ieee80211ax" #define HOSTAPD_PROPERTY_NAME_DISABLE11AX "disable_11ax" +#define HOSTAPD_PROPERTY_NAME_WMM_ENABLED "wmm_enabled" #define HOSTAPD_PROPERTY_NAME_STATE "state" +#define HOSTAPD_PROPERTY_NAME_BSS "bss" +#define HOSTAPD_PROPERTY_NAME_BSS_BSSID "bssid" +#define HOSTAPD_PROPERTY_NAME_BSS_SSID "ssid" +#define HOSTAPD_PROPERTY_NAME_BSS_NUM_STA "num_sta" // Helper macro to create a hostapd configuration file key with value delimiter. #define HOSTAPD_DELIMITED_KEY(name) name HOSTAPD_PROPERTY_KEY_VALUE_DELIMETER - // NOLINTEND(cppcoreguidelines-macro-usage) +// NOLINTEND(cppcoreguidelines-macro-usage) - static constexpr auto ProperyKeyValueNameDelimeter = HOSTAPD_PROPERTY_KEY_VALUE_DELIMETER; static constexpr auto PropertyNameIeee80211N = HOSTAPD_PROPERTY_NAME_IEEE80211N; static constexpr auto PropertyNameDisable11N = HOSTAPD_PROPERTY_NAME_DISABLE11N; static constexpr auto PropertyNameIeee80211AC = HOSTAPD_PROPERTY_NAME_IEEE80211AC; static constexpr auto PropertyNameDisable11AC = HOSTAPD_PROPERTY_NAME_DISABLE11AC; static constexpr auto PropertyNameIeee80211AX = HOSTAPD_PROPERTY_NAME_IEEE80211AX; static constexpr auto PropertyNameDisable11AX = HOSTAPD_PROPERTY_NAME_DISABLE11AX; - static constexpr auto PropertyNameWmmEnabled = "wmm_enabled"; + static constexpr auto PropertyNameWmmEnabled = HOSTAPD_PROPERTY_NAME_WMM_ENABLED; + static constexpr auto PropertyNameState = HOSTAPD_PROPERTY_NAME_STATE; + + // Indexed property names for BSS entries in the "STATUS" response. + static constexpr auto PropertyNameBss = HOSTAPD_PROPERTY_NAME_BSS; + static constexpr auto PropertyNameBssBssid = HOSTAPD_PROPERTY_NAME_BSS_BSSID; + static constexpr auto PropertyNameBssSsid = HOSTAPD_PROPERTY_NAME_BSS_SSID; + static constexpr auto PropertyNameNumStations = HOSTAPD_PROPERTY_NAME_BSS_NUM_STA; // Response properties for the "STATUS" command. // Note: all properties must be terminated with the key-value delimeter (=). From 6e48b486eb4cce967951c79a0225c7e68e25cb1d Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sat, 17 Feb 2024 00:06:50 +0000 Subject: [PATCH 13/21] Add ability to mark kvs as indexed. --- .../include/Wpa/WpaKeyValuePair.hxx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx b/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx index a2c23e71..8d81c934 100644 --- a/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx +++ b/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -38,27 +39,27 @@ struct WpaKeyValuePair * * @param key The key of the property. * @param presence Whether the property is required or optional. + * @param isIndexed Whether the property is indexed (i.e. has a numeric suffix in its key name). */ - constexpr WpaKeyValuePair(std::string_view key, WpaValuePresence presence) : + constexpr WpaKeyValuePair(std::string_view key, WpaValuePresence presence, bool isIndexed = false) : Key(key), - IsRequired(notstd::to_underlying(presence)) + IsRequired(notstd::to_underlying(presence)), + IsIndexed(isIndexed) { // Ensure the specified key ends with the delimiter. If not, this is a // programming error so throw a runtime exception. - if (!Key.ends_with(KeyDelimiter)) { + if (!IsIndexed && !Key.ends_with(KeyDelimiter)) { throw std::runtime_error(std::format("Key must end with {} delimiter.", KeyDelimiter)); } } /** - * Parses the input string and attempts to resolve the property value, - * assigning it to the Value member if found. Note that this function does - * not parse the value itself, it only resolves the value location in the - * input string, making it available for later parsing by derived classes - * that know its type/structure. + * @brief Parses the input string and attempts to resolve the property value, assigning it to the Value member if + * found. Note that this function does * not parse the value itself, it only resolves the value location in the + * input string, making it available for later parsing by derived classes that know its type/structure. * * The input string is expected to contain a property of the form: - * key=value, as is encoded by the WPA control protocol. * @brief + * key=value, as is encoded by the WPA control protocol. * * @param input * @return std::optional @@ -69,6 +70,7 @@ struct WpaKeyValuePair std::string_view Key; std::optional Value; bool IsRequired{ true }; + bool IsIndexed{ false }; }; } // namespace Wpa From 8f75df385ee5284551a5390db28a91493d17e184 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sat, 17 Feb 2024 03:02:59 +0000 Subject: [PATCH 14/21] Remove hard-coded delimeter. --- src/linux/wpa-controller/WpaKeyValuePair.cxx | 24 -------- .../wpa-controller/WpaResponseParser.cxx | 36 ++++++----- .../include/Wpa/ProtocolHostapd.hxx | 59 ++++++------------- .../include/Wpa/WpaKeyValuePair.hxx | 21 ------- 4 files changed, 36 insertions(+), 104 deletions(-) diff --git a/src/linux/wpa-controller/WpaKeyValuePair.cxx b/src/linux/wpa-controller/WpaKeyValuePair.cxx index 6969e08b..ab60b940 100644 --- a/src/linux/wpa-controller/WpaKeyValuePair.cxx +++ b/src/linux/wpa-controller/WpaKeyValuePair.cxx @@ -8,27 +8,3 @@ #include using namespace Wpa; - -std::optional -WpaKeyValuePair::TryParseValue(std::string_view input) -{ - // If not already parsed... - if (!Value.has_value()) { - // Find the starting position of the key. - const auto keyPosition = input.find(Key); - if (keyPosition != std::string_view::npos) { - // Assign the starting position of the value, advancing past the key. - input = input.substr(keyPosition + std::size(Key)); - - // Find the end of the line, marking the end of the value string. - const auto eolPosition = input.find('\n'); - if (eolPosition != std::string_view::npos) { - Value = input.substr(0, eolPosition); - } else { - LOGW << std::format("Failed to parse value for key: {} (missing eol)", Key); - } - } - } - - return Value; -} diff --git a/src/linux/wpa-controller/WpaResponseParser.cxx b/src/linux/wpa-controller/WpaResponseParser.cxx index 803a9eba..36b81d03 100644 --- a/src/linux/wpa-controller/WpaResponseParser.cxx +++ b/src/linux/wpa-controller/WpaResponseParser.cxx @@ -1,4 +1,5 @@ +#include #include #include #include @@ -60,6 +61,14 @@ WpaResponseParser::TryParseProperties() constexpr auto toStringView = [](auto&& sv) { return std::string_view{ std::data(sv), std::size(sv) }; }; + // Convert a key-value pair to its key. + constexpr auto toKey = [](auto&& keyValuePair) { + return keyValuePair.Key; + }; + // Determine if the property map has the specified key. + const auto hasKey = [&](auto&& key) { + return m_properties.contains(key); + }; if (std::empty(m_propertiesToParse)) { return true; @@ -68,8 +77,7 @@ WpaResponseParser::TryParseProperties() // Parse the payload into individual lines containing key value pairs. auto lines = m_responsePayload | std::views::split(ProtocolWpa::KeyValueLineDelimeter) | std::views::transform(toStringView); - // Parse each line into a key-value pair, and populate a map with them. - std::unordered_map properties; + // Parse each line into a key-value pair, and populate the property map with them. for (const auto line : lines) { auto keyValuePair = line | std::views::split(ProtocolWpa::KeyValueDelimiter) | std::views::transform(toStringView); auto keyValuePairIterator = std::begin(keyValuePair); @@ -89,26 +97,16 @@ WpaResponseParser::TryParseProperties() // Parse out the key and value, ensuring the key is non-empty (empty values are allowed). const auto key = *std::next(keyValuePairIterator, 0); const auto value = *std::next(keyValuePairIterator, 1); - if (!std::empty(key)) { - properties[key] = value; - LOGV << std::format("[{}] {}={}", std::size(properties), key, value); - } - } - - for (auto propertyToParseIterator = std::begin(m_propertiesToParse); propertyToParseIterator != std::end(m_propertiesToParse);) { - auto& propertyToParse = *propertyToParseIterator; - auto propertyValue = propertyToParse.TryParseValue(m_responsePayload); - if (propertyValue.has_value()) { - m_properties[propertyToParse.Key] = *propertyValue; - propertyToParseIterator = m_propertiesToParse.erase(propertyToParseIterator); + if (std::empty(key)) { continue; } - if (propertyToParse.IsRequired) { - LOGE << std::format("Failed to parse required property: {}\nPayload {}\n", propertyToParse.Key, m_responsePayload); - return false; - } - ++propertyToParseIterator; + + m_properties[key] = value; + LOGV << std::format("Parsed [{:03}] {}={}", std::size(m_properties), key, value); } + // Remove parsed properties from the list of properties to parse. + m_propertiesToParse.erase(std::begin(std::ranges::remove_if(m_propertiesToParse, hasKey, toKey)), std::end(m_propertiesToParse)); + return true; } diff --git a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx index fac2ad94..44409495 100644 --- a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx +++ b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx @@ -203,51 +203,30 @@ struct ProtocolHostapd : static constexpr auto PropertyNameSsid = "ssid"; -// Macros below used to avoid repeating the same string literals in multiple places. -// NOLINTBEGIN(cppcoreguidelines-macro-usage) -#define HOSTAPD_PROPERTY_KEY_VALUE_DELIMETER "=" -#define HOSTAPD_PROPERTY_NAME_IEEE80211N "ieee80211n" -#define HOSTAPD_PROPERTY_NAME_DISABLE11N "disable_11n" -#define HOSTAPD_PROPERTY_NAME_IEEE80211AC "ieee80211ac" -#define HOSTAPD_PROPERTY_NAME_DISABLE11AC "disable_11ac" -#define HOSTAPD_PROPERTY_NAME_IEEE80211AX "ieee80211ax" -#define HOSTAPD_PROPERTY_NAME_DISABLE11AX "disable_11ax" -#define HOSTAPD_PROPERTY_NAME_WMM_ENABLED "wmm_enabled" -#define HOSTAPD_PROPERTY_NAME_STATE "state" -#define HOSTAPD_PROPERTY_NAME_BSS "bss" -#define HOSTAPD_PROPERTY_NAME_BSS_BSSID "bssid" -#define HOSTAPD_PROPERTY_NAME_BSS_SSID "ssid" -#define HOSTAPD_PROPERTY_NAME_BSS_NUM_STA "num_sta" - -// Helper macro to create a hostapd configuration file key with value delimiter. -#define HOSTAPD_DELIMITED_KEY(name) name HOSTAPD_PROPERTY_KEY_VALUE_DELIMETER -// NOLINTEND(cppcoreguidelines-macro-usage) - - static constexpr auto PropertyNameIeee80211N = HOSTAPD_PROPERTY_NAME_IEEE80211N; - static constexpr auto PropertyNameDisable11N = HOSTAPD_PROPERTY_NAME_DISABLE11N; - static constexpr auto PropertyNameIeee80211AC = HOSTAPD_PROPERTY_NAME_IEEE80211AC; - static constexpr auto PropertyNameDisable11AC = HOSTAPD_PROPERTY_NAME_DISABLE11AC; - static constexpr auto PropertyNameIeee80211AX = HOSTAPD_PROPERTY_NAME_IEEE80211AX; - static constexpr auto PropertyNameDisable11AX = HOSTAPD_PROPERTY_NAME_DISABLE11AX; - static constexpr auto PropertyNameWmmEnabled = HOSTAPD_PROPERTY_NAME_WMM_ENABLED; - static constexpr auto PropertyNameState = HOSTAPD_PROPERTY_NAME_STATE; + static constexpr auto PropertyNameIeee80211N = "ieee80211n"; + static constexpr auto PropertyNameDisable11N = "disable_11n"; + static constexpr auto PropertyNameIeee80211AC = "ieee80211ac"; + static constexpr auto PropertyNameDisable11AC = "disable_11ac"; + static constexpr auto PropertyNameIeee80211AX = "ieee80211ax"; + static constexpr auto PropertyNameDisable11AX = "disable_11ax"; + static constexpr auto PropertyNameWmmEnabled = "wmm_enabled"; + static constexpr auto PropertyNameState = "state"; // Indexed property names for BSS entries in the "STATUS" response. - static constexpr auto PropertyNameBss = HOSTAPD_PROPERTY_NAME_BSS; - static constexpr auto PropertyNameBssBssid = HOSTAPD_PROPERTY_NAME_BSS_BSSID; - static constexpr auto PropertyNameBssSsid = HOSTAPD_PROPERTY_NAME_BSS_SSID; - static constexpr auto PropertyNameNumStations = HOSTAPD_PROPERTY_NAME_BSS_NUM_STA; + static constexpr auto PropertyNameBss = "bss"; + static constexpr auto PropertyNameBssBssid = "bssid"; + static constexpr auto PropertyNameBssSsid = "ssid"; + static constexpr auto PropertyNameBssNumStations = "num_sta"; // Response properties for the "STATUS" command. // Note: all properties must be terminated with the key-value delimeter (=). - static constexpr auto PropertyNameState = HOSTAPD_PROPERTY_NAME_STATE; - static constexpr auto ResponseStatusPropertyKeyState = HOSTAPD_DELIMITED_KEY(HOSTAPD_PROPERTY_NAME_STATE); - static constexpr auto ResponseStatusPropertyKeyIeee80211N = HOSTAPD_DELIMITED_KEY(HOSTAPD_PROPERTY_NAME_IEEE80211N); - static constexpr auto ResponseStatusPropertyKeyDisable11N = HOSTAPD_DELIMITED_KEY(HOSTAPD_PROPERTY_NAME_DISABLE11N); - static constexpr auto ResponseStatusPropertyKeyIeee80211AC = HOSTAPD_DELIMITED_KEY(HOSTAPD_PROPERTY_NAME_IEEE80211AC); - static constexpr auto ResponseStatusPropertyKeyDisableAC = HOSTAPD_DELIMITED_KEY(HOSTAPD_PROPERTY_NAME_DISABLE11AC); - static constexpr auto ResponseStatusPropertyKeyIeee80211AX = HOSTAPD_DELIMITED_KEY(HOSTAPD_PROPERTY_NAME_IEEE80211AX); - static constexpr auto ResponseStatusPropertyKeyDisableAX = HOSTAPD_DELIMITED_KEY(HOSTAPD_PROPERTY_NAME_DISABLE11AX); + static constexpr auto ResponseStatusPropertyKeyState = PropertyNameState; + static constexpr auto ResponseStatusPropertyKeyIeee80211N = PropertyNameIeee80211N; + static constexpr auto ResponseStatusPropertyKeyDisable11N = PropertyNameDisable11N; + static constexpr auto ResponseStatusPropertyKeyIeee80211AC = PropertyNameIeee80211AC; + static constexpr auto ResponseStatusPropertyKeyDisableAC = PropertyNameDisable11AC; + static constexpr auto ResponseStatusPropertyKeyIeee80211AX = PropertyNameIeee80211AX; + static constexpr auto ResponseStatusPropertyKeyDisableAX = PropertyNameDisable11AX; }; /** diff --git a/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx b/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx index 8d81c934..247d22a6 100644 --- a/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx +++ b/src/linux/wpa-controller/include/Wpa/WpaKeyValuePair.hxx @@ -2,9 +2,7 @@ #ifndef WPA_KEY_VALUE_PAIR_HXX #define WPA_KEY_VALUE_PAIR_HXX -#include #include -#include #include #include @@ -46,27 +44,8 @@ struct WpaKeyValuePair IsRequired(notstd::to_underlying(presence)), IsIndexed(isIndexed) { - // Ensure the specified key ends with the delimiter. If not, this is a - // programming error so throw a runtime exception. - if (!IsIndexed && !Key.ends_with(KeyDelimiter)) { - throw std::runtime_error(std::format("Key must end with {} delimiter.", KeyDelimiter)); - } } - /** - * @brief Parses the input string and attempts to resolve the property value, assigning it to the Value member if - * found. Note that this function does * not parse the value itself, it only resolves the value location in the - * input string, making it available for later parsing by derived classes that know its type/structure. - * - * The input string is expected to contain a property of the form: - * key=value, as is encoded by the WPA control protocol. - * - * @param input - * @return std::optional - */ - std::optional - TryParseValue(std::string_view input); - std::string_view Key; std::optional Value; bool IsRequired{ true }; From 8eae8745a7b03ba3ea4a160bd5928d390d349f97 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sat, 17 Feb 2024 04:02:01 +0000 Subject: [PATCH 15/21] Add parsing code for bss entries. --- src/linux/wpa-controller/WpaCommandStatus.cxx | 16 ++++++++++++++++ .../include/Wpa/ProtocolHostapd.hxx | 6 ++++-- .../wpa-controller/include/Wpa/ProtocolWpa.hxx | 2 ++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/linux/wpa-controller/WpaCommandStatus.cxx b/src/linux/wpa-controller/WpaCommandStatus.cxx index e73e0ea9..6d086001 100644 --- a/src/linux/wpa-controller/WpaCommandStatus.cxx +++ b/src/linux/wpa-controller/WpaCommandStatus.cxx @@ -1,6 +1,7 @@ #include #include +#include #include #include @@ -39,6 +40,7 @@ WpaStatusResponseParser::ParsePayload() const auto properties = GetProperties(); const auto response = std::make_shared(); auto& status = response->Status; + BssInfo bssInfo{}; for (const auto& [key, value] : properties) { if (key == ProtocolHostapd::ResponseStatusPropertyKeyState) { @@ -55,6 +57,20 @@ WpaStatusResponseParser::ParsePayload() ParseInt(value, status.Ieee80211ax); } else if (key == ProtocolHostapd::ResponseStatusPropertyKeyDisableAX) { ParseInt(value, status.Disable11ax); + } else if (key.starts_with(ProtocolHostapd::PropertyNameBss)) { + bssInfo.Interface = key; + } else if (key.starts_with(ProtocolHostapd::PropertyNameBssBssid)) { + bssInfo.Bssid = value; + } else if (key.starts_with(ProtocolHostapd::PropertyNameBssSsid)) { + bssInfo.Ssid = value; + } else if (key.starts_with(ProtocolHostapd::PropertyNameBssNumStations)) { + ParseInt(value, bssInfo.NumStations); + // TODO: this assumes num_sta is the last property per bss entry. This is currently true, but the code + // should not make this assumption to protect against future changes in hostapd control interface protocol. + // Instead, the property index should be read and when it changes, the bssInfo should be added to the + // response and a new bssInfo should be created. + status.Bss.push_back(std::move(bssInfo)); + bssInfo = {}; } } diff --git a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx index 44409495..6d6d426b 100644 --- a/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx +++ b/src/linux/wpa-controller/include/Wpa/ProtocolHostapd.hxx @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -49,6 +50,7 @@ struct BssInfo int NumStations{ 0 }; std::string Bssid; std::string Interface; + std::string Ssid; // Present with: // - CONFIG_IEEE80211BE compilation option @@ -156,8 +158,8 @@ struct HostapdStatus // // - The current operational mode supports the currently active frequency. // std::optional MaxTxPower; - // // Always present (but may be empty). - // std::vector Bss; + // Always present (but may be empty). + std::vector Bss; // // Present with: // // - Non-zero channel utilization. diff --git a/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx b/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx index 25a8633c..2f4ef926 100644 --- a/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx +++ b/src/linux/wpa-controller/include/Wpa/ProtocolWpa.hxx @@ -14,6 +14,8 @@ struct ProtocolWpa // The delimeter used to separate key-value pairs in the WPA control protocol. static constexpr auto KeyValueDelimiter = '='; static constexpr auto KeyValueLineDelimeter = '\n'; + static constexpr auto KeyValueIndexDelimeterStart = '['; + static constexpr auto KeyValueIndexDelimeterEnd = ']'; // Command payloads. static constexpr auto CommandPayloadPing = "PING"; From 3a9962400ba9d1107f8bca45aa8df521b5a161a6 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sun, 18 Feb 2024 05:02:35 +0000 Subject: [PATCH 16/21] Parse key indices. --- src/linux/wpa-controller/WpaCommandStatus.cxx | 44 ++++++++++++------- .../wpa-controller/WpaResponseParser.cxx | 27 ++++++++++-- .../include/Wpa/WpaResponseParser.hxx | 35 +++++++++------ 3 files changed, 71 insertions(+), 35 deletions(-) diff --git a/src/linux/wpa-controller/WpaCommandStatus.cxx b/src/linux/wpa-controller/WpaCommandStatus.cxx index 6d086001..b5dfb1be 100644 --- a/src/linux/wpa-controller/WpaCommandStatus.cxx +++ b/src/linux/wpa-controller/WpaCommandStatus.cxx @@ -4,11 +4,13 @@ #include #include +#include #include #include #include #include #include +#include #include "WpaParsingUtilities.hxx" @@ -27,6 +29,10 @@ WpaStatusResponseParser::WpaStatusResponseParser(const WpaCommand* command, std: { ProtocolHostapd::ResponseStatusPropertyKeyIeee80211N, WpaValuePresence::Required }, { ProtocolHostapd::ResponseStatusPropertyKeyIeee80211AC, WpaValuePresence::Required }, { ProtocolHostapd::ResponseStatusPropertyKeyIeee80211AX, WpaValuePresence::Required }, + { ProtocolHostapd::PropertyNameBss, WpaValuePresence::Required }, + { ProtocolHostapd::PropertyNameBssSsid, WpaValuePresence::Required }, + { ProtocolHostapd::PropertyNameBssBssid, WpaValuePresence::Required }, + { ProtocolHostapd::PropertyNameBssNumStations, WpaValuePresence::Required }, }) // clang-format on { @@ -37,12 +43,18 @@ WpaStatusResponseParser::ParsePayload() { using namespace Wpa::Parsing; - const auto properties = GetProperties(); + const auto& properties = GetProperties(); const auto response = std::make_shared(); auto& status = response->Status; - BssInfo bssInfo{}; - for (const auto& [key, value] : properties) { + const auto enforceRequiredBssInfoSize = [&](const auto sizeRequired) { + if (std::size(status.Bss) < sizeRequired) { + status.Bss.resize(sizeRequired); + } + }; + + for (const auto& [key, mapped] : properties) { + const auto& [value, index] = mapped; if (key == ProtocolHostapd::ResponseStatusPropertyKeyState) { status.State = HostapdInterfaceStateFromString(value); } else if (key == ProtocolHostapd::ResponseStatusPropertyKeyIeee80211N) { @@ -57,20 +69,18 @@ WpaStatusResponseParser::ParsePayload() ParseInt(value, status.Ieee80211ax); } else if (key == ProtocolHostapd::ResponseStatusPropertyKeyDisableAX) { ParseInt(value, status.Disable11ax); - } else if (key.starts_with(ProtocolHostapd::PropertyNameBss)) { - bssInfo.Interface = key; - } else if (key.starts_with(ProtocolHostapd::PropertyNameBssBssid)) { - bssInfo.Bssid = value; - } else if (key.starts_with(ProtocolHostapd::PropertyNameBssSsid)) { - bssInfo.Ssid = value; - } else if (key.starts_with(ProtocolHostapd::PropertyNameBssNumStations)) { - ParseInt(value, bssInfo.NumStations); - // TODO: this assumes num_sta is the last property per bss entry. This is currently true, but the code - // should not make this assumption to protect against future changes in hostapd control interface protocol. - // Instead, the property index should be read and when it changes, the bssInfo should be added to the - // response and a new bssInfo should be created. - status.Bss.push_back(std::move(bssInfo)); - bssInfo = {}; + } else if (key == ProtocolHostapd::PropertyNameBss) { + enforceRequiredBssInfoSize(index.value() + 1); + status.Bss[index.value()].Interface = value; + } else if (key == ProtocolHostapd::PropertyNameBssBssid) { + enforceRequiredBssInfoSize(index.value() + 1); + status.Bss[index.value()].Bssid = value; + } else if (key == ProtocolHostapd::PropertyNameBssSsid) { + enforceRequiredBssInfoSize(index.value() + 1); + status.Bss[index.value()].Ssid = value; + } else if (key == ProtocolHostapd::PropertyNameBssNumStations) { + enforceRequiredBssInfoSize(index.value() + 1); + ParseInt(value, status.Bss[index.value()].NumStations); } } diff --git a/src/linux/wpa-controller/WpaResponseParser.cxx b/src/linux/wpa-controller/WpaResponseParser.cxx index 36b81d03..a8da19c8 100644 --- a/src/linux/wpa-controller/WpaResponseParser.cxx +++ b/src/linux/wpa-controller/WpaResponseParser.cxx @@ -1,12 +1,16 @@ #include +#include #include #include #include #include +#include #include +#include #include #include +#include #include #include @@ -23,7 +27,7 @@ WpaResponseParser::WpaResponseParser(const WpaCommand* command, std::string_view { } -const std::unordered_map& +const std::unordered_map>>& WpaResponseParser::GetProperties() const noexcept { return m_properties; @@ -95,14 +99,29 @@ WpaResponseParser::TryParseProperties() } // Parse out the key and value, ensuring the key is non-empty (empty values are allowed). - const auto key = *std::next(keyValuePairIterator, 0); + auto key = *std::next(keyValuePairIterator, 0); const auto value = *std::next(keyValuePairIterator, 1); if (std::empty(key)) { continue; } - m_properties[key] = value; - LOGV << std::format("Parsed [{:03}] {}={}", std::size(m_properties), key, value); + // Check and parse key index in the form of "[]". + std::string keyIndexString{}; + std::optional keyIndex{ std::nullopt }; + const auto indexStart = key.find(ProtocolWpa::KeyValueIndexDelimeterStart); + if (indexStart != std::string_view::npos) { + const auto indexEnd = key.find(ProtocolWpa::KeyValueIndexDelimeterEnd); + if (indexEnd != std::string_view::npos) { + keyIndexString = std::string(key.substr(indexStart + 1, indexEnd - indexStart - 1)); + keyIndex = std::stoul(keyIndexString); + key = key.substr(0, indexStart); + } + } else { + keyIndexString = '*'; + } + + m_properties[key] = std::make_pair(value, keyIndex); + LOGV << std::format("Parsed [{:03}/{}] {}={}", std::size(m_properties), keyIndexString, key, value); } // Remove parsed properties from the list of properties to parse. diff --git a/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx b/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx index fdbd3a92..752a6a1e 100644 --- a/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx +++ b/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx @@ -2,10 +2,13 @@ #ifndef WPA_RESPONSE_PARSER_HXX #define WPA_RESPONSE_PARSER_HXX +#include #include #include +#include #include #include +#include #include #include @@ -26,12 +29,14 @@ struct WpaResponseParser virtual ~WpaResponseParser() = default; /** - * Prevent copy and move operations. + * Prevent copy and move operations. */ WpaResponseParser(const WpaResponseParser&) = delete; WpaResponseParser(WpaResponseParser&&) = delete; - WpaResponseParser& operator=(const WpaResponseParser&) = delete; - WpaResponseParser& operator=(WpaResponseParser&&) = delete; + WpaResponseParser& + operator=(const WpaResponseParser&) = delete; + WpaResponseParser& + operator=(WpaResponseParser&&) = delete; /** * @brief Construct a new WpaResponseParser object. @@ -53,16 +58,16 @@ struct WpaResponseParser /** * @brief Get the command associated with the response payload. - * - * @return const WpaCommand* + * + * @return const WpaCommand* */ const WpaCommand* GetCommand() const noexcept; /** * @brief Get the response payload. - * - * @return std::string_view + * + * @return std::string_view */ std::string_view GetResponsePayload() const noexcept; @@ -80,10 +85,10 @@ protected: /** * @brief Obtain the map of parsed properties. - * - * @return const std::unordered_map& + * + * @return const std::unordered_map>>& */ - const std::unordered_map& + const std::unordered_map>>& GetProperties() const noexcept; private: @@ -103,7 +108,7 @@ private: const WpaCommand* m_command; std::string_view m_responsePayload; std::vector m_propertiesToParse; - std::unordered_map m_properties{}; + std::unordered_map>> m_properties{}; }; /** @@ -116,12 +121,14 @@ struct WpaResponseParserFactory virtual ~WpaResponseParserFactory() = default; /** - * Prevent copy and move operations. + * Prevent copy and move operations. */ WpaResponseParserFactory(const WpaResponseParserFactory&) = delete; WpaResponseParserFactory(WpaResponseParserFactory&&) = delete; - WpaResponseParserFactory& operator=(const WpaResponseParserFactory&) = delete; - WpaResponseParserFactory& operator=(WpaResponseParserFactory&&) = delete; + WpaResponseParserFactory& + operator=(const WpaResponseParserFactory&) = delete; + WpaResponseParserFactory& + operator=(WpaResponseParserFactory&&) = delete; /** * @brief Create a response parser object. From c57b473fcb5f0a9421594821be39752bfeb0b9b9 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sun, 18 Feb 2024 05:03:00 +0000 Subject: [PATCH 17/21] Remove extraneous newlines from log messages. --- src/linux/wpa-controller/WpaController.cxx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/linux/wpa-controller/WpaController.cxx b/src/linux/wpa-controller/WpaController.cxx index 975fa706..f063ebbb 100644 --- a/src/linux/wpa-controller/WpaController.cxx +++ b/src/linux/wpa-controller/WpaController.cxx @@ -70,7 +70,7 @@ WpaController::GetCommandControlSocket() const auto controlSocketPath = m_controlSocketPath / m_interfaceName; struct wpa_ctrl* controlSocket = wpa_ctrl_open(controlSocketPath.c_str()); if (controlSocket == nullptr) { - LOGE << std::format("Failed to open control socket for {} interface at {}.\n", m_interfaceName, controlSocketPath.c_str()); + LOGE << std::format("Failed to open control socket for {} interface at {}.", m_interfaceName, controlSocketPath.c_str()); return nullptr; } @@ -85,29 +85,30 @@ WpaController::SendCommand(const WpaCommand& command) // Obtain a control socket connection to send the command over. struct wpa_ctrl* controlSocket = GetCommandControlSocket(); if (controlSocket == nullptr) { - LOGE << std::format("Failed to get control socket for {}.\n", m_interfaceName); + LOGE << std::format("Failed to get control socket for {}.", m_interfaceName); return nullptr; } // Send the command and receive the response. - std::array responseBuffer; + std::array responseBuffer{}; std::size_t responseSize = std::size(responseBuffer); auto commandPayload = command.GetPayload(); + LOGD << std::format("Sending wpa command to {} interface: '{}'", m_interfaceName, commandPayload); int ret = wpa_ctrl_request(controlSocket, std::data(commandPayload), std::size(commandPayload), std::data(responseBuffer), &responseSize, nullptr); switch (ret) { case 0: { - std::string_view responsePayload{ std::data(responseBuffer), responseSize }; + const std::string_view responsePayload{ std::data(responseBuffer), responseSize }; return command.ParseResponse(responsePayload); } case -1: - LOGE << std::format("Failed to send or receive command to {} interface.\n", m_interfaceName); + LOGE << std::format("Failed to send or receive command to/from {} interface.", m_interfaceName); return nullptr; case -2: - LOGE << std::format("Sending command to {} interface timed out.\n", m_interfaceName); + LOGE << std::format("Sending command to {} interface timed out.", m_interfaceName); return nullptr; default: - LOGE << std::format("Unknown error sending command to {} interface (ret={}).\n", m_interfaceName, ret); + LOGE << std::format("Unknown error sending command to {} interface (ret={}).", m_interfaceName, ret); return nullptr; } } From 108d2b701d4b5d09175ae7595680e95c33d6102d Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Sun, 18 Feb 2024 05:03:07 +0000 Subject: [PATCH 18/21] Add tests. --- .../include/Wpa/WpaResponseParser.hxx | 4 +- .../unit/linux/wpa-controller/TestHostapd.cxx | 55 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx b/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx index 752a6a1e..d61324bb 100644 --- a/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx +++ b/src/linux/wpa-controller/include/Wpa/WpaResponseParser.hxx @@ -85,8 +85,8 @@ protected: /** * @brief Obtain the map of parsed properties. - * - * @return const std::unordered_map>>& + * + * @return const std::unordered_map>>& */ const std::unordered_map>>& GetProperties() const noexcept; diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx index a4745821..0cc812ce 100644 --- a/tests/unit/linux/wpa-controller/TestHostapd.cxx +++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx @@ -1,5 +1,6 @@ #include +#include #include #include #include @@ -301,3 +302,57 @@ TEST_CASE("Send command: Terminate() ping failure (root)", "[wpa][hostapd][clien REQUIRE_THROWS_AS(hostapd.Ping(), HostapdException); } + +TEST_CASE("Send SetSsid() command (root)", "[wpa][hostapd][client][remote]") +{ + using namespace Wpa; + + constexpr auto SsidValid{ "whatever" }; + constexpr auto SsidInvalidNonPrintableCharacters {"SSID\x01\x02\x03"}; + constexpr auto SsidInvalidTooLong{ "This SSID is way too long to be valid because it has more than 32 characters" }; + + Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault); + + SECTION("SetSsid() doesn't throw") + { + REQUIRE_NOTHROW(hostapd.SetSsid(SsidValid)); + } + + SECTION("SetSsid() returns true for valid SSID") + { + REQUIRE(hostapd.SetSsid(SsidValid)); + } + + SECTION("SetSsid() throws for empty SSID") + { + REQUIRE_THROWS_AS(hostapd.SetSsid(""), HostapdException); + } + + SECTION("SetSsid() throws for SSID with length > 32") + { + REQUIRE_THROWS_AS(hostapd.SetSsid(SsidInvalidTooLong), HostapdException); + } + + SECTION("SetSsid() throws for SSID with non-printable characters") + { + REQUIRE_THROWS_AS(hostapd.SetSsid(SsidInvalidNonPrintableCharacters), HostapdException); + } + + SECTION("SetSsid() changes the SSID for valid input") + { + constexpr auto SsidToSet{ SsidValid }; + REQUIRE(hostapd.SetSsid(SsidToSet)); + const auto status = hostapd.GetStatus(); + REQUIRE(!std::empty(status.Bss)); + REQUIRE(status.Bss[0].Ssid == SsidToSet); + } + + SECTION("SetSsid() does not change the SSID for invalid inputs") + { + const auto* const SsidToSet{ SsidInvalidTooLong }; + REQUIRE_THROWS_AS(hostapd.SetSsid(SsidToSet), HostapdException); + const auto status = hostapd.GetStatus(); + REQUIRE(!std::empty(status.Bss)); + REQUIRE(status.Bss[0].Ssid != SsidToSet); + } +} \ No newline at end of file From 0a88020a50034bc06564012c654666f275a7e06c Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Mon, 19 Feb 2024 03:27:05 +0000 Subject: [PATCH 19/21] Fix check to workaround output bug in hostapd. --- .../unit/linux/wpa-controller/TestHostapd.cxx | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx index 0cc812ce..5f501e8c 100644 --- a/tests/unit/linux/wpa-controller/TestHostapd.cxx +++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx @@ -308,7 +308,6 @@ TEST_CASE("Send SetSsid() command (root)", "[wpa][hostapd][client][remote]") using namespace Wpa; constexpr auto SsidValid{ "whatever" }; - constexpr auto SsidInvalidNonPrintableCharacters {"SSID\x01\x02\x03"}; constexpr auto SsidInvalidTooLong{ "This SSID is way too long to be valid because it has more than 32 characters" }; Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault); @@ -333,11 +332,6 @@ TEST_CASE("Send SetSsid() command (root)", "[wpa][hostapd][client][remote]") REQUIRE_THROWS_AS(hostapd.SetSsid(SsidInvalidTooLong), HostapdException); } - SECTION("SetSsid() throws for SSID with non-printable characters") - { - REQUIRE_THROWS_AS(hostapd.SetSsid(SsidInvalidNonPrintableCharacters), HostapdException); - } - SECTION("SetSsid() changes the SSID for valid input") { constexpr auto SsidToSet{ SsidValid }; @@ -347,12 +341,19 @@ TEST_CASE("Send SetSsid() command (root)", "[wpa][hostapd][client][remote]") REQUIRE(status.Bss[0].Ssid == SsidToSet); } + // Note: The below tests use starts_with() to check the SSID instead of == as hostapd has a bug where attempting to + // provide an invalid SSID results in the reported SSID having some junk characters following the real SSID. + SECTION("SetSsid() does not change the SSID for invalid inputs") { + const auto statusInitial = hostapd.GetStatus(); + REQUIRE(!std::empty(statusInitial.Bss)); + const auto* const SsidToSet{ SsidInvalidTooLong }; REQUIRE_THROWS_AS(hostapd.SetSsid(SsidToSet), HostapdException); - const auto status = hostapd.GetStatus(); - REQUIRE(!std::empty(status.Bss)); - REQUIRE(status.Bss[0].Ssid != SsidToSet); + + const auto statusAfterFail = hostapd.GetStatus(); + REQUIRE(!std::empty(statusAfterFail.Bss)); + REQUIRE(statusAfterFail.Bss[0].Ssid.starts_with(statusInitial.Bss[0].Ssid)); } -} \ No newline at end of file +} From 7ff8c8cd1ef109d3313118dd67f0b17925bf97be Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Mon, 19 Feb 2024 03:27:50 +0000 Subject: [PATCH 20/21] Add debug output for set property commands. --- src/linux/wpa-controller/Hostapd.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx index bd38322a..599d7922 100644 --- a/src/linux/wpa-controller/Hostapd.cxx +++ b/src/linux/wpa-controller/Hostapd.cxx @@ -1,4 +1,5 @@ +#include #include #include @@ -96,6 +97,8 @@ Hostapd::GetProperty(std::string_view propertyName, std::string& propertyValue) bool Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyValue) { + LOGD << std::format("Attempting to set hostapd property '{}'({}) to '{}'({})", propertyName, std::size(propertyName), propertyValue, std::size(propertyValue)); + const WpaCommandSet setCommand(propertyName, propertyValue); const auto response = m_controller.SendCommand(setCommand); if (!response) { From 65e0b517d483b9d0eba3772277aff7b83c8ca69e Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Tue, 20 Feb 2024 16:20:00 +0000 Subject: [PATCH 21/21] Add IEEE 802.11 BSS definitions. --- .../include/microsoft/net/wifi/Ieee80211.hxx | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/common/wifi/core/include/microsoft/net/wifi/Ieee80211.hxx b/src/common/wifi/core/include/microsoft/net/wifi/Ieee80211.hxx index 0e725082..b53dae78 100644 --- a/src/common/wifi/core/include/microsoft/net/wifi/Ieee80211.hxx +++ b/src/common/wifi/core/include/microsoft/net/wifi/Ieee80211.hxx @@ -2,9 +2,11 @@ #ifndef IEEE_80211_HXX #define IEEE_80211_HXX +#include #include #include #include +#include #include @@ -268,6 +270,39 @@ enum class Ieee80211CipherSuite : uint32_t { Wep40 = MakeIeee80211Suite(Ieee80211CipherSuiteIdWep40), }; +/** + * @brief IEEE 802.11 BSS Types. + * + * Defined in IEEE 802.11-2020, Section 4.3. + */ +enum class Ieee80211BssType { + Unknown, + Infrastructure, // ESS + Independent, // IBSS + Personal, // PBSS + Mesh, // MBSS +}; + +/** + * @brief Number of octets per BSSID. + */ +static constexpr auto BssidNumOctets = 6; + +/** + * @brief BSSID type. + */ +using Ieee80211Bssid = std::array; + +/** + * @brief Information about a BSS. + */ +struct Ieee80211Bss +{ + Ieee80211BssType Type; + Ieee80211Bssid Bssid; + std::string Ssid; +}; + } // namespace Microsoft::Net::Wifi #endif // IEEE_80211_HXX