Apply review comments

project-chip · Jun 13, 2022 · c7a2f29 · c7a2f29
1 parent c31a15d
commit c7a2f29
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 8 deletions.
diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp
@@ -2008,20 +2008,20 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
     }
     break;
     case CommissioningStage::kSendTrustedRootCert: {
-        if (!params.FetchRootCert().HasValue() || !params.GetNoc().HasValue())
+        if (!params.GetRootCert().HasValue() || !params.GetNoc().HasValue())
         {
             ChipLogError(Controller, "No trusted root cert or NOC specified");
             CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
             return;
         }
-        CHIP_ERROR err = SendTrustedRootCertificate(proxy, params.FetchRootCert().Value(), timeout);
+        CHIP_ERROR err = SendTrustedRootCertificate(proxy, params.GetRootCert().Value(), timeout);
         if (err != CHIP_NO_ERROR)
         {
             ChipLogError(Controller, "Error sending trusted root certificate: %s", err.AsString());
             CommissioningStageComplete(err);
             return;
         }
-        err = proxy->SetPeerId(params.FetchRootCert().Value(), params.GetNoc().Value());
+        err = proxy->SetPeerId(params.GetRootCert().Value(), params.GetNoc().Value());
         if (err != CHIP_NO_ERROR)
         {
             ChipLogError(Controller, "Error setting peer id: %s", err.AsString());

diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h
@@ -143,7 +143,7 @@ class CommissioningParameters
     // The root certificate for the operational certificate chain. In the auto commissioner, this is set by by the kGenerateNOCChain
     // stage through the OperationalCredentialsDelegate.
     // This value must be set before calling PerformCommissioningStep for the kSendTrustedRootCert step.
-    const Optional<ByteSpan> FetchRootCert() const { return mRootCert; }
+    const Optional<ByteSpan> GetRootCert() const { return mRootCert; }
 
     // The node operational certificate for the node being commissioned. In the AutoCommissioner, this is set by by the
     // kGenerateNOCChain stage through the OperationalCredentialsDelegate.

diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
@@ -435,7 +435,7 @@ CHIP_ERROR FabricInfo::VerifyCredentials(const ByteSpan & noc, const ByteSpan &
     }
 
     ReturnErrorOnFailure(GeneratePeerId(rcac, fabricId, nodeId, &nocPeerId));
-    nocPubkey = P256PublicKey(certificates.GetLastCert()[0].mPublicKey);
+    nocPubkey = certificates.GetLastCert()[0].mPublicKey;
 
     return CHIP_NO_ERROR;
 }
@@ -446,7 +446,7 @@ CHIP_ERROR FabricInfo::FetchRootPubkey(Crypto::P256PublicKey & outPublicKey) con
     CHIP_ERROR err = Credentials::ExtractPublicKeyFromChipCert(mRootCert, publicKeySpan);
     if (err == CHIP_NO_ERROR)
     {
-        outPublicKey = P256PublicKey(publicKeySpan);
+        outPublicKey = publicKeySpan;
     }
 
     return err;

diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
@@ -249,7 +249,6 @@ class DLL_EXPORT FabricInfo
     mutable Crypto::P256Keypair * mOperationalKey = nullptr;
 #endif
     bool mHasExternallyOwnedOperationalKey = false;
-    bool mHasExternallyOwnedCertificates   = false;
 
     MutableByteSpan mRootCert;
     MutableByteSpan mICACert;

diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h
@@ -306,6 +306,14 @@ class P256PublicKey : public ECPKey<P256ECDSASignature>
         memcpy(&bytes[0], value.data(), N);
     }
 
+    template <size_t N>
+    P256PublicKey & operator=(const FixedByteSpan<N> & value)
+    {
+        static_assert(N == kP256_PublicKey_Length, "Can only initialize from proper sized byte span");
+        memcpy(&bytes[0], value.data(), N);
+        return *this;
+    }
+
     SupportedECPKeyTypes Type() const override { return SupportedECPKeyTypes::ECP256R1; }
     size_t Length() const override { return kP256_PublicKey_Length; }
     operator uint8_t *() override { return bytes; }

diff --git a/src/darwin/Framework/CHIP/CHIPP256KeypairBridge.mm b/src/darwin/Framework/CHIP/CHIPP256KeypairBridge.mm
@@ -161,7 +161,7 @@
         return CHIP_ERROR_INTERNAL;
     }
     chip::FixedByteSpan<kP256_PublicKey_Length> pubkeyBytes((const uint8_t *) pubkeyData.bytes);
-    *matterPubKey = P256PublicKey(pubkeyBytes);
+    *matterPubKey = pubkeyBytes;
 
     return CHIP_NO_ERROR;
 }