Skip to content

Commit

Permalink
Allow Darwin framework consumers to provide a controller NOC and keyp…
Browse files Browse the repository at this point in the history 
…air.

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 project-chip#18444
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple committed May 17, 2022
1 parent d79a0e3 commit bb2e6ef
Show file tree
Hide file tree
Showing 14 changed files with 878 additions and 298 deletions.
4 changes: 3 additions & 1 deletion examples/chip-tool-darwin/commands/common/CHIPCommandBridge.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 18 additions & 4 deletions src/credentials/FabricTable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -583,13 +589,21 @@ 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: 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);
Expand Down
18 changes: 2 additions & 16 deletions src/credentials/FabricTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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];

Expand Down
64 changes: 45 additions & 19 deletions src/darwin/Framework/CHIP/CHIPDeviceController.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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
Expand Down Expand Up @@ -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]) {
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down
Loading

0 comments on commit bb2e6ef

Please sign in to comment.