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.

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

Restyle Make FabricIndex be a common thing across all sessions. #14905

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions examples/chip-tool/commands/common/CommandInvoker.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,7 @@ CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, void * aContext,
//
// We assume the aDevice already has a Case session which is way we can use he established Secure Session
ReturnErrorOnFailure(invoker->InvokeGroupCommand(aDevice->GetExchangeManager(),
aDevice->GetSecureSession().Value()->AsSecureSession()->GetFabricIndex(),
groupId, aRequestData));
aDevice->GetSecureSession().Value()->GetFabricIndex(), groupId, aRequestData));

// invoker is already deleted and is not to be used
invoker.release();
Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ FabricIndex CommandHandler::GetAccessingFabricIndex() const
}
else
{
fabric = mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
fabric = mpExchangeCtx->GetSessionHandle()->GetFabricIndex();
}

return fabric;
Expand Down
2 changes: 1 addition & 1 deletion src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ CHIP_ERROR InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeConte
ChipLogProgress(InteractionModel,
"Deleting previous subscription from NodeId: " ChipLogFormatX64 ", FabricIndex: %" PRIu8,
ChipLogValueX64(apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId()),
apExchangeContext->GetSessionHandle()->AsSecureSession()->GetFabricIndex());
apExchangeContext->GetSessionHandle()->GetFabricIndex());
mReadHandlers.ReleaseObject(handler);
}

Expand Down
4 changes: 2 additions & 2 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)));

mPeerNodeId = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetPeerNodeId();
mFabricIndex = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetFabricIndex();
mFabricIndex = aReadPrepareParams.mSessionHolder->GetFabricIndex();

MoveToState(ClientState::AwaitingInitialReport);

Expand Down Expand Up @@ -801,7 +801,7 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)));

mPeerNodeId = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetPeerNodeId();
mFabricIndex = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetFabricIndex();
mFabricIndex = aReadPrepareParams.mSessionHolder->GetFabricIndex();

MoveToState(ClientState::AwaitingInitialReport);

Expand Down
2 changes: 1 addition & 1 deletion src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ bool ReadHandler::IsFromSubscriber(Messaging::ExchangeContext & apExchangeContex
{
return (IsType(InteractionType::Subscribe) &&
GetInitiatorNodeId() == apExchangeContext.GetSessionHandle()->AsSecureSession()->GetPeerNodeId() &&
GetAccessingFabricIndex() == apExchangeContext.GetSessionHandle()->AsSecureSession()->GetFabricIndex());
GetAccessingFabricIndex() == apExchangeContext.GetSessionHandle()->GetFabricIndex());
}

CHIP_ERROR ReadHandler::OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
Expand Down
2 changes: 1 addition & 1 deletion src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ FabricIndex WriteHandler::GetAccessingFabricIndex() const
}
else
{
fabric = mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
fabric = mpExchangeCtx->GetSessionHandle()->GetFabricIndex();
}

return fabric;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
* Once bindings are implemented, this may no longer be needed.
*/
SessionHandle handle = commandObj->GetExchangeContext()->GetSessionHandle();
server->SetFabricIndex(handle->AsSecureSession()->GetFabricIndex());
server->SetFabricIndex(handle->GetFabricIndex());
server->SetPeerNodeId(handle->AsSecureSession()->GetPeerNodeId());

