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 <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 7ef3d37..a382592 100644
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -118,24 +118,21 @@
(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.
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);
}
