Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
thivya-amazon authored and yunhanw-google committed Feb 3, 2024
1 parent 1eab9a5 commit 7866f6e
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 19 deletions.
4 changes: 2 additions & 2 deletions src/app/icd/client/CheckInDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions src/app/icd/client/DefaultCheckInDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/app/icd/client/DefaultCheckInDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 5 additions & 6 deletions src/app/icd/client/RefreshKeySender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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());
Expand Down
7 changes: 2 additions & 5 deletions src/app/icd/client/RefreshKeySender.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ class RefreshKeySender
public:
typedef Crypto::SensitiveDataBuffer<Crypto::kAES_CCM128_Key_Length> 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
Expand Down Expand Up @@ -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
*/
Expand Down

0 comments on commit 7866f6e

Please sign in to comment.