From 2f91b0e143b47019aace7df2654e01473ad2acee Mon Sep 17 00:00:00 2001
From: Evgeni Margolis <emargolis@google.com>
Date: Thu, 5 May 2022 01:48:40 -0700
Subject: [PATCH] addressed review comments

---
 src/credentials/CertificationDeclaration.cpp | 42 ++++++++++++++++----
 src/credentials/CertificationDeclaration.h   |  9 +++++
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/credentials/CertificationDeclaration.cpp b/src/credentials/CertificationDeclaration.cpp
index b937fd5e4c60b1..4c4f59e693bd2a 100644
--- a/src/credentials/CertificationDeclaration.cpp
+++ b/src/credentials/CertificationDeclaration.cpp
@@ -183,20 +183,25 @@ CHIP_ERROR DecodeCertificationElements(const ByteSpan & encodedCertElements, Cer
     }
     VerifyOrReturnError(err == CHIP_END_OF_TLV || err == CHIP_ERROR_UNEXPECTED_TLV_ELEMENT || err == CHIP_NO_ERROR, err);
 
-    if (reader.GetTag() == ContextTag(kTag_AuthorizedPAAList) && reader.GetType() == kTLVType_Array)
+    if (err != CHIP_END_OF_TLV && reader.GetTag() == ContextTag(kTag_AuthorizedPAAList))
     {
+        VerifyOrReturnError(reader.GetType() == kTLVType_Array, CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
+
         ReturnErrorOnFailure(reader.EnterContainer(outerContainer2));
 
         certElements.AuthorizedPAAListCount = 0;
-        while ((err = reader.Next(AnonymousTag())) == CHIP_NO_ERROR)
+        while ((err = reader.Next(kTLVType_ByteString, AnonymousTag())) == CHIP_NO_ERROR)
         {
+            VerifyOrReturnError(reader.GetLength() == kKeyIdentifierLength, CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
+
             ReturnErrorOnFailure(
                 reader.GetBytes(certElements.AuthorizedPAAList[certElements.AuthorizedPAAListCount++], kKeyIdentifierLength));
         }
         VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
         ReturnErrorOnFailure(reader.ExitContainer(outerContainer2));
+
+        err = reader.Next();
     }
-    err = reader.Next();
     VerifyOrReturnError(err == CHIP_END_OF_TLV || err == CHIP_ERROR_UNEXPECTED_TLV_ELEMENT || err == CHIP_NO_ERROR, err);
 
     ReturnErrorOnFailure(reader.ExitContainer(outerContainer1));
@@ -211,6 +216,7 @@ CHIP_ERROR DecodeCertificationElements(const ByteSpan & encodedCertElements, Cer
     CHIP_ERROR err;
     TLVReader reader;
     TLVType outerContainer;
+    TLVType outerContainer2;
 
     VerifyOrReturnError(encodedCertElements.size() <= kMaxCMSSignedCDMessage, CHIP_ERROR_INVALID_ARGUMENT);
 
@@ -227,8 +233,15 @@ CHIP_ERROR DecodeCertificationElements(const ByteSpan & encodedCertElements, Cer
     ReturnErrorOnFailure(reader.Get(certDeclContent.vendorId));
 
     ReturnErrorOnFailure(reader.Next(kTLVType_Array, ContextTag(kTag_ProductIdArray)));
+    ReturnErrorOnFailure(reader.EnterContainer(outerContainer2));
 
-    // skip PID Array
+    while ((err = reader.Next(kTLVType_UnsignedInteger, AnonymousTag())) == CHIP_NO_ERROR)
+    {
+        // Verifies that the TLV structure of PID Array is correct
+        // but skip the values
+    }
+    VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
+    ReturnErrorOnFailure(reader.ExitContainer(outerContainer2));
 
     ReturnErrorOnFailure(reader.Next(ContextTag(kTag_DeviceTypeId)));
     ReturnErrorOnFailure(reader.Get(certDeclContent.deviceTypeId));
@@ -265,13 +278,26 @@ CHIP_ERROR DecodeCertificationElements(const ByteSpan & encodedCertElements, Cer
     }
     VerifyOrReturnError(err == CHIP_END_OF_TLV || err == CHIP_ERROR_UNEXPECTED_TLV_ELEMENT || err == CHIP_NO_ERROR, err);
 
-    if (reader.GetTag() == ContextTag(kTag_AuthorizedPAAList) && reader.GetType() == kTLVType_Array)
+    if (err != CHIP_END_OF_TLV && reader.GetTag() == ContextTag(kTag_AuthorizedPAAList))
     {
+        VerifyOrReturnError(reader.GetType() == kTLVType_Array, CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
+
+        ReturnErrorOnFailure(reader.EnterContainer(outerContainer2));
+
+        while ((err = reader.Next(kTLVType_ByteString, AnonymousTag())) == CHIP_NO_ERROR)
+        {
+            VerifyOrReturnError(reader.GetLength() == kKeyIdentifierLength, CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
+            // Verifies that the TLV structure of the Authorized PAA List is correct
+            // but skip the values
+        }
+        VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
+
+        ReturnErrorOnFailure(reader.ExitContainer(outerContainer2));
+
         certDeclContent.authorizedPAAListPresent = true;
 
-        // skip optional Authorized PAA List
+        err = reader.Next();
     }
-    err = reader.Next();
     VerifyOrReturnError(err == CHIP_END_OF_TLV || err == CHIP_ERROR_UNEXPECTED_TLV_ELEMENT || err == CHIP_NO_ERROR, err);
 
     ReturnErrorOnFailure(reader.ExitContainer(outerContainer));
@@ -305,7 +331,7 @@ CHIP_ERROR CertificationElementsDecoder::FindAndEnterArray(const ByteSpan & enco
     ReturnErrorOnFailure(mReader.Next(kTLVType_Structure, AnonymousTag()));
     ReturnErrorOnFailure(mReader.EnterContainer(outerContainerType1));
 
-    // position to ProductId Array
+    // position to arrayTag Array
     CHIP_ERROR error = CHIP_NO_ERROR;
     do
     {
diff --git a/src/credentials/CertificationDeclaration.h b/src/credentials/CertificationDeclaration.h
index 3dedd7cd0c138b..148dd78f331bb4 100644
--- a/src/credentials/CertificationDeclaration.h
+++ b/src/credentials/CertificationDeclaration.h
@@ -100,7 +100,16 @@ class CertificationElementsDecoder
     bool HasAuthorizedPAA(const ByteSpan & encodedCertElements, const ByteSpan & authorizedPAA);
 
 private:
+    /**
+     * @brief Positions mReader inside at the top of an Array with listTag tag.
+     *
+     * @param[in] encodedCertElements  TLV encoded structure of CD elements.
+     * @param[in] listTag              A tag of an array to be found in the encodedCertElements TLV structure.
+     *
+     * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
+     **/
     CHIP_ERROR FindAndEnterArray(const ByteSpan & encodedCertElements, TLV::Tag listTag);
+
     CHIP_ERROR GetNextProductId(uint16_t & productId);
     CHIP_ERROR GetNextAuthorizedPAA(ByteSpan & authorizedPAA);