StandaloneAck handling: Untangle code path for UMH and StandaloneAck (#…

…19443)

* StandaloneAck handling: Untangle code path for UMH and StandaloneAck

* Fix comments from Boris

* Resolve comments

src/messaging/EphemeralExchangeDispatch.h

@@ -0,0 +1,54 @@
+/*
+ *    Copyright (c) 2022 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#pragma once
+
+#include <messaging/ExchangeMessageDispatch.h>
+#include <protocols/secure_channel/Constants.h>
+
+namespace chip {
+namespace Messaging {
+
+class EphemeralExchangeDispatch : public ExchangeMessageDispatch
+{
+public:
+    static ExchangeMessageDispatch & Instance()
+    {
+        static EphemeralExchangeDispatch instance;
+        return instance;
+    }
+
+    EphemeralExchangeDispatch() {}
+    ~EphemeralExchangeDispatch() override {}
+
+protected:
+    bool MessagePermitted(Protocols::Id protocol, uint8_t type) override
+    {
+        // Only permit StandaloneAck
+        return (protocol == Protocols::SecureChannel::Id &&
+                type == to_underlying(Protocols::SecureChannel::MsgType::StandaloneAck));
+    }
+
+    bool IsEncryptionRequired() const override
+    {
+        // This function should not be called at all
+        VerifyOrDie(false);
+        return false;
+    }
+};
+
+} // namespace Messaging
+} // namespace chip

src/messaging/ExchangeContext.cpp

@@ -39,6 +39,7 @@
 #include <lib/support/TypeTraits.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <messaging/ApplicationExchangeDispatch.h>
+#include <messaging/EphemeralExchangeDispatch.h>
 #include <messaging/ExchangeContext.h>
 #include <messaging/ExchangeMgr.h>
 #include <protocols/Protocols.h>
@@ -290,8 +291,8 @@ void ExchangeContextDeletor::Release(ExchangeContext * ec)
 }
 
 ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, const SessionHandle & session, bool Initiator,
-                                 ExchangeDelegate * delegate) :
-    mDispatch((delegate != nullptr) ? delegate->GetMessageDispatch() : ApplicationExchangeDispatch::Instance()),
+                                 ExchangeDelegate * delegate, bool isEphemeralExchange) :
+    mDispatch(GetMessageDispatch(isEphemeralExchange, delegate)),
     mSession(*this)
 {
     VerifyOrDie(mExchangeMgr == nullptr);
@@ -300,6 +301,7 @@ ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, cons
     mExchangeId  = ExchangeId;
     mSession.Grab(session);
     mFlags.Set(Flags::kFlagInitiator, Initiator);
+    mFlags.Set(Flags::kFlagEphemeralExchange, isEphemeralExchange);
     mDelegate = delegate;
 
     SetAckPending(false);
@@ -494,6 +496,12 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
         return CHIP_NO_ERROR;
     }
 
+    if (IsEphemeralExchange())
+    {
+        // The EphemeralExchange has done its job, since StandaloneAck is sent in previous FlushAcks() call.
+        return CHIP_NO_ERROR;
+    }
+
     // Since we got the response, cancel the response timer.
     CancelResponseTimer();
 
@@ -527,5 +535,16 @@ void ExchangeContext::MessageHandled()
     Close();
 }
 
+ExchangeMessageDispatch & ExchangeContext::GetMessageDispatch(bool isEphemeralExchange, ExchangeDelegate * delegate)
+{
+    if (isEphemeralExchange)
+        return EphemeralExchangeDispatch::Instance();
+
+    if (delegate != nullptr)
+        return delegate->GetMessageDispatch();
+
+    return ApplicationExchangeDispatch::Instance();
+}
+
 } // namespace Messaging
 } // namespace chip

src/messaging/ExchangeContext.h

@@ -66,7 +66,7 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
     typedef System::Clock::Timeout Timeout; // Type used to express the timeout in this ExchangeContext
 
     ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, const SessionHandle & session, bool Initiator,
-                    ExchangeDelegate * delegate);
+                    ExchangeDelegate * delegate, bool isEphemeralExchange = false);
 
     ~ExchangeContext() override;
 
@@ -154,8 +154,6 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
 
     ReliableMessageContext * GetReliableMessageContext() { return static_cast<ReliableMessageContext *>(this); };
 
-    ExchangeMessageDispatch & GetMessageDispatch() { return mDispatch; }
-
     SessionHandle GetSessionHandle() const
     {
         VerifyOrDie(mSession);
@@ -273,6 +271,8 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
      * exchange nor other component requests the active mode.
      */
     void UpdateSEDIntervalMode(bool activeMode);
