[openssl] master update

dev at ddvo.net dev at ddvo.net
Wed May 19 18:16:14 UTC 2021


The branch master has been updated
       via  e34e91d7e575a2f69119601f2d34655cb6816148 (commit)
       via  d6bf19a465968b6ecc98b479fc770651deaa4e01 (commit)
       via  558f2a014646bb057f3876b28e32b13d8178400e (commit)
       via  fc48b5c825352f519538ed26f2caa8aeca8b9ba0 (commit)
       via  e2abc685b70bc7d6525d4c1aab9e031b1986ddd8 (commit)
       via  aaa584cee7d2172b6a4a5165d685b473b07a0de3 (commit)
      from  da750b15c0e69f809243d56eceb37d56a8fc9cfd (commit)


- Log -----------------------------------------------------------------
commit e34e91d7e575a2f69119601f2d34655cb6816148
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu Mar 4 21:18:45 2021 +0100

    danetest.c: Improve code formatting
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14422)

commit d6bf19a465968b6ecc98b479fc770651deaa4e01
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu Mar 4 21:18:09 2021 +0100

    X509_STORE_CTX_get1_issuer(): Simplify code, reducing risk of failure
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14422)

commit 558f2a014646bb057f3876b28e32b13d8178400e
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu Mar 4 21:17:31 2021 +0100

    X509 build_chain(): Fix two potential memory leaks on issuer variable
    
    This also removes an inadequate guard: if (num == ctx->num_untrusted)
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14422)

commit fc48b5c825352f519538ed26f2caa8aeca8b9ba0
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu Mar 4 17:35:46 2021 +0100

    X509 build_chain(): Make the variable 'curr' local to the loop body
    
    This increases readability and maintainability.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14422)

commit e2abc685b70bc7d6525d4c1aab9e031b1986ddd8
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu Mar 4 10:59:18 2021 +0100

    X509 build_chain(): Rename variable 'depth' to 'max_depth'
    
    This should increase readability and maintainability.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14422)

commit aaa584cee7d2172b6a4a5165d685b473b07a0de3
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu Mar 4 10:56:27 2021 +0100

    X509 build_chain(): Restrict scope of 'self_signed' variable
    
    This should increase readability and maintainability.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14422)

-----------------------------------------------------------------------

Summary of changes:
 crypto/x509/x509_lu.c                      |  9 ++----
 crypto/x509/x509_vfy.c                     | 50 ++++++++++++++----------------
 doc/man3/X509_STORE_set_verify_cb_func.pod | 49 +++++++++++++++++------------
 test/danetest.c                            | 20 ++++++------
 util/missingcrypto.txt                     |  1 -
 5 files changed, 65 insertions(+), 64 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/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 4e6ce11f4e..ddb3378eee 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -2965,10 +2965,10 @@ static int dane_verify(X509_STORE_CTX *ctx)
 }
 
 /*
- * Get issuer, without duplicate suppression
+ * Get trusted issuer, without duplicate suppression
  * Returns -1 on internal error.
  */
