Skip to content

Commit

Permalink
Fix management of the commissioning window timeout. (#16692)
Browse files Browse the repository at this point in the history 
Fixes #16689

Fixes #16548
(the fact that most of our example apps by default started a
commissioning window with no timeout at all).

Fixes non-spec-compliant status codes and the lack of a check for the
3 minute minimum on a commissioning timeout window in the
Administrator Commissioning cluster.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Apr 12, 2022
1 parent 854b757 commit 1209264
Show file tree
Hide file tree
Showing 12 changed files with 430 additions and 184 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 @@ -392,7 +392,7 @@ class SetupListModel : public TouchesMatterStackModel
{
chip::Server::GetInstance().GetFabricTable().DeleteAllFabrics();
chip::Server::GetInstance().GetCommissioningWindowManager().OpenBasicCommissioningWindow(
kNoCommissioningTimeout, CommissioningWindowAdvertisement::kDnssdOnly);
CommissioningWindowManager::MaxCommissioningTimeout(), CommissioningWindowAdvertisement::kDnssdOnly);
}
}

Expand Down
13 changes: 7 additions & 6 deletions examples/tv-casting-app/linux/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <platform/ConfigurationManager.h>
#include <platform/DeviceControlServer.h>
#include <platform/TestOnlyCommissionableDataProvider.h>
#include <system/SystemClock.h>
#include <system/SystemLayer.h>
#include <transport/raw/PeerAddress.h>
#include <zap-generated/CHIPClusters.h>
Expand All @@ -56,11 +57,11 @@ struct TVExampleDeviceType
uint16_t id;
};

constexpr TVExampleDeviceType kKnownDeviceTypes[] = { { "video-player", 35 }, { "dimmable-light", 257 } };
constexpr int kKnownDeviceTypesCount = sizeof kKnownDeviceTypes / sizeof *kKnownDeviceTypes;
constexpr uint16_t kOptionDeviceType = 't';
constexpr uint16_t kCommissioningWindowTimeoutInSec = 3 * 60;
constexpr uint32_t kCommissionerDiscoveryTimeoutInMs = 5 * 1000;
constexpr TVExampleDeviceType kKnownDeviceTypes[] = { { "video-player", 35 }, { "dimmable-light", 257 } };
constexpr int kKnownDeviceTypesCount = sizeof kKnownDeviceTypes / sizeof *kKnownDeviceTypes;
constexpr uint16_t kOptionDeviceType = 't';
constexpr System::Clock::Seconds16 kCommissioningWindowTimeout = System::Clock::Seconds16(3 * 60);
constexpr uint32_t kCommissionerDiscoveryTimeoutInMs = 5 * 1000;

// TODO: Accept these values over CLI
const char * kContentUrl = "https://www.test.com/videoid";
Expand Down Expand Up @@ -143,7 +144,7 @@ void PrepareForCommissioning(const Dnssd::DiscoveredNodeData * selectedCommissio
Server::GetInstance().Init();
Server::GetInstance().GetFabricTable().DeleteAllFabrics();
ReturnOnFailure(
Server::GetInstance().GetCommissioningWindowManager().OpenBasicCommissioningWindow(kCommissioningWindowTimeoutInSec));
Server::GetInstance().GetCommissioningWindowManager().OpenBasicCommissioningWindow(kCommissioningWindowTimeout));

// Display onboarding payload
chip::DeviceLayer::ConfigurationMgr().LogDeviceConfig();
Expand Down
57 changes: 36 additions & 21 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 @@ -24,21 +24,22 @@
#include <app/AttributeAccessInterface.h>
#include <app/CommandHandler.h>
#include <app/ConcreteCommandPath.h>
#include <app/server/CommissioningWindowManager.h>
#include <app/server/Server.h>
#include <app/util/af.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
#include <platform/CommissionableDataProvider.h>
#include <protocols/interaction_model/Constants.h>
#include <setup_payload/SetupPayload.h>
#include <system/SystemClock.h>

using namespace chip;
using namespace chip::app;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::AdministratorCommissioning;

