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

Fix KeySetWrite command payload validation. #26726

Merged
merged 1 commit into from
May 23, 2023
Merged

Fix KeySetWrite command payload validation. #26726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 47 additions & 16 deletions src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ class GroupKeyManagementAttributeAccess : public AttributeAccessInterface
// Register for the GroupKeyManagement cluster on all endpoints.
GroupKeyManagementAttributeAccess() : AttributeAccessInterface(Optional<EndpointId>(0), GroupKeyManagement::Id) {}

// TODO: Once there is MCSP support, this may need to change.
static constexpr bool IsMCSPSupported() { return false; }

CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override
{
VerifyOrDie(aPath.mClusterId == GroupKeyManagement::Id);
Expand All @@ -117,8 +120,15 @@ class GroupKeyManagementAttributeAccess : public AttributeAccessInterface
{
case GroupKeyManagement::Attributes::ClusterRevision::Id:
return ReadClusterRevision(aPath.mEndpointId, aEncoder);
case Attributes::FeatureMap::Id:
return aEncoder.Encode(static_cast<uint32_t>(0));
case Attributes::FeatureMap::Id: {
uint32_t features = 0;
if (IsMCSPSupported())
{
// TODO: Once there is MCSP support, this will need to add the
// right feature bit.
}
return aEncoder.Encode(features);
}
case GroupKeyManagement::Attributes::GroupKeyMap::Id:
return ReadGroupKeyMap(aPath.mEndpointId, aEncoder);
case GroupKeyManagement::Attributes::GroupTable::Id:
Expand Down Expand Up @@ -296,29 +306,32 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(
chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::GroupKeyManagement::Commands::KeySetWrite::DecodableType & commandData)
{
auto provider = GetGroupDataProvider();
auto fabric = Server::GetInstance().GetFabricTable().FindFabricWithIndex(commandObj->GetAccessingFabricIndex());

if (nullptr == provider || nullptr == fabric)
if (commandData.groupKeySet.epochKey0.IsNull() || commandData.groupKeySet.epochStartTime0.IsNull() ||
commandData.groupKeySet.epochKey0.Value().empty() || (0 == commandData.groupKeySet.epochStartTime0.Value()))
{
commandObj->AddStatus(commandPath, Status::Failure);
// If the EpochKey0 field is null or its associated EpochStartTime0 field is null,
// then this command SHALL fail with an INVALID_COMMAND
commandObj->AddStatus(commandPath, Status::InvalidCommand);
return true;
}

uint8_t compressed_fabric_id_buffer[sizeof(uint64_t)];
MutableByteSpan compressed_fabric_id(compressed_fabric_id_buffer);
CHIP_ERROR err = fabric->GetCompressedFabricIdBytes(compressed_fabric_id);
if (CHIP_NO_ERROR != err)
if (commandData.groupKeySet.groupKeySecurityPolicy == GroupKeySecurityPolicyEnum::kUnknownEnumValue)
{
commandObj->AddStatus(commandPath, Status::Failure);
// If a client indicates an enumeration value to the server, that is not
// supported by the server, because it is ... a new value unrecognized
// by a legacy server, then the server SHALL generate a general
// constraint error
commandObj->AddStatus(commandPath, Status::ConstraintError);
return true;
}

if (commandData.groupKeySet.epochKey0.IsNull() || commandData.groupKeySet.epochStartTime0.IsNull() ||
commandData.groupKeySet.epochKey0.Value().empty() || (0 == commandData.groupKeySet.epochStartTime0.Value()))
if (!GroupKeyManagementAttributeAccess::IsMCSPSupported() &&
commandData.groupKeySet.groupKeySecurityPolicy == GroupKeySecurityPolicyEnum::kCacheAndSync)
{
// If the EpochKey0 field is null or its associated EpochStartTime0 field is null,
// then this command SHALL fail with an INVALID_COMMAND
// When CacheAndSync is not supported in the FeatureMap of this cluster,
// any action attempting to set CacheAndSync in the
// GroupKeySecurityPolicy field SHALL fail with an INVALID_COMMAND
// error.
commandObj->AddStatus(commandPath, Status::InvalidCommand);
return true;
}
Expand Down Expand Up @@ -366,6 +379,24 @@ bool emberAfGroupKeyManagementClusterKeySetWriteCallback(
keyset.num_keys_used++;
}

auto provider = GetGroupDataProvider();
auto fabric = Server::GetInstance().GetFabricTable().FindFabricWithIndex(commandObj->GetAccessingFabricIndex());

if (nullptr == provider || nullptr == fabric)
{
commandObj->AddStatus(commandPath, Status::Failure);
return true;
}

uint8_t compressed_fabric_id_buffer[sizeof(uint64_t)];
MutableByteSpan compressed_fabric_id(compressed_fabric_id_buffer);
CHIP_ERROR err = fabric->GetCompressedFabricIdBytes(compressed_fabric_id);
if (CHIP_NO_ERROR != err)
{
commandObj->AddStatus(commandPath, Status::Failure);
return true;
}

// Set KeySet
err = provider->SetKeySet(fabric->GetFabricIndex(), compressed_fabric_id, keyset);
if (CHIP_NO_ERROR == err)
Expand Down
91 changes: 87 additions & 4 deletions src/app/tests/suites/TestGroupKeyManagementCluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ tests:
EpochStartTime2: 1110002,
}

- label: "KeySet Write 2"
- label: "KeySet Write 2 CacheAndSync"
command: "KeySetWrite"
PICS: GRPKEY.S.F00
arguments:
values:
- name: "GroupKeySet"
Expand All @@ -108,9 +109,28 @@ tests:
EpochStartTime2: 2110002,
}

- label: "KeySet Write 3"
- label: "KeySet Write 2 TrustFirst"
command: "KeySetWrite"
PICS: "!GRPKEY.S.F00"
arguments:
values:
- name: "GroupKeySet"
value:
{
GroupKeySetID: 0x01a2,
GroupKeySecurityPolicy: 0,
EpochKey0: "\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf",
EpochStartTime0: 2110000,
EpochKey1: "\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef",
EpochStartTime1: 2110001,
EpochKey2: "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
EpochStartTime2: 2110002,
}

- label: "KeySet Write 3 CacheAndSync"
identity: "beta"
command: "KeySetWrite"
PICS: GRPKEY.S.F00
arguments:
values:
- name: "GroupKeySet"
Expand All @@ -128,6 +148,27 @@ tests:
EpochStartTime2: 2110002,
}

