Validate CSR in separate commissioning step

src/controller/AutoCommissioner.cpp

@@ -135,6 +135,8 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
     case CommissioningStage::kAttestationVerification:
         return CommissioningStage::kSendOpCertSigningRequest;
     case CommissioningStage::kSendOpCertSigningRequest:
+        return CommissioningStage::kValidateCSR;
+    case CommissioningStage::kValidateCSR:
         return CommissioningStage::kGenerateNOCChain;
     case CommissioningStage::kGenerateNOCChain:
         return CommissioningStage::kSendTrustedRootCert;

src/controller/CHIPDeviceController.cpp

@@ -997,6 +997,26 @@ CHIP_ERROR DeviceCommissioner::ValidateAttestationInfo(const Credentials::Device
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR DeviceCommissioner::ValidateCSR(DeviceProxy * proxy, const ByteSpan & NOCSRElements,
+                                           const ByteSpan & AttestationSignature, ByteSpan dac, ByteSpan csrNonce)
+{
+    MATTER_TRACE_EVENT_SCOPE("ValidateCSR", "DeviceCommissioner");
+    VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
+
+    DeviceAttestationVerifier * dacVerifier = GetDeviceAttestationVerifier();
+
+    P256PublicKey dacPubkey;
+    ReturnErrorOnFailure(ExtractPubkeyFromX509Cert(dac, dacPubkey));
+
+    // Retrieve attestation challenge
+    ByteSpan attestationChallenge =
+        proxy->GetSecureSession().Value()->AsSecureSession()->GetCryptoContext().GetAttestationChallenge();
+
+    // The operational CA should also verify this on its end during NOC generation, if end-to-end attestation is desired.
+    return dacVerifier->VerifyNodeOperationalCSRInformation(NOCSRElements, attestationChallenge, AttestationSignature, dacPubkey,
+                                                            csrNonce);
+}
+
 CHIP_ERROR DeviceCommissioner::SendOperationalCertificateSigningRequestCommand(DeviceProxy * device, const ByteSpan & csrNonce)
 {
     MATTER_TRACE_EVENT_SCOPE("SendOperationalCertificateSigningRequestCommand", "DeviceCommissioner");
@@ -1071,9 +1091,10 @@ CHIP_ERROR DeviceCommissioner::ProcessCSR(DeviceProxy * proxy, const ByteSpan &
     MATTER_TRACE_EVENT_SCOPE("ProcessOpCSR", "DeviceCommissioner");
     VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
 
-    ChipLogProgress(Controller, "Getting certificate chain for the device from the issuer");
+    // This will be used once the GenerateNOCChain
+    (void) csrNonce;
 
-    DeviceAttestationVerifier * dacVerifier = GetDeviceAttestationVerifier();
+    ChipLogProgress(Controller, "Getting certificate chain for the device from the issuer");
 
     P256PublicKey dacPubkey;
     ReturnErrorOnFailure(ExtractPubkeyFromX509Cert(dac, dacPubkey));
@@ -1082,19 +1103,15 @@ CHIP_ERROR DeviceCommissioner::ProcessCSR(DeviceProxy * proxy, const ByteSpan &
     ByteSpan attestationChallenge =
         proxy->GetSecureSession().Value()->AsSecureSession()->GetCryptoContext().GetAttestationChallenge();
 
-    // The operational CA should also verify this on its end during NOC generation, if end-to-end attestation is desired.
-    ReturnErrorOnFailure(dacVerifier->VerifyNodeOperationalCSRInformation(NOCSRElements, attestationChallenge, AttestationSignature,
-                                                                          dacPubkey, csrNonce));
-
     mOperationalCredentialsDelegate->SetNodeIdForNextNOCRequest(proxy->GetDeviceId());
 
     if (mFabricInfo != nullptr)
     {
         mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(mFabricInfo->GetFabricId());
     }
 
-    return mOperationalCredentialsDelegate->GenerateNOCChain(NOCSRElements, AttestationSignature, dac, ByteSpan(), ByteSpan(),
-                                                             &mDeviceNOCChainCallback);
+    return mOperationalCredentialsDelegate->GenerateNOCChain(NOCSRElements, csrNonce, AttestationSignature, attestationChallenge,
+                                                             dac, ByteSpan(), ByteSpan(), &mDeviceNOCChainCallback);
 }
 
 CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(DeviceProxy * device, const ByteSpan & nocCertBuf,
@@ -1809,6 +1826,23 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
         }
         SendOperationalCertificateSigningRequestCommand(proxy, params.GetCSRNonce().Value());
         break;
+    case CommissioningStage::kValidateCSR: {
+        if (!params.GetNOCChainGenerationParameters().HasValue() || !params.GetDAC().HasValue() || !params.GetCSRNonce().HasValue())
+        {
+            ChipLogError(Controller, "Unable to validate CSR");
+            return CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
+        }
+        // This is non-blocking, so send the callback immediately.
+        CHIP_ERROR err = ValidateCSR(proxy, params.GetNOCChainGenerationParameters().Value().nocsrElements,
+                                     params.GetNOCChainGenerationParameters().Value().signature, params.GetDAC().Value(),
+                                     params.GetCSRNonce().Value());
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(Controller, "Unable to validate CSR");
+        }
+        CommissioningStageComplete(err);
+    }
+    break;
     case CommissioningStage::kGenerateNOCChain: {
         if (!params.GetNOCChainGenerationParameters().HasValue() || !params.GetDAC().HasValue() || !params.GetCSRNonce().HasValue())
         {

src/controller/CHIPDeviceController.h

@@ -729,6 +729,20 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     CHIP_ERROR ProcessCSR(DeviceProxy * proxy, const ByteSpan & NOCSRElements, const ByteSpan & AttestationSignature, ByteSpan dac,
                           ByteSpan csrNonce);
 
+    /**
+     * @brief
+     *   This function validates the CSR information from the device.
+     *   (Reference: Specifications section 11.18.5.6. NOCSR Elements)
+     *
+     * @param[in] proxy           device proxy
+     * @param[in] NOCSRElements   CSR elements as per specifications section 11.22.5.6. NOCSR Elements.
+     * @param[in] AttestationSignature       Cryptographic signature generated for all the above fields.
+     * @param[in] dac               device attestation certificate
+     * @param[in] csrNonce          certificate signing request nonce
+     */
+    CHIP_ERROR ValidateCSR(DeviceProxy * proxy, const ByteSpan & NOCSRElements, const ByteSpan & AttestationSignature, ByteSpan dac,
+                           ByteSpan csrNonce);
+
     /**
      * @brief
      *   This function processes the DAC or PAI certificate sent by the device.

src/controller/CommissioningDelegate.cpp

@@ -65,6 +65,10 @@ const char * StageToString(CommissioningStage stage)
         return "SendOpCertSigningRequest";
         break;
 
+    case kValidateCSR:
+        return "ValidateCSR";
+        break;
+
     case kGenerateNOCChain:
         return "GenerateNOCChain";
         break;

src/controller/CommissioningDelegate.h

@@ -39,6 +39,7 @@ enum CommissioningStage : uint8_t
     kSendAttestationRequest,
     kAttestationVerification,
     kSendOpCertSigningRequest,
+    kValidateCSR,
     kGenerateNOCChain,
     kSendTrustedRootCert,
     kSendNOC,

src/controller/ExampleOperationalCredentialsIssuer.cpp

@@ -176,12 +176,17 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChainAfterValidation(
     return NewNodeOperationalX509Cert(noc_request, pubkey, mIntermediateIssuer, noc);
 }
 
-CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan & csrElements,
-                                                                 const ByteSpan & attestationSignature, const ByteSpan & DAC,
+CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce,
+                                                                 const ByteSpan & attestationSignature,
+                                                                 const ByteSpan & attestationChallenge, const ByteSpan & DAC,
                                                                  const ByteSpan & PAI, const ByteSpan & PAA,
                                                                  Callback::Callback<OnNOCChainGeneration> * onCompletion)
 {
     VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE);
+    // At this point, Credential issuer may wish to validate the CSR information
+    (void) attestationChallenge;
+    (void) csrNonce;
+
     NodeId assignedId;
     if (mNodeIdRequested)
     {

src/controller/ExampleOperationalCredentialsIssuer.h

@@ -55,9 +55,9 @@ class DLL_EXPORT ExampleOperationalCredentialsIssuer : public OperationalCredent
     ExampleOperationalCredentialsIssuer(uint32_t index = 0) { mIndex = index; }
     ~ExampleOperationalCredentialsIssuer() override {}
 
-    CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC,
-                                const ByteSpan & PAI, const ByteSpan & PAA,
-                                Callback::Callback<OnNOCChainGeneration> * onCompletion) override;
+    CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce, const ByteSpan & attestationSignature,
+                                const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI,
+                                const ByteSpan & PAA, Callback::Callback<OnNOCChainGeneration> * onCompletion) override;
 
     void SetNodeIdForNextNOCRequest(NodeId nodeId) override
     {

src/controller/OperationalCredentialsDelegate.h

@@ -51,17 +51,20 @@ class DLL_EXPORT OperationalCredentialsDelegate
      *
      *   The delegate will call `onCompletion` when the NOC certificate chain is ready.
      *
-     * @param[in] csrElements          CSR elements as per specifications section 11.22.5.6. NOCSR Elements.
+     * @param[in] csrElements          CSR elements as per specifications section 11.18.5.6. NOCSR Elements.
+     * @param[in] csrNonce             CSR nonce as described in 6.4.6.1
      * @param[in] attestationSignature Attestation signature as per specifications section 11.22.7.6. CSRResponse Command.
+     * @param[in] attestationChallenge Attestation challenge as per 11.18.5.7
      * @param[in] DAC                  Device attestation certificate received from the device being commissioned
      * @param[in] PAI                  Product Attestation Intermediate certificate
      * @param[in] PAA                  Product Attestation Authority certificate
      * @param[in] onCompletion         Callback handler to provide generated NOC chain to the caller of GenerateNOCChain()
      *
      * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code.
      */
-    virtual CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC,
-                                        const ByteSpan & PAI, const ByteSpan & PAA,
+    virtual CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce,
+                                        const ByteSpan & attestationSignature, const ByteSpan & attestationChallenge,
+                                        const ByteSpan & DAC, const ByteSpan & PAI, const ByteSpan & PAA,
                                         Callback::Callback<OnNOCChainGeneration> * onCompletion) = 0;
 
     /**

src/controller/java/AndroidOperationalCredentialsIssuer.cpp

@@ -120,8 +120,9 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::GenerateNOCChainAfterValidation(
     return NewNodeOperationalX509Cert(noc_request, pubkey, mIssuer, noc);
 }
 
-CHIP_ERROR AndroidOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan & csrElements,
-                                                                 const ByteSpan & attestationSignature, const ByteSpan & DAC,
+CHIP_ERROR AndroidOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce,
+                                                                 const ByteSpan & attestationSignature,
+                                                                 const ByteSpan & attestationChallenge, const ByteSpan & DAC,
                                                                  const ByteSpan & PAI, const ByteSpan & PAA,
                                                                  Callback::Callback<OnNOCChainGeneration> * onCompletion)
 {

src/controller/java/AndroidOperationalCredentialsIssuer.h

@@ -47,9 +47,9 @@ class DLL_EXPORT AndroidOperationalCredentialsIssuer : public OperationalCredent
 public:
     virtual ~AndroidOperationalCredentialsIssuer() {}
 
-    CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC,
-                                const ByteSpan & PAI, const ByteSpan & PAA,
-                                Callback::Callback<OnNOCChainGeneration> * onCompletion) override;
+    CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce, const ByteSpan & attestationSignature,
+                                const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI,
+                                const ByteSpan & PAA, Callback::Callback<OnNOCChainGeneration> * onCompletion) override;
 
     void SetNodeIdForNextNOCRequest(NodeId nodeId) override
     {

src/controller/python/OpCredsBinding.cpp

@@ -78,11 +78,12 @@ class OperationalCredentialsAdapter : public OperationalCredentialsDelegate
     }
 
 private:
-    CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & attestationSignature, const ByteSpan & DAC,
-                                const ByteSpan & PAI, const ByteSpan & PAA,
-                                Callback::Callback<OnNOCChainGeneration> * onCompletion) override
+    CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce, const ByteSpan & attestationSignature,
+                                const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI,
+                                const ByteSpan & PAA, Callback::Callback<OnNOCChainGeneration> * onCompletion) override
     {
-        return mExampleOpCredsIssuer.GenerateNOCChain(csrElements, attestationSignature, DAC, PAI, PAA, onCompletion);
+        return mExampleOpCredsIssuer.GenerateNOCChain(csrElements, csrNonce, attestationSignature, attestationChallenge, DAC, PAI,
+                                                      PAA, onCompletion);
     }
 
     void SetNodeIdForNextNOCRequest(NodeId nodeId) override { mExampleOpCredsIssuer.SetNodeIdForNextNOCRequest(nodeId); }

src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.h

@@ -46,9 +46,9 @@ class CHIPOperationalCredentialsDelegate : public chip::Controller::OperationalC
      */
     CHIP_ERROR init(CHIPPersistentStorageDelegateBridge * storage, ChipP256KeypairPtr nocSigner, NSData * _Nullable ipk);
 
-    CHIP_ERROR GenerateNOCChain(const chip::ByteSpan & csrElements, const chip::ByteSpan & attestationSignature,
-        const chip::ByteSpan & DAC, const chip::ByteSpan & PAI, const chip::ByteSpan & PAA,
-        chip::Callback::Callback<chip::Controller::OnNOCChainGeneration> * onCompletion) override;
+    CHIP_ERROR GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce, const ByteSpan & attestationSignature,
+        const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI, const ByteSpan & PAA,
+        Callback::Callback<OnNOCChainGeneration> * onCompletion) override;
 
     void SetNodeIdForNextNOCRequest(chip::NodeId nodeId) override
     {

src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm

@@ -351,9 +351,9 @@ static BOOL isRunningTests(void)
     return NewNodeOperationalX509Cert(noc_request, pubkey, *mIssuerKey, noc);
 }
 
-CHIP_ERROR CHIPOperationalCredentialsDelegate::GenerateNOCChain(const chip::ByteSpan & csrElements,
-    const chip::ByteSpan & attestationSignature, const chip::ByteSpan & DAC, const chip::ByteSpan & PAI, const chip::ByteSpan & PAA,
-    chip::Callback::Callback<chip::Controller::OnNOCChainGeneration> * onCompletion)
+CHIP_ERROR CHIPOperationalCredentialsDelegate::GenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce,
+    const ByteSpan & attestationSignature, const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI,
+    const ByteSpan & PAA, Callback::Callback<OnNOCChainGeneration> * onCompletion)
 {
     chip::NodeId assignedId;
     if (mNodeIdRequested) {