// Specifications section 5.4.2.3. Announcement Duration
constexpr uint32_t kMaxCommissionioningTimeoutSeconds = 15 * 60;
using namespace chip::Protocols;

class AdministratorCommissioningAttrAccess : public AttributeAccessInterface
{
Expand Down Expand Up @@ -91,13 +92,14 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
const Commands::OpenCommissioningWindow::DecodableType & commandData)
{
auto & commissioningTimeout = commandData.commissioningTimeout;
auto & pakeVerifier = commandData.PAKEVerifier;
auto & discriminator = commandData.discriminator;
auto & iterations = commandData.iterations;
auto & salt = commandData.salt;

Optional<StatusCode> status = Optional<StatusCode>::Missing();
auto commissioningTimeout = System::Clock::Seconds16(commandData.commissioningTimeout);
auto & pakeVerifier = commandData.PAKEVerifier;
auto & discriminator = commandData.discriminator;
auto & iterations = commandData.iterations;
auto & salt = commandData.salt;

Optional<StatusCode> status = Optional<StatusCode>::Missing();
InteractionModel::Status globalStatus = InteractionModel::Status::Success;
Spake2pVerifier verifier;

ChipLogProgress(Zcl, "Received command to open commissioning window");
Expand All @@ -117,9 +119,11 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
VerifyOrExit(salt.size() <= kSpake2p_Max_PBKDF_Salt_Length,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
VerifyOrExit(discriminator <= kMaxDiscriminatorValue, status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
VerifyOrExit(commissioningTimeout <= CommissioningWindowManager::MaxCommissioningTimeout(),
globalStatus = InteractionModel::Status::InvalidCommand);
VerifyOrExit(commissioningTimeout >= CommissioningWindowManager::MinCommissioningTimeout(),
globalStatus = InteractionModel::Status::InvalidCommand);
VerifyOrExit(discriminator <= kMaxDiscriminatorValue, globalStatus = InteractionModel::Status::InvalidCommand);

VerifyOrExit(verifier.Deserialize(pakeVerifier) == CHIP_NO_ERROR,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
Expand All @@ -134,12 +138,16 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
exit:
if (status.HasValue())
{
ChipLogError(Zcl, "Failed to open commissioning window. Status %d", status.Value());
ChipLogError(Zcl, "Failed to open commissioning window. Cluster status %d", status.Value());
commandObj->AddClusterSpecificFailure(commandPath, status.Value());
}
else
{
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS);
if (globalStatus != InteractionModel::Status::Success)
{
ChipLogError(Zcl, "Failed to open commissioning window. Global status %d", to_underlying(globalStatus));
}
commandObj->AddStatus(commandPath, globalStatus);
}
return true;
}
Expand All @@ -148,9 +156,10 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
const Commands::OpenBasicCommissioningWindow::DecodableType & commandData)
{
auto & commissioningTimeout = commandData.commissioningTimeout;
auto commissioningTimeout = System::Clock::Seconds16(commandData.commissioningTimeout);

Optional<StatusCode> status = Optional<StatusCode>::Missing();
Optional<StatusCode> status = Optional<StatusCode>::Missing();
InteractionModel::Status globalStatus = InteractionModel::Status::Success;
ChipLogProgress(Zcl, "Received command to open basic commissioning window");

FabricIndex fabricIndex = commandObj->GetAccessingFabricIndex();
Expand All @@ -160,8 +169,10 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
VerifyOrExit(Server::GetInstance().GetCommissioningWindowManager().CommissioningWindowStatus() ==
CommissioningWindowStatus::kWindowNotOpen,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY));
VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
VerifyOrExit(commissioningTimeout <= CommissioningWindowManager::MaxCommissioningTimeout(),
globalStatus = InteractionModel::Status::InvalidCommand);
VerifyOrExit(commissioningTimeout >= CommissioningWindowManager::MinCommissioningTimeout(),
globalStatus = InteractionModel::Status::InvalidCommand);
VerifyOrExit(Server::GetInstance().GetCommissioningWindowManager().OpenBasicCommissioningWindow(
commissioningTimeout, CommissioningWindowAdvertisement::kDnssdOnly) == CHIP_NO_ERROR,
status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR));
Expand All @@ -173,12 +184,16 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
exit:
if (status.HasValue())
{
ChipLogError(Zcl, "Failed to open commissioning window. Status %d", status.Value());
ChipLogError(Zcl, "Failed to open commissioning window. Cluster status %d", status.Value());
commandObj->AddClusterSpecificFailure(commandPath, status.Value());
}
else
{
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS);
if (globalStatus != InteractionModel::Status::Success)
{
ChipLogError(Zcl, "Failed to open commissioning window. Global status %d", to_underlying(globalStatus));
}
commandObj->AddStatus(commandPath, globalStatus);
}
return true;
}
Expand Down
63 changes: 39 additions & 24 deletions src/app/server/CommissioningWindowManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,13 @@
#include <platform/CommissionableDataProvider.h>