+
+    static ExchangeMessageDispatch & GetMessageDispatch(bool isEphemeralExchange, ExchangeDelegate * delegate);
 };
 
 } // namespace Messaging

src/messaging/ExchangeMgr.cpp

@@ -270,72 +270,94 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
         return;
     }
 
-    // If we found a handler or we need to send an ack, create an exchange to
-    // handle the message.
-    if (matchingUMH != nullptr || payloadHeader.NeedsAck())
+    // If we found a handler, create an exchange to handle the message.
+    if (matchingUMH != nullptr)
     {
         ExchangeDelegate * delegate = nullptr;
 
         // Fetch delegate from the handler
-        if (matchingUMH != nullptr)
+        CHIP_ERROR err = matchingUMH->Handler->OnUnsolicitedMessageReceived(payloadHeader, delegate);
+        if (err != CHIP_NO_ERROR)
         {
-            CHIP_ERROR err = matchingUMH->Handler->OnUnsolicitedMessageReceived(payloadHeader, delegate);
-            if (err != CHIP_NO_ERROR)
-            {
-                // Using same error message for all errors to reduce code size.
-                ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(err));
-                return;
-            }
+            // Using same error message for all errors to reduce code size.
+            ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(err));
+            SendStandaloneAckIfNeeded(packetHeader, payloadHeader, session, msgFlags, std::move(msgBuf));
+            return;
         }
 
-        // If rcvd msg is from initiator then this exchange is created as not Initiator.
-        // If rcvd msg is not from initiator then this exchange is created as Initiator.
-        // Note that if matchingUMH is not null then rcvd msg if from initiator.
-        // TODO: Figure out which channel to use for the received message
-        ExchangeContext * ec =
-            mContextPool.CreateObject(this, payloadHeader.GetExchangeID(), session, !payloadHeader.IsInitiator(), delegate);
+        ExchangeContext * ec = mContextPool.CreateObject(this, payloadHeader.GetExchangeID(), session, false, delegate);
 
         if (ec == nullptr)
         {
-            if (matchingUMH != nullptr && delegate != nullptr)
+            if (delegate != nullptr)
             {
                 matchingUMH->Handler->OnExchangeCreationFailed(delegate);
             }
 
             // Using same error message for all errors to reduce code size.
             ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(CHIP_ERROR_NO_MEMORY));
+            // No resource for creating new exchange, SendStandaloneAckIfNeeded probably also fails, so do not try it here
             return;
         }
 
         ChipLogDetail(ExchangeManager, "Handling via exchange: " ChipLogFormatExchange ", Delegate: %p", ChipLogValueExchange(ec),
                       ec->GetDelegate());
 
-        // Make sure the exchange stays alive through the code below even if we
-        // close it before calling HandleMessage.
-        ExchangeHandle ref(*ec);
-
-        // Ignore encryption-required mismatches for emphemeral exchanges,
-        // because those never have delegates anyway.
-        if (matchingUMH != nullptr && ec->IsEncryptionRequired() != packetHeader.IsEncrypted())
+        if (ec->IsEncryptionRequired() != packetHeader.IsEncrypted())
         {
-            // We want to still to do MRP processing for this message, but we do
-            // not want to deliver it to the application.  Just close the
-            // exchange (which will notify the delegate, null it out, etc), then
-            // go ahead and call HandleMessage() on it to do the MRP
-            // processing.null out the delegate on the exchange, pretend to
-            // matchingUMH that exchange creation failed, so it cleans up the
-            // delegate, then tell the exchagne to handle the message.
-            ChipLogProgress(ExchangeManager, "OnMessageReceived encryption mismatch");
+            ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(CHIP_ERROR_INVALID_MESSAGE_TYPE));
             ec->Close();
