Remove GenerateChipNOCChain (#20013)

* Remove GenerateChipNOCChain

* Address PR comment

* Address PR comment

src/controller/CHIPDeviceController.cpp

@@ -1195,9 +1195,10 @@ CHIP_ERROR DeviceCommissioner::IssueNOCChain(const ByteSpan & NOCSRElements, Nod
         mOperationalCredentialsDelegate->SetFabricIdForNextNOCRequest(GetFabricId());
     }
 
-    // Note: attestationSignature, attestationChallenge, DAC, PAI are not used by existing OperationalCredentialsIssuer.
-    return mOperationalCredentialsDelegate->GenerateChipNOCChain(NOCSRElements, ByteSpan(), ByteSpan(), ByteSpan(), ByteSpan(),
-                                                                 ByteSpan(), callback);
+    // Note: we don't have attestationSignature, attestationChallenge, DAC, PAI so we are just providing an empty ByteSpan
+    // for those arguments.
+    return mOperationalCredentialsDelegate->GenerateNOCChain(NOCSRElements, ByteSpan(), ByteSpan(), ByteSpan(), ByteSpan(),
+                                                             ByteSpan(), callback);
 }
 
 CHIP_ERROR DeviceCommissioner::ProcessCSR(DeviceProxy * proxy, const ByteSpan & NOCSRElements,

src/controller/CHIPDeviceController.h

@@ -648,7 +648,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     // Commissioner will establish new device connections after PASE.
     OperationalDeviceProxy * GetDeviceSession(const PeerId & peerId) override;
 
-    // Issue an NOC chain using the associated OperationalCredentialsDelegate.
+    // Issue an NOC chain using the associated OperationalCredentialsDelegate. The NOC chain will
+    // be provided in X509 DER format.
     // NOTE: This is only valid assuming that `mOperationalCredentialsDelegate` is what is desired
     // to issue the NOC chain.
     CHIP_ERROR IssueNOCChain(const ByteSpan & NOCSRElements, NodeId nodeId,

src/controller/ExampleOperationalCredentialsIssuer.cpp

@@ -263,103 +263,6 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateChipNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce,
-                                                                     const ByteSpan & attestationSignature,
-                                                                     const ByteSpan & attestationChallenge, const ByteSpan & DAC,
-                                                                     const ByteSpan & PAI,
-                                                                     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)
-    {
-        assignedId       = mNextRequestedNodeId;
-        mNodeIdRequested = false;
-    }
-    else
-    {
-        assignedId = mNextAvailableNodeId++;
-    }
-
-    ChipLogProgress(Controller, "Verifying Certificate Signing Request");
-    TLVReader reader;
-    reader.Init(csrElements);
-
-    if (reader.GetType() == kTLVType_NotSpecified)
-    {
-        ReturnErrorOnFailure(reader.Next());
-    }
-
-    VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_WRONG_TLV_TYPE);
-    VerifyOrReturnError(reader.GetTag() == AnonymousTag(), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
-
-    TLVType containerType;
-    ReturnErrorOnFailure(reader.EnterContainer(containerType));
-    ReturnErrorOnFailure(reader.Next(kTLVType_ByteString, TLV::ContextTag(1)));
-
-    ByteSpan csr(reader.GetReadPoint(), reader.GetLength());
-    reader.ExitContainer(containerType);
-
-    P256PublicKey pubkey;
-    ReturnErrorOnFailure(VerifyCertificateSigningRequest(csr.data(), csr.size(), pubkey));
-
-    chip::Platform::ScopedMemoryBuffer<uint8_t> noc;
-    ReturnErrorCodeIf(!noc.Alloc(kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY);
-    MutableByteSpan nocSpan(noc.Get(), kMaxCHIPDERCertLength);
-
-    chip::Platform::ScopedMemoryBuffer<uint8_t> icac;
-    ReturnErrorCodeIf(!icac.Alloc(kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY);
-    MutableByteSpan icacSpan(icac.Get(), kMaxCHIPDERCertLength);
-
-    chip::Platform::ScopedMemoryBuffer<uint8_t> rcac;
-    ReturnErrorCodeIf(!rcac.Alloc(kMaxCHIPDERCertLength), CHIP_ERROR_NO_MEMORY);
-    MutableByteSpan rcacSpan(rcac.Get(), kMaxCHIPDERCertLength);
-
-    ReturnErrorOnFailure(
-        GenerateNOCChainAfterValidation(assignedId, mNextFabricId, chip::kUndefinedCATs, pubkey, rcacSpan, icacSpan, nocSpan));
-
-    // TODO(#13825): Should always generate some IPK. Using a temporary fixed value until APIs are plumbed in to set it end-to-end
-    // TODO: Force callers to set IPK if used before GenerateNOCChain will succeed.
-    ByteSpan defaultIpkSpan = chip::GroupTesting::DefaultIpkValue::GetDefaultIpk();
-
-    // The below static assert validates a key assumption in types used (needed for public API conformance)
-    static_assert(CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES == kAES_CCM128_Key_Length, "IPK span sizing must match");
-
-    // Prepare IPK to be sent back. A more fully-fledged operational credentials delegate
-    // would obtain a suitable key per fabric.
-    uint8_t ipkValue[CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES];
-    Crypto::AesCcm128KeySpan ipkSpan(ipkValue);
-
-    ReturnErrorCodeIf(defaultIpkSpan.size() != sizeof(ipkValue), CHIP_ERROR_INTERNAL);
-    memcpy(&ipkValue[0], defaultIpkSpan.data(), defaultIpkSpan.size());
-
-    chip::Platform::ScopedMemoryBuffer<uint8_t> chipNoc;
-    ReturnErrorCodeIf(!chipNoc.Alloc(Credentials::kMaxCHIPCertLength), CHIP_ERROR_NO_MEMORY);
-    MutableByteSpan chipNocSpan(chipNoc.Get(), Credentials::kMaxCHIPCertLength);
-
-    chip::Platform::ScopedMemoryBuffer<uint8_t> chipIcac;
-    ReturnErrorCodeIf(!chipIcac.Alloc(Credentials::kMaxCHIPCertLength), CHIP_ERROR_NO_MEMORY);
-    MutableByteSpan chipIcacSpan(chipIcac.Get(), Credentials::kMaxCHIPCertLength);
-
-    chip::Platform::ScopedMemoryBuffer<uint8_t> chipRcac;
-    ReturnErrorCodeIf(!chipRcac.Alloc(Credentials::kMaxCHIPCertLength), CHIP_ERROR_NO_MEMORY);
-    MutableByteSpan chipRcacSpan(chipRcac.Get(), Credentials::kMaxCHIPCertLength);
-
-    ReturnErrorOnFailure(ConvertX509CertToChipCert(nocSpan, chipNocSpan));
-    ReturnErrorOnFailure(ConvertX509CertToChipCert(icacSpan, chipIcacSpan));
-    ReturnErrorOnFailure(ConvertX509CertToChipCert(rcacSpan, chipRcacSpan));
-
-    // Callback onto commissioner.
-    ChipLogProgress(Controller, "Providing certificate chain to the commissioner");
-    onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, chipNocSpan, chipIcacSpan, chipRcacSpan, MakeOptional(ipkSpan),
-                        Optional<NodeId>());
-    return CHIP_NO_ERROR;
-}
-
 CHIP_ERROR ExampleOperationalCredentialsIssuer::GetRandomOperationalNodeId(NodeId * aNodeId)
 {
     for (int i = 0; i < 10; ++i)

src/controller/ExampleOperationalCredentialsIssuer.h

@@ -59,10 +59,6 @@ class DLL_EXPORT ExampleOperationalCredentialsIssuer : public OperationalCredent
                                 const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI,
                                 Callback::Callback<OnNOCChainGeneration> * onCompletion) override;
 
-    CHIP_ERROR GenerateChipNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce, const ByteSpan & attestationSignature,
-                                    const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI,
-                                    Callback::Callback<OnNOCChainGeneration> * onCompletion) override;
-
     void SetNodeIdForNextNOCRequest(NodeId nodeId) override
     {
         mNextRequestedNodeId = nodeId;

src/controller/OperationalCredentialsDelegate.h

@@ -66,34 +66,6 @@ class DLL_EXPORT OperationalCredentialsDelegate
                                         const ByteSpan & DAC, const ByteSpan & PAI,
                                         Callback::Callback<OnNOCChainGeneration> * onCompletion) = 0;
 
