Properly manage operational key lifecycle for fail-safe

examples/chip-tool/commands/common/CHIPCommand.cpp

@@ -71,10 +71,12 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
 #endif
 
  ReturnLogErrorOnFailure(mDefaultStorage.Init());
+ ReturnLogErrorOnFailure(mOperationalKeystore.Init(&mDefaultStorage));
 
  chip::Controller::FactoryInitParams factoryInitParams;
 
  factoryInitParams.fabricIndependentStorage = &mDefaultStorage;
+ factoryInitParams.operationalKeystore = &mOperationalKeystore;
 
  // Init group data provider that will be used for all group keys and IPKs for the
  // chip-tool-configured fabrics. This is OK to do once since the fabric tables

examples/chip-tool/commands/common/CHIPCommand.h

@@ -23,6 +23,7 @@
 #include <commands/common/CredentialIssuerCommands.h>
 #include <commands/example/ExampleCredentialIssuerCommands.h>
 #include <credentials/GroupDataProviderImpl.h>
+#include <crypto/PersistentStorageOperationalKeystore.h>
 
 #pragma once
 
@@ -115,6 +116,8 @@ class CHIPCommand : public Command
 
  PersistentStorage mDefaultStorage;
  PersistentStorage mCommissionerStorage;
+ chip::PersistentStorageOperationalKeystore mOperationalKeystore;
+
  chip::Credentials::GroupDataProviderImpl mGroupDataProvider{ kMaxGroupsPerFabric, kMaxGroupKeysPerFabric };
  CredentialIssuerCommands * mCredIssuerCmds;
 

src/app/CASEClient.cpp

@@ -30,6 +30,8 @@ CHIP_ERROR CASEClient::EstablishSession(PeerId peer, const Transport::PeerAddres
  const ReliableMessageProtocolConfig & remoteMRPConfig,
  SessionEstablishmentDelegate * delegate)
 {
+ VerifyOrReturnError(mInitParams.fabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+
  // Create a UnauthenticatedSession for CASE pairing.
  Optional<SessionHandle> session = mInitParams.sessionManager->CreateUnauthenticatedSession(peerAddress, remoteMRPConfig);
  VerifyOrReturnError(session.HasValue(), CHIP_ERROR_NO_MEMORY);
@@ -45,7 +47,7 @@ CHIP_ERROR CASEClient::EstablishSession(PeerId peer, const Transport::PeerAddres
 
  mCASESession.SetGroupDataProvider(mInitParams.groupDataProvider);
  ReturnErrorOnFailure(mCASESession.EstablishSession(
- *mInitParams.sessionManager, mInitParams.fabricTable, mInitParams.fabricIndex, peer.GetNodeId(), exchange,
+ *mInitParams.sessionManager, mInitParams.fabricTable, ScopedNodeId{ peer.GetNodeId(), mInitParams.fabricIndex }, exchange,
  mInitParams.sessionResumptionStorage, mInitParams.certificateValidityPolicy, delegate, mInitParams.mrpLocalConfig));
 
  return CHIP_NO_ERROR;

src/app/OperationalDeviceProxy.h

@@ -199,7 +199,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,
  }
 
  /**
- * @brief Get the raw Fabric ID assigned to the device.
+ * @brief Get the fabricIndex
  */
  FabricIndex GetFabricIndex() const { return mFabricIndex; }
 

src/app/clusters/general-commissioning-server/general-commissioning-server.cpp

@@ -155,6 +155,9 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler *
  FailSafeContext & failSafeContext = DeviceLayer::DeviceControlServer::DeviceControlSvr().GetFailSafeContext();
  Commands::ArmFailSafeResponse::Type response;
 
+ ChipLogProgress(FailSafe, "GeneralCommissioning: Received ArmFailSafe (%us)",
+ static_cast<unsigned>(commandData.expiryLengthSeconds));
+
  /*
  * If the fail-safe timer is not fully disarmed, don't allow arming a new fail-safe.
  * If the fail-safe timer was not currently armed, then the fail-safe timer SHALL be armed.
@@ -214,7 +217,10 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
  MATTER_TRACE_EVENT_SCOPE("CommissioningComplete", "GeneralCommissioning");
 
  DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();
- const auto & failSafe = server->GetFailSafeContext();
+ auto & failSafe = server->GetFailSafeContext();
+ auto & fabricTable = Server::GetInstance().GetFabricTable();
+
+ ChipLogProgress(FailSafe, "GeneralCommissioning: Received CommissioningComplete");
 
  Commands::CommissioningCompleteResponse::Type response;
  if (!failSafe.IsFailSafeArmed())
@@ -231,16 +237,34 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
  !failSafe.MatchesFabricIndex(commandObj->GetAccessingFabricIndex()))
  {
  response.errorCode = CommissioningError::kInvalidAuthentication;
+ ChipLogError(FailSafe, "GeneralCommissioning: Got commissioning complete in invalid security context");
  }
  else
  {
+ if (failSafe.NocCommandHasBeenInvoked())
+ {
+ CHIP_ERROR err = fabricTable.CommitPendingFabricData();
+ if (err != CHIP_NO_ERROR)
+ {
+ ChipLogError(FailSafe, "GeneralCommissioning: Failed to commit pending fabric data: %" CHIP_ERROR_FORMAT,
+ err.Format());
+ }
+ else
+ {
+ ChipLogProgress(FailSafe, "GeneralCommissioning: Successfully commited pending fabric data");
+ }
+ CheckSuccess(err, Failure);
+ }
+
  /*
  * Pass fabric of commissioner to DeviceControlSvr.
  * This allows device to send messages back to commissioner.
  * Once bindings are implemented, this may no longer be needed.
  */
- CheckSuccess(server->CommissioningComplete(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex()),
- Failure);
+ failSafe.DisarmFailSafe();
+ CheckSuccess(
+ server->PostCommissioningCompleteEvent(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex()),
+ Failure);
 
  Breadcrumb::Set(commandPath.mEndpointId, 0);
  response.errorCode = CommissioningError::kOk;

src/app/clusters/network-commissioning/network-commissioning.cpp

@@ -486,7 +486,7 @@ void Instance::OnResult(Status commissioningError, CharSpan debugText, int32_t i
  }
  if (commissioningError == Status::kSuccess)
  {
- DeviceLayer::DeviceControlServer::DeviceControlSvr().ConnectNetworkForOperational(
+ DeviceLayer::DeviceControlServer::DeviceControlSvr().PostConnectedToOperationalNetworkEvent(
  ByteSpan(mLastNetworkID, mLastNetworkIDLen));
  mLastConnectErrorValue.SetNull();
  }