Skip to content

Commit

Permalink
CASE: Send busy status report if we receive a sigma1 and we are in th…
Browse files Browse the repository at this point in the history
…e middle of handshake (#28153)

* CASE: Send busy status report if we receive a sigma1 and we are in the
middle of handshake

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Addressing review comments

* few more review comments

* Addressed reviews and added the unit tests

* Restyled by clang-format

* Few more review comments

* Apply suggestions from code review

Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com>

* Moved todo to better place

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com>
  • Loading branch information
4 people authored and pull[bot] committed Apr 19, 2024
1 parent b6b6a95 commit 3809139
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 4 deletions.
27 changes: 23 additions & 4 deletions src/protocols/secure_channel/CASEServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,17 @@ CHIP_ERROR CASEServer::OnMessageReceived(Messaging::ExchangeContext * ec, const
bool watchdogFired = GetSession().InvokeBackgroundWorkWatchdog();
if (!watchdogFired)
{
// Handshake wasn't stuck, let it continue its work
// TODO: Send Busy response, #27473
ChipLogError(Inet, "CASE session is in establishing state, returning without responding");
return CHIP_NO_ERROR;
// Handshake wasn't stuck, send the busy status report and let the existing handshake continue.

// A successful CASE handshake can take several seconds and some may time out (30 seconds or more).
// TODO: Come up with better estimate: https://github.com/project-chip/connectedhomeip/issues/28288
// For now, setting minimum wait time to 5000 milliseconds.
CHIP_ERROR err = SendBusyStatusReport(ec, System::Clock::Milliseconds16(5000));
if (err != CHIP_NO_ERROR)
{
ChipLogError(Inet, "Failed to send the busy status report, err:%" CHIP_ERROR_FORMAT, err.Format());
}
return err;
}
}

Expand Down Expand Up @@ -181,4 +188,16 @@ void CASEServer::OnSessionEstablished(const SessionHandle & session)
ChipLogValueScopedNodeId(session->GetPeer()));
PrepareForSessionEstablishment(session->GetPeer());
}

CHIP_ERROR CASEServer::SendBusyStatusReport(Messaging::ExchangeContext * ec, System::Clock::Milliseconds16 minimumWaitTime)
{
ChipLogProgress(Inet, "Already in the middle of CASE handshake, sending busy status report");

System::PacketBufferHandle handle = Protocols::SecureChannel::StatusReport::MakeBusyStatusReportMessage(minimumWaitTime);
VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_NO_MEMORY);

ChipLogProgress(Inet, "Sending status report, exchange " ChipLogFormatExchange, ChipLogValueExchange(ec));
return ec->SendMessage(Protocols::SecureChannel::MsgType::StatusReport, std::move(handle));
}

} // namespace chip
8 changes: 8 additions & 0 deletions src/protocols/secure_channel/CASEServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <messaging/ExchangeDelegate.h>
#include <messaging/ExchangeMgr.h>
#include <protocols/secure_channel/CASESession.h>
#include <system/SystemClock.h>

namespace chip {

Expand Down Expand Up @@ -107,6 +108,13 @@ class CASEServer : public SessionEstablishmentDelegate,
*
*/
void PrepareForSessionEstablishment(const ScopedNodeId & previouslyEstablishedPeer = ScopedNodeId());

// If we are in the middle of handshake and receive a Sigma1 then respond with Busy status code.
// @param[in] ec Exchange Context
// @param[in] minimumWaitTime Minimum wait time reported to client before it can attempt to resend sigma1
//
// @return CHIP_NO_ERROR on success, error code otherwise
CHIP_ERROR SendBusyStatusReport(Messaging::ExchangeContext * ec, System::Clock::Milliseconds16 minimumWaitTime);
};

} // namespace chip
4 changes: 4 additions & 0 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1977,6 +1977,10 @@ CHIP_ERROR CASESession::OnFailureStatusReport(Protocols::SecureChannel::GeneralS
err = CHIP_ERROR_NO_SHARED_TRUSTED_ROOT;
break;

case kProtocolCodeBusy:
err = CHIP_ERROR_BUSY;
break;

default:
err = CHIP_ERROR_INTERNAL;
break;
Expand Down
23 changes: 23 additions & 0 deletions src/protocols/secure_channel/PairingSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,29 @@ class DLL_EXPORT PairingSession : public SessionDelegate
else
{
err = OnFailureStatusReport(report.GetGeneralCode(), report.GetProtocolCode());

if (report.GetGeneralCode() == Protocols::SecureChannel::GeneralStatusCode::kBusy &&
report.GetProtocolCode() == Protocols::SecureChannel::kProtocolCodeBusy)
{
if (!report.GetProtocolData().IsNull())
{
Encoding::LittleEndian::Reader reader(report.GetProtocolData()->Start(),
report.GetProtocolData()->DataLength());

uint16_t minimumWaitTime = 0;
err = reader.Read16(&minimumWaitTime).StatusCode();
if (err != CHIP_NO_ERROR)
{
ChipLogError(SecureChannel, "Failed to read the minimum wait time: %" CHIP_ERROR_FORMAT, err.Format());
}
else
{
// TODO: CASE: Notify minimum wait time to clients on receiving busy status report #28290
ChipLogProgress(SecureChannel, "Received busy status report with minimum wait time: %u ms",
minimumWaitTime);
}
}
}
}

return err;
Expand Down
28 changes: 28 additions & 0 deletions src/protocols/secure_channel/StatusReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,34 @@ size_t StatusReport::Size() const
return WriteToBuffer(emptyBuf).Needed();
}

