Resurrect TestPASESession unit test (#6266)

* Resurrect TestPASESession unit test * fix build error, and address review comments * restyled * add documentation, and address review comments * address comments
project-chip · Aug 24, 2021 · 9430564 · 9430564
1 parent 9cca020
commit 9430564
Show file tree

Hide file tree

Showing 8 changed files with 176 additions and 66 deletions.
diff --git a/src/messaging/tests/BUILD.gn b/src/messaging/tests/BUILD.gn
@@ -19,12 +19,30 @@ import("//build_overrides/nlunit_test.gni")
 
 import("${chip_root}/build/chip/chip_test_suite.gni")
 
-chip_test_suite("tests") {
-  output_name = "libMessagingLayerTests"
+static_library("helpers") {
+  output_name = "libMessagingTestHelpers"
+  output_dir = "${root_out_dir}/lib"
 
   sources = [
     "MessagingContext.cpp",
     "MessagingContext.h",
+  ]
+
+  cflags = [ "-Wconversion" ]
+
+  deps = [
+    "${chip_root}/src/messaging",
+    "${chip_root}/src/protocols",
+    "${chip_root}/src/transport",
+    "${chip_root}/src/transport/raw/tests:helpers",
+    "${nlio_root}:nlio",
+  ]
+}
+
+chip_test_suite("tests") {
+  output_name = "libMessagingLayerTests"
+
+  sources = [
     "TestExchangeMgr.cpp",
     "TestMessagingLayer.h",
     "TestReliableMessageProtocol.cpp",
@@ -33,6 +51,7 @@ chip_test_suite("tests") {
   cflags = [ "-Wconversion" ]
 
   public_deps = [
+    ":helpers",
     "${chip_root}/src/inet/tests:helpers",
     "${chip_root}/src/lib/core",
     "${chip_root}/src/lib/support",

diff --git a/src/messaging/tests/MessagingContext.cpp b/src/messaging/tests/MessagingContext.cpp
@@ -15,7 +15,7 @@
  *    limitations under the License.
  */
 
-#include <messaging/tests/MessagingContext.h>
+#include "MessagingContext.h"
 
 #include <support/CodeUtils.h>
 #include <support/ErrorStr.h>
@@ -53,13 +53,13 @@ CHIP_ERROR MessagingContext::Shutdown()
     return IOContext::Shutdown();
 }
 
-Messaging::ExchangeContext * MessagingContext::NewExchangeToPeer(Messaging::ExchangeDelegate * delegate)
+Messaging::ExchangeContext * MessagingContext::NewExchangeToPeer(Messaging::ExchangeDelegateBase * delegate)
 {
     // TODO: temprary create a SecureSessionHandle from node id, will be fix in PR 3602
     return mExchangeManager.NewContext({ GetDestinationNodeId(), GetPeerKeyId(), GetAdminId() }, delegate);
 }
 
-Messaging::ExchangeContext * MessagingContext::NewExchangeToLocal(Messaging::ExchangeDelegate * delegate)
+Messaging::ExchangeContext * MessagingContext::NewExchangeToLocal(Messaging::ExchangeDelegateBase * delegate)
 {
     // TODO: temprary create a SecureSessionHandle from node id, will be fix in PR 3602
     return mExchangeManager.NewContext({ GetSourceNodeId(), GetLocalKeyId(), GetAdminId() }, delegate);

diff --git a/src/messaging/tests/MessagingContext.h b/src/messaging/tests/MessagingContext.h
@@ -52,25 +52,41 @@ class MessagingContext : public IOContext
         Inet::IPAddress::FromString("127.0.0.1", addr);
         return addr;
     }
-    static constexpr NodeId GetSourceNodeId() { return 123654; }
-    static constexpr NodeId GetDestinationNodeId() { return 111222333; }
+    NodeId GetSourceNodeId() const { return mSourceNodeId; }
+    NodeId GetDestinationNodeId() const { return mDestinationNodeId; }
 
-    static constexpr uint16_t GetLocalKeyId() { return 1; }
-    static constexpr uint16_t GetPeerKeyId() { return 2; }
-    static constexpr uint16_t GetAdminId() { return 0; }
+    void SetSourceNodeId(NodeId nodeId) { mSourceNodeId = nodeId; }
+    void SetDestinationNodeId(NodeId nodeId) { mDestinationNodeId = nodeId; }
+
+    uint16_t GetLocalKeyId() const { return mLocalKeyId; }
+    uint16_t GetPeerKeyId() const { return mPeerKeyId; }
+
+    void SetLocalKeyId(uint16_t id) { mLocalKeyId = id; }
+    void SetPeerKeyId(uint16_t id) { mPeerKeyId = id; }
+
+    uint16_t GetAdminId() const { return mSrcAdminId; }
+    void SetAdminId(Transport::AdminId id)
+    {
+        mSrcAdminId  = id;
+        mDestAdminId = id;
+    }
 
     SecureSessionMgr & GetSecureSessionManager() { return mSecureSessionMgr; }
     Messaging::ExchangeManager & GetExchangeManager() { return mExchangeManager; }
 
-    Messaging::ExchangeContext * NewExchangeToPeer(Messaging::ExchangeDelegate * delegate);
-    Messaging::ExchangeContext * NewExchangeToLocal(Messaging::ExchangeDelegate * delegate);
+    Messaging::ExchangeContext * NewExchangeToPeer(Messaging::ExchangeDelegateBase * delegate);
+    Messaging::ExchangeContext * NewExchangeToLocal(Messaging::ExchangeDelegateBase * delegate);
 
     Credentials::OperationalCredentialSet & GetOperationalCredentialSet() { return mOperationalCredentialSet; }
 
 private:
     SecureSessionMgr mSecureSessionMgr;
     Messaging::ExchangeManager mExchangeManager;
 
+    NodeId mSourceNodeId      = 123654;
+    NodeId mDestinationNodeId = 111222333;
+    uint16_t mLocalKeyId      = 1;
+    uint16_t mPeerKeyId       = 2;
     Optional<Transport::PeerAddress> mPeer;
     SecurePairingUsingTestSecret mPairingPeerToLocal;
     SecurePairingUsingTestSecret mPairingLocalToPeer;

diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp
@@ -95,7 +95,7 @@ void PASESession::Clear()
 
     if (mExchangeCtxt != nullptr)
     {
-        mExchangeCtxt->Release();
+        mExchangeCtxt->Close();
         mExchangeCtxt = nullptr;
     }
 }
@@ -299,7 +299,7 @@ CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, uint32_t
     CHIP_ERROR err = Init(myKeyId, peerSetUpPINCode, delegate);
     SuccessOrExit(err);
 
-    mExchangeCtxt = exchangeCtxt->Retain();
+    mExchangeCtxt = exchangeCtxt;
     mExchangeCtxt->SetResponseTimeout(kSpake2p_Response_Timeout);
 
     mConnectionState.SetPeerAddress(peerAddress);
@@ -758,7 +758,7 @@ void PASESession::OnMessageReceived(ExchangeContext * ec, const PacketHeader & p
 
     if (mExchangeCtxt == nullptr)
     {
-        mExchangeCtxt = ec->Retain();
+        mExchangeCtxt = ec;
         mExchangeCtxt->SetResponseTimeout(kSpake2p_Response_Timeout);
     }
 

diff --git a/src/protocols/secure_channel/PASESession.h b/src/protocols/secure_channel/PASESession.h
@@ -115,6 +115,9 @@ class DLL_EXPORT PASESession : public Messaging::ExchangeDelegateBase, public Pa
      * @param peerSetUpPINCode Setup PIN code of the peer device
      * @param myKeyId          Key ID to be assigned to the secure session on the peer node
      * @param exchangeCtxt     The exchange context to send and receive messages with the peer
+     *                         Note: It's expected that the caller of this API hands over the
+     *                         ownership of the exchangeCtxt to PASESession object. PASESession
+     *                         will close the exchange on (successful/failed) handshake completion.
      * @param delegate         Callback object
      *
      * @return CHIP_ERROR      The result of initialization
@@ -200,9 +203,32 @@ class DLL_EXPORT PASESession : public Messaging::ExchangeDelegateBase, public Pa
     SessionEstablishmentExchangeDispatch & MessageDispatch() { return mMessageDispatch; }
 
     //// ExchangeDelegate Implementation ////
+    /**
+     * @brief
+     *   This function is the called by exchange context or exchange manager when it receives
+     *   a CHIP message corresponding to the context, or registered unsolicited message handler.
+     *
+     *   Note: If the function is called by unsolicited message handler, the ownership of the
+     *         provide exchange context is handed over to PASE Session object. The PASE Session
+     *         object ensures that the exchange will be closed on completion of the handshake.
+     *
+     *  @param[in]    ec            A pointer to the ExchangeContext object.
+     *  @param[in]    packetHeader  A reference to the PacketHeader object.
+     *  @param[in]    payloadHeader A reference to the PayloadHeader object.
+     *  @param[in]    payload       A handle to the PacketBuffer object holding the message payload.
+     */
     void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
                            System::PacketBufferHandle payload) override;
+
+    /**
+     * @brief
+     *   This function is the protocol callback to invoke when the timeout for the receipt
+     *   of a response message has expired.
+     *
+     *  @param[in]    ec            A pointer to the ExchangeContext object.
+     */
     void OnResponseTimeout(Messaging::ExchangeContext * ec) override;
+
     Messaging::ExchangeMessageDispatch * GetMessageDispatch(Messaging::ReliableMessageMgr * rmMgr,
                                                             SecureSessionMgr * sessionMgr) override
     {

diff --git a/src/protocols/secure_channel/RendezvousSession.cpp b/src/protocols/secure_channel/RendezvousSession.cpp
@@ -202,7 +202,6 @@ CHIP_ERROR RendezvousSession::Pair(uint32_t setupPINCode)
     ReturnErrorCodeIf(ctxt == nullptr, CHIP_ERROR_INTERNAL);
 
     CHIP_ERROR err = mPairingSession.Pair(mParams.GetPeerAddress(), setupPINCode, mNextKeyId++, ctxt, this);
-    ctxt->Release();
     return err;
 }
 

diff --git a/src/protocols/secure_channel/tests/BUILD.gn b/src/protocols/secure_channel/tests/BUILD.gn
@@ -8,11 +8,15 @@ import("${chip_root}/build/chip/chip_test_suite.gni")
 chip_test_suite("tests") {
   output_name = "libSecureChannelTests"
 
-  test_sources = [ "TestStatusReport.cpp" ]
+  test_sources = [
+    "TestPASESession.cpp",
+    "TestStatusReport.cpp",
+  ]
 
   public_deps = [
     "${chip_root}/src/lib/core",
     "${chip_root}/src/lib/support",
+    "${chip_root}/src/messaging/tests:helpers",
     "${chip_root}/src/protocols/secure_channel",
     "${nlio_root}:nlio",
     "${nlunit_test_root}:nlunit-test",