Skip to content

Commit

Permalink
[case] Do not expire reserved secure sessions on Sigma1 reception (#1…
Browse files Browse the repository at this point in the history
…7340)

Upon reception of Sigma1 CASEServer expires all sessions
with undefined fabric index and node ID, supposedly to
expire stale PASE sessions, but the code seems obsolete
since PASE sessions are already deleted by Commissioning
Window Manager on receiving CommissioningComplete command.

In fact, the code causes a problem in the following
scenario:
1. A device tries to establish a connection to another
   device, for example OTA Provider.
2. The controller tries establish a connection with the
   device, for example to read its cluster attribute.

In such a case, although CASE session between the device
and the OTA Provider will succeed, deriving a secure session
will fail because the CASE server will already have released
the session reserved by the CASE client.
  • Loading branch information
Damian-Nordic authored and pull[bot] committed Oct 17, 2023
1 parent 49d2cdc commit 2433817
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
7 changes: 0 additions & 7 deletions src/protocols/secure_channel/CASEServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,6 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec)
{
ReturnErrorCodeIf(ec == nullptr, CHIP_ERROR_INVALID_ARGUMENT);

// Mark any PASE sessions used for commissioning as stale.
// This is a workaround, as we currently don't have a way to identify
// secure sessions established via PASE protocol.
// TODO - Identify which PASE base secure channel was used
// for commissioning and drop it once commissioning is complete.
mSessionManager->ExpireAllPairings(kUndefinedNodeId, kUndefinedFabricIndex);

#if CONFIG_NETWORK_LAYER_BLE
// Close all BLE connections now since a CASE handshake has been initiated.
if (mBleLayer != nullptr)
Expand Down
23 changes: 15 additions & 8 deletions src/protocols/secure_channel/tests/TestCASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,12 @@ void CASE_SecurePairingHandshakeServerTest(nlTestSuite * inSuite, void * inConte
auto * pairingCommissioner = chip::Platform::New<CASESession>();
pairingCommissioner->SetGroupDataProvider(&gCommissionerGroupDataProvider);

SessionManager sessionManager;

TestContext & ctx = *reinterpret_cast<TestContext *>(inContext);

gLoopback.mSentMessageCount = 0;

// Use the same session manager on both CASE client and server sides to validate that both
// components may work simultaneously on a single device.
NL_TEST_ASSERT(inSuite,
gPairingServer.ListenForSessionEstablishment(&ctx.GetExchangeManager(), &ctx.GetTransportMgr(),
#if CONFIG_NETWORK_LAYER_BLE
Expand All @@ -323,22 +323,29 @@ void CASE_SecurePairingHandshakeServerTest(nlTestSuite * inSuite, void * inConte
NL_TEST_ASSERT(inSuite, fabric != nullptr);

NL_TEST_ASSERT(inSuite,
pairingCommissioner->EstablishSession(sessionManager, Transport::PeerAddress(Transport::Type::kBle), fabric,
Node01_01, contextCommissioner, nullptr,
&delegateCommissioner) == CHIP_NO_ERROR);
pairingCommissioner->EstablishSession(ctx.GetSecureSessionManager(),
Transport::PeerAddress(Transport::Type::kBle), fabric, Node01_01,
contextCommissioner, nullptr, &delegateCommissioner) == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite, gLoopback.mSentMessageCount == 5);
NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 1);

// Validate that secure session can be created after the pairing
SessionHolder sessionHolder;
NL_TEST_ASSERT(inSuite,
ctx.GetSecureSessionManager().NewPairing(sessionHolder, NullOptional, Node01_01, pairingCommissioner,
CryptoContext::SessionRole::kInitiator,
gCommissionerFabricIndex) == CHIP_NO_ERROR);

auto * pairingCommissioner1 = chip::Platform::New<CASESession>();
pairingCommissioner1->SetGroupDataProvider(&gCommissionerGroupDataProvider);
ExchangeContext * contextCommissioner1 = ctx.NewUnauthenticatedExchangeToBob(pairingCommissioner1);

NL_TEST_ASSERT(inSuite,
pairingCommissioner1->EstablishSession(sessionManager, Transport::PeerAddress(Transport::Type::kBle), fabric,
Node01_01, contextCommissioner1, nullptr,
&delegateCommissioner) == CHIP_NO_ERROR);
pairingCommissioner1->EstablishSession(ctx.GetSecureSessionManager(),
Transport::PeerAddress(Transport::Type::kBle), fabric, Node01_01,
contextCommissioner1, nullptr, &delegateCommissioner) == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();

chip::Platform::Delete(pairingCommissioner);
Expand Down

0 comments on commit 2433817

Please sign in to comment.