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)

diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index 4c234f5a745a6b32caa696ca47495f7fa18b75e1..3d8e4746cfb1693662bc91fa63af8705437e66b4 100644 (file)
--- 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 0bd23c21b11601298a7a95930e80666defa28a58..bce0fa760cbd588c7c6e0dcc7dd335c870768ec2 100644 (file)
--- 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. */

diff --git a/doc/man3/DEFINE_STACK_OF.pod b/doc/man3/DEFINE_STACK_OF.pod
index ad990f2cdb486490dfe785306daf6c698c662539..49b1efddc4599cbdf26db835b198b114383fea15 100644 (file)
--- a/doc/man3/DEFINE_STACK_OF.pod
+++ b/doc/man3/DEFINE_STACK_OF.pod
@@ -8,8 +8,9 @@ sk_TYPE_num, sk_TYPE_value, sk_TYPE_new, sk_TYPE_new_null,
 sk_TYPE_reserve, sk_TYPE_free, sk_TYPE_zero, sk_TYPE_delete,
 sk_TYPE_delete_ptr, sk_TYPE_push, sk_TYPE_unshift, sk_TYPE_pop,
 sk_TYPE_shift, sk_TYPE_pop_free, sk_TYPE_insert, sk_TYPE_set,
-sk_TYPE_find, sk_TYPE_find_ex, sk_TYPE_sort, sk_TYPE_is_sorted,
-sk_TYPE_dup, sk_TYPE_deep_copy, sk_TYPE_set_cmp_func, sk_TYPE_new_reserve
+sk_TYPE_find, sk_TYPE_find_ex, sk_TYPE_find_all, sk_TYPE_sort,
+sk_TYPE_is_sorted, sk_TYPE_dup, sk_TYPE_deep_copy, sk_TYPE_set_cmp_func,
+sk_TYPE_new_reserve
 - stack container
 
 =head1 SYNOPSIS
@@ -46,6 +47,7 @@ sk_TYPE_dup, sk_TYPE_deep_copy, sk_TYPE_set_cmp_func, sk_TYPE_new_reserve
  TYPE *sk_TYPE_set(STACK_OF(TYPE) *sk, int idx, const TYPE *ptr);
  int sk_TYPE_find(STACK_OF(TYPE) *sk, TYPE *ptr);
  int sk_TYPE_find_ex(STACK_OF(TYPE) *sk, TYPE *ptr);
+ int sk_TYPE_find_all(STACK_OF(TYPE) *sk, TYPE *ptr, int *pnum);
  void sk_TYPE_sort(const STACK_OF(TYPE) *sk);
  int sk_TYPE_is_sorted(const STACK_OF(TYPE) *sk);
  STACK_OF(TYPE) *sk_TYPE_dup(const STACK_OF(TYPE) *sk);
@@ -165,18 +167,23 @@ B<sk_I<TYPE>_find>() searches I<sk> for the element I<ptr>.  In the case
 where no comparison function has been specified, the function performs
 a linear search for a pointer equal to I<ptr>. The index of the first
 matching element is returned or B<-1> if there is no match. In the case
-where a comparison function has been specified, I<sk> is sorted then
+where a comparison function has been specified, I<sk> is sorted and
 B<sk_I<TYPE>_find>() returns the index of a matching element or B<-1> if there
-is no match. Note that, in this case, the matching element returned is
-not guaranteed to be the first; the comparison function will usually
+is no match. Note that, in this case the comparison function will usually
 compare the values pointed to rather than the pointers themselves and
-the order of elements in I<sk> could change.
+the order of elements in I<sk> can change.
 
 B<sk_I<TYPE>_find_ex>() operates like B<sk_I<TYPE>_find>() except when a
 comparison function has been specified and no matching element is found.
 Instead of returning B<-1>, B<sk_I<TYPE>_find_ex>() returns the index of the
 element either before or after the location where I<ptr> would be if it were
-present in I<sk>.
+present in I<sk>. The function also does not guarantee that the first matching
+element in the sorted stack is returned.
+
+B<sk_I<TYPE>_find_all>() operates like B<sk_I<TYPE>_find>() but it also
+sets the I<*pnum> to number of matching elements in the stack. In case
+no comparison function has been specified the I<*pnum> will be always set
+to 1 if matching element was found, 0 otherwise.
 
 B<sk_I<TYPE>_sort>() sorts I<sk> using the supplied comparison function.
 

