Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ICD] Fix multiple check-in/peer node ids handling in icd client #35304

Merged
merged 3 commits into from
Aug 30, 2024
Merged

[ICD] Fix multiple check-in/peer node ids handling in icd client #35304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3124b60
Fix multiple check-in/peer nodeId handling in icd client side
yunhanw-google Aug 29, 2024
83b38e5
Restyled by clang-format
restyled-commits Aug 30, 2024
ecf9faa
address comment
yunhanw-google Aug 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions examples/chip-tool/commands/clusters/ClusterCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class ClusterCommand : public InteractionModelCommands, public ModelCommand, pub
const chip::app::Clusters::IcdManagement::Commands::UnregisterClient::Type & value)
{
ReturnErrorOnFailure(InteractionModelCommands::SendCommand(device, endpointId, clusterId, commandId, value));
mScopedNodeId = chip::ScopedNodeId(value.checkInNodeID, device->GetSecureSession().Value()->GetFabricIndex());
mPeerNodeId = chip::ScopedNodeId(device->GetDeviceId(), device->GetSecureSession().Value()->GetFabricIndex());
return CHIP_NO_ERROR;
}

Expand All @@ -69,7 +69,8 @@ class ClusterCommand : public InteractionModelCommands, public ModelCommand, pub
const chip::app::Clusters::IcdManagement::Commands::RegisterClient::Type & value)
{
ReturnErrorOnFailure(InteractionModelCommands::SendCommand(device, endpointId, clusterId, commandId, value));
mScopedNodeId = chip::ScopedNodeId(value.checkInNodeID, device->GetSecureSession().Value()->GetFabricIndex());
mPeerNodeId = chip::ScopedNodeId(device->GetDeviceId(), device->GetSecureSession().Value()->GetFabricIndex());
mCheckInNodeId = chip::ScopedNodeId(value.checkInNodeID, device->GetSecureSession().Value()->GetFabricIndex());
mMonitoredSubject = value.monitoredSubject;
mClientType = value.clientType;
memcpy(mICDSymmetricKey, value.key.data(), value.key.size());
Expand Down Expand Up @@ -147,7 +148,9 @@ class ClusterCommand : public InteractionModelCommands, public ModelCommand, pub
return;
}
chip::app::ICDClientInfo clientInfo;
clientInfo.peer_node = mScopedNodeId;

