Skip to content

Commit

Permalink
Add interfaces to pass ipk and adminSubject from op creds delegate (p…
Browse files Browse the repository at this point in the history
…roject-chip#13830)

* Add interfaces to pass ipk and adminSubject from op creds delegate

The operational credentials delegate provides interfaces to pass
certificates back to the commissioner, but cannot fully specify the
contents of the AddNOC command.  Missing are the IPK and AdminSubject.
Without these, those can only be managed locally.

This commit adds optional interfaces to pass these from the delegate.
Where AdminSubject is not passed, the commissioner uses its own node ID
as before.  And where IPK is not passed, the commissioner writes a
0-length IPK, which is also as before and is what we are using until
GroupKeyManagement and other dependencies are in place.

Fixes project-chip#13503

* fix android build

* fix sanitization test

* fix iOS build

* per tcarmelveilleux, do not enclose 0-length IPK ind AddNOC

* remove Has methods from CommissioningParameters for brevity

* restyle

* Fix project-chip#12915 merge
  • Loading branch information
msandstedt authored Jan 26, 2022
1 parent b22f0ac commit d625d35
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 50 deletions.
27 changes: 16 additions & 11 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void AutoCommissioner::SetOperationalCredentialsDelegate(OperationalCredentialsD
CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParameters & params)
{
mParams = params;
if (params.HasThreadOperationalDataset())
if (params.GetThreadOperationalDataset().HasValue())
{
ByteSpan dataset = params.GetThreadOperationalDataset().Value();
if (dataset.size() > CommissioningParameters::kMaxThreadDatasetLen)
Expand All @@ -51,7 +51,7 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
ChipLogProgress(Controller, "Setting thread operational dataset from parameters");
mParams.SetThreadOperationalDataset(ByteSpan(mThreadOperationalDataset, dataset.size()));
}
if (params.HasWiFiCredentials())
if (params.GetWiFiCredentials().HasValue())
{
WiFiCredentials creds = params.GetWiFiCredentials().Value();
if (creds.ssid.size() > CommissioningParameters::kMaxSsidLen ||
Expand All @@ -67,7 +67,7 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
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.HasAttestationNonce())
if (params.GetAttestationNonce().HasValue())
{
ChipLogProgress(Controller, "Setting attestation nonce from parameters");
VerifyOrReturnError(params.GetAttestationNonce().Value().size() == sizeof(mAttestationNonce), CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -80,7 +80,7 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
}
mParams.SetAttestationNonce(ByteSpan(mAttestationNonce, sizeof(mAttestationNonce)));

if (params.HasCSRNonce())
if (params.GetCSRNonce().HasValue())
{
ChipLogProgress(Controller, "Setting CSR nonce from parameters");
VerifyOrReturnError(params.GetCSRNonce().Value().size() == sizeof(mCSRNonce), CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -128,11 +128,11 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStage(CommissioningStag
// TODO(cecille): device attestation casues operational cert provisioinging to happen, This should be a separate stage.
// For thread and wifi, this should go to network setup then enable. For on-network we can skip right to finding the
// operational network because the provisioning of certificates will trigger the device to start operational advertising.
if (mParams.HasWiFiCredentials())
if (mParams.GetWiFiCredentials().HasValue())
{
return CommissioningStage::kWiFiNetworkSetup;
}
else if (mParams.HasThreadOperationalDataset())
else if (mParams.GetThreadOperationalDataset().HasValue())
{
return CommissioningStage::kThreadNetworkSetup;
}
Expand All @@ -145,7 +145,7 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStage(CommissioningStag
#endif
}
case CommissioningStage::kWiFiNetworkSetup:
if (mParams.HasThreadOperationalDataset())
if (mParams.GetThreadOperationalDataset().HasValue())
{
return CommissioningStage::kThreadNetworkSetup;
}
Expand All @@ -154,7 +154,7 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStage(CommissioningStag
return CommissioningStage::kWiFiNetworkEnable;
}
case CommissioningStage::kThreadNetworkSetup:
if (mParams.HasWiFiCredentials())
if (mParams.GetWiFiCredentials().HasValue())
{
return CommissioningStage::kWiFiNetworkEnable;
}
Expand All @@ -164,7 +164,7 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStage(CommissioningStag
}

case CommissioningStage::kWiFiNetworkEnable:
if (mParams.HasThreadOperationalDataset())
if (mParams.GetThreadOperationalDataset().HasValue())
{
return CommissioningStage::kThreadNetworkEnable;
}
Expand Down Expand Up @@ -211,7 +211,8 @@ Optional<System::Clock::Timeout> AutoCommissioner::GetCommandTimeout(Commissioni
}
}

CHIP_ERROR AutoCommissioner::NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac)
CHIP_ERROR AutoCommissioner::NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, AesCcm128KeySpan ipk,
NodeId adminSubject)
{
// Reuse ICA Cert buffer for temporary store Root Cert.
MutableByteSpan rootCert = MutableByteSpan(mICACertBuffer);
Expand All @@ -237,6 +238,9 @@ CHIP_ERROR AutoCommissioner::NOCChainGenerated(ByteSpan noc, ByteSpan icac, Byte
mParams.SetIcac(ByteSpan());
}

mParams.SetIpk(ipk);
mParams.SetAdminSubject(adminSubject);

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -278,7 +282,8 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
case CommissioningStage::kGenerateNOCChain:
// For NOC chain generation, we re-use the buffers. NOCChainGenerated triggers the next stage before
// storing the returned certs, so just return here without triggering the next stage.
return NOCChainGenerated(report.Get<NocChain>().noc, report.Get<NocChain>().icac, report.Get<NocChain>().rcac);
return NOCChainGenerated(report.Get<NocChain>().noc, report.Get<NocChain>().icac, report.Get<NocChain>().rcac,
report.Get<NocChain>().ipk, report.Get<NocChain>().adminSubject);
case CommissioningStage::kFindOperational:
mOperationalDeviceProxy = report.Get<OperationalNodeFoundData>().operationalProxy;
break;
Expand Down
2 changes: 1 addition & 1 deletion src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class AutoCommissioner : public CommissioningDelegate
ByteSpan GetDAC() const { return ByteSpan(mDAC, mDACLen); }
ByteSpan GetPAI() const { return ByteSpan(mPAI, mPAILen); }

CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac);
CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, AesCcm128KeySpan ipk, NodeId adminSubject);
Optional<System::Clock::Timeout> GetCommandTimeout(CommissioningStage stage);

DeviceCommissioner * mCommissioner;
Expand Down
47 changes: 28 additions & 19 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId, CommissioningPa
ChipLogError(Controller, "Commissioning already in progress - not restarting");
return CHIP_ERROR_INCORRECT_STATE;
}
if (!params.HasWiFiCredentials() && !params.HasThreadOperationalDataset() && !mIsIPRendezvous)
if (!params.GetWiFiCredentials().HasValue() && !params.GetThreadOperationalDataset().HasValue() && !mIsIPRendezvous)
{
ChipLogError(Controller, "Network commissioning parameters are required for BLE auto commissioning.");
return CHIP_ERROR_INVALID_ARGUMENT;
Expand Down Expand Up @@ -1170,19 +1170,26 @@ void DeviceCommissioner::OnOperationalCertificateSigningRequest(void * context,
}

void DeviceCommissioner::OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac)
const ByteSpan & rcac, Optional<AesCcm128KeySpan> ipk,
Optional<NodeId> adminSubject)
{
CHIP_ERROR err = CHIP_NO_ERROR;

DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
CommissioningDelegate::CommissioningReport report(CommissioningStage::kGenerateNOCChain);

// TODO(#13825): If not passed by the signer, the commissioner should
// provide its current IPK to the commissionee in the AddNOC command.
const uint8_t placeHolderIpk[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };

ChipLogProgress(Controller, "Received callback from the CA for NOC Chain generation. Status %s", ErrorStr(status));
VerifyOrExit(commissioner->mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE);

// TODO - Verify that the generated root cert matches with commissioner's root cert

report.Set<NocChain>(NocChain(noc, icac, rcac));
report.Set<NocChain>(NocChain(noc, icac, rcac, ipk.HasValue() ? ipk.Value() : AesCcm128KeySpan(placeHolderIpk),
adminSubject.HasValue() ? adminSubject.Value() : commissioner->GetNodeId()));
err = commissioner->mCommissioningDelegate->CommissioningStepFinished(CHIP_NO_ERROR, report);
exit:
if (err != CHIP_NO_ERROR)
Expand Down Expand Up @@ -1222,7 +1229,8 @@ CHIP_ERROR DeviceCommissioner::ProcessOpCSR(DeviceProxy * proxy, const ByteSpan
}

CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(DeviceProxy * device, const ByteSpan & nocCertBuf,
const ByteSpan & icaCertBuf)
const ByteSpan & icaCertBuf, const AesCcm128KeySpan ipk,
const NodeId adminSubject)
{
VerifyOrReturnError(device != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
chip::Controller::OperationalCredentialsCluster cluster;
Expand All @@ -1231,8 +1239,7 @@ CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(DeviceProxy * device,
Callback::Cancelable * successCallback = mNOCResponseCallback.Cancel();
Callback::Cancelable * failureCallback = mOnCertFailureCallback.Cancel();

ReturnErrorOnFailure(cluster.AddNOC(successCallback, failureCallback, nocCertBuf, icaCertBuf, ByteSpan(nullptr, 0),
mLocalId.GetNodeId(), mVendorId));
ReturnErrorOnFailure(cluster.AddNOC(successCallback, failureCallback, nocCertBuf, icaCertBuf, ipk, adminSubject, mVendorId));

ChipLogProgress(Controller, "Sent operational certificate to the device");

Expand Down Expand Up @@ -1640,7 +1647,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
break;
case CommissioningStage::kSendAttestationRequest:
ChipLogProgress(Controller, "Sending Attestation Request to the device.");
if (!params.HasAttestationNonce())
if (!params.GetAttestationNonce().HasValue())
{
ChipLogError(Controller, "No attestation nonce found");
CommissioningDelegate::CommissioningReport report(step);
Expand All @@ -1651,8 +1658,8 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
break;
case CommissioningStage::kAttestationVerification:
ChipLogProgress(Controller, "Verifying attestation");
if (!params.HasAttestationElements() || !params.HasAttestationSignature() || !params.HasAttestationNonce() ||
!params.HasDAC() || !params.HasPAI())
if (!params.GetAttestationElements().HasValue() || !params.GetAttestationSignature().HasValue() ||
!params.GetAttestationNonce().HasValue() || !params.GetDAC().HasValue() || !params.GetPAI().HasValue())
{
ChipLogError(Controller, "Missing attestation information");
CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -1668,7 +1675,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
}
break;
case CommissioningStage::kSendOpCertSigningRequest:
if (!params.HasCSRNonce())
if (!params.GetCSRNonce().HasValue())
{
ChipLogError(Controller, "No CSR nonce found");
CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -1677,7 +1684,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
SendOperationalCertificateSigningRequestCommand(proxy, params.GetCSRNonce().Value());
break;
case CommissioningStage::kGenerateNOCChain: {
if (!params.HasNOCChainGenerationaParameters() || !params.HasDAC() || !params.HasCSRNonce())
if (!params.GetNOCChainGenerationParameters().HasValue() || !params.GetDAC().HasValue() || !params.GetCSRNonce().HasValue())
{
ChipLogError(Controller, "Unable to generate NOC chain parameters");
return CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -1697,7 +1704,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
}
break;
case CommissioningStage::kSendTrustedRootCert: {
if (!params.HasRootCert() || !params.HasNoc())
if (!params.GetRootCert().HasValue() || !params.GetNoc().HasValue())
{
ChipLogError(Controller, "No trusted root cert or NOC specified");
CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -1726,20 +1733,22 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
}
break;
case CommissioningStage::kSendNOC:
if (!params.HasNoc() || !params.HasIcac())
if (!params.GetNoc().HasValue() || !params.GetIcac().HasValue() || !params.GetIpk().HasValue() ||
!params.GetAdminSubject().HasValue())
{
ChipLogError(Controller, "No node operational certs specified");
ChipLogError(Controller, "AddNOC contents not specified");
CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
return;
}
ChipLogProgress(Controller, "Sending operational certificate chain to the device");
SendOperationalCertificate(proxy, params.GetNoc().Value(), params.GetIcac().Value());
SendOperationalCertificate(proxy, params.GetNoc().Value(), params.GetIcac().Value(), params.GetIpk().Value(),
params.GetAdminSubject().Value());
break;
case CommissioningStage::kConfigACL:
// TODO: Implement
break;
case CommissioningStage::kWiFiNetworkSetup: {
if (!params.HasWiFiCredentials())
if (!params.GetWiFiCredentials().HasValue())
{
ChipLogError(Controller, "No wifi credentials specified");
CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -1753,7 +1762,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
}
break;
case CommissioningStage::kThreadNetworkSetup: {
if (!params.HasThreadOperationalDataset())
if (!params.GetThreadOperationalDataset().HasValue())
{
ChipLogError(Controller, "No thread credentials specified");
CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -1767,7 +1776,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
}
break;
case CommissioningStage::kWiFiNetworkEnable: {
if (!params.HasWiFiCredentials())
if (!params.GetWiFiCredentials().HasValue())
{
ChipLogError(Controller, "No wifi credentials specified");
CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -1782,7 +1791,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
case CommissioningStage::kThreadNetworkEnable: {
ByteSpan extendedPanId;
chip::Thread::OperationalDataset operationalDataset;
if (!params.HasThreadOperationalDataset() ||
if (!params.GetThreadOperationalDataset().HasValue() ||
operationalDataset.Init(params.GetThreadOperationalDataset().Value()) != CHIP_NO_ERROR ||
operationalDataset.GetExtendedPanIdAsByteSpan(extendedPanId) != CHIP_NO_ERROR)
{
Expand Down
10 changes: 8 additions & 2 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,11 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate,
*/
uint64_t GetFabricId() const { return mFabricId; }

/**
* @brief Get the Node ID of this instance.
*/
NodeId GetNodeId() const { return mLocalId.GetNodeId(); }

void ReleaseOperationalDevice(NodeId remoteDeviceId);

protected:
Expand Down Expand Up @@ -716,7 +721,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
/* This function sends the operational credentials to the device.
The function does not hold a reference to the device object.
*/
CHIP_ERROR SendOperationalCertificate(DeviceProxy * device, const ByteSpan & nocCertBuf, const ByteSpan & icaCertBuf);
CHIP_ERROR SendOperationalCertificate(DeviceProxy * device, const ByteSpan & nocCertBuf, const ByteSpan & icaCertBuf,
AesCcm128KeySpan ipk, NodeId adminSubject);
/* This function sends the trusted root certificate to the device.
The function does not hold a reference to the device object.
*/
Expand Down Expand Up @@ -764,7 +770,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
static void OnDeviceAttestationInformationVerification(void * context, Credentials::AttestationVerificationResult result);

static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac);
const ByteSpan & rcac, Optional<AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);

/**
* @brief
Expand Down
Loading

0 comments on commit d625d35

Please sign in to comment.