X509_STORE_CTX_get1_issuer(): Simplify code, reducing risk of failure

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14422)

3 files changed, 22 insertions, 15 deletions

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index bce0fa760c..b36ddb69a1 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -321,7 +321,6 @@ int X509_STORE_CTX_get_by_subject(const X509_STORE_CTX *vs,
     stmp.type = X509_LU_NONE;
     stmp.data.ptr = NULL;
 
-
     X509_STORE_lock(store);
     tmp = X509_OBJECT_retrieve_by_subject(store->objs, type, name);
     X509_STORE_unlock(store);
@@ -728,12 +727,10 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
     if (ctx->check_issued(ctx, x, obj->data.x509)) {
         if (ossl_x509_check_cert_time(ctx, obj->data.x509, -1)) {
             *issuer = obj->data.x509;
-            if (!X509_up_ref(*issuer)) {
-                *issuer = NULL;
-                ok = -1;
-            }
+            /* |*issuer| has taken over the cert reference from |obj| */
+            obj->type = X509_LU_NONE;
             X509_OBJECT_free(obj);
-            return ok;
+            return 1;
         }
     }
     X509_OBJECT_free(obj);
diff --git a/doc/man3/X509_STORE_set_verify_cb_func.pod b/doc/man3/X509_STORE_set_verify_cb_func.pod
index 515a427aa3..00b2270b59 100644
--- a/doc/man3/X509_STORE_set_verify_cb_func.pod
+++ b/doc/man3/X509_STORE_set_verify_cb_func.pod
@@ -22,6 +22,7 @@ X509_STORE_get_check_revocation,
 X509_STORE_set_check_revocation,
 X509_STORE_get_check_issued,
 X509_STORE_set_check_issued,
+X509_STORE_CTX_get1_issuer,
 X509_STORE_get_get_issuer,
 X509_STORE_set_get_issuer,
 X509_STORE_CTX_get_verify,
@@ -64,10 +65,10 @@ X509_STORE_CTX_lookup_certs_fn, X509_STORE_CTX_lookup_crls_fn
  void X509_STORE_set_verify(X509_STORE *ctx, X509_STORE_CTX_verify_fn verify);
  X509_STORE_CTX_verify_fn X509_STORE_CTX_get_verify(const X509_STORE_CTX *ctx);
 
+ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x);
+ X509_STORE_CTX_get_issuer_fn X509_STORE_get_get_issuer(const X509_STORE_CTX *ctx);
  void X509_STORE_set_get_issuer(X509_STORE *ctx,
                                 X509_STORE_CTX_get_issuer_fn get_issuer);
- X509_STORE_CTX_get_issuer_fn
-     X509_STORE_get_get_issuer(const X509_STORE_CTX *ctx);
 
  void X509_STORE_set_check_issued(X509_STORE *ctx,
                                   X509_STORE_CTX_check_issued_fn check_issued);
@@ -137,12 +138,19 @@ on success.
 I<If no chain verification function is provided, the internal default
 function will be used instead.>
 
-X509_STORE_set_get_issuer() sets the function to get the issuer
-certificate that verifies the given certificate B<x>.
-When found, the issuer certificate must be assigned to B<*issuer>.
-This function must return 0 on failure and 1 on success.
-I<If no function to get the issuer is provided, the internal default
-function will be used instead.>
+X509_STORE_CTX_get1_issuer() tries to find a certificate from the I<store>
+component of I<ctx> with a subject name matching the issuer name of I<x>.
+On success it assigns to I<*issuer> the first match that is currently valid,
+or at least the most recently expired match if there is no currently valid one.
+If the function returns 1 the caller is responsible for freeing I<*issuer>.
+
+X509_STORE_set_get_issuer() sets the function I<get_issuer>
+to get the "best" candidate issuer certificate of the given certificate B<x>.
+When such a certificate is found, I<get_issuer> must up-ref and assign it
+to B<*issuer> and then return 1.
+Otherwise I<get_issuer> must return 0 if not found and -1 (or 0) on failure.
+If X509_STORE_set_get_issuer() is not used or I<get_issuer> is NULL
+then X509_STORE_CTX_get1_issuer() is used as the default implementation.
 
 X509_STORE_set_check_issued() sets the function to check that a given
 certificate B<x> is issued by the issuer certificate B<issuer>.
@@ -237,6 +245,9 @@ The X509_STORE_set_*() functions do not return a value.
 The X509_STORE_get_*() functions return a pointer of the appropriate
 function type.
 
+X509_STORE_CTX_get1_issuer() returns
+1 if a suitable certificate is found, 0 if not found, -1 on other error.
+
 =head1 SEE ALSO
 
 L<X509_STORE_CTX_set_verify_cb(3)>, L<X509_STORE_CTX_get0_chain(3)>,
diff --git a/util/missingcrypto.txt b/util/missingcrypto.txt
index 5847e6446b..0946be28a0 100644
--- a/util/missingcrypto.txt
+++ b/util/missingcrypto.txt
@@ -1297,7 +1297,6 @@ X509_STORE_CTX_get0_policy_tree(3)
 X509_STORE_CTX_get0_store(3)
 X509_STORE_CTX_get1_certs(3)
 X509_STORE_CTX_get1_crls(3)
-X509_STORE_CTX_get1_issuer(3)
 X509_STORE_CTX_get_by_subject(3)
 X509_STORE_CTX_get_explicit_policy(3)
 X509_STORE_CTX_get_obj_by_subject(3)