Skip to content

Commit

Permalink
Improve APIs for controller startup on Darwin. (#18361)
Browse files Browse the repository at this point in the history
* Improve APIs for controller startup on Darwin.

* Adds a way of starting up a controller with an intermediate certificate.

* Allows controlling whether controller startup reuses existing certificates or
  creates new ones, and whether the operational identity or operational keys
  change in the process.  In particular, allows starting a controller with the
  same operatonal keys as it was using last time.

* Removes framework-internal writes of node id to storage, apart from whatever
  writes the fabric table does.

* Removes framework-internal writes of root certificates, apart from whatever
  writes the fabric table does.

* Removes use of the kinda-broken PERSISTENT_KEY_OP macros from Darwin code.

* Address review comments.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Aug 31, 2023
1 parent e235695 commit cd440e5
Show file tree
Hide file tree
Showing 22 changed files with 1,370 additions and 234 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,14 @@

constexpr const char * identities[] = { kIdentityAlpha, kIdentityBeta, kIdentityGamma };
for (size_t i = 0; i < ArraySize(identities); ++i) {
auto controllerParams = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:nocSigner ipk:ipk];
controllerParams.vendorId = chip::VendorId::TestVendor1;
controllerParams.fabricId = i + 1;
auto controllerParams = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:nocSigner fabricId:(i + 1) ipk:ipk];

// We're not sure whether we're creating a new fabric or using an
// existing one, so just try both.
auto controller = [factory startControllerOnExistingFabric:controllerParams];
if (controller == nil) {
// Maybe we didn't have this fabric yet.
controllerParams.vendorId = @(chip::VendorId::TestVendor1);
controller = [factory startControllerOnNewFabric:controllerParams];
}
if (controller == nil) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,8 @@ void CHIPSetNextAvailableDeviceID(uint64_t id)
return;
}

__auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:keys ipk:keys.ipk];
params.vendorId = kTestVendorId;
params.fabricId = 1;
__auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:keys fabricId:1 ipk:keys.ipk];
params.vendorId = @(kTestVendorId);

// We're not sure whether we have a fabric configured already; try as if
// we did, and if not fall back to creating a new one.
Expand All @@ -114,9 +113,7 @@ void CHIPSetNextAvailableDeviceID(uint64_t id)
[controller shutdown];

NSLog(@"Starting up the stack");
__auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:keys ipk:keys.ipk];
params.vendorId = kTestVendorId;
params.fabricId = 1;
__auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:keys fabricId:1 ipk:keys.ipk];

