Skip to content

Commit dfc402f

Browse files
tcarmelveilleuxrestyled-commits
tcarmelveilleux
and
restyled-commits
authored
Implement VID Verification in OpCreds cluster (2/2) (project-chip#38469)
* Implement VID Verification in OpCreds cluster (2/2) - Add VVS/VVSC storage in PersistentStorageOpCertStore - Finished all attributes and commands in Opcreds cluster - Bumped Opcreds revision to 2 - Added all necessary testability features to matter_testing support for list subscriptions - NOTE: this includes project-chip#38445 temporarily until merged Testing done: - Integration tests through TC-OPCREDS-3.9 - More unit tests to follow, but coverage through TC-OPCREDS-3.9 is near exhaustive. * Restyled by clang-format * Restyled by autopep8 * Restyled by isort * Fix TC-CGEN-2.9 - Fabric table was incorrectly read, which could lead to issues when the fabric table has more fields than before. * Address review comments from cecille@ * Restyled by clang-format * Renamed TC-OPCREDS-3.9 to TC-OPCREDS-3.8 * Added comments to AttributeMatcher * Renamed test in TC_OPCREDS_3_8.py * Restyled by autopep8 * More logging in TC-CGEN-2.9 * Restyled by autopep8 * Restyled by isort * Add a verification against some node ID to diagnose CI * Restyled by clang-format * Fix hermeticity of TC-CGEN scripts in CI - The scripts did not factory reset properly so they were impacted by lack of hermeticity, causing a failure of CGEN-2.9 after unrelated master changes. - The method to find "commissioner's fabric" before some fabric removal was wrong in CGEN-2.9. Fixed the method to use CurrentFabricIndex * Add logging * Restyled by autopep8 * Revert "Restyled by clang-format" This reverts commit 66dee3d. * Revert "Add a verification against some node ID to diagnose CI" This reverts commit b188e36. * Use fix-cgen-2.9 * Update cgen --------- Co-authored-by: Restyled.io <[email protected]>
1 parent dfa96cb commit dfc402f

File tree

13 files changed

+1345
-403
lines changed

13 files changed

+1345
-403
lines changed

‎src/app/chip_data_model.gni‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,9 @@ template("chip_data_model") {
493493
"${_app_root}/clusters/${cluster}/${cluster}.cpp",
494494
"${_app_root}/clusters/${cluster}/${cluster}.h",
495495
]
496+
} else if (cluster == "operational-credentials-server") {
497+
sources += [ "${_app_root}/clusters/operational-credentials-server/operational-credentials-server.cpp" ]
498+
deps += [ "${chip_root}/zzz_generated/app-common/clusters/OperationalCredentials:metadata" ]
496499
} else {
497500
sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
498501
}

‎src/app/clusters/operational-credentials-server/operational-credentials-server.cpp‎

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include <clusters/OperationalCredentials/Attributes.h>
3737
#include <clusters/OperationalCredentials/Commands.h>
3838
#include <clusters/OperationalCredentials/Events.h>
39+
#include <clusters/OperationalCredentials/Metadata.h>
3940
#include <clusters/OperationalCredentials/Structs.h>
4041
#include <credentials/CHIPCert.h>
4142
#include <credentials/CertificationDeclaration.h>
@@ -63,13 +64,13 @@ using namespace chip::Protocols::InteractionModel;
6364

