Skip to content

Commit 693454e

Browse files
kghostpull[bot]
authored andcommitted
Handle OnSessionReleased in PairingSessions (#17881)
1 parent 91d0529 commit 693454e

File tree

7 files changed

+31
-9
lines changed

7 files changed

+31
-9
lines changed

src/protocols/secure_channel/CASESession.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,13 @@ CASESession::~CASESession()
127127
Clear();
128128
}
129129

130+
void CASESession::OnSessionReleased()
131+
{
132+
Clear();
133+
// Do this last in case the delegate frees us.
134+
mDelegate->OnSessionEstablishmentError(CHIP_ERROR_CONNECTION_ABORTED);
135+
}
136+
130137
void CASESession::Clear()
131138
{
132139
// This function zeroes out and resets the memory used by the object.

src/protocols/secure_channel/CASESession.h

+3
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
154154
void OnResponseTimeout(Messaging::ExchangeContext * ec) override;
155155
Messaging::ExchangeMessageDispatch & GetMessageDispatch() override { return SessionEstablishmentExchangeDispatch::Instance(); }
156156

157+
//// SessionReleaseDelegate ////
158+
void OnSessionReleased() override;
159+
157160
FabricIndex GetFabricIndex() const { return mFabricInfo != nullptr ? mFabricInfo->GetFabricIndex() : kUndefinedFabricIndex; }
158161

159162
// TODO: remove Clear, we should create a new instance instead reset the old instance.

src/protocols/secure_channel/PASESession.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ PASESession::~PASESession()
6969
Clear();
7070
}
7171

72+
void PASESession::OnSessionReleased()
73+
{
74+
Clear();
75+
// Do this last in case the delegate frees us.
76+
mDelegate->OnSessionEstablishmentError(CHIP_ERROR_CONNECTION_ABORTED);
77+
}
78+
7279
void PASESession::Finish()
7380
{
7481
mPairingComplete = true;

src/protocols/secure_channel/PASESession.h

+3
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ class DLL_EXPORT PASESession : public Messaging::UnsolicitedMessageHandler,
178178

179179
Messaging::ExchangeMessageDispatch & GetMessageDispatch() override { return SessionEstablishmentExchangeDispatch::Instance(); }
180180

181+
//// SessionReleaseDelegate ////
182+
void OnSessionReleased() override;
183+
181184
private:
182185
enum Spake2pErrorType : uint8_t
183186
{

src/protocols/secure_channel/PairingSession.cpp

+6-7
Original file line numberDiff line numberDiff line change
@@ -153,21 +153,20 @@ void PairingSession::Clear()
153153
mExchangeCtxt = nullptr;
154154
}
155155

156-
if (mSessionManager != nullptr)
156+
if (mSecureSessionHolder)
157157
{
158-
if (mSecureSessionHolder && !mSecureSessionHolder->AsSecureSession()->IsActiveSession())
158+
SessionHandle session = mSecureSessionHolder.Get();
159+
// Call Release before ExpirePairing because we don't want to receive OnSessionReleased() event here
160+
mSecureSessionHolder.Release();
161+
if (!session->AsSecureSession()->IsActiveSession() && mSessionManager != nullptr)
159162
{
160163
// Make sure to clean up our pending session, since we're the only
161164
// ones who have access to it do do so.
162-
mSessionManager->ExpirePairing(mSecureSessionHolder.Get());
165+
mSessionManager->ExpirePairing(session);
163166
}
164167
}
165168

166169
mPeerSessionId.ClearValue();
167-
// If we called ExpirePairing above, the holder has already released the
168-
// session (due to it being destroyed). If not, we need to release it.
169-
// Release is idempotent, so it's OK to just call it here.
170-
mSecureSessionHolder.Release();
171170
mSessionManager = nullptr;
172171
}
173172

src/protocols/secure_channel/PairingSession.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,10 @@ namespace chip {
3838

3939
class SessionManager;
4040

41-
class DLL_EXPORT PairingSession
41+
class DLL_EXPORT PairingSession : public SessionReleaseDelegate
4242
{
4343
public:
44+
PairingSession() : mSecureSessionHolder(*this) {}
4445
virtual ~PairingSession() { Clear(); }
4546

4647
virtual Transport::SecureSession::Type GetSecureSessionType() const = 0;
@@ -170,7 +171,7 @@ class DLL_EXPORT PairingSession
170171

171172
protected:
172173
CryptoContext::SessionRole mRole;
173-
SessionHolder mSecureSessionHolder;
174+
SessionHolderWithDelegate mSecureSessionHolder;
174175
// mSessionManager is set if we actually allocate a secure session, so we
175176
// can clean it up later as needed.
176177
SessionManager * mSessionManager = nullptr;

src/protocols/secure_channel/tests/TestPairingSession.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ class TestPairingSession : public PairingSession
4444
ScopedNodeId GetLocalScopedNodeId() const override { return ScopedNodeId(); }
4545
CATValues GetPeerCATs() const override { return CATValues(); };
4646

47+
void OnSessionReleased() override {}
48+
4749
const ReliableMessageProtocolConfig & GetRemoteMRPConfig() const { return mRemoteMRPConfig; }
4850

4951
CHIP_ERROR DeriveSecureSession(CryptoContext & session) const override { return CHIP_NO_ERROR; }

0 commit comments

Comments
 (0)