+            SendStandaloneAckIfNeeded(packetHeader, payloadHeader, session, msgFlags, std::move(msgBuf));
+            return;
         }
 
-        CHIP_ERROR err = ec->HandleMessage(packetHeader.GetMessageCounter(), payloadHeader, msgFlags, std::move(msgBuf));
+        err = ec->HandleMessage(packetHeader.GetMessageCounter(), payloadHeader, msgFlags, std::move(msgBuf));
         if (err != CHIP_NO_ERROR)
         {
             // Using same error message for all errors to reduce code size.
             ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(err));
         }
+        return;
     }
+
+    SendStandaloneAckIfNeeded(packetHeader, payloadHeader, session, msgFlags, std::move(msgBuf));
+    return;
+}
+
+void ExchangeManager::SendStandaloneAckIfNeeded(const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
+                                                const SessionHandle & session, MessageFlags msgFlags,
+                                                System::PacketBufferHandle && msgBuf)
+{
+    // If we need to send a StandaloneAck, create a EphemeralExchange for the purpose to send the StandaloneAck
+    if (!payloadHeader.NeedsAck())
+        return;
+
+    // If rcvd msg is from initiator then this exchange is created as not Initiator.
+    // If rcvd msg is not from initiator then this exchange is created as Initiator.
+    // Create a EphemeralExchange to generate a StandaloneAck
+    ExchangeContext * ec = mContextPool.CreateObject(this, payloadHeader.GetExchangeID(), session, !payloadHeader.IsInitiator(),
+                                                     nullptr, true /* IsEphemeralExchange */);
+
+    if (ec == nullptr)
+    {
+        // Using same error message for all errors to reduce code size.
+        ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(CHIP_ERROR_NO_MEMORY));
+        return;
+    }
+
+    ChipLogDetail(ExchangeManager, "Generating StandaloneAck via exchange: " ChipLogFormatExchange, ChipLogValueExchange(ec));
+
+    // No need to verify packet encryption type, the EphemeralExchange can handle both secure and insecure messages.
+
+    CHIP_ERROR err = ec->HandleMessage(packetHeader.GetMessageCounter(), payloadHeader, msgFlags, std::move(msgBuf));
+    if (err != CHIP_NO_ERROR)
+    {
+        // Using same error message for all errors to reduce code size.
+        ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(err));
+    }
+
+    // The exchange should be closed inside HandleMessage function. So don't bother close it here.
+    return;
 }
 
 void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * delegate)

src/messaging/ExchangeMgr.h

@@ -240,6 +240,8 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
 
     void OnMessageReceived(const PacketHeader & packetHeader, const PayloadHeader & payloadHeader, const SessionHandle & session,
                            DuplicateMessage isDuplicate, System::PacketBufferHandle && msgBuf) override;
+    void SendStandaloneAckIfNeeded(const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
+                                   const SessionHandle & session, MessageFlags msgFlags, System::PacketBufferHandle && msgBuf);
 };
 
 } // namespace Messaging

src/messaging/ReliableMessageContext.h

@@ -121,6 +121,9 @@ class ReliableMessageContext
     /// Determine whether this exchange is requesting Sleepy End Device active mode
     bool IsRequestingActiveMode() const;
 
+    /// Determine whether this exchange is a EphemeralExchange for replying a StandaloneAck
+    bool IsEphemeralExchange() const;
+
     /**
      * Get the reliable message manager that corresponds to this reliable
      * message context.
@@ -158,6 +161,9 @@ class ReliableMessageContext
 
         /// When set, signifies that the exchange is requesting Sleepy End Device active mode.
         kFlagActiveMode = (1u << 8),
+
+        /// When set, signifies that the exchange created sorely for replying a StandaloneAck
+        kFlagEphemeralExchange = (1u << 9),
     };
 
     BitFlags<Flags> mFlags; // Internal state flags
@@ -234,5 +240,10 @@ inline void ReliableMessageContext::SetRequestingActiveMode(bool activeMode)
     mFlags.Set(Flags::kFlagActiveMode, activeMode);
 }
 
+inline bool ReliableMessageContext::IsEphemeralExchange() const
+{
+    return mFlags.Has(Flags::kFlagEphemeralExchange);
+}
+
 } // namespace Messaging
 } // namespace chip