Skip to content

Commit

Permalink
Add smarts based on the protocol associated to the exchange
Browse files Browse the repository at this point in the history
  • Loading branch information
mkardous-silabs committed Oct 3, 2023
1 parent 5ea3ca3 commit 1677310
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/app/clusters/ota-requestor/DefaultOTARequestor.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class DefaultOTARequestor : public OTARequestorInterface, public BDXDownloader::
// For a receiver using BDX Protocol, all received messages will require a response except for a StatusReport
if (!payloadHeader.HasMessageType(chip::Protocols::SecureChannel::MsgType::StatusReport))
{
ec->WillSendMessage();
ec->WillSendMessage(true /* IsPeerActive */);
}

return CHIP_NO_ERROR;
Expand Down
11 changes: 7 additions & 4 deletions src/messaging/ExchangeContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
// is implicitly that response.
SetResponseExpected(false);

// If we received the expected response, we don't if the peer is still active after having sent the message
// If we received the expected response, we don't if the peer is still active after having sent the response
mIsPeerActive = false;
}

Expand Down Expand Up @@ -698,15 +698,18 @@ void ExchangeContext::ExchangeSessionHolder::GrabExpiredSession(const SessionHan
bool ExchangeContext::IsPeerActive()
{
// We can never assume the peer is active if we are sending a group message
VerifyOrReturnError(IsGroupExchangeContext(), false);
if (IsGroupExchangeContext())
return false;

// If sender is not initiator, it means the peer sent the first message.
// This means the peer is active if we are sending a message to the peer.
VerifyOrReturnError(!IsInitiator(), true);
if (!IsInitiator())
return true;

// If we create an Ephemeral Exchange, it was just to generate a StandaloneAck
// Since we are sending an ack, we know the peer is active
VerifyOrReturnError(IsEphemeralExchange(), true);
if (IsEphemeralExchange())
return true;

// Return previously stored state if we have no absolute answer
return mIsPeerActive;
Expand Down
10 changes: 8 additions & 2 deletions src/messaging/ExchangeContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
* A notification that we will have SendMessage called on us in the future
* (and should stay open until that happens).
*/
void WillSendMessage() { mFlags.Set(Flags::kFlagWillSendMessage); }
void WillSendMessage(bool isPeerActive = false)
{
mFlags.Set(Flags::kFlagWillSendMessage);
mIsPeerActive = isPeerActive;
}

/**
* Handle a received CHIP message on this exchange.
Expand Down Expand Up @@ -216,7 +220,7 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,

/**
* Functions checks with information available in the ExchangeContext if the peer is active or not
*
*
* IF group exchange -> peer is never active
* If we are not initiator -> peer is active
* If ephemeral context -> peer is active
Expand All @@ -228,6 +232,8 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
*/
bool IsPeerActive();

void SetPeerActive() { mIsPeerActive = true; }

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
SessionHolder & GetSessionHolder() { return mSession; }

Expand Down
116 changes: 116 additions & 0 deletions src/messaging/tests/TestReliableMessageProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1540,6 +1540,121 @@ void CheckLostResponseWithPiggyback(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0);
}

void CheckIsPeerActiveNotInitiator(nlTestSuite * inSuite, void * inContext)
{
/**
* This tests the following scenario:
* 1) A reliable message expecting a response is sent from the initiator to responder which is losts
* 2) Initiator resends the message a the IdleRetrans interval
* 3) Responder receives the message and sends a standalone ack
* 4) Responder sends a response and fails
* 5) Responder retries at the ActiveRestrans interval
* 6) Initiator receives the response
*/

TestContext & ctx = *reinterpret_cast<TestContext *>(inContext);

chip::System::PacketBufferHandle buffer = chip::MessagePacketBuffer::NewWithData(PAYLOAD, sizeof(PAYLOAD));
NL_TEST_ASSERT(inSuite, !buffer.IsNull());

CHIP_ERROR err = CHIP_NO_ERROR;

MockAppDelegate mockReceiver(ctx);
err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &mockReceiver);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

mockReceiver.mTestSuite = inSuite;

MockAppDelegate mockSender(ctx);
ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

mockSender.mTestSuite = inSuite;

exchange->GetSessionHandle()->AsSecureSession()->SetRemoteMRPConfig({
1000_ms32, // CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL
1000_ms32, // CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
});

ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr();
NL_TEST_ASSERT(inSuite, rm != nullptr);

// Ensure the retransmit table is empty right now
NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0);

auto & loopback = ctx.GetLoopback();
loopback.mSentMessageCount = 0;
loopback.mNumMessagesToDrop = 1;
loopback.mDroppedMessageCount = 0;

mockReceiver.mRetainExchange = true;
mockSender.mRetainExchange = true;

NL_TEST_ASSERT(inSuite, !exchange->IsPeerActive());

err = exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), SendFlags(SendMessageFlags::kExpectResponse));
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();

