From 5e1522d7a7ec9ccf4347cb4dbd0c27145e1c80a8 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 25 Dec 2023 22:52:22 -0500 Subject: [PATCH] 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 Reviewed-by: Bob Beck (cherry picked from commit 0c8bc4653e34892dc291b48fb38e180ce92b5921) --- crypto/x509/v3_purp.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index a7a4e888e2..fdd920f779 100644 --- a/crypto/x509/v3_purp.c +++ b/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); }