using namespace chip::app::Clusters;
using namespace chip::System::Clock;

namespace {

// As per specifications (Section 13.3), Nodes SHALL exit commissioning mode after 20 failed commission attempts.
constexpr uint8_t kMaxFailedCommissioningAttempts = 20;

void HandleCommissioningWindowTimeout(chip::System::Layer * aSystemLayer, void * aAppState)
{
chip::CommissioningWindowManager * commissionMgr = static_cast<chip::CommissioningWindowManager *>(aAppState);
commissionMgr->CloseCommissioningWindow();
}

void HandleSessionEstablishmentTimeout(chip::System::Layer * aSystemLayer, void * aAppState)
{
chip::CommissioningWindowManager * commissionMgr = static_cast<chip::CommissioningWindowManager *>(aAppState);
Expand Down Expand Up @@ -88,6 +83,9 @@ void CommissioningWindowManager::ResetState()

memset(&mECMPASEVerifier, 0, sizeof(mECMPASEVerifier));
memset(mECMSalt, 0, sizeof(mECMSalt));

DeviceLayer::SystemLayer().CancelTimer(HandleCommissioningWindowTimeout, this);
mCommissioningTimeoutTimerArmed = false;
}

void CommissioningWindowManager::Cleanup()
Expand All @@ -108,14 +106,15 @@ void CommissioningWindowManager::OnSessionEstablishmentError(CHIP_ERROR err)
#endif
if (mFailedCommissioningAttempts < kMaxFailedCommissioningAttempts)
{
// If the number of commissioning attempts have not exceeded maximum retries, let's reopen
// the pairing window.
err = OpenCommissioningWindow();
// If the number of commissioning attempts has not exceeded maximum
// retries, let's start listening for commissioning connections again.
err = AdvertiseAndListenForPASE();
}

// If the commissioning attempts limit exceeded, or reopening the commissioning window failed.
if (err != CHIP_NO_ERROR)
{
// The commissioning attempts limit was exceeded, or listening for
// commmissioning connections failed.
Cleanup();

if (mAppDelegate != nullptr)
Expand All @@ -135,6 +134,8 @@ void CommissioningWindowManager::OnSessionEstablishmentStarted()
void CommissioningWindowManager::OnSessionEstablished()
{
DeviceLayer::SystemLayer().CancelTimer(HandleSessionEstablishmentTimeout, this);
DeviceLayer::SystemLayer().CancelTimer(HandleCommissioningWindowTimeout, this);
mCommissioningTimeoutTimerArmed = false;
SessionHolder sessionHolder;
CHIP_ERROR err = mServer->GetSecureSessionManager().NewPairing(
sessionHolder, Optional<Transport::PeerAddress>::Value(mPairingSession.GetPeerAddress()), mPairingSession.GetPeerNodeId(),
Expand All @@ -158,19 +159,28 @@ void CommissioningWindowManager::OnSessionEstablished()
ChipLogProgress(AppServer, "Device completed Rendezvous process");
}

CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow()
CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow(Seconds16 commissioningTimeout)
{
VerifyOrReturnError(commissioningTimeout <= MaxCommissioningTimeout() &&
commissioningTimeout >= mMinCommissioningTimeoutOverride.ValueOr(MinCommissioningTimeout()),
CHIP_ERROR_INVALID_ARGUMENT);

ReturnErrorOnFailure(DeviceLayer::SystemLayer().StartTimer(commissioningTimeout, HandleCommissioningWindowTimeout, this));

mCommissioningTimeoutTimerArmed = true;

return AdvertiseAndListenForPASE();
}

CHIP_ERROR CommissioningWindowManager::AdvertiseAndListenForPASE()
{
VerifyOrReturnError(mCommissioningTimeoutTimerArmed, CHIP_ERROR_INCORRECT_STATE);

uint16_t keyID = 0;
ReturnErrorOnFailure(mIDAllocator->Allocate(keyID));

mPairingSession.Clear();

if (mCommissioningTimeoutSeconds != kNoCommissioningTimeout)
{
ReturnErrorOnFailure(DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds16(mCommissioningTimeoutSeconds),
HandleCommissioningWindowTimeout, this));
}

ReturnErrorOnFailure(mServer->GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(
Protocols::SecureChannel::MsgType::PBKDFParamRequest, &mPairingSession));
mListeningForPASE = true;
Expand Down Expand Up @@ -210,7 +220,7 @@ CHIP_ERROR CommissioningWindowManager::OpenCommissioningWindow()
return CHIP_NO_ERROR;
}

CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(uint16_t commissioningTimeoutSeconds,
CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(Seconds16 commissioningTimeout,
CommissioningWindowAdvertisement advertisementMode)
{
RestoreDiscriminator();
Expand All @@ -224,11 +234,10 @@ CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(uint16_t com
#endif // CONFIG_NETWORK_LAYER_BLE

mFailedCommissioningAttempts = 0;
mCommissioningTimeoutSeconds = commissioningTimeoutSeconds;

mUseECM = false;

CHIP_ERROR err = OpenCommissioningWindow();
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout);
if (err != CHIP_NO_ERROR)
{
Cleanup();
Expand All @@ -237,7 +246,7 @@ CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(uint16_t com
return err;
}

CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(uint16_t commissioningTimeoutSeconds, uint16_t discriminator,
CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(Seconds16 commissioningTimeout, uint16_t discriminator,
Spake2pVerifier & verifier, uint32_t iterations,
ByteSpan salt)
{
Expand All @@ -251,7 +260,6 @@ CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(uint16_t
mECMSaltLength = static_cast<uint32_t>(salt.size());

mFailedCommissioningAttempts = 0;
mCommissioningTimeoutSeconds = commissioningTimeoutSeconds;

mECMDiscriminator = discriminator;
mECMIterations = iterations;
Expand All @@ -260,7 +268,7 @@ CHIP_ERROR CommissioningWindowManager::OpenEnhancedCommissioningWindow(uint16_t

mUseECM = true;

CHIP_ERROR err = OpenCommissioningWindow();
CHIP_ERROR err = OpenCommissioningWindow(commissioningTimeout);
if (err != CHIP_NO_ERROR)
{
Cleanup();
Expand Down Expand Up @@ -357,7 +365,7 @@ CHIP_ERROR CommissioningWindowManager::StopAdvertisement(bool aShuttingDown)
{
// Stop advertising commissioning mode, since we're not accepting PASE
// connections right now. If we start accepting them again (via
// OpenCommissioningWindow) that will call StartAdvertisement as needed.
// AdvertiseAndListenForPASE) that will call StartAdvertisement as needed.
app::DnssdServer::Instance().StartServer();
}

Expand All @@ -384,4 +392,11 @@ CHIP_ERROR CommissioningWindowManager::RestoreDiscriminator()
return app::DnssdServer::Instance().SetEphemeralDiscriminator(NullOptional);
}

void CommissioningWindowManager::HandleCommissioningWindowTimeout(chip::System::Layer * aSystemLayer, void * aAppState)
{
auto * commissionMgr = static_cast<CommissioningWindowManager *>(aAppState);
commissionMgr->mCommissioningTimeoutTimerArmed = false;
commissionMgr->CloseCommissioningWindow();
}

} // namespace chip
Loading

0 comments on commit 1209264

Please sign in to comment.