diff --git a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h index b72b2845ebf47c..09788507210aaf 100644 --- a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h +++ b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h @@ -48,7 +48,9 @@ class OpenCommissioningWindowCommand : public CHIPCommand &mIteration, "Number of PBKDF iterations to use to derive the verifier. Ignored if 'option' is 0."); AddArgument("discriminator", 0, 4096, &mDiscriminator, "Discriminator to use for advertising. Ignored if 'option' is 0."); AddArgument("timeout", 0, UINT16_MAX, &mTimeout, "Time, in seconds, before this command is considered to have timed out."); - AddArgument("salt", &mSalt, "Salt payload encoded in hexadecimal. Random salt will be generated if absent"); + AddArgument("salt", &mSalt, + "Salt payload encoded in hexadecimal. Random salt will be generated if absent. " + "This needs to be present if verifier is provided, corresponding to salt used for generating verifier"); AddArgument("verifier", &mVerifier, "PAKE Passcode verifier encoded in hexadecimal format. Will be generated from random setup pin and other " "params if absent"); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 85eeeae165a099..1c4b490faa891a 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -167,16 +167,6 @@ struct CommissionerInitParams : public ControllerInitParams Credentials::DeviceAttestationVerifier * deviceAttestationVerifier = nullptr; }; -// Interface class for DeviceController methods that need to be mocked -class IDeviceController -{ -public: - virtual ~IDeviceController() = default; - virtual CHIP_ERROR GetConnectedDevice(NodeId peerNodeId, chip::Callback::Callback * onConnection, - chip::Callback::Callback * onFailure, - TransportPayloadCapability transportPayloadCapability) = 0; -}; - /** * @brief * Controller applications can use this class to communicate with already paired CHIP devices. The @@ -185,7 +175,7 @@ class IDeviceController * and device pairing information for individual devices). Alternatively, this class can retrieve the * relevant information when the application tries to communicate with the device */ -class DLL_EXPORT DeviceController : public AbstractDnssdDiscoveryController, IDeviceController +class DLL_EXPORT DeviceController : public AbstractDnssdDiscoveryController { public: DeviceController(); @@ -253,10 +243,10 @@ class DLL_EXPORT DeviceController : public AbstractDnssdDiscoveryController, IDe * An error return from this function means that neither callback has been * called yet, and neither callback will be called in the future. */ - CHIP_ERROR + virtual CHIP_ERROR GetConnectedDevice(NodeId peerNodeId, Callback::Callback * onConnection, - chip::Callback::Callback * onFailure, - TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kMRPPayload) override + Callback::Callback * onFailure, + TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kMRPPayload) { VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); mSystemState->CASESessionMgr()->FindOrEstablishSession(ScopedNodeId(peerNodeId, GetFabricIndex()), onConnection, onFailure, diff --git a/src/controller/CommissioningWindowOpener.cpp b/src/controller/CommissioningWindowOpener.cpp index b0c219f4699c80..255d2df4e967aa 100644 --- a/src/controller/CommissioningWindowOpener.cpp +++ b/src/controller/CommissioningWindowOpener.cpp @@ -98,7 +98,8 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const Commissionin } mSetupPayload.version = 0; - mSetupPayload.discriminator.SetLongValue(params.GetDiscriminator()); + mDiscriminator.SetLongValue(params.GetDiscriminator()); + mSetupPayload.discriminator = mDiscriminator; mSetupPayload.rendezvousInformation.SetValue(RendezvousInformationFlag::kOnNetwork); if (params.HasSalt()) @@ -156,10 +157,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const Commissionin mCommissioningWindowTimeout = params.GetTimeout(); mPBKDFIterations = params.GetIteration(); mCommissioningWindowOption = CommissioningWindowOption::kTokenWithProvidedPIN; - mSetupPayload = SetupPayload(); - mSetupPayload.version = 0; - mSetupPayload.discriminator.SetLongValue(params.GetDiscriminator()); - mSetupPayload.rendezvousInformation.SetValue(RendezvousInformationFlag::kOnNetwork); + mDiscriminator.SetLongValue(params.GetDiscriminator()); mNextStep = Step::kOpenCommissioningWindow; @@ -184,7 +182,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowInternal(Messaging: AdministratorCommissioning::Commands::OpenCommissioningWindow::Type request; request.commissioningTimeout = mCommissioningWindowTimeout.count(); request.PAKEPasscodeVerifier = serializedVerifierSpan; - request.discriminator = mSetupPayload.discriminator.GetLongValue(); + request.discriminator = mDiscriminator.GetLongValue(); request.iterations = mPBKDFIterations; request.salt = mPBKDFSalt; diff --git a/src/controller/CommissioningWindowOpener.h b/src/controller/CommissioningWindowOpener.h index 2fb97d424fc06a..b657345ca53219 100644 --- a/src/controller/CommissioningWindowOpener.h +++ b/src/controller/CommissioningWindowOpener.h @@ -164,6 +164,7 @@ class CommissioningWindowOpener Callback::Callback * mCommissioningWindowVerifierCallback = nullptr; Callback::Callback * mBasicCommissioningWindowCallback = nullptr; SetupPayload mSetupPayload; + SetupDiscriminator mDiscriminator{}; NodeId mNodeId = kUndefinedNodeId; System::Clock::Seconds16 mCommissioningWindowTimeout = System::Clock::kZero; CommissioningWindowOption mCommissioningWindowOption = CommissioningWindowOption::kOriginalSetupCode; diff --git a/src/controller/CommissioningWindowParams.h b/src/controller/CommissioningWindowParams.h index a653560e6923d5..ba8667006369b4 100644 --- a/src/controller/CommissioningWindowParams.h +++ b/src/controller/CommissioningWindowParams.h @@ -40,7 +40,11 @@ class CommissioningWindowCommonParams public: CommissioningWindowCommonParams() = default; - NodeId GetNodeId() const { return mNodeId; } + NodeId GetNodeId() const + { + VerifyOrDie(mNodeId != kUndefinedNodeId); + return mNodeId; + } // The node identifier of device Derived & SetNodeId(NodeId nodeId) { @@ -67,18 +71,19 @@ class CommissioningWindowCommonParams } // The long discriminator for the DNS-SD advertisement. - uint16_t GetDiscriminator() const { return mDiscriminator; } + uint16_t GetDiscriminator() const { return mDiscriminator.Value(); } + bool HasDiscriminator() const { return mDiscriminator.HasValue(); } Derived & SetDiscriminator(uint16_t discriminator) { - mDiscriminator = discriminator; + mDiscriminator = MakeOptional(discriminator); return static_cast(*this); } private: - NodeId mNodeId = 0; - System::Clock::Seconds16 mTimeout = System::Clock::Seconds16(300); - uint32_t mIteration = 1000; - uint16_t mDiscriminator = 0; + NodeId mNodeId = kUndefinedNodeId; + System::Clock::Seconds16 mTimeout = System::Clock::Seconds16(300); // Defaulting + uint32_t mIteration = 1000; // Defaulting + Optional mDiscriminator = NullOptional; // Using optional type to avoid picking a sentinnel in valid range }; class CommissioningWindowPasscodeParams : public CommissioningWindowCommonParams @@ -87,9 +92,8 @@ class CommissioningWindowPasscodeParams : public CommissioningWindowCommonParams CommissioningWindowPasscodeParams() = default; bool HasSetupPIN() const { return mSetupPIN.HasValue(); } - // Get the value of salt if present. - // Returns 0 if absent, make sure to check HasSetupPIN() if a valid value is required. - uint32_t GetSetupPIN() const { return mSetupPIN.ValueOr(0); } + // Get the value of setup PIN (Passcode) if present, crashes otherwise. + uint32_t GetSetupPIN() const { return mSetupPIN.Value(); } // The setup PIN (Passcode) to use. A random one will be generated if not provided. CommissioningWindowPasscodeParams & SetSetupPIN(uint32_t setupPIN) { return SetSetupPIN(MakeOptional(setupPIN)); } // The setup PIN (Passcode) to use. A random one will be generated if NullOptional is used. @@ -148,21 +152,21 @@ class CommissioningWindowVerifierParams : public CommissioningWindowCommonParams public: CommissioningWindowVerifierParams() = default; - ByteSpan GetVerifier() const { return mVerifier; } + ByteSpan GetVerifier() const { return mVerifier.Value(); } // The PAKE passcode verifier generated with enclosed iterations, salt and not-enclosed passcode. CommissioningWindowVerifierParams & SetVerifier(ByteSpan verifier) { - mVerifier = verifier; + mVerifier = MakeOptional(verifier); return *this; } - ByteSpan GetSalt() const { return mSalt; } + ByteSpan GetSalt() const { return mSalt.Value(); } // The salt that was used to generate the verifier. // It must be at least kSpake2p_Min_PBKDF_Salt_Length bytes. // Note: This is REQUIRED when verifier is used CommissioningWindowVerifierParams & SetSalt(ByteSpan salt) { - mSalt = salt; + mSalt = MakeOptional(salt); return *this; } @@ -176,8 +180,8 @@ class CommissioningWindowVerifierParams : public CommissioningWindowCommonParams } private: - ByteSpan mVerifier; - ByteSpan mSalt; + Optional mSalt; + Optional mVerifier; Callback::Callback * mCallback = nullptr; }; diff --git a/src/controller/tests/TestCommissioningWindowOpener.cpp b/src/controller/tests/TestCommissioningWindowOpener.cpp index dbaba8016eb53c..01aa42fbe8b17a 100644 --- a/src/controller/tests/TestCommissioningWindowOpener.cpp +++ b/src/controller/tests/TestCommissioningWindowOpener.cpp @@ -82,7 +82,7 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Success) EXPECT_EQ(err, CHIP_NO_ERROR); } -TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_NoSalt) +TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_InvalidSalt) { Callback::Callback callback(OCWVerifierCallback, this); @@ -91,12 +91,13 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_No .SetTimeout(300) .SetIteration(sTestSpake2p01_IterationCount) .SetDiscriminator(3840) + .SetSalt(ByteSpan()) .SetVerifier(ByteSpan(sTestSpake2p01_SerializedVerifier)) .SetCallback(&callback)); EXPECT_EQ(err, CHIP_ERROR_INVALID_ARGUMENT); } -TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_NoVerifier) +TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_InvalidVerifier) { Callback::Callback callback(OCWVerifierCallback, this); @@ -106,6 +107,7 @@ TEST_F(TestCommissioningWindowOpener, OpenCommissioningWindowVerifier_Failure_No .SetIteration(sTestSpake2p01_IterationCount) .SetDiscriminator(3840) .SetSalt(ByteSpan(sTestSpake2p01_Salt)) + .SetVerifier(ByteSpan()) .SetCallback(&callback)); EXPECT_EQ(err, CHIP_ERROR_INVALID_ARGUMENT); }