sController = [[MatterControllerFactory sharedInstance] startControllerOnExistingFabric:params];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ - (void)setupUI
- (void)_clearTextFields
{
CHIPDeviceController * chipController = InitializeCHIP();
_nodeIDTextField.text = [NSString stringWithFormat:@"%@", chipController.getControllerNodeId];
_nodeIDTextField.text = [NSString stringWithFormat:@"%@", chipController.controllerNodeId];
_endpointIDTextField.text = @"1";
_groupIDTextField.text = @"0";
_clusterIDTextField.text = @"";
Expand Down
12 changes: 12 additions & 0 deletions src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@
51E0310027EA20D20083DC9C /* CHIPControllerAccessControl.h in Headers */ = {isa = PBXBuildFile; fileRef = 51E030FE27EA20D20083DC9C /* CHIPControllerAccessControl.h */; };
51E0310127EA20D20083DC9C /* CHIPControllerAccessControl.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51E030FF27EA20D20083DC9C /* CHIPControllerAccessControl.mm */; };
51E24E73274E0DAC007CCF6E /* CHIPErrorTestUtils.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51E24E72274E0DAC007CCF6E /* CHIPErrorTestUtils.mm */; };
51E51FBF282AD37A00FC978D /* CHIPDeviceControllerStartupParams.h in Headers */ = {isa = PBXBuildFile; fileRef = 51E51FBC282AD37A00FC978D /* CHIPDeviceControllerStartupParams.h */; settings = {ATTRIBUTES = (Public, ); }; };
51E51FC0282AD37A00FC978D /* CHIPDeviceControllerStartupParams_Internal.h in Headers */ = {isa = PBXBuildFile; fileRef = 51E51FBD282AD37A00FC978D /* CHIPDeviceControllerStartupParams_Internal.h */; };
51E51FC1282AD37A00FC978D /* CHIPDeviceControllerStartupParams.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51E51FBE282AD37A00FC978D /* CHIPDeviceControllerStartupParams.mm */; };
5A60370827EA1FF60020DB79 /* CHIPAttributeCacheContainer+XPC.h in Headers */ = {isa = PBXBuildFile; fileRef = 5A60370727EA1FF60020DB79 /* CHIPAttributeCacheContainer+XPC.h */; };
5A6FEC9027B563D900F25F42 /* CHIPDeviceControllerOverXPC.m in Sources */ = {isa = PBXBuildFile; fileRef = 5A6FEC8F27B563D900F25F42 /* CHIPDeviceControllerOverXPC.m */; };
5A6FEC9227B5669C00F25F42 /* CHIPDeviceControllerOverXPC.h in Headers */ = {isa = PBXBuildFile; fileRef = 5A6FEC8D27B5624E00F25F42 /* CHIPDeviceControllerOverXPC.h */; };
Expand Down Expand Up @@ -161,6 +164,9 @@
51E030FE27EA20D20083DC9C /* CHIPControllerAccessControl.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CHIPControllerAccessControl.h; sourceTree = "<group>"; };
51E030FF27EA20D20083DC9C /* CHIPControllerAccessControl.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPControllerAccessControl.mm; sourceTree = "<group>"; };
51E24E72274E0DAC007CCF6E /* CHIPErrorTestUtils.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPErrorTestUtils.mm; sourceTree = "<group>"; };
51E51FBC282AD37A00FC978D /* CHIPDeviceControllerStartupParams.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CHIPDeviceControllerStartupParams.h; sourceTree = "<group>"; };
51E51FBD282AD37A00FC978D /* CHIPDeviceControllerStartupParams_Internal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CHIPDeviceControllerStartupParams_Internal.h; sourceTree = "<group>"; };
51E51FBE282AD37A00FC978D /* CHIPDeviceControllerStartupParams.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPDeviceControllerStartupParams.mm; sourceTree = "<group>"; };
5A60370727EA1FF60020DB79 /* CHIPAttributeCacheContainer+XPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "CHIPAttributeCacheContainer+XPC.h"; sourceTree = "<group>"; };
5A6FEC8B27B5609C00F25F42 /* CHIPDeviceOverXPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPDeviceOverXPC.h; sourceTree = "<group>"; };
5A6FEC8D27B5624E00F25F42 /* CHIPDeviceControllerOverXPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPDeviceControllerOverXPC.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -341,6 +347,9 @@
991DC0872475F47D00C13860 /* CHIPDeviceController.mm */,
5136660F28067D540025EDAE /* CHIPDeviceController_Internal.h */,
5136661128067D540025EDAE /* MatterControllerFactory_Internal.h */,
51E51FBD282AD37A00FC978D /* CHIPDeviceControllerStartupParams_Internal.h */,
51E51FBC282AD37A00FC978D /* CHIPDeviceControllerStartupParams.h */,
51E51FBE282AD37A00FC978D /* CHIPDeviceControllerStartupParams.mm */,
5136661228067D550025EDAE /* MatterControllerFactory.h */,
5136661028067D540025EDAE /* MatterControllerFactory.mm */,
5A7947E227C0101200434CF2 /* CHIPDeviceController+XPC.h */,
Expand Down Expand Up @@ -398,6 +407,7 @@
buildActionMask = 2147483647;
files = (
517BF3F0282B62B800A8B7DB /* MTRCertificates.h in Headers */,
51E51FBF282AD37A00FC978D /* CHIPDeviceControllerStartupParams.h in Headers */,
5136661628067D550025EDAE /* MatterControllerFactory.h in Headers */,
5A6FEC9927B5C88900F25F42 /* CHIPDeviceOverXPC.h in Headers */,
51B22C222740CB1D008D5055 /* CHIPCommandPayloadsObjc.h in Headers */,
Expand Down Expand Up @@ -436,6 +446,7 @@
1EC4CE6425CC276600D7304F /* CHIPClustersObjc.h in Headers */,
2C5EEEF6268A85C400CAE3D3 /* CHIPDeviceConnectionBridge.h in Headers */,
2C8C8FC0253E0C2100797F05 /* CHIPPersistentStorageDelegateBridge.h in Headers */,
51E51FC0282AD37A00FC978D /* CHIPDeviceControllerStartupParams_Internal.h in Headers */,
998F286F26D55EC5001846C6 /* CHIPP256KeypairBridge.h in Headers */,
2C222ADF255C811800E446B9 /* CHIPDevice_Internal.h in Headers */,
51E0310027EA20D20083DC9C /* CHIPControllerAccessControl.h in Headers */,
Expand Down Expand Up @@ -584,6 +595,7 @@
1ED276E226C5812A00547A89 /* CHIPCluster.mm in Sources */,
B2E0D7B3245B0B5C003C5B48 /* CHIPError.mm in Sources */,
1E85730C265519AE0050A4D9 /* callback-stub.cpp in Sources */,
51E51FC1282AD37A00FC978D /* CHIPDeviceControllerStartupParams.mm in Sources */,
1ED276E026C57CF000547A89 /* CHIPCallbackBridge.mm in Sources */,
517BF3F1282B62B800A8B7DB /* MTRCertificates.mm in Sources */,
5A6FEC9627B5983000F25F42 /* CHIPDeviceControllerXPCConnection.m in Sources */,
Expand Down
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ static_library("framework") {
"CHIPDeviceConnectionBridge.mm",
"CHIPDeviceController.h",
"CHIPDeviceController.mm",
"CHIPDeviceControllerStartupParams.h",
"CHIPDeviceControllerStartupParams.mm",
"CHIPDeviceControllerStartupParams_Internal.h",
"CHIPDeviceController_Internal.h",
"CHIPDevicePairingDelegate.h",
"CHIPDevicePairingDelegateBridge.h",
Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/CHIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#import <CHIP/CHIPDeviceAttestationDelegate.h>
#import <CHIP/CHIPDeviceController+XPC.h>
#import <CHIP/CHIPDeviceController.h>
#import <CHIP/CHIPDeviceControllerStartupParams.h>
#import <CHIP/CHIPDevicePairingDelegate.h>
#import <CHIP/CHIPError.h>
#import <CHIP/CHIPKeypair.h>
Expand Down
11 changes: 6 additions & 5 deletions src/darwin/Framework/CHIP/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ typedef void (^CHIPDeviceConnectionCallback)(CHIPDevice * _Nullable device, NSEr

@property (readonly, nonatomic) BOOL isRunning;

/**
* Return the Node Id assigned to the controller. Will return nil if the
* controller is not running (and hence does not know its node id).
*/
@property (readonly, nonatomic, nullable) NSNumber * controllerNodeId;

/**
* Start pairing for a device with the given ID, using the provided setup PIN
* to establish a PASE connection.
Expand Down Expand Up @@ -114,11 +120,6 @@ typedef void (^CHIPDeviceConnectionCallback)(CHIPDevice * _Nullable device, NSEr
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

/**
* Return the Node Id assigned to the controller.
*/
- (NSNumber *)getControllerNodeId;

/**
* Set the Delegate for the Device Pairing as well as the Queue on which the Delegate callbacks will be triggered
*
Expand Down
145 changes: 64 additions & 81 deletions src/darwin/Framework/CHIP/CHIPDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#import "CHIPDeviceController.h"

#import "CHIPCommissioningParameters.h"
#import "CHIPDeviceControllerStartupParams.h"
#import "CHIPDeviceControllerStartupParams_Internal.h"
#import "CHIPDevicePairingDelegateBridge.h"
#import "CHIPDevice_Internal.h"
#import "CHIPError_Internal.h"
Expand All @@ -27,6 +29,7 @@
#import "CHIPPersistentStorageDelegateBridge.h"
#import "CHIPSetupPayload.h"
#import "MatterControllerFactory_Internal.h"
#import "NSDataSpanConversion.h"
#import <setup_payload/ManualSetupPayloadGenerator.h>
#import <setup_payload/SetupPayload.h>
#import <zap-generated/CHIPClustersObjc.h>
Expand All @@ -47,8 +50,6 @@
#include <setup_payload/ManualSetupPayloadGenerator.h>
#include <system/SystemClock.h>

static const char * const CHIP_COMMISSIONER_DEVICE_ID_KEY = "com.zigbee.chip.commissioner.device_id";

static NSString * const kErrorCommissionerInit = @"Init failure while initializing a commissioner";
static NSString * const kErrorIPKInit = @"Init failure while initializing IPK";
static NSString * const kErrorOperationalCredentialsInit = @"Init failure while creating operational credentials delegate";
Expand All @@ -71,7 +72,6 @@ @interface CHIPDeviceController ()
@property (readonly) CHIPOperationalCredentialsDelegate * operationalCredentialsDelegate;
@property (readonly) CHIPP256KeypairBridge keypairBridge;
@property (readonly) CHIPDeviceAttestationDelegateBridge * deviceAttestationDelegateBridge;
@property (readonly) chip::NodeId localDeviceId;
@property (readonly) MatterControllerFactory * factory;
@end

Expand Down Expand Up @@ -140,20 +140,8 @@ - (void)cleanup
}
}

- (BOOL)startup:(CHIPDeviceControllerStartupParams *)startupParams
- (BOOL)startup:(CHIPDeviceControllerStartupParamsInternal *)startupParams
{
if (startupParams.vendorId == chip::VendorId::Common) {
// Shouldn't be using the "standard" vendor ID for actual devices.
CHIP_LOG_ERROR("%d is not a valid vendorId to initialize a device controller with", startupParams.vendorId);
return NO;
}

if (startupParams.fabricId == chip::kUndefinedFabricId) {
// Shouldn't be using the "standard" vendor ID for actual devices.
CHIP_LOG_ERROR("%llu is not a valid fabric id to initialize a device controller with", startupParams.fabricId);
return NO;
}

__block BOOL commissionerInitialized = NO;
if ([self isRunning]) {
CHIP_LOG_ERROR("Unexpected duplicate call to startup");
Expand All @@ -165,21 +153,39 @@ - (BOOL)startup:(CHIPDeviceControllerStartupParams *)startupParams
return;
}

if (startupParams.vendorId == nil || [startupParams.vendorId unsignedShortValue] == chip::VendorId::Common) {
// Shouldn't be using the "standard" vendor ID for actual devices.
CHIP_LOG_ERROR("%@ is not a valid vendorId to initialize a device controller with", startupParams.vendorId);
return;
}

if (startupParams.nodeId == nil) {
CHIP_LOG_ERROR("Can't start a controller if we don't know what node id it is");
return;
}

if ([startupParams nocSignerMatchesCerts] == NO) {
CHIP_LOG_ERROR("Provided nocSigner does not match certificates");
return;
}

if (startupParams.operationalCertificate != nil && startupParams.operationalKeypair == nullptr) {
CHIP_LOG_ERROR("Have no operational keypair for our operational certificate");
return;
}

CHIP_ERROR errorCode = CHIP_ERROR_INCORRECT_STATE;

// create a CHIPP256KeypairBridge here and pass it to the operationalCredentialsDelegate
_keypairBridge.Init(startupParams.rootCAKeypair);
_keypairBridge.Init(startupParams.nocSigner);
auto nativeBridge = std::make_unique<chip::Crypto::CHIPP256KeypairNativeBridge>(_keypairBridge);

errorCode
= _operationalCredentialsDelegate->init(_factory.storageDelegateBridge, std::move(nativeBridge), startupParams.ipk);
errorCode = _operationalCredentialsDelegate->Init(_factory.storageDelegateBridge, std::move(nativeBridge),
startupParams.ipk, startupParams.rootCertificate, startupParams.intermediateCertificate);
if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorOperationalCredentialsInit]) {
return;
}

// initialize NodeID if needed
[self _getControllerNodeId];

_cppCommissioner = new chip::Controller::DeviceCommissioner();
if ([self checkForStartError:(_cppCommissioner != nullptr) logMsg:kErrorCommissionerInit]) {
return;
Expand All @@ -191,31 +197,35 @@ - (BOOL)startup:(CHIPDeviceControllerStartupParams *)startupParams

commissionerParams.operationalCredentialsDelegate = _operationalCredentialsDelegate;

chip::Crypto::P256Keypair ephemeralKey;
errorCode = ephemeralKey.Initialize();
if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) {
return;
}

NSMutableData * nocBuffer = [[NSMutableData alloc] initWithLength:chip::Controller::kMaxCHIPDERCertLength];
chip::MutableByteSpan noc((uint8_t *) [nocBuffer mutableBytes], chip::Controller::kMaxCHIPDERCertLength);

NSMutableData * rcacBuffer = [[NSMutableData alloc] initWithLength:chip::Controller::kMaxCHIPDERCertLength];
chip::MutableByteSpan rcac((uint8_t *) [rcacBuffer mutableBytes], chip::Controller::kMaxCHIPDERCertLength);

chip::MutableByteSpan icac;
commissionerParams.controllerRCAC = _operationalCredentialsDelegate->RootCertSpan();
commissionerParams.controllerICAC = _operationalCredentialsDelegate->IntermediateCertSpan();

errorCode = _operationalCredentialsDelegate->GenerateNOCChainAfterValidation(
_localDeviceId, startupParams.fabricId, chip::kUndefinedCATs, ephemeralKey.Pubkey(), rcac, icac, noc);
chip::Crypto::P256Keypair operationalKeypair;
if (startupParams.operationalKeypair != nullptr) {
errorCode = operationalKeypair.Deserialize(*startupParams.operationalKeypair);
} else {
errorCode = operationalKeypair.Initialize();
}
if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) {
return;
}

commissionerParams.operationalKeypair = &ephemeralKey;
commissionerParams.controllerRCAC = rcac;
commissionerParams.controllerICAC = icac;
commissionerParams.controllerNOC = noc;
commissionerParams.controllerVendorId = static_cast<chip::VendorId>(startupParams.vendorId);
commissionerParams.operationalKeypair = &operationalKeypair;

// nocBuffer might not be used, but if it is it needs to live long enough.
uint8_t nocBuffer[chip::Controller::kMaxCHIPDERCertLength];
if (startupParams.operationalCertificate) {
commissionerParams.controllerNOC = AsByteSpan(startupParams.operationalCertificate);
} else {
chip::MutableByteSpan noc(nocBuffer);

errorCode = _operationalCredentialsDelegate->GenerateNOC([startupParams.nodeId unsignedLongLongValue],
startupParams.fabricId, chip::kUndefinedCATs, operationalKeypair.Pubkey(), noc);
if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) {
return;
}
commissionerParams.controllerNOC = noc;
}
commissionerParams.controllerVendorId = static_cast<chip::VendorId>([startupParams.vendorId unsignedShortValue]);

auto & factory = chip::Controller::DeviceControllerFactory::GetInstance();

Expand Down Expand Up @@ -253,31 +263,24 @@ - (BOOL)startup:(CHIPDeviceControllerStartupParams *)startupParams
return commissionerInitialized;
}

- (NSNumber *)getControllerNodeId
- (NSNumber *)controllerNodeId
{
if (![self isRunning]) {
CHIP_LOG_ERROR("A controller has no node id if it has not been started");
return nil;
}
__block NSNumber * nodeID;
dispatch_sync(_chipWorkQueue, ^{
nodeID = [self _getControllerNodeId];
if (![self isRunning]) {
CHIP_LOG_ERROR("A controller has no node id if it has not been started");
nodeID = nil;
} else {
nodeID = @(_cppCommissioner->GetNodeId());
}
});
return nodeID;
}

- (NSNumber *)_getControllerNodeId
{
uint16_t deviceIdLength = sizeof(_localDeviceId);
if (CHIP_NO_ERROR
!= _factory.storageDelegateBridge->SyncGetKeyValue(CHIP_COMMISSIONER_DEVICE_ID_KEY, &_localDeviceId, deviceIdLength)) {
_localDeviceId = arc4random();
_localDeviceId = _localDeviceId << 32 | arc4random();
CHIP_LOG_ERROR("Assigned %llx node ID to the controller", _localDeviceId);

_factory.storageDelegateBridge->SyncSetKeyValue(CHIP_COMMISSIONER_DEVICE_ID_KEY, &_localDeviceId, sizeof(_localDeviceId));
} else {
CHIP_LOG_ERROR("Found %llx node ID for the controller", _localDeviceId);
}
return [NSNumber numberWithUnsignedLongLong:_localDeviceId];
}

- (BOOL)pairDevice:(uint64_t)deviceID
discriminator:(uint16_t)discriminator
setupPINCode:(uint32_t)setupPINCode
Expand Down Expand Up @@ -718,23 +721,3 @@ - (CHIP_ERROR)isRunningOnFabric:(chip::FabricInfo *)fabric isRunning:(BOOL *)isR
}

@end

@implementation CHIPDeviceControllerStartupParams

- (instancetype)initWithKeypair:(id<CHIPKeypair>)rootCAKeypair ipk:(NSData *)ipk
{
if (!(self = [super init])) {
return nil;
}

_rootCAKeypair = rootCAKeypair;
_ipk = ipk;

// Set various invalid values.
_vendorId = chip::VendorId::Common;
_fabricId = chip::kUndefinedFabricId;

return self;
}

@end
Loading

0 comments on commit cd440e5

Please sign in to comment.