Remove all -1 returns from X509_check_purpose

Of external callers of this function, almost all are not actually doing
anything with this operation and are just trying to trigger
x509v3_cache_extensions. Triggering that is no longer necessarily now
that the structure is opaque and accessors do it for you.

There were three callers that wanted the actual operation here. One of
them correctly handled the tri-state return, but did not distinguish 0
from -1. The other two did not and would misinterpret -1 as success! So
this change is actually more compatible with OpenSSL callers than
OpenSSL's actual behavior.

Change-Id: Ifedba52dd9d4e031fc919276fd08ec22cfd33bf2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65153
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 0c8bc4653e34892dc291b48fb38e180ce92b5921)

crypto/x509/v3_purp.c

@@ -118,26 +118,21 @@ static const X509_PURPOSE xstandard[] = {
      (char *)"timestampsign", NULL},
 };
 
-// As much as I'd like to make X509_check_purpose use a "const" X509* I
-// really can't because it does recalculate hashes and do other non-const
-// things. If |id| is -1 it just calls |x509v3_cache_extensions| for its
-// side-effect.
-// Returns 1 on success, 0 if x does not allow purpose, -1 on (internal) error.
 int X509_check_purpose(X509 *x, int id, int ca) {
-  int idx;
-  const X509_PURPOSE *pt;
+  // This differs from OpenSSL, which uses -1 to indicate a fatal error and 0 to
+  // indicate an invalid certificate. BoringSSL uses 0 for both.
   if (!x509v3_cache_extensions(x)) {
-    return -1;
+    return 0;
   }
 
   if (id == -1) {
     return 1;
   }
-  idx = X509_PURPOSE_get_by_id(id);
+  int idx = X509_PURPOSE_get_by_id(id);
   if (idx == -1) {
-    return -1;
+    return 0;
   }
-  pt = X509_PURPOSE_get0(idx);
+  const X509_PURPOSE *pt = X509_PURPOSE_get0(idx);
   return pt->check_purpose(pt, x, ca);
 }