From 352964787d1c39804ddb6286fa893d06a59f1a6e Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Thu, 3 Aug 2023 12:48:31 +0530 Subject: [PATCH] CASE: Send busy status report if we receive a sigma1 and we are in the 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 * 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 * Moved todo to better place --------- Co-authored-by: Boris Zbarsky Co-authored-by: Restyled.io Co-authored-by: Tennessee Carmel-Veilleux --- src/protocols/secure_channel/CASEServer.cpp | 27 ++++++++++++++--- src/protocols/secure_channel/CASEServer.h | 8 +++++ src/protocols/secure_channel/CASESession.cpp | 4 +++ src/protocols/secure_channel/PairingSession.h | 23 +++++++++++++++ src/protocols/secure_channel/StatusReport.cpp | 28 ++++++++++++++++++ src/protocols/secure_channel/StatusReport.h | 10 +++++++ .../secure_channel/tests/TestStatusReport.cpp | 29 +++++++++++++++++++ 7 files changed, 125 insertions(+), 4 deletions(-) diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index b78f5f40630680..f8d580fdd9957b 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -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; } } @@ -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 diff --git a/src/protocols/secure_channel/CASEServer.h b/src/protocols/secure_channel/CASEServer.h index 894d496c93bfac..0c1f1d26dcb963 100644 --- a/src/protocols/secure_channel/CASEServer.h +++ b/src/protocols/secure_channel/CASEServer.h @@ -22,6 +22,7 @@ #include #include #include +#include namespace chip { @@ -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 diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 07a75cf5fcbb2a..2afc40feacaf02 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -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; diff --git a/src/protocols/secure_channel/PairingSession.h b/src/protocols/secure_channel/PairingSession.h index 6e59f06dcaa660..ccbbd4a452ae64 100644 --- a/src/protocols/secure_channel/PairingSession.h +++ b/src/protocols/secure_channel/PairingSession.h @@ -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; diff --git a/src/protocols/secure_channel/StatusReport.cpp b/src/protocols/secure_channel/StatusReport.cpp index 18df6f7383e516..e31c832b469f51 100644 --- a/src/protocols/secure_channel/StatusReport.cpp +++ b/src/protocols/secure_channel/StatusReport.cpp @@ -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 diff --git a/src/protocols/secure_channel/StatusReport.h b/src/protocols/secure_channel/StatusReport.h index cd26b798eab1db..e25f2c6dc1a20b 100644 --- a/src/protocols/secure_channel/StatusReport.h +++ b/src/protocols/secure_channel/StatusReport.h @@ -21,6 +21,7 @@ #include #include #include +#include #include namespace chip { @@ -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; diff --git a/src/protocols/secure_channel/tests/TestStatusReport.cpp b/src/protocols/secure_channel/tests/TestStatusReport.cpp index 35cbe1b5bca65e..093453029b156b 100644 --- a/src/protocols/secure_channel/tests/TestStatusReport.cpp +++ b/src/protocols/secure_channel/tests/TestStatusReport.cpp @@ -16,6 +16,7 @@ * limitations under the License. */ +#include #include #include #include @@ -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 /** @@ -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() };