-static int get_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *cert)
+static int get1_trusted_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *cert)
 {
     STACK_OF(X509) *saved_chain = ctx->chain;
     int ok;
@@ -2985,15 +2985,13 @@ static int build_chain(X509_STORE_CTX *ctx)
 {
     SSL_DANE *dane = ctx->dane;
     int num = sk_X509_num(ctx->chain);
-    X509 *curr = sk_X509_value(ctx->chain, num - 1); /* current end of chain */
-    int self_signed = X509_self_signed(curr, 0); /* always refers to curr */
     STACK_OF(X509) *sk_untrusted = NULL;
     unsigned int search;
     int may_trusted = 0;
     int may_alternate = 0;
     int trust = X509_TRUST_UNTRUSTED;
     int alt_untrusted = 0;
-    int depth;
+    int max_depth;
     int ok = 0;
     int prev_error = ctx->error;
     int i;
@@ -3001,8 +2999,6 @@ static int build_chain(X509_STORE_CTX *ctx)
     /* Our chain starts with a single untrusted element. */
     if (!ossl_assert(num == 1 && ctx->num_untrusted == num))
         goto int_err;
-    if (self_signed < 0)
-        goto int_err;
 
 #define S_DOUNTRUSTED (1 << 0) /* Search untrusted chain */
 #define S_DOTRUSTED   (1 << 1) /* Search trusted store */
@@ -3051,10 +3047,10 @@ static int build_chain(X509_STORE_CTX *ctx)
      * Build chains up to one longer the limit, later fail if we hit the limit,
      * with an X509_V_ERR_CERT_CHAIN_TOO_LONG error code.
      */
-    depth = ctx->param->depth + 1;
+    max_depth = ctx->param->depth + 1;
 
     while (search != 0) {
-        X509 *issuer = NULL;
+        X509 *curr, *issuer = NULL;
 
         num = sk_X509_num(ctx->chain);
         ctx->error_depth = num - 1;
@@ -3094,7 +3090,8 @@ static int build_chain(X509_STORE_CTX *ctx)
             }
             curr = sk_X509_value(ctx->chain, i - 1);
 
-            ok = num > depth ? 0 : get_issuer(&issuer, ctx, curr);
+            /* Note: get1_trusted_issuer() must be used even if self-signed. */
+            ok = num > max_depth ? 0 : get1_trusted_issuer(&issuer, ctx, curr);
 
             if (ok < 0) {
                 trust = -1;
@@ -3103,6 +3100,12 @@ static int build_chain(X509_STORE_CTX *ctx)
             }
 
             if (ok > 0) {
+                int self_signed = X509_self_signed(curr, 0);
+
+                if (self_signed < 0) {
+                    X509_free(issuer);
+                    goto int_err;
+                }
                 /*
                  * Alternative trusted issuer for a mid-chain untrusted cert?
                  * Pop the untrusted cert's successors and retry.  We might now
@@ -3143,14 +3146,13 @@ static int build_chain(X509_STORE_CTX *ctx)
                  * trusted matching issuer.  Otherwise, grow the chain.
                  */
                 if (!self_signed) {
-                    curr = issuer;
-                    if ((self_signed = X509_self_signed(curr, 0)) < 0)
-                        goto int_err;
-                    if (!sk_X509_push(ctx->chain, curr)) {
+                    if (!sk_X509_push(ctx->chain, issuer)) {
                         X509_free(issuer);
                         goto memerr;
                     }
-                } else if (num == ctx->num_untrusted) {
+                    if ((self_signed = X509_self_signed(issuer, 0)) < 0)
+                        goto int_err;
+                } else {
                     /*
                      * We have a self-signed certificate that has the same
                      * subject name (and perhaps keyid and/or serial number) as
@@ -3165,8 +3167,6 @@ static int build_chain(X509_STORE_CTX *ctx)
                         X509_free(curr);
                         ctx->num_untrusted = --num;
                         (void)sk_X509_set(ctx->chain, num, issuer);
-                        curr = issuer;
-                        /* no need to update self_signed */
                     }
                 }
 
@@ -3212,7 +3212,6 @@ static int build_chain(X509_STORE_CTX *ctx)
                 /* Search for a trusted issuer of a shorter chain */
                 search |= S_DOALTERNATE;
                 alt_untrusted = ctx->num_untrusted - 1;
-                self_signed = 0;
             }
         }
 
@@ -3224,11 +3223,11 @@ static int build_chain(X509_STORE_CTX *ctx)
             if (!ossl_assert(num == ctx->num_untrusted))
                 goto int_err;
             curr = sk_X509_value(ctx->chain, num - 1);
-            issuer = (self_signed || num > depth) ?
+            issuer = (X509_self_signed(curr, 0) || num > max_depth) ?
                 NULL : find_issuer(ctx, sk_untrusted, curr);
             if (issuer == NULL) {
                 /*
-                 * Once we have reached a self-signed cert or num exceeds depth
+                 * Once we have reached a self-signed cert or num > max_depth
                  * or can't find an issuer in the untrusted list we stop looking
                  * there and start looking only in the trust store if enabled.
                  */
@@ -3245,9 +3244,6 @@ static int build_chain(X509_STORE_CTX *ctx)
                 goto int_err;
 
             ++ctx->num_untrusted;
-            curr = issuer;
-            if ((self_signed = X509_self_signed(curr, 0)) < 0)
-                goto int_err;
 
             /* Check for DANE-TA trust of the topmost untrusted certificate. */
             trust = check_dane_issuer(ctx, ctx->num_untrusted - 1);
@@ -3265,7 +3261,7 @@ static int build_chain(X509_STORE_CTX *ctx)
      * signers, or else direct leaf PKIX trust.
      */
     num = sk_X509_num(ctx->chain);
-    if (num <= depth) {
+    if (num <= max_depth) {
         if (trust == X509_TRUST_UNTRUSTED && DANETLS_HAS_DANE_TA(dane))
             trust = check_dane_pkeys(ctx);
         if (trust == X509_TRUST_UNTRUSTED && num == ctx->num_untrusted)
@@ -3293,14 +3289,14 @@ static int build_chain(X509_STORE_CTX *ctx)
         case X509_V_OK:
             break;
         }
-        CB_FAIL_IF(num > depth,
+        CB_FAIL_IF(num > max_depth,
                    ctx, NULL, num - 1, X509_V_ERR_CERT_CHAIN_TOO_LONG);
         CB_FAIL_IF(DANETLS_ENABLED(dane)
                        && (!DANETLS_HAS_PKIX(dane) || dane->pdpth >= 0),
                    ctx, NULL, num - 1, X509_V_ERR_DANE_NO_MATCH);
-        if (self_signed)
+        if (X509_self_signed(sk_X509_value(ctx->chain, num - 1), 0))
             return verify_cb_cert(ctx, NULL, num - 1,
-                                  sk_X509_num(ctx->chain) == 1
+                                  num == 1
                                   ? X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT
                                   : X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN);
         return verify_cb_cert(ctx, NULL, num - 1,
diff --git a/doc/man3/X509_STORE_set_verify_cb_func.pod b/doc/man3/X509_STORE_set_verify_cb_func.pod
index 515a427aa3..5e59cbe5cc 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);
@@ -121,14 +122,14 @@ X509_STORE_CTX_lookup_certs_fn, X509_STORE_CTX_lookup_crls_fn
 
 =head1 DESCRIPTION
 
-X509_STORE_set_verify_cb() sets the verification callback of B<ctx> to
-B<verify_cb> overwriting the previous callback.
+X509_STORE_set_verify_cb() sets the verification callback of I<ctx> to
+I<verify_cb> overwriting the previous callback.
 The callback assigned with this function becomes a default for the one
 that can be assigned directly to the corresponding B<X509_STORE_CTX>,
 please see L<X509_STORE_CTX_set_verify_cb(3)> for further information.
 
 X509_STORE_set_verify() sets the final chain verification function for
-B<ctx> to B<verify>.
+I<ctx> to I<verify>.
 Its purpose is to go through the chain of certificates and check that
 all signatures are valid and that the current time is within the
 limits of each certificate's first and last validity time.
@@ -137,17 +138,24 @@ 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 I<x>.
+When such a certificate is found, I<get_issuer> must up-ref and assign it
+to I<*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>.
-This function must return 0 on failure (among others if B<x> hasn't
-been issued with B<issuer>) and 1 on success.
+certificate I<x> is issued by the issuer certificate I<issuer>.
+This function must return 0 on failure (among others if I<x> hasn't
+been issued with I<issuer>) and 1 on success.
 I<If no function to get the issuer is provided, the internal default
 function will be used instead.>
 
@@ -160,20 +168,20 @@ I<If no function to get the issuer is provided, the internal default
 function will be used instead.>
 
 X509_STORE_set_get_crl() sets the function to get the crl for a given
-certificate B<x>.
-When found, the crl must be assigned to B<*crl>.
+certificate I<x>.
+When found, the crl must be assigned to I<*crl>.
 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_set_check_crl() sets the function to check the validity of
-the given B<crl>.
+the given I<crl>.
 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_set_cert_crl() sets the function to check the revocation
-status of the given certificate B<x> against the given B<crl>.
+status of the given certificate I<x> against the given I<crl>.
 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.>
@@ -186,7 +194,7 @@ function will be used instead.>
 
 X509_STORE_set_lookup_certs() and X509_STORE_set_lookup_crls() set the
 functions to look up all the certs or all the CRLs that match the
-given name B<nm>.
+given name I<nm>.
 These functions return NULL on failure and a pointer to a stack of
 certificates (B<X509>) or to a stack of CRLs (B<X509_CRL>) on
 success.
@@ -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/test/danetest.c b/test/danetest.c
index 7d4b0c88a7..6217e5470d 100644
--- a/test/danetest.c
+++ b/test/danetest.c
@@ -20,7 +20,7 @@
 #include <openssl/err.h>
 #include <openssl/conf.h>
 #ifndef OPENSSL_NO_ENGINE
-#include <openssl/engine.h>
+# include <openssl/engine.h>
 #endif
 #include "testutil.h"
 
@@ -68,10 +68,10 @@ static int verify_chain(SSL *ssl, STACK_OF(X509) *chain)
                                                      ssl)))
         goto end;
 
-    X509_STORE_CTX_set_default(store_ctx,
-            SSL_is_server(ssl) ? "ssl_client" : "ssl_server");
+    X509_STORE_CTX_set_default(store_ctx, SSL_is_server(ssl)
+                               ? "ssl_client" : "ssl_server");
     X509_VERIFY_PARAM_set1(X509_STORE_CTX_get0_param(store_ctx),
-            SSL_get0_param(ssl));
+                           SSL_get0_param(ssl));
     store_ctx_dane_init(store_ctx, ssl);
 
     if (SSL_get_verify_callback(ssl) != NULL)
