From c7f2c43e5cc5aa0208ccbd1c00290e468a960b32 Mon Sep 17 00:00:00 2001 From: Marc Lepage Date: Wed, 19 Jan 2022 12:39:45 -0500 Subject: [PATCH 1/2] Place PASESession on new fabric PASE sessions start with no fabric, but during commissioning, after OperationalCredentialsCluster::AddNOC, they should be placed on the newly commissioned fabric so administrative actions pertaining to the newly commissioned fabric can be performed over the PASE session, if desired. Work towards issue #10242 --- .../operational-credentials-server.cpp | 3 +++ src/transport/SecureSession.cpp | 6 +++--- src/transport/SecureSession.h | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index b95982e6d2a729..664d254665fcb8 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -450,6 +450,9 @@ bool emberAfOperationalCredentialsClusterAddNOCCallback(app::CommandHandler * co err = Server::GetInstance().GetFabricTable().Store(fabricIndex); VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err)); + // Notify the secure session of the new fabric. + commandObj->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->NewFabric(commandObj->GetAccessingFabricIndex()); + // We might have a new operational identity, so we should start advertising it right away. app::DnssdServer::Instance().AdvertiseOperational(); diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp index 3a9be04d8912e6..09b2afeb5972a8 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -32,9 +32,9 @@ Access::SubjectDescriptor SecureSession::GetSubjectDescriptor() const } else if (IsPAKEKeyId(mPeerNodeId)) { - subjectDescriptor.authMode = Access::AuthMode::kPase; - subjectDescriptor.subject = mPeerNodeId; - // TODO(#10242): PASE *can* have fabric in some situations + subjectDescriptor.authMode = Access::AuthMode::kPase; + subjectDescriptor.subject = mPeerNodeId; + subjectDescriptor.fabricIndex = mPaseFabric; } else { diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 05efecc190ae32..a43cf3479bb26f 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -114,6 +114,18 @@ class SecureSession : public Session uint16_t GetPeerSessionId() const { return mPeerSessionId; } FabricIndex GetFabricIndex() const { return mFabric; } + // Should only be called for PASE sessions, which start with undefined fabric, + // to place on a newly commissioned fabric after successful + // OperationalCredentialsCluster::AddNOC + CHIP_ERROR NewFabric(FabricIndex fabricIndex) + { + // TODO: should check that secure session type is PASE and current value is undefined + // (i.e. that it's called exactly once in proper circumstances) + // but that's difficult until issue #13711 is addressed + mPaseFabric = fabricIndex; + return CHIP_NO_ERROR; + } + System::Clock::Timestamp GetLastActivityTime() const { return mLastActivityTime; } void MarkActive() { mLastActivityTime = System::SystemClock().GetMonotonicTimestamp(); } @@ -141,6 +153,10 @@ class SecureSession : public Session const uint16_t mPeerSessionId; const FabricIndex mFabric; + // PASE sessions start with undefined fabric, but can be placed on a newly + // commissioned fabric after successful OperationalCredentialsCluster::AddNOC + FabricIndex mPaseFabric = kUndefinedFabricIndex; + PeerAddress mPeerAddress; System::Clock::Timestamp mLastActivityTime; ReliableMessageProtocolConfig mMRPConfig; From a89403e282abd201bb06e30ffbaca634f9632f32 Mon Sep 17 00:00:00 2001 From: Marc Lepage Date: Wed, 19 Jan 2022 16:45:39 -0500 Subject: [PATCH 2/2] Change SecureSession::NewFabric to alter mFabric This makes it easier to also have it be the accessing fabric after the change. --- .../operational-credentials-server.cpp | 2 +- src/transport/SecureSession.cpp | 2 +- src/transport/SecureSession.h | 20 +++++++++++-------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index 664d254665fcb8..d0132cee86ba34 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -451,7 +451,7 @@ bool emberAfOperationalCredentialsClusterAddNOCCallback(app::CommandHandler * co VerifyOrExit(err == CHIP_NO_ERROR, nocResponse = ConvertToNOCResponseStatus(err)); // Notify the secure session of the new fabric. - commandObj->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->NewFabric(commandObj->GetAccessingFabricIndex()); + commandObj->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->NewFabric(fabricIndex); // We might have a new operational identity, so we should start advertising it right away. app::DnssdServer::Instance().AdvertiseOperational(); diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp index 09b2afeb5972a8..67e4378ba4e0e0 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -34,7 +34,7 @@ Access::SubjectDescriptor SecureSession::GetSubjectDescriptor() const { subjectDescriptor.authMode = Access::AuthMode::kPase; subjectDescriptor.subject = mPeerNodeId; - subjectDescriptor.fabricIndex = mPaseFabric; + subjectDescriptor.fabricIndex = mFabric; } else { diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index a43cf3479bb26f..5edcac726f96d2 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -115,14 +115,19 @@ class SecureSession : public Session FabricIndex GetFabricIndex() const { return mFabric; } // Should only be called for PASE sessions, which start with undefined fabric, - // to place on a newly commissioned fabric after successful + // to migrate to a newly commissioned fabric after successful // OperationalCredentialsCluster::AddNOC CHIP_ERROR NewFabric(FabricIndex fabricIndex) { - // TODO: should check that secure session type is PASE and current value is undefined - // (i.e. that it's called exactly once in proper circumstances) - // but that's difficult until issue #13711 is addressed - mPaseFabric = fabricIndex; +#if 0 + // TODO(#13711): this check won't work until the issue is addressed + if (mSecureSessionType == Type::kPASE) + { + mFabric = fabricIndex; + } +#else + mFabric = fabricIndex; +#endif return CHIP_NO_ERROR; } @@ -151,11 +156,10 @@ class SecureSession : public Session const CATValues mPeerCATs; const uint16_t mLocalSessionId; const uint16_t mPeerSessionId; - const FabricIndex mFabric; - // PASE sessions start with undefined fabric, but can be placed on a newly + // PASE sessions start with undefined fabric, but are migrated to a newly // commissioned fabric after successful OperationalCredentialsCluster::AddNOC - FabricIndex mPaseFabric = kUndefinedFabricIndex; + FabricIndex mFabric; PeerAddress mPeerAddress; System::Clock::Timestamp mLastActivityTime;