Improve the implementation of X509_STORE_CTX_get1_issuer()

It is possible for the stack of X509_OBJECTs held in an X509_STORE_CTX to
have a custom compare function associated with it. Normally (by default)
this uses X509_NAME_cmp(). The X509_STORE_CTX_get1_issuer() function
assumed that it would always be X509_NAME_cmp().

By implementing OPENSSL_sk_find_all() function we can avoid explicitly
using X509_NAME_cmp() in X509_STORE_CTX_get1_issuer().

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14728)

2 files changed, 34 insertions, 22 deletions

diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index 4c234f5a74..3d8e4746cf 100644
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -297,7 +297,7 @@ void *OPENSSL_sk_delete(OPENSSL_STACK *st, int loc)
 }
 
 static int internal_find(OPENSSL_STACK *st, const void *data,
-                         int ret_val_options)
+                         int ret_val_options, int *pnum)
 {
     const void *r;
     int i;
@@ -307,8 +307,13 @@ static int internal_find(OPENSSL_STACK *st, const void *data,
 
     if (st->comp == NULL) {
         for (i = 0; i < st->num; i++)
-            if (st->data[i] == data)
+            if (st->data[i] == data) {
+                if (pnum != NULL)
+                    *pnum = 1;
                 return i;
+            }
+        if (pnum != NULL)
+            *pnum = 0;
         return -1;
     }
 
@@ -319,20 +324,41 @@ static int internal_find(OPENSSL_STACK *st, const void *data,
     }
     if (data == NULL)
         return -1;
+    if (pnum != NULL)
+        ret_val_options |= OSSL_BSEARCH_FIRST_VALUE_ON_MATCH;
     r = ossl_bsearch(&data, st->data, st->num, sizeof(void *), st->comp,
                      ret_val_options);
 
+    if (pnum != NULL) {
+        *pnum = 0;
+        if (r != NULL) {
+            const void **p = (const void **)r;
+
+            while (p < st->data + st->num) {
+                if (st->comp(&data, p) != 0)
+                    break;
+                ++*pnum;
+                ++p;
+            }
+        }
+    }
+
     return r == NULL ? -1 : (int)((const void **)r - st->data);
 }
 
 int OPENSSL_sk_find(OPENSSL_STACK *st, const void *data)
 {
-    return internal_find(st, data, OSSL_BSEARCH_FIRST_VALUE_ON_MATCH);
+    return internal_find(st, data, OSSL_BSEARCH_FIRST_VALUE_ON_MATCH, NULL);
 }
 
 int OPENSSL_sk_find_ex(OPENSSL_STACK *st, const void *data)
 {
-    return internal_find(st, data, OSSL_BSEARCH_VALUE_ON_NOMATCH);
+    return internal_find(st, data, OSSL_BSEARCH_VALUE_ON_NOMATCH, NULL);
+}
+
+int OPENSSL_sk_find_all(OPENSSL_STACK *st, const void *data, int *pnum)
+{
+    return internal_find(st, data, OSSL_BSEARCH_FIRST_VALUE_ON_MATCH, pnum);
 }
 
 int OPENSSL_sk_push(OPENSSL_STACK *st, const void *data)
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 0bd23c21b1..bce0fa760c 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -516,19 +516,7 @@ static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, X509_LOOKUP_TYPE type,
         return -1;
     }
 
-    idx = sk_X509_OBJECT_find(h, &stmp);
-    if (idx >= 0 && pnmatch) {
-        int tidx;
-        const X509_OBJECT *tobj, *pstmp;
-        *pnmatch = 1;
-        pstmp = &stmp;
-        for (tidx = idx + 1; tidx < sk_X509_OBJECT_num(h); tidx++) {
-            tobj = sk_X509_OBJECT_value(h, tidx);
-            if (x509_object_cmp(&tobj, &pstmp))
-                break;
-            (*pnmatch)++;
-        }
-    }
+    idx = sk_X509_OBJECT_find_all(h, &stmp, pnmatch);
     return idx;
 }
 
@@ -725,7 +713,7 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
     const X509_NAME *xn;
     X509_OBJECT *obj = X509_OBJECT_new(), *pobj = NULL;
     X509_STORE *store = ctx->store;
-    int i, ok, idx, ret;
+    int i, ok, idx, ret, nmatch = 0;
 
     if (obj == NULL)
         return -1;
@@ -761,16 +749,14 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
     /* Find index of first currently valid cert accepted by 'check_issued' */
     ret = 0;
     X509_STORE_lock(store);
-    idx = X509_OBJECT_idx_by_subject(store->objs, X509_LU_X509, xn);
+    idx = x509_object_idx_cnt(store->objs, X509_LU_X509, xn, &nmatch);
     if (idx != -1) { /* should be true as we've had at least one match */
         /* Look through all matching certs for suitable issuer */
-        for (i = idx; i < sk_X509_OBJECT_num(store->objs); i++) {
+        for (i = idx; i < idx + nmatch; i++) {
             pobj = sk_X509_OBJECT_value(store->objs, i);
             /* See if we've run past the matches */
             if (pobj->type != X509_LU_X509)
                 break;
-            if (X509_NAME_cmp(X509_get_subject_name(pobj->data.x509), xn) != 0)
-                break; /* Not more cert matches xn */
             if (ctx->check_issued(ctx, x, pobj->data.x509)) {
                 ret = 1;
                 /* If times check fine, exit with match, else keep looking. */