Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update code PairDevice calls to work with network params. #14429

Merged
merged 13 commits into from
Mar 9, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Better way to do commissioning parameters
That whole "Add" thing was terrible.
cecille committed Jan 27, 2022
commit f165a51696e3961f7fe92b624e76760b06b276fa
82 changes: 14 additions & 68 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
@@ -25,16 +25,6 @@
namespace chip {
namespace Controller {

AutoCommissioner::AutoCommissioner(DeviceCommissioner * commissioner) : mCommissioner(commissioner)
{
ChipLogProgress(Controller, "Setting attestation nonce to random value");
Crypto::DRBG_get_bytes(mAttestationNonce, sizeof(mAttestationNonce));
mParams.SetAttestationNonce(ByteSpan(mAttestationNonce, sizeof(mAttestationNonce)));
ChipLogProgress(Controller, "Setting CSR nonce to random value");
Crypto::DRBG_get_bytes(mCSRNonce, sizeof(mCSRNonce));
mParams.SetCSRNonce(ByteSpan(mCSRNonce, sizeof(mCSRNonce)));
}

AutoCommissioner::~AutoCommissioner()
{
ReleaseDAC();
@@ -49,62 +39,6 @@ void AutoCommissioner::SetOperationalCredentialsDelegate(OperationalCredentialsD
CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParameters & params)
{
mParams = params;
// Call AddCommissioningParameters to swap bytespans to internal memory.
return AddCommissioningParameters(params);
}

CHIP_ERROR AutoCommissioner::AddCommissioningParameters(const CommissioningParameters & params)
{
if (params.GetFailsafeTimerSeconds().HasValue())
{
mParams.SetFailsafeTimerSeconds(params.GetFailsafeTimerSeconds().Value());
}
if (params.GetNOCChainGenerationParameters().HasValue())
{
mParams.SetNOCChainGenerationParameters(params.GetNOCChainGenerationParameters().Value());
}
if (params.GetRootCert().HasValue())
{
mParams.SetRootCert(params.GetRootCert().Value());
}
if (params.GetNoc().HasValue())
{
mParams.SetNoc(params.GetNoc().Value());
}
if (params.GetIcac().HasValue())
{
mParams.SetIcac(params.GetIcac().Value());
}
if (params.GetIpk().HasValue())
{
mParams.SetIpk(params.GetIpk().Value());
}
if (params.GetAdminSubject().HasValue())
{
mParams.SetAdminSubject(params.GetAdminSubject().Value());
}
if (params.GetAdminSubject().HasValue())
{
mParams.SetAdminSubject(params.GetAdminSubject().Value());
}
if (params.GetAttestationElements().HasValue())
{
mParams.SetAttestationElements(params.GetAttestationElements().Value());
}
if (params.GetAttestationSignature().HasValue())
{
mParams.SetAttestationSignature(params.GetAttestationSignature().Value());
}
if (params.GetPAI().HasValue())
{
ReleasePAI();
SetPAI(params.GetPAI().Value());
}
if (params.GetDAC().HasValue())
{
ReleaseDAC();
SetDAC(params.GetDAC().Value());
}
if (params.GetThreadOperationalDataset().HasValue())
{
ByteSpan dataset = params.GetThreadOperationalDataset().Value();
@@ -132,20 +66,32 @@ CHIP_ERROR AutoCommissioner::AddCommissioningParameters(const CommissioningParam
mParams.SetWiFiCredentials(
WiFiCredentials(ByteSpan(mSsid, creds.ssid.size()), ByteSpan(mCredentials, creds.credentials.size())));
}
// If the AttestationNonce is passed in, using that else using a random one..
if (params.GetAttestationNonce().HasValue())
{
ChipLogProgress(Controller, "Setting attestation nonce from parameters");
VerifyOrReturnError(params.GetAttestationNonce().Value().size() == sizeof(mAttestationNonce), CHIP_ERROR_INVALID_ARGUMENT);
memcpy(mAttestationNonce, params.GetAttestationNonce().Value().data(), params.GetAttestationNonce().Value().size());
mParams.SetAttestationNonce(ByteSpan(mAttestationNonce, sizeof(mAttestationNonce)));
}
else
{
ChipLogProgress(Controller, "Setting attestation nonce to random value");
Crypto::DRBG_get_bytes(mAttestationNonce, sizeof(mAttestationNonce));
}
mParams.SetAttestationNonce(ByteSpan(mAttestationNonce, sizeof(mAttestationNonce)));

if (params.GetCSRNonce().HasValue())
{
ChipLogProgress(Controller, "Setting CSR nonce from parameters");
VerifyOrReturnError(params.GetCSRNonce().Value().size() == sizeof(mCSRNonce), CHIP_ERROR_INVALID_ARGUMENT);
memcpy(mCSRNonce, params.GetCSRNonce().Value().data(), params.GetCSRNonce().Value().size());
mParams.SetCSRNonce(ByteSpan(mCSRNonce, sizeof(mCSRNonce)));
}
else
{
ChipLogProgress(Controller, "Setting CSR nonce to random value");
Crypto::DRBG_get_bytes(mCSRNonce, sizeof(mCSRNonce));
}
mParams.SetCSRNonce(ByteSpan(mCSRNonce, sizeof(mCSRNonce)));

return CHIP_NO_ERROR;
}
7 changes: 1 addition & 6 deletions src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
@@ -28,14 +28,9 @@ class DeviceCommissioner;
class AutoCommissioner : public CommissioningDelegate
{
public:
AutoCommissioner(DeviceCommissioner * commissioner);
AutoCommissioner(DeviceCommissioner * commissioner) : mCommissioner(commissioner) {}
~AutoCommissioner();

CHIP_ERROR SetCommissioningParameters(const CommissioningParameters & params);

// Overwrites only the given commissioning parameters.
CHIP_ERROR AddCommissioningParameters(const CommissioningParameters & params);

void SetOperationalCredentialsDelegate(OperationalCredentialsDelegate * operationalCredentialsDelegate);

void StartCommissioning(CommissioneeDeviceProxy * proxy);
17 changes: 10 additions & 7 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
@@ -746,7 +746,7 @@ CHIP_ERROR DeviceCommissioner::GetConnectedDevice(NodeId deviceId, Callback::Cal

CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, const char * setUpCode, const CommissioningParameters & params)
{
ReturnErrorOnFailure(mAutoCommissioner.AddCommissioningParameters(params));
ReturnErrorOnFailure(mAutoCommissioner.SetCommissioningParameters(params));
return mSetUpCodePairer.PairDevice(remoteDeviceId, setUpCode);
}

@@ -757,8 +757,8 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, const char * se

CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParameters & params)
{
CommissioningParameters commissioningParams;
return PairDevice(remoteDeviceId, params, commissioningParams);
ReturnErrorOnFailure(EstablishPASEConnection(remoteDeviceId, params));
return Commission(remoteDeviceId);
}

CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParameters & rendezvousParams,
@@ -881,6 +881,12 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
}

CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId, CommissioningParameters & params)
{
ReturnErrorOnFailure(mAutoCommissioner.SetCommissioningParameters(params));
return Commission(remoteDeviceId);
}

CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId)
{
// TODO(cecille): Can we get rid of mDeviceBeingCommissioned and use the remote id instead? Would require storing the
// commissioning stage in the device.
@@ -902,7 +908,6 @@ CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId, CommissioningPa
OnSessionEstablishmentTimeoutCallback, this);

mAutoCommissioner.SetOperationalCredentialsDelegate(mOperationalCredentialsDelegate);
ReturnErrorOnFailure(mAutoCommissioner.AddCommissioningParameters(params));
if (device->IsSecureConnected())
{
mAutoCommissioner.StartCommissioning(device);
@@ -1609,9 +1614,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
GeneralCommissioningCluster genCom;
// TODO: should get the endpoint information from the descriptor cluster.
SetupCluster(genCom, proxy, endpoint, timeout);
constexpr uint16_t kDefaultFailsafeSeconds = 60;
genCom.ArmFailSafe(mSuccess.Cancel(), mFailure.Cancel(), params.GetFailsafeTimerSeconds().ValueOr(kDefaultFailsafeSeconds),
breadcrumb, kCommandTimeoutMs);
genCom.ArmFailSafe(mSuccess.Cancel(), mFailure.Cancel(), params.GetFailsafeTimerSeconds(), breadcrumb, kCommandTimeoutMs);
}
break;
case CommissioningStage::kConfigRegulatory: {
1 change: 1 addition & 0 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
@@ -533,6 +533,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
* @param[in] remoteDeviceId The remote device Id.
* @param[in] params The commissioning parameters
*/
CHIP_ERROR Commission(NodeId remoteDeviceId);
CHIP_ERROR Commission(NodeId remoteDeviceId, CommissioningParameters & params);

CHIP_ERROR GetDeviceBeingCommissioned(NodeId deviceId, CommissioneeDeviceProxy ** device);
6 changes: 3 additions & 3 deletions src/controller/CommissioningDelegate.h
Original file line number Diff line number Diff line change
@@ -73,7 +73,7 @@ class CommissioningParameters
static constexpr size_t kMaxThreadDatasetLen = 254;
static constexpr size_t kMaxSsidLen = 32;
static constexpr size_t kMaxCredentialsLen = 64;
const Optional<uint16_t> GetFailsafeTimerSeconds() const { return mFailsafeTimerSeconds; }
uint16_t GetFailsafeTimerSeconds() const { return mFailsafeTimerSeconds; }
const Optional<ByteSpan> GetCSRNonce() const { return mCSRNonce; }
const Optional<ByteSpan> GetAttestationNonce() const { return mAttestationNonce; }
const Optional<WiFiCredentials> GetWiFiCredentials() const { return mWiFiCreds; }
@@ -95,7 +95,7 @@ class CommissioningParameters

CommissioningParameters & SetFailsafeTimerSeconds(uint16_t seconds)
{
mFailsafeTimerSeconds.SetValue(seconds);
mFailsafeTimerSeconds = seconds;
return *this;
}

@@ -182,7 +182,7 @@ class CommissioningParameters
void SetCompletionStatus(CHIP_ERROR err) { completionStatus = err; }

private:
Optional<uint16_t> mFailsafeTimerSeconds;
uint16_t mFailsafeTimerSeconds = 60;
Optional<ByteSpan> mCSRNonce; ///< CSR Nonce passed by the commissioner
Optional<ByteSpan> mAttestationNonce; ///< Attestation Nonce passed by the commissioner
Optional<WiFiCredentials> mWiFiCreds;