From 6ec9f10a36cb6a47aeb3a02a078036ef0955774f Mon Sep 17 00:00:00 2001 From: Thivya Ashokkumar Date: Tue, 30 Jan 2024 17:45:37 -0800 Subject: [PATCH] Addressed review comments --- src/app/icd/client/CheckInDelegate.h | 4 ++-- src/app/icd/client/DefaultCheckInDelegate.cpp | 10 +++++----- src/app/icd/client/DefaultCheckInDelegate.h | 2 +- src/app/icd/client/RefreshKeySender.cpp | 11 +++++------ src/app/icd/client/RefreshKeySender.h | 7 ++----- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/app/icd/client/CheckInDelegate.h b/src/app/icd/client/CheckInDelegate.h index b4055680b7194f..5c931d414aab2c 100644 --- a/src/app/icd/client/CheckInDelegate.h +++ b/src/app/icd/client/CheckInDelegate.h @@ -72,10 +72,10 @@ class DLL_EXPORT CheckInDelegate * * @param[in] refreshKeySender - pointer to the RefreshKeySender object that was used for the key refresh process. The caller * will NOT use this pointer any more. - * @param[in] aError - CHIP_NO_ERROR indicates successful re-registration using the new key + * @param[in] error - CHIP_NO_ERROR indicates successful re-registration using the new key * Other errors indicate the failure reason. */ - virtual void OnKeyRefreshDone(RefreshKeySender * refreshKeySender, CHIP_ERROR aError) = 0; + virtual void OnKeyRefreshDone(RefreshKeySender * refreshKeySender, CHIP_ERROR error) = 0; }; } // namespace app diff --git a/src/app/icd/client/DefaultCheckInDelegate.cpp b/src/app/icd/client/DefaultCheckInDelegate.cpp index 3045374538b1f5..8df98006f6e9e0 100644 --- a/src/app/icd/client/DefaultCheckInDelegate.cpp +++ b/src/app/icd/client/DefaultCheckInDelegate.cpp @@ -46,7 +46,6 @@ void DefaultCheckInDelegate::OnCheckInComplete(const ICDClientInfo & clientInfo) RefreshKeySender * DefaultCheckInDelegate::OnKeyRefreshNeeded(ICDClientInfo & clientInfo, ICDClientStorage * clientStorage) { CHIP_ERROR err = CHIP_NO_ERROR; - RefreshKeySender::RefreshKeyBuffer newKey; err = Crypto::DRBG_get_bytes(newKey.Bytes(), newKey.Capacity()); @@ -64,20 +63,21 @@ RefreshKeySender * DefaultCheckInDelegate::OnKeyRefreshNeeded(ICDClientInfo & cl return refreshKeySender; } -void DefaultCheckInDelegate::OnKeyRefreshDone(RefreshKeySender * refreshKeySender, CHIP_ERROR aError) +void DefaultCheckInDelegate::OnKeyRefreshDone(RefreshKeySender * refreshKeySender, CHIP_ERROR error) { - if (aError == CHIP_NO_ERROR) + if (error == CHIP_NO_ERROR) { ChipLogProgress(ICD, "Re-registration with new key completed successfully"); } else { - ChipLogError(ICD, "Re-registration with new key failed with error : %" CHIP_ERROR_FORMAT, aError.Format()); - // The application can take corrective action based on the error received. + ChipLogError(ICD, "Re-registration with new key failed with error : %" CHIP_ERROR_FORMAT, error.Format()); + // The callee can take corrective action based on the error received. } if (refreshKeySender != nullptr) { Platform::Delete(refreshKeySender); + refreshKeySender = nullptr; } } } // namespace app diff --git a/src/app/icd/client/DefaultCheckInDelegate.h b/src/app/icd/client/DefaultCheckInDelegate.h index 47605c3bdd17fa..e7b856677734bd 100644 --- a/src/app/icd/client/DefaultCheckInDelegate.h +++ b/src/app/icd/client/DefaultCheckInDelegate.h @@ -34,7 +34,7 @@ class DefaultCheckInDelegate : public CheckInDelegate CHIP_ERROR Init(ICDClientStorage * storage); void OnCheckInComplete(const ICDClientInfo & clientInfo) override; RefreshKeySender * OnKeyRefreshNeeded(ICDClientInfo & clientInfo, ICDClientStorage * clientStorage) override; - void OnKeyRefreshDone(RefreshKeySender * refreshKeySender, CHIP_ERROR aError) override; + void OnKeyRefreshDone(RefreshKeySender * refreshKeySender, CHIP_ERROR error) override; private: ICDClientStorage * mpStorage = nullptr; diff --git a/src/app/icd/client/RefreshKeySender.cpp b/src/app/icd/client/RefreshKeySender.cpp index 8b335d6d8b1981..45cbbd9c987a68 100644 --- a/src/app/icd/client/RefreshKeySender.cpp +++ b/src/app/icd/client/RefreshKeySender.cpp @@ -27,14 +27,14 @@ namespace chip { namespace app { -RefreshKeySender::RefreshKeySender(CheckInDelegate * apCheckInDelegate, const ICDClientInfo & aICDClientInfo, - ICDClientStorage * aICDClientStorage, const RefreshKeyBuffer & aRefreshKeyBuffer) : - mICDClientInfo(aICDClientInfo), - mpICDClientStorage(aICDClientStorage), mpCheckInDelegate(apCheckInDelegate), mOnConnectedCallback(HandleDeviceConnected, this), +RefreshKeySender::RefreshKeySender(CheckInDelegate * checkInDelegate, const ICDClientInfo & icdClientInfo, + ICDClientStorage * icdClientStorage, const RefreshKeyBuffer & refreshKeyBuffer) : + mICDClientInfo(icdClientInfo), + mpICDClientStorage(icdClientStorage), mpCheckInDelegate(checkInDelegate), mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this) { - mNewKey = aRefreshKeyBuffer; + mNewKey = refreshKeyBuffer; } CHIP_ERROR RefreshKeySender::RegisterClientWithNewKey(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) @@ -48,7 +48,6 @@ CHIP_ERROR RefreshKeySender::RegisterClientWithNewKey(Messaging::ExchangeManager mICDClientInfo.offset = 0; mpICDClientStorage->RemoveKey(mICDClientInfo); error = mpICDClientStorage->SetKey(mICDClientInfo, mNewKey.Span()); - if (error != CHIP_NO_ERROR) { ChipLogError(ICD, "Failed to set the new key after re-registration: %" CHIP_ERROR_FORMAT, error.Format()); diff --git a/src/app/icd/client/RefreshKeySender.h b/src/app/icd/client/RefreshKeySender.h index 9d22709c402fd9..9dae7f39515a85 100644 --- a/src/app/icd/client/RefreshKeySender.h +++ b/src/app/icd/client/RefreshKeySender.h @@ -43,8 +43,8 @@ class RefreshKeySender public: typedef Crypto::SensitiveDataBuffer RefreshKeyBuffer; - RefreshKeySender(CheckInDelegate * apCheckInDelegate, const ICDClientInfo & aICDClientInfo, - ICDClientStorage * aICDClientStorage, const RefreshKeyBuffer & aRefreshKeyBuffer); + RefreshKeySender(CheckInDelegate * checkInDelegate, const ICDClientInfo & icdClientInfo, ICDClientStorage * icdClientStorage, + const RefreshKeyBuffer & refreshKeyBuffer); /** * @brief Sets up a CASE session to the peer for re-registering a client with the peer when a key refresh is required to avoid @@ -77,9 +77,6 @@ class RefreshKeySender /** * @brief Used to send a re-registration command to the peer using a new key. * - * @param[in] clientInfo - ICDClientInfo object representing the state associated with the node that sent the check-in - * message. The callee can use the clientInfo to determine the type of key to generate. - * @param[in] keyData - New key to re-register the client with the server * @param[in] exchangeMgr - exchange manager to use for the re-registration * @param[in] sessionHandle - session handle to use for the re-registration */