Skip to content

Commit 98bf9d5

Browse files
kkasperczyk-nokkasperczyk-no
committed
Fix max timeout for open commissioning window
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
1 parent 83d345e commit 98bf9d5

File tree

8 files changed

+32
-29
lines changed

8 files changed

+32
-29
lines changed

‎examples/all-clusters-app/esp32/main/DeviceWithDisplay.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ class SetupListModel : public TouchesMatterStackModel
604604
{
605605
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
606606
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
607-
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
607+
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
608608
CommissioningWindowAdvertisement::kDnssdOnly);
609609
}
610610
}

‎examples/all-clusters-minimal-app/esp32/main/DeviceWithDisplay.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ class SetupListModel : public TouchesMatterStackModel
450450
{
451451
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
452452
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
453-
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
453+
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
454454
CommissioningWindowAdvertisement::kDnssdOnly);
455455
}
456456
}

‎examples/lighting-app/esp32/main/DeviceWithDisplay.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class SetupListModel : public TouchesMatterStackModel
141141
{
142142
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
143143
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
144-
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
144+
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
145145
CommissioningWindowAdvertisement::kDnssdOnly);
146146
}
147147
}

‎examples/platform/asr/shell/matter_shell.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ void asr_matter_reset(Reset_t type)
6767
{
6868
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
6969
auto & commissionMgr = chip::Server::GetInstance().GetCommissioningWindowManager();
70-
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
70+
commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
7171
CommissioningWindowAdvertisement::kDnssdOnly);
7272
}
7373
}

‎src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp

+10-11
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
9090
auto & discriminator = commandData.discriminator;
9191
auto & iterations = commandData.iterations;
9292
auto & salt = commandData.salt;
93+
CHIP_ERROR err = CHIP_NO_ERROR;
9394

9495
Optional<StatusCode> status = Optional<StatusCode>::Missing();
9596
Status globalStatus = Status::Success;
@@ -110,14 +111,12 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
110111
VerifyOrExit(iterations <= kSpake2p_Max_PBKDF_Iterations, status.Emplace(StatusCode::kPAKEParameterError));
111112
VerifyOrExit(salt.size() >= kSpake2p_Min_PBKDF_Salt_Length, status.Emplace(StatusCode::kPAKEParameterError));
112113
VerifyOrExit(salt.size() <= kSpake2p_Max_PBKDF_Salt_Length, status.Emplace(StatusCode::kPAKEParameterError));
113-
VerifyOrExit(commissioningTimeout <= commissionMgr.MaxCommissioningTimeout(), globalStatus = Status::InvalidCommand);
114-
VerifyOrExit(commissioningTimeout >= commissionMgr.MinCommissioningTimeout(), globalStatus = Status::InvalidCommand);
115114
VerifyOrExit(discriminator <= kMaxDiscriminatorValue, globalStatus = Status::InvalidCommand);
116-
117115
VerifyOrExit(verifier.Deserialize(pakeVerifier) == CHIP_NO_ERROR, status.Emplace(StatusCode::kPAKEParameterError));
118-
VerifyOrExit(commissionMgr.OpenEnhancedCommissioningWindow(commissioningTimeout, discriminator, verifier, iterations, salt,
119-
fabricIndex, fabricInfo->GetVendorId()) == CHIP_NO_ERROR,
120-
status.Emplace(StatusCode::kPAKEParameterError));
116+
err = commissionMgr.OpenEnhancedCommissioningWindow(commissioningTimeout, discriminator, verifier, iterations, salt,
117+
fabricIndex, fabricInfo->GetVendorId());
118+
VerifyOrExit(err != CHIP_ERROR_INVALID_ARGUMENT, globalStatus = Status::InvalidCommand);
119+
VerifyOrExit(err == CHIP_NO_ERROR, status.Emplace(StatusCode::kPAKEParameterError));
121120
ChipLogProgress(Zcl, "Commissioning window is now open");
122121

123122
exit:
@@ -153,16 +152,16 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
153152
const FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(fabricIndex);
154153
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
155154
auto & commissionMgr = Server::GetInstance().GetCommissioningWindowManager();
155+
CHIP_ERROR err = CHIP_NO_ERROR;
156156

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

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

168167
exit:

‎src/app/server/CommissioningWindowManager.cpp

+7-5
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,9 @@ void CommissioningWindowManager::OnSessionEstablished(const SessionHandle & sess
242242
}
243243
}
244244

