Skip to content

Commit

Permalink
Small fixes in access control event logging (#15658)
Browse files Browse the repository at this point in the history 
Noticed these issues while inspecting the event logging code.

- Subjects now need to be transcoded via a convert function
- PASE subject should go into adminPasscodeID field

Verified that writing group subjects 4444, 5555, and 6666
properly got converted to node IDs 0xFFFFFFFFFFFF115C,
0xFFFFFFFFFFFF15B3, and 0xFFFFFFFFFFFF1A0A, and reading
them properly converted them back.
  • Loading branch information
@mlepage-google @pull
mlepage-google authored and pull[bot] committed Oct 25, 2023
1 parent 5da8a0a commit f3a898a
Showing 1 changed file with 24 additions and 19 deletions.
43 changes: 24 additions & 19 deletions src/app/clusters/access-control-server/access-control-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,11 @@ struct AccessControlEntryCodec
{
for (size_t i = 0; i < subjectCount; ++i)
{
Subject subject;
ReturnErrorOnFailure(entry.GetSubject(i, subject.nodeId));
ReturnErrorOnFailure(Convert(subject.nodeId, subject));
subjectBuffer[i] = subject.nodeId;
NodeId subject;
ReturnErrorOnFailure(entry.GetSubject(i, subject));
Subject tmp;
ReturnErrorOnFailure(AccessControlEntryCodec::Convert(subject, tmp));
subjectBuffer[i] = tmp.nodeId;
}
staging.subjects.SetNonNull(subjectBuffer, subjectCount);
}
Expand Down Expand Up @@ -302,9 +303,10 @@ struct AccessControlEntryCodec
auto iterator = staging.subjects.Value().begin();
while (iterator.Next())
{
Subject subject = { .nodeId = iterator.GetValue(), .authMode = staging.authMode };
ReturnErrorOnFailure(Convert(subject, subject.nodeId));
ReturnErrorOnFailure(entry.AddSubject(nullptr, subject.nodeId));
Subject tmp = { .nodeId = iterator.GetValue(), .authMode = staging.authMode };
NodeId subject;
ReturnErrorOnFailure(Convert(tmp, subject));
ReturnErrorOnFailure(entry.AddSubject(nullptr, subject));
}
ReturnErrorOnFailure(iterator.GetStatus());
}
Expand Down Expand Up @@ -358,8 +360,8 @@ class AccessControlAttribute : public chip::app::AttributeAccessInterface

constexpr uint16_t AccessControlAttribute::ClusterRevision;

CHIP_ERROR LogAccessControlEvent(const AccessControl::Entry & entry, const Access::SubjectDescriptor & subjectDescriptor,
AccessControlCluster::ChangeTypeEnum changeType)
CHIP_ERROR LogEntryChangedEvent(const AccessControl::Entry & entry, const Access::SubjectDescriptor & subjectDescriptor,
AccessControlCluster::ChangeTypeEnum changeType)
{
CHIP_ERROR err;

Expand Down Expand Up @@ -393,7 +395,11 @@ CHIP_ERROR LogAccessControlEvent(const AccessControl::Entry & entry, const Acces
{
for (size_t i = 0; i < subjectCount; ++i)
{
ReturnErrorOnFailure(entry.GetSubject(i, subjectBuffer[i]));
NodeId subject;
ReturnErrorOnFailure(entry.GetSubject(i, subject));
Subject tmp;
ReturnErrorOnFailure(AccessControlEntryCodec::Convert(subject, tmp));
subjectBuffer[i] = tmp.nodeId;
}
staging.subjects.SetNonNull(subjectBuffer, subjectCount);
}
Expand All @@ -420,17 +426,16 @@ CHIP_ERROR LogAccessControlEvent(const AccessControl::Entry & entry, const Acces
}
else if (subjectDescriptor.authMode == Access::AuthMode::kPase)
{
adminNodeID.SetNonNull(PAKEKeyIdFromNodeId(subjectDescriptor.subject));
adminPasscodeID.SetNonNull(PAKEKeyIdFromNodeId(subjectDescriptor.subject));
}

AccessControlCluster::Events::AccessControlEntryChanged::Type event{ subjectDescriptor.fabricIndex, adminNodeID,
adminPasscodeID, changeType, latestValue };

// AccessControl event only occurs on endpoint 0.
err = LogEvent(event, 0, eventNumber);
if (CHIP_NO_ERROR != err)
{
ChipLogError(DataManagement, "AccessControl: Failed to record AccessControlEntryChanged event");
ChipLogError(DataManagement, "AccessControlCluster: log event failed");
}

return err;
Expand Down Expand Up @@ -524,14 +529,14 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aP
if (i < oldCount)
{
ReturnErrorOnFailure(GetAccessControl().UpdateEntry(i, iterator.GetValue().entry, &accessingFabricIndex));
ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
AccessControlCluster::ChangeTypeEnum::kChanged));
ReturnErrorOnFailure(LogEntryChangedEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
AccessControlCluster::ChangeTypeEnum::kChanged));
}
else
{
ReturnErrorOnFailure(GetAccessControl().CreateEntry(nullptr, iterator.GetValue().entry, &accessingFabricIndex));
ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
AccessControlCluster::ChangeTypeEnum::kAdded));
ReturnErrorOnFailure(LogEntryChangedEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
AccessControlCluster::ChangeTypeEnum::kAdded));
}
++i;
}
Expand All @@ -544,7 +549,7 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aP
--oldCount;
ReturnErrorOnFailure(GetAccessControl().ReadEntry(oldCount, entry, &accessingFabricIndex));
ReturnErrorOnFailure(
LogAccessControlEvent(entry, aDecoder.GetSubjectDescriptor(), AccessControlCluster::ChangeTypeEnum::kRemoved));
LogEntryChangedEvent(entry, aDecoder.GetSubjectDescriptor(), AccessControlCluster::ChangeTypeEnum::kRemoved));
ReturnErrorOnFailure(GetAccessControl().DeleteEntry(oldCount, &accessingFabricIndex));
}
}
Expand All @@ -555,7 +560,7 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aP

ReturnErrorOnFailure(GetAccessControl().CreateEntry(nullptr, item.entry, &accessingFabricIndex));
ReturnErrorOnFailure(
LogAccessControlEvent(item.entry, aDecoder.GetSubjectDescriptor(), AccessControlCluster::ChangeTypeEnum::kAdded));
LogEntryChangedEvent(item.entry, aDecoder.GetSubjectDescriptor(), AccessControlCluster::ChangeTypeEnum::kAdded));
}
else
{
Expand Down

0 comments on commit f3a898a

Please sign in to comment.