diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 78581dca42c657..f5a4ddf281618a 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -561,6 +561,12 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, */ CHIP_ERROR NetworkCredentialsReady(); + /** + * @brief + * This function returns the current CommissioningStage for this commissioner. + */ + CommissioningStage GetCommissioningStage() { return mCommissioningStage; } + #if CONFIG_NETWORK_LAYER_BLE #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE /** diff --git a/src/controller/java/AndroidOperationalCredentialsIssuer.cpp b/src/controller/java/AndroidOperationalCredentialsIssuer.cpp index 249921ae17c923..59872b01486b52 100644 --- a/src/controller/java/AndroidOperationalCredentialsIssuer.cpp +++ b/src/controller/java/AndroidOperationalCredentialsIssuer.cpp @@ -205,7 +205,6 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::CallbackGenerateNOCChain(const B P256PublicKey pubkey; ReturnErrorOnFailure(VerifyCertificateSigningRequest(csr.data(), csr.size(), pubkey)); - // TODO: verify signed by DAC creds? ChipLogProgress(chipTool, "VerifyCertificateSigningRequest"); jobject csrInfo; diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index 2020eb27e45d8a..2f2ccb58cd9d5a 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -524,7 +524,9 @@ JNI_METHOD(void, setUseJavaCallbackForNOCRequest) if (useCallback) { - // if we are assigning a callback, then make the device commissioner delegate verification to the cloud + // if we are assigning a callback, then make the device commissioner delegate verification to the + // PartialDACVerifier so that DAC chain and CD validation can be performed by custom code + // triggered by ChipDeviceController.NOCChainIssuer.onNOCChainGenerationNeeded(). wrapper->Controller()->SetDeviceAttestationVerifier(wrapper->GetPartialDACVerifier()); } else @@ -554,6 +556,17 @@ JNI_METHOD(void, updateCommissioningNetworkCredentials) ChipLogError(Controller, "UpdateCommissioningParameters failed. Err = %" CHIP_ERROR_FORMAT, err.Format()); JniReferences::GetInstance().ThrowError(env, sChipDeviceControllerExceptionCls, err); } + + // Only invoke NetworkCredentialsReady when called in response to NetworkScan result + if (wrapper->Controller()->GetCommissioningStage() == CommissioningStage::kNeedsNetworkCreds) + { + err = wrapper->Controller()->NetworkCredentialsReady(); + if (err != CHIP_NO_ERROR) + { + ChipLogError(Controller, "NetworkCredentialsReady failed. Err = %" CHIP_ERROR_FORMAT, err.Format()); + JniReferences::GetInstance().ThrowError(env, sChipDeviceControllerExceptionCls, err); + } + } } JNI_METHOD(jbyteArray, convertX509CertToMatterCert) diff --git a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java index 541f83e40aba99..97627b56c4ce1f 100644 --- a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java +++ b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java @@ -63,9 +63,17 @@ public void setScanNetworksListener(ScanNetworksListener listener) { } /** - * Sets this DeviceController to use the given issuer for issuing operational certs. By default, - * the DeviceController uses an internal, OperationalCredentialsDelegate (see - * AndroidOperationalCredentialsIssuer) + * Sets this DeviceController to use the given issuer for issuing operational certs and verifying + * the DAC. By default, the DeviceController uses an internal, OperationalCredentialsDelegate (see + * AndroidOperationalCredentialsIssuer). + * + *

When a NOCChainIssuer is set for this controller, then onNOCChainGenerationNeeded will be + * called when the NOC CSR needs to be signed and DAC verified. This allows for custom credentials + * issuer and DAC verifier implementations, for example, when a proprietary cloud API will perform + * DAC verification and the CSR signing. + * + *