245-
CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow(Seconds32 commissioningTimeout)
245+
CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow(Seconds32 commissioningTimeout, bool isBle)
246246
{
247-
VerifyOrReturnError(commissioningTimeout <= MaxCommissioningTimeout() && commissioningTimeout >= MinCommissioningTimeout(),
247+
VerifyOrReturnError(commissioningTimeout <= MaxCommissioningTimeout(isBle) && commissioningTimeout >= MinCommissioningTimeout(),
248248
CHIP_ERROR_INVALID_ARGUMENT);
249249
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
250250
VerifyOrReturnError(failSafeContext.IsFailSafeFullyDisarmed(), CHIP_ERROR_INCORRECT_STATE);
@@ -306,11 +306,13 @@ CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(Seconds32 co
306306
CommissioningWindowAdvertisement advertisementMode)
307307
{
308308
RestoreDiscriminator();
309+
bool isBle = false;
309310

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

320322
mUseECM = false;
321323

322-
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout);
324+
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout, isBle);
323325
if (err != CHIP_NO_ERROR)
324326
{
325327
Cleanup();
@@ -362,7 +364,7 @@ CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(Seconds32
362364

363365
mUseECM = true;
364366

365-
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout);
367+
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout, false);
366368
if (err != CHIP_NO_ERROR)
367369
{
368370
Cleanup();

‎src/app/server/CommissioningWindowManager.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,17 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
5858
return CHIP_NO_ERROR;
5959
}
6060

61-
static constexpr System::Clock::Seconds32 MaxCommissioningTimeout()
61+
static constexpr System::Clock::Seconds32 MaxCommissioningTimeout(bool isBle)
6262
{
6363
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
64-
// Specification section 2.3.1 - Extended Announcement Duration up to 48h
65-
return System::Clock::Seconds32(60 * 60 * 48);
66-
#else
64+
if (isBle)
65+
{
66+
// Specification section 2.3.1 - Extended Announcement Duration up to 48h
67+
return System::Clock::Seconds32(60 * 60 * 48);
68+
}
69+
#endif
6770
// Specification section 5.4.2.3. Announcement Duration says 15 minutes.
6871
return System::Clock::Seconds32(15 * 60);
69-
#endif
7072
}
7173

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

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

151153
// Start advertising and listening for PASE connections. Should only be
152154
// called when a commissioning window timeout timer is running.

‎src/app/tests/TestCommissioningWindowManager.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ void CheckCommissioningWindowManagerBasicWindowOpenCloseTask(intptr_t context)
156156

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

159-
EXPECT_EQ(commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(),
159+
EXPECT_EQ(commissionMgr.OpenBasicCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false),
160160
CommissioningWindowAdvertisement::kDnssdOnly),
161161
CHIP_NO_ERROR);
162162
EXPECT_TRUE(commissionMgr.IsCommissioningWindowOpen());
@@ -194,7 +194,7 @@ void CheckCommissioningWindowManagerBasicWindowOpenCloseFromClusterTask(intptr_t
194194
constexpr auto fabricIndex = static_cast<chip::FabricIndex>(1);
195195
constexpr auto vendorId = static_cast<chip::VendorId>(0xFFF3);
196196
EXPECT_EQ(commissionMgr.OpenBasicCommissioningWindowForAdministratorCommissioningCluster(
197-
commissionMgr.MaxCommissioningTimeout(), fabricIndex, vendorId),
197+
commissionMgr.MaxCommissioningTimeout(false), fabricIndex, vendorId),
198198
CHIP_NO_ERROR);
199199
EXPECT_TRUE(commissionMgr.IsCommissioningWindowOpen());
200200
EXPECT_EQ(commissionMgr.CommissioningWindowStatusForCluster(),
@@ -350,7 +350,7 @@ void CheckCommissioningWindowManagerEnhancedWindowTask(intptr_t context)
350350

351351
constexpr auto fabricIndex = static_cast<chip::FabricIndex>(1);
352352
constexpr auto vendorId = static_cast<chip::VendorId>(0xFFF3);
353-
EXPECT_EQ(commissionMgr.OpenEnhancedCommissioningWindow(commissionMgr.MaxCommissioningTimeout(), newDiscriminator, verifier,
353+
EXPECT_EQ(commissionMgr.OpenEnhancedCommissioningWindow(commissionMgr.MaxCommissioningTimeout(false), newDiscriminator, verifier,
354354
kIterations, saltData, fabricIndex, vendorId),
355355
CHIP_NO_ERROR);
356356
EXPECT_TRUE(commissionMgr.IsCommissioningWindowOpen());

0 commit comments

Comments
 (0)