From 2133394e8e2f6a103e40472af4af51b5e99d6a47 Mon Sep 17 00:00:00 2001
From: Jerry Johns <johnsj@google.com>
Date: Wed, 19 Jan 2022 14:04:56 -0800
Subject: [PATCH] Catch errors retrieving certs (#13724)

Problem:

We're not catching errors when calling `GetRootPubkey` and
`GetTrustedRootId`. This results in failures happening a fair bit later
in non-obvious ways in some of the Cirque runs.

Fix:

Catch the error so that we can at least log where the error is happening
in CI.
---
 .../operational-credentials-server.cpp              |  7 +++++--
 src/credentials/FabricTable.cpp                     | 13 +++++++++++--
 src/credentials/FabricTable.h                       | 12 ++++--------
 src/protocols/secure_channel/CASESession.cpp        |  4 ++--
 4 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
index b95982e6d2a729..aa4f268b21604c 100644
--- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
+++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp
@@ -103,8 +103,11 @@ CHIP_ERROR OperationalCredentialsAttrAccess::ReadFabricsList(EndpointId endpoint
             fabricDescriptor.vendorId    = fabricInfo.GetVendorId();
             fabricDescriptor.fabricId    = fabricInfo.GetFabricId();
 
-            fabricDescriptor.label         = fabricInfo.GetFabricLabel();
-            fabricDescriptor.rootPublicKey = fabricInfo.GetRootPubkey();
+            fabricDescriptor.label = fabricInfo.GetFabricLabel();
+
+            Credentials::P256PublicKeySpan pubKey;
+            ReturnErrorOnFailure(fabricInfo.GetRootPubkey(pubKey));
+            fabricDescriptor.rootPublicKey = pubKey;
 
             ReturnErrorOnFailure(encoder.Encode(fabricDescriptor));
         }
diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
index ae19040827457a..06fb786e4c2467 100644
--- a/src/credentials/FabricTable.cpp
+++ b/src/credentials/FabricTable.cpp
@@ -192,7 +192,14 @@ CHIP_ERROR FabricInfo::GetCompressedId(FabricId fabricId, NodeId nodeId, PeerId
     ReturnErrorCodeIf(compressedPeerId == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     uint8_t compressedFabricIdBuf[sizeof(uint64_t)];
     MutableByteSpan compressedFabricIdSpan(compressedFabricIdBuf);
-    P256PublicKey rootPubkey(GetRootPubkey());
+    P256PublicKey rootPubkey;
+
+    {
+        P256PublicKeySpan rootPubkeySpan;
+        ReturnErrorOnFailure(GetRootPubkey(rootPubkeySpan));
+        rootPubkey = rootPubkeySpan;
+    }
+
     ChipLogDetail(Inet, "Generating compressed fabric ID using uncompressed fabric ID 0x" ChipLogFormatX64 " and root pubkey",
                   ChipLogValueX64(fabricId));
     ChipLogByteSpan(Inet, ByteSpan(rootPubkey.ConstBytes(), rootPubkey.Length()));
@@ -332,7 +339,9 @@ CHIP_ERROR FabricInfo::GenerateDestinationID(const ByteSpan & ipk, const ByteSpa
         kSigmaParamRandomNumberSize + kP256_PublicKey_Length + sizeof(FabricId) + sizeof(NodeId);
     HMAC_sha hmac;
     uint8_t destinationMessage[kDestinationMessageLen];
-    P256PublicKeySpan rootPubkeySpan = GetRootPubkey();
+    P256PublicKeySpan rootPubkeySpan;
+
+    ReturnErrorOnFailure(GetRootPubkey(rootPubkeySpan));
 
     Encoding::LittleEndian::BufferWriter bbuf(destinationMessage, sizeof(destinationMessage));
 
diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
index 0f3a3b9730bd7c..7457b2f4b28e55 100644
--- a/src/credentials/FabricTable.h
+++ b/src/credentials/FabricTable.h
@@ -213,18 +213,14 @@ class DLL_EXPORT FabricInfo
         return CHIP_NO_ERROR;
     }
 
-    Credentials::CertificateKeyId GetTrustedRootId() const
+    CHIP_ERROR GetTrustedRootId(Credentials::CertificateKeyId & skid) const
     {
-        Credentials::CertificateKeyId skid;
-        Credentials::ExtractSKIDFromChipCert(mRootCert, skid);
-        return skid;
+        return Credentials::ExtractSKIDFromChipCert(mRootCert, skid);
     }
 
-    Credentials::P256PublicKeySpan GetRootPubkey() const
+    CHIP_ERROR GetRootPubkey(Credentials::P256PublicKeySpan & publicKey) const
     {
-        Credentials::P256PublicKeySpan publicKey;
-        Credentials::ExtractPublicKeyFromChipCert(mRootCert, publicKey);
-        return publicKey;
+        return Credentials::ExtractPublicKeyFromChipCert(mRootCert, publicKey);
     }
 
     CHIP_ERROR VerifyCredentials(const ByteSpan & noc, const ByteSpan & icac, Credentials::ValidationContext & context,
diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
index 15df65fa573855..e88980435c9eef 100644
--- a/src/protocols/secure_channel/CASESession.cpp
+++ b/src/protocols/secure_channel/CASESession.cpp
@@ -534,7 +534,7 @@ CHIP_ERROR CASESession::SendSigma2()
     ByteSpan nocCert;
     ReturnErrorOnFailure(mFabricInfo->GetNOCCert(nocCert));
 
-    mTrustedRootId = mFabricInfo->GetTrustedRootId();
+    ReturnErrorOnFailure(mFabricInfo->GetTrustedRootId(mTrustedRootId));
     VerifyOrReturnError(!mTrustedRootId.empty(), CHIP_ERROR_INTERNAL);
 
     // Fill in the random value
@@ -915,7 +915,7 @@ CHIP_ERROR CASESession::SendSigma3()
     SuccessOrExit(err = mFabricInfo->GetICACert(icaCert));
     SuccessOrExit(err = mFabricInfo->GetNOCCert(nocCert));
 
-    mTrustedRootId = mFabricInfo->GetTrustedRootId();
+    SuccessOrExit(err = mFabricInfo->GetTrustedRootId(mTrustedRootId));
     VerifyOrExit(!mTrustedRootId.empty(), err = CHIP_ERROR_INTERNAL);
 
     // Prepare Sigma3 TBS Data Blob