When a NOCChainIssuer is set for this controller, the PartialDACVerifier will be used rather + * than the DefaultDACVerifier. * * @param issuer */ @@ -692,8 +700,12 @@ protected void finalize() throws Throwable { public interface NOCChainIssuer { /** * When a NOCChainIssuer is set for this controller, then onNOCChainGenerationNeeded will be - * called when the NOC CSR needs to be signed. This allows for custom credentials issuer - * implementations, for example, when a proprietary cloud API will perform the CSR signing. + * called when the DAC chain must be verified and NOC chain needs to be issued from a CSR. This + * allows for custom credentials issuer and DAC verifier implementations, for example, when a + * proprietary cloud API will perform DAC verification and the NOC chain issuance from CSR. + * + *

When a NOCChainIssuer is set for this controller, the PartialDACVerifier will be used + * rather than the DefaultDACVerifier. * *

The commissioning workflow will stop upon the onNOCChainGenerationNeeded callback and * resume once onNOCChainGeneration is called. @@ -716,6 +728,11 @@ public interface NOCChainIssuer { *

Set the AttemptNetworkScanWiFi or AttemptNetworkScanThread to configure the enable/disable * WiFi or Thread network scan during commissioning in the the default CommissioningDelegate used * by the ChipDeviceCommissioner. + * + *

When the callbacks onScanNetworksFailure or onScanNetworksSuccess are invoked, the + * commissioning flow has reached the kNeedsNetworkCreds and will wait to advance until this + * device controller's updateCommissioningNetworkCredentials method is called with the desired + * network credentials set. */ public interface ScanNetworksListener { /** Notifies when scan networks call fails. */ diff --git a/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.cpp b/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.cpp index e37155886d61ce..4dd6ab7d35a8f2 100644 --- a/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.cpp +++ b/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.cpp @@ -36,6 +36,10 @@ namespace Credentials { // As per specifications section 11.22.5.1. Constant RESP_MAX constexpr size_t kMaxResponseLength = 900; +/** + * The implementation should track DefaultDACVerifier::VerifyAttestationInformation but with the checks + * disabled that are outlined at the top of DacOnlyPartialAttestationVerifier.h. + */ void PartialDACVerifier::VerifyAttestationInformation(const DeviceAttestationVerifier::AttestationInfo & info, Callback::Callback * onCompletion) { @@ -144,7 +148,7 @@ void PartialDACVerifier::VerifyAttestationInformation(const DeviceAttestationVer } exit: - onCompletion->mCall(onCompletion->mContext, attestationError); // TODO: is this check getting done? + onCompletion->mCall(onCompletion->mContext, attestationError); } } // namespace Credentials diff --git a/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.h b/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.h index acd28f66d71054..a987c7c9779ce7 100644 --- a/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.h +++ b/src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.h @@ -21,11 +21,37 @@ namespace chip { namespace Credentials { +/** + * @brief + * This class is based upon the DefaultDACVerifier but has all checks removed which require + * local availability of trust anchors that are not available from the commissionee, such as the + * PAA root certificates and the CSA keys used to sign the Certification Declaration (CD). + * + * This class should only be used in conjunction with an OperationalCredentialsDelegate + * which performs the removed checks. For example, an OperationalCredentialsDelegate implementation + * might send the DAC chain and signed CD to custom code which obtains these keys from the DCL. + * + * Specifically, the following list of checks have been removed: + * (1) Make sure the PAA is valid and approved by CSA. + * (2) vid-scoped PAA check: if the PAA is vid scoped, then its vid must match the DAC vid. + * (3) cert chain check: verify PAI is signed by PAA, and DAC is signed by PAI. + * (4) PAA subject key id extraction: the PAA subject key must match the PAA key referenced in the PAI. + * (5) CD signature check: make sure a valid CSA CD key is used to sign the CD. + * + * Any other checks performed by the DefaultDACVerifier should be performed here too. Changes + * made to DefaultDACVerifier::VerifyAttestationInformation should be made to + * PartialDACVerifier::VerifyAttestationInformation. + */ class PartialDACVerifier : public DefaultDACVerifier { public: PartialDACVerifier() {} + /** + * @brief + * The implementation should track DefaultDACVerifier::VerifyAttestationInformation but with the checks + * disabled that are outlined at the top of DacOnlyPartialAttestationVerifier.h. + */ void VerifyAttestationInformation(const DeviceAttestationVerifier::AttestationInfo & info, Callback::Callback * onCompletion) override;