Skip to content

Commit

Permalink
Fix max timeout for open commissioning window
Browse files Browse the repository at this point in the history 
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Fixes: project-chip#35505
  • Loading branch information
@kkasperczyk-no
kkasperczyk-no committed Sep 10, 2024
1 parent 83d345e commit 98bf9d5
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 29 deletions.
2 changes: 1 addition & 1 deletion examples/all-clusters-app/esp32/main/DeviceWithDisplay.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ class SetupListModel : public TouchesMatterStackModel
{
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
CommissioningWindowAdvertisement::kDnssdOnly);
}
}
Expand Down
2 changes: 1 addition & 1 deletion examples/all-clusters-minimal-app/esp32/main/DeviceWithDisplay.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ class SetupListModel : public TouchesMatterStackModel
{
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
CommissioningWindowAdvertisement::kDnssdOnly);
}
}
Expand Down
2 changes: 1 addition & 1 deletion examples/lighting-app/esp32/main/DeviceWithDisplay.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class SetupListModel : public TouchesMatterStackModel
{
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
CommissioningWindowAdvertisement::kDnssdOnly);
}
}
Expand Down
2 changes: 1 addition & 1 deletion examples/platform/asr/shell/matter_shell.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void asr_matter_reset(Reset_t type)
{
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
CommissioningWindowAdvertisement::kDnssdOnly);
}
}
Expand Down
21 changes: 10 additions & 11 deletions src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
auto & discriminator = commandData.discriminator;
auto & iterations = commandData.iterations;
auto & salt = commandData.salt;
CHIP_ERROR err = CHIP_NO_ERROR;

Optional<StatusCode> status = Optional<StatusCode>::Missing();
Status globalStatus = Status::Success;
Expand All @@ -110,14 +111,12 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
VerifyOrExit(iterations <= kSpake2p_Max_PBKDF_Iterations, status.Emplace(StatusCode::kPAKEParameterError));
VerifyOrExit(salt.size() >= kSpake2p_Min_PBKDF_Salt_Length, status.Emplace(StatusCode::kPAKEParameterError));
VerifyOrExit(salt.size() <= kSpake2p_Max_PBKDF_Salt_Length, status.Emplace(StatusCode::kPAKEParameterError));
VerifyOrExit(commissioningTimeout <= commissionMgr.MaxCommissioningTimeout(), globalStatus = Status::InvalidCommand);
VerifyOrExit(commissioningTimeout >= commissionMgr.MinCommissioningTimeout(), globalStatus = Status::InvalidCommand);
VerifyOrExit(discriminator <= kMaxDiscriminatorValue, globalStatus = Status::InvalidCommand);

VerifyOrExit(verifier.Deserialize(pakeVerifier) == CHIP_NO_ERROR, status.Emplace(StatusCode::kPAKEParameterError));
VerifyOrExit(commissionMgr.OpenEnhancedCommissioningWindow(commissioningTimeout, discriminator, verifier, iterations, salt,
fabricIndex, fabricInfo->GetVendorId()) == CHIP_NO_ERROR,
status.Emplace(StatusCode::kPAKEParameterError));
err = commissionMgr.OpenEnhancedCommissioningWindow(commissioningTimeout, discriminator, verifier, iterations, salt,
fabricIndex, fabricInfo->GetVendorId());
VerifyOrExit(err != CHIP_ERROR_INVALID_ARGUMENT, globalStatus = Status::InvalidCommand);
VerifyOrExit(err == CHIP_NO_ERROR, status.Emplace(StatusCode::kPAKEParameterError));
ChipLogProgress(Zcl, "Commissioning window is now open");

exit:
Expand Down Expand Up @@ -153,16 +152,16 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
const FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(fabricIndex);
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
auto & commissionMgr = Server::GetInstance().GetCommissioningWindowManager();
CHIP_ERROR err = CHIP_NO_ERROR;

VerifyOrExit(fabricInfo != nullptr, status.Emplace(StatusCode::kPAKEParameterError));

VerifyOrExit(!commissionMgr.IsCommissioningWindowOpen(), status.Emplace(StatusCode::kBusy));
VerifyOrExit(failSafeContext.IsFailSafeFullyDisarmed(), status.Emplace(StatusCode::kBusy));
VerifyOrExit(commissioningTimeout <= commissionMgr.MaxCommissioningTimeout(), globalStatus = Status::InvalidCommand);
VerifyOrExit(commissioningTimeout >= commissionMgr.MinCommissioningTimeout(), globalStatus = Status::InvalidCommand);
VerifyOrExit(commissionMgr.OpenBasicCommissioningWindowForAdministratorCommissioningCluster(
commissioningTimeout, fabricIndex, fabricInfo->GetVendorId()) == CHIP_NO_ERROR,
status.Emplace(StatusCode::kPAKEParameterError));
err = commissionMgr.OpenBasicCommissioningWindowForAdministratorCommissioningCluster(commissioningTimeout, fabricIndex,
fabricInfo->GetVendorId());
VerifyOrExit(err != CHIP_ERROR_INVALID_ARGUMENT, globalStatus = Status::InvalidCommand);
VerifyOrExit(err == CHIP_NO_ERROR, status.Emplace(StatusCode::kPAKEParameterError));
ChipLogProgress(Zcl, "Commissioning window is now open");