clientInfo.peer_node = mPeerNodeId;
clientInfo.check_in_node = mCheckInNodeId;
clientInfo.monitored_subject = mMonitoredSubject;
clientInfo.start_icd_counter = value.ICDCounter;
clientInfo.client_type = mClientType;
Expand All @@ -159,7 +162,7 @@ class ClusterCommand : public InteractionModelCommands, public ModelCommand, pub
if ((path.mEndpointId == chip::kRootEndpointId) && (path.mClusterId == chip::app::Clusters::IcdManagement::Id) &&
(path.mCommandId == chip::app::Clusters::IcdManagement::Commands::UnregisterClient::Id))
{
ClearICDEntry(mScopedNodeId);
ClearICDEntry(mPeerNodeId);
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -260,9 +263,21 @@ class ClusterCommand : public InteractionModelCommands, public ModelCommand, pub
private:
chip::ClusterId mClusterId;
chip::CommandId mCommandId;
chip::ScopedNodeId mScopedNodeId;
uint64_t mMonitoredSubject = static_cast<uint64_t>(0);
// The scoped node ID to which RegisterClient and UnregisterClient command will be sent. Not set for other commands.
chip::ScopedNodeId mPeerNodeId;
// The scoped node ID to which a Check-In message will be sent. Only set for the RegisterClient command.
chip::ScopedNodeId mCheckInNodeId;

// Used to determine if a particular client has an active subscription for the given entry.
// The MonitoredSubject, when it is a NodeID, MAY be the same as the CheckInNodeID.
// The MonitoredSubject gives the registering client the flexibility of having a different
// CheckInNodeID from the MonitoredSubject.
uint64_t mMonitoredSubject = static_cast<uint64_t>(0);

// Client type of the client registering
chip::app::Clusters::IcdManagement::ClientTypeEnum mClientType = chip::app::Clusters::IcdManagement::ClientTypeEnum::kPermanent;

// Shared secret between the client and the ICD to encrypt the Check-In message.
uint8_t mICDSymmetricKey[chip::Crypto::kAES_CCM128_Key_Length];

CHIP_ERROR mError = CHIP_NO_ERROR;
Expand Down
9 changes: 6 additions & 3 deletions examples/chip-tool/commands/icd/ICDCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,17 @@ CHIP_ERROR ICDListCommand::RunCommand()
fprintf(stderr, " +------------------------------------------------------------------------------------------+\n");
fprintf(stderr, " | %-88s |\n", "Known ICDs:");
fprintf(stderr, " +------------------------------------------------------------------------------------------+\n");
fprintf(stderr, " | %20s | %15s | %15s | %16s | %10s |\n", "Fabric Index:Node ID", "Start Counter", "Counter Offset",
"MonitoredSubject", "ClientType");
fprintf(stderr, " | %20s | %20s | %15s | %15s | %16s | %10s |\n", "Fabric Index:Peer Node ID", "Fabric Index:CheckIn Node ID",
"Start Counter", "Counter Offset", "MonitoredSubject", "ClientType");

while (iter->Next(info))
{
fprintf(stderr, " +------------------------------------------------------------------------------------------+\n");
fprintf(stderr, " | %3" PRIu32 ":" ChipLogFormatX64 " | %15" PRIu32 " | %15" PRIu32 " | " ChipLogFormatX64 " | %10u |\n",
fprintf(stderr,
" | %3" PRIu32 ":" ChipLogFormatX64 " | %3" PRIu32 ":" ChipLogFormatX64 " | %15" PRIu32 " | %15" PRIu32
" | " ChipLogFormatX64 " | %10u |\n",
static_cast<uint32_t>(info.peer_node.GetFabricIndex()), ChipLogValueX64(info.peer_node.GetNodeId()),
static_cast<uint32_t>(info.check_in_node.GetFabricIndex()), ChipLogValueX64(info.check_in_node.GetNodeId()),
info.start_icd_counter, info.offset, ChipLogValueX64(info.monitored_subject),
static_cast<uint8_t>(info.client_type));

Expand Down
3 changes: 2 additions & 1 deletion examples/chip-tool/commands/pairing/PairingCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,8 @@ void PairingCommand::OnICDRegistrationComplete(ScopedNodeId nodeId, uint32_t icd
sizeof(icdSymmetricKeyHex), chip::Encoding::HexFlags::kNullTerminate);

app::ICDClientInfo clientInfo;
clientInfo.peer_node = chip::ScopedNodeId(mICDCheckInNodeId.Value(), nodeId.GetFabricIndex());
clientInfo.check_in_node = chip::ScopedNodeId(mICDCheckInNodeId.Value(), nodeId.GetFabricIndex());
clientInfo.peer_node = nodeId;
clientInfo.monitored_subject = mICDMonitoredSubject.Value();
clientInfo.start_icd_counter = icdCounter;

Expand Down
9 changes: 8 additions & 1 deletion src/app/icd/client/DefaultICDClientStorage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,16 +235,22 @@ CHIP_ERROR DefaultICDClientStorage::Load(FabricIndex fabricIndex, std::vector<IC
ICDClientInfo clientInfo;
TLV::TLVType ICDClientInfoType;
NodeId nodeId;
NodeId checkInNodeId;
FabricIndex fabric;
ReturnErrorOnFailure(reader.EnterContainer(ICDClientInfoType));
// Peer Node ID
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kPeerNodeId)));
ReturnErrorOnFailure(reader.Get(nodeId));

ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kCheckInNodeId)));
ReturnErrorOnFailure(reader.Get(checkInNodeId));
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved

// Fabric Index
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kFabricIndex)));
ReturnErrorOnFailure(reader.Get(fabric));
clientInfo.peer_node = ScopedNodeId(nodeId, fabric);

clientInfo.peer_node = ScopedNodeId(nodeId, fabric);
clientInfo.check_in_node = ScopedNodeId(checkInNodeId, fabric);

