From 1679938dc6eb362a480c0fa847ea2f0aac14c293 Mon Sep 17 00:00:00 2001 From: Evgeny Margolis Date: Tue, 25 Jan 2022 09:29:24 -0800 Subject: [PATCH] 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> --- src/protocols/secure_channel/CASESession.cpp | 5 ++--- src/protocols/secure_channel/CASESession.h | 4 +--- src/protocols/secure_channel/PASESession.cpp | 7 ++----- src/protocols/secure_channel/PASESession.h | 9 ++++----- src/transport/PairingSession.h | 16 +++++++--------- src/transport/tests/TestPairingSession.cpp | 8 +++++--- 6 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index e88980435c9eef..d0a4d50f859d85 100644 --- 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 index ef1309790b5b48..5fb31830457d2f 100644 --- 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 index 260dc47e8d7990..473420097f27f5 100644 --- 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 index f3ff1ae62f0df0..1d3b5b93290f60 100644 --- 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,7 +330,7 @@ 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. @@ -340,7 +338,8 @@ class SecurePairingUsingTestSecret : public PairingSession 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 index 22e13365d72b65..b33dc52d726222 100644 --- 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 index 9c4b094f1b56ae..1701d42015d3c3 100644 --- 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;