exit:
Expand Down
12 changes: 7 additions & 5 deletions src/app/server/CommissioningWindowManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ void CommissioningWindowManager::OnSessionEstablished(const SessionHandle & sess
}
}

CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow(Seconds32 commissioningTimeout)
CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow(Seconds32 commissioningTimeout, bool isBle)
{
VerifyOrReturnError(commissioningTimeout <= MaxCommissioningTimeout() && commissioningTimeout >= MinCommissioningTimeout(),
VerifyOrReturnError(commissioningTimeout <= MaxCommissioningTimeout(isBle) && commissioningTimeout >= MinCommissioningTimeout(),
CHIP_ERROR_INVALID_ARGUMENT);
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
VerifyOrReturnError(failSafeContext.IsFailSafeFullyDisarmed(), CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -306,11 +306,13 @@ CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(Seconds32 co
CommissioningWindowAdvertisement advertisementMode)
{
RestoreDiscriminator();
bool isBle = false;

#if CONFIG_NETWORK_LAYER_BLE
// Enable BLE advertisements if commissioning window is to be opened on all supported
// transports, and BLE is supported on the current device.
SetBLE(advertisementMode == chip::CommissioningWindowAdvertisement::kAllSupported);
isBle = advertisementMode == chip::CommissioningWindowAdvertisement::kAllSupported;
SetBLE(isBle);
#else
SetBLE(false);
#endif // CONFIG_NETWORK_LAYER_BLE
Expand All @@ -319,7 +321,7 @@ CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(Seconds32 co

mUseECM = false;

CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout);
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout, isBle);
if (err != CHIP_NO_ERROR)
{
Cleanup();
Expand Down Expand Up @@ -362,7 +364,7 @@ CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(Seconds32

mUseECM = true;

CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout);
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout, false);
if (err != CHIP_NO_ERROR)
{
Cleanup();
Expand Down
14 changes: 8 additions & 6 deletions src/app/server/CommissioningWindowManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,17 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
return CHIP_NO_ERROR;
}

static constexpr System::Clock::Seconds32 MaxCommissioningTimeout()
static constexpr System::Clock::Seconds32 MaxCommissioningTimeout(bool isBle)
{
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
// Specification section 2.3.1 - Extended Announcement Duration up to 48h
return System::Clock::Seconds32(60 * 60 * 48);
#else
if (isBle)
{
// Specification section 2.3.1 - Extended Announcement Duration up to 48h
return System::Clock::Seconds32(60 * 60 * 48);
}
#endif
// Specification section 5.4.2.3. Announcement Duration says 15 minutes.
return System::Clock::Seconds32(15 * 60);
#endif
}

System::Clock::Seconds32 MinCommissioningTimeout() const
Expand Down Expand Up @@ -146,7 +148,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,

// Start a timer that will call HandleCommissioningWindowTimeout, and then
// start advertising and listen for PASE.
CHIP_ERROR OpenCommissioningWindow(System::Clock::Seconds32 commissioningTimeout);
CHIP_ERROR OpenCommissioningWindow(System::Clock::Seconds32 commissioningTimeout, bool isBle);

// Start advertising and listening for PASE connections. Should only be
// called when a commissioning window timeout timer is running.
Expand Down
6 changes: 3 additions & 3 deletions src/app/tests/TestCommissioningWindowManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ void CheckCommissioningWindowManagerBasicWindowOpenCloseTask(intptr_t context)

CommissioningWindowManager & commissionMgr = Server::GetInstance().GetCommissioningWindowManager();

EXPECT_EQ(commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
EXPECT_EQ(commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
CommissioningWindowAdvertisement::kDnssdOnly),
CHIP_NO_ERROR);
EXPECT_TRUE(commissionMgr.IsCommissioningWindowOpen());
Expand Down Expand Up @@ -194,7 +194,7 @@ void CheckCommissioningWindowManagerBasicWindowOpenCloseFromClusterTask(intptr_t
constexpr auto fabricIndex = static_cast<chip::FabricIndex>(1);
constexpr auto vendorId = static_cast<chip::VendorId>(0xFFF3);
EXPECT_EQ(commissionMgr.OpenBasicCommissioningWindowForAdministratorCommissioningCluster(
commissionMgr.MaxCommissioningTimeout(), fabricIndex, vendorId),
commissionMgr.MaxCommissioningTimeout(false), fabricIndex, vendorId),
CHIP_NO_ERROR);
EXPECT_TRUE(commissionMgr.IsCommissioningWindowOpen());
EXPECT_EQ(commissionMgr.CommissioningWindowStatusForCluster(),
Expand Down Expand Up @@ -350,7 +350,7 @@ void CheckCommissioningWindowManagerEnhancedWindowTask(intptr_t context)

constexpr auto fabricIndex = static_cast<chip::FabricIndex>(1);
constexpr auto vendorId = static_cast<chip::VendorId>(0xFFF3);
EXPECT_EQ(commissionMgr.OpenEnhancedCommissioningWindow(commissionMgr.MaxCommissioningTimeout(), newDiscriminator, verifier,
EXPECT_EQ(commissionMgr.OpenEnhancedCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false), newDiscriminator, verifier,
kIterations, saltData, fabricIndex, vendorId),
CHIP_NO_ERROR);
EXPECT_TRUE(commissionMgr.IsCommissioningWindowOpen());
Expand Down

0 comments on commit 98bf9d5

Please sign in to comment.