From 7193a5b3b106f75156e203e4744101ee0a719411 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 29 Jul 2021 17:30:33 -0700 Subject: [PATCH] cleanup delegate API --- src/controller/CHIPDeviceController.cpp | 33 ++++++++++++++++--- .../ExampleOperationalCredentialsIssuer.cpp | 18 +++++----- .../ExampleOperationalCredentialsIssuer.h | 18 ++++++++-- .../OperationalCredentialsDelegate.h | 23 +++++++++---- .../java/AndroidDeviceControllerWrapper.cpp | 18 +++++----- .../java/AndroidDeviceControllerWrapper.h | 15 +++++++-- .../CHIP/CHIPOperationalCredentialsDelegate.h | 17 ++++++++-- .../CHIPOperationalCredentialsDelegate.mm | 18 +++++----- 8 files changed, 116 insertions(+), 44 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index dff2194cc81be7..3875e1f389b2cb 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -281,8 +281,31 @@ CHIP_ERROR DeviceController::LoadLocalCredentials(Transport::FabricInfo * fabric ChipLogProgress(Controller, "Getting certificate chain for the controller from the issuer"); + // As per specifications section 11.22.5.1. Constant RESP_MAX + constexpr uint16_t kMaxRspLen = 900; + chip::Platform::ScopedMemoryBuffer csrElements; + ReturnErrorCodeIf(!csrElements.Alloc(kMaxRspLen), CHIP_ERROR_NO_MEMORY); + + TLV::TLVWriter csrElementWriter; + TLV::TLVType containerType; + csrElementWriter.Init(csrElements.Get(), kMaxRspLen); + ReturnErrorOnFailure(csrElementWriter.StartContainer(TLV::AnonymousTag, TLV::TLVType::kTLVType_Structure, containerType)); + ReturnErrorOnFailure(csrElementWriter.Put(TLV::ContextTag(1), ByteSpan(CSR.Get(), csrLength))); + + // TODO - Need a mechanism to generate CSRNonce for commissioner's CSR + ReturnErrorOnFailure(csrElementWriter.Put(TLV::ContextTag(2), ByteSpan())); + ReturnErrorOnFailure(csrElementWriter.Put(TLV::ContextTag(3), ByteSpan())); + ReturnErrorOnFailure(csrElementWriter.Put(TLV::ContextTag(4), ByteSpan())); + ReturnErrorOnFailure(csrElementWriter.Put(TLV::ContextTag(5), ByteSpan())); + ReturnErrorOnFailure(csrElementWriter.EndContainer(containerType)); + ReturnErrorOnFailure(csrElementWriter.Finalize()); + + mOperationalCredentialsDelegate->SetNodeIdForNextNOCRequest(mLocalDeviceId); + mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(0); + + // TODO - Need a mechanism to generate signature for commissioner's CSR ReturnErrorOnFailure(mOperationalCredentialsDelegate->GenerateNOCChain( - Optional(mLocalDeviceId), 0, ByteSpan(CSR.Get(), csrLength), ByteSpan(), ByteSpan(), ByteSpan(), ByteSpan(), + ByteSpan(csrElements.Get(), csrElementWriter.GetLengthWritten()), ByteSpan(), ByteSpan(), ByteSpan(), ByteSpan(), &mLocalNOCChainCallback)); ReturnErrorOnFailure(mFabrics.Store(fabric->GetFabricIndex())); @@ -1296,9 +1319,11 @@ CHIP_ERROR DeviceCommissioner::ProcessOpCSR(const ByteSpan & NOCSRElements, cons ChipLogProgress(Controller, "Getting certificate chain for the device from the issuer"); - return mOperationalCredentialsDelegate->GenerateNOCChain(Optional(device->GetDeviceId()), 0, NOCSRElements, - AttestationSignature, ByteSpan(), ByteSpan(), ByteSpan(), - &mDeviceNOCChainCallback); + mOperationalCredentialsDelegate->SetNodeIdForNextNOCRequest(device->GetDeviceId()); + mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(0); + + return mOperationalCredentialsDelegate->GenerateNOCChain(NOCSRElements, AttestationSignature, ByteSpan(), ByteSpan(), + ByteSpan(), &mDeviceNOCChainCallback); } CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(Device * device, const ByteSpan & opCertBuf) diff --git a/src/controller/ExampleOperationalCredentialsIssuer.cpp b/src/controller/ExampleOperationalCredentialsIssuer.cpp index 4e0b1df50c757b..2f63d901828d95 100644 --- a/src/controller/ExampleOperationalCredentialsIssuer.cpp +++ b/src/controller/ExampleOperationalCredentialsIssuer.cpp @@ -89,24 +89,24 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::Initialize(PersistentStorageDele return CHIP_NO_ERROR; } -CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const Optional & nodeId, FabricId fabricId, - const ByteSpan & csrElements, +CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC, const ByteSpan & PAI, const ByteSpan & PAA, Callback::Callback * onCompletion) { VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); NodeId assignedId; - if (nodeId.HasValue()) + if (mNodeIdRequested) { - assignedId = nodeId.Value(); + assignedId = mNextRequestedNodeId; + mNodeIdRequested = false; } else { assignedId = mNextAvailableNodeId++; } - X509CertRequestParams noc_request = { 1, mIntermediateIssuerId, mNow, mNow + mValidity, true, fabricId, true, assignedId }; + X509CertRequestParams noc_request = { 1, mIntermediateIssuerId, mNow, mNow + mValidity, true, mNextFabricId, true, assignedId }; ChipLogProgress(Controller, "Verifying Certificate Signing Request"); TLVReader reader; @@ -138,7 +138,7 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const Optional< ReturnErrorOnFailure(NewNodeOperationalX509Cert(noc_request, CertificateIssuerLevel::kIssuerIsIntermediateCA, pubkey, mIntermediateIssuer, noc.Get(), kMaxCHIPDERCertLength, nocLen)); - X509CertRequestParams icac_request = { 0, mIssuerId, mNow, mNow + mValidity, true, fabricId, false, 0 }; + X509CertRequestParams icac_request = { 0, mIssuerId, mNow, mNow + mValidity, true, mNextFabricId, false, 0 }; chip::Platform::ScopedMemoryBuffer icac; ReturnErrorCodeIf(!icac.Alloc(kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY); @@ -153,19 +153,19 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const Optional< uint16_t rootCertBufLen = kMaxCHIPDERCertLength; CHIP_ERROR err = CHIP_NO_ERROR; - PERSISTENT_KEY_OP(fabricId, kOperationalCredentialsRootCertificateStorage, key, + PERSISTENT_KEY_OP(mNextFabricId, kOperationalCredentialsRootCertificateStorage, key, err = mStorage->SyncGetKeyValue(key, rcac.Get(), rootCertBufLen)); if (err != CHIP_NO_ERROR) { // Storage doesn't have an existing root certificate. Let's create one and add it to the storage. - X509CertRequestParams request = { 0, mIssuerId, mNow, mNow + mValidity, true, fabricId, false, 0 }; + X509CertRequestParams request = { 0, mIssuerId, mNow, mNow + mValidity, true, mNextFabricId, false, 0 }; uint32_t outCertLen = 0; ChipLogProgress(Controller, "Generating RCAC"); ReturnErrorOnFailure(NewRootX509Cert(request, mIssuer, rcac.Get(), kMaxCHIPDERCertLength, outCertLen)); VerifyOrReturnError(CanCastTo(outCertLen), CHIP_ERROR_INVALID_ARGUMENT); rootCertBufLen = static_cast(outCertLen); - PERSISTENT_KEY_OP(fabricId, kOperationalCredentialsRootCertificateStorage, key, + PERSISTENT_KEY_OP(mNextFabricId, kOperationalCredentialsRootCertificateStorage, key, err = mStorage->SyncSetKeyValue(key, rcac.Get(), rootCertBufLen)); ReturnErrorOnFailure(err); } diff --git a/src/controller/ExampleOperationalCredentialsIssuer.h b/src/controller/ExampleOperationalCredentialsIssuer.h index a2c21d09decaab..ca94f081f57338 100644 --- a/src/controller/ExampleOperationalCredentialsIssuer.h +++ b/src/controller/ExampleOperationalCredentialsIssuer.h @@ -43,9 +43,17 @@ class DLL_EXPORT ExampleOperationalCredentialsIssuer : public OperationalCredent public: virtual ~ExampleOperationalCredentialsIssuer() {} - CHIP_ERROR GenerateNOCChain(const Optional & nodeId, FabricId fabricId, const ByteSpan & csrElements, - const ByteSpan & attestationSignature, const ByteSpan & DAC, const ByteSpan & PAI, - const ByteSpan & PAA, Callback::Callback * onCompletion) override; + CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC, + const ByteSpan & PAI, const ByteSpan & PAA, + Callback::Callback * onCompletion) override; + + void SetNodeIdForNextNOCRequest(NodeId nodeId) override + { + mNextRequestedNodeId = nodeId; + mNodeIdRequested = true; + } + + void SetFabricIdForNextNOCRequest(FabricId fabricId) override { mNextFabricId = fabricId; } /** * @brief Initialize the issuer with the keypair in the storage. @@ -88,6 +96,10 @@ class DLL_EXPORT ExampleOperationalCredentialsIssuer : public OperationalCredent NodeId mNextAvailableNodeId = 1; PersistentStorageDelegate * mStorage = nullptr; + + NodeId mNextRequestedNodeId = 1; + FabricId mNextFabricId = 0; + bool mNodeIdRequested = false; }; } // namespace Controller diff --git a/src/controller/OperationalCredentialsDelegate.h b/src/controller/OperationalCredentialsDelegate.h index dcc3d2ae858d55..0f451d4f7fcfdb 100644 --- a/src/controller/OperationalCredentialsDelegate.h +++ b/src/controller/OperationalCredentialsDelegate.h @@ -50,9 +50,6 @@ class DLL_EXPORT OperationalCredentialsDelegate * * The delegate will call `onCompletion` when the NOC certificate chain is ready. * - * @param[in] nodeId Optional node ID. If provided, the generated NOC must use the provided ID. - * If ID is not provided, the delegate must generate one. - * @param[in] fabricId Fabric ID for which the certificate is being requested. * @param[in] csrElements CSR elements as per specifications section 11.22.5.6. NOCSR Elements. * @param[in] attestationSignature Attestation signature as per specifications section 11.22.7.6. CSRResponse Command. * @param[in] DAC Device attestation certificate received from the device being commissioned @@ -62,9 +59,23 @@ class DLL_EXPORT OperationalCredentialsDelegate * * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code. */ - virtual CHIP_ERROR GenerateNOCChain(const Optional & nodeId, FabricId fabricId, const ByteSpan & csrElements, - const ByteSpan & attestationSignature, const ByteSpan & DAC, const ByteSpan & PAI, - const ByteSpan & PAA, Callback::Callback * onCompletion) = 0; + virtual CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC, + const ByteSpan & PAI, const ByteSpan & PAA, + Callback::Callback * onCompletion) = 0; + + /** + * This function sets the node ID for which the next NOC Chain would be requested. The node ID is + * provided as a hint, and the delegate implementation may chose to ignore it and pick node ID of + * their choice. + */ + virtual void SetNodeIdForNextNOCRequest(NodeId nodeId) {} + + /** + * This function sets the fabric ID for which the next NOC Chain should be generated. This API is + * not required to be implemented if the delegate implementation has other mechanisms to find the + * fabric ID. + */ + virtual void SetFabricIdForNextNOCRequest(FabricId fabricId) {} }; } // namespace Controller diff --git a/src/controller/java/AndroidDeviceControllerWrapper.cpp b/src/controller/java/AndroidDeviceControllerWrapper.cpp index 359f2e475cdaa4..ba46c922ba82c7 100644 --- a/src/controller/java/AndroidDeviceControllerWrapper.cpp +++ b/src/controller/java/AndroidDeviceControllerWrapper.cpp @@ -63,8 +63,7 @@ void AndroidDeviceControllerWrapper::CallJavaMethod(const char * methodName, jin // TODO Refactor this API to match latest spec, so that GenerateNodeOperationalCertificate receives the full CSR Elements data // payload. -CHIP_ERROR AndroidDeviceControllerWrapper::GenerateNOCChain(const Optional & nodeId, FabricId fabricId, - const ByteSpan & csrElements, const ByteSpan & attestationSignature, +CHIP_ERROR AndroidDeviceControllerWrapper::GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC, const ByteSpan & PAI, const ByteSpan & PAA, Callback::Callback * onCompletion) { @@ -82,16 +81,19 @@ CHIP_ERROR AndroidDeviceControllerWrapper::GenerateNOCChain(const Optional(csrElements.size())); @@ -128,18 +130,18 @@ CHIP_ERROR AndroidDeviceControllerWrapper::GenerateNOCChain(const Optional(outCertLen), CHIP_ERROR_INVALID_ARGUMENT); rootCertBufLen = static_cast(outCertLen); - PERSISTENT_KEY_OP(fabricId, kOperationalCredentialsRootCertificateStorage, key, + PERSISTENT_KEY_OP(mNextFabricId, kOperationalCredentialsRootCertificateStorage, key, err = SyncSetKeyValue(key, rcac.Get(), rootCertBufLen)); ReturnErrorOnFailure(err); } diff --git a/src/controller/java/AndroidDeviceControllerWrapper.h b/src/controller/java/AndroidDeviceControllerWrapper.h index a06845e9f4b96a..f7f7918d0da816 100644 --- a/src/controller/java/AndroidDeviceControllerWrapper.h +++ b/src/controller/java/AndroidDeviceControllerWrapper.h @@ -58,11 +58,18 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR error) override; // OperationalCredentialsDelegate implementation - CHIP_ERROR GenerateNOCChain(const chip::Optional & nodeId, chip::FabricId fabricId, - const chip::ByteSpan & csrElements, const chip::ByteSpan & attestationSignature, + CHIP_ERROR GenerateNOCChain(const chip::ByteSpan & csrElements, const chip::ByteSpan & attestationSignature, const chip::ByteSpan & DAC, const chip::ByteSpan & PAI, const chip::ByteSpan & PAA, chip::Callback::Callback * onCompletion) override; + void SetNodeIdForNextNOCRequest(NodeId nodeId) override + { + mNextRequestedNodeId = nodeId; + mNodeIdRequested = true; + } + + void SetFabricIdForNextNOCRequest(FabricId fabricId) override { mNextFabricId = fabricId; } + // DeviceStatusDelegate implementation void OnMessage(chip::System::PacketBufferHandle && msg) override; void OnStatusChange(void) override; @@ -99,6 +106,10 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel chip::NodeId mNextAvailableNodeId = 1; + NodeId mNextRequestedNodeId = 1; + FabricId mNextFabricId = 0; + bool mNodeIdRequested = false; + AndroidDeviceControllerWrapper(ChipDeviceControllerPtr controller, pthread_mutex_t * stackLock) : mController(std::move(controller)), mStackLock(stackLock) { diff --git a/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.h b/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.h index 86216312324b26..ddad008a3c4d28 100644 --- a/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.h +++ b/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.h @@ -33,11 +33,18 @@ class CHIPOperationalCredentialsDelegate : public chip::Controller::OperationalC CHIP_ERROR init(CHIPPersistentStorageDelegateBridge * storage); - CHIP_ERROR GenerateNOCChain(const chip::Optional & nodeId, chip::FabricId fabricId, - const chip::ByteSpan & csrElements, const chip::ByteSpan & attestationSignature, const chip::ByteSpan & DAC, - const chip::ByteSpan & PAI, const chip::ByteSpan & PAA, + CHIP_ERROR GenerateNOCChain(const chip::ByteSpan & csrElements, const chip::ByteSpan & attestationSignature, + const chip::ByteSpan & DAC, const chip::ByteSpan & PAI, const chip::ByteSpan & PAA, chip::Callback::Callback * onCompletion) override; + void SetNodeIdForNextNOCRequest(chip::NodeId nodeId) override + { + mNextRequestedNodeId = nodeId; + mNodeIdRequested = true; + } + + void SetFabricIdForNextNOCRequest(chip::FabricId fabricId) override { mNextFabricId = fabricId; } + void SetDeviceID(chip::NodeId deviceId) { mDeviceBeingPaired = deviceId; } void ResetDeviceID() { mDeviceBeingPaired = chip::kUndefinedNodeId; } @@ -69,6 +76,10 @@ class CHIPOperationalCredentialsDelegate : public chip::Controller::OperationalC CHIPPersistentStorageDelegateBridge * mStorage; chip::NodeId mDeviceBeingPaired = chip::kUndefinedNodeId; + + chip::NodeId mNextRequestedNodeId = 1; + chip::FabricId mNextFabricId = 0; + bool mNodeIdRequested = false; }; NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm index 63f87aab24cf5e..29354223609dab 100644 --- a/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm +++ b/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm @@ -242,9 +242,8 @@ static BOOL isRunningTests(void) return CHIP_NO_ERROR; } -CHIP_ERROR CHIPOperationalCredentialsDelegate::GenerateNOCChain(const chip::Optional & nodeId, - chip::FabricId fabricId, const chip::ByteSpan & csrElements, const chip::ByteSpan & attestationSignature, - const chip::ByteSpan & DAC, const chip::ByteSpan & PAI, const chip::ByteSpan & PAA, +CHIP_ERROR CHIPOperationalCredentialsDelegate::GenerateNOCChain(const chip::ByteSpan & csrElements, + const chip::ByteSpan & attestationSignature, const chip::ByteSpan & DAC, const chip::ByteSpan & PAI, const chip::ByteSpan & PAA, chip::Callback::Callback * onCompletion) { uint32_t validityStart, validityEnd; @@ -260,8 +259,9 @@ static BOOL isRunningTests(void) } chip::NodeId assignedId; - if (nodeId.HasValue()) { - assignedId = nodeId.Value(); + if (mNodeIdRequested) { + assignedId = mNextRequestedNodeId; + mNodeIdRequested = false; } else { if (mDeviceBeingPaired == chip::kUndefinedNodeId) { return CHIP_ERROR_INCORRECT_STATE; @@ -270,7 +270,7 @@ static BOOL isRunningTests(void) } chip::Credentials::X509CertRequestParams noc_request - = { 1, mIssuerId, validityStart, validityEnd, true, fabricId, true, assignedId }; + = { 1, mIssuerId, validityStart, validityEnd, true, mNextFabricId, true, assignedId }; TLVReader reader; reader.Init(csrElements.data(), static_cast(csrElements.size())); @@ -306,11 +306,11 @@ static BOOL isRunningTests(void) CHIP_ERROR err = CHIP_NO_ERROR; PERSISTENT_KEY_OP( - fabricId, kOperationalCredentialsRootCertificateStorage, key, err = mStorage->SyncGetKeyValue(key, rcac, rcacLen)); + mNextFabricId, kOperationalCredentialsRootCertificateStorage, key, err = mStorage->SyncGetKeyValue(key, rcac, rcacLen)); if (err != CHIP_NO_ERROR) { chip::Credentials::X509CertRequestParams rcac_request - = { 0, mIssuerId, validityStart, validityEnd, true, fabricId, false, 0 }; + = { 0, mIssuerId, validityStart, validityEnd, true, mNextFabricId, false, 0 }; uint32_t outCertLen = 0; ReturnErrorOnFailure(chip::Credentials::NewRootX509Cert( rcac_request, mIssuerKey, rcac, chip::Controller::kMaxCHIPDERCertLength, outCertLen)); @@ -318,7 +318,7 @@ static BOOL isRunningTests(void) VerifyOrReturnError(CanCastTo(outCertLen), CHIP_ERROR_INVALID_ARGUMENT); rcacLen = static_cast(outCertLen); PERSISTENT_KEY_OP( - fabricId, kOperationalCredentialsRootCertificateStorage, key, err = mStorage->SyncSetKeyValue(key, rcac, rcacLen)); + mNextFabricId, kOperationalCredentialsRootCertificateStorage, key, err = mStorage->SyncSetKeyValue(key, rcac, rcacLen)); ReturnErrorOnFailure(err); }