Allow Darwin framework consumers to provide a controller NOC and keyp…

…air (#18519)

* Allow Darwin framework consumers to provide a controller NOC and keypair.

Specific changes:

* Rename initWithKeypair API to initWithSigningKeypair, since we can
  now also init with an operational keypair.

* Fix FabricInfo::SetOperationalKeypair to correcty handle the
  mHasExternallyOwnedOperationalKey case.  The old code would try to
  deserialize into the externally owned keypair.

* Fix FabricInfo::GetOperationalKey to just return the key even it
  it's null, instead of allocating a random key that does not match
  anything.  I have checked that consumers all either null-check the
  call or have just called SetOperationalKeypair or
  SetExternallyOwnedOperationalKeypair.  SetFabricInfo is updated to
  error out if the incoming fabric info has a null operational key.
  This change was needed for basic API sanity in terms of not
  accidentally switching a fabric from an externally managed
  operational key to an internally managed randomly generated one.

* Fix backwards boolean check in FabricInfo::Reset that was causing us
  to leak internally managed keys and try to delete externally managed
  ones.

* Change Darwin CHIPDeviceControllerStartupParams to allow providing
  an operational keypair to be used for the NOC.

* Change Darwin CHIPDeviceControllerStartupParams To allow providing a
  NOC instead of having one generated inside the framework.

* Refactor the code for initializing
  CHIPDeviceControllerStartupParamsInternal to better share code and
  support the new functionality.

* Allow initializing CHIPOperationalCredentialsDelegate without a
  NOC-signing keypair.  This is needed because the SDK's controller
  init requires a credentials delegate.  When initialized in this way,
  the delegate will just return error when asked to create a NOC.

* Added tests for the new API (which caught a number of the issues
  listed above).

Fixes #18444

* Address review comments.

examples/chip-tool-darwin/commands/common/CHIPCommandBridge.mm

@@ -56,7 +56,9 @@
 
  constexpr const char * identities[] = { kIdentityAlpha, kIdentityBeta, kIdentityGamma };
  for (size_t i = 0; i < ArraySize(identities); ++i) {
- auto controllerParams = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:nocSigner fabricId:(i + 1) ipk:ipk];
+ auto controllerParams = [[CHIPDeviceControllerStartupParams alloc] initWithSigningKeypair: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.

src/credentials/FabricTable.cpp

@@ -324,10 +324,16 @@ CHIP_ERROR FabricInfo::SetOperationalKeypair(const P256Keypair * keyPair)
 {
  VerifyOrReturnError(keyPair != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
- mHasExternallyOwnedOperationalKey = false;
-
  P256SerializedKeypair serialized;
  ReturnErrorOnFailure(keyPair->Serialize(serialized));
+
+ if (mHasExternallyOwnedOperationalKey)
+ {
+ // Drop it, so we will allocate an internally owned one.
+ mOperationalKey = nullptr;
+ mHasExternallyOwnedOperationalKey = false;
+ }
+
  if (mOperationalKey == nullptr)
  {
 #ifdef ENABLE_HSM_CASE_OPS_KEY
@@ -583,13 +589,22 @@ CHIP_ERROR FabricInfo::SetFabricInfo(FabricInfo & newFabric)
  ReturnErrorOnFailure(VerifyCredentials(newFabric.mNOCCert, newFabric.mICACert, newFabric.mRootCert, validContext, operationalId,
  fabricId, pubkey));
 
+ auto * operationalKey = newFabric.GetOperationalKey();
+ if (operationalKey == nullptr)
+ {
+ return CHIP_ERROR_INVALID_ARGUMENT;
+ }
+
+ // TODO: https://github.com/project-chip/connectedhomeip/issues/8433 -- Should verify that pubkey matches operationalKey's
+ // public key.
+
  if (newFabric.mHasExternallyOwnedOperationalKey)
  {
- ReturnErrorOnFailure(SetExternallyOwnedOperationalKeypair(newFabric.GetOperationalKey()));
+ ReturnErrorOnFailure(SetExternallyOwnedOperationalKeypair(operationalKey));
  }
  else
  {
- ReturnErrorOnFailure(SetOperationalKeypair(newFabric.GetOperationalKey()));
+ ReturnErrorOnFailure(SetOperationalKeypair(operationalKey));
  }
 
  SetRootCert(newFabric.mRootCert);

src/credentials/FabricTable.h

@@ -109,21 +109,7 @@ class DLL_EXPORT FabricInfo
 
  void SetVendorId(uint16_t vendorId) { mVendorId = vendorId; }
 
- Crypto::P256Keypair * GetOperationalKey() const
- {
- if (!mHasExternallyOwnedOperationalKey && (mOperationalKey == nullptr))
- {
- // TODO: Refactor the following two cases to go through SetOperationalKey()
-#ifdef ENABLE_HSM_CASE_OPS_KEY
- mOperationalKey = chip::Platform::New<Crypto::P256KeypairHSM>();
- mOperationalKey->CreateOperationalKey(mFabricIndex);
-#else
- mOperationalKey = chip::Platform::New<Crypto::P256Keypair>();
- mOperationalKey->Initialize();
-#endif
- }
- return mOperationalKey;
- }
+ Crypto::P256Keypair * GetOperationalKey() const { return mOperationalKey; }
 
  /**
  * Sets the P256Keypair used for this fabric. This will make a copy of the keypair
@@ -212,7 +198,7 @@ class DLL_EXPORT FabricInfo
  mVendorId = VendorId::NotSpecified;
  mFabricLabel[0] = '\0';
 
- if (mHasExternallyOwnedOperationalKey && mOperationalKey != nullptr)
+ if (!mHasExternallyOwnedOperationalKey && mOperationalKey != nullptr)
  {
  chip::Platform::Delete(mOperationalKey);
  }

src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m

@@ -87,7 +87,7 @@ void CHIPSetNextAvailableDeviceID(uint64_t id)
  return;
  }
 
- __auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:keys fabricId:1 ipk:keys.ipk];
+ __auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithSigningKeypair:keys fabricId:1 ipk:keys.ipk];
  params.vendorId = @(kTestVendorId);
 
  // We're not sure whether we have a fabric configured already; try as if
@@ -113,7 +113,7 @@ void CHIPSetNextAvailableDeviceID(uint64_t id)
  [controller shutdown];
 
  NSLog(@"Starting up the stack");
- __auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithKeypair:keys fabricId:1 ipk:keys.ipk];
+ __auto_type * params = [[CHIPDeviceControllerStartupParams alloc] initWithSigningKeypair:keys fabricId:1 ipk:keys.ipk];
 
  sController = [[MatterControllerFactory sharedInstance] startControllerOnExistingFabric:params];
 

src/darwin/Framework/CHIP/CHIPDeviceController.mm

@@ -52,7 +52,9 @@
 
 static NSString * const kErrorCommissionerInit = @"Init failure while initializing a commissioner";
 static NSString * const kErrorIPKInit = @"Init failure while initializing IPK";
+static NSString * const kErrorSigningKeypairInit = @"Init failure while creating signing keypair bridge";
 static NSString * const kErrorOperationalCredentialsInit = @"Init failure while creating operational credentials delegate";
+static NSString * const kErrorOperationalKeypairInit = @"Init failure while creating operational keypair bridge";
 static NSString * const kErrorPairingInit = @"Init failure while creating a pairing delegate";
 static NSString * const kErrorPairDevice = @"Failure while pairing the device";
 static NSString * const kErrorUnpairDevice = @"Failure while unpairing the device";
@@ -70,7 +72,9 @@ @interface CHIPDeviceController ()
 @property (readonly) chip::Controller::DeviceCommissioner * cppCommissioner;
 @property (readonly) CHIPDevicePairingDelegateBridge * pairingDelegateBridge;
 @property (readonly) CHIPOperationalCredentialsDelegate * operationalCredentialsDelegate;
-@property (readonly) CHIPP256KeypairBridge keypairBridge;
+@property (readonly) CHIPP256KeypairBridge signingKeypairBridge;
+@property (readonly) CHIPP256KeypairBridge operationalKeypairBridge;
+@property (readonly) chip::Optional<chip::CHIPP256KeypairNativeBridge> operationalKeypairNativeBridge;
 @property (readonly) CHIPDeviceAttestationDelegateBridge * deviceAttestationDelegateBridge;
 @property (readonly) MatterControllerFactory * factory;
 @end
@@ -159,27 +163,33 @@ - (BOOL)startup:(CHIPDeviceControllerStartupParamsInternal *)startupParams
  return;
  }
 
- if (startupParams.nodeId == nil) {
+ if (startupParams.operationalCertificate == nil && 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");
+ if ([startupParams keypairsMatchCertificates] == NO) {
+ CHIP_LOG_ERROR("Provided keypairs do not match certificates");
  return;
  }
 
- if (startupParams.operationalCertificate != nil && startupParams.operationalKeypair == nullptr) {
+ if (startupParams.operationalCertificate != nil && startupParams.operationalKeypair == nil
+ && startupParams.serializedOperationalKeypair == 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.nocSigner);
- auto nativeBridge = std::make_unique<chip::Crypto::CHIPP256KeypairNativeBridge>(_keypairBridge);
-
+ std::unique_ptr<chip::Crypto::CHIPP256KeypairNativeBridge> nativeBridge;
+ if (startupParams.nocSigner) {
+ errorCode = _signingKeypairBridge.Init(startupParams.nocSigner);
+ if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorSigningKeypairInit]) {
+ return;
+ }
+ nativeBridge = std::make_unique<chip::Crypto::CHIPP256KeypairNativeBridge>(_signingKeypairBridge);
+ }
  errorCode = _operationalCredentialsDelegate->Init(_factory.storageDelegateBridge, std::move(nativeBridge),
  startupParams.ipk, startupParams.rootCertificate, startupParams.intermediateCertificate);
  if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorOperationalCredentialsInit]) {
@@ -191,6 +201,15 @@ - (BOOL)startup:(CHIPDeviceControllerStartupParamsInternal *)startupParams
  return;
  }
 
+ // internallyCreatedOperationalKeypair might not be used, but
+ // if it is it needs to live long enough (until after we are
+ // done using commissionerParams).
+ chip::Crypto::P256Keypair internallyCreatedOperationalKeypair;
+ // nocBuffer might not be used, but if it is it needs to live
+ // long enough (until after we are done using
+ // commissionerParams).
+ uint8_t nocBuffer[chip::Controller::kMaxCHIPDERCertLength];
+
  chip::Controller::SetupParams commissionerParams;
 
  commissionerParams.pairingDelegate = _pairingDelegateBridge;
@@ -200,26 +219,33 @@ - (BOOL)startup:(CHIPDeviceControllerStartupParamsInternal *)startupParams
  commissionerParams.controllerRCAC = _operationalCredentialsDelegate->RootCertSpan();
  commissionerParams.controllerICAC = _operationalCredentialsDelegate->IntermediateCertSpan();
 
- chip::Crypto::P256Keypair operationalKeypair;
- if (startupParams.operationalKeypair != nullptr) {
- errorCode = operationalKeypair.Deserialize(*startupParams.operationalKeypair);
+ if (startupParams.operationalKeypair != nil) {
+ errorCode = _operationalKeypairBridge.Init(startupParams.operationalKeypair);
+ if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorOperationalKeypairInit]) {
+ return;
+ }
+ _operationalKeypairNativeBridge.Emplace(_operationalKeypairBridge);
+ commissionerParams.operationalKeypair = &_operationalKeypairNativeBridge.Value();
+ commissionerParams.hasExternallyOwnedOperationalKeypair = true;
  } else {
- errorCode = operationalKeypair.Initialize();
- }
- if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) {
- return;
+ if (startupParams.serializedOperationalKeypair != nullptr) {
+ errorCode = internallyCreatedOperationalKeypair.Deserialize(*startupParams.serializedOperationalKeypair);
+ } else {
+ errorCode = internallyCreatedOperationalKeypair.Initialize();
+ }
+ if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) {
+ return;
+ }
+ commissionerParams.operationalKeypair = &internallyCreatedOperationalKeypair;
  }
- 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);
+ startupParams.fabricId, chip::kUndefinedCATs, commissionerParams.operationalKeypair->Pubkey(), noc);
  if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) {
  return;
  }