Skip to content

Commit

Permalink
Fix Darwin framework operational advertising for new fabrics. (#26005)
Browse files Browse the repository at this point in the history
Bringing up a new fabric for the very first time was not correctly starting
operational advertising for that fabric, because we were not propagating that
state to the actual CHIPDeviceController.  It happened to work for existing
fabrics, because bringing up the controller factory starts operational
advertising for fabrics that are already known at that point.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jul 19, 2023
1 parent ce1307f commit 2190864
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 3 deletions.
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
commissionerParams.controllerNOC = noc;
}
commissionerParams.controllerVendorId = static_cast<chip::VendorId>([startupParams.vendorID unsignedShortValue]);
commissionerParams.enableServerInteractions = startupParams.advertiseOperational;
commissionerParams.deviceAttestationVerifier = _factory.deviceAttestationVerifier;

auto & factory = chip::Controller::DeviceControllerFactory::GetInstance();
Expand Down
8 changes: 5 additions & 3 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ @interface MTRDeviceControllerFactory ()
@property (readonly) Credentials::PersistentStorageOpCertStore * opCertStore;
@property (readonly) MTROperationalBrowser * operationalBrowser;
@property () chip::Credentials::DeviceAttestationVerifier * deviceAttestationVerifier;
@property (readonly) BOOL advertiseOperational;

- (BOOL)findMatchingFabric:(FabricTable &)fabricTable
params:(MTRDeviceControllerStartupParams *)params
Expand Down Expand Up @@ -435,9 +436,7 @@ - (BOOL)startControllerFactory:(MTRDeviceControllerFactoryParams *)startupParams
if (startupParams.port != nil) {
params.listenPort = [startupParams.port unsignedShortValue];
}
if (startupParams.shouldStartServer == YES) {
params.enableServerInteractions = true;
}
params.enableServerInteractions = startupParams.shouldStartServer;

params.groupDataProvider = _groupDataProvider;
params.sessionKeystore = _sessionKeystore;
Expand Down Expand Up @@ -470,6 +469,7 @@ - (BOOL)startControllerFactory:(MTRDeviceControllerFactoryParams *)startupParams
_controllerFactory->RetainSystemState();
_controllerFactory->ReleaseSystemState();

self->_advertiseOperational = startupParams.shouldStartServer;
self->_running = YES;
});

Expand Down Expand Up @@ -566,6 +566,7 @@ - (MTRDeviceController * _Nullable)createControllerOnExistingFabric:(MTRDeviceCo
params = [[MTRDeviceControllerStartupParamsInternal alloc] initForExistingFabric:fabricTable
fabricIndex:fabric->GetFabricIndex()
keystore:_keystore
advertiseOperational:self.advertiseOperational
params:startupParams];
if (params == nil) {
fabricError = CHIP_ERROR_NO_MEMORY;
Expand Down Expand Up @@ -649,6 +650,7 @@ - (MTRDeviceController * _Nullable)createControllerOnNewFabric:(MTRDeviceControl

params = [[MTRDeviceControllerStartupParamsInternal alloc] initForNewFabric:fabricTable
keystore:_keystore
advertiseOperational:self.advertiseOperational
params:startupParams];
if (params == nil) {
fabricError = CHIP_ERROR_NO_MEMORY;
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ - (instancetype)initWithParams:(MTRDeviceControllerStartupParams *)params

- (instancetype)initForNewFabric:(chip::FabricTable *)fabricTable
keystore:(chip::Crypto::OperationalKeystore *)keystore
advertiseOperational:(BOOL)advertiseOperational
params:(MTRDeviceControllerStartupParams *)params
{
if (!(self = [self initWithParams:params])) {
Expand Down Expand Up @@ -240,13 +241,15 @@ - (instancetype)initForNewFabric:(chip::FabricTable *)fabricTable

_fabricTable = fabricTable;
_keystore = keystore;
_advertiseOperational = advertiseOperational;

return self;
}

- (instancetype)initForExistingFabric:(FabricTable *)fabricTable
fabricIndex:(FabricIndex)fabricIndex
keystore:(chip::Crypto::OperationalKeystore *)keystore
advertiseOperational:(BOOL)advertiseOperational
params:(MTRDeviceControllerStartupParams *)params
{
if (!(self = [self initWithParams:params])) {
Expand Down Expand Up @@ -352,6 +355,7 @@ - (instancetype)initForExistingFabric:(FabricTable *)fabricTable
_fabricTable = fabricTable;
_fabricIndex.Emplace(fabricIndex);
_keystore = keystore;
_advertiseOperational = advertiseOperational;

return self;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ NS_ASSUME_NONNULL_BEGIN
// Key store we're using with our fabric table, for sanity checks.
@property (nonatomic, assign, readonly) chip::Crypto::OperationalKeystore * keystore;

@property (nonatomic, assign, readonly) BOOL advertiseOperational;

/**
* Helper method that checks that our keypairs match our certificates.
* Specifically:
Expand All @@ -73,6 +75,7 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (instancetype)initForNewFabric:(chip::FabricTable *)fabricTable
keystore:(chip::Crypto::OperationalKeystore *)keystore
advertiseOperational:(BOOL)advertiseOperational
params:(MTRDeviceControllerStartupParams *)params;

/**
Expand All @@ -81,6 +84,7 @@ NS_ASSUME_NONNULL_BEGIN
- (instancetype)initForExistingFabric:(chip::FabricTable *)fabricTable
fabricIndex:(chip::FabricIndex)fabricIndex
keystore:(chip::Crypto::OperationalKeystore *)keystore
advertiseOperational:(BOOL)advertiseOperational
params:(MTRDeviceControllerStartupParams *)params;

/**
Expand Down

0 comments on commit 2190864

Please sign in to comment.