From 5281585bac79f4026e47be11f16a5d53b2a21e7c Mon Sep 17 00:00:00 2001 From: Jerry Johns Date: Tue, 1 Mar 2022 06:32:40 -0800 Subject: [PATCH] Enable server interactions on Controllers (#14998) * Enable CASE Server on a Controller This minimally enables the CASE server on devices that utilize the `DeviceController` and related classes. Specifically, it instantiates the CASEServer as part of initializing the `DeviceControllerFactory`, sets it up to listen for unsolicited messages (Sigma R1) and enables the advertisement of the controller's operational identity on the network to permit discovery and resolution. Testing: Creates two Python REPL instances with matching persistent storage op certs. On the first REPL instance, ran: >> devCtrl2 = fabricAdmins[0].NewController(nodeId=22) On the second REPL instance, ran: >> devCtrl.ResolveNode(22) Confirmed from the logs that CASE was established. * Review feeback * More review feedback * Shifting of shutdown logic to be in the right sequence * Accidentally added an include... * Fixed persistent app on QPG to not set chip_system_config_use_open_thread_udp to false to fix compilation issues with Server.cpp --- examples/persistent-storage/qpg/args.gni | 1 + src/app/server/Dnssd.cpp | 36 +++++++------ src/app/server/Dnssd.h | 19 +++++++ src/app/server/Server.cpp | 6 ++- src/controller/BUILD.gn | 1 + src/controller/CHIPDeviceController.cpp | 11 ++++ src/controller/CHIPDeviceController.h | 8 +++ .../CHIPDeviceControllerFactory.cpp | 51 +++++++++++++++++++ src/controller/CHIPDeviceControllerFactory.h | 17 +++++++ .../CHIPDeviceControllerSystemState.h | 7 ++- .../ChipDeviceController-ScriptBinding.cpp | 5 +- src/controller/python/OpCredsBinding.cpp | 2 + src/protocols/secure_channel/CASEServer.cpp | 7 ++- src/protocols/secure_channel/CASEServer.h | 6 +-- .../secure_channel/tests/TestCASESession.cpp | 5 +- 15 files changed, 148 insertions(+), 34 deletions(-) diff --git a/examples/persistent-storage/qpg/args.gni b/examples/persistent-storage/qpg/args.gni index 3e9a1662c0299b..69c0b0040eacc4 100644 --- a/examples/persistent-storage/qpg/args.gni +++ b/examples/persistent-storage/qpg/args.gni @@ -17,6 +17,7 @@ import("//build_overrides/chip.gni") import("${chip_root}/examples/platform/qpg/args.gni") qpg_sdk_target = get_label_info(":sdk", "label_no_toolchain") chip_enable_openthread = false +chip_system_config_use_open_thread_udp = false declare_args() { # Disable lock tracking, since our FreeRTOS configuration does not set diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 3449af33cf2e2c..355fa44d7eef8d 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -41,21 +41,6 @@ namespace chip { namespace app { namespace { -bool HaveOperationalCredentials() -{ - // Look for any fabric info that has a useful operational identity. - for (const FabricInfo & fabricInfo : Server::GetInstance().GetFabricTable()) - { - if (fabricInfo.IsInitialized()) - { - return true; - } - } - - ChipLogProgress(Discovery, "Failed to find a valid admin pairing. Node ID unknown"); - return false; -} - void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent * event) { if (event->Type == DeviceLayer::DeviceEventType::kDnssdPlatformInitialized @@ -78,6 +63,23 @@ void OnPlatformEventWrapper(const DeviceLayer::ChipDeviceEvent * event, intptr_t constexpr System::Clock::Timestamp DnssdServer::kTimeoutCleared; +bool DnssdServer::HaveOperationalCredentials() +{ + VerifyOrDie(mFabricTable != nullptr); + + // Look for any fabric info that has a useful operational identity. + for (const FabricInfo & fabricInfo : *mFabricTable) + { + if (fabricInfo.IsInitialized()) + { + return true; + } + } + + ChipLogProgress(Discovery, "Failed to find a valid admin pairing. Node ID unknown"); + return false; +} + #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY constexpr const char kExtendedDiscoveryTimeoutKeypairStorage[] = "ExtDiscKey"; @@ -250,7 +252,9 @@ CHIP_ERROR DnssdServer::GetCommissionableInstanceName(char * buffer, size_t buff /// Set MDNS operational advertisement CHIP_ERROR DnssdServer::AdvertiseOperational() { - for (const FabricInfo & fabricInfo : Server::GetInstance().GetFabricTable()) + VerifyOrDie(mFabricTable != nullptr); + + for (const FabricInfo & fabricInfo : *mFabricTable) { if (fabricInfo.IsInitialized()) { diff --git a/src/app/server/Dnssd.h b/src/app/server/Dnssd.h index 3b99f156b62574..ebe7a51499c84d 100644 --- a/src/app/server/Dnssd.h +++ b/src/app/server/Dnssd.h @@ -17,6 +17,7 @@ #pragma once +#include #include #include #include @@ -63,6 +64,16 @@ class DLL_EXPORT DnssdServer /// Gets the factory-new state commissionable node discovery timeout int16_t GetDiscoveryTimeoutSecs() { return mDiscoveryTimeoutSecs; } + // + // Override the referenced fabric table from the default that is present + // in Server::GetInstance().GetFabricTable() to something else. + // + void SetFabricTable(FabricTable * table) + { + VerifyOrDie(table != nullptr); + mFabricTable = table; + } + /// Callback from Discovery Expiration timer /// Checks if discovery has expired and if so, /// kicks off extend discovery (when enabled) @@ -111,6 +122,12 @@ class DLL_EXPORT DnssdServer /// Set MDNS commissionable node advertisement CHIP_ERROR AdvertiseCommissionableNode(chip::Dnssd::CommissioningMode mode); + // + // Check if we have any valid operational credentials present in the fabric table and return true + // if we do. + // + bool HaveOperationalCredentials(); + Time::TimeSource mTimeSource; void ClearTimeouts() @@ -121,6 +138,8 @@ class DLL_EXPORT DnssdServer #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY } + FabricTable * mFabricTable = nullptr; + // Helper for StartServer. void StartServer(Optional mode); diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 961131a05ddd71..88692049c3a8ee 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -139,6 +139,10 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint err = mFabrics.Init(&mDeviceStorage); SuccessOrExit(err); +#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD + app::DnssdServer::Instance().SetFabricTable(&mFabrics); +#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD + // Group data provider must be initialized after mDeviceStorage err = mGroupsProvider.Init(); SuccessOrExit(err); @@ -248,7 +252,7 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint #endif err = mCASEServer.ListenForSessionEstablishment(&mExchangeMgr, &mTransports, chip::DeviceLayer::ConnectivityMgr().GetBleLayer(), - &mSessions, &mFabrics, &mSessionIDAllocator); + &mSessions, &mFabrics); SuccessOrExit(err); err = mCASESessionManager.Init(); diff --git a/src/controller/BUILD.gn b/src/controller/BUILD.gn index 4b58532fb46eba..84702a7f0fddb3 100644 --- a/src/controller/BUILD.gn +++ b/src/controller/BUILD.gn @@ -60,6 +60,7 @@ static_library("controller") { public_deps = [ "${chip_root}/src/app", + "${chip_root}/src/app/server", "${chip_root}/src/lib/core", "${chip_root}/src/lib/dnssd", "${chip_root}/src/lib/support", diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 774f899eefa4d6..c7db1e3a4cd035 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -45,6 +45,8 @@ #include #endif +#include + #include #include #include @@ -139,6 +141,15 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) if (params.operationalKeypair != nullptr || !params.controllerNOC.empty() || !params.controllerRCAC.empty()) { ReturnErrorOnFailure(ProcessControllerNOCChain(params)); + + if (params.enableServerInteractions) + { + // + // Advertise our operational identity on the network to facilitate discovery by clients that look to + // establish CASE with a controller that is also offering server-side capabilities (e.g an OTA provider). + // + app::DnssdServer::Instance().AdvertiseOperational(); + } } DeviceProxyInitParams deviceInitParams = { diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index a7c796e906b2a6..63a04bf9635348 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -108,6 +108,14 @@ struct ControllerInitParams ByteSpan controllerICAC; ByteSpan controllerRCAC; + // + // Controls enabling server cluster interactions on a controller. This in turn + // causes the following to get enabled: + // + // - Advertisement of active controller operational identities. + // + bool enableServerInteractions = false; + uint16_t controllerVendorId; }; diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index 4eb72e430ac80d..be6579a576673c 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/src/controller/CHIPDeviceControllerFactory.cpp @@ -32,6 +32,9 @@ #include #endif +#include +#include + using namespace chip::Inet; using namespace chip::System; using namespace chip::Credentials; @@ -119,6 +122,9 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params) stateParams.transportMgr = chip::Platform::New(); + // + // The logic below expects IPv6 to be at index 0 of this tuple. Please do not alter that. + // ReturnErrorOnFailure(stateParams.transportMgr->Init(Transport::UdpListenParameters(stateParams.udpEndPointManager) .SetAddressType(Inet::IPAddressType::kIPv6) .SetListenPort(mListenPort) @@ -154,6 +160,43 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params) ReturnErrorOnFailure(Dnssd::Resolver::Instance().Init(stateParams.udpEndPointManager)); #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD + if (params.enableServerInteractions) + { + stateParams.caseServer = chip::Platform::New(); + + // + // Enable listening for session establishment messages. + // + // We pass in a nullptr for the BLELayer since we're not permitting usage of BLE in this server modality for the controller, + // especially since it will interrupt other potential usages of BLE by the controller acting in a commissioning capacity. + // + ReturnErrorOnFailure(stateParams.caseServer->ListenForSessionEstablishment( + stateParams.exchangeMgr, stateParams.transportMgr, nullptr, stateParams.sessionMgr, stateParams.fabricTable)); + + // + // We need to advertise the port that we're listening to for unsolicited messages over UDP. However, we have both a IPv4 + // and IPv6 endpoint to pick from. Given that the listen port passed in may be set to 0 (which then has the kernel select + // a valid port at bind time), that will result in two possible ports being provided back from the resultant endpoint + // initializations. Since IPv6 is POR for Matter, let's go ahead and pick that port. + // + app::DnssdServer::Instance().SetSecuredPort(stateParams.transportMgr->GetTransport().GetImplAtIndex<0>().GetBoundPort()); + + // + // TODO: This is a hack to workaround the fact that we have a bi-polar stack that has controller and server modalities that + // are mutually exclusive in terms of initialization of key stack singletons. Consequently, DnssdServer accesses + // Server::GetInstance().GetFabricTable() to access the fabric table, but we don't want to do that when we're initializing + // the controller logic since the factory here has its own fabric table. + // + // Consequently, reach in set the fabric table pointer to point to the right version. + // + app::DnssdServer::Instance().SetFabricTable(stateParams.fabricTable); + + // + // Start up the DNS-SD server + // + chip::app::DnssdServer::Instance().StartServer(chip::Dnssd::CommissioningMode::kDisabled); + } + // store the system state mSystemState = chip::Platform::New(stateParams); ChipLogDetail(Controller, "System State Initialized..."); @@ -174,6 +217,8 @@ void DeviceControllerFactory::PopulateInitParams(ControllerInitParams & controll controllerParams.systemState = mSystemState; controllerParams.controllerVendorId = params.controllerVendorId; + + controllerParams.enableServerInteractions = params.enableServerInteractions; } CHIP_ERROR DeviceControllerFactory::SetupController(SetupParams params, DeviceController & controller) @@ -236,6 +281,12 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown() ChipLogDetail(Controller, "Shutting down the System State, this will teardown the CHIP Stack"); + if (mCASEServer != nullptr) + { + chip::Platform::Delete(mCASEServer); + mCASEServer = nullptr; + } + #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD Dnssd::Resolver::Instance().Shutdown(); #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h index aee4497aa3319c..e67d4fc78a694e 100644 --- a/src/controller/CHIPDeviceControllerFactory.h +++ b/src/controller/CHIPDeviceControllerFactory.h @@ -60,6 +60,15 @@ struct SetupParams // The Device Pairing Delegated used to initialize a Commissioner DevicePairingDelegate * pairingDelegate = nullptr; + // + // Controls enabling server cluster interactions on a controller. This in turn + // causes the following to get enabled: + // + // - CASEServer to listen for unsolicited Sigma1 messages. + // - Advertisement of active controller operational identities. + // + bool enableServerInteractions = false; + Credentials::DeviceAttestationVerifier * deviceAttestationVerifier = nullptr; CommissioningDelegate * defaultCommissioner = nullptr; }; @@ -77,6 +86,14 @@ struct FactoryInitParams Ble::BleLayer * bleLayer = nullptr; #endif + // + // Controls enabling server cluster interactions on a controller. This in turn + // causes the following to get enabled: + // + // - Advertisement of active controller operational identities. + // + bool enableServerInteractions = false; + /* The port used for operational communication to listen for and send messages over UDP/TCP. * The default value of `0` will pick any available port. */ uint16_t listenPort = 0; diff --git a/src/controller/CHIPDeviceControllerSystemState.h b/src/controller/CHIPDeviceControllerSystemState.h index 6c2a3e0cfb726c..ebb0c87be3ef8a 100644 --- a/src/controller/CHIPDeviceControllerSystemState.h +++ b/src/controller/CHIPDeviceControllerSystemState.h @@ -30,7 +30,9 @@ #pragma once #include +#include #include + #include #include #if CONFIG_DEVICE_LAYER @@ -72,6 +74,7 @@ struct DeviceControllerSystemStateParams Messaging::ExchangeManager * exchangeMgr = nullptr; secure_channel::MessageCounterManager * messageCounterManager = nullptr; FabricTable * fabricTable = nullptr; + CASEServer * caseServer = nullptr; }; // A representation of the internal state maintained by the DeviceControllerFactory @@ -84,7 +87,8 @@ class DeviceControllerSystemState DeviceControllerSystemState(DeviceControllerSystemStateParams params) : mSystemLayer(params.systemLayer), mTCPEndPointManager(params.tcpEndPointManager), mUDPEndPointManager(params.udpEndPointManager), mTransportMgr(params.transportMgr), mSessionMgr(params.sessionMgr), - mExchangeMgr(params.exchangeMgr), mMessageCounterManager(params.messageCounterManager), mFabrics(params.fabricTable) + mExchangeMgr(params.exchangeMgr), mMessageCounterManager(params.messageCounterManager), mFabrics(params.fabricTable), + mCASEServer(params.caseServer) { #if CONFIG_NETWORK_LAYER_BLE mBleLayer = params.bleLayer; @@ -145,6 +149,7 @@ class DeviceControllerSystemState Messaging::ExchangeManager * mExchangeMgr = nullptr; secure_channel::MessageCounterManager * mMessageCounterManager = nullptr; FabricTable * mFabrics = nullptr; + CASEServer * mCASEServer = nullptr; std::atomic mRefCount{ 1 }; diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index dd9f45ce57c414..04f1e16131f9ac 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -226,6 +226,7 @@ ChipError::StorageType pychip_DeviceController_StackInit() FactoryInitParams factoryParams; factoryParams.fabricStorage = &sFabricStorage; factoryParams.fabricIndependentStorage = sStorageAdapter; + factoryParams.enableServerInteractions = true; ReturnErrorOnFailure(DeviceControllerFactory::GetInstance().Init(factoryParams).AsInteger()); @@ -240,10 +241,6 @@ ChipError::StorageType pychip_DeviceController_StackInit() // DeviceControllerFactory::GetInstance().RetainSystemState(); -#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY - chip::app::DnssdServer::Instance().StartServer(chip::Dnssd::CommissioningMode::kDisabled); -#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY - return CHIP_NO_ERROR.AsInteger(); } diff --git a/src/controller/python/OpCredsBinding.cpp b/src/controller/python/OpCredsBinding.cpp index d5cc06aee8e0bb..f810d835ac4c0c 100644 --- a/src/controller/python/OpCredsBinding.cpp +++ b/src/controller/python/OpCredsBinding.cpp @@ -173,6 +173,8 @@ ChipError::StorageType pychip_OpCreds_AllocateController(OpCredsContext * contex initParams.controllerRCAC = rcacSpan; initParams.controllerICAC = icacSpan; initParams.controllerNOC = nocSpan; + initParams.enableServerInteractions = true; + if (useTestCommissioner) { initParams.defaultCommissioner = &sTestCommissioner; diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index 28f5d5f94d3969..275e63da08cff6 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -31,7 +31,7 @@ namespace chip { CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager * exchangeManager, TransportMgrBase * transportMgr, Ble::BleLayer * bleLayer, SessionManager * sessionManager, - FabricTable * fabrics, SessionIDAllocator * idAllocator) + FabricTable * fabrics) { VerifyOrReturnError(transportMgr != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(exchangeManager != nullptr, CHIP_ERROR_INVALID_ARGUMENT); @@ -42,7 +42,6 @@ CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager mSessionManager = sessionManager; mFabrics = fabrics; mExchangeManager = exchangeManager; - mIDAllocator = idAllocator; Cleanup(); return CHIP_NO_ERROR; @@ -68,7 +67,7 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec) } #endif - ReturnErrorOnFailure(mIDAllocator->Allocate(mSessionKeyId)); + ReturnErrorOnFailure(mSessionIDAllocator.Allocate(mSessionKeyId)); // Setup CASE state machine using the credentials for the current fabric. ReturnErrorOnFailure(GetSession().ListenForSessionEstablishment( @@ -116,7 +115,7 @@ void CASEServer::Cleanup() void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) { ChipLogProgress(Inet, "CASE Session establishment failed: %s", ErrorStr(err)); - mIDAllocator->Free(mSessionKeyId); + mSessionIDAllocator.Free(mSessionKeyId); Cleanup(); } diff --git a/src/protocols/secure_channel/CASEServer.h b/src/protocols/secure_channel/CASEServer.h index 95fea0b43e9e2e..256c0fc04fa58c 100644 --- a/src/protocols/secure_channel/CASEServer.h +++ b/src/protocols/secure_channel/CASEServer.h @@ -38,8 +38,7 @@ class CASEServer : public SessionEstablishmentDelegate, public Messaging::Exchan } CHIP_ERROR ListenForSessionEstablishment(Messaging::ExchangeManager * exchangeManager, TransportMgrBase * transportMgr, - Ble::BleLayer * bleLayer, SessionManager * sessionManager, FabricTable * fabrics, - SessionIDAllocator * idAllocator); + Ble::BleLayer * bleLayer, SessionManager * sessionManager, FabricTable * fabrics); //////////// SessionEstablishmentDelegate Implementation /////////////// void OnSessionEstablishmentError(CHIP_ERROR error) override; @@ -62,11 +61,10 @@ class CASEServer : public SessionEstablishmentDelegate, public Messaging::Exchan Ble::BleLayer * mBleLayer = nullptr; FabricTable * mFabrics = nullptr; + SessionIDAllocator mSessionIDAllocator; CHIP_ERROR InitCASEHandshake(Messaging::ExchangeContext * ec); - SessionIDAllocator * mIDAllocator = nullptr; - void Cleanup(); }; diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp index 7e2b673f964ebf..06d13c05fd875a 100644 --- a/src/protocols/secure_channel/tests/TestCASESession.cpp +++ b/src/protocols/secure_channel/tests/TestCASESession.cpp @@ -362,12 +362,9 @@ void CASE_SecurePairingHandshakeServerTest(nlTestSuite * inSuite, void * inConte gLoopback.mSentMessageCount = 0; - SessionIDAllocator idAllocator; - NL_TEST_ASSERT(inSuite, gPairingServer.ListenForSessionEstablishment(&ctx.GetExchangeManager(), &ctx.GetTransportMgr(), nullptr, - &ctx.GetSecureSessionManager(), &gDeviceFabrics, - &idAllocator) == CHIP_NO_ERROR); + &ctx.GetSecureSessionManager(), &gDeviceFabrics) == CHIP_NO_ERROR); ExchangeContext * contextCommissioner = ctx.NewUnauthenticatedExchangeToBob(pairingCommissioner);