Skip to content

Commit de624ce

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. Added isBle flag to be able to determine what max timeout should be used for the particular scenario. Fixes: project-chip#35505
1 parent 83d345e commit de624ce

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)