// Start ICD Counter
ReturnErrorOnFailure(reader.Next(TLV::ContextTag(ClientInfoTag::kStartICDCounter)));
Expand Down Expand Up @@ -323,6 +329,7 @@ CHIP_ERROR DefaultICDClientStorage::SerializeToTlv(TLV::TLVWriter & writer, cons
TLV::TLVType ICDClientInfoContainerType;
ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, ICDClientInfoContainerType));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kPeerNodeId), clientInfo.peer_node.GetNodeId()));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kCheckInNodeId), clientInfo.check_in_node.GetNodeId()));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kFabricIndex), clientInfo.peer_node.GetFabricIndex()));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kStartICDCounter), clientInfo.start_icd_counter));
ReturnErrorOnFailure(writer.Put(TLV::ContextTag(ClientInfoTag::kOffset), clientInfo.offset));
Expand Down
20 changes: 11 additions & 9 deletions src/app/icd/client/DefaultICDClientStorage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,14 @@ class DefaultICDClientStorage : public ICDClientStorage
enum class ClientInfoTag : uint8_t
{
kPeerNodeId = 1,
kFabricIndex = 2,
kStartICDCounter = 3,
kOffset = 4,
kMonitoredSubject = 5,
kAesKeyHandle = 6,
kHmacKeyHandle = 7,
kClientType = 8,
kCheckInNodeId = 2,
kFabricIndex = 3,
kStartICDCounter = 4,
kOffset = 5,
kMonitoredSubject = 6,
kAesKeyHandle = 7,
kHmacKeyHandle = 8,
kClientType = 9,
};

enum class CounterTag : uint8_t
Expand Down Expand Up @@ -158,8 +159,9 @@ class DefaultICDClientStorage : public ICDClientStorage
{
// All the fields added together
return TLV::EstimateStructOverhead(
sizeof(NodeId), sizeof(FabricIndex), sizeof(uint32_t) /*start_icd_counter*/, sizeof(uint32_t) /*offset*/,
sizeof(uint64_t) /*monitored_subject*/, sizeof(Crypto::Symmetric128BitsKeyByteArray) /*aes_key_handle*/,
sizeof(NodeId), sizeof(NodeId), sizeof(FabricIndex), sizeof(uint32_t) /*start_icd_counter*/,
sizeof(uint32_t) /*offset*/, sizeof(uint64_t) /*monitored_subject*/,
sizeof(Crypto::Symmetric128BitsKeyByteArray) /*aes_key_handle*/,
sizeof(Crypto::Symmetric128BitsKeyByteArray) /*hmac_key_handle*/, sizeof(uint8_t) /*client_type*/);
}