diff --git a/include/openssl/safestack.h.in b/include/openssl/safestack.h.in
index aac56608ca3aef1e076599a99218d9ff58b490ff..7bd4410dfc7bec48c4ea74609f134b6ed7ec8f86 100644 (file)
--- a/include/openssl/safestack.h.in
+++ b/include/openssl/safestack.h.in
@@ -146,6 +146,10 @@ extern "C" {
     { \
         return OPENSSL_sk_find_ex((OPENSSL_STACK *)sk, (const void *)ptr); \
     } \
+    static ossl_unused ossl_inline int sk_##t1##_find_all(STACK_OF(t1) *sk, t2 *ptr, int *pnum) \
+    { \
+        return OPENSSL_sk_find_all((OPENSSL_STACK *)sk, (const void *)ptr, pnum); \
+    } \
     static ossl_unused ossl_inline void sk_##t1##_sort(STACK_OF(t1) *sk) \
     { \
         OPENSSL_sk_sort((OPENSSL_STACK *)sk); \

diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index 031b672ed17c25abffb423a386b9e0bf3433d40c..79c25030cb51923a5ba867b7219b113268c8d88c 100644 (file)
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -45,6 +45,7 @@ void *OPENSSL_sk_delete(OPENSSL_STACK *st, int loc);
 void *OPENSSL_sk_delete_ptr(OPENSSL_STACK *st, const void *p);
 int OPENSSL_sk_find(OPENSSL_STACK *st, const void *data);
 int OPENSSL_sk_find_ex(OPENSSL_STACK *st, const void *data);
+int OPENSSL_sk_find_all(OPENSSL_STACK *st, const void *data, int *pnum);
 int OPENSSL_sk_push(OPENSSL_STACK *st, const void *data);
 int OPENSSL_sk_unshift(OPENSSL_STACK *st, const void *data);
 void *OPENSSL_sk_shift(OPENSSL_STACK *st);

diff --git a/util/libcrypto.num b/util/libcrypto.num
index b938f11d1ebf6dda9a263bdf468d8d2ceec61a9b..f49ebeef45cbc5838b271cc81d37533e07547a9c 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -5347,6 +5347,7 @@ EVP_ASYM_CIPHER_description             ? 3_0_0   EXIST::FUNCTION:
 EVP_KEM_description                     ?      3_0_0   EXIST::FUNCTION:
 EVP_KEYEXCH_description                 ?      3_0_0   EXIST::FUNCTION:
 EVP_KDF_description                     ?      3_0_0   EXIST::FUNCTION:
+OPENSSL_sk_find_all                     ?      3_0_0   EXIST::FUNCTION:
 X509_CRL_new_ex                         ?      3_0_0   EXIST::FUNCTION:
 OSSL_PARAM_dup                          ?      3_0_0   EXIST::FUNCTION:
 OSSL_PARAM_merge                        ?      3_0_0   EXIST::FUNCTION:

diff --git a/util/perl/OpenSSL/stackhash.pm b/util/perl/OpenSSL/stackhash.pm
index 7cf9c26411eb13759310af39528d571c3c666a78..f99e1690a22e98181f8ff1ad9ad1874cee6293d0 100644 (file)
--- a/util/perl/OpenSSL/stackhash.pm
+++ b/util/perl/OpenSSL/stackhash.pm
@@ -45,6 +45,7 @@ SKM_DEFINE_STACK_OF_INTERNAL(${nametype}, ${realtype}, ${plaintype})
 #define sk_${nametype}_set(sk, idx, ptr) ((${realtype} *)OPENSSL_sk_set(ossl_check_${nametype}_sk_type(sk), (idx), ossl_check_${nametype}_type(ptr)))
 #define sk_${nametype}_find(sk, ptr) OPENSSL_sk_find(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_type(ptr))
 #define sk_${nametype}_find_ex(sk, ptr) OPENSSL_sk_find_ex(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_type(ptr))
+#define sk_${nametype}_find_all(sk, ptr, pnum) OPENSSL_sk_find_all(ossl_check_${nametype}_sk_type(sk), ossl_check_${nametype}_type(ptr), pnum)
 #define sk_${nametype}_sort(sk) OPENSSL_sk_sort(ossl_check_${nametype}_sk_type(sk))
 #define sk_${nametype}_is_sorted(sk) OPENSSL_sk_is_sorted(ossl_check_const_${nametype}_sk_type(sk))
 #define sk_${nametype}_dup(sk) ((STACK_OF(${nametype}) *)OPENSSL_sk_dup(ossl_check_const_${nametype}_sk_type(sk)))