Skip to content

Commit

Permalink
Ensure that a PASE session doesn't outlive the fail-safe (and vice ve…
Browse files Browse the repository at this point in the history
…rsa). (#19636)

Per spec, when a fail-safe expires step 1 is:

> Terminate any open PASE secure session by clearing any associated Secure
> Session Context at the Server.

We were not doing that.

Similarly, per spec if a PASE session is terminated by CloseSession, it should
expire the pending fail-safe, if any.  We weren't doing that either.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 16, 2023
1 parent eba5bac commit 5302409
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 8 deletions.
49 changes: 43 additions & 6 deletions src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,18 @@ void CommissioningWindowManager::OnPlatformEvent(const DeviceLayer::ChipDeviceEv
mCommissioningTimeoutTimerArmed = false;
Cleanup();
mServer->GetSecureSessionManager().ExpireAllPASEPairings();
// That should have cleared out mPASESession.
#if CONFIG_NETWORK_LAYER_BLE
mServer->GetBleLayerObject()->CloseAllBleConnections();
#endif
}
else if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
{
ChipLogError(AppServer, "Failsafe timer expired");
if (mPASESession)
{
mPASESession->AsSecureSession()->MarkForRemoval();
}
HandleFailedAttempt(CHIP_ERROR_TIMEOUT);
}
else if (event->Type == DeviceLayer::DeviceEventType::kOperationalNetworkEnabled)
Expand Down Expand Up @@ -98,11 +103,7 @@ void CommissioningWindowManager::ResetState()
void CommissioningWindowManager::Cleanup()
{
StopAdvertisement(/* aShuttingDown = */ false);
DeviceLayer::FailSafeContext & failSafeContext = DeviceLayer::DeviceControlServer::DeviceControlSvr().GetFailSafeContext();
if (failSafeContext.IsFailSafeArmed())
{
failSafeContext.ForceFailSafeTimerExpiry();
}
ExpireFailSafeIfArmed();

ResetState();
}
Expand Down Expand Up @@ -164,20 +165,31 @@ void CommissioningWindowManager::OnSessionEstablished(const SessionHandle & sess
DeviceLayer::FailSafeContext & failSafeContext = DeviceLayer::DeviceControlServer::DeviceControlSvr().GetFailSafeContext();
// This should never be armed because we don't allow CASE sessions to arm the failsafe when the commissioning window is open and
// we check that the failsafe is not armed before opening the commissioning window. None the less, it is good to double-check.
CHIP_ERROR err = CHIP_NO_ERROR;
if (failSafeContext.IsFailSafeArmed())
{
ChipLogError(AppServer, "Error - arm failsafe is already armed on PASE session establishment completion");
}
else
{
CHIP_ERROR err = failSafeContext.ArmFailSafe(kUndefinedFabricId, System::Clock::Seconds16(60));
err = failSafeContext.ArmFailSafe(kUndefinedFabricId, System::Clock::Seconds16(60));
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "Error arming failsafe on PASE session establishment completion");
// Don't allow a PASE session to hang around without a fail-safe.
session->AsSecureSession()->MarkForRemoval();
HandleFailedAttempt(err);
}
}

ChipLogProgress(AppServer, "Device completed Rendezvous process");

if (err == CHIP_NO_ERROR)
{
// When the now-armed fail-safe is disarmed or expires it will handle
// clearing out mPASESession.
mPASESession.Grab(session);
}
}

CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow(Seconds16 commissioningTimeout)
Expand Down Expand Up @@ -445,4 +457,29 @@ void CommissioningWindowManager::HandleCommissioningWindowTimeout(chip::System::
commissionMgr->CloseCommissioningWindow();
}

void CommissioningWindowManager::OnSessionReleased()
{
// The PASE session has died, probably due to CloseSession. Immediately
// expire the fail-safe, if it's still armed (which it might not be if the
// PASE session is being released due to the fail-safe expiring or being
// disarmed).
//
// Expiring the fail-safe will make us start listening for new PASE sessions
// as needed.
//
// Note that at this point the fail-safe _must_ be associated with our PASE
// session, since we arm it when the PASE session is set up, and anything
// that disarms the fail-safe would also tear down the PASE session.
ExpireFailSafeIfArmed();
}

void CommissioningWindowManager::ExpireFailSafeIfArmed()
{
DeviceLayer::FailSafeContext & failSafeContext = DeviceLayer::DeviceControlServer::DeviceControlSvr().GetFailSafeContext();
if (failSafeContext.IsFailSafeArmed())
{
failSafeContext.ForceFailSafeTimerExpiry();
}
}

} // namespace chip
17 changes: 15 additions & 2 deletions src/app/server/CommissioningWindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ enum class CommissioningWindowAdvertisement

class Server;

class CommissioningWindowManager : public SessionEstablishmentDelegate, public app::CommissioningModeProvider
class CommissioningWindowManager : public SessionEstablishmentDelegate,
public app::CommissioningModeProvider,
public SessionDelegate
{
public:
CommissioningWindowManager() {}
CommissioningWindowManager() : mPASESession(*this) {}

CHIP_ERROR Init(Server * server)
{
Expand Down Expand Up @@ -97,6 +99,9 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate, public a
void OverrideMinCommissioningTimeout(System::Clock::Seconds16 timeout) { mMinCommissioningTimeoutOverride.SetValue(timeout); }

private:
//////////// SessionDelegate Implementation ///////////////
void OnSessionReleased() override;

void SetBLE(bool ble) { mIsBLE = ble; }

CHIP_ERROR SetTemporaryDiscriminator(uint16_t discriminator);
Expand Down Expand Up @@ -140,6 +145,11 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate, public a
*/
static void HandleCommissioningWindowTimeout(chip::System::Layer * aSystemLayer, void * aAppState);

/**
* Helper to immediately expire the fail-safe if it's currently armed.
*/
void ExpireFailSafeIfArmed();

AppDelegate * mAppDelegate = nullptr;
Server * mServer = nullptr;

Expand Down Expand Up @@ -167,6 +177,9 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate, public a
// For tests only, so that we can test the commissioning window timeout
// without having to wait 3 minutes.
Optional<System::Clock::Seconds16> mMinCommissioningTimeoutOverride;

// The PASE session we are using, so we can handle CloseSession properly.
SessionHolderWithDelegate mPASESession;
};

} // namespace chip

0 comments on commit 5302409

Please sign in to comment.