@@ -95,7 +95,7 @@ static STACK_OF(X509) *load_chain(BIO *fp, int nelem)
     char *header = 0;
     unsigned char *data = 0;
     long len;
-    char *errtype = 0;                /* if error: cert or pkey? */
+    char *errtype = 0; /* if error: cert or pkey? */
     STACK_OF(X509) *chain;
     typedef X509 *(*d2i_X509_t)(X509 **, const unsigned char **, long);
 
@@ -107,8 +107,8 @@ static STACK_OF(X509) *load_chain(BIO *fp, int nelem)
          && PEM_read_bio(fp, &name, &header, &data, &len) == 1;
          ++count) {
         if (strcmp(name, PEM_STRING_X509) == 0
-                    || strcmp(name, PEM_STRING_X509_TRUSTED) == 0
-                    || strcmp(name, PEM_STRING_X509_OLD) == 0) {
+                || strcmp(name, PEM_STRING_X509_TRUSTED) == 0
+                || strcmp(name, PEM_STRING_X509_OLD) == 0) {
             d2i_X509_t d = strcmp(name, PEM_STRING_X509_TRUSTED) != 0
                 ? d2i_X509_AUX : d2i_X509;
             X509 *cert;
@@ -391,10 +391,8 @@ static int run_tlsatest(void)
             || !TEST_ptr(ctx = SSL_CTX_new(TLS_client_method()))
             || !TEST_int_gt(SSL_CTX_dane_enable(ctx), 0)
             || !TEST_true(SSL_CTX_load_verify_file(ctx, CAfile))
-            || !TEST_int_gt(SSL_CTX_dane_mtype_set(ctx, EVP_sha512(), 2, 1),
-                            0)
-            || !TEST_int_gt(SSL_CTX_dane_mtype_set(ctx, EVP_sha256(), 1, 2),
-                            0)
+            || !TEST_int_gt(SSL_CTX_dane_mtype_set(ctx, EVP_sha512(), 2, 1), 0)
+            || !TEST_int_gt(SSL_CTX_dane_mtype_set(ctx, EVP_sha256(), 1, 2), 0)
             || !TEST_int_gt(test_tlsafile(ctx, basedomain, f, tlsafile), 0))
         goto end;
     ret = 1;
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)


More information about the openssl-commits mailing list