// Verify that the first message is dropped
NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 1);
NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 1);
NL_TEST_ASSERT(inSuite, loopback.mNumMessagesToDrop == 0);

// Make sure retransmit was not done before the idle restrans interval hits
ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mSentMessageCount >= 1; });
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite, !exchange->IsPeerActive());

// // Make sure nothing happened
NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 1);
NL_TEST_ASSERT(inSuite, !mockReceiver.IsOnMessageReceivedCalled);

// // Retrasnmit message
ctx.GetIOContext().DriveIOUntil(2000_ms32, [&] { return loopback.mSentMessageCount >= 2; });
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite, !exchange->IsPeerActive());

// // Make sure nothing happened
NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 2);
NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled);

// // Verify that the receiver considers the sender is active
NL_TEST_ASSERT(inSuite, !exchange->IsPeerActive());
NL_TEST_ASSERT(inSuite, mockReceiver.mExchange->IsPeerActive());

mockReceiver.mExchange->GetSessionHandle()->AsSecureSession()->SetRemoteMRPConfig({
1000_ms32, // CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL
100_ms32, // CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
});

// Now send a message from the other side.
buffer = chip::MessagePacketBuffer::NewWithData(PAYLOAD, sizeof(PAYLOAD));
NL_TEST_ASSERT(inSuite, !buffer.IsNull());

// Make receiver message fail once
loopback.mNumMessagesToDrop = 1;

err = mockReceiver.mExchange->SendMessage(Echo::MsgType::EchoResponse, std::move(buffer));
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();

// Make sure nothing happened
NL_TEST_ASSERT(inSuite, loopback.mDroppedMessageCount == 2);
NL_TEST_ASSERT(inSuite, loopback.mNumMessagesToDrop == 0);
NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 3);
NL_TEST_ASSERT(inSuite, !mockSender.IsOnMessageReceivedCalled);

// // Retrasnmit message
ctx.GetIOContext().DriveIOUntil(150_ms32, [&] { return loopback.mSentMessageCount >= 4; });
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite, mockSender.IsOnMessageReceivedCalled);
NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 4);
}

void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext)
{
/**
Expand Down Expand Up @@ -1733,6 +1848,7 @@ const nlTest sTests[] = {
CheckLostResponseWithPiggyback),
NL_TEST_DEF("Test that an application-level response-to-response after a lost standalone ack to the initial message works",
CheckLostStandaloneAck),
NL_TEST_DEF("Test Is Peer Active Retry logic", CheckIsPeerActiveNotInitiator),
NL_TEST_DEF("Test MRP backoff algorithm", CheckGetBackoff),
NL_TEST_SENTINEL(),
};
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/bdx/TransferFacilitator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ CHIP_ERROR TransferFacilitator::OnMessageReceived(chip::Messaging::ExchangeConte
// transfer could necessitate a response if they are received at the wrong time.
// For this reason, it is left up to the application logic to call ExchangeContext::Close() when it has determined that the
// transfer is finished.
mExchangeCtx->WillSendMessage();
mExchangeCtx->WillSendMessage(true /* IsPeerActive */);

return err;
}
Expand Down
8 changes: 6 additions & 2 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,9 @@ CHIP_ERROR CASESession::SendSigma3a()
{
SuccessOrExit(err = helper->ScheduleWork());
mSendSigma3Helper = helper;
mExchangeCtxt->WillSendMessage();

// When we send sigma3, peer has processed sigma 1 and sent sigma2 which means it is active
mExchangeCtxt->WillSendMessage(true /* IsPeerActive */);
mState = State::kSendSigma3Pending;
}
else
Expand Down Expand Up @@ -1702,7 +1704,9 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg)

SuccessOrExit(err = helper->ScheduleWork());
mHandleSigma3Helper = helper;
mExchangeCtxt->WillSendMessage();

// When we receive sigma3, peer has received and processed sigma2 which means it is active
mExchangeCtxt->WillSendMessage(true /* IsPeerActive */);
mState = State::kHandleSigma3Pending;
}

Expand Down
4 changes: 4 additions & 0 deletions src/protocols/secure_channel/PASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,10 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl
MsgType msgType = static_cast<MsgType>(payloadHeader.GetMessageType());
SuccessOrExit(err);

// Once we receive a message for the PASE protocol, we know the Peer is active
// Since PASE doesn't notify the exchange that it will send a message, we force the isPeerActive flag
mExchangeCtxt->SetPeerActive();

#if CHIP_CONFIG_SLOW_CRYPTO
if (msgType == MsgType::PBKDFParamRequest || msgType == MsgType::PBKDFParamResponse || msgType == MsgType::PASE_Pake1 ||
msgType == MsgType::PASE_Pake2 || msgType == MsgType::PASE_Pake3)
Expand Down

0 comments on commit 1677310

Please sign in to comment.