Changeset 64883 in vbox for trunk/src/VBox/Runtime/common/crypto/x509-certpaths.cpp

Timestamp:

Dec 15, 2016 3:26:20 PM (8 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

112290

Message:

IPRT/ASN.1: Refactored array handling (SET OF, SEQUENCE OF) to use a pointer array instead of an object instance array. The old approach would move objects around in memory after they'd be initialized/decoded, making certain core optimziations involving pointers to object members impossible, as well as causing potentially causing trouble when modifying structures that takes down pointers after decoding. Fixed validation bug in rtCrX509Name_CheckSanityExtra where it didn't check that the RDNs had subitems but instead checked the parent twice (slight risk).

File:

• trunk/src/VBox/Runtime/common/crypto/x509-certpaths.cpp (modified) (24 diffs)

trunk/src/VBox/Runtime/common/crypto/x509-certpaths.cpp

@@ -723 +723 @@
     if (pThis->pUntrustedCertsSet)
     {
-        uint32_t const  cCerts  = pThis->pUntrustedCertsSet->cItems;
-        PCRTCRPKCS7CERT paCerts = pThis->pUntrustedCertsSet->paItems;
+        uint32_t const        cCerts   = pThis->pUntrustedCertsSet->cItems;
+        PRTCRPKCS7CERT const *papCerts = pThis->pUntrustedCertsSet->papItems;
         for (uint32_t i = 0; i < cCerts; i++)
-            if (   paCerts[i].enmChoice == RTCRPKCS7CERTCHOICE_X509
-                && RTCrX509Certificate_MatchSubjectOrAltSubjectByRfc5280(paCerts[i].u.pX509Cert, pIssuer))
-                rtCrX509CertPathsAddIssuer(pThis, pNode, paCerts[i].u.pX509Cert, NULL, RTCRX509CERTPATHNODE_SRC_UNTRUSTED_SET);
+        {
+            PCRTCRPKCS7CERT pCert = papCerts[i];
+            if (   pCert->enmChoice == RTCRPKCS7CERTCHOICE_X509
+                && RTCrX509Certificate_MatchSubjectOrAltSubjectByRfc5280(pCert->u.pX509Cert, pIssuer))
+                rtCrX509CertPathsAddIssuer(pThis, pNode, pCert->u.pX509Cert, NULL, RTCRX509CERTPATHNODE_SRC_UNTRUSTED_SET);
+        }
     }
 }
