Skip to content

Commit

Permalink
Enable server interactions on Controllers (#14998)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mrjerryjohns authored and pull[bot] committed Sep 15, 2023
1 parent 171080d commit 5281585
Show file tree
Hide file tree
Showing 15 changed files with 148 additions and 34 deletions.
1 change: 1 addition & 0 deletions examples/persistent-storage/qpg/args.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 20 additions & 16 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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";
Expand Down Expand Up @@ -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())
{
Expand Down
19 changes: 19 additions & 0 deletions src/app/server/Dnssd.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#pragma once

#include <credentials/FabricTable.h>
#include <lib/core/CHIPError.h>
#include <lib/core/Optional.h>
#include <lib/dnssd/Advertiser.h>
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<Time::Source::kSystem> mTimeSource;

void ClearTimeouts()
Expand All @@ -121,6 +138,8 @@ class DLL_EXPORT DnssdServer
#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
}

FabricTable * mFabricTable = nullptr;

// Helper for StartServer.
void StartServer(Optional<Dnssd::CommissioningMode> mode);

Expand Down
6 changes: 5 additions & 1 deletion src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/controller/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
11 changes: 11 additions & 0 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
#include <platform/ConfigurationManager.h>
#endif

#include <app/server/Dnssd.h>

#include <app/InteractionModelEngine.h>
#include <app/OperationalDeviceProxy.h>
#include <app/util/error-mapping.h>
Expand Down Expand Up @@ -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 = {
Expand Down
8 changes: 8 additions & 0 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
51 changes: 51 additions & 0 deletions src/controller/CHIPDeviceControllerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
#include <platform/ConfigurationManager.h>
#endif

#include <app/server/Dnssd.h>
#include <protocols/secure_channel/CASEServer.h>

using namespace chip::Inet;
using namespace chip::System;
using namespace chip::Credentials;
Expand Down Expand Up @@ -119,6 +122,9 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)

stateParams.transportMgr = chip::Platform::New<DeviceTransportMgr>();

//
// 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)
Expand Down Expand Up @@ -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<CASEServer>();

//
// 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<DeviceControllerSystemState>(stateParams);
ChipLogDetail(Controller, "System State Initialized...");
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions src/controller/CHIPDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion src/controller/CHIPDeviceControllerSystemState.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
#pragma once

#include <credentials/FabricTable.h>
#include <protocols/secure_channel/CASEServer.h>
#include <protocols/secure_channel/MessageCounterManager.h>

#include <transport/TransportMgr.h>
#include <transport/raw/UDP.h>
#if CONFIG_DEVICE_LAYER
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -145,6 +149,7 @@ class DeviceControllerSystemState
Messaging::ExchangeManager * mExchangeMgr = nullptr;
secure_channel::MessageCounterManager * mMessageCounterManager = nullptr;
FabricTable * mFabrics = nullptr;
CASEServer * mCASEServer = nullptr;

std::atomic<uint32_t> mRefCount{ 1 };

Expand Down
5 changes: 1 addition & 4 deletions src/controller/python/ChipDeviceController-ScriptBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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();
}

Expand Down
2 changes: 2 additions & 0 deletions src/controller/python/OpCredsBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions src/protocols/secure_channel/CASEServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -42,7 +42,6 @@ CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager
mSessionManager = sessionManager;
mFabrics = fabrics;
mExchangeManager = exchangeManager;
mIDAllocator = idAllocator;

Cleanup();
return CHIP_NO_ERROR;
Expand All @@ -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(
Expand Down Expand Up @@ -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();
}

Expand Down
Loading

0 comments on commit 5281585

Please sign in to comment.