diff --git a/config/pw_unit_test/BUILD.gn b/config/pw_unit_test/BUILD.gn index 20889115f745..26d4dfb623b4 100644 --- a/config/pw_unit_test/BUILD.gn +++ b/config/pw_unit_test/BUILD.gn @@ -17,6 +17,7 @@ import("//build_overrides/chip.gni") import("//build_overrides/pigweed.gni") import("${chip_root}/build/chip/tests.gni") +import("${chip_root}/src/lib/core/core.gni") import("$dir_pw_build/target_types.gni") pw_source_set("define_overrides") { @@ -24,7 +25,10 @@ pw_source_set("define_overrides") { } config("define_options") { - if (chip_fake_platform && chip_link_tests) { + # pw_unit_test's light backend constructs each test fixture in a single static + # memory pool, so the pool has to fit the largest fixture in the build. Use a + # larger pool for host builds that run controller-side tests with large fixtures. + if (chip_fake_platform || chip_target_style != "embedded") { defines = [ "PW_UNIT_TEST_CONFIG_MEMORY_POOL_SIZE=65536" ] } else { defines = [ "PW_UNIT_TEST_CONFIG_MEMORY_POOL_SIZE=16384" ] diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp index a48c87e8b9b2..068f1e6c3754 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.cpp +++ b/examples/chip-tool/commands/pairing/PairingCommand.cpp @@ -62,6 +62,17 @@ namespace { // Endpoint used for the CommissioningProxy cluster when --proxy-endpoint is not given. constexpr chip::EndpointId kDefaultProxyEndpointId = 1; +// Endpoint used for the Network Identity Management cluster when --pdc-netim-endpoint-id is not given. +constexpr chip::EndpointId kDefaultNETIMEndpointId = 1; + +bool IsNoPasswordMarker(chip::ByteSpan password) +{ + // Use a one-character marker value (which is invalid in every supported Wi-Fi password encoding) + // to signal that no password is to be used. An empty string, which would otherwise be the more + // obvious choice, is used by the Network Commissioning Cluster to represent an open network. + return password.data_equal(ByteSpan::fromCharSpan("-"_span)); +} + // Upper bound on back-to-back null-Message polls that yield a message but no reply. // A conformant proxy drains in a handful; the bound only stops a misbehaving one from // spinning the commissioner. @@ -108,6 +119,17 @@ CHIP_ERROR PairingCommand::RunCommand() mDeviceIsICD = false; + if (mPDCRegistrarNodeId.HasValue()) + { + mPDCRegistrar.emplace(CurrentCommissioner(), mPDCRegistrarNodeId.Value(), + mPDCRegistrarEndpointId.ValueOr(kDefaultNETIMEndpointId)); + } + else if (IsNoPasswordMarker(mPassword)) + { + ChipLogError(chipTool, "Either a password (or '') or --pdc-netim-node-id is required"); + return CHIP_ERROR_INVALID_ARGUMENT; + } + if (mCASEAuthTags.HasValue() && mCASEAuthTags.Value().size() <= kMaxSubjectCATAttributeCount) { CATValues cats = kUndefinedCATs; @@ -197,6 +219,69 @@ CHIP_ERROR PairingCommand::RunInternal(NodeId remoteId) return err; } +void PairingCommand::Shutdown() +{ + if (mPDCRegistrar.has_value()) + { + // Release the registrar before ResetArguments() invalidates the arguments it was built from. + // Stop the pairing first, as the NetworkIdentityRegistrar contract requires: this run may have + // ended on a timeout, with commissioning still under way and the commissioner still pointing at + // the registrar. An error just means there was nothing left to stop. + RETURN_SAFELY_IGNORED CurrentCommissioner().StopPairing(mNodeId); + + // Drop the idle notification before releasing the registrar: destroying it aborts whatever + // is in flight, which would otherwise release the waiter and set an exit status from here, + // re-entering StopWaiting() while the rest of the shutdown is still running. + mPDCRegistrarIdleCallback.Cancel(); + + // Anything still in flight is a revocation the run did not last long enough to see through; + // the destructor aborts it and the commissioner reports the identity left behind. + mPDCRegistrar.reset(); + } + CHIPCommand::Shutdown(); +} + +void PairingCommand::FinishCommand(CHIP_ERROR aExitErr) +{ + // A rollback of the Network Client Identity may still be under way; the commissioner does not + // generally wait (letting it complete in the background), but we should before quitting. + VerifyOrReturn(!DeferExitForPDCRegistrar(aExitErr)); + + SetCommandExitStatus(aExitErr); +} + +bool PairingCommand::DeferExitForPDCRegistrar(CHIP_ERROR aExitErr) +{ + VerifyOrReturnValue(mPDCRegistrar.has_value() && !mPDCRegistrar->IsIdle(), false); + + ChipLogProgress(chipTool, "Waiting for the Network Client Identity revocation to complete"); + mPDCRegistrarExitErr = aExitErr; + + // Stop the registrar taking on anything new, so that the revocation in flight is all we wait for. + mPDCRegistrar->StopAcceptingRequests(); + mPDCRegistrar->WaitForIdle(&mPDCRegistrarIdleCallback); + return true; +} + +void PairingCommand::OnPDCRegistrarIdle(void * context) +{ + auto * self = static_cast(context); + self->SetCommandExitStatus(self->mPDCRegistrarExitErr); +} + +WiFiCredentials PairingCommand::GetWiFiCredentials() +{ + if (!mPDCRegistrar.has_value()) + { + return WiFiCredentials(mSSID, mPassword); + } + if (IsNoPasswordMarker(mPassword)) + { + return WiFiCredentials(mSSID, &mPDCRegistrar.value()); // PDC only + } + return WiFiCredentials(mSSID, &mPDCRegistrar.value(), mPassword); // PDC if supported +} + CommissioningParameters PairingCommand::GetCommissioningParameters() { auto params = CommissioningParameters(); @@ -209,13 +294,13 @@ CommissioningParameters PairingCommand::GetCommissioningParameters() switch (mNetworkType) { case PairingNetworkType::WiFi: - params.SetWiFiCredentials(Controller::WiFiCredentials(mSSID, mPassword)); + params.SetWiFiCredentials(GetWiFiCredentials()); break; case PairingNetworkType::Thread: params.SetThreadOperationalDataset(mOperationalDataset); break; case PairingNetworkType::WiFiOrThread: - params.SetWiFiCredentials(Controller::WiFiCredentials(mSSID, mPassword)); + params.SetWiFiCredentials(GetWiFiCredentials()); params.SetThreadOperationalDataset(mOperationalDataset); break; case PairingNetworkType::None: @@ -576,11 +661,12 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) if (mPairingMode == PairingMode::Proxy) { // Clean up the proxy session before exiting, regardless of success or failure. + // The disconnect completing is what eventually reaches FinishCommand(). SendProxyDisconnect(err); return; } - SetCommandExitStatus(err); + FinishCommand(err); } void PairingCommand::OnReadCommissioningInfo(const Controller::ReadCommissioningInfo & info) @@ -706,8 +792,7 @@ CHIP_ERROR PairingCommand::WiFiCredentialsNeeded(EndpointId endpoint) auto & commissioner = CurrentCommissioner(); CommissioningParameters params = commissioner.GetCommissioningParameters(); - auto credentials = Controller::WiFiCredentials(mSSID, mPassword); - params.SetWiFiCredentials(credentials); + params.SetWiFiCredentials(GetWiFiCredentials()); TEMPORARY_RETURN_IGNORED commissioner.UpdateCommissioningParameters(params); TEMPORARY_RETURN_IGNORED commissioner.NetworkCredentialsReady(); @@ -1138,7 +1223,7 @@ void PairingCommand::OnError(const chip::app::CommandSender * client, CHIP_ERROR { // The disconnect is best-effort; log but use the original exit status. ChipLogDetail(chipTool, "PairViaProxy: ProxyDisconnectRequest error (ignored): %" CHIP_ERROR_FORMAT, error.Format()); - SetCommandExitStatus(mProxyDisconnectExitErr); + FinishCommand(mProxyDisconnectExitErr); return; } ChipLogError(chipTool, "PairViaProxy CommandSender error: %" CHIP_ERROR_FORMAT, error.Format()); @@ -1190,12 +1275,12 @@ void PairingCommand::OnDone(chip::app::CommandSender * client) { mProxyDisconnectCmdSender.reset(); mProxySession.Release(); - SetCommandExitStatus(mProxyDisconnectExitErr); + FinishCommand(mProxyDisconnectExitErr); } } // Send ProxyDisconnectRequest to clean up the proxy session, then exit. -// SetCommandExitStatus is deferred until the response (or a timeout) is received so +// Finishing the command is deferred until the response (or a timeout) is received so // that chip-tool keeps the TCP session alive long enough for the proxy to reply. void PairingCommand::SendProxyDisconnect(CHIP_ERROR exitErr, bool aCancelPendingConnect) { @@ -1204,7 +1289,7 @@ void PairingCommand::SendProxyDisconnect(CHIP_ERROR exitErr, bool aCancelPending const bool haveSomethingToSend = aCancelPendingConnect || mProxySessionActive; if (!haveSomethingToSend || mProxyExchangeMgr == nullptr || !static_cast(mProxySession)) { - SetCommandExitStatus(exitErr); + FinishCommand(exitErr); return; } @@ -1252,7 +1337,7 @@ void PairingCommand::SendProxyDisconnect(CHIP_ERROR exitErr, bool aCancelPending { ChipLogError(chipTool, "PairViaProxy: failed to allocate CommandSender for ProxyDisconnectRequest"); mProxySession.Release(); - SetCommandExitStatus(exitErr); + FinishCommand(exitErr); return; } @@ -1261,13 +1346,13 @@ void PairingCommand::SendProxyDisconnect(CHIP_ERROR exitErr, bool aCancelPending { ChipLogError(chipTool, "PairViaProxy: failed to send ProxyDisconnectRequest"); mProxySession.Release(); - SetCommandExitStatus(exitErr); + FinishCommand(exitErr); return; } ChipLogProgress(chipTool, "PairViaProxy: sent ProxyDisconnectRequest, waiting for response"); mProxyDisconnectCmdSender = std::move(cmdSender); - // SetCommandExitStatus is deferred until OnDone/OnError fires for mProxyDisconnectCmdSender. + // Finishing the command is deferred until OnDone/OnError fires for mProxyDisconnectCmdSender. } // ProxyTransportDelegate — called by ProxyTransport when it needs to forward diff --git a/examples/chip-tool/commands/pairing/PairingCommand.h b/examples/chip-tool/commands/pairing/PairingCommand.h index 8731ae038f59..c24b2c327838 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.h +++ b/examples/chip-tool/commands/pairing/PairingCommand.h @@ -21,6 +21,7 @@ #include "../common/CHIPCommand.h" #include #include +#include #include #include @@ -80,8 +81,8 @@ class PairingCommand : public CHIPCommand, CHIPCommand(commandName, credIssuerCmds), mPairingMode(mode), mNetworkType(networkType), mFilterType(filterType), mRemoteAddr{ IPAddress::Any, chip::Inet::InterfaceId::Null() }, mComplex_TimeZones(&mTimeZoneList), - mComplex_DSTOffsets(&mDSTOffsetList), mCurrentFabricRemoveCallback(OnCurrentFabricRemove, this), - mOnProxyConnectedCallback(OnProxyDeviceConnected, this), + mComplex_DSTOffsets(&mDSTOffsetList), mPDCRegistrarIdleCallback(OnPDCRegistrarIdle, this), + mCurrentFabricRemoveCallback(OnCurrentFabricRemove, this), mOnProxyConnectedCallback(OnProxyDeviceConnected, this), mOnProxyConnectionFailureCallback(OnProxyDeviceConnectionFailed, this) { AddArgument("node-id", 0, UINT64_MAX, &mNodeId); @@ -118,6 +119,19 @@ class PairingCommand : public CHIPCommand, break; } + if (networkType == PairingNetworkType::WiFi || networkType == PairingNetworkType::WiFiOrThread) + { + AddArgument( + "pdc-netim-node-id", 0, UINT64_MAX, &mPDCRegistrarNodeId, + "Node ID on this fabric hosting a Network Identity Management cluster that can grant the commissionee " + "access to the Wi-Fi network using Per-Device Credentials (PDC). If given, PDC is used whenever the " + "commissionee supports it, and 'password' only serves as a fallback for commissionees that do not. " + "Pass \"-\" as the password to require PDC, as distinct from an empty password, which means an open network."); + AddArgument("pdc-netim-endpoint-id", 0, UINT16_MAX, &mPDCRegistrarEndpointId, + "Endpoint on the Network Infrastructure Manager hosting the Network Identity Management cluster. " + "Defaults to 1."); + } + switch (mode) { case PairingMode::None: @@ -278,6 +292,7 @@ class PairingCommand : public CHIPCommand, /////////// CHIPCommand Interface ///////// CHIP_ERROR RunCommand() override; chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(mTimeout.ValueOr(120)); } + void Shutdown() override; /////////// DevicePairingDelegate Interface ///////// void OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status) override; @@ -310,6 +325,23 @@ class PairingCommand : public CHIPCommand, CHIP_ERROR PairWithMdnsOrBleByIndexWithCode(NodeId remoteId, uint16_t index); CHIP_ERROR Unpair(NodeId remoteId); chip::Controller::CommissioningParameters GetCommissioningParameters(); + chip::Controller::WiFiCredentials GetWiFiCredentials(); + + /** + * Finishes the command with the given status, once nothing is left to wind down. Paths that + * end a pairing run go through here rather than calling SetCommandExitStatus() directly, so + * that a Network Client Identity revocation still in flight gets to complete first. + */ + void FinishCommand(CHIP_ERROR aExitErr); + + /** + * If the PDC registrar still has a Network Client Identity revocation in flight -- which the + * commissioner issues without waiting for it -- shut it down gracefully and defer exiting with + * aExitErr until it is done, so the entry does not survive us on the network. Returns true if the + * exit was deferred, in which case the caller must not set an exit status itself. + */ + bool DeferExitForPDCRegistrar(CHIP_ERROR aExitErr); + static void OnPDCRegistrarIdle(void * context); CHIP_ERROR MaybeDisplayTermsAndConditions(chip::Controller::CommissioningParameters & params); CHIP_ERROR GetMeshcopCommissionParams(chip::Controller::SetUpCodePairer::ThreadMeshcopCommissionParameters & meshcopCommissionParams); @@ -360,6 +392,15 @@ class PairingCommand : public CHIPCommand, chip::ByteSpan mOperationalDataset; chip::ByteSpan mSSID; chip::ByteSpan mPassword; + // Network Infrastructure Manager to obtain Wi-Fi Per-Device Credentials from, if any. + chip::Optional mPDCRegistrarNodeId; + chip::Optional mPDCRegistrarEndpointId; + // Built from the two arguments above by RunCommand() and released again by Shutdown(), so that + // it outlives the commissioning attempt whose CommissioningParameters point at it. + std::optional mPDCRegistrar; + chip::Callback::Callback mPDCRegistrarIdleCallback; + // Exit status to deliver once the registrar reports itself idle. + CHIP_ERROR mPDCRegistrarExitErr = CHIP_NO_ERROR; char * mOnboardingPayload = nullptr; uint64_t mDiscoveryFilterCode = 0; char * mDiscoveryFilterInstanceName = nullptr; diff --git a/examples/lighting-app/esp32/sdkconfig_pdc.defaults b/examples/lighting-app/esp32/sdkconfig_pdc.defaults new file mode 100644 index 000000000000..50150c9c54cf --- /dev/null +++ b/examples/lighting-app/esp32/sdkconfig_pdc.defaults @@ -0,0 +1,41 @@ +# +# Copyright (c) 2026 Project CHIP Authors +# All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Description: +# Overlay enabling Per-Device Credentials. Apply on top of another defaults +# file, e.g. +# idf.py -D 'SDKCONFIG_DEFAULTS=sdkconfig.defaults;sdkconfig_pdc.defaults' reconfigure +# + +CONFIG_ENABLE_WIFI_PDC=y + +# PDC associates with the AP using EAP-TLS over WPA3-Enterprise. +CONFIG_ESP_WIFI_ENTERPRISE_SUPPORT=y +CONFIG_ESP_WIFI_MBEDTLS_TLS_CLIENT=y + +# Matter requires EAP-TLS 1.3 (RFC 9190). ESP-IDF still considers this experimental, and the +# option makes every TLS user in the application prefer 1.3, not just the supplicant. +CONFIG_IDF_EXPERIMENTAL_FEATURES=y +CONFIG_ESP_WIFI_EAP_TLS1_3=y + +# Supplicant debug logging, while PDC association is still being brought up. The Kconfig option +# only compiles the wpa_printf() calls in; ESPWiFiDriver::Init() raises the "wpa" tag to DEBUG at +# runtime, which needs the compile-time ceiling lifted off INFO as well. Comment all three out to +# get the ~60 kB and the serial bandwidth back. +CONFIG_ESP_WIFI_DEBUG_PRINT=y +# CONFIG_LOG_MAXIMUM_EQUALS_DEFAULT is not set +CONFIG_LOG_MAXIMUM_LEVEL_DEBUG=y +CONFIG_ESP_CONSOLE_UART_BAUDRATE=921600 diff --git a/src/app/clusters/network-identity-management-server/BUILD.gn b/src/app/clusters/network-identity-management-server/BUILD.gn index 58bf1069dc66..3a7e2fbc8154 100644 --- a/src/app/clusters/network-identity-management-server/BUILD.gn +++ b/src/app/clusters/network-identity-management-server/BUILD.gn @@ -21,7 +21,6 @@ source_set("network-identity-management-server") { "AuthenticatorDriver.h", "DefaultNetworkIdentityStorage.cpp", "DefaultNetworkIdentityStorage.h", - "Logging.h", "NetworkAdministratorSecret.cpp", "NetworkAdministratorSecret.h", "NetworkIdentityKeystore.h", diff --git a/src/app/clusters/network-identity-management-server/Logging.h b/src/app/clusters/network-identity-management-server/Logging.h deleted file mode 100644 index 598010b8a99f..000000000000 --- a/src/app/clusters/network-identity-management-server/Logging.h +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (c) 2026 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include -#include - -/** - * @brief Logging format macro for CertificateKeyId. - * - * Usage: - * ChipLogProgress(Zcl, "Identifier: " ChipLogFormatKeyId, ChipLogValueKeyId(id)); - */ -#define ChipLogFormatKeyId "%08" PRIX32 "%08" PRIX32 "%08" PRIX32 "%08" PRIX32 "%08" PRIX32 - -/** - * @brief Logging value macro for CertificateKeyId. - * Takes a CertificateKeyId (FixedByteSpan<20>) or a value implicitly convertible to it, - * e.g. a uint8_t[20] or std::array. - * NOTE: The argument to ChipLogValueKeyId may be evaluated multiple times. - */ -#define ChipLogValueKeyId(id) \ - chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data()), \ - chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 4), \ - chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 8), \ - chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 12), \ - chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 16) - -static_assert(chip::Credentials::CertificateKeyId::size() == 20); // hard-coded in ChipLogValueKeyId diff --git a/src/app/clusters/network-identity-management-server/NetworkIdentityManagementCluster.cpp b/src/app/clusters/network-identity-management-server/NetworkIdentityManagementCluster.cpp index 1b254cf6a79c..05704d08b39a 100644 --- a/src/app/clusters/network-identity-management-server/NetworkIdentityManagementCluster.cpp +++ b/src/app/clusters/network-identity-management-server/NetworkIdentityManagementCluster.cpp @@ -17,7 +17,6 @@ #include -#include #include #include #include diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp index 028252b65850..738b9dd4cda3 100644 --- a/src/controller/AutoCommissioner.cpp +++ b/src/controller/AutoCommissioner.cpp @@ -103,6 +103,26 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam // Note that all of the copy operations use memmove() instead of memcpy(), because the caller // may be passing a modified shallow copy of our CommissioningParmeters, i.e. where various spans // already point into the buffers we're copying into, and memcpy() with overlapping buffers is UB. + // + // Note: Only the parameters that are inputs are copied from params below. Output-only + // parameters that we populate ourselves as commissioning progresses (the generated NOC chain, + // the PDC network and client identities, the attestation elements) are cleared here and left + // for the stage that produces them to set again. + // + // Dropping them is safe because the delegate only gets to call us back mid-commissioning at + // kICDGetRegistrationInfo and kNeedsNetworkCreds, both of which are past kSendNOC: the attestation + // values were consumed at kAttestationVerification, and the CSR and NOC chain by kSendNOC, while the + // PDC values are not produced until the network setup stages that follow. A retry walks the flow back + // to kScanNetworks, but CommissioningStepFinished() clears the PDC parameters before the delegate is + // asked for credentials again, so there is nothing stale to carry forward either. State that does have + // to outlive a rewrite is deliberately kept out of CommissioningParameters: mDeviceCommissioningInfo + // and the network attempt type here, and the pending Network Client Identity rollback in the + // DeviceCommissioner. + // + // Note this assignment also replaces the scalar parameters, including ones we derived from the + // commissionee ourselves. Of those only the failsafe timer is read again after a delegate callback + // (at kFailsafeBeforeWiFiEnable / kFailsafeBeforeThreadEnable), and losing it there is harmless: the + // fail-safe is already armed with the recommended value and never gets shortened. mParams = params; mParams.ClearExternalBufferDependentValues(); @@ -115,10 +135,11 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam mParams.SetThreadOperationalDataset(dataset); } - if (params.GetWiFiCredentials().HasValue()) + auto wiFiCredentialsParam = params.GetWiFiCredentials(); // optional copied by value + if (wiFiCredentialsParam.HasValue()) { - WiFiCredentials creds = params.GetWiFiCredentials().Value(); // shallow struct copy - ReturnErrorOnFailure(RelocateSpan(creds.ssid, mSsid), // + WiFiCredentials & creds = wiFiCredentialsParam.Value(); + ReturnErrorOnFailure(RelocateSpan(creds.ssid, mSsid), // ChipLogError(Controller, "WiFiCredentials.ssid is too large")); ReturnErrorOnFailure(RelocateSpan(creds.credentials, mCredentials), ChipLogError(Controller, "WiFiCredentials.credentials is too large")); @@ -164,6 +185,18 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam mParams.SetCSRNonce(ByteSpan(mCSRNonce)); } + // Unlike the CSR nonce above, we only copy a PDC possession nonce that was actually supplied; the + // fallback to a random value happens lazily in kPDCGetNetworkIdentity, i.e. only once we know we + // are going to use PDC at all. Leaving mParams without a nonce here is what signals that. + if (params.GetPDCPossessionNonce().HasValue()) + { + ByteSpan possessionNonce = params.GetPDCPossessionNonce().Value(); + ReturnErrorOnFailure(RelocateSpan(possessionNonce, mPossessionNonce, /* exactSize = */ true), + ChipLogError(Controller, "PDC possession nonce length is invalid")); + ChipLogProgress(Controller, "Setting PDC possession nonce from parameters"); + mParams.SetPDCPossessionNonce(possessionNonce); + } + if (params.GetDSTOffsets().HasValue()) { ChipLogProgress(Controller, "Setting DST offsets from parameters"); @@ -259,6 +292,14 @@ const CommissioningParameters & AutoCommissioner::GetCommissioningParameters() c return mParams; } +void AutoCommissioner::ClearPDCParameters() +{ + mParams.ClearPDCNetworkIdentity(); + mParams.ClearPDCPossessionNonce(); + mParams.ClearPDCClientIdentity(); + mParams.ClearPDCPossessionSignature(); +} + CommissioningStage AutoCommissioner::GetNextCommissioningStage(CommissioningStage currentStage, CHIP_ERROR & lastErr) { auto nextStage = GetNextCommissioningStageInternal(currentStage, lastErr); @@ -332,14 +373,32 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageNetworkSetup(Commi if (networkToUse == NetworkType::kWiFi) { - if (mParams.GetWiFiCredentials().HasValue()) + // We need credentials, request them if necessary. + auto wiFiCredentialsParam = mParams.GetWiFiCredentials(); // optional copied by value + VerifyOrReturnValue(wiFiCredentialsParam.HasValue(), CommissioningStage::kRequestWiFiCredentials); + + auto & credentials = wiFiCredentialsParam.Value(); + if (credentials.registrar != nullptr) { - // Just go ahead and set that up. - return CommissioningStage::kWiFiNetworkSetup; + if (mDeviceCommissioningInfo.network.wifi.supportsPerDeviceCredentials) + { + // We will use PDC, so we need to obtain the Network Identity if we don't have it yet. + VerifyOrReturnValue(mParams.GetPDCNetworkIdentity().HasValue(), CommissioningStage::kPDCGetNetworkIdentity); + } + else if (credentials.hasCredentials) + { + ChipLogProgress(Controller, "Commissionee does not support PDC, using plain Wi-Fi credentials instead"); + } + else + { + // Nothing to configure the commissionee with. Fall through to kWiFiNetworkSetup and let it + // fail there, so that this counts as a network setup failure and we reach the secondary + // network (if any) via the normal failover path. + ChipLogError(Controller, "Commissionee does not support PDC and no plain Wi-Fi credentials are available"); + } } - // We need credentials but don't have them. We need to ask for those. - return CommissioningStage::kRequestWiFiCredentials; + return CommissioningStage::kWiFiNetworkSetup; } // networkToUse must be kThread here. @@ -492,7 +551,18 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio return CommissioningStage::kNeedsNetworkCreds; case CommissioningStage::kNeedsNetworkCreds: return GetNextCommissioningStageNetworkSetup(currentStage, lastErr); + case CommissioningStage::kPDCGetNetworkIdentity: + // We now have the Network Identity, so this will select kWiFiNetworkSetup. + return GetNextCommissioningStageNetworkSetup(currentStage, lastErr); case CommissioningStage::kWiFiNetworkSetup: + if (mParams.GetPDCNetworkIdentity().HasValue()) + { + // We're configuring the commissionee for PDC, so the Network Client Identity it + // provided from kWiFiNetworkSetup needs to be registered before ConnectNetwork. + return CommissioningStage::kPDCRegisterClientIdentity; + } + return CommissioningStage::kFailsafeBeforeWiFiEnable; + case CommissioningStage::kPDCRegisterClientIdentity: return CommissioningStage::kFailsafeBeforeWiFiEnable; case CommissioningStage::kThreadNetworkSetup: return CommissioningStage::kFailsafeBeforeThreadEnable; @@ -622,6 +692,9 @@ EndpointId AutoCommissioner::GetEndpoint(const CommissioningStage & stage) const case CommissioningStage::kRemoveWiFiNetworkConfig: case CommissioningStage::kRemoveThreadNetworkConfig: return kRootEndpointId; + case CommissioningStage::kPDCGetNetworkIdentity: + case CommissioningStage::kPDCRegisterClientIdentity: + return kInvalidEndpointId; // interact with the NetworkIdentityRegistrar, not with the commissionee default: return kRootEndpointId; } @@ -777,6 +850,7 @@ CHIP_ERROR AutoCommissioner::NOCChainGenerated(ByteSpan noc, ByteSpan icac, Byte void AutoCommissioner::CleanupCommissioning() { + ClearPDCParameters(); ResetNetworkAttemptType(); mPAI.Free(); mDAC.Free(); @@ -850,9 +924,15 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio // TODO: This doesn't actually work, because in order to provide credentials someone // had to SetWiFiCredentials() or SetThreadOperationalDataset() on our params, so // IsScanNeeded() will no longer test true for that network technology. + // Scanning is the wrong condition in any case: the question is whether the application + // is able to supply another set of credentials, which a CommissioningParameters flag + // along the lines of RetryNetworkCredentials would say directly. // // TODO: A retry also has to remove the configuration we just wrote before writing - // another one, (unless it is for the same NetworkID). + // another one, (unless it is for the same NetworkID). Under PDC this is not just + // untidy: the Network Client Identity registered for the old configuration is only + // rolled back on RemoveNetwork or a failed attempt, so kPDCRegisterClientIdentity will + // refuse to register a second one while it is still outstanding. if (IsScanNeeded()) { if (completionStatus.err == CHIP_NO_ERROR) @@ -863,6 +943,8 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio // Walk back the completed stage to kScanNetworks. // This will allow the app to try another network. report.stageCompleted = CommissioningStage::kScanNetworks; + + ClearPDCParameters(); // parameters from a failed attempt are no longer valid / useful } } @@ -883,6 +965,13 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio return true; } + // Likewise if we could not reach the network's Network Identity Management provider to + // obtain a Network Identity: the other network type might still work. + if (stage == kPDCGetNetworkIdentity) + { + return true; + } + return false; }; @@ -1023,7 +1112,39 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio case CommissioningStage::kICDRegistration: // Noting to do. DevicePairingDelegate will handle this. break; + case CommissioningStage::kPDCGetNetworkIdentity: { + ByteSpan networkIdentity = report.Get().networkIdentity; + ReturnErrorOnFailure(RelocateSpan(networkIdentity, mNetworkIdentity)); + mParams.SetPDCNetworkIdentity(networkIdentity); + + // Use a random possession nonce for kWiFiNetworkSetup unless the application supplied one. + if (!mParams.GetPDCPossessionNonce().HasValue()) + { + ReturnErrorOnFailure(Crypto::DRBG_get_bytes(mPossessionNonce, sizeof(mPossessionNonce))); + mParams.SetPDCPossessionNonce(ByteSpan(mPossessionNonce)); + } + break; + } case CommissioningStage::kWiFiNetworkSetup: + mWroteNetworkConfig = true; + if (mParams.GetPDCNetworkIdentity().HasValue()) + { + // We configured the commissionee for PDC, so it returned a client identity along with + // proof that it holds the corresponding private key, both already checked for shape by + // the DeviceCommissioner (which verifies the proof itself during + // kPDCRegisterClientIdentity). Note that we need to copy the underlying bytes: the + // report spans point into the response message buffer. + const auto & info = report.Get(); + + ByteSpan clientIdentity = info.clientIdentity; + ReturnErrorOnFailure(RelocateSpan(clientIdentity, mClientIdentity)); + mParams.SetPDCClientIdentity(clientIdentity); + + ByteSpan possessionSignature = info.possessionSignature; + ReturnErrorOnFailure(RelocateSpan(possessionSignature, mPossessionSignature, /* exactSize = */ true)); + mParams.SetPDCPossessionSignature(possessionSignature); + } + break; case CommissioningStage::kThreadNetworkSetup: mWroteNetworkConfig = true; break; diff --git a/src/controller/AutoCommissioner.h b/src/controller/AutoCommissioner.h index 61895ea56d4e..1bafb198f698 100644 --- a/src/controller/AutoCommissioner.h +++ b/src/controller/AutoCommissioner.h @@ -79,6 +79,9 @@ class AutoCommissioner : public CommissioningDelegate // Adjust the failsafe timer if CommissioningDelegate GetCASEFailsafeTimerSeconds is set void SetCASEFailsafeTimerIfNeeded(); + // Reset PDC parameters that should not carry over between network commissioning attempts. + void ClearPDCParameters(); + const ByteSpan GetDAC() { return mDAC.Span(); } const ByteSpan GetPAI() { return mPAI.Span(); } @@ -168,6 +171,10 @@ class AutoCommissioner : public CommissioningDelegate uint8_t mCredentials[CommissioningParameters::kMaxCredentialsLen]; uint8_t mThreadOperationalDataset[CommissioningParameters::kMaxThreadDatasetLen]; char mCountryCode[CommissioningParameters::kMaxCountryCodeLen]; + uint8_t mNetworkIdentity[CommissioningParameters::kMaxNetworkIdentityLen]; + uint8_t mClientIdentity[CommissioningParameters::kMaxNetworkIdentityLen]; + uint8_t mPossessionNonce[CommissioningParameters::kPossessionNonceLen]; + uint8_t mPossessionSignature[CommissioningParameters::kPossessionSignatureLen]; // Time zone is statically allocated because it is max 2 and not trivially destructible static constexpr size_t kMaxSupportedTimeZones = 2; diff --git a/src/controller/BUILD.gn b/src/controller/BUILD.gn index 3eb851e65321..c823bfe49e32 100644 --- a/src/controller/BUILD.gn +++ b/src/controller/BUILD.gn @@ -70,6 +70,7 @@ static_library("controller") { "DeviceDiscoveryDelegate.h", "DevicePairingDelegate.h", "ExampleOperationalCredentialsIssuer.h", + "NetworkIdentityRegistrar.h", "SetUpCodePairer.h", ] @@ -86,7 +87,11 @@ static_library("controller") { "CommissionerDiscoveryController.cpp", "CommissionerDiscoveryController.h", "CommissioningDelegate.cpp", + "ControllerOperation.cpp", + "ControllerOperation.h", "ExampleOperationalCredentialsIssuer.cpp", + "NetworkIdentityManagementRegistrar.cpp", + "NetworkIdentityManagementRegistrar.h", "SetUpCodePairer.cpp", ] diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 1d0c13e853ba..3213f4ffc24f 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -481,7 +481,9 @@ DeviceCommissioner::DeviceCommissioner() : mOnDeviceConnectionRetryCallback(OnDeviceConnectionRetryFn, this), #endif // CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES mDeviceAttestationInformationVerificationCallback(OnDeviceAttestationInformationVerification, this), - mDeviceNOCChainCallback(OnDeviceNOCChainGeneration, this), mSetUpCodePairer(this) + mDeviceNOCChainCallback(OnDeviceNOCChainGeneration, this), mOnNetworkIdentityRequestCallback(OnNetworkIdentityAvailable, this), + mOnNetworkClientRegistrationCallback(OnClientRegistered, this), + mOnNetworkClientUnregistrationCallback(OnClientUnregistered, this), mSetUpCodePairer(this) { #if CHIP_DEVICE_CONFIG_ENABLE_JOINT_FABRIC (void) mPeerAdminJFAdminClusterEndpointId; @@ -567,6 +569,23 @@ void DeviceCommissioner::Shutdown() CancelCommissioningInteractions(); + // A synchronous shutdown cannot carry out a rollback: revoking a Network Client Identity takes a + // round trip to the network, and we are about to stop being able to make one. Rather than issue a + // RemoveClient that almost certainly will not get out, say clearly what has been left behind. + // Note this is the one place a revocation in flight has to be abandoned rather than left running: + // the registrar is holding a callback that points at us, and we are about to go away. + if (mOnNetworkClientUnregistrationCallback.IsRegistered()) + { + mOnNetworkClientUnregistrationCallback.Cancel(); + ReportUnrevokedNetworkClientIdentity(mRevokedClientIdentifier, "commissioner shut down (unregister in progress)"); + } + if (mNetworkClientRegistration.HasValue()) + { + ReportUnrevokedNetworkClientIdentity(mNetworkClientRegistration.clientIdentifier, + "commissioner shut down (unregister pending)"); + mNetworkClientRegistration.Clear(); + } + #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY // make this commissioner discoverable if (mUdcTransportMgr != nullptr) { @@ -1238,6 +1257,28 @@ void DeviceCommissioner::CancelCommissioningInteractions() ChipLogDetail(Controller, "Cancelling CASE setup for step '%s'", StageToString(mCommissioningStage)); CancelCASECallbacks(); } + if (mOnNetworkIdentityRequestCallback.IsRegistered()) + { + ChipLogDetail(Controller, "Cancelling network identity request for step '%s'", StageToString(mCommissioningStage)); + mOnNetworkIdentityRequestCallback.Cancel(); + } + if (mOnNetworkClientRegistrationCallback.IsRegistered()) + { + ChipLogDetail(Controller, "Cancelling network client registration for step '%s'", StageToString(mCommissioningStage)); + mOnNetworkClientRegistrationCallback.Cancel(); + } + if (mOnNetworkClientUnregistrationCallback.mCall != OnClientUnregistered) + { + // Drop the continuation waiting on the revocation, since it belongs to the attempt being + // cancelled, whose caller is completed by other means from here. Note we deliberately do not + // cancel the revocation itself: this attempt being over is no reason to leave a stale client + // registration behind, and the revocation is not an interaction of this attempt anyway (the + // registration it undoes was given up when it was issued). So we let it run to completion and + // simply do nothing in particular once it does. + ChipLogDetail(Controller, "Dropping the continuation of a network client revocation for step '%s'", + StageToString(mCommissioningStage)); + mOnNetworkClientUnregistrationCallback.mCall = OnClientUnregistered; + } } void DeviceCommissioner::CancelCASECallbacks() @@ -2072,9 +2113,11 @@ void DeviceCommissioner::CleanupCommissioning(DeviceProxy * proxy, NodeId nodeId // At this point, proxy == mDeviceBeingCommissioned, nodeId == mDeviceBeingCommissioned->GetDeviceId() mCommissioningCompletionStatus = completionStatus; - if (completionStatus.err == CHIP_NO_ERROR) { + // Commissioning succeeded, so the Client Network Identity (if any) will not be rolled back. + mNetworkClientRegistration.Clear(); + // CommissioningStageComplete uses mDeviceBeingCommissioned, which can // be commissionee if we are cleaning up before we've gone operational. Normally // that would not happen in this non-error case, _except_ if we were told to skip sending @@ -2095,8 +2138,16 @@ void DeviceCommissioner::CleanupCommissioning(DeviceProxy * proxy, NodeId nodeId } // Send the callbacks, we're done. SendCommissioningCompleteCallbacks(nodeId, mCommissioningCompletionStatus); + return; } - else if (completionStatus.err == CHIP_ERROR_CANCELLED) + + // A Network Client Identity is only of use to the commissionee if it ends up on the network we + // registered it for, so if we did register one we need to roll it back because commissioning failed. + // Note this is independent of whether the network configuration we wrote to the commissionee itself + // is ever removed: that is left to a retry or to the failsafe. + bool identityRollbackOngoing = RollBackNetworkClientIdentity(); + + if (completionStatus.err == CHIP_ERROR_CANCELLED) { // If we're cleaning up because cancellation has been requested via StopPairing(), expire the failsafe // in the background and reset our state synchronously, so a new commissioning attempt can be started. @@ -2130,6 +2181,21 @@ void DeviceCommissioner::CleanupCommissioning(DeviceProxy * proxy, NodeId nodeId // If we were already doing network setup, we need to retain the pase session and start again from network setup stage. // We do not need to reset the failsafe here because we want to keep everything on the device up to this point, so just // send the completion callbacks (see "Commissioning Flows Error Handling" in the spec). + // + // This is the case the application is most likely to answer by retrying, possibly against a different network, so wait + // for any Network Client Identity rollback to complete before we complete commissioning. This ensures the registrar + // is idle at that point and can be swapped out if necessary without having to abandon an in-progress rollback. + if (identityRollbackOngoing) + { + // Finish from OnClientUnregisteredFromCleanupFinishCommissioning() instead of here. A StopPairing() in this + // window discards the continuation (CancelCommissioningInteractions() resets mCall), but the attempt is still + // completed exactly once: StopPairing() follows up with CommissioningStageComplete(CHIP_ERROR_CANCELLED), and a + // non-OK error short-circuits GetNextCommissioningStageInternal() to kCleanup (rather than kError), which + // re-enters here and finishes synchronously via the CHIP_ERROR_CANCELLED branch above. + mOnNetworkClientUnregistrationCallback.mCall = OnClientUnregisteredFromCleanupFinishCommissioning; + mOnNetworkClientUnregistrationFinishNodeId = nodeId; + return; + } CommissioningStageComplete(CHIP_NO_ERROR); SendCommissioningCompleteCallbacks(nodeId, mCommissioningCompletionStatus); } @@ -2150,6 +2216,15 @@ void DeviceCommissioner::CleanupCommissioning(DeviceProxy * proxy, NodeId nodeId } } +void DeviceCommissioner::OnClientUnregisteredFromCleanupFinishCommissioning(void * context, CHIP_ERROR status) +{ + OnClientUnregistered(context, status); // call base variant first + DeviceCommissioner * commissioner = static_cast(context); + commissioner->CommissioningStageComplete(CHIP_NO_ERROR); + commissioner->SendCommissioningCompleteCallbacks(commissioner->mOnNetworkClientUnregistrationFinishNodeId, + commissioner->mCommissioningCompletionStatus); +} + void DeviceCommissioner::OnDisarmFailsafe(void * context, const GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data) { @@ -2664,8 +2739,10 @@ CHIP_ERROR DeviceCommissioner::ParseNetworkCommissioningInfo(ReadCommissioningIn { if (features.Has(NetworkCommissioning::Feature::kWiFiNetworkInterface)) { - ChipLogProgress(Controller, "NetworkCommissioning Features: has WiFi. endpointid = %u", path.mEndpointId); - info.network.wifi.endpoint = path.mEndpointId; + info.network.wifi.endpoint = path.mEndpointId; + info.network.wifi.supportsPerDeviceCredentials = features.Has(NetworkCommissioning::Feature::kPerDeviceCredentials); + ChipLogProgress(Controller, "NetworkCommissioning Features: has WiFi. endpointid = %u pdc = %u", path.mEndpointId, + info.network.wifi.supportsPerDeviceCredentials); } else if (features.Has(NetworkCommissioning::Feature::kThreadNetworkInterface)) { @@ -3128,12 +3205,29 @@ CHIP_ERROR DeviceCommissioner::ICDRegistrationInfoReady() return CHIP_NO_ERROR; } +// Checks the Per-Device Credentials fields of a NetworkConfigResponse from a commissionee we +// configured for PDC. Only their shape is checked here: the possession signature is verified against +// the nonce during kPDCRegisterClientIdentity, which is where the nonce is to hand. +static CHIP_ERROR +ValidatePDCClientIdentityResponse(const NetworkCommissioning::Commands::NetworkConfigResponse::DecodableType & data) +{ + VerifyOrReturnError(data.clientIdentity.HasValue(), CHIP_ERROR_MISSING_TLV_ELEMENT, + ChipLogError(Controller, "Commissionee did not return a Network Client Identity")); + VerifyOrReturnError(data.clientIdentity.Value().size() <= CommissioningParameters::kMaxNetworkIdentityLen, + CHIP_ERROR_MESSAGE_TOO_LONG, + ChipLogError(Controller, "Commissionee returned an oversized Network Client Identity")); + VerifyOrReturnError(data.possessionSignature.HasValue(), CHIP_ERROR_MISSING_TLV_ELEMENT, + ChipLogError(Controller, "Commissionee did not prove possession of its Network Client Identity")); + VerifyOrReturnError(data.possessionSignature.Value().size() == CommissioningParameters::kPossessionSignatureLen, + CHIP_ERROR_INVALID_SIGNATURE, + ChipLogError(Controller, "Commissionee returned a possession signature of the wrong length")); + return CHIP_NO_ERROR; +} + void DeviceCommissioner::OnNetworkConfigResponse(void * context, const NetworkCommissioning::Commands::NetworkConfigResponse::DecodableType & data) { DeviceCommissioner * commissioner = static_cast(context); - CommissioningDelegate::CommissioningReport report; - CHIP_ERROR err = CHIP_NO_ERROR; ChipLogProgress(Controller, "Received NetworkConfig response, networkingStatus=%u", to_underlying(data.networkingStatus)); @@ -3146,12 +3240,174 @@ void DeviceCommissioner::OnNetworkConfigResponse(void * context, } else if (data.networkingStatus != NetworkCommissioning::NetworkCommissioningStatusEnum::kSuccess) { - err = CHIP_ERROR_INTERNAL; // Preserve debugText alongside the status enum so callers can distinguish // ambiguous statuses (e.g. kAuthFailure: "wrong password" vs "regulatory restriction"). + CommissioningDelegate::CommissioningReport report; report.Set(data.networkingStatus, data.debugText.ValueOr(CharSpan{})); + commissioner->CommissioningStageComplete(CHIP_ERROR_INTERNAL, report); + return; } - commissioner->CommissioningStageComplete(err, report); + + // Removing the Wi-Fi configuration takes any Network Client Identity it was using with it, so + // there is no longer any point in the commissionee holding access to that network. If the + // revocation is happening asynchronously, set up the callback to finish this stage only once + // it finishes, so that a retry can register another Network Client Identity, against this + // registrar or a different one. + if (commissioner->mCommissioningStage == CommissioningStage::kRemoveWiFiNetworkConfig && + commissioner->RollBackNetworkClientIdentity()) + { + commissioner->mOnNetworkClientUnregistrationCallback.mCall = OnClientUnregisteredFromNetworkConfigResponseCompleteStage; + return; + } + + CommissioningDelegate::CommissioningReport report; + if (commissioner->mCommissioningStage == kWiFiNetworkSetup && + commissioner->mCommissioningDelegate->GetCommissioningParameters().GetPDCNetworkIdentity().HasValue()) + { + // We configured the commissionee for Per-Device Credentials, so it owes us the Network Client + // Identity it generated for itself along with a signature proving it holds the corresponding + // private key. Check the shape of that here rather than leaving it to the delegate, so that a + // commissionee failing to hold up its end fails this stage like any other Network Commissioning + // problem and gets the same failover to the secondary network. + CHIP_ERROR err = ValidatePDCClientIdentityResponse(data); + if (err != CHIP_NO_ERROR) + { + commissioner->CommissioningStageComplete(err, report); + return; + } + report.Set(data.clientIdentity.Value(), data.possessionSignature.Value()); + } + + commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report); +} + +void DeviceCommissioner::OnClientUnregisteredFromNetworkConfigResponseCompleteStage(void * context, CHIP_ERROR status) +{ + OnClientUnregistered(context, status); // call base variant first + static_cast(context)->CommissioningStageComplete(CHIP_NO_ERROR); +} + +void DeviceCommissioner::OnNetworkIdentityAvailable(void * context, CHIP_ERROR error, ByteSpan networkIdentity) +{ + DeviceCommissioner * commissioner = static_cast(context); + VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kPDCGetNetworkIdentity); + + CommissioningDelegate::CommissioningReport report; + Credentials::CertificateKeyIdStorage networkIdentifier; + SuccessOrExitAction(error, ChipLogFailure(error, Controller, "Failed to obtain Network Identity")); + + error = Credentials::ValidateChipNetworkIdentity(networkIdentity, Credentials::MutableCertificateKeyId(networkIdentifier)); + SuccessOrExitAction(error, ChipLogFailure(error, Controller, "Registrar provided an invalid Network Identity")); + + ChipLogProgress(Controller, "Obtained Network Identity " ChipLogFormatKeyId, ChipLogValueKeyId(networkIdentifier)); + report.Set(networkIdentity); +exit: + commissioner->CommissioningStageComplete(error, report); +} + +CHIP_ERROR DeviceCommissioner::VerifyNetworkClientIdentity(ByteSpan clientIdentity, ByteSpan possessionSignature, ByteSpan nonce, + Credentials::MutableCertificateKeyId outClientIdentifier) +{ + // Validating the identity also gives us the key identifier we need to roll the registration back. + ReturnErrorAndLogOnFailure(Credentials::ValidateChipNetworkIdentity(clientIdentity, outClientIdentifier), Controller, + "Commissionee returned an invalid Network Client Identity"); + + // These were checked when the commissionee returned them (see ValidatePDCClientIdentityResponse) and + // the nonce when it was accepted as a parameter, so a mismatch here is a plumbing error on the part + // of the delegate rather than something the commissionee did. Not logged for that reason. + VerifyOrReturnError(clientIdentity.size() <= CommissioningParameters::kMaxNetworkIdentityLen, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(nonce.size() == CommissioningParameters::kPossessionNonceLen, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(possessionSignature.size() == CommissioningParameters::kPossessionSignatureLen, + CHIP_ERROR_INVALID_ARGUMENT); + + // The commissionee proves possession of the identity's private key by signing + // (NetworkClientIdentity || PossessionNonce). + uint8_t tbsMessage[CommissioningParameters::kMaxNetworkIdentityLen + CommissioningParameters::kPossessionNonceLen]; + memcpy(tbsMessage, clientIdentity.data(), clientIdentity.size()); + memcpy(tbsMessage + clientIdentity.size(), nonce.data(), nonce.size()); + + Credentials::P256PublicKeySpan publicKeySpan; + ReturnErrorOnFailure(Credentials::ExtractPublicKeyFromChipCert(clientIdentity, publicKeySpan)); + Crypto::P256PublicKey publicKey(publicKeySpan); + + Crypto::P256ECDSASignature signature; + static_assert(signature.Capacity() >= CommissioningParameters::kPossessionSignatureLen); + ReturnErrorOnFailure(signature.SetLength(possessionSignature.size())); + memcpy(signature.Bytes(), possessionSignature.data(), possessionSignature.size()); + + ReturnErrorAndLogOnFailure(publicKey.ECDSA_validate_msg_signature(tbsMessage, clientIdentity.size() + nonce.size(), signature), + Controller, "Commissionee failed to prove possession of its Network Client Identity"); + return CHIP_NO_ERROR; +} + +void DeviceCommissioner::ReportUnrevokedNetworkClientIdentity(Credentials::CertificateKeyId clientIdentifier, const char * reason, + CHIP_ERROR error) +{ + ChipLogError(Controller, "Network Client Identity " ChipLogFormatKeyId " left registered: %s%s%s", + ChipLogValueKeyId(clientIdentifier), reason, // + (error != CHIP_NO_ERROR ? " - " : ""), // + (error != CHIP_NO_ERROR ? error.AsString() : "")); +} + +bool DeviceCommissioner::RollBackNetworkClientIdentity() +{ + VerifyOrReturnValue(mNetworkClientRegistration.HasValue(), false); + + // If we have a revocation in flight, it can only be using the base (background) variant of the + // callback. Otherwise we wouldn't be able to safely abandon it here. This means we're also already + // set up for UnregisterClient() below completing synchronously, which calls the base variant only. + VerifyOrDie(mOnNetworkClientUnregistrationCallback.mCall == OnClientUnregistered); + + // If there is still a rollback ongoing, we need to abandon it now, since we + // need the callback object to keep track of this new revocation request. + // This is an obscure corner case, and can only happen with a background rollback. + if (mOnNetworkClientUnregistrationCallback.IsRegistered()) + { + mOnNetworkClientUnregistrationCallback.Cancel(); + ReportUnrevokedNetworkClientIdentity(mRevokedClientIdentifier, "unregistration abandoned for subsequent rollback"); + } + + // Remember which identity we are giving up, for the benefit of the logging above and in + // OnClientUnregistered(): the registration this came from is about to be cleared, and a later + // one may overwrite its identifier while this revocation is still in flight. + mRevokedClientIdentifier = mNetworkClientRegistration.clientIdentifier; + + ChipLogProgress(Controller, "Revoking commissionee Network Client Identity " ChipLogFormatKeyId, + ChipLogValueKeyId(mRevokedClientIdentifier)); + NetworkIdentityRegistrar * registrar = mNetworkClientRegistration.registrar; + mNetworkClientRegistration.Clear(); // it is the revocation's business from here, however it turns out + registrar->UnregisterClient(mRevokedClientIdentifier, &mOnNetworkClientUnregistrationCallback); + return mOnNetworkClientUnregistrationCallback.IsRegistered(); +} + +void DeviceCommissioner::OnClientUnregistered(void * context, CHIP_ERROR status) +{ + DeviceCommissioner * commissioner = static_cast(context); + if (status != CHIP_NO_ERROR) + { + commissioner->ReportUnrevokedNetworkClientIdentity(commissioner->mRevokedClientIdentifier, "unregister failed", status); + } + + // Reset the callback function pointer to this base variant + commissioner->mOnNetworkClientUnregistrationCallback.mCall = OnClientUnregistered; +} + +void DeviceCommissioner::OnClientRegistered(void * context, CHIP_ERROR status, bool determinate) +{ + DeviceCommissioner * commissioner = static_cast(context); + VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kPDCRegisterClientIdentity); + + ChipLogFailure(status, Controller, "Failed to register Network Client Identity (%s)", + determinate ? "no rollback needed" : "rollback may be necessary"); + + // Only a determinate failure (where we know the client definitely wasn't registered) lets us + // safely skip the rollback. This covers cases like failing to connect to the NIM at all, or + // never getting the command out; anything that leaves the outcome open is revoked instead. + if (status != CHIP_NO_ERROR && determinate) + { + commissioner->mNetworkClientRegistration.Clear(); + } + commissioner->CommissioningStageComplete(status); } void DeviceCommissioner::OnConnectNetworkResponse( @@ -3403,9 +3659,10 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio } case CommissioningStage::kScanNetworks: { NetworkCommissioning::Commands::ScanNetworks::Type request; - if (params.GetWiFiCredentials().HasValue()) + auto wiFiCredentialsParam = params.GetWiFiCredentials(); // optional copied by value + if (wiFiCredentialsParam.HasValue()) { - request.ssid.Emplace(params.GetWiFiCredentials().Value().ssid); + request.ssid.Emplace(wiFiCredentialsParam.Value().ssid); } request.breadcrumb.Emplace(breadcrumb); CHIP_ERROR err = SendCommissioningCommand(proxy, request, OnScanNetworksResponse, OnScanNetworksFailure, endpoint, timeout); @@ -3764,17 +4021,65 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio CommissioningStageComplete(err); return; } + case CommissioningStage::kPDCGetNetworkIdentity: { + auto * registrar = params.GetWiFiNetworkIdentityRegistrar(); + if (registrar == nullptr) + { + ChipLogError(Controller, "Missing NetworkIdentityRegistrar"); + CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); + return; + } + + // Async call, completes via OnNetworkIdentityAvailableFn (may be called synchronously). + registrar->GetNetworkIdentity(&mOnNetworkIdentityRequestCallback); + return; + } case CommissioningStage::kWiFiNetworkSetup: { - if (!params.GetWiFiCredentials().HasValue()) + auto wiFiCredentialsParam = params.GetWiFiCredentials(); // optional copied by value + if (!wiFiCredentialsParam.HasValue()) { ChipLogError(Controller, "No wifi credentials specified"); CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); return; } + auto & credentials = wiFiCredentialsParam.Value(); NetworkCommissioning::Commands::AddOrUpdateWiFiNetwork::Type request; - request.ssid = params.GetWiFiCredentials().Value().ssid; - request.credentials = params.GetWiFiCredentials().Value().credentials; + request.ssid = credentials.ssid; + + // The presence or absence of the PDC Network identity selects the kind of Wi-Fi setup we're being asked to perform. + if (params.GetPDCNetworkIdentity().HasValue()) + { + // PDC commissioning: The credentials field must be left empty in this case; the + // commissionee will generate a Network Client Identity and sign our nonce with it to + // prove possession of the corresponding private key. + if (!params.GetPDCPossessionNonce().HasValue()) + { + ChipLogError(Controller, "No possession nonce found"); + CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); + return; + } + if (params.GetPDCPossessionNonce().Value().size() != CommissioningParameters::kPossessionNonceLen) + { + ChipLogError(Controller, "Invalid possession nonce"); + CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); + return; + } + request.networkIdentity.Emplace(params.GetPDCNetworkIdentity().Value()); + request.possessionNonce.Emplace(params.GetPDCPossessionNonce().Value()); + } + else + { + // Plain Wi-Fi commissioning (passphrase or open network) + if (!credentials.hasCredentials) + { + ChipLogError(Controller, "No plain wifi credentials specified"); + CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); + return; + } + request.credentials = credentials.credentials; + } + request.breadcrumb.Emplace(breadcrumb); CHIP_ERROR err = SendCommissioningCommand(proxy, request, OnNetworkConfigResponse, OnBasicFailure, endpoint, timeout); if (err != CHIP_NO_ERROR) @@ -3786,6 +4091,57 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio } } break; + case CommissioningStage::kPDCRegisterClientIdentity: { + auto * registrar = params.GetWiFiNetworkIdentityRegistrar(); + if (registrar == nullptr) + { + ChipLogError(Controller, "Missing NetworkIdentityRegistrar"); + CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); + return; + } + if (!params.GetPDCClientIdentity().HasValue() || !params.GetPDCPossessionNonce().HasValue() || + !params.GetPDCPossessionSignature().HasValue()) + { + ChipLogError(Controller, "Missing Network Client Identity registration parameters"); + CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); + return; + } + + // We support a single outstanding registration at a time. A delegate that wants to keep + // several Network Client Identities alive calls SetManagePDCClientIdentityRollback(false), in + // which case nothing is ever outstanding here and this does not apply. + if (mNetworkClientRegistration.HasValue()) + { + ChipLogError(Controller, "A previously registered Network Client Identity is still outstanding"); + CommissioningStageComplete(CHIP_ERROR_INCORRECT_STATE); + return; + } + + ByteSpan clientIdentity = params.GetPDCClientIdentity().Value(); + + Credentials::CertificateKeyIdStorage clientIdentifier; + CHIP_ERROR err = VerifyNetworkClientIdentity(clientIdentity, params.GetPDCPossessionSignature().Value(), + params.GetPDCPossessionNonce().Value(), + Credentials::MutableCertificateKeyId(clientIdentifier)); + if (err != CHIP_NO_ERROR) + { + CommissioningStageComplete(err); + return; + } + + if (params.GetManagePDCClientIdentityRollback()) + { + // Keep track of this registration since we may need to roll it back. + mNetworkClientRegistration.registrar = registrar; + mNetworkClientRegistration.clientIdentifier = clientIdentifier; + } + + // Async call, completes via OnClientRegisteredFn (may be called synchronously). + ChipLogProgress(Controller, "Registering commissionee Network Client Identity " ChipLogFormatKeyId, + ChipLogValueKeyId(clientIdentifier)); + registrar->RegisterClient(clientIdentity, &mOnNetworkClientRegistrationCallback); + return; + } case CommissioningStage::kThreadNetworkSetup: { if (!params.GetThreadOperationalDataset().HasValue()) { @@ -3815,14 +4171,15 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio ExtendFailsafeBeforeNetworkEnable(proxy, params, step); break; case CommissioningStage::kWiFiNetworkEnable: { - if (!params.GetWiFiCredentials().HasValue()) + auto wiFiCredentialsParam = params.GetWiFiCredentials(); // optional copied by value + if (!wiFiCredentialsParam.HasValue()) { ChipLogError(Controller, "No wifi credentials specified"); CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); return; } NetworkCommissioning::Commands::ConnectNetwork::Type request; - request.networkID = params.GetWiFiCredentials().Value().ssid; + request.networkID = wiFiCredentialsParam.Value().ssid; request.breadcrumb.Emplace(breadcrumb); CHIP_ERROR err = CHIP_NO_ERROR; @@ -3943,14 +4300,15 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio break; } case CommissioningStage::kRemoveWiFiNetworkConfig: { - if (!params.GetWiFiCredentials().HasValue()) + auto wiFiCredentialsParam = params.GetWiFiCredentials(); // optional copied by value + if (!wiFiCredentialsParam.HasValue()) { ChipLogError(Controller, "No Wi-Fi credentials configured at commissioner!"); CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT); return; } NetworkCommissioning::Commands::RemoveNetwork::Type request; - request.networkID = params.GetWiFiCredentials().Value().ssid; + request.networkID = wiFiCredentialsParam.Value().ssid; request.breadcrumb.Emplace(breadcrumb); CHIP_ERROR err = SendCommissioningCommand(proxy, request, OnNetworkConfigResponse, OnBasicFailure, endpoint, timeout); if (err != CHIP_NO_ERROR) diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index b9717a669b26..745a2662ab13 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -1053,6 +1053,43 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, const app::Clusters::NetworkCommissioning::Commands::NetworkConfigResponse::DecodableType & data); static void OnConnectNetworkResponse( void * context, const chip::app::Clusters::NetworkCommissioning::Commands::ConnectNetworkResponse::DecodableType & data); + + /* Callbacks for the NetworkIdentityRegistrar during PDC commissioning. */ + static void OnNetworkIdentityAvailable(void * context, CHIP_ERROR status, ByteSpan networkIdentity); + static void OnClientRegistered(void * context, CHIP_ERROR status, bool determinate); + + /* Revocation completion callbacks. OnClientUnregistered() is the base variant, installed whenever + nothing is waiting on the revocation; it reports a failure and resets mCall back to itself. The + variants below carry on with whatever the caller that installed them was waiting to do, and each + has to call the base variant first, so that a re-entrant rollback finds mCall back at its + resting state -- which RollBackNetworkClientIdentity() asserts. */ + static void OnClientUnregistered(void * context, CHIP_ERROR status); + static void OnClientUnregisteredFromNetworkConfigResponseCompleteStage(void * context, CHIP_ERROR status); + static void OnClientUnregisteredFromCleanupFinishCommissioning(void * context, CHIP_ERROR status); + + /* Validates the Network Client Identity the commissionee generated for itself and verifies its + signature over (clientIdentity || nonce), proving it holds the corresponding private key. + Outputs the identity's key identifier, which is what a rollback needs. */ + static CHIP_ERROR VerifyNetworkClientIdentity(ByteSpan clientIdentity, ByteSpan possessionSignature, ByteSpan nonce, + Credentials::MutableCertificateKeyId outClientIdentifier); + + /* Revokes the Network Client Identity registration made during the kPDCRegisterClientIdentity + stage, if we still hold that obligation. Idempotent. + + Returns true if the revocation is in flight, in which case the caller may replace + mOnNetworkClientUnregistrationCallback.mCall with one of the variants above to carry on once + the registrar is done. Returns false if there was nothing to revoke, the registrar failed + the call outright, or it completed re-entrantly: in all of these cases the caller has to + carry on by itself, and the callback has been left at the base variant. */ + bool RollBackNetworkClientIdentity(); + + /* Reports a Network Client Identity that we registered on behalf of a commissionee and are unable + to revoke again, for whichever of the several reasons `detail` describes. The entry it stands + for survives on the network, where only an out-of-band audit against the fabric can find it, so + this log line is the only record of which identity to go looking for. */ + void ReportUnrevokedNetworkClientIdentity(Credentials::CertificateKeyId clientIdentifier, const char * reason, + CHIP_ERROR error = CHIP_NO_ERROR); + static void OnCommissioningCompleteResponse( void * context, const chip::app::Clusters::GeneralCommissioning::Commands::CommissioningCompleteResponse::DecodableType & data); @@ -1172,6 +1209,48 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, mDeviceAttestationInformationVerificationCallback; chip::Callback::Callback mDeviceNOCChainCallback; + + chip::Callback::Callback mOnNetworkIdentityRequestCallback; + chip::Callback::Callback mOnNetworkClientRegistrationCallback; + + // Tracks the Network Client Identity revocation in flight, if any. Its mCall rests at the base + // OnClientUnregistered() variant, and a caller that needs to carry on once the revocation lands + // swaps in one of the other variants; RollBackNetworkClientIdentity() may only be entered at the + // resting state, and cancels (and reports) a revocation still using the callback object when it + // needs it for a new one. See those declarations above for the full invariants. + chip::Callback::Callback mOnNetworkClientUnregistrationCallback; + + // A Network Client Identity we have registered with a NetworkIdentityRegistrar on behalf of + // the commissionee, which we owe a matching UnregisterClient() unless the commissionee ends up + // actually using it. Taken on by the kPDCRegisterClientIdentity stage if (and only if) + // CommissioningParameters::GetManagePDCClientIdentityRollback() is true, and discharged by + // RollBackNetworkClientIdentity(). Note the obligation is taken on before the registrar is + // called and survives a registration that fails, unless the registrar reports the failure as + // determinate; see OnClientRegistered(). The identifier is recorded with the registration + // because the delegate is free to clear or overwrite the PDCClientIdentity parameter it is + // derived from at any time. Note we only manage one outstanding registration at a time, since + // we expect initial commissioning to provision exactly one operational network connection: the + // ConnectNetwork / operational discovery / CommissioningComplete flow only validates one set + // of connection parameters. + struct NetworkClientRegistration + { + bool HasValue() const { return registrar != nullptr; } + void Clear() { registrar = nullptr; } + + NetworkIdentityRegistrar * registrar = nullptr; // the registrar the identity was registered with + Credentials::CertificateKeyIdStorage clientIdentifier{}; + }; + NetworkClientRegistration mNetworkClientRegistration; + + // The Network Client Identity of the revocation in flight, or of the last one we gave up on. + // Only used for logging, but kept apart from mNetworkClientRegistration because that copy is + // overwritten by the next registration, which a revocation left running can outlive. + Credentials::CertificateKeyIdStorage mRevokedClientIdentifier{}; + + // The node OnClientUnregisteredFromCleanupFinishCommissioning() has to report on. + // Captured because CommissioningStageComplete() clears mDeviceBeingCommissioned. + NodeId mOnNetworkClientUnregistrationFinishNodeId = kUndefinedNodeId; + SetUpCodePairer mSetUpCodePairer; AutoCommissioner mAutoCommissioner; CommissioningDelegate * mDefaultCommissioner = diff --git a/src/controller/CommissioningDelegate.cpp b/src/controller/CommissioningDelegate.cpp index 431aa1bdd842..23beac948cd7 100644 --- a/src/controller/CommissioningDelegate.cpp +++ b/src/controller/CommissioningDelegate.cpp @@ -97,6 +97,9 @@ const char * StageToString(CommissioningStage stage) case kWiFiNetworkSetup: return "WiFiNetworkSetup"; + case kPDCRegisterClientIdentity: + return "PDCRegisterClientIdentity"; + case kThreadNetworkSetup: return "ThreadNetworkSetup"; @@ -151,6 +154,9 @@ const char * StageToString(CommissioningStage stage) case kRequestThreadCredentials: return "RequestThreadCredentials"; + case kPDCGetNetworkIdentity: + return "PDCGetNetworkIdentity"; + case kCleanup: return "Cleanup"; @@ -240,6 +246,9 @@ const char * MetricKeyForCommissioningStage(CommissioningStage stage) case kWiFiNetworkSetup: return "core_commissioning_stage_wifi_network_setup"; + case kPDCRegisterClientIdentity: + return "core_commissioning_stage_pdc_register_client_identity"; + case kThreadNetworkSetup: return "core_commissioning_stage_thread_network_setup"; @@ -294,6 +303,9 @@ const char * MetricKeyForCommissioningStage(CommissioningStage stage) case kRequestThreadCredentials: return "core_commissioning_stage_request_thread_credentials"; + case kPDCGetNetworkIdentity: + return "core_commissioning_stage_pdc_get_network_identity"; + case kCleanup: return "core_commissioning_stage_cleanup"; diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index 71669962cee9..8d956b92d923 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -22,6 +22,8 @@ #include #include #include +#include +#include #include #include #include @@ -69,6 +71,7 @@ enum CommissioningStage : uint8_t // whether the logic in AutoCommissioner::CommissioningStepFinished that checks for "network // failure" conditions still makes sense. kWiFiNetworkSetup, ///< Send AddOrUpdateWiFiNetwork (0x31:2) command to the device + kPDCRegisterClientIdentity, ///< Register PDC Client Identity via the NetworkIdentityRegistrar kThreadNetworkSetup, ///< Send AddOrUpdateThreadNetwork (0x31:3) command to the device kFailsafeBeforeWiFiEnable, ///< Extend the fail-safe before doing kWiFiNetworkEnable kFailsafeBeforeThreadEnable, ///< Extend the fail-safe before doing kThreadNetworkEnable @@ -99,6 +102,7 @@ enum CommissioningStage : uint8_t kConfigureTCAcknowledgments, ///< Send SetTCAcknowledgements (0x30:6) command to the device kRequestWiFiCredentials, ///< Wi-Fi credentials are needed; ask for those. kRequestThreadCredentials, ///< Thread credentials are needed; ask for those. + kPDCGetNetworkIdentity, ///< Retrieve PDC Network Identity from the NetworkIdentityRegistrar kCleanup, ///< Call delegates with status, free memory, clear timers and state. #if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING kUnpoweredPhaseComplete, ///< Commissioning completed until connect network for unpowered commissioning (NFC) @@ -121,8 +125,42 @@ const char * MetricKeyForCommissioningStage(CommissioningStage stage); struct WiFiCredentials { ByteSpan ssid; + + /// WPA-Personal passphrase, or empty for an open network. Only meaningful if `hasCredentials`. ByteSpan credentials; - WiFiCredentials(ByteSpan newSsid, ByteSpan newCreds) : ssid(newSsid), credentials(newCreds) {} + + /// Whether a passphrase was supplied. + /// Only false if the PDC-only constructor was used, which implies registrar != nullptr. + /// Note: Not folded into `credentials` (by making it a std::optional) to preserve source compat. + bool hasCredentials = true; + + /// Grants access to a network that uses Per-Device Credentials (PDC). If this is set and the + /// commissionee's Network Commissioning cluster advertises the PDC feature, the commissioner + /// will configure the commissionee for PDC instead of using `credentials`. + /// + /// The registrar must outlive the commissioning attempt; see NetworkIdentityRegistrar. + /// + /// Note that a registrar is not itself tied to Wi-Fi, even though this is currently the only + /// place to supply one: PDC over another link layer would need its own way to reach one. + NetworkIdentityRegistrar * registrar = nullptr; + + /// WPA-Personal, or an open network if `aCredentials` is empty. + WiFiCredentials(ByteSpan aSsid, ByteSpan aCredentials) : ssid(aSsid), credentials(aCredentials) {} + + /// Per-Device Credentials only: commissioning fails if the commissionee does not support PDC. + /// NetworkIdentityRegistrar must be non-null. + WiFiCredentials(ByteSpan aSsid, NetworkIdentityRegistrar * aRegistrar) : + ssid(aSsid), hasCredentials(false), registrar(aRegistrar) + { + VerifyOrDie(aRegistrar != nullptr); + } + + /// Prefer Per-Device Credentials, falling back to WPA-Personal / open network `credentials` + /// for commissionees that do not support PDC. The NetworkIdentityRegistrar may be null, in + /// which case this constructor is equivalent to `WiFiCredentials(aSsid, aCredentials)`. + WiFiCredentials(ByteSpan aSsid, NetworkIdentityRegistrar * aRegistrar, ByteSpan aCredentials) : + ssid(aSsid), credentials(aCredentials), registrar(aRegistrar) + {} }; struct TermsAndConditionsAcknowledgement @@ -180,6 +218,12 @@ class CommissioningParameters static constexpr size_t kMaxCredentialsLen = 64; static constexpr size_t kMaxCountryCodeLen = 2; + static constexpr size_t kMaxNetworkIdentityLen = Credentials::kMaxCHIPCompactNetworkIdentityLength; + // Duplicates the server-side NetworkCommissioning::kPossessionNonceSize, which lives in a cluster + // implementation header the controller cannot depend on. TestPDCCommissioning.cpp asserts they agree. + static constexpr size_t kPossessionNonceLen = 32; + static constexpr size_t kPossessionSignatureLen = Crypto::kP256_ECDSA_Signature_Length_Raw; + // Value to use when setting the commissioning failsafe timer on the node being commissioned. // If the failsafe timer value is passed in as part of the commissioning parameters, that value will be used. If not supplied, // the AutoCommissioner will set this to the recommended value read from the node. If that is not set, it will fall back to the @@ -260,10 +304,61 @@ class CommissioningParameters // kSendAttestationRequest step. const Optional GetAttestationNonce() const { return mAttestationNonce; } - // WiFi SSID and credentials to use when adding/updating and enabling WiFi on the node. - // This value must be set before calling PerformCommissioningStep for the kWiFiNetworkSetup or kWiFiNetworkEnable steps. + // Wi-Fi SSID, credentials and/or PDC NetworkIdentityRegistrar to use when adding/updating and + // enabling Wi-Fi on the node. This value must be set before calling PerformCommissioningStep + // for the kWiFiNetworkSetup or kWiFiNetworkEnable steps. const Optional GetWiFiCredentials() const { return mWiFiCreds; } + // Returns GetWiFiCredentials().registrar, or nullptr. + NetworkIdentityRegistrar * GetWiFiNetworkIdentityRegistrar() const + { + return mWiFiCreds.HasValue() ? mWiFiCreds.Value().registrar : nullptr; + } + + // The Network Identity of the operational network, in compact-pdc-identity TLV format. + // If present, kWiFiNetworkSetup will configure the commissionee for PDC using this identity. + // The AutoCommissioner populates this from the PDCNetworkIdentityInfo report returned + // by kPDCGetNetworkIdentity, but a CommissioningDelegate is free to obtain the Network Identity + // in some other way and bypass that step entirely. + // Note: Whoever supplies the identity is responsible for first checking that the commissionee + // supports PDC (generally via the network.wifi.supportsPerDeviceCredentials flag of the + // ReadCommissioningInfo report returned by kReadCommissioningInfo), since a commissionee that + // does not support PDC could otherwise misinterpret a PDC AddOrUpdateWiFiNetwork command as + // configuring a connection to an open network. The AutoCommissioner performs that check itself + // before scheduling kPDCGetNetworkIdentity, so the obligation only falls to a delegate that sets + // this parameter up front. + const Optional GetPDCNetworkIdentity() const { return mPDCNetworkIdentity; } + + // The nonce sent to the commissionee during kWiFiNetworkSetup, and signed by it to prove + // that it has possession of the private key for the Network Client Identity it returns. + // Must be exactly kPossessionNonceLen bytes long. + // When using the AutoCommissioner, a random nonce will be generated if not supplied, and a + // supplied nonce will be cleared after the commissioning attempt ends, or when retrying + // commissioning with another Wi-Fi network. + // Used during kPDCRegisterClientIdentity to validate that possession proof. + const Optional GetPDCPossessionNonce() const { return mPDCPossessionNonce; } + + // The Network Client Identity to register with the NetworkIdentityRegistrar during + // kPDCRegisterClientIdentity, in compact-pdc-identity TLV format. + // The AutoCommissioner populates this from the PDCClientIdentityInfo report returned by kWiFiNetworkSetup. + // This must be set before calling PerformCommissioningStep for the kPDCRegisterClientIdentity step. + const Optional GetPDCClientIdentity() const { return mPDCClientIdentity; } + + // The commissionee's proof-of-possession signature over the Client Identity and the Possession Nonce. + // Verified during kPDCRegisterClientIdentity prior to registering the client identity. + // The AutoCommissioner populates this from the PDCClientIdentityInfo report returned by kWiFiNetworkSetup. + // This must be set before calling PerformCommissioningStep for the kPDCRegisterClientIdentity step. + const Optional GetPDCPossessionSignature() const { return mPDCPossessionSignature; } + + // Whether the DeviceCommissioner is responsible for rolling back registration of the Network + // Client Identity registered in kPDCRegisterClientIdentity if commissioning does not succeed. + // Defaults to true. Setting this to false means the delegate assumes the obligation to revoke + // the registration if necessary. It must do so if it needs to keep multiple Network Client + // Identities alive during a commissioning attempt, because the commissioner only tracks one + // registration at a time. The value is read during kPDCRegisterClientIdentity, so changing + // it afterwards has no effect on the rollback of a registration that has already been made. + bool GetManagePDCClientIdentityRollback() const { return mManagePDCClientIdentityRollback; } + // Thread operational dataset to use when adding/updating and enabling the thread network on the node. // This value must be set before calling PerformCommissioningStep for the kThreadNetworkSetup or kThreadNetworkEnable steps. const Optional GetThreadOperationalDataset() const { return mThreadOperationalDataset; } @@ -451,6 +546,44 @@ class CommissioningParameters return *this; } + CommissioningParameters & SetPDCNetworkIdentity(ByteSpan networkIdentity) + { + mPDCNetworkIdentity.SetValue(networkIdentity); + return *this; + } + + void ClearPDCNetworkIdentity() { mPDCNetworkIdentity.ClearValue(); } + + CommissioningParameters & SetPDCPossessionNonce(ByteSpan possessionNonce) + { + mPDCPossessionNonce.SetValue(possessionNonce); + return *this; + } + + void ClearPDCPossessionNonce() { mPDCPossessionNonce.ClearValue(); } + + CommissioningParameters & SetPDCClientIdentity(ByteSpan clientIdentity) + { + mPDCClientIdentity.SetValue(clientIdentity); + return *this; + } + + void ClearPDCClientIdentity() { mPDCClientIdentity.ClearValue(); } + + CommissioningParameters & SetPDCPossessionSignature(ByteSpan possessionSignature) + { + mPDCPossessionSignature.SetValue(possessionSignature); + return *this; + } + + void ClearPDCPossessionSignature() { mPDCPossessionSignature.ClearValue(); } + + CommissioningParameters & SetManagePDCClientIdentityRollback(bool manageRollback) + { + mManagePDCClientIdentityRollback = manageRollback; + return *this; + } + // If a ThreadOperationalDataset is provided, then the ThreadNetworkScan will not be attempted CommissioningParameters & SetThreadOperationalDataset(ByteSpan threadOperationalDataset) { @@ -662,6 +795,10 @@ class CommissioningParameters mCSRNonce.ClearValue(); mAttestationNonce.ClearValue(); mWiFiCreds.ClearValue(); + mPDCNetworkIdentity.ClearValue(); + mPDCPossessionNonce.ClearValue(); + mPDCClientIdentity.ClearValue(); + mPDCPossessionSignature.ClearValue(); mCountryCode.ClearValue(); mThreadOperationalDataset.ClearValue(); mNOCChainGenerationParameters.ClearValue(); @@ -693,6 +830,11 @@ class CommissioningParameters Optional mCSRNonce; Optional mAttestationNonce; Optional mWiFiCreds; + Optional mPDCNetworkIdentity; + Optional mPDCPossessionNonce; + Optional mPDCClientIdentity; + Optional mPDCPossessionSignature; + bool mManagePDCClientIdentityRollback = true; Optional mCountryCode; Optional mTermsAndConditionsAcknowledgement; Optional mThreadOperationalDataset; @@ -774,6 +916,24 @@ struct OperationalNodeFoundData OperationalDeviceProxy operationalProxy; }; +/// Reported by kPDCGetNetworkIdentity: the Network Identity obtained from the NetworkIdentityRegistrar. +struct PDCNetworkIdentityInfo +{ + PDCNetworkIdentityInfo(ByteSpan aNetworkIdentity) : networkIdentity(aNetworkIdentity) {} + ByteSpan networkIdentity; +}; + +/// Reported by kWiFiNetworkSetup when the commissionee was configured for PDC: the Network Client +/// Identity it generated, and its signature over (clientIdentity || possessionNonce). +struct PDCClientIdentityInfo +{ + PDCClientIdentityInfo(ByteSpan aClientIdentity, ByteSpan aPossessionSignature) : + clientIdentity(aClientIdentity), possessionSignature(aPossessionSignature) + {} + ByteSpan clientIdentity; + ByteSpan possessionSignature; +}; + struct NetworkClusterInfo { EndpointId endpoint = kInvalidEndpointId; @@ -781,6 +941,8 @@ struct NetworkClusterInfo // maxScanTime == 0 means we don't know; normal commissioning step timeouts // will apply in that case. app::Clusters::NetworkCommissioning::Attributes::ScanMaxTimeSeconds::TypeInfo::DecodableType maxScanTime = 0; + // Whether the cluster advertises the Per-Device Credentials feature. + bool supportsPerDeviceCredentials = false; }; struct NetworkClusters { @@ -929,7 +1091,11 @@ class CommissioningDelegate * kSendTrustedRootCert: None * kSendNOC: OperationalCertErrorInfo if AddNOC returned a non-success NodeOperationalCertStatusEnum * kConfigureTrustedTimeSource: None - * kWiFiNetworkSetup: NetworkCommissioningStatusInfo if there is an error + * kPDCGetNetworkIdentity: PDCNetworkIdentityInfo + * kWiFiNetworkSetup: PDCClientIdentityInfo if the commissionee was configured for PDC; its shape + * has been validated, so a success for such a commissionee always carries one. + * NetworkCommissioningStatusInfo if there is an error + * kPDCRegisterClientIdentity: None * kThreadNetworkSetup: NetworkCommissioningStatusInfo if there is an error * kWiFiNetworkEnable: NetworkCommissioningStatusInfo if there is an error * kThreadNetworkEnable: NetworkCommissioningStatusInfo if there is an error @@ -943,7 +1109,7 @@ class CommissioningDelegate struct CommissioningReport : Variant + TimeZoneResponseInfo, Credentials::JCM::TrustVerificationError, PDCNetworkIdentityInfo, PDCClientIdentityInfo> { CommissioningReport() : stageCompleted(CommissioningStage::kError) {} CommissioningStage stageCompleted; diff --git a/src/controller/ControllerOperation.cpp b/src/controller/ControllerOperation.cpp new file mode 100644 index 000000000000..99f7ae65d75d --- /dev/null +++ b/src/controller/ControllerOperation.cpp @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2026 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include + +namespace chip { +namespace Controller { + +void ControllerOperationBase::Start(DeviceController & controller, NodeId nodeId, Callback::Cancelable::Owned onCompletion) +{ + Callback::CancelableOperationBase::Start(std::move(onCompletion)); + CHIP_ERROR err = controller.GetConnectedDevice(nodeId, &mDeviceConnected, &mDeviceConnectionFailure); + if (err != CHIP_NO_ERROR) + { + return OnConnectionFailure(err); + } +} + +void ControllerOperationBase::OnFinished(bool cancelled) +{ + mDeviceConnected.Cancel(); + mDeviceConnectionFailure.Cancel(); + Callback::CancelableOperationBase::OnFinished(cancelled); +} + +} // namespace Controller +} // namespace chip diff --git a/src/controller/ControllerOperation.h b/src/controller/ControllerOperation.h new file mode 100644 index 000000000000..02aec8924e74 --- /dev/null +++ b/src/controller/ControllerOperation.h @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2026 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include + +namespace chip { +namespace Controller { + +class DeviceController; + +class ControllerOperationBase : public Callback::CancelableOperationBase +{ +protected: + /** + * Obtains a CASE session to the given node, reporting the outcome via OnConnected() or + * OnConnectionFailure(). + * + * Note that a controller unable to even attempt the connection is reported as a connection + * failure rather than returned, since the completion is already owned by then. A subclass that + * completes the operation from OnConnectionFailure() therefore invokes the caller's callback + * re-entrantly from here, which may reuse or destroy the operation: nothing may touch it after + * this returns. + */ + void Start(DeviceController & controller, NodeId nodeId, Callback::Cancelable::Owned onCompletion); + + void OnFinished(bool cancelled) override; + + virtual void OnConnected(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) = 0; + virtual void OnConnectionFailure(CHIP_ERROR error) = 0; + +private: + Callback::Callback mDeviceConnected{ + [](void * context, Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) { + static_cast(context)->OnConnected(exchangeMgr, sessionHandle); + }, + this + }; + Callback::Callback mDeviceConnectionFailure{ + [](void * context, const ScopedNodeId & peerId, CHIP_ERROR error) { + static_cast(context)->OnConnectionFailure(error); + }, + this + }; +}; + +template +using ControllerOperation = Callback::TypedOperation; + +} // namespace Controller +} // namespace chip diff --git a/src/controller/NetworkIdentityManagementRegistrar.cpp b/src/controller/NetworkIdentityManagementRegistrar.cpp new file mode 100644 index 000000000000..1082477baeda --- /dev/null +++ b/src/controller/NetworkIdentityManagementRegistrar.cpp @@ -0,0 +1,239 @@ +/* + * Copyright (c) 2026 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace chip { +namespace Controller { + +using namespace app::Clusters::NetworkIdentityManagement; + +namespace { + +// AddClient and RemoveClient are timed invokes. +constexpr uint16_t kTimedInvokeTimeoutMs = 10000; + +using OnIdleCallback = Callback::Callback; + +} // namespace + +void NetworkIdentityManagementRegistrar::StopAcceptingRequests() +{ + mStopped = true; +} + +void NetworkIdentityManagementRegistrar::Shutdown() +{ + StopAcceptingRequests(); // refuse further calls first, otherwise a callback could attempt to start a new operation + + mQueryIdentity.AbortIfPending(); + mAddClient.AbortIfPending(); + mRemoveClient.AbortIfPending(); + OperationFinished(); // whether or not we finished any operations, see OperationFinished() sentinel logic. +} + +bool NetworkIdentityManagementRegistrar::IsIdle() const +{ + return !mQueryIdentity.IsPending() && !mAddClient.IsPending() && !mRemoveClient.IsPending(); +} + +void NetworkIdentityManagementRegistrar::WaitForIdle(OnIdleCallback::Owned onIdle) +{ + VerifyOrReturn(!IsIdle(), onIdle.Invoke()); + mIdleWaiters.Enqueue(onIdle.Take()); +} + +void NetworkIdentityManagementRegistrar::OperationFinished() +{ + VerifyOrReturn(IsIdle() && !mIdleWaiters.IsEmpty()); + + // An idle callback can cause us to no longer be idle (by starting an operation), and can also + // deallocate us (which calls Shutdown() and recurses into OperationFinished()), or trigger + // a recursive OperationFinished() call in other ways. By always letting the innermost frame + // do the work of calling any remaining callbacks, we can avoid touching `this` after it may + // have been destroyed. The sentinel is enqueued last, so reaching the sentinel guarantees that + // all waiters have been called. + bool recursed = false; + OnIdleCallback sentinel([](void * context) { *static_cast(context) = true; }, &recursed); + mIdleWaiters.Enqueue(sentinel.Cancel()); + + while (!mIdleWaiters.IsEmpty()) + { + auto * waiter = mIdleWaiters.First(); + Callback::CallbackDeque::Dequeue(waiter); + OnIdleCallback::FromCancelable(waiter)->Invoke(); + VerifyOrReturn(!recursed); // destructor recurses, so after this point we know we're still alive + VerifyOrReturn(IsIdle()); + } +} + +void NetworkIdentityManagementRegistrar::GetNetworkIdentity(Callback::Callback::Owned onCompletion) +{ + VerifyOrReturn(!mStopped, onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE, ByteSpan())); + VerifyOrReturn(!mQueryIdentity.IsPending(), onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE, ByteSpan())); + mQueryIdentity.Start(mController, mNodeId, mEndpoint, std::move(onCompletion)); +} + +void NetworkIdentityManagementRegistrar::RegisterClient(ByteSpan clientIdentity, + Callback::Callback::Owned onCompletion) +{ + VerifyOrReturn(!mStopped, onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE, /* determinate = */ true)); + VerifyOrReturn(!mAddClient.IsPending(), onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE, /* determinate = */ true)); + mAddClient.Start(mController, mNodeId, mEndpoint, clientIdentity, std::move(onCompletion)); +} + +void NetworkIdentityManagementRegistrar::UnregisterClient(Credentials::CertificateKeyId clientIdentifier, + Callback::Callback::Owned onCompletion) +{ + VerifyOrReturn(!mStopped, onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE)); + VerifyOrReturn(!mRemoveClient.IsPending(), onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE)); + mRemoveClient.Start(mController, mNodeId, mEndpoint, clientIdentifier, std::move(onCompletion)); +} + +void NetworkIdentityManagementRegistrar::Operation::AbortIfPending() +{ + if (IsPending()) + { + Fail(CHIP_ERROR_CANCELLED); // don't DeferOperationFinished() + } +} + +void NetworkIdentityManagementRegistrar::Operation::OnConnectionFailure(CHIP_ERROR error) +{ + ChipLogFailure(error, Controller, "Failed to establish a session with the Network Infrastructure Manager"); + auto notify = mRegistrar.DeferOperationFinished(); + Fail(error); +} + +void NetworkIdentityManagementRegistrar::Operation::OnFinished(bool cancelled) +{ + // Tear down an invocation we are no longer interested in. Reaching this from within one of the + // handlers installed by InvokeCommand() is not a concern: they clear mCancelInvoke first. + if (mCancelInvoke) + { + mCancelInvoke(); + mCancelInvoke = nullptr; + } + + ControllerOperationBase::OnFinished(cancelled); + mCommandSent = false; // whatever we sent is done with; the operation is free to be started again + + if (cancelled) + { + mRegistrar.OperationFinished(); // no completion coming, we're finished now + } +} + +void NetworkIdentityManagementRegistrar::QueryIdentityOperation::OnConnected(Messaging::ExchangeManager & exchangeMgr, + const SessionHandle & session) +{ + // Ask for the network's current identity of the only type PDC defines, rather than for a + // specific entry in the NIM's table: which one is current is up to the NIM. + Commands::QueryIdentity::Type request; + request.networkIdentityType.Emplace(IdentityTypeEnum::kEcdsa); + + InvokeCommand(exchangeMgr, session, request, + [this](const app::ConcreteCommandPath &, const app::StatusIB &, + const Commands::QueryIdentityResponse::DecodableType & response) { + // The identity points into the response message, which is exactly as long-lived + // as the OnNetworkIdentityAvailable callback needs it to be. + Complete(CHIP_NO_ERROR, response.identity); + }); +} + +void NetworkIdentityManagementRegistrar::AddClientOperation::Start(DeviceController & controller, NodeId nodeId, + EndpointId endpoint, ByteSpan clientIdentity, + Completion::Owned onCompletion) +{ + // Refuse before starting, so that the completion is delivered without the operation ever taking + // it on. This is also why the identity buffer is only touched once we know we are taking the + // call on. Also reject an empty identity outright (otherwise memcpy would need a null guard). + VerifyOrReturn(!clientIdentity.empty() && clientIdentity.size() <= sizeof(mClientIdentity), + onCompletion.Invoke(CHIP_ERROR_INVALID_ARGUMENT, /* determinate = */ true)); + memcpy(mClientIdentity, clientIdentity.data(), clientIdentity.size()); + mClientIdentityLength = static_cast(clientIdentity.size()); // range asserted at declaration + + Base::Start(controller, nodeId, endpoint, std::move(onCompletion)); +} + +void NetworkIdentityManagementRegistrar::AddClientOperation::OnConnected(Messaging::ExchangeManager & exchangeMgr, + const SessionHandle & session) +{ + Commands::AddClient::Type request; + request.clientIdentity = ByteSpan(mClientIdentity, mClientIdentityLength); + + InvokeCommand( + exchangeMgr, session, request, + [this](const app::ConcreteCommandPath &, const app::StatusIB &, + const Commands::AddClientResponse::DecodableType & response) { + ChipLogProgress(Controller, "Network Client Identity registered at client index %u", response.clientIndex); + Complete(CHIP_NO_ERROR, /* determinate = */ true); + }, + MakeOptional(kTimedInvokeTimeoutMs)); +} + +void NetworkIdentityManagementRegistrar::RemoveClientOperation::Start(DeviceController & controller, NodeId nodeId, + EndpointId endpoint, + Credentials::CertificateKeyId clientIdentifier, + Completion::Owned onCompletion) +{ + memcpy(mClientIdentifier.data(), clientIdentifier.data(), mClientIdentifier.size()); + Base::Start(controller, nodeId, endpoint, std::move(onCompletion)); +} + +void NetworkIdentityManagementRegistrar::RemoveClientOperation::OnConnected(Messaging::ExchangeManager & exchangeMgr, + const SessionHandle & session) +{ + Commands::RemoveClient::Type request; + request.clientIdentifier.Emplace(ByteSpan(mClientIdentifier)); + + InvokeCommand( + exchangeMgr, session, request, + [this](const app::ConcreteCommandPath &, const app::StatusIB &, const app::DataModel::NullObjectType &) { + ChipLogProgress(Controller, "Network Client Identity revoked"); + Complete(CHIP_NO_ERROR); + }, + MakeOptional(kTimedInvokeTimeoutMs)); +} + +void NetworkIdentityManagementRegistrar::RemoveClientOperation::Fail(CHIP_ERROR error) +{ + if (error == CHIP_IM_GLOBAL_STATUS(NotFound)) + { + // Revocation is required to be idempotent, so this is a success as far as we care. + ChipLogDetail(Controller, "Network Client Identity was already revoked"); + Complete(CHIP_NO_ERROR); + return; + } + Complete(error); +} + +} // namespace Controller +} // namespace chip diff --git a/src/controller/NetworkIdentityManagementRegistrar.h b/src/controller/NetworkIdentityManagementRegistrar.h new file mode 100644 index 000000000000..e74c677f43e0 --- /dev/null +++ b/src/controller/NetworkIdentityManagementRegistrar.h @@ -0,0 +1,297 @@ +/* + * Copyright (c) 2026 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace chip { +namespace Controller { + +/** + * Callback for NetworkIdentityManagementRegistrar::WaitForIdle(). + */ +typedef void (*OnNetworkIdentityRegistrarIdleFunct)(void * context); + +/** + * A NetworkIdentityRegistrar that drives the Network Identity Management cluster on a Network + * Infrastructure Manager (NIM), using a borrowed DeviceController to reach it. (Note that this + * may or may not be the same DeviceController that uses this registrar during commissioning.) + * + * Each operation independently obtains a CASE session to the NIM via the controller (relying on the + * session caching in CASESessionManager) and then invokes a single command on it, so the registrar + * holds no session of its own in between. Both the controller and the NIM node must remain valid + * for the lifetime of the registrar. + */ +class DLL_EXPORT NetworkIdentityManagementRegistrar : public NetworkIdentityRegistrar +{ +public: + /** + * @param controller Controller used to reach the NIM. Borrowed; must outlive this object. + * @param nodeId Node ID of the NIM on the controller's fabric. + * @param endpoint Endpoint hosting the Network Identity Management cluster. There is no + * fixed endpoint for it, so this has to be discovered or configured. + */ + NetworkIdentityManagementRegistrar(DeviceController & controller, NodeId nodeId, EndpointId endpoint) : + mController(controller), mNodeId(nodeId), mEndpoint(endpoint) + {} + ~NetworkIdentityManagementRegistrar() override { Shutdown(); } + + // Not copyable + NetworkIdentityManagementRegistrar(const NetworkIdentityManagementRegistrar &) = delete; + NetworkIdentityManagementRegistrar & operator=(const NetworkIdentityManagementRegistrar &) = delete; + + /** + * Refuses further calls, so that whatever is in flight now is all this registrar will ever have + * to do, but leaves those outstanding operations running. Idempotent. + * + * Combine with WaitForIdle() to determine when the registrar is safe to deallocate. + */ + void StopAcceptingRequests(); + + /** + * Refuses further calls and completes any outstanding operations with CHIP_ERROR_CANCELLED, so + * that the registrar can be deallocated once this returns. Idempotent, and called by the + * destructor, so an owner only needs it to reclaim one early. + * + * Note this releases any WaitForIdle() callbacks as well, since the registrar does end up idle: + * an owner that goes on to deallocate the registrar itself must cancel its waiter first, or it + * will be told the registrar is idle while this call is still unwinding. + * + * If WaitForIdle() is not used, the registrar may also be deallocated directly from a request + * completion -- but not from one this method delivers: it aborts each outstanding operation in + * turn, so it is still touching the registrar after any one of those completions returns. An + * owner reclaiming the registrar from here deallocates it once this returns instead. + */ + void Shutdown(); + + /** + * Registers a callback to be invoked once the registrar has no requests in flight, called + * synchronously if it is idle already. The callback is unregistered once called. + * + * The callback may deallocate the registrar, which is the main reason to wait on it in the first + * place. It may also start another request, in which case any remaining waiters stay registered + * until the registrar is idle again. + * + * An owner that does not use this method at all may instead deallocate the registrar from the + * completion of a request. Mixing the two is not supported: a completion runs while the registrar + * is still unwinding, and it is only safe to deallocate there because with no waiters registered + * there is nothing left for the registrar to do afterwards. + */ + void WaitForIdle(Callback::Callback::Owned onIdle); + + /** + * Returns true if the registrar has no operations in flight. + */ + bool IsIdle() const; + + // NetworkIdentityRegistrar implementation + void GetNetworkIdentity(Callback::Callback::Owned onCompletion) override; + void RegisterClient(ByteSpan clientIdentity, Callback::Callback::Owned onCompletion) override; + void UnregisterClient(Credentials::CertificateKeyId clientIdentifier, + Callback::Callback::Owned onCompletion) override; + +private: + // Called as an operation finishes, i.e. once it has stopped being pending *and* delivered its + // completion, to release the WaitForIdle() callers once the last one has. + void OperationFinished(); + + // Arranges for OperationFinished() to be called as the enclosing scope exits, which is how an + // operation reports itself finished only once it has delivered its completion. Notifying + // after the fact rather than from Operation::OnFinished() is what lets us see if a completion + // that starts a new request: we are not idle then, and a caller waiting for us to be must not + // hear otherwise. + // Whether there is anything to notify is decided up front, to allow an owner that does not use + // WaitForIdle() to deallocate the registrar from a completion callback; otherwise the deferred + // OperationFinished() call would be a use-after-free. Note that WaitForIdle() from within the + // completion works as expected regardless: If we're already idle it invokes the callback + // synchronously, and if we are not they will be enqueued, and the deferred OperationFinished() + // wouldn't have called them yet anyway. + // Defined ahead of Operation, which uses it, because its return type is deduced. + [[nodiscard]] auto DeferOperationFinished() + { + return MakeDefer([registrar = mIdleWaiters.IsEmpty() ? nullptr : this] { + VerifyOrReturn(registrar != nullptr); + registrar->OperationFinished(); + }); + } + + // What our three operations have in common: each connects to the NIM, invokes a single command + // on it, and reports anything that stops it getting an answer back to the caller. A subclass + // sends its command from OnConnected() via InvokeCommand(), which is the only thing it should + // be doing there: InvokeCommand() completes the operation one way or the other. + class Operation : public ControllerOperationBase + { + using Base = ControllerOperationBase; + + public: + explicit Operation(NetworkIdentityManagementRegistrar & registrar) : mRegistrar(registrar) {} + + // Completes the operation with CHIP_ERROR_CANCELLED if it is in flight, otherwise a no-op. + // Does not DeferOperationFinished(): the registrar is the one calling this method. + void AbortIfPending(); + + protected: + void Start(DeviceController & controller, NodeId nodeId, EndpointId endpoint, Callback::Cancelable::Owned onCompletion) + { + mEndpoint = endpoint; + Base::Start(controller, nodeId, std::move(onCompletion)); + } + + // Completes the operation (whatever its completion signature is) based on the given error. + // Every failure a started operation can suffer arrives here, so a subclass has a single + // place to make sense of them. + virtual void Fail(CHIP_ERROR error) = 0; + + // Whether the command has gone out, i.e. whether a failure from here on could still have + // taken effect on the NIM. Note this is cleared as the operation finishes, which happens + // before the completion is delivered, so it has to be read on the way into Complete(). + bool CommandSent() const { return mCommandSent; } + + // Sends the given request to the endpoint passed to Start(), reporting a response to + // onSuccess, which is responsible for completing the operation, or any failure (including + // failure to send at all) to Fail(). The operation is reported finished to the registrar + // once either of those calls returns. + // + // The invocation is tied to the operation's lifecycle: it is cancelled if the operation is + // cancelled or completed before the response arrives, so nothing is left pointing at the + // operation. Note the handlers run with the invocation already released: cancelling one + // deletes the CommandSender, which must not happen from within its own callback. The + // CommandSender tears itself down as the callback returns, so there is nothing left to + // cancel at that point anyway. + template + void InvokeCommand(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & session, const RequestType & request, + OnSuccess onSuccess, const Optional & timedInvokeTimeoutMs = NullOptional, + const Optional & responseTimeout = NullOptional) + { + // Record the command as sent before it goes out, since a handler may run before + // InvokeCommandRequest() returns and is free to reuse or destroy the operation. + mCommandSent = true; + CHIP_ERROR err = InvokeCommandRequest( + &exchangeMgr, session, mEndpoint, request, + [this, onSuccess](auto &&... args) { + mCancelInvoke = nullptr; + auto notify = mRegistrar.DeferOperationFinished(); + onSuccess(std::forward(args)...); + }, + [this](CHIP_ERROR error) { + mCancelInvoke = nullptr; + auto notify = mRegistrar.DeferOperationFinished(); + Fail(error); + }, + timedInvokeTimeoutMs, responseTimeout, &mCancelInvoke); + if (err != CHIP_NO_ERROR) + { + mCommandSent = false; // no handler ran, and nothing was sent after all + auto notify = mRegistrar.DeferOperationFinished(); + Fail(err); + } + } + + private: + void OnConnectionFailure(CHIP_ERROR error) final; + void OnFinished(bool cancelled) final; + + NetworkIdentityManagementRegistrar & mRegistrar; + Internal::InvokeCancelFn mCancelInvoke; + EndpointId mEndpoint = kInvalidEndpointId; + bool mCommandSent = false; // see CommandSent() + }; + + class QueryIdentityOperation final : public Callback::TypedOperation + { + using Base = Callback::TypedOperation; + + public: + using Base::Base; + using Base::Start; + + private: + void OnConnected(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & session) override; + void Fail(CHIP_ERROR error) override { Complete(error, ByteSpan()); } + }; + + class AddClientOperation final : public Callback::TypedOperation + { + using Base = Callback::TypedOperation; + + public: + using Base::Base; + + // Takes a copy of the identity, since the caller's span does not outlive the call. + void Start(DeviceController & controller, NodeId nodeId, EndpointId endpoint, ByteSpan clientIdentity, + Completion::Owned onCompletion); + + private: + void OnConnected(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & session) override; + + // A failure is only determinate if the AddClient never went out: once it has, an error that + // stopped the NIM from acting on it is indistinguishable from one that lost us the answer. + void Fail(CHIP_ERROR error) override { Complete(error, /* determinate = */ !CommandSent()); } + + uint8_t mClientIdentity[Credentials::kMaxCHIPCompactNetworkIdentityLength]; + uint8_t mClientIdentityLength = 0; + static_assert(std::numeric_limits::max() >= sizeof(mClientIdentity)); + }; + + class RemoveClientOperation final : public Callback::TypedOperation + { + using Base = Callback::TypedOperation; + + public: + using Base::Base; + + // Takes a copy of the identifier, since the caller's span does not outlive the call. + void Start(DeviceController & controller, NodeId nodeId, EndpointId endpoint, + Credentials::CertificateKeyId clientIdentifier, Completion::Owned onCompletion); + + private: + void OnConnected(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & session) override; + void Fail(CHIP_ERROR error) override; + + Credentials::CertificateKeyIdStorage mClientIdentifier; + }; + + DeviceController & mController; + const NodeId mNodeId; + const EndpointId mEndpoint; + + bool mStopped = false; // set by StopAcceptingRequests() and never cleared + Callback::CallbackDeque mIdleWaiters; // WaitForIdle() callbacks + + QueryIdentityOperation mQueryIdentity{ *this }; + AddClientOperation mAddClient{ *this }; + RemoveClientOperation mRemoveClient{ *this }; +}; + +} // namespace Controller +} // namespace chip diff --git a/src/controller/NetworkIdentityRegistrar.h b/src/controller/NetworkIdentityRegistrar.h new file mode 100644 index 000000000000..b9e4b6fe84c5 --- /dev/null +++ b/src/controller/NetworkIdentityRegistrar.h @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2026 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace chip { +namespace Controller { + +/** + * Callback for NetworkIdentityRegistrar::GetNetworkIdentity() + * @param networkIdentity The Network Identity in compact-pdc-identity TLV format. + * Only valid if status is CHIP_NO_ERROR. + */ +typedef void (*OnNetworkIdentityAvailableFunct)(void * context, CHIP_ERROR status, ByteSpan networkIdentity); + +/** + * Callback for NetworkIdentityRegistrar::RegisterClient() + * + * @param determinate Whether the outcome of the request is known for certain. A failure is + * determinate if registering the client definitely did not take effect, which is + * the case for a request that never reached the network at all. A failure that + * leaves the outcome open (a response that never arrived, a session that dropped, + * a request abandoned in flight) is indeterminate, and the client may well have + * been registered. Pass false whenever there is any doubt. + * A success is by definition determinate, so pass true along with CHIP_NO_ERROR. + */ +typedef void (*OnClientRegisteredFunct)(void * context, CHIP_ERROR status, bool determinate); + +/** + * Callback for NetworkIdentityRegistrar::UnregisterClient() + */ +typedef void (*OnClientUnregisteredFunct)(void * context, CHIP_ERROR status); + +/** + * Grants a commissioner access to a network that uses Per-Device Credentials (PDC). + * + * The operations mirror the Network Identity Management cluster one-to-one; a registrar is + * generally a client of that cluster on a Network Infrastructure Manager, but nothing here + * requires that. Deciding *which* network to join (i.e. determining the SSID) is not part of this + * interface; that information is supplied to the commissioner separately, alongside the + * registrar, in WiFiCredentials. + * + * Asynchronous methods do not return a status to their caller: they must invoke their callback + * exactly once. Completing the callback synchronously, i.e. before returning from the method, is + * allowed, and is the expected way to signal errors that prevent the operation from starting. + * + * Callbacks are passed as Owned tokens, so a pending callback is cancellable per the usual Callback + * contract: cancelling it makes the registrar relinquish it, and a cancelled request must never be + * completed (e.g. it could result in a UAF if the context the callback points to was deallocated). + * + * The memory backing any span passed to a registrar method is only guaranteed to remain valid for + * the duration of that call, and any span passed to a callback also need only remain valid for the + * duration of the callback. + * + * Lifecycle: + * - A registrar instance usually serves a single DeviceCommissioner, which commissions one device + * at a time. It may serve any number of consecutive commissioning attempts, so an instance can + * live for as long as the commissioner does. + * - The commissioner keeps at most one GetNetworkIdentity() or RegisterClient() call, and at + * most one UnregisterClient() call in flight at the same time. UnregisterClient() can overlap the + * other calls, because a revoke issued as a commissioning attempt ends is not waited for. + * - Unless documented otherwise, concrete registrar implementations may be relying on these + * constraints on concurrent requests, and therefore will not support sharing a single registrar + * instance between multiple commissioners. + * - The registrar must remain valid as long as it is referenced by the CommissioningParameters of + * an in-progress commissioning attempt and/or has any active asynchronous operations for which + * the commissioner is still owed a callback. + * - Beyond that, how a registrar is created, shut down and destroyed is up to the concrete class + * and its owner: the commissioner does not own the registrar, so this interface deliberately + * says nothing about it. The recipe below is therefore phrased in terms of what a concrete + * class would have to offer, not in terms of methods declared here. + * + * Note that while DeviceCommissioner::StopPairing() will synchronously abort and clean up an + * ongoing commissioning attempt, it does generally *not* result in the registrar becoming idle + * synchronously: If a Network Client Identity was already registered via the registrar, the + * commissioner will issue an UnregisterClient() call to revoke it. Unless that request succeeds (or + * fails) synchronously, it will not be complete by the time StopPairing() returns. If a registrar + * must be released synchronously (e.g. to swap it with one pointed at a different network), the + * recipe is to (1) stop the pairing that uses the registrar, and then (2) call a Shutdown() method + * (which the concrete registrar class would have to expose) that in turn calls the callbacks of + * any outstanding operations with a status of `CHIP_ERROR_CANCELLED`, and rejects any further calls + * synchronously. + */ +class DLL_EXPORT NetworkIdentityRegistrar +{ +public: + virtual ~NetworkIdentityRegistrar() = default; + + /** + * Retrieves the Network Identity of the network this registrar represents. + * Maps onto the Network Identity Management cluster QueryIdentity command. + */ + virtual void GetNetworkIdentity(Callback::Callback::Owned onCompletion) = 0; + + /** + * Grants the holder of the given Network Client Identity access to the network. + * Maps onto the Network Identity Management cluster AddClient command. + * + * The commissioner keeps at most one registration outstanding at a time, and revokes it again + * unless the commissionee ends up using the network it was granted access to, so a registrar + * does not need to track pending registrations itself. Note this includes a registration that + * failed indeterminately, since such a failure does not establish that nothing was granted. + * + * @param clientIdentity Network Client Identity in compact-pdc-identity TLV format. + */ + virtual void RegisterClient(ByteSpan clientIdentity, Callback::Callback::Owned onCompletion) = 0; + + /** + * Revokes a previously granted access. Called when a later commissioning step fails, or + * when the Wi-Fi network is abandoned in favour of another network technology. + * Maps onto the Network Identity Management cluster RemoveClient command. + * + * Must be idempotent: the commissioner revokes a registration whose RegisterClient() call was + * cancelled, or failed without a determinate outcome, because such a call may still have taken + * effect on the network. + * + * The commissioner only logs the status it is given; a failure here leaves an entry that only an + * out-of-band audit against the fabric can clean up. It waits for the completion where it needs + * the revocation to be ordered against what it does next, and otherwise leaves the call running. + * A registrar that has the ability to do so MAY take full responsibility for seeing a revocation + * through to completion by synchronously completing the request with CHIP_NO_ERROR. + * + * @param clientIdentifier The key identifier of the client identity to revoke. + */ + virtual void UnregisterClient(Credentials::CertificateKeyId clientIdentifier, + Callback::Callback::Owned onCompletion) = 0; +}; + +} // namespace Controller +} // namespace chip diff --git a/src/controller/tests/AutoCommissionerTestAccess.h b/src/controller/tests/AutoCommissionerTestAccess.h index 3b066f96e93a..101db6741c68 100644 --- a/src/controller/tests/AutoCommissionerTestAccess.h +++ b/src/controller/tests/AutoCommissionerTestAccess.h @@ -50,6 +50,8 @@ class AutoCommissionerTestAccess void CleanupCommissioning() { mCommissioner->CleanupCommissioning(); } + void ClearPDCParameters() { mCommissioner->ClearPDCParameters(); } + CommissioneeDeviceProxy * GetCommissioneeDeviceProxy() { return mCommissioner->GetCommissioneeDeviceProxy(); } Optional GetCommandTimeout(DeviceProxy * device, Controller::CommissioningStage stage) const @@ -95,6 +97,10 @@ class AutoCommissionerTestAccess void SetUTCRequirements(bool requiresUTC) { mCommissioner->mDeviceCommissioningInfo.requiresUTC = requiresUTC; } + void TryPrimaryNetwork() { mCommissioner->TryPrimaryNetwork(); } + + bool TryingPrimaryNetwork() const { return mCommissioner->TryingPrimaryNetwork(); } + void TrySecondaryNetwork() { mCommissioner->TrySecondaryNetwork(); } bool TryingSecondaryNetwork() const { return mCommissioner->TryingSecondaryNetwork(); } diff --git a/src/controller/tests/BUILD.gn b/src/controller/tests/BUILD.gn index c6f87af38d58..4827b92902b1 100644 --- a/src/controller/tests/BUILD.gn +++ b/src/controller/tests/BUILD.gn @@ -56,6 +56,8 @@ chip_test_suite("tests") { "TestAutoCommissioner.cpp", "TestICDManagementResponses.cpp", "TestNetworkConfigResponses.cpp", + "TestNetworkIdentityManagementRegistrar.cpp", + "TestPDCCommissioning.cpp", "TestParseICDInfo.cpp", ] } @@ -71,6 +73,7 @@ chip_test_suite("tests") { ] public_deps = [ + "${chip_root}/src/app/clusters/network-commissioning:constants", "${chip_root}/src/app/common:cluster-objects", "${chip_root}/src/app/tests:helpers", "${chip_root}/src/controller", diff --git a/src/controller/tests/DeviceCommissionerTestAccess.h b/src/controller/tests/DeviceCommissionerTestAccess.h index 6f36ec6220bb..9ece8a312e00 100644 --- a/src/controller/tests/DeviceCommissionerTestAccess.h +++ b/src/controller/tests/DeviceCommissionerTestAccess.h @@ -38,8 +38,37 @@ class DeviceCommissionerTestAccess void SetCommissioningStage(Controller::CommissioningStage stage) { mCommissioner->mCommissioningStage = stage; } + void SetCommissioningDelegate(Controller::CommissioningDelegate * delegate) + { + mCommissioner->mCommissioningDelegate = delegate; + } + void SetDeviceBeingCommissioned(DeviceProxy * device) { mCommissioner->mDeviceBeingCommissioned = device; } + static CHIP_ERROR VerifyNetworkClientIdentity(ByteSpan clientIdentity, ByteSpan possessionSignature, ByteSpan nonce, + Credentials::MutableCertificateKeyId outClientIdentifier) + { + return Controller::DeviceCommissioner::VerifyNetworkClientIdentity(clientIdentity, possessionSignature, nonce, + outClientIdentifier); + } + + void SetNetworkClientRegistration(Controller::NetworkIdentityRegistrar * registrar, ByteSpan clientIdentifier) + { + mCommissioner->mNetworkClientRegistration.registrar = registrar; + VerifyOrDie(clientIdentifier.size() == mCommissioner->mNetworkClientRegistration.clientIdentifier.size()); + memcpy(mCommissioner->mNetworkClientRegistration.clientIdentifier.data(), clientIdentifier.data(), clientIdentifier.size()); + } + + Controller::NetworkIdentityRegistrar * GetNetworkClientRegistrar() const + { + return mCommissioner->mNetworkClientRegistration.registrar; + } + + // Returns true if the revocation is in flight, i.e. a continuation could be installed for it. + bool RollBackNetworkClientIdentity() { return mCommissioner->RollBackNetworkClientIdentity(); } + + void CancelCommissioningInteractions() { mCommissioner->CancelCommissioningInteractions(); } + static void OnICDManagementRegisterClientResponse( Controller::DeviceCommissioner * commissioner, const app::Clusters::IcdManagement::Commands::RegisterClientResponse::DecodableType & data) diff --git a/src/controller/tests/TestAutoCommissioner.cpp b/src/controller/tests/TestAutoCommissioner.cpp index a05e97faca85..a10f44cd11f8 100644 --- a/src/controller/tests/TestAutoCommissioner.cpp +++ b/src/controller/tests/TestAutoCommissioner.cpp @@ -108,6 +108,17 @@ TEST_F(AutoCommissionerTest, DetectsCSRNonceExceedsBuffer) ASSERT_EQ(r, CHIP_ERROR_INVALID_ARGUMENT); } +TEST_F(AutoCommissionerTest, DetectsPDCPossessionNonceExceedsBuffer) +{ + auto possession_nonce_buffer_up = std::make_unique(CommissioningParameters::kPossessionNonceLen + 1); + + mParams.SetPDCPossessionNonce(ByteSpan{ possession_nonce_buffer_up.get(), CommissioningParameters::kPossessionNonceLen + 1 }); + + auto r = mCommissioner.SetCommissioningParameters(mParams); + + ASSERT_EQ(r, CHIP_ERROR_INVALID_ARGUMENT); +} + TEST_F(AutoCommissionerTest, FeaturesPassedDSTOffsetsValue) { app::Clusters::TimeSynchronization::Structs::DSTOffsetStruct::Type sDSTBuf; @@ -698,6 +709,7 @@ TEST_F(AutoCommissionerTest, SetCommissioningParametersCopiesSpans) CommissioningParameters params{}; params.SetAttestationNonce(sourceSpan32); params.SetCSRNonce(sourceSpan32); + params.SetPDCPossessionNonce(sourceSpan32); params.SetThreadOperationalDataset(sourceSpan32); params.SetWiFiCredentials(WiFiCredentials(sourceSpan32, sourceSpan32)); params.SetCountryCode(sourceCountryCode); @@ -712,6 +724,10 @@ TEST_F(AutoCommissionerTest, SetCommissioningParametersCopiesSpans) EXPECT_NE(storedParams.GetCSRNonce().Value().data(), sourceSpan32.data()); EXPECT_TRUE(storedParams.GetCSRNonce().Value().data_equal(sourceSpan32)); + ASSERT_TRUE(storedParams.GetPDCPossessionNonce().HasValue()); + EXPECT_NE(storedParams.GetPDCPossessionNonce().Value().data(), sourceSpan32.data()); + EXPECT_TRUE(storedParams.GetPDCPossessionNonce().Value().data_equal(sourceSpan32)); + ASSERT_TRUE(storedParams.GetThreadOperationalDataset().HasValue()); EXPECT_NE(storedParams.GetThreadOperationalDataset().Value().data(), sourceSpan32.data()); EXPECT_TRUE(storedParams.GetThreadOperationalDataset().Value().data_equal(sourceSpan32)); diff --git a/src/controller/tests/TestCommissioningDelegate.cpp b/src/controller/tests/TestCommissioningDelegate.cpp index ebf352df4b13..f83148f2ba0c 100644 --- a/src/controller/tests/TestCommissioningDelegate.cpp +++ b/src/controller/tests/TestCommissioningDelegate.cpp @@ -158,6 +158,17 @@ TEST_F(CommissioningDelegateTest, CommissioningParameters_DefaultsAndSettersExer ASSERT_TRUE(p.GetAttemptWiFiNetworkScan().HasValue()); EXPECT_FALSE(p.GetAttemptWiFiNetworkScan().Value()); + // Per-Device Credentials state + const uint8_t nonce[CommissioningParameters::kPossessionNonceLen] = {}; + p.SetPDCPossessionNonce(ByteSpan{ nonce }); + EXPECT_TRUE(p.GetPDCPossessionNonce().HasValue()); + + // The commissioner manages rollback of the Network Client Identity registration unless the + // delegate takes that over. + EXPECT_TRUE(p.GetManagePDCClientIdentityRollback()); + p.SetManagePDCClientIdentityRollback(false); + EXPECT_FALSE(p.GetManagePDCClientIdentityRollback()); + const uint8_t tds[] = { 0x07, 0x07, 0x07 }; // any opaque dataset bytes ok for exercising API p.SetThreadOperationalDataset(ByteSpan{ tds }); EXPECT_TRUE(p.GetThreadOperationalDataset().HasValue()); @@ -248,6 +259,7 @@ TEST_F(CommissioningDelegateTest, CommissioningParameters_DefaultsAndSettersExer EXPECT_FALSE(p.GetCSRNonce().HasValue()); EXPECT_FALSE(p.GetAttestationNonce().HasValue()); EXPECT_FALSE(p.GetWiFiCredentials().HasValue()); + EXPECT_FALSE(p.GetPDCPossessionNonce().HasValue()); EXPECT_FALSE(p.GetCountryCode().HasValue()); EXPECT_FALSE(p.GetThreadOperationalDataset().HasValue()); EXPECT_FALSE(p.GetNOCChainGenerationParameters().HasValue()); diff --git a/src/controller/tests/TestNetworkIdentityManagementRegistrar.cpp b/src/controller/tests/TestNetworkIdentityManagementRegistrar.cpp new file mode 100644 index 000000000000..c791b26aec01 --- /dev/null +++ b/src/controller/tests/TestNetworkIdentityManagementRegistrar.cpp @@ -0,0 +1,665 @@ +/* + * + * Copyright (c) 2026 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// The lifecycle of a NetworkIdentityManagementRegistrar: which calls it accepts, how it reports the +// outcomes it owes its caller, the two ways it can be shut down, and when it counts as idle. Its +// operations get as far as asking the controller for a session here and no further, since a stub +// controller has none to give; what the cluster commands look like on the wire needs to be covered +// by integration tests. + +#include + +#include +#include +#include +#include +#include +#include + +#include +#include + +using namespace chip; +using namespace chip::Controller; + +namespace { + +constexpr NodeId kNimNodeId = 0x1234; +constexpr EndpointId kNimEndpoint = 1; + +// A 20-byte key identifier, which is the only shape RemoveClient accepts. +constexpr uint8_t kClientIdentifierBytes[Credentials::kKeyIdentifierLength] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, + 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, + 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13 }; +constexpr Credentials::CertificateKeyId kClientIdentifier{ kClientIdentifierBytes }; + +// RegisterClient() only checks that an identity is non-empty and fits, and nothing here gets as far +// as putting one on the wire, so the contents are arbitrary. +constexpr uint8_t kClientIdentityBytes[] = { 0x15, 0x18 }; +constexpr ByteSpan kClientIdentity{ kClientIdentityBytes }; + +/** + * A controller that establishes no sessions of its own: it holds on to the callbacks it is given so + * that a test decides if and when a connection attempt resolves, which is what lets an operation sit + * in flight. Refusing outright is the third option, mirroring a controller that is not initialized. + */ +class StubDeviceController : public DeviceController +{ +public: + void RefuseConnections() { mAcceptConnections = false; } + bool ConnectionPending() const { return mOnFailure != nullptr; } + + CHIP_ERROR GetConnectedDevice(NodeId peerNodeId, Callback::Callback * onConnection, + Callback::Callback * onFailure, + TransportPayloadCapability transportPayloadCapability) override + { + VerifyOrReturnError(mAcceptConnections, CHIP_ERROR_INCORRECT_STATE); + VerifyOrDie(!ConnectionPending()); + mOnConnection = onConnection; + mOnFailure = onFailure; + return CHIP_NO_ERROR; + } + + // Fails the connection attempt in flight, which is as far as an operation gets here: without a + // real session there is no way to let one reach the invoke. + void FailPendingConnection(CHIP_ERROR error) + { + VerifyOrDie(ConnectionPending()); + auto * onFailure = mOnFailure; + mOnConnection = nullptr; + mOnFailure = nullptr; + onFailure->mCall(onFailure->mContext, ScopedNodeId(kNimNodeId, kUndefinedFabricIndex), error); + } + +private: + bool mAcceptConnections = true; + Callback::Callback * mOnConnection = nullptr; + Callback::Callback * mOnFailure = nullptr; +}; + +class TestNetworkIdentityManagementRegistrar : public ::testing::Test +{ +protected: + // Records a status the registrar reported, so a test can tell "not yet" from "reported". + struct StatusRecorder + { + bool Called() const { return status.has_value(); } + CHIP_ERROR Status() const + { + VerifyOrDie(status.has_value()); + return status.value(); + } + + std::optional status; + Callback::Callback callback{ + [](void * context, CHIP_ERROR aStatus) { static_cast(context)->status = aStatus; }, this + }; + }; + + // Records the outcome of a registration, which unlike the other operations also says whether its + // status is determinate, i.e. whether the AddClient it stands for definitely had no effect. + struct RegistrationRecorder + { + bool Called() const { return status.has_value(); } + CHIP_ERROR Status() const + { + VerifyOrDie(status.has_value()); + return status.value(); + } + + std::optional status; + bool determinate = false; + Callback::Callback callback{ [](void * context, CHIP_ERROR aStatus, bool aDeterminate) { + auto * self = static_cast(context); + self->status = aStatus; + self->determinate = aDeterminate; + }, + this }; + }; + + // Records that the registrar reported itself idle. + struct IdleRecorder + { + bool called = false; + + Callback::Callback callback{ + [](void * context) { static_cast(context)->called = true; }, this + }; + }; + + StubDeviceController mController; + NetworkIdentityManagementRegistrar mRegistrar{ mController, kNimNodeId, kNimEndpoint }; +}; + +TEST_F(TestNetworkIdentityManagementRegistrar, StartsIdle) +{ + EXPECT_TRUE(mRegistrar.IsIdle()); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, IsBusyWhileARevocationIsInFlight) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + EXPECT_FALSE(mRegistrar.IsIdle()); + EXPECT_FALSE(revocation.Called()); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(mRegistrar.IsIdle()); + ASSERT_TRUE(revocation.Called()); + EXPECT_EQ(revocation.Status(), CHIP_ERROR_TIMEOUT); +} + +// A controller that cannot even attempt the connection is reported like any other outcome. The +// registrar has taken the callback on by then, so unlike a refused call this one is in flight, however +// briefly, and leaves the registrar idle again once it completes. +TEST_F(TestNetworkIdentityManagementRegistrar, AControllerRefusalIsReportedThroughTheCallback) +{ + mController.RefuseConnections(); + + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + ASSERT_TRUE(revocation.Called()); + EXPECT_EQ(revocation.Status(), CHIP_ERROR_INCORRECT_STATE); + EXPECT_TRUE(mRegistrar.IsIdle()); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, RefusesAnOverlappingRevocation) +{ + StatusRecorder first, second; + mRegistrar.UnregisterClient(kClientIdentifier, &first.callback); + + // A refusal is delivered through the callback, synchronously, and leaves the request that is + // already in flight alone. + mRegistrar.UnregisterClient(kClientIdentifier, &second.callback); + ASSERT_TRUE(second.Called()); + EXPECT_EQ(second.Status(), CHIP_ERROR_INCORRECT_STATE); + EXPECT_FALSE(first.Called()); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, ShutdownReportsAnOperationInFlightAsCancelled) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + mRegistrar.Shutdown(); + ASSERT_TRUE(revocation.Called()); + EXPECT_EQ(revocation.Status(), CHIP_ERROR_CANCELLED); + EXPECT_TRUE(mRegistrar.IsIdle()); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, RefusesCallsAfterShutdown) +{ + mRegistrar.Shutdown(); + + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + ASSERT_TRUE(revocation.Called()); + EXPECT_EQ(revocation.Status(), CHIP_ERROR_INCORRECT_STATE); +} + +// A refused call and a session that never materialises both leave the NIM untouched, and the +// registrar saying so is what spares its caller a RemoveClient for access that was never granted. +TEST_F(TestNetworkIdentityManagementRegistrar, ARefusedRegistrationIsADeterminateFailure) +{ + mRegistrar.Shutdown(); + + RegistrationRecorder registration; + mRegistrar.RegisterClient(kClientIdentity, ®istration.callback); + ASSERT_TRUE(registration.Called()); + EXPECT_EQ(registration.Status(), CHIP_ERROR_INCORRECT_STATE); + EXPECT_TRUE(registration.determinate); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, ARegistrationThatNeverGetsASessionIsADeterminateFailure) +{ + RegistrationRecorder registration; + mRegistrar.RegisterClient(kClientIdentity, ®istration.callback); + ASSERT_FALSE(registration.Called()); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + ASSERT_TRUE(registration.Called()); + EXPECT_EQ(registration.Status(), CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(registration.determinate) << "the AddClient cannot have been sent without a session"; +} + +// Abandoning a registration is the one case whose determinacy depends on how far it got. Still +// waiting for a session, as here, it cannot have been acted on. Once the command is in flight it can, +// so the caller is left to revoke an identity that may or may not be on the network. That is not +// reachable with a mock controller that never produces a session, and is left to integration testing. +TEST_F(TestNetworkIdentityManagementRegistrar, ARegistrationAbandonedWhileConnectingIsADeterminateFailure) +{ + RegistrationRecorder registration; + mRegistrar.RegisterClient(kClientIdentity, ®istration.callback); + + mRegistrar.Shutdown(); + ASSERT_TRUE(registration.Called()); + EXPECT_EQ(registration.Status(), CHIP_ERROR_CANCELLED); + EXPECT_TRUE(registration.determinate); +} + +// Revoking from a failed registration's completion is what a commissioner does with an indeterminate +// failure, and with a synchronous one it means UnregisterClient() is called while RegisterClient() is +// still on the stack. The failure is forced here by refusing both requests outright, which makes for +// the tightest version of that nesting: what matters is that the registrar comes out of it idle with +// both callers answered, not what determinacy it reported on the way. +TEST_F(TestNetworkIdentityManagementRegistrar, ARevocationCanStartFromAFailedRegistration) +{ + mController.RefuseConnections(); + + StatusRecorder revocation; + struct Registration + { + NetworkIdentityManagementRegistrar & registrar; + StatusRecorder & revocation; + int completions = 0; + + Callback::Callback callback{ [](void * context, CHIP_ERROR, bool) { + auto * self = static_cast(context); + self->completions++; + self->registrar.UnregisterClient(kClientIdentifier, + &self->revocation.callback); + }, + this }; + }; + Registration registration{ mRegistrar, revocation }; + + mRegistrar.RegisterClient(kClientIdentity, ®istration.callback); + EXPECT_EQ(registration.completions, 1); + ASSERT_TRUE(revocation.Called()); + EXPECT_EQ(revocation.Status(), CHIP_ERROR_INCORRECT_STATE); + EXPECT_TRUE(mRegistrar.IsIdle()); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, WaitForIdleCompletesImmediatelyWhenThereIsNothingToWaitFor) +{ + IdleRecorder idle; + mRegistrar.WaitForIdle(&idle.callback); + EXPECT_TRUE(idle.called); +} + +// The graceful half of a shutdown: nothing new gets in, but what is in flight is left alone. +TEST_F(TestNetworkIdentityManagementRegistrar, StopAcceptingRequestsLeavesAnOperationInFlight) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + mRegistrar.StopAcceptingRequests(); + EXPECT_FALSE(revocation.Called()); + EXPECT_FALSE(mRegistrar.IsIdle()); + + StatusRecorder refused; + mRegistrar.UnregisterClient(kClientIdentifier, &refused.callback); + ASSERT_TRUE(refused.Called()); + EXPECT_EQ(refused.Status(), CHIP_ERROR_INCORRECT_STATE); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(revocation.Called()); +} + +// The point of the whole thing: an owner that is about to go away can let an in-flight revocation +// reach the network first. +TEST_F(TestNetworkIdentityManagementRegistrar, WaitForIdleWaitsForAnOperationInFlight) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + IdleRecorder idle; + mRegistrar.WaitForIdle(&idle.callback); + EXPECT_FALSE(idle.called); + EXPECT_FALSE(revocation.Called()); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(idle.called); + ASSERT_TRUE(revocation.Called()); + EXPECT_EQ(revocation.Status(), CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(mRegistrar.IsIdle()); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, ReleasesEveryWaitingCaller) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + IdleRecorder first, second; + mRegistrar.WaitForIdle(&first.callback); + mRegistrar.WaitForIdle(&second.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(first.called); + EXPECT_TRUE(second.called); +} + +// Idle means the caller has its answer, not just that the network is done with us: a waiter that +// tears things down must not do so while a completion is still on its way to whoever asked for it. +TEST_F(TestNetworkIdentityManagementRegistrar, TheIdleNotificationFollowsTheCompletion) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + struct OrderObserver + { + StatusRecorder & revocation; + bool sawCompletion = false; + + Callback::Callback callback{ [](void * context) { + auto * self = static_cast(context); + self->sawCompletion = self->revocation.Called(); + }, + this }; + }; + OrderObserver observer{ revocation }; + mRegistrar.WaitForIdle(&observer.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(observer.sawCompletion); +} + +// And the reason the ordering matters: a completion that starts the next request leaves the +// registrar busy, which is only visible because the idle check happens after the completion. +TEST_F(TestNetworkIdentityManagementRegistrar, ARequestStartedFromACompletionSuppressesTheIdleNotification) +{ + // Revokes a second time from the completion of the first revocation, standing in for a caller + // that drives the registrar through a sequence of requests. + struct ChainedRequest + { + NetworkIdentityManagementRegistrar & registrar; + int completions = 0; + + Callback::Callback callback{ [](void * context, CHIP_ERROR) { + auto * self = static_cast(context); + if (++self->completions == 1) + { + self->registrar.UnregisterClient(kClientIdentifier, + &self->callback); + } + }, + this }; + }; + ChainedRequest chained{ mRegistrar }; + mRegistrar.UnregisterClient(kClientIdentifier, &chained.callback); + + IdleRecorder idle; + mRegistrar.WaitForIdle(&idle.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + ASSERT_EQ(chained.completions, 1); + EXPECT_FALSE(mRegistrar.IsIdle()); // the second revocation was accepted + EXPECT_FALSE(idle.called); + + // The second revocation finishing is what finally releases the waiter. + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_EQ(chained.completions, 2); + EXPECT_TRUE(idle.called); +} + +// Escalating a graceful shutdown to an abrupt one, which is what an owner that has run out of +// patience (or out of time) does. +TEST_F(TestNetworkIdentityManagementRegistrar, ShutdownReleasesAWaitingCaller) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + IdleRecorder idle; + mRegistrar.WaitForIdle(&idle.callback); + ASSERT_FALSE(idle.called); + + mRegistrar.Shutdown(); + EXPECT_TRUE(idle.called); + ASSERT_TRUE(revocation.Called()); + EXPECT_EQ(revocation.Status(), CHIP_ERROR_CANCELLED); +} + +TEST_F(TestNetworkIdentityManagementRegistrar, DestructionReleasesAWaitingCaller) +{ + auto owner = std::make_unique(mController, kNimNodeId, kNimEndpoint); + + StatusRecorder revocation; + owner->UnregisterClient(kClientIdentifier, &revocation.callback); + + IdleRecorder idle; + owner->WaitForIdle(&idle.callback); + ASSERT_FALSE(idle.called); + + owner.reset(); + EXPECT_TRUE(idle.called); + EXPECT_TRUE(revocation.Called()); +} + +// The idle notification is an ordinary cancelable callback, so an owner that goes away can withdraw +// it instead of being called back into freed memory. +TEST_F(TestNetworkIdentityManagementRegistrar, CancellingTheIdleCallbackSuppressesIt) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + IdleRecorder idle; + mRegistrar.WaitForIdle(&idle.callback); + idle.callback.Cancel(); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_FALSE(idle.called); + EXPECT_TRUE(revocation.Called()); +} + +// Withdrawing a callback the registrar is in the middle of a round of notifications for. This is a +// stand-in for one waiter destroying another as they unwind together. +TEST_F(TestNetworkIdentityManagementRegistrar, AWaiterCanCancelAnotherFromItsCallback) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + IdleRecorder second; + struct Canceller + { + IdleRecorder & other; + bool called = false; + + Callback::Callback callback{ [](void * context) { + auto * self = static_cast(context); + self->called = true; + self->other.callback.Cancel(); + }, + this }; + }; + Canceller first{ second }; + + mRegistrar.WaitForIdle(&first.callback); + mRegistrar.WaitForIdle(&second.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(first.called); + EXPECT_FALSE(second.called); +} + +// What WaitForIdle() is for: an owner reclaims the registrar as soon as it is safe to. Nothing may +// touch the registrar after the callback returns, and a waiter queued behind the one that destroyed +// it is still owed its callback, which the destructor delivers on the way out. +// +// The registrar is on the heap here rather than the fixture's member so that the read of a destroyed +// registrar this guards against lands in freed memory, where ASAN can see it. +TEST_F(TestNetworkIdentityManagementRegistrar, AWaiterCanDestroyTheRegistrar) +{ + auto owner = std::make_unique(mController, kNimNodeId, kNimEndpoint); + + StatusRecorder revocation; + owner->UnregisterClient(kClientIdentifier, &revocation.callback); + + struct Destroyer + { + std::unique_ptr & owner; + bool called = false; + + Callback::Callback callback{ [](void * context) { + auto * self = static_cast(context); + self->called = true; + self->owner.reset(); + }, + this }; + }; + Destroyer destroyer{ owner }; + IdleRecorder behind; + + owner->WaitForIdle(&destroyer.callback); + owner->WaitForIdle(&behind.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(destroyer.called); + EXPECT_TRUE(behind.called); + EXPECT_FALSE(owner); +} + +// A waiter that starts a request of its own is not the registrar's cue to keep telling the others it +// is idle, since it no longer is. They stay queued for the next time it actually is. +TEST_F(TestNetworkIdentityManagementRegistrar, AWaiterThatStartsARequestKeepsTheOthersWaiting) +{ + StatusRecorder first; + mRegistrar.UnregisterClient(kClientIdentifier, &first.callback); + + struct Restarter + { + NetworkIdentityManagementRegistrar & registrar; + StatusRecorder & second; + + Callback::Callback callback{ [](void * context) { + auto * self = static_cast(context); + self->registrar.UnregisterClient( + kClientIdentifier, &self->second.callback); + }, + this }; + }; + StatusRecorder second; + Restarter restarter{ mRegistrar, second }; + IdleRecorder behind; + + mRegistrar.WaitForIdle(&restarter.callback); + mRegistrar.WaitForIdle(&behind.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + ASSERT_FALSE(second.Called()); // the request the waiter started is in flight + EXPECT_FALSE(mRegistrar.IsIdle()); + EXPECT_FALSE(behind.called); + + // The request the waiter started finishing is what makes the registrar idle again. + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(second.Called()); + EXPECT_TRUE(behind.called); +} + +// Escalating to an abrupt shutdown from a waiter's callback, the other half of what an owner might do +// with the registrar once it hears it is idle. +TEST_F(TestNetworkIdentityManagementRegistrar, AWaiterCanShutDownTheRegistrar) +{ + StatusRecorder revocation; + mRegistrar.UnregisterClient(kClientIdentifier, &revocation.callback); + + struct Shutter + { + NetworkIdentityManagementRegistrar & registrar; + bool called = false; + + Callback::Callback callback{ [](void * context) { + auto * self = static_cast(context); + self->called = true; + self->registrar.Shutdown(); + }, + this }; + }; + Shutter shutter{ mRegistrar }; + IdleRecorder behind; + + mRegistrar.WaitForIdle(&shutter.callback); + mRegistrar.WaitForIdle(&behind.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_TRUE(shutter.called); + EXPECT_TRUE(behind.called); + EXPECT_TRUE(mRegistrar.IsIdle()); +} + +// The awkward combination: a waiter starts a request and then destroys the registrar anyway. What +// makes this safe is the destructor's abort, which both reports the abandoned request and leaves the +// registrar idle, so the frame still walking the waiters is told to stop looking at it. +TEST_F(TestNetworkIdentityManagementRegistrar, AWaiterCanDestroyTheRegistrarAfterStartingARequest) +{ + auto owner = std::make_unique(mController, kNimNodeId, kNimEndpoint); + + StatusRecorder first; + owner->UnregisterClient(kClientIdentifier, &first.callback); + + struct RestarterAndDestroyer + { + std::unique_ptr & owner; + StatusRecorder & second; + + Callback::Callback callback{ [](void * context) { + auto * self = + static_cast(context); + self->owner->UnregisterClient(kClientIdentifier, + &self->second.callback); + self->owner.reset(); + }, + this }; + }; + StatusRecorder second; + RestarterAndDestroyer destroyer{ owner, second }; + IdleRecorder behind; + + owner->WaitForIdle(&destroyer.callback); + owner->WaitForIdle(&behind.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + EXPECT_FALSE(owner); + + // The request the waiter started never reached the network, and the destructor said so. + ASSERT_TRUE(second.Called()); + EXPECT_EQ(second.Status(), CHIP_ERROR_CANCELLED); + EXPECT_TRUE(behind.called); +} + +// An owner that never waits for idle can reclaim the registrar from the completion of its request +// instead, which is only safe because a registrar with no waiters has nothing left to do once the +// completion has been delivered. Heap-allocated so ASAN sees a read of the freed registrar. +TEST_F(TestNetworkIdentityManagementRegistrar, ACompletionCanDestroyARegistrarNobodyIsWaitingOn) +{ + auto owner = std::make_unique(mController, kNimNodeId, kNimEndpoint); + + struct Destroyer + { + std::unique_ptr & owner; + std::optional status; + + Callback::Callback callback{ [](void * context, CHIP_ERROR aStatus) { + auto * self = static_cast(context); + self->status = aStatus; + self->owner.reset(); + }, + this }; + }; + Destroyer destroyer{ owner }; + owner->UnregisterClient(kClientIdentifier, &destroyer.callback); + + mController.FailPendingConnection(CHIP_ERROR_TIMEOUT); + ASSERT_TRUE(destroyer.status.has_value()); + EXPECT_EQ(destroyer.status.value(), CHIP_ERROR_TIMEOUT); + EXPECT_FALSE(owner); +} + +} // namespace diff --git a/src/controller/tests/TestPDCCommissioning.cpp b/src/controller/tests/TestPDCCommissioning.cpp new file mode 100644 index 000000000000..0545773843ad --- /dev/null +++ b/src/controller/tests/TestPDCCommissioning.cpp @@ -0,0 +1,1488 @@ +/* + * + * Copyright (c) 2026 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Commissioning a device onto a Wi-Fi network that uses Per-Device Credentials (PDC), i.e. Matter +// Core specification chapter 15. The work is split between the AutoCommissioner, which decides the +// stage sequence and owns the buffers the identities are copied into, and the DeviceCommissioner, +// which drives the NetworkIdentityRegistrar, verifies the commissionee's proof of possession, and +// owns the rollback; there is a fixture for each. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +// The commissioner and the Network Commissioning cluster have to agree on the size of the +// PossessionNonce, but the controller cannot include the cluster's constants.h (a server cluster +// implementation header), so CommissioningParameters declares its own copy. Pin the two together +// here, in a test that is free to include both. +static_assert(chip::Controller::CommissioningParameters::kPossessionNonceLen == + chip::app::Clusters::NetworkCommissioning::kPossessionNonceSize); + +using namespace chip; +using namespace chip::app::Clusters; +using namespace chip::Controller; +using namespace chip::Testing; + +namespace { + +// Records what the commissioner asked of it. GetNetworkIdentity() is never reached by either +// fixture, and RegisterClient() only by tests that opt in with AcceptRegistrations(), so those fail +// here to catch unexpected calls. +class MockNetworkIdentityRegistrar : public NetworkIdentityRegistrar +{ +public: + void GetNetworkIdentity(Callback::Callback::Owned onCompletion) override + { + ADD_FAILURE() << "unexpected GetNetworkIdentity()"; + onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE, ByteSpan()); + } + + void AcceptRegistrations() { mAcceptRegistrations = true; } + + /// Takes registration calls on but completes them with `status`. A determinate failure is one + /// where the registrar knows the AddClient it stands for did not take effect; an indeterminate + /// one leaves that open, which is what a lost response or a dropped session looks like. + void FailRegistrations(CHIP_ERROR status, bool determinate) + { + mAcceptRegistrations = true; + mRegistrationStatus = status; + mRegistrationIsDeterminate = determinate; + } + + void RegisterClient(ByteSpan clientIdentity, Callback::Callback::Owned onCompletion) override + { + if (!mAcceptRegistrations) + { + ADD_FAILURE() << "unexpected RegisterClient()"; + onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE, /* determinate = */ true); + return; + } + mRegisterCalls++; + VerifyOrReturn(clientIdentity.size() <= sizeof(mRegisteredIdentity), + onCompletion.Invoke(CHIP_ERROR_BUFFER_TOO_SMALL, /* determinate = */ true)); + memcpy(mRegisteredIdentity, clientIdentity.data(), clientIdentity.size()); + mRegisteredIdentityLen = clientIdentity.size(); + + // Completing synchronously is explicitly allowed, and there is no bookkeeping to it: the + // callback was never registered with us, so the caller has no opportunity to cancel it. + onCompletion.Invoke(mRegistrationStatus, mRegistrationIsDeterminate); + } + + /// Hold UnregisterClient() calls instead of completing them re-entrantly, so a test can drive the + /// points where the commissioner waits for a revocation. + void DeferRevocations() { mDeferRevocations = true; } + + bool HasPendingRevocation() const { return mPendingUnregister != nullptr; } + + /// Completes a held revocation, as a registrar does once it has heard back from the network. + void CompleteRevocation(CHIP_ERROR status = CHIP_NO_ERROR) + { + Callback::Cancelable * cancelable = mPendingUnregister; + ASSERT_NE(cancelable, nullptr); + mPendingUnregister = nullptr; + + // Surrender our registration before completing, so that the callback is a one-shot. Note this + // must not go through Cancel(): that runs OnRevocationCancelled(), which is how we recognise + // the commissioner giving up on a revocation, and a completion is the opposite of that. Clearing + // mCancel is enough -- the fields we borrowed are indeterminate again once we are unregistered. + cancelable->mCancel = nullptr; + + auto * onCompletion = Callback::Callback::FromCancelable(cancelable); + onCompletion->Invoke(status); + } + + void UnregisterClient(Credentials::CertificateKeyId clientIdentifier, + Callback::Callback::Owned onCompletion) override + { + mUnregisterCalls++; + memcpy(mUnregisteredIdentifier.data(), clientIdentifier.data(), mUnregisteredIdentifier.size()); + + if (!mDeferRevocations) + { + onCompletion.Invoke(CHIP_NO_ERROR); + return; + } + + // Enforce the contract rather than quietly taking the callback over, as a real registrar does: + // a caller only ever has one registration to give up, so this means it has lost track of one. + // (A caller reusing the same callback object cancels the previous revocation when it hands the + // callback over, and so is never caught by this.) + VerifyOrReturn(mPendingUnregister == nullptr, onCompletion.Invoke(CHIP_ERROR_INCORRECT_STATE)); + + // Take ownership of the Cancelable and install a cancel function, so that IsRegistered() + // holds while the request is in flight and we notice if the commissioner gives up on it. + Callback::Cancelable * cancelable = onCompletion.Take(); + cancelable->mContextA = this; + cancelable->mCancel = OnRevocationCancelled; + mPendingUnregister = cancelable; + } + + ByteSpan RegisteredIdentity() const { return ByteSpan(mRegisteredIdentity, mRegisteredIdentityLen); } + ByteSpan UnregisteredIdentifier() const { return ByteSpan(mUnregisteredIdentifier.data(), mUnregisteredIdentifier.size()); } + + int mRegisterCalls = 0; + int mUnregisterCalls = 0; + int mCancelledRevocations = 0; + +private: + static void OnRevocationCancelled(Callback::Cancelable * cancelable) + { + auto * self = static_cast(cancelable->mContextA); + self->mCancelledRevocations++; + self->mPendingUnregister = nullptr; + } + + bool mAcceptRegistrations = false; + CHIP_ERROR mRegistrationStatus = CHIP_NO_ERROR; + bool mRegistrationIsDeterminate = true; + bool mDeferRevocations = false; + Callback::Cancelable * mPendingUnregister = nullptr; + uint8_t mRegisteredIdentity[Credentials::kMaxCHIPCompactNetworkIdentityLength]; + size_t mRegisteredIdentityLen = 0; + Credentials::CertificateKeyIdStorage mUnregisteredIdentifier{}; +}; + +constexpr char kSsidChars[] = "test-network"; +constexpr char kPassphraseChars[] = "passphrase"; +const ByteSpan kSsid(reinterpret_cast(kSsidChars), sizeof(kSsidChars) - 1); +const ByteSpan kPassphrase(reinterpret_cast(kPassphraseChars), sizeof(kPassphraseChars) - 1); + +// Arbitrary fixed nonce, for tests that are not concerned with how the real one is generated. +constexpr uint8_t kPossessionNonceBytes[CommissioningParameters::kPossessionNonceLen] = { 0xa5 }; +const ByteSpan kPossessionNonce(kPossessionNonceBytes); + +// A Network (Client) Identity and the key needed to sign a possession nonce with it. +class TestNetworkIdentity +{ +public: + TestNetworkIdentity() + { + VerifyOrDie(mKeypair.Initialize(Crypto::ECPKeyTarget::ECDSA) == CHIP_NO_ERROR); + MutableByteSpan identity(mIdentityBuffer); + VerifyOrDie(Credentials::NewChipNetworkIdentity(mKeypair, identity) == CHIP_NO_ERROR); + mIdentityLen = identity.size(); + } + + ByteSpan Identity() const { return ByteSpan(mIdentityBuffer, mIdentityLen); } + + // Produces a signature over (identity || nonce), i.e. the proof of possession the commissionee + // is required to return alongside its Network Client Identity. + CHIP_ERROR SignPossession(ByteSpan nonce, Crypto::P256ECDSASignature & signature) const + { + uint8_t tbs[sizeof(mIdentityBuffer) + CommissioningParameters::kPossessionNonceLen]; + VerifyOrReturnError(mIdentityLen + nonce.size() <= sizeof(tbs), CHIP_ERROR_BUFFER_TOO_SMALL); + memcpy(tbs, mIdentityBuffer, mIdentityLen); + memcpy(tbs + mIdentityLen, nonce.data(), nonce.size()); + return mKeypair.ECDSA_sign_msg(tbs, mIdentityLen + nonce.size(), signature); + } + +private: + Crypto::P256Keypair mKeypair; + uint8_t mIdentityBuffer[Credentials::kMaxCHIPCompactNetworkIdentityLength]; + size_t mIdentityLen = 0; +}; + +// --------------------------------------------------------------------------- +// AutoCommissioner +// --------------------------------------------------------------------------- + +// CommissioningStepFinished() processes the report, works out the next stage, and then performs it. +// This fixture has no device proxy to perform anything against, so PerformStep() bails out with +// this error. Reaching that point is what tells us the report was processed and the flow was +// routed, which is all these tests are about; a report the AutoCommissioner rejects fails earlier +// and with its own distinct error, which the tests covering that assert directly. +// +// Driving the steps for real is not an option here: DeviceCommissioner::PerformCommissioningStep() +// is not virtual, so it cannot be stubbed out, and performing it in earnest needs a live PASE +// session. End-to-end coverage belongs in the TestCommissioner harness in OpCredsBinding.cpp, which +// needs a NetworkIdentityRegistrar implementation and a PDC-capable commissionee to exist first. +constexpr CHIP_ERROR kStoppedAtPerformStep = CHIP_ERROR_INCORRECT_STATE; + +// The stage sequence PDC produces, and the buffers the identities are copied into on the way through. +// Records the completion of a commissioning attempt, to tell "finished" apart from "still waiting". +class CompletionRecordingPairingDelegate : public DevicePairingDelegate +{ +public: + void OnCommissioningComplete(NodeId, CHIP_ERROR error) override + { + mCompletions++; + mLastError = error; + } + + int mCompletions = 0; + CHIP_ERROR mLastError = CHIP_NO_ERROR; +}; + +class AutoCommissionerPDCTest : public ::testing::Test +{ +protected: + // Sets up a commissionee that only supports Wi-Fi, and applies the given parameters. + void Configure(const CommissioningParameters & params, bool supportsPDC) + { + ASSERT_EQ(mCommissioner.SetCommissioningParameters(params), CHIP_NO_ERROR); + mAccess.SetCommissioner(&mDeviceCommissioner); + + ReadCommissioningInfo & info = mAccess.GetDeviceCommissioningInfo(); + info.network.wifi.endpoint = kRootEndpointId; + info.network.wifi.supportsPerDeviceCredentials = supportsPDC; + info.network.thread.endpoint = kInvalidEndpointId; + } + + // Parameters carrying an SSID and a registrar, with no passphrase to fall back on. + CommissioningParameters PDCOnlyParams() + { + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, &mRegistrar)); + return params; + } + + // Parameters carrying an SSID, a registrar, and a passphrase (possibly empty) as a fallback. + CommissioningParameters PDCWithFallbackParams(ByteSpan passphrase) + { + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, &mRegistrar, passphrase)); + return params; + } + + // Drives the stage the AutoCommissioner would be in after a successful kPDCGetNetworkIdentity, + // which is what makes the network's Network Identity available. + void CompleteGetNetworkIdentity() + { + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kPDCGetNetworkIdentity; + report.Set(mNetworkIdentity.Identity()); + EXPECT_EQ(mCommissioner.CommissioningStepFinished(CHIP_NO_ERROR, report), kStoppedAtPerformStep); + } + + // Feeds back the client identity and possession signature a PDC commissionee would return. + CHIP_ERROR CompleteWiFiNetworkSetup() + { + Crypto::P256ECDSASignature signature; + VerifyOrDie(mClientIdentity.SignPossession(PossessionNonce(), signature) == CHIP_NO_ERROR); + + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kWiFiNetworkSetup; + report.Set(mClientIdentity.Identity(), signature.Span()); + return mCommissioner.CommissioningStepFinished(CHIP_NO_ERROR, report); + } + + ByteSpan PossessionNonce() { return mAccess.AccessParams().GetPDCPossessionNonce().Value(); } + + void SetCompletionError(CHIP_ERROR err) + { + CompletionStatus status; + status.err = err; + mAccess.AccessParams().SetCompletionStatus(status); + } + + AutoCommissioner mCommissioner{}; + AutoCommissionerTestAccess mAccess{ &mCommissioner }; + DeviceCommissioner mDeviceCommissioner{}; + MockNetworkIdentityRegistrar mRegistrar; + TestNetworkIdentity mNetworkIdentity; + TestNetworkIdentity mClientIdentity; +}; + +// A registrar plus a PDC-capable commissionee takes the PDC path; once the Network Identity has +// been obtained the flow rejoins kWiFiNetworkSetup, with the client identity registered before the +// network is enabled. +TEST_F(AutoCommissionerPDCTest, StageGraphWithRegistrarAndPDCSupport) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + CHIP_ERROR err = CHIP_NO_ERROR; + + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kPDCGetNetworkIdentity); + EXPECT_EQ(err, CHIP_NO_ERROR); + + CompleteGetNetworkIdentity(); + ASSERT_TRUE(mAccess.AccessParams().GetPDCNetworkIdentity().HasValue()); + + // Now that we have the identity, the fetch is not repeated. + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kPDCGetNetworkIdentity, err), kWiFiNetworkSetup); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kWiFiNetworkSetup, err), kPDCRegisterClientIdentity); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kPDCRegisterClientIdentity, err), kFailsafeBeforeWiFiEnable); + EXPECT_EQ(err, CHIP_NO_ERROR); +} + +// Supplying the registrar late, in response to kNeedsNetworkCreds, must converge on the same graph. +TEST_F(AutoCommissionerPDCTest, StageGraphConvergesForLateSuppliedRegistrar) +{ + // First pass: no credentials at all, so the commissioner asks for them. + Configure(CommissioningParameters(), /* supportsPDC = */ true); + CHIP_ERROR err = CHIP_NO_ERROR; + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kICDRegistration, err), kRequestWiFiCredentials); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kRequestWiFiCredentials, err), kNeedsNetworkCreds); + + // The application answers with an SSID and a registrar, exactly as it would up front. + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kNeedsNetworkCreds, err), kPDCGetNetworkIdentity); + + CompleteGetNetworkIdentity(); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kPDCGetNetworkIdentity, err), kWiFiNetworkSetup); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kWiFiNetworkSetup, err), kPDCRegisterClientIdentity); + EXPECT_EQ(err, CHIP_NO_ERROR); +} + +// A registrar is not usable against a commissionee without the PDC feature; a passphrase supplied +// alongside it is the fallback. +TEST_F(AutoCommissionerPDCTest, PassphraseIsUsedWhenCommissioneeLacksPDC) +{ + Configure(PDCWithFallbackParams(kPassphrase), /* supportsPDC = */ false); + CHIP_ERROR err = CHIP_NO_ERROR; + + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kWiFiNetworkSetup); + // No Network Identity, so no client identity to register either. + EXPECT_FALSE(mAccess.AccessParams().GetPDCNetworkIdentity().HasValue()); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kWiFiNetworkSetup, err), kFailsafeBeforeWiFiEnable); + EXPECT_EQ(err, CHIP_NO_ERROR); +} + +// An empty passphrase is a valid configuration -- an open network -- and must not be mistaken for +// "no passphrase supplied", which is what a PDC-only registrar means. +TEST_F(AutoCommissionerPDCTest, OpenNetworkIsUsedAsFallbackWhenCommissioneeLacksPDC) +{ + Configure(PDCWithFallbackParams(ByteSpan()), /* supportsPDC = */ false); + CHIP_ERROR err = CHIP_NO_ERROR; + + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kWiFiNetworkSetup); + EXPECT_EQ(err, CHIP_NO_ERROR); + ASSERT_TRUE(mAccess.AccessParams().GetWiFiCredentials().HasValue()); + EXPECT_TRUE(mAccess.AccessParams().GetWiFiCredentials().Value().hasCredentials); + EXPECT_TRUE(mAccess.AccessParams().GetWiFiCredentials().Value().credentials.empty()); +} + +// An open network without any registrar involved must behave exactly as it did before PDC existed. +TEST_F(AutoCommissionerPDCTest, OpenNetworkWithoutRegistrarIsConfigured) +{ + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, ByteSpan())); + Configure(params, /* supportsPDC = */ true); + CHIP_ERROR err = CHIP_NO_ERROR; + + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kWiFiNetworkSetup); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kWiFiNetworkSetup, err), kFailsafeBeforeWiFiEnable); + EXPECT_EQ(err, CHIP_NO_ERROR); +} + +// "PDC and nothing else" against a commissionee that cannot do PDC is unsatisfiable. Rather than +// deciding that during stage selection, we run kWiFiNetworkSetup anyway and let the commissioner +// reject it, which puts us on the ordinary step-failure path. With no other network to try, the +// attempt ends and the error is reported rather than swallowed. +TEST_F(AutoCommissionerPDCTest, PDCOnlyFailsWhenCommissioneeLacksPDC) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ false); + CHIP_ERROR err = CHIP_NO_ERROR; + + // No Network Identity was obtained, so kWiFiNetworkSetup has nothing to configure the + // commissionee with; see DeviceCommissionerPDCTest.RejectsPDCOnlyCredentialsWithoutIdentity. + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kWiFiNetworkSetup); + EXPECT_EQ(err, CHIP_NO_ERROR); + EXPECT_FALSE(mAccess.AccessParams().GetPDCNetworkIdentity().HasValue()); + + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kWiFiNetworkSetup; + EXPECT_EQ(mCommissioner.CommissioningStepFinished(CHIP_ERROR_INVALID_ARGUMENT, report), kStoppedAtPerformStep); + EXPECT_FALSE(mAccess.TryingSecondaryNetwork()); + EXPECT_EQ(mAccess.AccessParams().GetCompletionStatus().err, CHIP_ERROR_INVALID_ARGUMENT); +} + +// ...but if the application also supplied a Thread dataset, then "Wi-Fi via PDC or Thread" is what +// it asked for, so the rejected Wi-Fi stage fails over to the secondary network. +TEST_F(AutoCommissionerPDCTest, PDCOnlyFallsBackToSecondaryNetworkWhenCommissioneeLacksPDC) +{ + CommissioningParameters params = PDCOnlyParams(); + params.SetThreadOperationalDataset(ByteSpan()); + params.SetSupportsConcurrentConnection(true); + ASSERT_EQ(mCommissioner.SetCommissioningParameters(params), CHIP_NO_ERROR); + mAccess.SetCommissioner(&mDeviceCommissioner); + + ReadCommissioningInfo & info = mAccess.GetDeviceCommissioningInfo(); + info.network.wifi.endpoint = kRootEndpointId; + info.network.wifi.supportsPerDeviceCredentials = false; + info.network.thread.endpoint = 1; + + CHIP_ERROR err = CHIP_NO_ERROR; + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kWiFiNetworkSetup); + ASSERT_TRUE(mAccess.TryingPrimaryNetwork()); + + // The commissioner rejects the stage, and that is treated as any other network failure. + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kWiFiNetworkSetup; + EXPECT_EQ(mCommissioner.CommissioningStepFinished(CHIP_ERROR_INVALID_ARGUMENT, report), kStoppedAtPerformStep); + EXPECT_TRUE(mAccess.TryingSecondaryNetwork()); + + // We never wrote a configuration, so there is nothing to remove on the way to Thread. + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kPrimaryOperationalNetworkFailed, err), kThreadNetworkSetup); + EXPECT_EQ(err, CHIP_NO_ERROR); +} + +// A plain passphrase must produce exactly the graph it produced before PDC existed, whether or not +// the commissionee happens to support PDC. +TEST_F(AutoCommissionerPDCTest, PassphraseOnlyIsUnaffectedByPDCSupport) +{ + for (bool supportsPDC : { false, true }) + { + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, kPassphrase)); + Configure(params, supportsPDC); + CHIP_ERROR err = CHIP_NO_ERROR; + + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kWiFiNetworkSetup); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kWiFiNetworkSetup, err), kFailsafeBeforeWiFiEnable); + EXPECT_EQ(err, CHIP_NO_ERROR); + } +} + +// A fresh possession nonce is generated for each PDC exchange, and an application-supplied one is +// honoured, as for the attestation and CSR nonces. +TEST_F(AutoCommissionerPDCTest, PossessionNonceIsGeneratedUnlessSupplied) +{ + // Nothing is generated until we know we are actually going to configure a commissionee for PDC. + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + EXPECT_FALSE(mAccess.AccessParams().GetPDCPossessionNonce().HasValue()); + + CompleteGetNetworkIdentity(); + ASSERT_TRUE(mAccess.AccessParams().GetPDCPossessionNonce().HasValue()); + ASSERT_EQ(PossessionNonce().size(), CommissioningParameters::kPossessionNonceLen); + + // Resetting the PDC state discards the nonce, so the next exchange uses a different challenge. + uint8_t previousNonce[CommissioningParameters::kPossessionNonceLen]; + memcpy(previousNonce, PossessionNonce().data(), sizeof(previousNonce)); + mAccess.ClearPDCParameters(); + CompleteGetNetworkIdentity(); + EXPECT_FALSE(PossessionNonce().data_equal(ByteSpan(previousNonce))); + + // An application-supplied nonce is copied up front and used as-is. + uint8_t suppliedNonce[CommissioningParameters::kPossessionNonceLen]; + memset(suppliedNonce, 0xA5, sizeof(suppliedNonce)); + CommissioningParameters params = PDCOnlyParams(); + params.SetPDCPossessionNonce(ByteSpan(suppliedNonce)); + Configure(params, /* supportsPDC = */ true); + EXPECT_TRUE(PossessionNonce().data_equal(ByteSpan(suppliedNonce))); + CompleteGetNetworkIdentity(); + EXPECT_TRUE(PossessionNonce().data_equal(ByteSpan(suppliedNonce))); + + // A wrongly sized nonce is rejected rather than silently truncated / extended + uint8_t shortNonce[CommissioningParameters::kPossessionNonceLen - 1] = {}; + CommissioningParameters badParams = PDCOnlyParams(); + badParams.SetPDCPossessionNonce(ByteSpan(shortNonce)); + EXPECT_EQ(mCommissioner.SetCommissioningParameters(badParams), CHIP_ERROR_INVALID_ARGUMENT); +} + +// The AutoCommissioner takes its own copy of the client identity and signature, because the report's +// spans point into the response message buffer. Verification is the DeviceCommissioner's job. +TEST_F(AutoCommissionerPDCTest, CapturesClientIdentityAndPossessionSignature) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + CompleteGetNetworkIdentity(); + + EXPECT_EQ(CompleteWiFiNetworkSetup(), kStoppedAtPerformStep); + ASSERT_TRUE(mAccess.AccessParams().GetPDCClientIdentity().HasValue()); + EXPECT_TRUE(mAccess.AccessParams().GetPDCClientIdentity().Value().data_equal(mClientIdentity.Identity())); + ASSERT_TRUE(mAccess.AccessParams().GetPDCPossessionSignature().HasValue()); + EXPECT_EQ(mAccess.AccessParams().GetPDCPossessionSignature().Value().size(), CommissioningParameters::kPossessionSignatureLen); +} + +// ...and this is the one test that puts both halves together, because the handover is the only place +// the possession nonce the AutoCommissioner generated meets the verification the DeviceCommissioner +// does with it: neither fixture on its own can tell that those two agree. +// +// Only the stages that send a command to the commissionee are stood in for, by reporting them +// complete as the commissionee would. Everything in between really runs, in the real +// DeviceCommissioner, off the AutoCommissioner's own stage decisions -- including the rollback, +// which is what a DeviceCommissioner stage entails rather than something a test can ask for. Note it +// is kCleanup that entails it here, not kRemoveWiFiNetworkConfig: that stage sends RemoveNetwork, so +// performing it needs a session this fixture has no way to produce (see kStoppedAtPerformStep), and +// DeviceCommissionerRevocationTest drives its response handler directly instead. +TEST_F(AutoCommissionerPDCTest, ClientIdentityIsRegisteredAndRolledBackOnFailure) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + mRegistrar.AcceptRegistrations(); + CompleteGetNetworkIdentity(); + + // Give the AutoCommissioner a commissionee to hang the stages off, so that the steps it picks + // reach the DeviceCommissioner rather than being lost in PerformStep(). + CommissioneeDeviceProxy commissionee; + mAccess.SetCommissioneeDeviceProxy(&commissionee); + + // Reporting kWiFiNetworkSetup complete drives all of the following in one synchronous chain: + // kPDCRegisterClientIdentity registers the identity with the network, having checked the + // commissionee's proof of possession against our own nonce + // kFailsafeBeforeWiFiEnable fails, because this commissioner has no commissionee device of its + // own to extend the fail-safe on. Any mid-flow failure would do; this + // is the one the fixture can produce without sending a command. + // kCleanup rolls the registration back, the attempt having failed + ASSERT_EQ(CompleteWiFiNetworkSetup(), CHIP_NO_ERROR); + const CompletionStatus & status = mAccess.AccessParams().GetCompletionStatus(); + ASSERT_EQ(status.err, CHIP_ERROR_INCORRECT_STATE) << "the attempt failed somewhere unexpected"; + ASSERT_TRUE(status.failedStage.HasValue()); + ASSERT_EQ(status.failedStage.Value(), kFailsafeBeforeWiFiEnable); + + // The identity the network was given is the one the commissionee returned, so the possession + // signature was accepted over the nonce the AutoCommissioner generated and stored. + EXPECT_EQ(mRegistrar.mRegisterCalls, 1); + EXPECT_TRUE(mRegistrar.RegisteredIdentity().data_equal(mClientIdentity.Identity())); + + // ...and it was revoked again by its key identifier, which only the DeviceCommissioner computed. + Credentials::CertificateKeyIdStorage clientIdentifier{}; + ASSERT_EQ(Credentials::ExtractIdentifierFromChipNetworkIdentity(mClientIdentity.Identity(), + Credentials::MutableCertificateKeyId(clientIdentifier)), + CHIP_NO_ERROR); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_TRUE(mRegistrar.UnregisteredIdentifier().data_equal(ByteSpan(clientIdentifier))); + EXPECT_EQ(DeviceCommissionerTestAccess(&mDeviceCommissioner).GetNetworkClientRegistrar(), nullptr); +} + +// A registration that fails indeterminately is revoked from the cleanup of the attempt it just +// failed, which for a registrar that completes synchronously means the revocation starts while its +// own RegisterClient() call is still on the stack. Registrars are told to expect that; this is the +// flow that produces it. +TEST_F(AutoCommissionerPDCTest, AnIndeterminateRegistrationFailureIsRolledBackFromCleanup) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + mRegistrar.FailRegistrations(CHIP_ERROR_TIMEOUT, /* determinate = */ false); + CompleteGetNetworkIdentity(); + + CommissioneeDeviceProxy commissionee; + mAccess.SetCommissioneeDeviceProxy(&commissionee); + + // kPDCRegisterClientIdentity is the stage that fails this time, rather than the one after it. + ASSERT_EQ(CompleteWiFiNetworkSetup(), CHIP_NO_ERROR); + const CompletionStatus & status = mAccess.AccessParams().GetCompletionStatus(); + EXPECT_EQ(status.err, CHIP_ERROR_TIMEOUT); + ASSERT_TRUE(status.failedStage.HasValue()); + EXPECT_EQ(status.failedStage.Value(), kPDCRegisterClientIdentity); + + Credentials::CertificateKeyIdStorage clientIdentifier{}; + ASSERT_EQ(Credentials::ExtractIdentifierFromChipNetworkIdentity(mClientIdentity.Identity(), + Credentials::MutableCertificateKeyId(clientIdentifier)), + CHIP_NO_ERROR); + EXPECT_EQ(mRegistrar.mRegisterCalls, 1); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_TRUE(mRegistrar.UnregisteredIdentifier().data_equal(ByteSpan(clientIdentifier))); + EXPECT_EQ(DeviceCommissionerTestAccess(&mDeviceCommissioner).GetNetworkClientRegistrar(), nullptr); +} + +// Whereas a registrar that knows nothing was granted spares the failing attempt the round trip. +TEST_F(AutoCommissionerPDCTest, ADeterminateRegistrationFailureIsNotRolledBack) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + mRegistrar.FailRegistrations(CHIP_ERROR_TIMEOUT, /* determinate = */ true); + CompleteGetNetworkIdentity(); + + CommissioneeDeviceProxy commissionee; + mAccess.SetCommissioneeDeviceProxy(&commissionee); + + ASSERT_EQ(CompleteWiFiNetworkSetup(), CHIP_NO_ERROR); + EXPECT_EQ(mAccess.AccessParams().GetCompletionStatus().err, CHIP_ERROR_TIMEOUT); + EXPECT_EQ(mRegistrar.mRegisterCalls, 1); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 0); +} + +// The same failure, but with a registrar that does not revoke synchronously: cleanup holds the +// attempt open until the revocation lands, so that a retry (possibly against a different network) +// finds the registrar idle rather than colliding with the identity being abandoned. +TEST_F(AutoCommissionerPDCTest, CleanupWaitsForAnOutstandingRevocation) +{ + CompletionRecordingPairingDelegate pairingDelegate; + mDeviceCommissioner.RegisterPairingDelegate(&pairingDelegate); + + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + mRegistrar.AcceptRegistrations(); + mRegistrar.DeferRevocations(); + CompleteGetNetworkIdentity(); + + CommissioneeDeviceProxy commissionee; + mAccess.SetCommissioneeDeviceProxy(&commissionee); + + // Fails at kFailsafeBeforeWiFiEnable and reaches kCleanup, exactly as the test above. + ASSERT_EQ(CompleteWiFiNetworkSetup(), CHIP_NO_ERROR); + ASSERT_EQ(mRegistrar.mUnregisterCalls, 1); + + // The registration is given up immediately, but the attempt is not finished until the network + // has actually been told, so the application cannot retry into a half-revoked state. + ASSERT_TRUE(mRegistrar.HasPendingRevocation()); + EXPECT_EQ(DeviceCommissionerTestAccess(&mDeviceCommissioner).GetNetworkClientRegistrar(), nullptr); + EXPECT_EQ(pairingDelegate.mCompletions, 0) << "completed before the revocation did"; + + mRegistrar.CompleteRevocation(); + EXPECT_EQ(pairingDelegate.mCompletions, 1); + EXPECT_EQ(pairingDelegate.mLastError, CHIP_ERROR_INCORRECT_STATE) << "the original failure was lost"; +} + +// Why the PDC parameters are left alone when the Wi-Fi configuration is removed: that stage is only +// reachable from kPrimaryOperationalNetworkFailed with Wi-Fi on the root endpoint, so by the time it +// completes we are committed to the secondary network and the next selection is Thread, which reads +// no PDC parameters. They are stale but inert, and CleanupCommissioning() clears them at the end of +// the attempt. (Revoking the client identity registration is the DeviceCommissioner's job, off the +// back of the RemoveNetwork response.) +TEST_F(AutoCommissionerPDCTest, RemovingWiFiConfigProceedsToThreadNetworkSetup) +{ + CommissioningParameters params = PDCOnlyParams(); + params.SetThreadOperationalDataset(ByteSpan()); + params.SetSupportsConcurrentConnection(true); + ASSERT_EQ(mCommissioner.SetCommissioningParameters(params), CHIP_NO_ERROR); + mAccess.SetCommissioner(&mDeviceCommissioner); + + ReadCommissioningInfo & info = mAccess.GetDeviceCommissioningInfo(); + info.network.wifi.endpoint = kRootEndpointId; + info.network.wifi.supportsPerDeviceCredentials = true; + info.network.thread.endpoint = 1; + + // Configure the commissionee for PDC on the primary (Wi-Fi) network. + CHIP_ERROR err = CHIP_NO_ERROR; + EXPECT_EQ(mAccess.GetNextCommissioningStageNetworkSetup(kNeedsNetworkCreds, err), kPDCGetNetworkIdentity); + ASSERT_TRUE(mAccess.TryingPrimaryNetwork()); + CompleteGetNetworkIdentity(); + EXPECT_EQ(CompleteWiFiNetworkSetup(), kStoppedAtPerformStep); + + // ConnectNetwork failed outright, so the primary network is given up on. Note the absence of a + // NetworkCommissioningStatusInfo report: this is the failover path, not the retry path. + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kWiFiNetworkEnable; + EXPECT_EQ(mCommissioner.CommissioningStepFinished(CHIP_ERROR_INTERNAL, report), kStoppedAtPerformStep); + ASSERT_TRUE(mAccess.TryingSecondaryNetwork()); + + // That failover is the only way to reach kRemoveWiFiNetworkConfig, and from there the flow can + // only go to Thread; the Wi-Fi branch of the network selection is never entered again. + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kPrimaryOperationalNetworkFailed, err), kRemoveWiFiNetworkConfig); + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kRemoveWiFiNetworkConfig, err), kThreadNetworkSetup); + EXPECT_EQ(err, CHIP_NO_ERROR); +} + +// A failed network configuration that the application is allowed to retry walks the flow back to +// kScanNetworks, and this is the one case where the PDC parameters have to go: the retry re-enters +// the Wi-Fi branch, and reusing the Network Identity (never mind the possession nonce) would mean +// challenging the commissionee with a nonce it has already answered. +TEST_F(AutoCommissionerPDCTest, RetryingAfterNetworkEnableFailureClearsPDCParameters) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + + // The retry is gated on IsScanNeeded(), which cannot be arranged through the application-level + // parameters: SetWiFiCredentials() clears the scan flag, and AutoCommissioner:: + // SetCommissioningParameters() re-applies the credentials, so it has to be set on the + // AutoCommissioner's own copy afterwards. Scanning is the wrong condition in the first place -- + // see the TODO above the IsScanNeeded() check in CommissioningStepFinished() -- and this + // workaround goes away with the follow-up change to the restart flow. + mAccess.AccessParams().SetAttemptWiFiNetworkScan(true); + ASSERT_TRUE(mAccess.IsScanNeeded()); + + CompleteGetNetworkIdentity(); + EXPECT_EQ(CompleteWiFiNetworkSetup(), kStoppedAtPerformStep); + ASSERT_TRUE(mAccess.AccessParams().GetPDCNetworkIdentity().HasValue()); + + // Give the AutoCommissioner a commissionee to talk to, so that the stage it restarts at is + // actually driven rather than lost in PerformStep(). + CommissioneeDeviceProxy commissionee; + mAccess.SetCommissioneeDeviceProxy(&commissionee); + + // ConnectNetwork failed: the commissionee could not find the network we configured. + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kWiFiNetworkEnable; + report.Set( + app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum::kNetworkNotFound, CharSpan()); + EXPECT_EQ(mCommissioner.CommissioningStepFinished(CHIP_ERROR_INTERNAL, report), CHIP_NO_ERROR); + + // Walked back to kScanNetworks, whose successor asks the application for credentials again. + EXPECT_EQ(mDeviceCommissioner.GetCommissioningStage(), kNeedsNetworkCreds); + EXPECT_FALSE(mAccess.AccessParams().GetPDCNetworkIdentity().HasValue()); + EXPECT_FALSE(mAccess.AccessParams().GetPDCPossessionNonce().HasValue()); + EXPECT_FALSE(mAccess.AccessParams().GetPDCClientIdentity().HasValue()); + EXPECT_FALSE(mAccess.AccessParams().GetPDCPossessionSignature().HasValue()); +} + +// What that walk-back does *not* do is give up the Network Client Identity registered for the +// configuration it is retrying: the registration is only rolled back on RemoveNetwork or at the end +// of the attempt, neither of which the walk-back goes through. So the retry runs into the +// single-outstanding-registration check in kPDCRegisterClientIdentity and fails the attempt, which is +// what finally revokes the stale registration -- see the second TODO above the IsScanNeeded() check +// in CommissioningStepFinished(). This test pins that behaviour rather than endorsing it; a retry +// that removed the old configuration first would not get here at all. +TEST_F(AutoCommissionerPDCTest, RetryingWithAnOutstandingRegistrationFailsTheAttempt) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + mAccess.AccessParams().SetAttemptWiFiNetworkScan(true); // see the test above + ASSERT_TRUE(mAccess.IsScanNeeded()); + + CompleteGetNetworkIdentity(); + EXPECT_EQ(CompleteWiFiNetworkSetup(), kStoppedAtPerformStep); + + // The registration this fixture cannot perform (kWiFiNetworkSetup was never sent, so neither was the + // step after it) would have left the commissioner holding exactly this. + Credentials::CertificateKeyIdStorage clientIdentifier{}; + ASSERT_EQ(Credentials::ExtractIdentifierFromChipNetworkIdentity(mClientIdentity.Identity(), + Credentials::MutableCertificateKeyId(clientIdentifier)), + CHIP_NO_ERROR); + DeviceCommissionerTestAccess deviceCommissionerAccess{ &mDeviceCommissioner }; + deviceCommissionerAccess.SetNetworkClientRegistration(&mRegistrar, ByteSpan(clientIdentifier)); + + CommissioneeDeviceProxy commissionee; + mAccess.SetCommissioneeDeviceProxy(&commissionee); + + // ConnectNetwork failed: the commissionee could not find the network we configured. + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kWiFiNetworkEnable; + report.Set( + app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum::kNetworkNotFound, CharSpan()); + ASSERT_EQ(mCommissioner.CommissioningStepFinished(CHIP_ERROR_INTERNAL, report), CHIP_NO_ERROR); + ASSERT_EQ(mDeviceCommissioner.GetCommissioningStage(), kNeedsNetworkCreds); + + // Walking back is not an end to the attempt, so we are still on the hook for the registration. + EXPECT_EQ(deviceCommissionerAccess.GetNetworkClientRegistrar(), &mRegistrar); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 0); + + // The application answers with credentials again and the flow runs up to the second registration, + // which the commissioner refuses. The registrar is left to fail an unexpected RegisterClient(). + mAccess.SetCommissioneeDeviceProxy(nullptr); // don't perform kWiFiNetworkSetup, we cannot send it + CompleteGetNetworkIdentity(); + mAccess.SetCommissioneeDeviceProxy(&commissionee); + ASSERT_EQ(CompleteWiFiNetworkSetup(), CHIP_NO_ERROR); + + const CompletionStatus & status = mAccess.AccessParams().GetCompletionStatus(); + EXPECT_EQ(status.err, CHIP_ERROR_INCORRECT_STATE); + ASSERT_TRUE(status.failedStage.HasValue()); + EXPECT_EQ(status.failedStage.Value(), kPDCRegisterClientIdentity); + + // Cleanup is what eventually revokes the identity the retry tripped over. + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_TRUE(mRegistrar.UnregisteredIdentifier().data_equal(ByteSpan(clientIdentifier))); + EXPECT_EQ(deviceCommissionerAccess.GetNetworkClientRegistrar(), nullptr); +} + +// Cleanup is the backstop, and the only reset a second commissioning attempt is guaranteed to get: +// DeviceCommissioner::Commission(NodeId) reuses the parameters from the previous attempt without +// going through SetCommissioningParameters(), so ClearExternalBufferDependentValues() does not +// necessarily run in between. Left behind, the Network Identity would be written to a commissionee +// that may be joining a different network, and -- worse -- the possession nonce would be reused as a +// challenge against a different commissionee. +TEST_F(AutoCommissionerPDCTest, CleanupClearsPDCParameters) +{ + Configure(PDCOnlyParams(), /* supportsPDC = */ true); + CompleteGetNetworkIdentity(); + EXPECT_EQ(CompleteWiFiNetworkSetup(), kStoppedAtPerformStep); + ASSERT_TRUE(mAccess.AccessParams().GetPDCNetworkIdentity().HasValue()); + ASSERT_TRUE(mAccess.AccessParams().GetPDCPossessionNonce().HasValue()); + + mAccess.CleanupCommissioning(); + + EXPECT_FALSE(mAccess.AccessParams().GetPDCNetworkIdentity().HasValue()); + EXPECT_FALSE(mAccess.AccessParams().GetPDCPossessionNonce().HasValue()); + EXPECT_FALSE(mAccess.AccessParams().GetPDCClientIdentity().HasValue()); + EXPECT_FALSE(mAccess.AccessParams().GetPDCPossessionSignature().HasValue()); +} + +// Cancellation and errors during either of the new stages must fall through to cleanup rather than +// advancing the flow. +TEST_F(AutoCommissionerPDCTest, NewStagesHonourStopAndError) +{ + for (CommissioningStage stage : { kPDCGetNetworkIdentity, kPDCRegisterClientIdentity }) + { + { + AutoCommissioner commissioner; + AutoCommissionerTestAccess access{ &commissioner }; + CHIP_ERROR err = CHIP_ERROR_INTERNAL; + EXPECT_EQ(access.AccessGetNextCommissioningStageInternal(stage, err), kCleanup); + } + { + AutoCommissioner commissioner; + AutoCommissionerTestAccess access{ &commissioner }; + commissioner.StopCommissioning(); + CHIP_ERROR err = CHIP_NO_ERROR; + EXPECT_EQ(access.AccessGetNextCommissioningStageInternal(stage, err), kCleanup); + } + } +} + +// CleanupCommissioning() may defer completing an attempt until an outstanding revocation lands, and +// a StopPairing() in that window discards the continuation that would have completed it. What keeps +// the attempt from being stranded is that StopPairing() follows up with a cancelled stage completion +// that routes back into kCleanup -- if a failed kCleanup ever mapped to kError instead, the re-entry +// would be suppressed and no completion callback would ever be delivered. +TEST_F(AutoCommissionerPDCTest, FailedCleanupRoutesBackToCleanup) +{ + for (CHIP_ERROR err : { CHIP_ERROR_CANCELLED, CHIP_ERROR_INTERNAL }) + { + AutoCommissioner commissioner; + AutoCommissionerTestAccess access{ &commissioner }; + EXPECT_EQ(access.AccessGetNextCommissioningStageInternal(kCleanup, err), kCleanup); + } +} + +// Being unable to reach the network's identity provider is a credentials-class failure, so the +// commissioner switches the attempt over to the secondary network rather than giving up. +TEST_F(AutoCommissionerPDCTest, GetNetworkIdentityFailureSwitchesToSecondaryNetwork) +{ + CommissioningParameters params = PDCOnlyParams(); + params.SetThreadOperationalDataset(ByteSpan()); + params.SetSupportsConcurrentConnection(true); + ASSERT_EQ(mCommissioner.SetCommissioningParameters(params), CHIP_NO_ERROR); + mAccess.SetCommissioner(&mDeviceCommissioner); + + ReadCommissioningInfo & info = mAccess.GetDeviceCommissioningInfo(); + info.network.wifi.endpoint = kRootEndpointId; + info.network.wifi.supportsPerDeviceCredentials = true; + info.network.thread.endpoint = 1; + mAccess.TryPrimaryNetwork(); + + CommissioningDelegate::CommissioningReport report; + report.stageCompleted = kPDCGetNetworkIdentity; + // The error is swallowed and the flow is redirected to the secondary network. + EXPECT_EQ(mCommissioner.CommissioningStepFinished(CHIP_ERROR_TIMEOUT, report), kStoppedAtPerformStep); + EXPECT_TRUE(mAccess.TryingSecondaryNetwork()); + + // We never got as far as writing a Wi-Fi configuration, so there is nothing on the commissionee + // to remove and the flow goes straight to the secondary network. + CHIP_ERROR err = CHIP_NO_ERROR; + EXPECT_EQ(mAccess.AccessGetNextCommissioningStageInternal(kPrimaryOperationalNetworkFailed, err), kThreadNetworkSetup); + EXPECT_EQ(err, CHIP_NO_ERROR); +} + +// --------------------------------------------------------------------------- +// DeviceCommissioner +// --------------------------------------------------------------------------- + +// PerformCommissioningStep() hangs the stage off a device proxy, but kPDCRegisterClientIdentity only +// talks to the registrar, so nothing here is ever reached. +class StubDeviceProxy : public DeviceProxy +{ +public: + void Disconnect() override {} + NodeId GetDeviceId() const override { return 0x1234; } + Messaging::ExchangeManager * GetExchangeManager() const override { return nullptr; } + Optional GetSecureSession() const override { return NullOptional; } + +private: + bool IsSecureConnected() const override { return false; } +}; + +// Captures the outcome of a stage driven directly via PerformCommissioningStep(), and stops there: +// returning an error would send the commissioner into cleanup instead. +class RecordingCommissioningDelegate : public CommissioningDelegate +{ +public: + // Stages the parameters a step is performed with, or read against. Spans are the caller's to keep + // alive, which the fixtures below do. + CHIP_ERROR SetCommissioningParameters(const CommissioningParameters & params) override + { + mParams = params; + return CHIP_NO_ERROR; + } + const CommissioningParameters & GetCommissioningParameters() const override { return mParams; } + void SetOperationalCredentialsDelegate(OperationalCredentialsDelegate *) override {} + CHIP_ERROR StartCommissioning(DeviceCommissioner *, CommissioneeDeviceProxy *) override { return CHIP_ERROR_NOT_IMPLEMENTED; } + + CHIP_ERROR CommissioningStepFinished(CHIP_ERROR err, CommissioningReport report) override + { + mCompletions++; + mLastError = err; + mLastStage = report.stageCompleted; + if (report.Is()) + { + mLastClientIdentityInfo.SetValue(report.Get()); + } + return CHIP_NO_ERROR; + } + + int mCompletions = 0; + CHIP_ERROR mLastError = CHIP_ERROR_INTERNAL; + CommissioningStage mLastStage = kError; + Optional mLastClientIdentityInfo; + +private: + CommissioningParameters mParams; +}; + +// Verifying the commissionee's proof of possession, and the rollback obligation the commissioner +// takes on when it registers a Network Client Identity. None of this involves the AutoCommissioner, +// so a fixed possession nonce stands in for the generated one. +class DeviceCommissionerPDCTest : public ::testing::Test +{ +protected: + // Puts the commissioner into the state it would be in after a successful kPDCRegisterClientIdentity: + // holding an obligation to roll mClientIdentity's registration back. + void SimulateSuccessfulRegistration() { mAccess.SetNetworkClientRegistration(&mRegistrar, ClientIdentifier()); } + + // Parameters as they stand when kPDCRegisterClientIdentity runs: the registrar to register with, plus + // the client identity and proof of possession the commissionee returned from kWiFiNetworkSetup. + CommissioningParameters RegisterClientIdentityParams(const TestNetworkIdentity & clientIdentity) + { + VerifyOrDie(clientIdentity.SignPossession(kPossessionNonce, mPossessionSignature) == CHIP_NO_ERROR); + + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, &mRegistrar)); + params.SetPDCClientIdentity(clientIdentity.Identity()); + params.SetPDCPossessionNonce(kPossessionNonce); + params.SetPDCPossessionSignature(mPossessionSignature.Span()); + return params; + } + + // Runs the stage that registers a client identity with the network. The registrar completes + // synchronously, so the stage has finished by the time this returns. + void PerformRegisterClientIdentity(CommissioningParameters & params) + { + mRegistrar.AcceptRegistrations(); // a no-op for a test that called FailRegistrations() + mCommissioner.PerformCommissioningStep(&mDeviceProxy, kPDCRegisterClientIdentity, params, &mDelegate, kRootEndpointId, + NullOptional); + } + + // The key identifier of mClientIdentity, i.e. what a rollback of its registration must name. + ByteSpan ClientIdentifier() + { + VerifyOrDie(Credentials::ExtractIdentifierFromChipNetworkIdentity( + mClientIdentity.Identity(), Credentials::MutableCertificateKeyId(mClientIdentifier)) == CHIP_NO_ERROR); + return ByteSpan(mClientIdentifier); + } + + DeviceCommissioner mCommissioner{}; + DeviceCommissionerTestAccess mAccess{ &mCommissioner }; + MockNetworkIdentityRegistrar mRegistrar; + StubDeviceProxy mDeviceProxy; + RecordingCommissioningDelegate mDelegate; + TestNetworkIdentity mClientIdentity; + TestNetworkIdentity mOtherIdentity; + Credentials::CertificateKeyIdStorage mClientIdentifier{}; + Crypto::P256ECDSASignature mPossessionSignature; +}; + +// The AutoCommissioner deliberately runs kWiFiNetworkSetup with PDC-only credentials even when the +// commissionee cannot do PDC, and relies on being rejected here to reach the ordinary step-failure +// path. Nothing is sent to the commissionee. +TEST_F(DeviceCommissionerPDCTest, RejectsPDCOnlyCredentialsWithoutIdentity) +{ + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, &mRegistrar)); + ASSERT_FALSE(params.GetPDCNetworkIdentity().HasValue()); + + mCommissioner.PerformCommissioningStep(&mDeviceProxy, kWiFiNetworkSetup, params, &mDelegate, kRootEndpointId, NullOptional); + + EXPECT_EQ(mDelegate.mLastStage, kWiFiNetworkSetup); + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_INVALID_ARGUMENT); +} + +// The commissionee's proof of possession is checked before its identity is registered with the +// network, and validating the identity yields the key identifier a rollback needs. +TEST_F(DeviceCommissionerPDCTest, AcceptsValidPossessionSignature) +{ + Crypto::P256ECDSASignature signature; + ASSERT_EQ(mClientIdentity.SignPossession(kPossessionNonce, signature), CHIP_NO_ERROR); + + Credentials::CertificateKeyIdStorage identifier{}; + EXPECT_EQ(DeviceCommissionerTestAccess::VerifyNetworkClientIdentity( + mClientIdentity.Identity(), signature.Span(), kPossessionNonce, Credentials::MutableCertificateKeyId(identifier)), + CHIP_NO_ERROR); + EXPECT_TRUE(ByteSpan(identifier).data_equal(ClientIdentifier())); +} + +TEST_F(DeviceCommissionerPDCTest, RejectsPossessionSignatureOverWrongNonce) +{ + uint8_t wrongNonce[CommissioningParameters::kPossessionNonceLen] = {}; + Crypto::P256ECDSASignature signature; + ASSERT_EQ(mClientIdentity.SignPossession(ByteSpan(wrongNonce), signature), CHIP_NO_ERROR); + + Credentials::CertificateKeyIdStorage identifier{}; + EXPECT_EQ(DeviceCommissionerTestAccess::VerifyNetworkClientIdentity( + mClientIdentity.Identity(), signature.Span(), kPossessionNonce, Credentials::MutableCertificateKeyId(identifier)), + CHIP_ERROR_INVALID_SIGNATURE); +} + +// A signature made by a different key must not pass, even though it is well formed. +TEST_F(DeviceCommissionerPDCTest, RejectsPossessionSignatureFromAnotherKey) +{ + Crypto::P256ECDSASignature signature; + ASSERT_EQ(mOtherIdentity.SignPossession(kPossessionNonce, signature), CHIP_NO_ERROR); + + Credentials::CertificateKeyIdStorage identifier{}; + EXPECT_EQ(DeviceCommissionerTestAccess::VerifyNetworkClientIdentity( + mClientIdentity.Identity(), signature.Span(), kPossessionNonce, Credentials::MutableCertificateKeyId(identifier)), + CHIP_ERROR_INVALID_SIGNATURE); +} + +TEST_F(DeviceCommissionerPDCTest, RejectsMalformedClientIdentity) +{ + uint8_t garbage[64] = {}; + Crypto::P256ECDSASignature signature; + ASSERT_EQ(mClientIdentity.SignPossession(kPossessionNonce, signature), CHIP_NO_ERROR); + + Credentials::CertificateKeyIdStorage identifier{}; + EXPECT_NE(DeviceCommissionerTestAccess::VerifyNetworkClientIdentity(ByteSpan(garbage), signature.Span(), kPossessionNonce, + Credentials::MutableCertificateKeyId(identifier)), + CHIP_NO_ERROR); +} + +// Rolling back revokes the identity we registered, naming it by the key identifier we recorded when +// we took the obligation on. +TEST_F(DeviceCommissionerPDCTest, RollsBackRegistration) +{ + SimulateSuccessfulRegistration(); + + mAccess.RollBackNetworkClientIdentity(); + + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_TRUE(mRegistrar.UnregisteredIdentifier().data_equal(ClientIdentifier())); + EXPECT_EQ(mAccess.GetNetworkClientRegistrar(), nullptr); +} + +// Rolling back is idempotent: the first one discharges the obligation, so a second is a no-op. Not +// hypothetical -- a mid-flight rollback is always followed by the one in cleanup. +TEST_F(DeviceCommissionerPDCTest, RollbackIsIdempotent) +{ + SimulateSuccessfulRegistration(); + + mAccess.RollBackNetworkClientIdentity(); + mAccess.RollBackNetworkClientIdentity(); + + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); +} + +// Nothing to roll back if we never got as far as registering an identity. +TEST_F(DeviceCommissionerPDCTest, RollingBackWithoutARegistrationIsANoOp) +{ + mAccess.RollBackNetworkClientIdentity(); + + EXPECT_EQ(mRegistrar.mUnregisterCalls, 0); +} + +// Registering an identity while the commissioner owns the rollback arms the obligation, so a later +// failure revokes exactly the identity that was registered. +TEST_F(DeviceCommissionerPDCTest, RegisteringAnIdentityArmsTheRollbackByDefault) +{ + CommissioningParameters params = RegisterClientIdentityParams(mClientIdentity); + ASSERT_TRUE(params.GetManagePDCClientIdentityRollback()); + PerformRegisterClientIdentity(params); + + EXPECT_EQ(mDelegate.mLastStage, kPDCRegisterClientIdentity); + EXPECT_EQ(mDelegate.mLastError, CHIP_NO_ERROR); + EXPECT_EQ(mRegistrar.mRegisterCalls, 1); + EXPECT_TRUE(mRegistrar.RegisteredIdentity().data_equal(mClientIdentity.Identity())); + EXPECT_EQ(mAccess.GetNetworkClientRegistrar(), &mRegistrar); + + mAccess.RollBackNetworkClientIdentity(); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_TRUE(mRegistrar.UnregisteredIdentifier().data_equal(ClientIdentifier())); +} + +// A registration that fails without a determinate outcome arms the rollback all the same: the +// AddClient it stands for may have reached the network with only the response going missing, and +// revoking an identity that was never registered is the cheaper of the two ways to be wrong. +TEST_F(DeviceCommissionerPDCTest, IndeterminateRegistrationFailureArmsTheRollback) +{ + CommissioningParameters params = RegisterClientIdentityParams(mClientIdentity); + mRegistrar.FailRegistrations(CHIP_ERROR_TIMEOUT, /* determinate = */ false); + PerformRegisterClientIdentity(params); + + EXPECT_EQ(mDelegate.mLastStage, kPDCRegisterClientIdentity); + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_TIMEOUT) << "the failure was swallowed"; + EXPECT_EQ(mRegistrar.mRegisterCalls, 1); + EXPECT_EQ(mAccess.GetNetworkClientRegistrar(), &mRegistrar); + + mAccess.RollBackNetworkClientIdentity(); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_TRUE(mRegistrar.UnregisteredIdentifier().data_equal(ClientIdentifier())); +} + +// A registrar that can say for certain that nothing was granted -- because it never got the command +// out, or because the network turned it down -- saves the attempt a pointless revocation. This is the +// only failure that leaves us owing nothing. +TEST_F(DeviceCommissionerPDCTest, DeterminateRegistrationFailureArmsNothing) +{ + CommissioningParameters params = RegisterClientIdentityParams(mClientIdentity); + mRegistrar.FailRegistrations(CHIP_ERROR_TIMEOUT, /* determinate = */ true); + PerformRegisterClientIdentity(params); + + EXPECT_EQ(mDelegate.mLastStage, kPDCRegisterClientIdentity); + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_TIMEOUT); + EXPECT_EQ(mRegistrar.mRegisterCalls, 1); + EXPECT_EQ(mAccess.GetNetworkClientRegistrar(), nullptr); + + mAccess.RollBackNetworkClientIdentity(); // as CleanupCommissioning would on failure + EXPECT_EQ(mRegistrar.mUnregisterCalls, 0); +} + +// A delegate that manages the registration itself owns the decision for a failed registration too, +// determinate or not, so there is still nothing for us to roll back. +TEST_F(DeviceCommissionerPDCTest, FailedRegistrationArmsNothingWhenTheDelegateManagesRollback) +{ + CommissioningParameters params = RegisterClientIdentityParams(mClientIdentity); + params.SetManagePDCClientIdentityRollback(false); + mRegistrar.FailRegistrations(CHIP_ERROR_TIMEOUT, /* determinate = */ false); + PerformRegisterClientIdentity(params); + + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_TIMEOUT); + EXPECT_EQ(mAccess.GetNetworkClientRegistrar(), nullptr); + + mAccess.RollBackNetworkClientIdentity(); // as CleanupCommissioning would on failure + EXPECT_EQ(mRegistrar.mUnregisterCalls, 0); +} + +// A delegate that manages the registration itself still gets the client registered with the network, +// but leaves us with nothing to roll back: the identity is now that delegate's to keep or revoke. +TEST_F(DeviceCommissionerPDCTest, RegisteringAnIdentityArmsNothingWhenTheDelegateManagesRollback) +{ + CommissioningParameters params = RegisterClientIdentityParams(mClientIdentity); + params.SetManagePDCClientIdentityRollback(false); + PerformRegisterClientIdentity(params); + + EXPECT_EQ(mDelegate.mLastStage, kPDCRegisterClientIdentity); + EXPECT_EQ(mDelegate.mLastError, CHIP_NO_ERROR); + EXPECT_EQ(mRegistrar.mRegisterCalls, 1); + EXPECT_TRUE(mRegistrar.RegisteredIdentity().data_equal(mClientIdentity.Identity())); + EXPECT_EQ(mAccess.GetNetworkClientRegistrar(), nullptr); + + mAccess.RollBackNetworkClientIdentity(); // as CleanupCommissioning would on failure + EXPECT_EQ(mRegistrar.mUnregisterCalls, 0); +} + +// Why the decision is latched at registration time rather than taken back afterwards: with the +// commissioner out of the picture nothing is ever outstanding, so a delegate that keeps several +// Network Client Identities alive is not blocked by the single-slot check on the second registration. +TEST_F(DeviceCommissionerPDCTest, DelegateManagedRollbackAllowsSeveralRegistrations) +{ + for (const TestNetworkIdentity * identity : { &mClientIdentity, &mOtherIdentity }) + { + CommissioningParameters params = RegisterClientIdentityParams(*identity); + params.SetManagePDCClientIdentityRollback(false); + PerformRegisterClientIdentity(params); + EXPECT_EQ(mDelegate.mLastError, CHIP_NO_ERROR); + } + + EXPECT_EQ(mDelegate.mCompletions, 2); + EXPECT_EQ(mRegistrar.mRegisterCalls, 2); +} + +// The shape of the NetworkConfigResponse a commissionee configured for Per-Device Credentials has to +// return. The commissioner checks this as it reads the response, rather than leaving it to the +// delegate, so that a non-conformant commissionee fails kWiFiNetworkSetup like any other Network +// Commissioning problem -- and so gets the same failover to the secondary network. +class DeviceCommissionerPDCResponseTest : public ::testing::Test +{ +protected: + void SetUp() override + { + ASSERT_EQ(mDelegate.SetCommissioningParameters(PDCParams()), CHIP_NO_ERROR); + mAccess.SetCommissioningDelegate(&mDelegate); + mAccess.SetDeviceBeingCommissioned(&mDeviceProxy); + mAccess.SetCommissioningStage(kWiFiNetworkSetup); + } + + // Parameters as they stand once kWiFiNetworkSetup has configured the commissionee for PDC, which + // is what obliges it to return a client identity and a proof of possession. + CommissioningParameters PDCParams() + { + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, &mRegistrar)); + params.SetPDCNetworkIdentity(mNetworkIdentity.Identity()); + params.SetPDCPossessionNonce(kPossessionNonce); + return params; + } + + // Delivers a successful AddOrUpdateWiFiNetwork response carrying the given PDC fields. + void DeliverResponse(Optional clientIdentity, Optional possessionSignature) + { + NetworkCommissioning::Commands::NetworkConfigResponse::DecodableType data; + data.networkingStatus = NetworkCommissioning::NetworkCommissioningStatusEnum::kSuccess; + data.clientIdentity = clientIdentity; + data.possessionSignature = possessionSignature; + DeviceCommissionerTestAccess::OnNetworkConfigResponse(&mCommissioner, data); + } + + // The proof of possession a conformant commissionee returns over our nonce. + ByteSpan PossessionSignature() + { + VerifyOrDie(mClientIdentity.SignPossession(kPossessionNonce, mPossessionSignature) == CHIP_NO_ERROR); + return mPossessionSignature.Span(); + } + + DeviceCommissioner mCommissioner{}; + DeviceCommissionerTestAccess mAccess{ &mCommissioner }; + MockNetworkIdentityRegistrar mRegistrar; + StubDeviceProxy mDeviceProxy; + RecordingCommissioningDelegate mDelegate; + TestNetworkIdentity mNetworkIdentity; + TestNetworkIdentity mClientIdentity; + Crypto::P256ECDSASignature mPossessionSignature; +}; + +TEST_F(DeviceCommissionerPDCResponseTest, AcceptsAConformantResponse) +{ + DeliverResponse(MakeOptional(mClientIdentity.Identity()), MakeOptional(PossessionSignature())); + + EXPECT_EQ(mDelegate.mLastStage, kWiFiNetworkSetup); + EXPECT_EQ(mDelegate.mLastError, CHIP_NO_ERROR); + EXPECT_EQ(mDelegate.mCompletions, 1); + ASSERT_TRUE(mDelegate.mLastClientIdentityInfo.HasValue()); + EXPECT_TRUE(mDelegate.mLastClientIdentityInfo.Value().clientIdentity.data_equal(mClientIdentity.Identity())); + EXPECT_TRUE(mDelegate.mLastClientIdentityInfo.Value().possessionSignature.data_equal(mPossessionSignature.Span())); +} + +// Each of the ways the response can fail to carry a usable identity fails the stage, and passes no +// PDC report on to the delegate. Failing the stage -- rather than the delegate refusing the report -- +// is what puts these on the ordinary network-failure path, which +// AutoCommissionerPDCTest.PDCOnlyFallsBackToSecondaryNetworkWhenCommissioneeLacksPDC covers. Note the +// signature itself is not checked here: that needs the possession nonce, and happens during +// kPDCRegisterClientIdentity. +TEST_F(DeviceCommissionerPDCResponseTest, RejectsAMissingClientIdentity) +{ + DeliverResponse(NullOptional, MakeOptional(PossessionSignature())); + + EXPECT_EQ(mDelegate.mLastStage, kWiFiNetworkSetup); + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_MISSING_TLV_ELEMENT); + EXPECT_FALSE(mDelegate.mLastClientIdentityInfo.HasValue()); +} + +TEST_F(DeviceCommissionerPDCResponseTest, RejectsAnOversizedClientIdentity) +{ + uint8_t oversized[Credentials::kMaxCHIPCompactNetworkIdentityLength + 1] = {}; + DeliverResponse(MakeOptional(ByteSpan(oversized)), MakeOptional(PossessionSignature())); + + EXPECT_EQ(mDelegate.mLastStage, kWiFiNetworkSetup); + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_MESSAGE_TOO_LONG); + EXPECT_FALSE(mDelegate.mLastClientIdentityInfo.HasValue()); +} + +TEST_F(DeviceCommissionerPDCResponseTest, RejectsAMissingPossessionSignature) +{ + DeliverResponse(MakeOptional(mClientIdentity.Identity()), NullOptional); + + EXPECT_EQ(mDelegate.mLastStage, kWiFiNetworkSetup); + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_MISSING_TLV_ELEMENT); + EXPECT_FALSE(mDelegate.mLastClientIdentityInfo.HasValue()); +} + +TEST_F(DeviceCommissionerPDCResponseTest, RejectsAWrongLengthPossessionSignature) +{ + uint8_t truncated[CommissioningParameters::kPossessionSignatureLen - 1] = {}; + DeliverResponse(MakeOptional(mClientIdentity.Identity()), MakeOptional(ByteSpan(truncated))); + + EXPECT_EQ(mDelegate.mLastStage, kWiFiNetworkSetup); + EXPECT_EQ(mDelegate.mLastError, CHIP_ERROR_INVALID_SIGNATURE); + EXPECT_FALSE(mDelegate.mLastClientIdentityInfo.HasValue()); +} + +// Whether the response is held to the PDC shape is decided by what we asked the commissionee for, not +// by what it happened to send: a commissionee volunteering PDC fields we did not configure it for is +// not held to them, and nothing is reported that the delegate has no nonce to check. +TEST_F(DeviceCommissionerPDCResponseTest, IgnoresPDCFieldsWhenPDCWasNotRequested) +{ + CommissioningParameters params; + params.SetWiFiCredentials(WiFiCredentials(kSsid, kPassphrase)); + ASSERT_EQ(mDelegate.SetCommissioningParameters(params), CHIP_NO_ERROR); + + uint8_t truncated[CommissioningParameters::kPossessionSignatureLen - 1] = {}; + DeliverResponse(MakeOptional(mClientIdentity.Identity()), MakeOptional(ByteSpan(truncated))); + + EXPECT_EQ(mDelegate.mLastStage, kWiFiNetworkSetup); + EXPECT_EQ(mDelegate.mLastError, CHIP_NO_ERROR); + EXPECT_FALSE(mDelegate.mLastClientIdentityInfo.HasValue()); +} + +// Waiting for a revocation. Revoking a Network Client Identity is best-effort and its outcome changes +// nothing, so the commissioner normally lets the call run on unwatched. It waits only where the +// ordering is observable: removing a Wi-Fi configuration, because a retry may want to register another +// identity, against this registrar or a different one. +class DeviceCommissionerRevocationTest : public ::testing::Test +{ +protected: + void SetUp() override + { + mCommissioner.RegisterPairingDelegate(&mPairingDelegate); + mAccess.SetDeviceBeingCommissioned(&mDeviceProxy); + + // As after a successful kPDCRegisterClientIdentity. + VerifyOrDie(Credentials::ExtractIdentifierFromChipNetworkIdentity( + mClientIdentity.Identity(), Credentials::MutableCertificateKeyId(mClientIdentifier)) == CHIP_NO_ERROR); + mAccess.SetNetworkClientRegistration(&mRegistrar, ByteSpan(mClientIdentifier)); + } + + // Delivers a successful NetworkConfigResponse as though it had arrived during the given stage. + void DeliverSuccessResponse(CommissioningStage stage) + { + mAccess.SetCommissioningStage(stage); + + NetworkCommissioning::Commands::NetworkConfigResponse::DecodableType data; + data.networkingStatus = NetworkCommissioning::NetworkCommissioningStatusEnum::kSuccess; + DeviceCommissionerTestAccess::OnNetworkConfigResponse(&mCommissioner, data); + } + + class RecordingPairingDelegate : public DevicePairingDelegate + { + public: + void OnCommissioningStatusUpdate(PeerId, CommissioningStage stageCompleted, CHIP_ERROR error) override + { + mStatusUpdates++; + mLastStage = stageCompleted; + mLastError = error; + } + + int mStatusUpdates = 0; + CommissioningStage mLastStage = kError; + CHIP_ERROR mLastError = CHIP_ERROR_INTERNAL; + }; + + DeviceCommissioner mCommissioner{}; + DeviceCommissionerTestAccess mAccess{ &mCommissioner }; + MockNetworkIdentityRegistrar mRegistrar; + StubDeviceProxy mDeviceProxy; + RecordingPairingDelegate mPairingDelegate; + TestNetworkIdentity mClientIdentity; + Credentials::CertificateKeyIdStorage mClientIdentifier{}; +}; + +// A registrar that completes re-entrantly leaves nothing to wait for, so the stage is completed inline +// as it would be without PDC -- exactly once, not once here and again from the continuation. This is +// the case a warm cache produces, and the one all the other rollback tests exercise. +TEST_F(DeviceCommissionerRevocationTest, ReentrantRevocationCompletesTheStageExactlyOnce) +{ + DeliverSuccessResponse(kRemoveWiFiNetworkConfig); + + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_FALSE(mRegistrar.HasPendingRevocation()); + EXPECT_EQ(mPairingDelegate.mStatusUpdates, 1); + EXPECT_EQ(mPairingDelegate.mLastStage, kRemoveWiFiNetworkConfig); + EXPECT_EQ(mPairingDelegate.mLastError, CHIP_NO_ERROR); +} + +// The obligation is discharged when the revocation is handed over, not when it completes: there is +// nothing further we could do about a failure, and a second rollback must not repeat the call. +TEST_F(DeviceCommissionerRevocationTest, ObligationIsGivenUpBeforeTheRevocationCompletes) +{ + mRegistrar.DeferRevocations(); + + mAccess.RollBackNetworkClientIdentity(); + ASSERT_TRUE(mRegistrar.HasPendingRevocation()); + + EXPECT_EQ(mAccess.GetNetworkClientRegistrar(), nullptr); + mAccess.RollBackNetworkClientIdentity(); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 1); +} + +// Removing the Wi-Fi configuration finishes only once the identity it was using has been given up, so +// that a retry can register another one without tripping over the one we are abandoning. +TEST_F(DeviceCommissionerRevocationTest, WiFiConfigRemovalWaitsForTheRevocation) +{ + mRegistrar.DeferRevocations(); + + DeliverSuccessResponse(kRemoveWiFiNetworkConfig); + ASSERT_EQ(mRegistrar.mUnregisterCalls, 1); + EXPECT_EQ(mPairingDelegate.mStatusUpdates, 0) << "stage completed before the revocation did"; + + mRegistrar.CompleteRevocation(); + EXPECT_EQ(mPairingDelegate.mStatusUpdates, 1); + EXPECT_EQ(mPairingDelegate.mLastStage, kRemoveWiFiNetworkConfig); + EXPECT_EQ(mPairingDelegate.mLastError, CHIP_NO_ERROR); +} + +// A revocation we could not complete is not a reason to fail the removal: the entry is left for an +// out-of-band audit, and the commissionee no longer has the configuration that used it either way. +TEST_F(DeviceCommissionerRevocationTest, WiFiConfigRemovalSucceedsDespiteAFailedRevocation) +{ + mRegistrar.DeferRevocations(); + + DeliverSuccessResponse(kRemoveWiFiNetworkConfig); + mRegistrar.CompleteRevocation(CHIP_ERROR_TIMEOUT); + + EXPECT_EQ(mPairingDelegate.mStatusUpdates, 1); + EXPECT_EQ(mPairingDelegate.mLastError, CHIP_NO_ERROR); +} + +// Removing the Thread configuration has no Network Client Identity to give up, so nothing is waited +// for -- and nothing is revoked, because a Wi-Fi registration is not ours to withdraw here. +TEST_F(DeviceCommissionerRevocationTest, ThreadConfigRemovalDoesNotWait) +{ + mRegistrar.DeferRevocations(); + + DeliverSuccessResponse(kRemoveThreadNetworkConfig); + + EXPECT_EQ(mRegistrar.mUnregisterCalls, 0); + EXPECT_EQ(mPairingDelegate.mStatusUpdates, 1); + EXPECT_EQ(mPairingDelegate.mLastStage, kRemoveThreadNetworkConfig); +} + +// Giving up on a revocation we are no longer waiting for must not keep the callback a new one needs. +// A registrar holds us to one revocation at a time, so without the commissioner cancelling first the +// second one would simply be refused. Cancelling only drops our tracking of it; the RemoveClient may +// still take effect at the network, which is no worse than one we never waited for in the first place. +TEST_F(DeviceCommissionerRevocationTest, StaleRevocationIsAbandonedForANewOne) +{ + mRegistrar.DeferRevocations(); + mAccess.RollBackNetworkClientIdentity(); + ASSERT_TRUE(mRegistrar.HasPendingRevocation()); + + mAccess.SetNetworkClientRegistration(&mRegistrar, ByteSpan(mClientIdentifier)); + mAccess.RollBackNetworkClientIdentity(); + + EXPECT_EQ(mRegistrar.mCancelledRevocations, 1); + EXPECT_EQ(mRegistrar.mUnregisterCalls, 2); + EXPECT_TRUE(mRegistrar.HasPendingRevocation()) << "the registrar refused the second revocation"; +} + +// Cancelling the interactions of the attempt drops the continuation waiting on the revocation, but +// deliberately not the revocation itself: it undoes a registration we gave up when we issued it, so +// abandoning it would strand the entry on the network for no reason. This is why StopPairing() does +// not leave the registrar idle, as NetworkIdentityRegistrar documents. +TEST_F(DeviceCommissionerRevocationTest, CancellingInteractionsLeavesTheRevocationRunning) +{ + mRegistrar.DeferRevocations(); + + DeliverSuccessResponse(kRemoveWiFiNetworkConfig); + ASSERT_TRUE(mRegistrar.HasPendingRevocation()); + + mAccess.CancelCommissioningInteractions(); + + EXPECT_EQ(mRegistrar.mCancelledRevocations, 0); + EXPECT_TRUE(mRegistrar.HasPendingRevocation()); + + // The stage we were holding open is not completed behind whoever cancelled us: they are finishing + // the attempt themselves, and completing it here would report a stage they have moved past. So + // letting the revocation land must run the base variant, not the continuation we just dropped. + mRegistrar.CompleteRevocation(); + EXPECT_EQ(mPairingDelegate.mStatusUpdates, 0); +} + +// ...and the wait must not be left lying around either, or the next revocation -- which nobody is +// waiting for -- would complete whatever stage the cancelled one was holding open. +TEST_F(DeviceCommissionerRevocationTest, CancellingAWaitDoesNotLeaveItForTheNextRevocation) +{ + mRegistrar.DeferRevocations(); + DeliverSuccessResponse(kRemoveWiFiNetworkConfig); + mAccess.CancelCommissioningInteractions(); + ASSERT_EQ(mPairingDelegate.mStatusUpdates, 0); + + // A fresh attempt gets as far as registering an identity, and then gives it up again. + mAccess.SetNetworkClientRegistration(&mRegistrar, ByteSpan(mClientIdentifier)); + mAccess.RollBackNetworkClientIdentity(); + ASSERT_TRUE(mRegistrar.HasPendingRevocation()); + mRegistrar.CompleteRevocation(); + + EXPECT_EQ(mPairingDelegate.mStatusUpdates, 0) << "a stage was completed by a revocation nobody waited for"; +} +} // namespace diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h index f4f8eebcc5d5..0701d3bc36fc 100644 --- a/src/credentials/CHIPCert.h +++ b/src/credentials/CHIPCert.h @@ -28,12 +28,14 @@ #pragma once #include +#include #include #include #include #include #include +#include #include #include #include @@ -419,6 +421,31 @@ using MutableCertificateKeyId = FixedSpan; */ using CertificateKeyIdStorage = std::array; +/** + * @def ChipLogFormatKeyId + * @def ChipLogValueKeyId(id) + * + * @brief Logging format and value macros for a CertificateKeyId, rendered as a hexadecimal big + * endian value. Takes a CertificateKeyId or a value implicitly convertible to it, e.g. a + * uint8_t[20] or a CertificateKeyIdStorage. + * + * NOTE: The argument to ChipLogValueKeyId is evaluated multiple times. + * + * Usage: + * ChipLogProgress(Zcl, "Identifier: " ChipLogFormatKeyId, ChipLogValueKeyId(id)); + */ +#define ChipLogFormatKeyId "%08" PRIX32 "%08" PRIX32 "%08" PRIX32 "%08" PRIX32 "%08" PRIX32 +// clang-format off +#define ChipLogValueKeyId(id) \ + chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data()), \ + chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 4), \ + chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 8), \ + chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 12), \ + chip::Encoding::BigEndian::Get32(chip::Credentials::CertificateKeyId(id).data() + 16) +// clang-format on + +static_assert(CertificateKeyId::size() == 20); // ChipLog{Format,Value}KeyId hard-code the size + /** * @brief A data structure for holding a P256 ECDSA signature, without the ownership of it. */ diff --git a/src/lib/core/CHIPCallback.h b/src/lib/core/CHIPCallback.h index 98d394c1a000..f800d0214743 100644 --- a/src/lib/core/CHIPCallback.h +++ b/src/lib/core/CHIPCallback.h @@ -522,6 +522,10 @@ class CallbackDeque : protected Cancelable private: static void _Dequeue(Cancelable * ca) { + // The static analyzer does not model the circular list invariant (ca->mPrev->mNext == ca), + // so the store below appears not to update the head of the containing deque. On a drain + // loop it therefore believes First() can return a node that Invalidate() already nulled out. + // NOLINTNEXTLINE(clang-analyzer-core.NullDereference) ca->mNext->mPrev = ca->mPrev; ca->mPrev->mNext = ca->mNext; } diff --git a/src/python_testing/TC_CGEN_2_2.py b/src/python_testing/TC_CGEN_2_2.py index 7f65b530d6db..8e619a67bb89 100644 --- a/src/python_testing/TC_CGEN_2_2.py +++ b/src/python_testing/TC_CGEN_2_2.py @@ -487,7 +487,7 @@ async def test_TC_CGEN_2_2(self): # Commissioning stage numbers - we should find a better way to match these to the C++ code # CommissioningDelegate.h # TODO: https://github.com/project-chip/connectedhomeip/issues/36629 - kFindOperationalForCommissioningComplete = 30 + kFindOperationalForCommissioningComplete = 32 log.info( 'Step #21 - TH2 Commissioning stage SetTestCommissionerPrematureCompleteAfter enum: %s', kFindOperationalForCommissioningComplete)