@@ -1057 +1060 @@
 {
     for (uint32_t i = 0; i < pName->cItems; i++)
-        for (uint32_t j = 0; j < pName->paItems[i].cItems; j++)
-        {
-            PRTCRX509ATTRIBUTETYPEANDVALUE pAttrib = &pName->paItems[i].paItems[j];
+    {
+        PCRTCRX509RELATIVEDISTINGUISHEDNAME const pRdn = pName->papItems[i];
+        for (uint32_t j = 0; j < pRdn->cItems; j++)
+        {
+            PRTCRX509ATTRIBUTETYPEANDVALUE pAttrib = pRdn->papItems[j];
 
             const char *pszType = pAttrib->Type.szObjId;
@@ -1118 +1123 @@
                 rtDumpPrintf(pfnPrintfV, pvUser, "<not-string: uTag=%#x>", pAttrib->Value.u.Core.uTag);
         }
+    }
 }
 
@@ -1350 +1356 @@
  * @param   pThis               The validator instance.
  * @param   cSubtrees           The number of sub-trees to add.
- * @param   paSubtrees          Array of sub-trees to add.
- */
-static bool rtCrX509CpvAddPermittedSubtrees(PRTCRX509CERTPATHSINT pThis, uint32_t cSubtrees, PCRTCRX509GENERALSUBTREE paSubtrees)
+ * @param   papSubtrees         Array of sub-trees to add.
+ */
+static bool rtCrX509CpvAddPermittedSubtrees(PRTCRX509CERTPATHSINT pThis, uint32_t cSubtrees,
+                                            PRTCRX509GENERALSUBTREE const *papSubtrees)
 {
     /*
@@ -1375 +1382 @@
     for (uint32_t iSrc = 0; iSrc < cSubtrees; iSrc++)
     {
-        if (!rtCrX509CpvCheckSubtreeValidity(pThis, &paSubtrees[iSrc]))
+        if (!rtCrX509CpvCheckSubtreeValidity(pThis, papSubtrees[iSrc]))
             return false;
-        pThis->v.papPermittedSubtrees[iDst] = &paSubtrees[iSrc];
+        pThis->v.papPermittedSubtrees[iDst] = papSubtrees[iSrc];
         iDst++;
     }
@@ -1383 +1390 @@
 
     return true;
+}
+
+
+/**
+ * Adds a one permitted sub-tree.
+ *
+ * We store reference to each individual sub-tree because we must support
+ * intersection calculation.
+ *
+ * @returns success indiciator.
+ * @param   pThis               The validator instance.
+ * @param   pSubtree            Array of sub-trees to add.
+ */
+static bool rtCrX509CpvAddPermittedSubtree(PRTCRX509CERTPATHSINT pThis, PCRTCRX509GENERALSUBTREE pSubtree)
+{
+    return rtCrX509CpvAddPermittedSubtrees(pThis, 1, (PRTCRX509GENERALSUBTREE const *)&pSubtree);
 }
 
@@ -1405 +1428 @@
     }
 
-    uint32_t                    cRight  = pSubtrees->cItems;
-    PCRTCRX509GENERALSUBTREE    paRight = pSubtrees->paItems;
+    uint32_t                       cRight   = pSubtrees->cItems;
+    PRTCRX509GENERALSUBTREE const *papRight = pSubtrees->papItems;
     if (cRight == 0)
     {
@@ -1417 +1440 @@
     PCRTCRX509GENERALSUBTREE   *papLeft = pThis->v.papPermittedSubtrees;
     if (!cLeft) /* first name constraint, no initial constraint */
-        return rtCrX509CpvAddPermittedSubtrees(pThis, cRight, paRight);
+        return rtCrX509CpvAddPermittedSubtrees(pThis, cRight, papRight);
 
     /*
@@ -1431 +1454 @@
     for (uint32_t iRight = 0; iRight < cRight; iRight++)
     {
-        if (!rtCrX509CpvCheckSubtreeValidity(pThis, &paRight[iRight]))
+        if (!rtCrX509CpvCheckSubtreeValidity(pThis, papRight[iRight]))
             return false;
 
-        RTCRX509GENERALNAMECHOICE const enmRightChoice = paRight[iRight].Base.enmChoice;
+        RTCRX509GENERALNAMECHOICE const enmRightChoice = papRight[iRight]->Base.enmChoice;
         afRightTags[enmRightChoice] = true;
 
@@ -1441 +1464 @@
             if (papLeft[iLeft]->Base.enmChoice == enmRightChoice)
             {
-                if (RTCrX509GeneralSubtree_Compare(papLeft[iLeft], &paRight[iRight]) == 0)
+                if (RTCrX509GeneralSubtree_Compare(papLeft[iLeft], papRight[iRight]) == 0)
                 {
                     if (!fHaveRight)
                     {
                         fHaveRight = true;
-                        rtCrX509CpvAddPermittedSubtrees(pThis, 1, papLeft[iLeft]);
+                        rtCrX509CpvAddPermittedSubtree(pThis, papLeft[iLeft]);
                     }
                 }
-                else if (RTCrX509GeneralSubtree_ConstraintMatch(papLeft[iLeft], &paRight[iRight]))
+                else if (RTCrX509GeneralSubtree_ConstraintMatch(papLeft[iLeft], papRight[iRight]))
                 {
                     if (!fHaveRight)
                     {
                         fHaveRight = true;
-                        rtCrX509CpvAddPermittedSubtrees(pThis, 1, &paRight[iRight]);
+                        rtCrX509CpvAddPermittedSubtree(pThis, papRight[iRight]);
                     }
                 }
-                else if (RTCrX509GeneralSubtree_ConstraintMatch(&paRight[iRight], papLeft[iLeft]))
-                    rtCrX509CpvAddPermittedSubtrees(pThis, 1, papLeft[iLeft]);
+                else if (RTCrX509GeneralSubtree_ConstraintMatch(papRight[iRight], papLeft[iLeft]))
+                    rtCrX509CpvAddPermittedSubtree(pThis, papLeft[iLeft]);
             }
     }
@@ -1467 +1490 @@
     for (uint32_t iLeft = 0; iLeft < cLeft; iLeft++)
         if (!afRightTags[papLeft[iLeft]->Base.enmChoice])
-            rtCrX509CpvAddPermittedSubtrees(pThis, 1, papLeft[iLeft]);
+            rtCrX509CpvAddPermittedSubtree(pThis, papLeft[iLeft]);
 
     /*
@@ -1541 +1564 @@
         uint32_t j = pSubTrees->cItems;
         while (j-- > 0)
-            if (   RTCRX509GENERALNAME_IS_DIRECTORY_NAME(&pSubTrees->paItems[j].Base)
-                && RTCrX509Name_ConstraintMatch(&pSubTrees->paItems[j].Base.u.pT4->DirectoryName, pName))
+        {
+            PCRTCRX509GENERALSUBTREE const pSubTree = pSubTrees->papItems[j];
+            if (   RTCRX509GENERALNAME_IS_DIRECTORY_NAME(&pSubTree->Base)
+                && RTCrX509Name_ConstraintMatch(&pSubTree->Base.u.pT4->DirectoryName, pName))
                 return true;
+        }
     }
     return false;
@@ -1566 +1592 @@
         uint32_t j = pSubTrees->cItems;
         while (j-- > 0)
-            if (RTCrX509GeneralName_ConstraintMatch(&pSubTrees->paItems[j].Base, pGeneralName))
+            if (RTCrX509GeneralName_ConstraintMatch(&pSubTrees->papItems[j]->Base, pGeneralName))
                 return true;
     }
@@ -1951 +1977 @@
     if (pThis->pInitialPermittedSubtrees)
         rtCrX509CpvAddPermittedSubtrees(pThis, pThis->pInitialPermittedSubtrees->cItems,
-                                        pThis->pInitialPermittedSubtrees->paItems);
+                                        pThis->pInitialPermittedSubtrees->papItems);
     if (pThis->pInitialExcludedSubtrees)
         rtCrX509CpvAddExcludedSubtrees(pThis, pThis->pInitialExcludedSubtrees);
@@ -2053 +2079 @@
         uint32_t i = pAltSubjectName->cItems;
         while (i-- > 0)
-            if (   !rtCrX509CpvIsGeneralNamePermitted(pThis, &pAltSubjectName->paItems[i])
-                || rtCrX509CpvIsGeneralNameExcluded(pThis, &pAltSubjectName->paItems[i]))
+            if (   !rtCrX509CpvIsGeneralNamePermitted(pThis, pAltSubjectName->papItems[i])
+                || rtCrX509CpvIsGeneralNameExcluded(pThis, pAltSubjectName->papItems[i]))
                 return rtCrX509CpvFailed(pThis, VERR_CR_X509_CPV_ALT_NAME_NOT_PERMITTED,
                                          "Alternative name #%u is is not permitted by current name constraints", i);
@@ -2081 +2107 @@
         while (i-- > 0)
         {
-            PCRTCRX509POLICYQUALIFIERINFOS const    pQualifiers = &pPolicies->paItems[i].PolicyQualifiers;
-            PCRTASN1OBJID const                     pIdP        = &pPolicies->paItems[i].PolicyIdentifier;
+            PCRTCRX509POLICYQUALIFIERINFOS const    pQualifiers = &pPolicies->papItems[i]->PolicyQualifiers;
+            PCRTASN1OBJID const                     pIdP        = &pPolicies->papItems[i]->PolicyIdentifier;
             if (RTAsn1ObjId_CompareWithString(pIdP, RTCRX509_ID_CE_CP_ANY_POLICY_OID) == 0)
             {
@@ -2132 +2158 @@
                 || (pNode->pParent && fSelfIssued) ) )
         {
-            PCRTCRX509POLICYQUALIFIERINFOS pApQ = &pPolicies->paItems[iAnyPolicy].PolicyQualifiers;
+            PCRTCRX509POLICYQUALIFIERINFOS pApQ = &pPolicies->papItems[iAnyPolicy]->PolicyQualifiers;
             RTListForEach(pListAbove, pCur, RTCRX509CERTPATHSPOLICYNODE, DepthEntry)
             {
@@ -2183 +2209 @@
     while (i-- > 0)
     {
-        if (RTAsn1ObjId_CompareWithString(&pPolicyMappings->paItems[i].IssuerDomainPolicy, RTCRX509_ID_CE_CP_ANY_POLICY_OID) == 0)
+        PCRTCRX509POLICYMAPPING const pOne = pPolicyMappings->papItems[i];
+        if (RTAsn1ObjId_CompareWithString(&pOne->IssuerDomainPolicy, RTCRX509_ID_CE_CP_ANY_POLICY_OID) == 0)
             return rtCrX509CpvFailed(pThis, VERR_CR_X509_CPV_INVALID_POLICY_MAPPING,
                                      "Invalid policy mapping %#u: IssuerDomainPolicy is anyPolicy.", i);
 
-        if (RTAsn1ObjId_CompareWithString(&pPolicyMappings->paItems[i].SubjectDomainPolicy, RTCRX509_ID_CE_CP_ANY_POLICY_OID) == 0)
+        if (RTAsn1ObjId_CompareWithString(&pOne->SubjectDomainPolicy, RTCRX509_ID_CE_CP_ANY_POLICY_OID) == 0)
             return rtCrX509CpvFailed(pThis, VERR_CR_X509_CPV_INVALID_POLICY_MAPPING,
                                      "Invalid policy mapping %#u: SubjectDomainPolicy is anyPolicy.", i);
@@ -2201 +2228 @@
         while (i-- > 0)
         {
+            PCRTCRX509POLICYMAPPING const pOne = pPolicyMappings->papItems[i];
+
             uint32_t cFound = 0;
             RTListForEach(&pThis->v.paValidPolicyDepthLists[iDepth], pCur, RTCRX509CERTPATHSPOLICYNODE, DepthEntry)
             {
-                if (RTAsn1ObjId_Compare(pCur->pValidPolicy, &pPolicyMappings->paItems[i].IssuerDomainPolicy))
+                if (RTAsn1ObjId_Compare(pCur->pValidPolicy, &pOne->IssuerDomainPolicy))
                 {
                     if (!pCur->fAlreadyMapped)
                     {
                         pCur->fAlreadyMapped = true;
-                        pCur->pExpectedPolicyFirst = &pPolicyMappings->paItems[i].SubjectDomainPolicy;
+                        pCur->pExpectedPolicyFirst = &pOne->SubjectDomainPolicy;
                     }
                     else
@@ -2221 +2250 @@
                                                      pCur->cMoreExpectedPolicySet, iDepth);
                         pCur->papMoreExpectedPolicySet = (PCRTASN1OBJID *)pvNew;
-                        pCur->papMoreExpectedPolicySet[iExpected] = &pPolicyMappings->paItems[i].SubjectDomainPolicy;
+                        pCur->papMoreExpectedPolicySet[iExpected] = &pOne->SubjectDomainPolicy;
                         pCur->cMoreExpectedPolicySet = iExpected  + 1;
                     }
@@ -2238 +2267 @@
                     {
                         if (!rtCrX509CpvPolicyTreeInsertNew(pThis, pCur->pParent, iDepth,
-                                                            &pPolicyMappings->paItems[i].IssuerDomainPolicy,
+                                                            &pOne->IssuerDomainPolicy,
                                                             pCur->pPolicyQualifiers,
-                                                            &pPolicyMappings->paItems[i].SubjectDomainPolicy))
+                                                            &pOne->SubjectDomainPolicy))
                             return false;
                         break;
@@ -2258 +2287 @@
         while (i-- > 0)
         {
+            PCRTCRX509POLICYMAPPING const pOne = pPolicyMappings->papItems[i];
             RTListForEachSafe(&pThis->v.paValidPolicyDepthLists[iDepth], pCur, pNext, RTCRX509CERTPATHSPOLICYNODE, DepthEntry)
             {
-                if (RTAsn1ObjId_Compare(pCur->pValidPolicy, &pPolicyMappings->paItems[i].IssuerDomainPolicy))
+                if (RTAsn1ObjId_Compare(pCur->pValidPolicy, &pOne->IssuerDomainPolicy))
                 {
                     rtCrX509CpvPolicyTreeDestroyNode(pThis, pCur);
@@ -2410 +2440 @@
 static bool rtCrX509CpvCheckCriticalExtensions(PRTCRX509CERTPATHSINT pThis, PRTCRX509CERTPATHNODE pNode)
 {
-    uint32_t                cLeft = pNode->pCert->TbsCertificate.T3.Extensions.cItems;
-    PCRTCRX509EXTENSION     pCur  = pNode->pCert->TbsCertificate.T3.Extensions.paItems;
+    uint32_t                  cLeft = pNode->pCert->TbsCertificate.T3.Extensions.cItems;
+    PRTCRX509EXTENSION const *ppCur = pNode->pCert->TbsCertificate.T3.Extensions.papItems;
     while (cLeft-- > 0)
     {
+        PCRTCRX509EXTENSION const pCur = *ppCur;
         if (pCur->Critical.fValue)
         {
@@ -2431 +2462 @@
         }
 
-        pCur++;
+        ppCur++;
     }