Bug 1112461 - Have libpkix match classic & mozilla::pkix in preferring newer certs to older certs. r=wtc
authorRyan Sleevi <ryan.sleevi@gmail.com>
Thu, 08 Jan 2015 17:59:02 -0800 (2015-01-09)
changeset 11337 34e1379ff6c77f6c2dc52b542eafbe9c18034828
parent 11336 6978c29bd763e8e20c4e837ef4cdc7f7d6e802bc
child 11338 f7b7800ac9e9096d92c35864f7a16af42e7f23c7
push id542
push userryan.sleevi@gmail.com
push dateFri, 09 Jan 2015 23:01:42 +0000 (2015-01-09)
reviewerswtc
bugs1112461
Bug 1112461 - Have libpkix match classic & mozilla::pkix in preferring newer certs to older certs. r=wtc
lib/libpkix/pkix/checker/pkix_revocationchecker.c
lib/libpkix/pkix/top/pkix_build.c
--- a/lib/libpkix/pkix/checker/pkix_revocationchecker.c
+++ b/lib/libpkix/pkix/checker/pkix_revocationchecker.c
@@ -132,32 +132,38 @@ pkix_RevocationChecker_RegisterSelf(void
         entry.comparator = NULL;
         entry.duplicateFunction = pkix_RevocationChecker_Duplicate;
 
         systemClasses[PKIX_REVOCATIONCHECKER_TYPE] = entry;
 
         PKIX_RETURN(REVOCATIONCHECKER);
 }
 
-/* Sort methods by theirs priorities */
+/* Sort methods by their priorities (lower priority = higher preference) */
 static PKIX_Error *
 pkix_RevocationChecker_SortComparator(
         PKIX_PL_Object *obj1,
         PKIX_PL_Object *obj2,
         PKIX_Int32 *pResult,
         void *plContext)
 {
     pkix_RevocationMethod *method1 = NULL, *method2 = NULL;
     
     PKIX_ENTER(BUILD, "pkix_RevocationChecker_SortComparator");
     
     method1 = (pkix_RevocationMethod *)obj1;
     method2 = (pkix_RevocationMethod *)obj2;
     
-    *pResult = (method1->priority > method2->priority);
+    if (method1->priority < method2->priority) {
+      *pResult = -1;
+    } else if (method1->priority > method2->priority) {
+      *pResult = 1;
+    } else {
+      *pResult = 0;
+    }
     
     PKIX_RETURN(BUILD);
 }
 
 
 /* --Public-Functions--------------------------------------------- */
 
 
--- a/lib/libpkix/pkix/top/pkix_build.c
+++ b/lib/libpkix/pkix/top/pkix_build.c
@@ -655,19 +655,21 @@ pkix_ForwardBuilderState_IsIOPending(
 
 /* --Private-BuildChain-Functions------------------------------------------- */
 
 /*
  * FUNCTION: pkix_Build_SortCertComparator
  * DESCRIPTION:
  *
  *  This Function takes two Certificates cast in "obj1" and "obj2",
- *  compares their validity NotAfter dates and returns the result at
- *  "pResult". The comparison key(s) can be expanded by using other
- *  data in the Certificate in the future.
+ *  compares them to determine which is a more preferable certificate
+ *  for chain building. This Function is suitable for use as a
+ *  comparator callback for pkix_List_BubbleSort, setting "*pResult" to
+ *  > 0 if "obj1" is less desirable than "obj2" and < 0 if "obj1"
+ *  is more desirable than "obj2".
  *
  * PARAMETERS:
  *  "obj1"
  *      Address of the PKIX_PL_Object that is a cast of PKIX_PL_Cert.
  *      Must be non-NULL.
  *  "obj2"
  *      Address of the PKIX_PL_Object that is a cast of PKIX_PL_Cert.
  *      Must be non-NULL.
@@ -686,24 +688,24 @@ static PKIX_Error *
 pkix_Build_SortCertComparator(
         PKIX_PL_Object *obj1,
         PKIX_PL_Object *obj2,
         PKIX_Int32 *pResult,
         void *plContext)
 {
         PKIX_PL_Date *date1 = NULL;
         PKIX_PL_Date *date2 = NULL;
-        PKIX_Boolean result = PKIX_FALSE;
+        PKIX_Int32 result = 0;
 
         PKIX_ENTER(BUILD, "pkix_Build_SortCertComparator");
         PKIX_NULLCHECK_THREE(obj1, obj2, pResult);
 
         /*
          * For sorting candidate certificates, we use NotAfter date as the
-         * sorted key for now (can be expanded if desired in the future).
+         * comparison key for now (can be expanded if desired in the future).
          *
          * In PKIX_BuildChain, the List of CertStores was reordered so that
          * trusted CertStores are ahead of untrusted CertStores. That sort, or
          * this one, could be taken out if it is determined that it doesn't help
          * performance, or in some way hinders the solution of choosing desired
          * candidates.
          */
 
@@ -722,17 +724,22 @@ pkix_Build_SortCertComparator(
         
         PKIX_CHECK(PKIX_PL_Object_Compare
                 ((PKIX_PL_Object *)date1,
                 (PKIX_PL_Object *)date2,
                 &result,
                 plContext),
                 PKIX_OBJECTCOMPARATORFAILED);
 
-        *pResult = !result;
+        /*
+         * Invert the result, so that if date1 is greater than date2,
+         * obj1 is sorted before obj2. This is because pkix_List_BubbleSort
+         * sorts in ascending order.
+         */
+        *pResult = -result;
 
 cleanup:
 
         PKIX_DECREF(date1);
         PKIX_DECREF(date2);
 
         PKIX_RETURN(BUILD);
 }