Don't Clear mSecureSessionType on PairingSession::Clear() (#13772)

* Don't Clear mSecureSessionType on PairingSession::Clear() * Update src/transport/PairingSession.h Co-authored-by: Marc Lepage <67919234+mlepage-google@users.noreply.github.com> * Made mSecureSessionType const and initialize it from an arg to the constructor. * cleanup Co-authored-by: Marc Lepage <67919234+mlepage-google@users.noreply.github.com>
project-chip · Oct 23, 2023 · 1679938 · 1679938
1 parent ca3032d
commit 1679938
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 28 deletions.
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2021 Project CHIP Authors
+ *    Copyright (c) 2021-2022 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -89,9 +89,8 @@ using HKDF_sha_crypto = HKDF_sha;
 // The session establishment fails if the response is not received within timeout window.
 static constexpr ExchangeContext::Timeout kSigma_Response_Timeout = System::Clock::Seconds16(30);
 
-CASESession::CASESession()
+CASESession::CASESession() : PairingSession(Transport::SecureSession::Type::kCASE)
 {
-    SetSecureSessionType(Transport::SecureSession::Type::kCASE);
     mTrustedRootId = CertificateKeyId();
 }
 

diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2021 Project CHIP Authors
+ *    Copyright (c) 2021-2022 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -75,8 +75,6 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
     CASESession();
     CASESession(CASESession &&)      = default;
     CASESession(const CASESession &) = default;
-    CASESession & operator=(const CASESession &) = default;
-    CASESession & operator=(CASESession &&) = default;
 
     virtual ~CASESession();
 

diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020-2021 Project CHIP Authors
+ *    Copyright (c) 2020-2022 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -71,10 +71,7 @@ using PBKDF2_sha256_crypto = PBKDF2_sha256HSM;
 using PBKDF2_sha256_crypto = PBKDF2_sha256;
 #endif
 
-PASESession::PASESession()
-{
-    SetSecureSessionType(Transport::SecureSession::Type::kPASE);
-}
+PASESession::PASESession() : PairingSession(Transport::SecureSession::Type::kPASE) {}
 
 PASESession::~PASESession()
 {

diff --git a/src/protocols/secure_channel/PASESession.h b/src/protocols/secure_channel/PASESession.h
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020 Project CHIP Authors
+ *    Copyright (c) 2020-2022 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -85,8 +85,6 @@ class DLL_EXPORT PASESession : public Messaging::ExchangeDelegate, public Pairin
     PASESession();
     PASESession(PASESession &&)      = default;
     PASESession(const PASESession &) = delete;
-    PASESession & operator=(const PASESession &) = default;
-    PASESession & operator=(PASESession &&) = default;
 
     virtual ~PASESession();
 
@@ -332,15 +330,16 @@ constexpr chip::NodeId kTestDeviceNodeId     = 12344321;
 class SecurePairingUsingTestSecret : public PairingSession
 {
 public:
-    SecurePairingUsingTestSecret()
+    SecurePairingUsingTestSecret() : PairingSession(Transport::SecureSession::Type::kPASE)
     {
         // Do not set to 0 to prevent unwanted unsecured session
         // since the session type is unknown.
         SetLocalSessionId(1);
         SetPeerSessionId(1);
     }
 
-    SecurePairingUsingTestSecret(uint16_t peerSessionId, uint16_t localSessionId)
+    SecurePairingUsingTestSecret(uint16_t peerSessionId, uint16_t localSessionId) :
+        PairingSession(Transport::SecureSession::Type::kPASE)
     {
         SetLocalSessionId(localSessionId);
         SetPeerSessionId(peerSessionId);

diff --git a/src/transport/PairingSession.h b/src/transport/PairingSession.h
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2021 Project CHIP Authors
+ *    Copyright (c) 2021-2022 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -38,7 +38,7 @@ namespace chip {
 class DLL_EXPORT PairingSession
 {
 public:
-    PairingSession() {}
+    PairingSession(Transport::SecureSession::Type secureSessionType) : mSecureSessionType(secureSessionType) {}
     virtual ~PairingSession() {}
 
     Transport::SecureSession::Type GetSecureSessionType() const { return mSecureSessionType; }
@@ -104,7 +104,6 @@ class DLL_EXPORT PairingSession
                                           TLV::TLVWriter & tlvWriter);
 
 protected:
-    void SetSecureSessionType(Transport::SecureSession::Type secureSessionType) { mSecureSessionType = secureSessionType; }
     void SetPeerNodeId(NodeId peerNodeId) { mPeerNodeId = peerNodeId; }
     void SetPeerCATs(CATValues peerCATs) { mPeerCATs = peerCATs; }
     void SetPeerSessionId(uint16_t id) { mPeerSessionId.SetValue(id); }
@@ -177,17 +176,16 @@ class DLL_EXPORT PairingSession
     // TODO: remove Clear, we should create a new instance instead reset the old instance.
     void Clear()
     {
-        mSecureSessionType = Transport::SecureSession::Type::kUndefined;
-        mPeerNodeId        = kUndefinedNodeId;
-        mPeerCATs          = kUndefinedCATs;
-        mPeerAddress       = Transport::PeerAddress::Uninitialized();
+        mPeerNodeId  = kUndefinedNodeId;
+        mPeerCATs    = kUndefinedCATs;
+        mPeerAddress = Transport::PeerAddress::Uninitialized();
         mPeerSessionId.ClearValue();
         mLocalSessionId = kInvalidKeyId;
     }
 
 private:
-    Transport::SecureSession::Type mSecureSessionType = Transport::SecureSession::Type::kUndefined;
-    NodeId mPeerNodeId                                = kUndefinedNodeId;
+    const Transport::SecureSession::Type mSecureSessionType;
+    NodeId mPeerNodeId = kUndefinedNodeId;
     CATValues mPeerCATs;
 
     // TODO: the local key id should be allocateed at start

diff --git a/src/transport/tests/TestPairingSession.cpp b/src/transport/tests/TestPairingSession.cpp
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020 Project CHIP Authors
+ *    Copyright (c) 2020-2022 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -41,6 +41,8 @@ using namespace chip::System::Clock;
 class TestPairingSession : public PairingSession
 {
 public:
+    TestPairingSession(Transport::SecureSession::Type secureSessionType) : PairingSession(secureSessionType) {}
+
     CHIP_ERROR DeriveSecureSession(CryptoContext & session, CryptoContext::SessionRole role) override { return CHIP_NO_ERROR; }
 
     CHIP_ERROR DecodeMRPParametersIfPresent(TLV::Tag expectedTag, System::PacketBufferTLVReader & tlvReader)
@@ -51,7 +53,7 @@ class TestPairingSession : public PairingSession
 
 void PairingSessionEncodeDecodeMRPParams(nlTestSuite * inSuite, void * inContext)
 {
-    TestPairingSession session;
+    TestPairingSession session(Transport::SecureSession::Type::kCASE);
 
     ReliableMessageProtocolConfig config(Milliseconds32(100), Milliseconds32(200));
 
@@ -84,7 +86,7 @@ void PairingSessionEncodeDecodeMRPParams(nlTestSuite * inSuite, void * inContext
 
 void PairingSessionTryDecodeMissingMRPParams(nlTestSuite * inSuite, void * inContext)
 {
-    TestPairingSession session;
+    TestPairingSession session(Transport::SecureSession::Type::kPASE);
 
     System::PacketBufferHandle buf = System::PacketBufferHandle::New(64, 0);
     System::PacketBufferTLVWriter writer;