CheckSuccess(server->CommissioningComplete(), Failure);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ class FabricCleanupExchangeDelegate : public chip::Messaging::ExchangeDelegate
void OnResponseTimeout(chip::Messaging::ExchangeContext * ec) override {}
void OnExchangeClosing(chip::Messaging::ExchangeContext * ec) override
{
FabricIndex currentFabricIndex = ec->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
FabricIndex currentFabricIndex = ec->GetSessionHandle()->GetFabricIndex();
ec->GetExchangeMgr()->GetSessionManager()->ExpireAllPairingsForFabric(currentFabricIndex);
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPCluster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ CHIP_ERROR ClusterBase::AssociateWithGroup(DeviceProxy * device, GroupId groupId
{
// Local copy to preserve original SessionHandle for future Unicast communication.
Optional<SessionHandle> session = mDevice->GetExchangeManager()->GetSessionManager()->CreateGroupSession(
groupId, mDevice->GetSecureSession().Value()->AsSecureSession()->GetFabricIndex());
groupId, mDevice->GetSecureSession().Value()->GetFabricIndex());
// Sanity check
if (!session.HasValue() || !session.Value()->IsGroupSession())
{
Expand Down
4 changes: 1 addition & 3 deletions src/transport/GroupSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace Transport {
class GroupSession : public Session
{
public:
GroupSession(GroupId group, FabricIndex fabricIndex) : mGroupId(group), mFabricIndex(fabricIndex) {}
GroupSession(GroupId group, FabricIndex fabricIndex) : mGroupId(group) { SetFabricIndex(fabricIndex); }
~GroupSession() { NotifySessionReleased(); }

Session::SessionType GetSessionType() const override { return Session::SessionType::kGroup; }
Expand Down Expand Up @@ -59,11 +59,9 @@ class GroupSession : public Session
}

GroupId GetGroupId() const { return mGroupId; }
FabricIndex GetFabricIndex() const { return mFabricIndex; }

private:
const GroupId mGroupId;
const FabricIndex mFabricIndex;
};

/*
Expand Down
4 changes: 2 additions & 2 deletions src/transport/SecureSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ Access::SubjectDescriptor SecureSession::GetSubjectDescriptor() const
subjectDescriptor.authMode = Access::AuthMode::kCase;
subjectDescriptor.subject = mPeerNodeId;
subjectDescriptor.cats = mPeerCATs;
subjectDescriptor.fabricIndex = mFabric;
subjectDescriptor.fabricIndex = GetFabricIndex();
}
else if (IsPAKEKeyId(mPeerNodeId))
{
subjectDescriptor.authMode = Access::AuthMode::kPase;
subjectDescriptor.subject = mPeerNodeId;
subjectDescriptor.fabricIndex = mFabric;
subjectDescriptor.fabricIndex = GetFabricIndex();
}
else
{
Expand Down
15 changes: 6 additions & 9 deletions src/transport/SecureSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ class SecureSession : public Session
FabricIndex fabric, const ReliableMessageProtocolConfig & config) :
mSecureSessionType(secureSessionType),
mPeerNodeId(peerNodeId), mPeerCATs(peerCATs), mLocalSessionId(localSessionId), mPeerSessionId(peerSessionId),
mFabric(fabric), mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()), mMRPConfig(config)
{}
mLastActivityTime(System::SystemClock().GetMonotonicTimestamp()), mMRPConfig(config)
{
SetFabricIndex(fabric);
}
~SecureSession() { NotifySessionReleased(); }

SecureSession(SecureSession &&) = delete;
Expand Down Expand Up @@ -112,7 +114,6 @@ class SecureSession : public Session

uint16_t GetLocalSessionId() const { return mLocalSessionId; }
uint16_t GetPeerSessionId() const { return mPeerSessionId; }
FabricIndex GetFabricIndex() const { return mFabric; }

// Should only be called for PASE sessions, which start with undefined fabric,
// to migrate to a newly commissioned fabric after successful
Expand All @@ -123,10 +124,10 @@ class SecureSession : public Session
// TODO(#13711): this check won't work until the issue is addressed
if (mSecureSessionType == Type::kPASE)
{
mFabric = fabricIndex;
SetFabricIndex(fabricIndex);
}
#else
mFabric = fabricIndex;
SetFabricIndex(fabricIndex);
#endif
return CHIP_NO_ERROR;
}
Expand All @@ -145,10 +146,6 @@ class SecureSession : public Session
const uint16_t mLocalSessionId;
const uint16_t mPeerSessionId;

// PASE sessions start with undefined fabric, but are migrated to a newly
// commissioned fabric after successful OperationalCredentialsCluster::AddNOC
FabricIndex mFabric;

PeerAddress mPeerAddress;
System::Clock::Timestamp mLastActivityTime;
ReliableMessageProtocolConfig mMRPConfig;
Expand Down
10 changes: 10 additions & 0 deletions src/transport/Session.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#pragma once

#include <credentials/FabricTable.h>
#include <lib/core/CHIPConfig.h>
#include <messaging/ReliableMessageProtocolConfig.h>
#include <transport/SessionHolder.h>
Expand All @@ -33,6 +34,10 @@ class Session
public:
virtual ~Session() {}

static constexpr FabricIndex kInvalidFabricIndex = 0;

static_assert(kInvalidFabricIndex < chip::kMinValidFabricIndex, "Invalid fabric index must be invalid");

enum class SessionType : uint8_t
{
kUndefined = 0,
Expand Down Expand Up @@ -67,6 +72,8 @@ class Session
virtual const ReliableMessageProtocolConfig & GetMRPConfig() const = 0;
virtual System::Clock::Milliseconds32 GetAckTimeout() const = 0;

FabricIndex GetFabricIndex() const { return mFabricIndex; }

SecureSession * AsSecureSession();
UnauthenticatedSession * AsUnauthenticatedSession();
GroupSession * AsGroupSession();
Expand All @@ -85,8 +92,11 @@ class Session
}
}

void SetFabricIndex(FabricIndex index) { mFabricIndex = index; }

private:
IntrusiveList<SessionHolder> mHolders;
FabricIndex mFabricIndex = kInvalidFabricIndex;
};

} // namespace Transport
Expand Down