Expand Down
2 changes: 2 additions & 0 deletions src/app/icd/client/ICDClientInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace app {
struct ICDClientInfo
{
ScopedNodeId peer_node;
ScopedNodeId check_in_node;
uint32_t start_icd_counter = 0;
uint32_t offset = 0;
Clusters::IcdManagement::ClientTypeEnum client_type = Clusters::IcdManagement::ClientTypeEnum::kPermanent;
Expand All @@ -44,6 +45,7 @@ struct ICDClientInfo
ICDClientInfo & operator=(const ICDClientInfo & other)
{
peer_node = other.peer_node;
check_in_node = other.check_in_node;
start_icd_counter = other.start_icd_counter;
offset = other.offset;
client_type = other.client_type;
Expand Down
2 changes: 1 addition & 1 deletion src/app/icd/client/RefreshKeySender.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ CHIP_ERROR RefreshKeySender::RegisterClientWithNewKey(Messaging::ExchangeManager
EndpointId endpointId = 0;

Clusters::IcdManagement::Commands::RegisterClient::Type registerClientCommand;
registerClientCommand.checkInNodeID = mICDClientInfo.peer_node.GetNodeId();
registerClientCommand.checkInNodeID = mICDClientInfo.check_in_node.GetNodeId();
registerClientCommand.monitoredSubject = mICDClientInfo.monitored_subject;
registerClientCommand.key = mNewKey.Span();
return Controller::InvokeCommandRequest(&exchangeMgr, sessionHandle, endpointId, registerClientCommand, onSuccess, onFailure);
Expand Down
31 changes: 18 additions & 13 deletions src/controller/java/AndroidCheckInDelegate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
#include <lib/support/JniReferences.h>
#include <lib/support/logging/CHIPLogging.h>

#define PARSE_CLIENT_INFO(_clientInfo, _peerNodeId, _startCounter, _offset, _monitoredSubject, _jniICDAesKey, _jniICDHmacKey) \
#define PARSE_CLIENT_INFO(_clientInfo, _peerNodeId, _checkInNodeId, _startCounter, _offset, _monitoredSubject, _jniICDAesKey, \
_jniICDHmacKey) \
jlong _peerNodeId = static_cast<jlong>(_clientInfo.peer_node.GetNodeId()); \
jlong _checkInNodeId = static_cast<jlong>(_clientInfo.check_in_node.GetNodeId()); \
jlong _startCounter = static_cast<jlong>(_clientInfo.start_icd_counter); \
jlong _offset = static_cast<jlong>(_clientInfo.offset); \
jlong _monitoredSubject = static_cast<jlong>(_clientInfo.monitored_subject); \
Expand Down Expand Up @@ -53,24 +55,26 @@ CHIP_ERROR AndroidCheckInDelegate::SetDelegate(jobject checkInDelegateObj)

void AndroidCheckInDelegate::OnCheckInComplete(const ICDClientInfo & clientInfo)
{
ChipLogProgress(
ICD, "Check In Message processing complete: start_counter=%" PRIu32 " offset=%" PRIu32 " nodeid=" ChipLogFormatScopedNodeId,
clientInfo.start_icd_counter, clientInfo.offset, ChipLogValueScopedNodeId(clientInfo.peer_node));
ChipLogProgress(ICD,
"Check In Message processing complete: start_counter=%" PRIu32 " offset=%" PRIu32
" peernodeid=" ChipLogFormatScopedNodeId " checkinnodeid=" ChipLogFormatScopedNodeId,
clientInfo.start_icd_counter, clientInfo.offset, ChipLogValueScopedNodeId(clientInfo.peer_node),
ChipLogValueScopedNodeId(clientInfo.check_in_node));

VerifyOrReturn(mCheckInDelegate.HasValidObjectRef(), ChipLogProgress(ICD, "check-in delegate is not implemented!"));

JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread();
VerifyOrReturn(env != nullptr, ChipLogError(Controller, "JNIEnv is null!"));
PARSE_CLIENT_INFO(clientInfo, peerNodeId, startCounter, offset, monitoredSubject, jniICDAesKey, jniICDHmacKey)
PARSE_CLIENT_INFO(clientInfo, peerNodeId, checkInNodeId, startCounter, offset, monitoredSubject, jniICDAesKey, jniICDHmacKey)

jmethodID onCheckInCompleteMethodID = nullptr;
CHIP_ERROR err = chip::JniReferences::GetInstance().FindMethod(env, mCheckInDelegate.ObjectRef(), "onCheckInComplete",
"(JJJJ[B[B)V", &onCheckInCompleteMethodID);
"(JJJJJ[B[B)V", &onCheckInCompleteMethodID);
VerifyOrReturn(err == CHIP_NO_ERROR,
ChipLogProgress(ICD, "onCheckInComplete - FindMethod is failed! : %" CHIP_ERROR_FORMAT, err.Format()));

env->CallVoidMethod(mCheckInDelegate.ObjectRef(), onCheckInCompleteMethodID, peerNodeId, startCounter, offset, monitoredSubject,
jniICDAesKey.jniValue(), jniICDHmacKey.jniValue());
env->CallVoidMethod(mCheckInDelegate.ObjectRef(), onCheckInCompleteMethodID, peerNodeId, checkInNodeId, startCounter, offset,
monitoredSubject, jniICDAesKey.jniValue(), jniICDHmacKey.jniValue());
}

RefreshKeySender * AndroidCheckInDelegate::OnKeyRefreshNeeded(ICDClientInfo & clientInfo, ICDClientStorage * clientStorage)
Expand All @@ -84,17 +88,18 @@ RefreshKeySender * AndroidCheckInDelegate::OnKeyRefreshNeeded(ICDClientInfo & cl
JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread();
VerifyOrReturnValue(env != nullptr, nullptr, ChipLogError(Controller, "JNIEnv is null!"));

PARSE_CLIENT_INFO(clientInfo, peerNodeId, startCounter, offset, monitoredSubject, jniICDAesKey, jniICDHmacKey)
PARSE_CLIENT_INFO(clientInfo, peerNodeId, checkInNodeId, startCounter, offset, monitoredSubject, jniICDAesKey,
jniICDHmacKey)

jmethodID onKeyRefreshNeededMethodID = nullptr;
err = chip::JniReferences::GetInstance().FindMethod(env, mCheckInDelegate.ObjectRef(), "onKeyRefreshNeeded", "(JJJJ[B[B)V",
err = chip::JniReferences::GetInstance().FindMethod(env, mCheckInDelegate.ObjectRef(), "onKeyRefreshNeeded", "(JJJJJ[B[B)V",
&onKeyRefreshNeededMethodID);
VerifyOrReturnValue(err == CHIP_NO_ERROR, nullptr,
ChipLogProgress(ICD, "onKeyRefreshNeeded - FindMethod is failed! : %" CHIP_ERROR_FORMAT, err.Format()));

jbyteArray key = static_cast<jbyteArray>(env->CallObjectMethod(mCheckInDelegate.ObjectRef(), onKeyRefreshNeededMethodID,
peerNodeId, startCounter, offset, monitoredSubject,
jniICDAesKey.jniValue(), jniICDHmacKey.jniValue()));
jbyteArray key = static_cast<jbyteArray>(
env->CallObjectMethod(mCheckInDelegate.ObjectRef(), onKeyRefreshNeededMethodID, peerNodeId, checkInNodeId, startCounter,
offset, monitoredSubject, jniICDAesKey.jniValue(), jniICDHmacKey.jniValue()));

if (key != nullptr)
{
Expand Down
8 changes: 6 additions & 2 deletions src/controller/java/AndroidDeviceControllerWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,8 @@ void AndroidDeviceControllerWrapper::OnICDRegistrationComplete(chip::ScopedNodeI
CHIP_ERROR err = CHIP_NO_ERROR;
chip::app::ICDClientInfo clientInfo;
clientInfo.peer_node = icdNodeId;
clientInfo.check_in_node = chip::ScopedNodeId(mAutoCommissioner.GetCommissioningParameters().GetICDCheckInNodeId().Value(),
icdNodeId.GetFabricIndex());
clientInfo.monitored_subject = mAutoCommissioner.GetCommissioningParameters().GetICDMonitoredSubject().Value();
clientInfo.start_icd_counter = icdCounter;

Expand Down Expand Up @@ -1056,7 +1058,7 @@ void AndroidDeviceControllerWrapper::OnICDRegistrationComplete(chip::ScopedNodeI
methodErr = chip::JniReferences::GetInstance().GetLocalClassRef(env, "chip/devicecontroller/ICDDeviceInfo", icdDeviceInfoClass);
VerifyOrReturn(methodErr == CHIP_NO_ERROR, ChipLogError(Controller, "Could not find class ICDDeviceInfo"));

icdDeviceInfoStructCtor = env->GetMethodID(icdDeviceInfoClass, "<init>", "([BILjava/lang/String;JJIJJJJI)V");
icdDeviceInfoStructCtor = env->GetMethodID(icdDeviceInfoClass, "<init>", "([BILjava/lang/String;JJIJJJJJI)V");
VerifyOrReturn(icdDeviceInfoStructCtor != nullptr, ChipLogError(Controller, "Could not find ICDDeviceInfo constructor"));

methodErr =
Expand All @@ -1069,7 +1071,9 @@ void AndroidDeviceControllerWrapper::OnICDRegistrationComplete(chip::ScopedNodeI
icdDeviceInfoObj = env->NewObject(
icdDeviceInfoClass, icdDeviceInfoStructCtor, jSymmetricKey, static_cast<jint>(mUserActiveModeTriggerHint.Raw()),
jUserActiveModeTriggerInstruction, static_cast<jlong>(mIdleModeDuration), static_cast<jlong>(mActiveModeDuration),
static_cast<jint>(mActiveModeThreshold), static_cast<jlong>(icdNodeId.GetNodeId()), static_cast<jlong>(icdCounter),
static_cast<jint>(mActiveModeThreshold), static_cast<jlong>(icdNodeId.GetNodeId()),
static_cast<jlong>(mAutoCommissioner.GetCommissioningParameters().GetICDCheckInNodeId().Value()),
static_cast<jlong>(icdCounter),
static_cast<jlong>(mAutoCommissioner.GetCommissioningParameters().GetICDMonitoredSubject().Value()),
static_cast<jlong>(Controller()->GetFabricId()), static_cast<jint>(Controller()->GetFabricIndex()));

Expand Down
Loading
Loading