From 5241395046b26757b9f29cd262db92e690939a64 Mon Sep 17 00:00:00 2001
From: Yufeng Wang <yufengwang@google.com>
Date: Tue, 4 May 2021 12:34:42 -0700
Subject: [PATCH] Disable CRMP if we send/receive messages not over UDP
 transport (#6407)

* Disable CRMP if we send/receive messages not over UDP transport

* Declare reliableTransmissionRequested when it is first use
---
 src/messaging/ExchangeContext.cpp                    | 12 +++++++++++-
 src/messaging/ExchangeMessageDispatch.cpp            |  4 ++--
 src/messaging/ExchangeMessageDispatch.h              |  2 +-
 src/messaging/ReliableMessageMgr.cpp                 |  6 +++---
 .../SessionEstablishmentExchangeDispatch.h           |  6 +++---
 5 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp
index 92fb9873db8009..52672579ccf529 100644
--- a/src/messaging/ExchangeContext.cpp
+++ b/src/messaging/ExchangeContext.cpp
@@ -123,7 +123,17 @@ CHIP_ERROR ExchangeContext::SendMessageImpl(Protocols::Id protocolId, uint8_t ms
     // an error arising below. at the end, we have to close it.
     Retain();
 
-    bool reliableTransmissionRequested = !sendFlags.Has(SendMessageFlags::kNoAutoRequestAck);
+    bool reliableTransmissionRequested = true;
+
+    // If sending via UDP and NoAutoRequestAck send flag is not specificed, request reliable transmission.
+    if (state && state->GetPeerAddress().GetTransportType() != Transport::Type::kUdp)
+    {
+        reliableTransmissionRequested = false;
+    }
+    else
+    {
+        reliableTransmissionRequested = !sendFlags.Has(SendMessageFlags::kNoAutoRequestAck);
+    }
 
     ExchangeMessageDispatch * dispatch = GetMessageDispatch();
     ApplicationExchangeDispatch defaultDispatch;
diff --git a/src/messaging/ExchangeMessageDispatch.cpp b/src/messaging/ExchangeMessageDispatch.cpp
index 0429eb62fa530a..5afc11d71cd525 100644
--- a/src/messaging/ExchangeMessageDispatch.cpp
+++ b/src/messaging/ExchangeMessageDispatch.cpp
@@ -65,7 +65,7 @@ CHIP_ERROR ExchangeMessageDispatch::SendMessage(SecureSessionHandle session, uin
 #endif
     }
 
-    if (!IsTransportReliable() && reliableMessageContext->AutoRequestAck() && mReliableMessageMgr != nullptr &&
+    if (IsReliableTransmissionAllowed() && reliableMessageContext->AutoRequestAck() && mReliableMessageMgr != nullptr &&
         isReliableTransmission)
     {
         payloadHeader.SetNeedsAck(true);
@@ -105,7 +105,7 @@ CHIP_ERROR ExchangeMessageDispatch::OnMessageReceived(const PayloadHeader & payl
     ReturnErrorCodeIf(!MessagePermitted(payloadHeader.GetProtocolID().GetProtocolId(), payloadHeader.GetMessageType()),
                       CHIP_ERROR_INVALID_ARGUMENT);
 
-    if (!IsTransportReliable())
+    if (IsReliableTransmissionAllowed())
     {
         if (payloadHeader.IsAckMsg() && payloadHeader.GetAckId().HasValue())
         {
diff --git a/src/messaging/ExchangeMessageDispatch.h b/src/messaging/ExchangeMessageDispatch.h
index e48e1151d9106f..ef2cb596b28445 100644
--- a/src/messaging/ExchangeMessageDispatch.h
+++ b/src/messaging/ExchangeMessageDispatch.h
@@ -63,7 +63,7 @@ class ExchangeMessageDispatch
     virtual CHIP_ERROR SendMessageImpl(SecureSessionHandle session, PayloadHeader & payloadHeader,
                                        System::PacketBufferHandle && message, EncryptedPacketBufferHandle * retainedMessage) = 0;
 
-    virtual bool IsTransportReliable() { return false; }
+    virtual bool IsReliableTransmissionAllowed() { return true; }
 
 private:
     ReliableMessageMgr * mReliableMessageMgr = nullptr;
diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp
index 53fb46ad2d5a67..76f4b858ecc5d8 100644
--- a/src/messaging/ReliableMessageMgr.cpp
+++ b/src/messaging/ReliableMessageMgr.cpp
@@ -344,11 +344,11 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry)
 
     VerifyOrReturnError(rc != nullptr, err);
 
-    const ExchangeMessageDispatch * transport = rc->GetExchangeContext()->GetMessageDispatch();
-    VerifyOrExit(transport != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
+    const ExchangeMessageDispatch * dispatcher = rc->GetExchangeContext()->GetMessageDispatch();
+    VerifyOrExit(dispatcher != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
 
     err =
-        transport->ResendMessage(rc->GetExchangeContext()->GetSecureSession(), std::move(entry->retainedBuf), &entry->retainedBuf);
+        dispatcher->ResendMessage(rc->GetExchangeContext()->GetSecureSession(), std::move(entry->retainedBuf), &entry->retainedBuf);
     SuccessOrExit(err);
 
     // Update the counters
diff --git a/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.h b/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.h
index 3d78091ab3a335..b222d7b318ab7b 100644
--- a/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.h
+++ b/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.h
@@ -57,10 +57,10 @@ class SessionEstablishmentExchangeDispatch : public Messaging::ExchangeMessageDi
 
     bool MessagePermitted(uint16_t protocol, uint8_t type) override;
 
-    bool IsTransportReliable() override
+    bool IsReliableTransmissionAllowed() override
     {
-        // If the underlying transport is not UDP.
-        return (mPeerAddress.GetTransportType() != Transport::Type::kUdp);
+        // If the underlying transport is UDP.
+        return (mPeerAddress.GetTransportType() == Transport::Type::kUdp);
     }
 
 private: