Skip to content

Commit

Permalink
Don't Clear mSecureSessionType on PairingSession::Clear() (project-ch…
Browse files Browse the repository at this point in the history
…ip#13772)

* Don't Clear mSecureSessionType on PairingSession::Clear()

* Update src/transport/PairingSession.h

Co-authored-by: Marc Lepage <[email protected]>

* Made mSecureSessionType const and initialize it from an arg to the constructor.

* cleanup

Co-authored-by: Marc Lepage <[email protected]>
  • Loading branch information
2 people authored and selissia committed Jan 28, 2022
1 parent 9cfeb8d commit ebe999a
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 28 deletions.
5 changes: 2 additions & 3 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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();
}

Expand Down
4 changes: 1 addition & 3 deletions src/protocols/secure_channel/CASESession.h
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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();

Expand Down
7 changes: 2 additions & 5 deletions src/protocols/secure_channel/PASESession.cpp
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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()
{
Expand Down
9 changes: 4 additions & 5 deletions src/protocols/secure_channel/PASESession.h
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
16 changes: 7 additions & 9 deletions src/transport/PairingSession.h
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -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); }
Expand Down Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/transport/tests/TestPairingSession.cpp
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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)
Expand All @@ -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));

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit ebe999a

Please sign in to comment.