-    /**
-     * @brief
-     *   This function generates an operational certificate chain for a remote device that is being commissioned.
-     *   The API generates the certificate in CHIP cert form.
-     *
-     *   The delegate is expected to use the certificate authority whose certificate
-     *   is returned in `GetRootCACertificate()` API call.
-     *
-     *   The delegate will call `onCompletion` when the NOC certificate chain is ready.
-     *
-     * @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] 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 GenerateChipNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce,
-                                            const ByteSpan & attestationSignature, const ByteSpan & attestationChallenge,
-                                            const ByteSpan & DAC, const ByteSpan & PAI,
-                                            Callback::Callback<OnNOCChainGeneration> * onCompletion)
-    {
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
-
     /**
      *   This function sets the node ID for which the next NOC Chain would be requested. The node ID is
      *   provided as a hint, and the delegate implementation may chose to ignore it and pick node ID of

src/controller/python/ChipDeviceController-IssueNocChain.cpp

@@ -23,6 +23,9 @@
 #include <lib/core/CHIPError.h>
 #include <lib/support/Span.h>
 
+using namespace chip;
+using namespace chip::Credentials;
+
 typedef void PyObject;
 
 using namespace chip;
@@ -50,13 +53,51 @@ void pychip_DeviceController_IssueNOCChainCallback(void * context, CHIP_ERROR st
                                                    const ByteSpan & rcac, Optional<Crypto::AesCcm128KeySpan> ipk,
                                                    Optional<NodeId> adminSubject)
 {
-    if (pychip_DeviceController_IssueNOCChainCallbackPythonCallbackFunct != nullptr)
+    if (pychip_DeviceController_IssueNOCChainCallbackPythonCallbackFunct == nullptr)
+    {
+        return;
+    }
+
+    chip::Platform::ScopedMemoryBuffer<uint8_t> chipNoc;
+    chip::Platform::ScopedMemoryBuffer<uint8_t> chipIcac;
+    chip::Platform::ScopedMemoryBuffer<uint8_t> chipRcac;
+    MutableByteSpan chipNocSpan;
+    MutableByteSpan chipIcacSpan;
+    MutableByteSpan chipRcacSpan;
+
+    Crypto::AesCcm128KeySpan ipkData;
+    ipkData = ipk.ValueOr(Crypto::AesCcm128KeySpan());
+
+    CHIP_ERROR err = status;
+    if (err != CHIP_NO_ERROR)
+    {
+        ExitNow();
+    }
+    VerifyOrExit(chipNoc.Alloc(Credentials::kMaxCHIPCertLength), err = CHIP_ERROR_NO_MEMORY);
+    chipNocSpan = MutableByteSpan(chipNoc.Get(), Credentials::kMaxCHIPCertLength);
+
+    VerifyOrExit(chipIcac.Alloc(Credentials::kMaxCHIPCertLength), err = CHIP_ERROR_NO_MEMORY);
+    chipIcacSpan = MutableByteSpan(chipIcac.Get(), Credentials::kMaxCHIPCertLength);
+
+    VerifyOrExit(chipRcac.Alloc(Credentials::kMaxCHIPCertLength), err = CHIP_ERROR_NO_MEMORY);
+    chipRcacSpan = MutableByteSpan(chipRcac.Get(), Credentials::kMaxCHIPCertLength);
+
+    SuccessOrExit(err = ConvertX509CertToChipCert(noc, chipNocSpan));
+    SuccessOrExit(err = ConvertX509CertToChipCert(icac, chipIcacSpan));
+    SuccessOrExit(err = ConvertX509CertToChipCert(rcac, chipRcacSpan));
+
+exit:
+    if (err == CHIP_NO_ERROR)
     {
-        Crypto::AesCcm128KeySpan ipkData;
-        ipkData = ipk.ValueOr(Crypto::AesCcm128KeySpan());
         pychip_DeviceController_IssueNOCChainCallbackPythonCallbackFunct(
-            context, status.AsInteger(), noc.data(), noc.size(), icac.data(), icac.size(), rcac.data(), rcac.size(), ipkData.data(),
-            ipk.HasValue() ? ipkData.size() : 0, adminSubject.ValueOr(kUndefinedNodeId));
+            context, err.AsInteger(), chipNocSpan.data(), chipNocSpan.size(), chipIcacSpan.data(), chipIcacSpan.size(),
+            chipRcacSpan.data(), chipRcacSpan.size(), ipkData.data(), ipk.HasValue() ? ipkData.size() : 0,
+            adminSubject.ValueOr(kUndefinedNodeId));
+    }
+    else
+    {
+        pychip_DeviceController_IssueNOCChainCallbackPythonCallbackFunct(context, err.AsInteger(), nullptr, 0, nullptr, 0, nullptr,
+                                                                         0, nullptr, 0, 0);
     }
 }
 

src/controller/python/OpCredsBinding.cpp

@@ -86,14 +86,6 @@ class OperationalCredentialsAdapter : public OperationalCredentialsDelegate
                                                       onCompletion);
     }
 
-    CHIP_ERROR GenerateChipNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce, const ByteSpan & attestationSignature,
-                                    const ByteSpan & attestationChallenge, const ByteSpan & DAC, const ByteSpan & PAI,
-                                    Callback::Callback<OnNOCChainGeneration> * onCompletion) override
-    {
-        return mExampleOpCredsIssuer.GenerateChipNOCChain(csrElements, csrNonce, attestationSignature, attestationChallenge, DAC,
-                                                          PAI, onCompletion);
-    }
-
     void SetNodeIdForNextNOCRequest(NodeId nodeId) override { mExampleOpCredsIssuer.SetNodeIdForNextNOCRequest(nodeId); }
 
     void SetFabricIdForNextNOCRequest(FabricId fabricId) override { mExampleOpCredsIssuer.SetFabricIdForNextNOCRequest(fabricId); }

src/controller/python/chip/ChipDeviceCtrl.py

@@ -29,6 +29,7 @@
 from __future__ import print_function
 import asyncio
 from ctypes import *
+from dataclasses import dataclass
 
 from .ChipStack import *
 from .interaction_model import InteractionModelError, delegate as im
@@ -60,20 +61,37 @@
 # else seems to do it.
 _DeviceAvailableFunct = CFUNCTYPE(None, c_void_p, c_uint32)
 
-
 _IssueNOCChainCallbackPythonCallbackFunct = CFUNCTYPE(
     None, py_object, c_uint32, c_void_p, c_size_t, c_void_p, c_size_t, c_void_p, c_size_t, c_void_p, c_size_t, c_uint64)
 
 
+@dataclass
+class NOCChain:
+    nocBytes: bytes
+    icacBytes: bytes
+    rcacBytes: bytes
+    ipkBytes: bytes
+    adminSubject: int
+
+
 @_IssueNOCChainCallbackPythonCallbackFunct
 def _IssueNOCChainCallbackPythonCallback(devCtrl, status: int, noc: c_void_p, nocLen: int, icac: c_void_p, icacLen: int, rcac: c_void_p, rcacLen: int, ipk: c_void_p, ipkLen: int, adminSubject: int):
-    nocBytes = string_at(noc, nocLen)[:]
-    icacBytes = string_at(icac, icacLen)[:]
-    rcacBytes = string_at(rcac, rcacLen)[:]
-    ipkBytes = None
-    if ipkLen > 0:
-        ipkBytes = string_at(ipk, ipkLen)[:]
-    devCtrl.NOCChainCallback(status, nocBytes, icacBytes, rcacBytes, ipkBytes, adminSubject)
+    nocChain = NOCChain(None, None, None, None, 0)
+    if status == 0:
+        nocBytes = None
+        if nocLen > 0:
+            nocBytes = string_at(noc, nocLen)[:]
+        icacBytes = None
+        if icacLen > 0:
+            icacBytes = string_at(icac, icacLen)[:]
+        rcacBytes = None
+        if rcacLen > 0:
+            rcacBytes = string_at(rcac, rcacLen)[:]
+        ipkBytes = None
+        if ipkLen > 0:
+            ipkBytes = string_at(ipk, ipkLen)[:]
+        nocChain = NOCChain(nocBytes, icacBytes, rcacBytes, ipkBytes, adminSubject)
+    devCtrl.NOCChainCallback(nocChain)
 
 # This is a fix for WEAV-429. Jay Logue recommends revisiting this at a later
 # date to allow for truly multiple instances so this is temporary.
@@ -345,8 +363,8 @@ def CommissionIP(self, ipaddr, setupPinCode, nodeid):
             return False
         return self._ChipStack.commissioningEventRes == 0
 
-    def NOCChainCallback(self, status, noc, icac, rcac, ipk, adminSubject):
-        self._ChipStack.callbackRes = (noc, icac, rcac, ipk, adminSubject)
+    def NOCChainCallback(self, nocChain):
+        self._ChipStack.callbackRes = nocChain
         self._ChipStack.completeEvent.set()
         return
 
@@ -978,6 +996,8 @@ def SetBlockingCB(self, blockingCB):
         self._ChipStack.blockingCB = blockingCB
 
     def IssueNOCChain(self, csr: Clusters.OperationalCredentials.Commands.CSRResponse, nodeId: int):
+        """Issue an NOC chain using the associated OperationalCredentialsDelegate.
+        The NOC chain will be provided in TLV cert format."""
         self.CheckIsActive()
 
         return self._ChipStack.CallAsync(