- label: "KeySet Write 3 TrustFirst"
identity: "beta"
command: "KeySetWrite"
PICS: "!GRPKEY.S.F00"
arguments:
values:
- name: "GroupKeySet"
value:
{
GroupKeySetID: 0x01a3,
GroupKeySecurityPolicy: 0,
EpochKey0:
"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
EpochStartTime0: 2110000,
EpochKey1: "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f",
EpochStartTime1: 2110001,
EpochKey2:
"\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f",
EpochStartTime2: 2110002,
}

- label: "KeySet Read"
command: "KeySetRead"
arguments:
Expand Down Expand Up @@ -502,8 +543,9 @@ tests:
response:
error: NOT_FOUND

- label: "KeySet Read (not removed)"
- label: "KeySet Read (not removed) CacheAndSync"
command: "KeySetRead"
PICS: GRPKEY.S.F00
arguments:
values:
- name: "GroupKeySetID"
Expand All @@ -523,6 +565,28 @@ tests:
EpochStartTime2: 2110002,
}

- label: "KeySet Read (not removed) TrustFirst"
command: "KeySetRead"
PICS: GRPKEY.S.F00
arguments:
values:
- name: "GroupKeySetID"
value: 0x01a2
response:
values:
- name: "GroupKeySet"
value:
{
GroupKeySetID: 0x01a2,
GroupKeySecurityPolicy: 0,
EpochKey0: null,
EpochStartTime0: 2110000,
EpochKey1: null,
EpochStartTime1: 2110001,
EpochKey2: null,
EpochStartTime2: 2110002,
}

- label: "Remove Group 1"
cluster: "Groups"
endpoint: 1
Expand Down Expand Up @@ -608,8 +672,9 @@ tests:
EpochStartTime2: 1110002,
}

- label: "KeySet Write 2"
- label: "KeySet Write 2 CacheAndSync"
command: "KeySetWrite"
PICS: GRPKEY.S.F00
arguments:
values:
- name: "GroupKeySet"
Expand All @@ -625,6 +690,24 @@ tests:
EpochStartTime2: 2110002,
}

- label: "KeySet Write 2 TrustFirst"
command: "KeySetWrite"
PICS: "!GRPKEY.S.F00"
arguments:
values:
- name: "GroupKeySet"
value:
{
GroupKeySetID: 0x01a2,
GroupKeySecurityPolicy: 0,
EpochKey0: "\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf",
EpochStartTime0: 2110000,
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
EpochKey1: "\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef",
EpochStartTime1: 2110001,
EpochKey2: "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
EpochStartTime2: 2110002,
}

- label: "Map Group 1 and Group 2 to KeySet 1 and group 2 to KeySet 2"
command: "writeAttribute"
attribute: "GroupKeyMap"
Expand Down
8 changes: 8 additions & 0 deletions src/app/tests/suites/certification/PICS.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3616,6 +3616,14 @@ PICS:
client?"
id: GRPKEY.C

#
# server / features
#
- label:
"Does the DUT(Server) support Group Key Management CacheAndSync
feature?"
id: GRPKEY.S.F00

#
# client / attributes
#
Expand Down
7 changes: 5 additions & 2 deletions src/app/tests/suites/certification/ci-pics-values
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,9 +580,12 @@ G.C.C03.Tx=0
G.C.C04.Tx=0
G.C.C05.Tx=0

GRPKEY.C=1
GRPKEY.S.A0001=0
GRPKEY.S=1
# No support for the CacheAndSync feature so far
GRPKEY.S.F00=0
GRPKEY.S.A0001=0

GRPKEY.C=1
GRPKEY.C.A0000=1
GRPKEY.C.A0001=0
GRPKEY.C.C00.Tx=1
Expand Down
Loading