Skip to content

Commit

Permalink
Per bzbarsky, add a helper to discard our exchange reference without …
Browse files Browse the repository at this point in the history
…closing it
  • Loading branch information
msandstedt committed Jan 7, 2022
1 parent 3d04424 commit ba75737
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 31 deletions.
45 changes: 26 additions & 19 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,19 @@ void CASESession::CloseExchange()
}
}

void CASESession::DiscardExchange()
{
if (mExchangeCtxt != nullptr)
{
// Make sure the exchange doesn't try to notify us when it closes,
// since we might be dead by then.
mExchangeCtxt->SetDelegate(nullptr);
// Null out mExchangeCtxt so that Clear() doesn't try closing it. The
// exchange will handle that.
mExchangeCtxt = nullptr;
}
}

CHIP_ERROR CASESession::ToCachable(CASESessionCachable & cachableSession)
{
const NodeId peerNodeId = GetPeerNodeId();
Expand Down Expand Up @@ -252,12 +265,9 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec)
VerifyOrReturn(mExchangeCtxt == ec, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout exchange doesn't match"));
ChipLogError(SecureChannel, "CASESession timed out while waiting for a response from the peer. Current state was %" PRIu8,
mState);
// Make sure the exchange doesn't try to notify us when it closes,
// since we might be dead by then.
mExchangeCtxt->SetDelegate(nullptr);
// Null out mExchangeCtxt so that Clear() doesn't try closing it. The
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
mExchangeCtxt = nullptr;
DiscardExchange();
Clear();
// Do this last in case the delegate frees us.
mDelegate->OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT);
Expand Down Expand Up @@ -687,11 +697,9 @@ CHIP_ERROR CASESession::HandleSigma2Resume(System::PacketBufferHandle && msg)

mCASESessionEstablished = true;

// Make sure the exchange doesn't try to notify us when it closes,
// since we might be dead by then.
mExchangeCtxt->SetDelegate(nullptr);
// Forget our exchange, as no additional messages are expected from the peer
mExchangeCtxt = nullptr;
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
DiscardExchange();

// Call delegate to indicate session establishment is successful
// Do this last in case the delegate frees us.
Expand Down Expand Up @@ -1125,9 +1133,9 @@ CHIP_ERROR CASESession::HandleSigma3(System::PacketBufferHandle && msg)

mCASESessionEstablished = true;

// Forget our exchange, as no additional messages are expected from the peer
mExchangeCtxt->SetDelegate(nullptr);
mExchangeCtxt = nullptr;
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
DiscardExchange();

// Call delegate to indicate session establishment is successful
// Do this last in case the delegate frees us.
Expand Down Expand Up @@ -1311,9 +1319,9 @@ void CASESession::OnSuccessStatusReport()
ChipLogProgress(SecureChannel, "Success status report received. Session was established");
mCASESessionEstablished = true;

// Forget our exchange, as no additional messages are expected from the peer
mExchangeCtxt->SetDelegate(nullptr);
mExchangeCtxt = nullptr;
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
DiscardExchange();

mState = kInitialized;

Expand Down Expand Up @@ -1534,10 +1542,9 @@ CHIP_ERROR CASESession::OnMessageReceived(ExchangeContext * ec, const PayloadHea
// Call delegate to indicate session establishment failure.
if (err != CHIP_NO_ERROR)
{
// Null out mExchangeCtxt so that Clear() doesn't try closing it. The
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
mExchangeCtxt->SetDelegate(nullptr);
mExchangeCtxt = nullptr;
DiscardExchange();
Clear();
// Do this last in case the delegate frees us.
mDelegate->OnSessionEstablishmentError(err);
Expand Down
6 changes: 6 additions & 0 deletions src/protocols/secure_channel/CASESession.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin

void CloseExchange();

/**
* Clear our reference to our exchange context pointer so that it can close
* itself at some later time.
*/
void DiscardExchange();

// TODO: Remove this and replace with system method to retrieve current time
CHIP_ERROR SetEffectiveTime(void);

Expand Down
35 changes: 23 additions & 12 deletions src/protocols/secure_channel/PASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,19 @@ void PASESession::CloseExchange()
}
}

void PASESession::DiscardExchange()
{
if (mExchangeCtxt != nullptr)
{
// Make sure the exchange doesn't try to notify us when it closes,
// since we might be dead by then.
mExchangeCtxt->SetDelegate(nullptr);
// Null out mExchangeCtxt so that Clear() doesn't try closing it. The
// exchange will handle that.
mExchangeCtxt = nullptr;
}
}

CHIP_ERROR PASESession::Serialize(PASESessionSerialized & output)
{
PASESessionSerializable serializable;
Expand Down Expand Up @@ -343,10 +356,9 @@ void PASESession::OnResponseTimeout(ExchangeContext * ec)
ChipLogError(SecureChannel,
"PASESession timed out while waiting for a response from the peer. Expected message type was %" PRIu8,
to_underlying(mNextExpectedMsg));
// Null out mExchangeCtxt so that Clear() doesn't try closing it. The
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
mExchangeCtxt->SetDelegate(nullptr);
mExchangeCtxt = nullptr;
DiscardExchange();
Clear();
// Do this last in case the delegate frees us.
mDelegate->OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT);
Expand Down Expand Up @@ -825,9 +837,9 @@ CHIP_ERROR PASESession::HandleMsg3(System::PacketBufferHandle && msg)

mPairingComplete = true;

// Forget our exchange, as no additional messages are expected from the peer
mExchangeCtxt->SetDelegate(nullptr);
mExchangeCtxt = nullptr;
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
DiscardExchange();

// Call delegate to indicate pairing completion
// Do this last in case the delegate frees us.
Expand All @@ -846,9 +858,9 @@ void PASESession::OnSuccessStatusReport()
{
mPairingComplete = true;

// Forget our exchange, as no additional messages are expected from the peer
mExchangeCtxt->SetDelegate(nullptr);
mExchangeCtxt = nullptr;
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
DiscardExchange();

// Call delegate to indicate pairing completion
// Do this last in case the delegate frees us.
Expand Down Expand Up @@ -942,10 +954,9 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl
// Call delegate to indicate pairing failure
if (err != CHIP_NO_ERROR)
{
// Null out mExchangeCtxt so that Clear() doesn't try closing it. The
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
mExchangeCtxt->SetDelegate(nullptr);
mExchangeCtxt = nullptr;
DiscardExchange();
Clear();
ChipLogError(SecureChannel, "Failed during PASE session setup. %s", ErrorStr(err));
// Do this last in case the delegate frees us.
Expand Down
6 changes: 6 additions & 0 deletions src/protocols/secure_channel/PASESession.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,12 @@ class DLL_EXPORT PASESession : public Messaging::ExchangeDelegate, public Pairin

void CloseExchange();

/**
* Clear our reference to our exchange context pointer so that it can close
* itself at some later time.
*/
void DiscardExchange();

SessionEstablishmentDelegate * mDelegate = nullptr;

Protocols::SecureChannel::MsgType mNextExpectedMsg = Protocols::SecureChannel::MsgType::PASE_PakeError;
Expand Down

0 comments on commit ba75737

Please sign in to comment.