6465
namespace {
6566

67+
constexpr auto kDACCertificate = CertificateChainTypeEnum::kDACCertificate;
68+
constexpr auto kPAICertificate = CertificateChainTypeEnum::kPAICertificate;
69+
6670
void SendNOCResponse(app::CommandHandler * commandObj, const ConcreteCommandPath & path, NodeOperationalCertStatusEnum status,
6771
uint8_t index, const CharSpan & debug_text);
6872
NodeOperationalCertStatusEnum ConvertToNOCResponseStatus(CHIP_ERROR err);
6973

70-
constexpr auto kDACCertificate = CertificateChainTypeEnum::kDACCertificate;
71-
constexpr auto kPAICertificate = CertificateChainTypeEnum::kPAICertificate;
72-
7374
CHIP_ERROR CreateAccessControlEntryForNewFabricAdministrator(const Access::SubjectDescriptor & subjectDescriptor,
7475
FabricIndex fabricIndex, uint64_t subject)
7576
{
@@ -247,10 +248,11 @@ OperationalCredentialsAttrAccess gAttrAccess;
247248

248249
CHIP_ERROR OperationalCredentialsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
249250
{
250-
VerifyOrDie(aPath.mClusterId == Clusters::OperationalCredentials::Id);
251-
252251
switch (aPath.mAttributeId)
253252
{
253+
case Attributes::ClusterRevision::Id: {
254+
return aEncoder.Encode(kRevision);
255+
}
254256
case Attributes::NOCs::Id: {
255257
return ReadNOCs(aPath.mEndpointId, aEncoder);
256258
}
@@ -1232,8 +1234,59 @@ bool emberAfOperationalCredentialsClusterSetVIDVerificationStatementCallback(
12321234
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
12331235
const Commands::SetVIDVerificationStatement::DecodableType & commandData)
12341236
{
1235-
(void) commandData;
1236-
commandObj->AddStatus(commandPath, Status::UnsupportedCommand);
1237+
1238+
FabricIndex fabricIndex = commandObj->GetAccessingFabricIndex();
1239+
ChipLogProgress(Zcl, "OpCreds: Received a SetVIDVerificationStatement Command for FabricIndex 0x%x",
1240+
static_cast<unsigned>(fabricIndex));
1241+
1242+
if (!commandData.vendorID.HasValue() && !commandData.VIDVerificationStatement.HasValue() && !commandData.vvsc.HasValue())
1243+
{
1244+
commandObj->AddStatus(commandPath, Status::InvalidCommand);
1245+
return true;
1246+
}
1247+
1248+
if (commandData.vendorID.HasValue() && !IsVendorIdValidOperationally(commandData.vendorID.Value()))
1249+
{
1250+
commandObj->AddStatus(commandPath, Status::ConstraintError);
1251+
return true;
1252+
}
1253+
1254+
auto & fabricTable = Server::GetInstance().GetFabricTable();
1255+
1256+
bool fabricChangesOccurred = false;
1257+
CHIP_ERROR err = fabricTable.SetVIDVerificationStatementElements(
1258+
fabricIndex, commandData.vendorID, commandData.VIDVerificationStatement, commandData.vvsc, fabricChangesOccurred);
1259+
if (err == CHIP_ERROR_INVALID_ARGUMENT)
1260+
{
1261+
commandObj->AddStatus(commandPath, Status::ConstraintError);
1262+
}
1263+
else if (err == CHIP_ERROR_INCORRECT_STATE)
1264+
{
1265+
commandObj->AddStatus(commandPath, Status::InvalidCommand);
1266+
}
1267+
else if (err != CHIP_NO_ERROR)
1268+
{
1269+
// We have no idea what happened; just report failure.
1270+
StatusIB status(err);
1271+
commandObj->AddStatus(commandPath, status.mStatus);
1272+
}
1273+
else
1274+
{
1275+
commandObj->AddStatus(commandPath, Status::Success);
1276+
}
1277+
1278+
// Handle dirty-marking if anything changed. Only `Fabrics` attribute is reported since `NOCs`
1279+
// is not reportable (`C` quality).
1280+
if (fabricChangesOccurred)
1281+
{
1282+
MatterReportingAttributeChangeCallback(commandPath.mEndpointId, OperationalCredentials::Id,
1283+
OperationalCredentials::Attributes::Fabrics::Id);
1284+
}
1285+
1286+
if (err != CHIP_NO_ERROR)
1287+
{
1288+
ChipLogError(Zcl, "SetVIDVerificationStatement failed: %" CHIP_ERROR_FORMAT, err.Format());
1289+
}
12371290
return true;
12381291
}
12391292

‎src/app/tests/suites/TestOperationalCredentialsCluster.yaml‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,21 @@ tests:
2828
- name: "nodeId"
2929
value: nodeId
3030

31+
- label: "ClusterRevision must be the latest"
32+
command: "readAttribute"
33+
attribute: "ClusterRevision"
34+
response:
35+
constraints:
36+
type: int16u
37+
value: 2
38+
3139
- label: "Read number of supported fabrics"
3240
command: "readAttribute"
3341
attribute: "SupportedFabrics"
3442
response:
3543
constraints:
3644
type: int8u
37-
minValue: 4
45+
minValue: 5
3846

3947
- label: "Read number of commissioned fabrics"
4048
command: "readAttribute"
@@ -130,13 +138,3 @@ tests:
130138
values:
131139
- name: "FabricIndex"
132140
value: 1
133-
134-
# Not yet implemented!
135-
- label: "Check SetVIDVerificationStatement not implemented"
136-
command: "SetVIDVerificationStatement"
137-
arguments:
138-
values:
139-
- name: "VendorID"
140-
value: 0xfff1
141-
response:
142-
error: UNSUPPORTED_COMMAND

‎src/controller/python/chip/ChipDeviceCtrl.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,12 +1059,12 @@ def GetCompressedFabricId(self):
10591059

10601060
return fabricid.value
10611061

1062-
def GetFabricIdInternal(self):
1062+
def GetFabricIdInternal(self) -> int:
10631063
'''
10641064
Get the fabric ID from the object. Only used to validate cached value from property.
10651065
10661066
Returns:
1067-
int: The compressed fabric ID as a 64-bit integer.
1067+
int: The raw fabric ID as a 64-bit integer.
10681068
10691069
Raises:
10701070
ChipStackError: On failure.
@@ -1080,12 +1080,12 @@ def GetFabricIdInternal(self):
10801080

10811081
return fabricid.value
10821082

1083-
def GetFabricIndexInternal(self):
1083+
def GetFabricIndexInternal(self) -> int:
10841084
'''
10851085
Get the fabric index from the object. Only used to validate cached value from property.
10861086
10871087
Returns:
1088-
int: The compressed fabric ID as a 64-bit integer.
1088+
int: fabric index in local fabric table associated with this controller.
10891089
10901090
Raises:
10911091
ChipStackError: On failure.

‎src/credentials/FabricTable.cpp‎

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <lib/support/SafeInt.h>
3030
#include <lib/support/ScopedBuffer.h>
3131
#include <lib/support/TypeTraits.h>
32+
#include <lib/support/logging/CHIPLogging.h>
3233
#include <platform/LockTracker.h>
3334
#include <tracing/macros.h>
3435

@@ -2180,22 +2181,16 @@ CHIP_ERROR FabricTable::SetShouldAdvertiseIdentity(FabricIndex fabricIndex, Adve
21802181
CHIP_ERROR FabricTable::FetchVIDVerificationStatement(FabricIndex fabricIndex, MutableByteSpan & outVIDVerificationStatement) const
21812182
{
21822183
VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
2183-
VerifyOrReturnError(outVIDVerificationStatement.size() >= kVendorIdVerificationStatementV1Size, CHIP_ERROR_BUFFER_TOO_SMALL);
2184-
2185-
// TODO(#38308): Add VIDVerificationStatement loading support. Empty for now since setting is still
2186-
// to be done and the result will be correct with more of the actual code running.
2187-
outVIDVerificationStatement.reduce_size(0);
2188-
return CHIP_NO_ERROR;
2184+
return mOpCertStore->GetVidVerificationElement(
2185+
fabricIndex, OperationalCertificateStore::VidVerificationElement::kVidVerificationStatement, outVIDVerificationStatement);
21892186
}
21902187

21912188
CHIP_ERROR FabricTable::FetchVVSC(FabricIndex fabricIndex, MutableByteSpan & outVVSC) const
21922189
{
21932190
VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
2194-
VerifyOrReturnError(outVVSC.size() >= kMaxCHIPCertLength, CHIP_ERROR_BUFFER_TOO_SMALL);
21952191

2196-
// TODO(#38308): Add VVSC loading support. Empty for now since setting is still
2197-
// to be done and the result will be correct with more of the actual code running.
2198-
outVVSC.reduce_size(0);
2192+
return mOpCertStore->GetVidVerificationElement(fabricIndex, OperationalCertificateStore::VidVerificationElement::kVvsc,
2193+
outVVSC);
21992194
return CHIP_NO_ERROR;
22002195
}
22012196

@@ -2246,4 +2241,66 @@ CHIP_ERROR FabricTable::SignVIDVerificationRequest(FabricIndex fabricIndex, Byte
22462241
return CHIP_NO_ERROR;
22472242
}
22482243

2244+
CHIP_ERROR FabricTable::SetVIDVerificationStatementElements(FabricIndex fabricIndex, Optional<uint16_t> vendorId,
2245+
Optional<ByteSpan> VIDVerificationStatement, Optional<ByteSpan> VVSC,
2246+
bool & outFabricTableWasChanged)
2247+
{
2248+
VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INTERNAL);
2249+
VerifyOrReturnError(IsValidFabricIndex(fabricIndex), CHIP_ERROR_INVALID_ARGUMENT);
2250+
2251+
FabricInfo * fabricInfo = GetMutableFabricByIndex(fabricIndex);
2252+
VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
2253+
2254+
// PendingNew fabric means AddNOC had been called. ShadowPending fabric is the overlay version
2255+
// that is used when UpdateNOC had been called. This is to detect either condition of not
2256+
// fully committed fabric.
2257+
bool isTargetFabricPending = (GetPendingNewFabricIndex() == fabricIndex) ||
2258+
((GetShadowPendingFabricEntry() != nullptr) && (GetShadowPendingFabricEntry()->GetFabricIndex() == fabricIndex));
2259+
2260+
outFabricTableWasChanged = false;
2261+
2262+
// Start with VVSC first as it's the most likely to fail.
2263+
if (VVSC.HasValue())
2264+
{
2265+
ReturnErrorOnFailure(mOpCertStore->UpdateVidVerificationSignerCertForFabric(fabricIndex, VVSC.Value()));
2266+
}
2267+
2268+
if (VIDVerificationStatement.HasValue())
2269+
{
2270+
bool wasVvsEqual = false;
2271+
{
2272+
// This is in a scope to save stack space from getting too deep.
2273+
uint8_t vidVerificationStatementBuffer[Crypto::kVendorIdVerificationStatementV1Size];
2274+
MutableByteSpan vidVerificationStatementSpan{ vidVerificationStatementBuffer };
2275+
ReturnErrorOnFailure(mOpCertStore->GetVidVerificationElement(
2276+
fabricIndex, OperationalCertificateStore::VidVerificationElement::kVidVerificationStatement,
2277+
vidVerificationStatementSpan));
2278+
wasVvsEqual = vidVerificationStatementSpan.data_equal(VIDVerificationStatement.Value());
2279+
}
2280+
2281+
if (!wasVvsEqual)
2282+
{
2283+
ReturnErrorOnFailure(
2284+
mOpCertStore->UpdateVidVerificationStatementForFabric(fabricIndex, VIDVerificationStatement.Value()));
2285+
outFabricTableWasChanged = true;
2286+
}
2287+
}
2288+
2289+
if (vendorId.HasValue())
2290+
{
2291+
if (static_cast<uint16_t>(fabricInfo->GetVendorId()) != vendorId.Value())
2292+
{
2293+
fabricInfo->SetVendorId(static_cast<VendorId>(vendorId.Value()));
2294+
// Immediately commit Vendor ID if not a pending fabric. Otherwise the commit occurs on CommissioningComplete.
2295+
if (!isTargetFabricPending)
2296+
{
2297+
ReturnErrorOnFailure(StoreFabricMetadata(fabricInfo));
2298+
outFabricTableWasChanged = true;
2299+
}
2300+
}
2301+
}
2302+
2303+
return CHIP_NO_ERROR;
2304+
}
2305+
22492306
} // namespace chip

‎src/credentials/FabricTable.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ class DLL_EXPORT FabricInfo
104104

105105
CHIP_ERROR FetchRootPubkey(Crypto::P256PublicKey & outPublicKey) const;
106106

107+
void SetVendorId(VendorId vendorId) { mVendorId = vendorId; }
107108
VendorId GetVendorId() const { return mVendorId; }
108109

109110
bool IsInitialized() const { return (mFabricIndex != kUndefinedFabricIndex) && IsOperationalNodeId(mNodeId); }
@@ -1098,6 +1099,25 @@ class DLL_EXPORT FabricTable
10981099
CHIP_ERROR SignVIDVerificationRequest(FabricIndex fabricIndex, ByteSpan clientChallenge, ByteSpan attestationChallenge,
10991100
SignVIDVerificationResponseData & outResponse);
11001101

1102+
/**
1103+
* @brief Handle setting data related to Fabric Table VID Verification.
1104+
*
1105+
* This is on purpose structured to mirror the SetVIDVerificationStatement Operational Credentials Cluster command
1106+
*
1107+
* @param[in] fabricIndex - Fabric Index for which to produce the response.
1108+
* @param[in] vendorID - New VendorID to set on the Fabric (ignored if missing)
1109+
* @param[in] VIDVerificationStatement - VID Verification Statement to add/remove (ignored if missing)
1110+
* @param[in] VVSC - VID Verification Signing Certificate to add/remove (ignored if missing)
1111+
* @param[out] outFabricTableWasChanged - This is set to true if FabricTable saw a change from prior value, even if
1112+
* method returns an error (appies to VIDVerificationStatement and VendorID)
1113+
* @retval CHIP_NO_ERROR on success
1114+
* @retval CHIP_ERROR_INVALID_ARGUMENT if vendorID, VVSC or VIDVerificationStatement are not correct (maps to CONSTRAINT_ERROR)
1115+
* @retval CHIP_ERROR_INCORRECT_STATE if VVSC cannot be set due to ICAC presence (maps to INVALID_COMMAND)
1116+
*/
1117+
CHIP_ERROR SetVIDVerificationStatementElements(FabricIndex fabricIndex, Optional<uint16_t> vendorId,
1118+
Optional<ByteSpan> VIDVerificationStatement, Optional<ByteSpan> VVSC,
1119+
bool & outFabricTableWasChanged);
1120+
11011121
private:
11021122
enum class StateFlags : uint16_t
11031123
{

0 commit comments

Comments
 (0)