System::PacketBufferHandle StatusReport::MakeBusyStatusReportMessage(System::Clock::Milliseconds16 minimumWaitTime)
{
using namespace Protocols::SecureChannel;
constexpr uint8_t kBusyStatusReportProtocolDataSize = sizeof(minimumWaitTime.count()); // 16-bits

auto handle = System::PacketBufferHandle::New(kBusyStatusReportProtocolDataSize, 0);
VerifyOrReturnValue(!handle.IsNull(), handle,
ChipLogError(SecureChannel, "Failed to allocate protocol data for busy status report"));

// Build the protocol data with minimum wait time
Encoding::LittleEndian::PacketBufferWriter protocolDataBufferWriter(std::move(handle));
protocolDataBufferWriter.Put16(minimumWaitTime.count());
handle = protocolDataBufferWriter.Finalize();
VerifyOrReturnValue(!handle.IsNull(), handle,
ChipLogError(SecureChannel, "Failed to finalize protocol data for busy status report"));

// Build a busy status report
StatusReport statusReport(GeneralStatusCode::kBusy, Protocols::SecureChannel::Id, kProtocolCodeBusy, std::move(handle));

// Build the status report message
handle = System::PacketBufferHandle::New(statusReport.Size());
VerifyOrReturnValue(!handle.IsNull(), handle, ChipLogError(SecureChannel, "Failed to allocate status report message"));
Encoding::LittleEndian::PacketBufferWriter bbuf(std::move(handle));

statusReport.WriteToBuffer(bbuf);
return bbuf.Finalize();
}

} // namespace SecureChannel
} // namespace Protocols
} // namespace chip
10 changes: 10 additions & 0 deletions src/protocols/secure_channel/StatusReport.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <lib/support/BufferWriter.h>
#include <protocols/Protocols.h>
#include <protocols/secure_channel/Constants.h>
#include <system/SystemClock.h>
#include <system/SystemPacketBuffer.h>

namespace chip {
Expand Down Expand Up @@ -94,6 +95,15 @@ class DLL_EXPORT StatusReport
uint16_t GetProtocolCode() const { return mProtocolCode; }
const System::PacketBufferHandle & GetProtocolData() const { return mProtocolData; }

/**
* Builds a busy status report with protocol data containing the minimum wait time.
*
* @param[in] minimumWaitTime Time in milliseconds before initiator retries the request
*
* @return Packet buffer handle which can be passed to SendMessage.
*/
static System::PacketBufferHandle MakeBusyStatusReportMessage(System::Clock::Milliseconds16 minimumWaitTime);

private:
GeneralStatusCode mGeneralCode;
Protocols::Id mProtocolId;
Expand Down
29 changes: 29 additions & 0 deletions src/protocols/secure_channel/tests/TestStatusReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* limitations under the License.
*/

#include <lib/support/BufferReader.h>
#include <lib/support/BufferWriter.h>
#include <lib/support/CHIPMem.h>
#include <lib/support/UnitTestRegistration.h>
Expand Down Expand Up @@ -104,6 +105,33 @@ void TestBadStatusReport(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR);
}

void TestMakeBusyStatusReport(nlTestSuite * inSuite, void * inContext)
{
GeneralStatusCode generalCode = GeneralStatusCode::kBusy;
auto protocolId = SecureChannel::Id;
uint16_t protocolCode = kProtocolCodeBusy;
System::Clock::Milliseconds16 minimumWaitTime = System::Clock::Milliseconds16(5000);

System::PacketBufferHandle handle = StatusReport::MakeBusyStatusReportMessage(minimumWaitTime);
NL_TEST_ASSERT(inSuite, !handle.IsNull());

StatusReport reportToParse;
CHIP_ERROR err = reportToParse.Parse(std::move(handle));
NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(inSuite, reportToParse.GetGeneralCode() == generalCode);
NL_TEST_ASSERT(inSuite, reportToParse.GetProtocolId() == protocolId);
NL_TEST_ASSERT(inSuite, reportToParse.GetProtocolCode() == protocolCode);

const System::PacketBufferHandle & rcvData = reportToParse.GetProtocolData();
NL_TEST_ASSERT(inSuite, !rcvData.IsNull());
NL_TEST_ASSERT(inSuite, rcvData->DataLength() == sizeof(minimumWaitTime));

uint16_t readMinimumWaitTime = 0;
Encoding::LittleEndian::Reader reader(rcvData->Start(), rcvData->DataLength());
NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == reader.Read16(&readMinimumWaitTime).StatusCode());
NL_TEST_ASSERT(inSuite, System::Clock::Milliseconds16(readMinimumWaitTime) == minimumWaitTime);
}

// Test Suite

/**
Expand All @@ -115,6 +143,7 @@ static const nlTest sTests[] =
NL_TEST_DEF("TestStatusReport_NoData", TestStatusReport_NoData),
NL_TEST_DEF("TestStatusReport_WithData", TestStatusReport_WithData),
NL_TEST_DEF("TestBadStatusReport", TestBadStatusReport),
NL_TEST_DEF("TestMakeBusyStatusReport", TestMakeBusyStatusReport),

NL_TEST_SENTINEL()
};
Expand Down

0 comments on commit 3809139

Please sign in to comment.