[openssl] master update

dev at ddvo.net dev at ddvo.net
Thu Feb 4 15:43:39 UTC 2021


The branch master has been updated
       via  88444854affe31ce08a5daaf4b6afc86e6972c63 (commit)
      from  af4d6c26af0bfaa837589b4fe39ec4942dd4c5b3 (commit)


- Log -----------------------------------------------------------------
commit 88444854affe31ce08a5daaf4b6afc86e6972c63
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Sun Oct 4 21:55:49 2020 +0200

    x509_vfy.c: Improve coding style and comments all over the file
    
    No changes in semantics.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/13070)

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

Summary of changes:
 crypto/x509/x509_vfy.c | 680 ++++++++++++++++++++++---------------------------
 1 file changed, 301 insertions(+), 379 deletions(-)

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 8e78c13b8e..ec7df5caa6 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -29,41 +29,16 @@
 
 /* CRL score values */
 
-/* No unhandled critical extensions */
-
-#define CRL_SCORE_NOCRITICAL    0x100
-
-/* certificate is within CRL scope */
-
-#define CRL_SCORE_SCOPE         0x080
-
-/* CRL times valid */
-
-#define CRL_SCORE_TIME          0x040
-
-/* Issuer name matches certificate */
-
-#define CRL_SCORE_ISSUER_NAME   0x020
-
-/* If this score or above CRL is probably valid */
-
-#define CRL_SCORE_VALID (CRL_SCORE_NOCRITICAL|CRL_SCORE_TIME|CRL_SCORE_SCOPE)
-
-/* CRL issuer is certificate issuer */
-
-#define CRL_SCORE_ISSUER_CERT   0x018
-
-/* CRL issuer is on certificate path */
-
-#define CRL_SCORE_SAME_PATH     0x008
-
-/* CRL issuer matches CRL AKID */
-
-#define CRL_SCORE_AKID          0x004
-
-/* Have a delta CRL with valid times */
-
-#define CRL_SCORE_TIME_DELTA    0x002
+#define CRL_SCORE_NOCRITICAL    0x100 /* No unhandled critical extensions */
+#define CRL_SCORE_SCOPE         0x080 /* certificate is within CRL scope */
+#define CRL_SCORE_TIME          0x040 /* CRL times valid */
+#define CRL_SCORE_ISSUER_NAME   0x020 /* Issuer name matches certificate */
+#define CRL_SCORE_VALID /* If this score or above CRL is probably valid */ \
+    (CRL_SCORE_NOCRITICAL | CRL_SCORE_TIME | CRL_SCORE_SCOPE)
+#define CRL_SCORE_ISSUER_CERT   0x018 /* CRL issuer is certificate issuer */
+#define CRL_SCORE_SAME_PATH     0x008 /* CRL issuer is on certificate path */
+#define CRL_SCORE_AKID          0x004 /* CRL issuer matches CRL AKID */
+#define CRL_SCORE_TIME_DELTA    0x002 /* Have a delta CRL with valid times */
 
 static int build_chain(X509_STORE_CTX *ctx);
 static int verify_chain(X509_STORE_CTX *ctx);
@@ -137,6 +112,7 @@ static X509 *lookup_cert_match(X509_STORE_CTX *ctx, X509 *x)
     STACK_OF(X509) *certs;
     X509 *xtmp = NULL;
     int i;
+
     /* Lookup all certs with matching subject name */
     ERR_set_mark();
     certs = ctx->lookup_certs(ctx, X509_get_subject_name(x));
@@ -233,26 +209,26 @@ static int verify_chain(X509_STORE_CTX *ctx)
         (ok = check_id(ctx)) == 0 || 1)
         X509_get_pubkey_parameters(NULL, ctx->chain);
     if (ok == 0 || (ok = ctx->check_revocation(ctx)) == 0)
-        return ok;
+        return 0;
 
     err = X509_chain_check_suiteb(&ctx->error_depth, NULL, ctx->chain,
                                   ctx->param->flags);
     CB_FAIL_IF(err != X509_V_OK, ctx, NULL, ctx->error_depth, err);
 
     /* Verify chain signatures and expiration times */
-    ok = (ctx->verify != NULL) ? ctx->verify(ctx) : internal_verify(ctx);
+    ok = ctx->verify != NULL ? ctx->verify(ctx) : internal_verify(ctx);
     if (!ok)
-        return ok;
+        return 0;
 
     if ((ok = check_name_constraints(ctx)) == 0)
-        return ok;
+        return 0;
 
 #ifndef OPENSSL_NO_RFC3779
     /* RFC 3779 path validation, now that CRL check has been done */
     if ((ok = X509v3_asid_validate_path(ctx)) == 0)
-        return ok;
+        return 0;
     if ((ok = X509v3_addr_validate_path(ctx)) == 0)
-        return ok;
+        return 0;
 #endif
 
     /* If we get this far evaluate policies */
@@ -292,10 +268,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
     CB_FAIL_IF(!check_key_level(ctx, ctx->cert),
                ctx, ctx->cert, 0, X509_V_ERR_EE_KEY_TOO_SMALL);
 
-    if (DANETLS_ENABLED(dane))
-        ret = dane_verify(ctx);
-    else
-        ret = verify_chain(ctx);
+    ret = DANETLS_ENABLED(dane) ? dane_verify(ctx) : verify_chain(ctx);
 
     /*
      * Safety-net.  If we are returning an error, we must also set ctx->error,
@@ -353,13 +326,9 @@ static int check_issued(ossl_unused X509_STORE_CTX *ctx, X509 *x, X509 *issuer)
 static int get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
 {
     *issuer = find_issuer(ctx, ctx->other_ctx, x);
+    if (*issuer != NULL && X509_up_ref(*issuer))
+        return 1;
 
-    if (*issuer == NULL || !X509_up_ref(*issuer))
-        goto err;
-
-    return 1;
-
- err:
     *issuer = NULL;
     return 0;
 }
@@ -440,10 +409,8 @@ static int check_chain(X509_STORE_CTX *ctx)
 {
     int i, must_be_ca, plen = 0;
     X509 *x;
-    int proxy_path_length = 0;
-    int purpose;
-    int allow_proxy_certs;
-    int num = sk_X509_num(ctx->chain);
+    int ret, proxy_path_length = 0;
+    int purpose, allow_proxy_certs, num = sk_X509_num(ctx->chain);
 
     /*-
      *  must_be_ca can have 1 of 3 values:
@@ -457,23 +424,21 @@ static int check_chain(X509_STORE_CTX *ctx)
     must_be_ca = -1;
 
     /* CRL path validation */
-    if (ctx->parent) {
+    if (ctx->parent != NULL) {
         allow_proxy_certs = 0;
         purpose = X509_PURPOSE_CRL_SIGN;
     } else {
         allow_proxy_certs =
-            ! !(ctx->param->flags & X509_V_FLAG_ALLOW_PROXY_CERTS);
+            (ctx->param->flags & X509_V_FLAG_ALLOW_PROXY_CERTS) != 0;
         purpose = ctx->param->purpose;
     }
 
     for (i = 0; i < num; i++) {
-        int ret;
-
         x = sk_X509_value(ctx->chain, i);
         CB_FAIL_IF((ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL) == 0
                        && (x->ex_flags & EXFLAG_CRITICAL) != 0,
                    ctx, x, i, X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION);
-        CB_FAIL_IF(!allow_proxy_certs && (x->ex_flags & EXFLAG_PROXY),
+        CB_FAIL_IF(!allow_proxy_certs && (x->ex_flags & EXFLAG_PROXY) != 0,
                    ctx, x, i, X509_V_ERR_PROXY_CERTIFICATES_NOT_ALLOWED);
         ret = X509_check_ca(x);
         switch (must_be_ca) {
@@ -489,7 +454,7 @@ static int check_chain(X509_STORE_CTX *ctx)
             /* X509_V_FLAG_X509_STRICT is implicit for intermediate CAs */
             CB_FAIL_IF(ret == 0
                        || ((i + 1 < num
-                            || ctx->param->flags & X509_V_FLAG_X509_STRICT)
+                            || (ctx->param->flags & X509_V_FLAG_X509_STRICT) != 0)
                            && ret != 1), ctx, x, i, X509_V_ERR_INVALID_CA);
             break;
         }
@@ -607,8 +572,9 @@ static int check_chain(X509_STORE_CTX *ctx)
             }
             proxy_path_length++;
             must_be_ca = 0;
-        } else
+        } else {
             must_be_ca = 1;
+        }
     }
     return 1;
 }
@@ -644,7 +610,7 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
         int j;
 
         /* Ignore self-issued certs unless last in chain */
-        if (i && (x->ex_flags & EXFLAG_SI))
+        if (i != 0 && (x->ex_flags & EXFLAG_SI) != 0)
             continue;
 
         /*
@@ -653,16 +619,16 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
          * added.
          * (RFC 3820: 3.4, 4.1.3 (a)(4))
          */
-        if (x->ex_flags & EXFLAG_PROXY) {
+        if ((x->ex_flags & EXFLAG_PROXY) != 0) {
             X509_NAME *tmpsubject = X509_get_subject_name(x);
             X509_NAME *tmpissuer = X509_get_issuer_name(x);
             X509_NAME_ENTRY *tmpentry = NULL;
-            int last_object_nid = 0;
+            int last_nid = 0;
             int err = X509_V_OK;
-            int last_object_loc = X509_NAME_entry_count(tmpsubject) - 1;
+            int last_loc = X509_NAME_entry_count(tmpsubject) - 1;
 
             /* Check that there are at least two RDNs */
-            if (last_object_loc < 1) {
+            if (last_loc < 1) {
                 err = X509_V_ERR_PROXY_SUBJECT_NAME_VIOLATION;
                 goto proxy_name_done;
             }
@@ -681,10 +647,9 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
              * Check that the last subject component isn't part of a
              * multi-valued RDN
              */
-            if (X509_NAME_ENTRY_set(X509_NAME_get_entry(tmpsubject,
-                                                        last_object_loc))
+            if (X509_NAME_ENTRY_set(X509_NAME_get_entry(tmpsubject, last_loc))
                 == X509_NAME_ENTRY_set(X509_NAME_get_entry(tmpsubject,
-                                                           last_object_loc - 1))) {
+                                                           last_loc - 1))) {
                 err = X509_V_ERR_PROXY_SUBJECT_NAME_VIOLATION;
                 goto proxy_name_done;
             }
@@ -700,12 +665,10 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
                 return 0;
             }
 
-            tmpentry =
-                X509_NAME_delete_entry(tmpsubject, last_object_loc);
-            last_object_nid =
-                OBJ_obj2nid(X509_NAME_ENTRY_get_object(tmpentry));
+            tmpentry = X509_NAME_delete_entry(tmpsubject, last_loc);
+            last_nid = OBJ_obj2nid(X509_NAME_ENTRY_get_object(tmpentry));
 
-            if (last_object_nid != NID_commonName
+            if (last_nid != NID_commonName
                 || X509_NAME_cmp(tmpsubject, tmpissuer) != 0) {
                 err = X509_V_ERR_PROXY_SUBJECT_NAME_VIOLATION;
             }
@@ -713,7 +676,7 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
             X509_NAME_ENTRY_free(tmpentry);
             X509_NAME_free(tmpsubject);
 
-         proxy_name_done:
+        proxy_name_done:
             CB_FAIL_IF(err != X509_V_OK, ctx, x, i, err);
         }
 
@@ -780,15 +743,17 @@ static int check_id(X509_STORE_CTX *ctx)
 {
     X509_VERIFY_PARAM *vpm = ctx->param;
     X509 *x = ctx->cert;
-    if (vpm->hosts && check_hosts(x, vpm) <= 0) {
+
+    if (vpm->hosts != NULL && check_hosts(x, vpm) <= 0) {
         if (!check_id_error(ctx, X509_V_ERR_HOSTNAME_MISMATCH))
             return 0;
     }
-    if (vpm->email && X509_check_email(x, vpm->email, vpm->emaillen, 0) <= 0) {
+    if (vpm->email != NULL
+            && X509_check_email(x, vpm->email, vpm->emaillen, 0) <= 0) {
         if (!check_id_error(ctx, X509_V_ERR_EMAIL_MISMATCH))
             return 0;
     }
-    if (vpm->ip && X509_check_ip(x, vpm->ip, vpm->iplen, 0) <= 0) {
+    if (vpm->ip != NULL && X509_check_ip(x, vpm->ip, vpm->iplen, 0) <= 0) {
         if (!check_id_error(ctx, X509_V_ERR_IP_ADDRESS_MISMATCH))
             return 0;
     }
@@ -850,7 +815,7 @@ static int check_trust(X509_STORE_CTX *ctx, int num_untrusted)
         i = 0;
         x = sk_X509_value(ctx->chain, i);
         mx = lookup_cert_match(ctx, x);
-        if (!mx)
+        if (mx == NULL)
             return X509_TRUST_UNTRUSTED;
 
         /*
@@ -864,7 +829,7 @@ static int check_trust(X509_STORE_CTX *ctx, int num_untrusted)
         }
 
         /* Replace leaf with trusted match */
-        (void) sk_X509_set(ctx->chain, 0, mx);
+        (void)sk_X509_set(ctx->chain, 0, mx);
         X509_free(x);
         ctx->num_untrusted = 0;
         goto trusted;
@@ -894,11 +859,12 @@ static int check_trust(X509_STORE_CTX *ctx, int num_untrusted)
 static int check_revocation(X509_STORE_CTX *ctx)
 {
     int i = 0, last = 0, ok = 0;
-    if (!(ctx->param->flags & X509_V_FLAG_CRL_CHECK))
+
+    if ((ctx->param->flags & X509_V_FLAG_CRL_CHECK) == 0)
         return 1;
-    if (ctx->param->flags & X509_V_FLAG_CRL_CHECK_ALL)
+    if ((ctx->param->flags & X509_V_FLAG_CRL_CHECK_ALL) != 0) {
         last = sk_X509_num(ctx->chain) - 1;
-    else {
+    } else {
         /* If checking CRL paths this isn't the EE certificate */
         if (ctx->parent)
             return 1;
@@ -925,14 +891,14 @@ static int check_cert(X509_STORE_CTX *ctx)
     ctx->current_crl_score = 0;
     ctx->current_reasons = 0;
 
-    if (x->ex_flags & EXFLAG_PROXY)
+    if ((x->ex_flags & EXFLAG_PROXY) != 0)
         return 1;
 
     while (ctx->current_reasons != CRLDP_ALL_REASONS) {
         unsigned int last_reasons = ctx->current_reasons;
 
         /* Try to retrieve relevant CRL */
-        if (ctx->get_crl)
+        if (ctx->get_crl != NULL)
             ok = ctx->get_crl(ctx, &crl, x);
         else
             ok = get_crl_delta(ctx, &crl, &dcrl, x);
@@ -946,15 +912,16 @@ static int check_cert(X509_STORE_CTX *ctx)
         if (!ok)
             goto done;
 
-        if (dcrl) {
+        if (dcrl != NULL) {
             ok = ctx->check_crl(ctx, dcrl);
             if (!ok)
                 goto done;
             ok = ctx->cert_crl(ctx, dcrl, x);
             if (!ok)
                 goto done;
-        } else
+        } else {
             ok = 1;
+        }
 
         /* Don't look in full CRL if delta reason is removefromCRL */
         if (ok != 2) {
@@ -992,9 +959,9 @@ static int check_crl_time(X509_STORE_CTX *ctx, X509_CRL *crl, int notify)
 
     if (notify)
         ctx->current_crl = crl;
-    if (ctx->param->flags & X509_V_FLAG_USE_CHECK_TIME)
+    if ((ctx->param->flags & X509_V_FLAG_USE_CHECK_TIME) != 0)
         ptime = &ctx->param->check_time;
-    else if (ctx->param->flags & X509_V_FLAG_NO_CHECK_TIME)
+    else if ((ctx->param->flags & X509_V_FLAG_NO_CHECK_TIME) != 0)
         return 1;
     else
         ptime = NULL;
@@ -1024,10 +991,8 @@ static int check_crl_time(X509_STORE_CTX *ctx, X509_CRL *crl, int notify)
                 return 0;
         }
         /* Ignore expiration of base CRL is delta is valid */
-        if ((i < 0) && !(ctx->current_crl_score & CRL_SCORE_TIME_DELTA)) {
-            if (!notify)
-                return 0;
-            if (!verify_cb_crl(ctx, X509_V_ERR_CRL_HAS_EXPIRED))
+        if (i < 0 && (ctx->current_crl_score & CRL_SCORE_TIME_DELTA) == 0) {
+            if (!notify || !verify_cb_crl(ctx, X509_V_ERR_CRL_HAS_EXPIRED))
                 return 0;
         }
     }
@@ -1057,6 +1022,7 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
         /* If current CRL is equivalent use it if it is newer */
         if (crl_score == best_score && best_crl != NULL) {
             int day, sec;
+
             if (ASN1_TIME_diff(&day, &sec, X509_CRL_get0_lastUpdate(best_crl),
                                X509_CRL_get0_lastUpdate(crl)) == 0)
                 continue;
@@ -1073,7 +1039,7 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
         best_reasons = reasons;
     }
 
-    if (best_crl) {
+    if (best_crl != NULL) {
         X509_CRL_free(*pcrl);
         *pcrl = best_crl;
         *pissuer = best_crl_issuer;
@@ -1097,50 +1063,44 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
  */
 static int crl_extension_match(X509_CRL *a, X509_CRL *b, int nid)
 {
-    ASN1_OCTET_STRING *exta, *extb;
-    int i;
-    i = X509_CRL_get_ext_by_NID(a, nid, -1);
+    ASN1_OCTET_STRING *exta = NULL, *extb = NULL;
+    int i = X509_CRL_get_ext_by_NID(a, nid, -1);
+
     if (i >= 0) {
         /* Can't have multiple occurrences */
         if (X509_CRL_get_ext_by_NID(a, nid, i) != -1)
             return 0;
         exta = X509_EXTENSION_get_data(X509_CRL_get_ext(a, i));
-    } else
-        exta = NULL;
+    }
 
     i = X509_CRL_get_ext_by_NID(b, nid, -1);
-
     if (i >= 0) {
-
         if (X509_CRL_get_ext_by_NID(b, nid, i) != -1)
             return 0;
         extb = X509_EXTENSION_get_data(X509_CRL_get_ext(b, i));
-    } else
-        extb = NULL;
+    }
 
-    if (!exta && !extb)
+    if (exta == NULL && extb == NULL)
         return 1;
 
-    if (!exta || !extb)
-        return 0;
-
-    if (ASN1_OCTET_STRING_cmp(exta, extb))
+    if (exta == NULL || extb == NULL)
         return 0;
 
-    return 1;
+    return ASN1_OCTET_STRING_cmp(exta, extb) == 0;
 }
 
 /* See if a base and delta are compatible */
 static int check_delta_base(X509_CRL *delta, X509_CRL *base)
 {
     /* Delta CRL must be a delta */
-    if (!delta->base_crl_number)
+    if (delta->base_crl_number == NULL)
         return 0;
     /* Base must have a CRL number */
-    if (!base->crl_number)
+    if (base->crl_number == NULL)
         return 0;
     /* Issuer names must match */
-    if (X509_NAME_cmp(X509_CRL_get_issuer(base), X509_CRL_get_issuer(delta)))
+    if (X509_NAME_cmp(X509_CRL_get_issuer(base),
+                      X509_CRL_get_issuer(delta)) != 0)
         return 0;
     /* AKID and IDP must match */
     if (!crl_extension_match(delta, base, NID_authority_key_identifier))
@@ -1151,9 +1111,7 @@ static int check_delta_base(X509_CRL *delta, X509_CRL *base)
     if (ASN1_INTEGER_cmp(delta->base_crl_number, base->crl_number) > 0)
         return 0;
     /* Delta CRL number must exceed full CRL number */
-    if (ASN1_INTEGER_cmp(delta->crl_number, base->crl_number) > 0)
-        return 1;
-    return 0;
+    return ASN1_INTEGER_cmp(delta->crl_number, base->crl_number) > 0;
 }
 
 /*
@@ -1165,9 +1123,10 @@ static void get_delta_sk(X509_STORE_CTX *ctx, X509_CRL **dcrl, int *pscore,
 {
     X509_CRL *delta;
     int i;
-    if (!(ctx->param->flags & X509_V_FLAG_USE_DELTAS))
+
+    if ((ctx->param->flags & X509_V_FLAG_USE_DELTAS) == 0)
         return;
-    if (!((ctx->current_cert->ex_flags | base->flags) & EXFLAG_FRESHEST))
+    if (((ctx->current_cert->ex_flags | base->flags) & EXFLAG_FRESHEST) == 0)
         return;
     for (i = 0; i < sk_X509_CRL_num(crls); i++) {
         delta = sk_X509_CRL_value(crls, i);
@@ -1192,35 +1151,35 @@ static void get_delta_sk(X509_STORE_CTX *ctx, X509_CRL **dcrl, int *pscore,
 static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer,
                          unsigned int *preasons, X509_CRL *crl, X509 *x)
 {
-
     int crl_score = 0;
     unsigned int tmp_reasons = *preasons, crl_reasons;
 
     /* First see if we can reject CRL straight away */
 
     /* Invalid IDP cannot be processed */
-    if (crl->idp_flags & IDP_INVALID)
+    if ((crl->idp_flags & IDP_INVALID) != 0)
         return 0;
     /* Reason codes or indirect CRLs need extended CRL support */
-    if (!(ctx->param->flags & X509_V_FLAG_EXTENDED_CRL_SUPPORT)) {
+    if ((ctx->param->flags & X509_V_FLAG_EXTENDED_CRL_SUPPORT) == 0) {
         if (crl->idp_flags & (IDP_INDIRECT | IDP_REASONS))
             return 0;
-    } else if (crl->idp_flags & IDP_REASONS) {
+    } else if ((crl->idp_flags & IDP_REASONS) != 0) {
         /* If no new reasons reject */
-        if (!(crl->idp_reasons & ~tmp_reasons))
+        if ((crl->idp_reasons & ~tmp_reasons) == 0)
             return 0;
     }
     /* Don't process deltas at this stage */
-    else if (crl->base_crl_number)
+    else if (crl->base_crl_number != NULL)
         return 0;
     /* If issuer name doesn't match certificate need indirect CRL */
-    if (X509_NAME_cmp(X509_get_issuer_name(x), X509_CRL_get_issuer(crl))) {
-        if (!(crl->idp_flags & IDP_INDIRECT))
+    if (X509_NAME_cmp(X509_get_issuer_name(x), X509_CRL_get_issuer(crl)) != 0) {
+        if ((crl->idp_flags & IDP_INDIRECT) == 0)
             return 0;
-    } else
+    } else {
         crl_score |= CRL_SCORE_ISSUER_NAME;
+    }
 
-    if (!(crl->flags & EXFLAG_CRITICAL))
+    if ((crl->flags & EXFLAG_CRITICAL) == 0)
         crl_score |= CRL_SCORE_NOCRITICAL;
 
     /* Check expiration */
@@ -1231,14 +1190,13 @@ static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer,
     crl_akid_check(ctx, crl, pissuer, &crl_score);
 
     /* If we can't locate certificate issuer at this point forget it */
-    if (!(crl_score & CRL_SCORE_AKID))
+    if ((crl_score & CRL_SCORE_AKID) == 0)
         return 0;
 
     /* Check cert for matching CRL distribution points */
-
     if (crl_crldp_check(x, crl, crl_score, &crl_reasons)) {
         /* If no new reasons reject */
-        if (!(crl_reasons & ~tmp_reasons))
+        if ((crl_reasons & ~tmp_reasons) == 0)
             return 0;
         tmp_reasons |= crl_reasons;
         crl_score |= CRL_SCORE_SCOPE;
@@ -1283,7 +1241,7 @@ static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl,
     }
 
     /* Anything else needs extended CRL support */
-    if (!(ctx->param->flags & X509_V_FLAG_EXTENDED_CRL_SUPPORT))
+    if ((ctx->param->flags & X509_V_FLAG_EXTENDED_CRL_SUPPORT) == 0)
         return;
 
     /*
@@ -1292,7 +1250,7 @@ static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl,
      */
     for (i = 0; i < sk_X509_num(ctx->untrusted); i++) {
         crl_issuer = sk_X509_value(ctx->untrusted, i);
-        if (X509_NAME_cmp(X509_get_subject_name(crl_issuer), cnm))
+        if (X509_NAME_cmp(X509_get_subject_name(crl_issuer), cnm) != 0)
             continue;
         if (X509_check_akid(crl_issuer, crl->akid) == X509_V_OK) {
             *pissuer = crl_issuer;
@@ -1314,7 +1272,7 @@ static int check_crl_path(X509_STORE_CTX *ctx, X509 *x)
     int ret;
 
     /* Don't allow recursive CRL path validation */
-    if (ctx->parent)
+    if (ctx->parent != NULL)
         return 0;
     if (!X509_STORE_CTX_init(&crl_ctx, ctx->store, x, ctx->untrusted))
         return -1;
@@ -1350,12 +1308,10 @@ static int check_crl_chain(X509_STORE_CTX *ctx,
                            STACK_OF(X509) *cert_path,
                            STACK_OF(X509) *crl_path)
 {
-    X509 *cert_ta, *crl_ta;
-    cert_ta = sk_X509_value(cert_path, sk_X509_num(cert_path) - 1);
-    crl_ta = sk_X509_value(crl_path, sk_X509_num(crl_path) - 1);
-    if (!X509_cmp(cert_ta, crl_ta))
-        return 1;
-    return 0;
+    X509 *cert_ta = sk_X509_value(cert_path, sk_X509_num(cert_path) - 1);
+    X509 *crl_ta = sk_X509_value(crl_path, sk_X509_num(crl_path) - 1);
+
+    return X509_cmp(cert_ta, crl_ta) == 0;
 }
 
 /*-
@@ -1371,25 +1327,23 @@ static int idp_check_dp(DIST_POINT_NAME *a, DIST_POINT_NAME *b)
     GENERAL_NAMES *gens = NULL;
     GENERAL_NAME *gena, *genb;
     int i, j;
-    if (!a || !b)
+
+    if (a == NULL || b == NULL)
         return 1;
     if (a->type == 1) {
-        if (!a->dpname)
+        if (a->dpname == NULL)
             return 0;
         /* Case 1: two X509_NAME */
         if (b->type == 1) {
-            if (!b->dpname)
-                return 0;
-            if (!X509_NAME_cmp(a->dpname, b->dpname))
-                return 1;
-            else
+            if (b->dpname == NULL)
                 return 0;
+            return X509_NAME_cmp(a->dpname, b->dpname) == 0;
         }
         /* Case 2: set name and GENERAL_NAMES appropriately */
         nm = a->dpname;
         gens = b->name.fullname;
     } else if (b->type == 1) {
-        if (!b->dpname)
+        if (b->dpname == NULL)
             return 0;
         /* Case 2: set name and GENERAL_NAMES appropriately */
         gens = a->name.fullname;
@@ -1397,12 +1351,12 @@ static int idp_check_dp(DIST_POINT_NAME *a, DIST_POINT_NAME *b)
     }
 
     /* Handle case 2 with one GENERAL_NAMES and one X509_NAME */
-    if (nm) {
+    if (nm != NULL) {
         for (i = 0; i < sk_GENERAL_NAME_num(gens); i++) {
             gena = sk_GENERAL_NAME_value(gens, i);
             if (gena->type != GEN_DIRNAME)
                 continue;
-            if (!X509_NAME_cmp(nm, gena->d.directoryName))
+            if (X509_NAME_cmp(nm, gena->d.directoryName) == 0)
                 return 1;
         }
         return 0;
@@ -1414,7 +1368,7 @@ static int idp_check_dp(DIST_POINT_NAME *a, DIST_POINT_NAME *b)
         gena = sk_GENERAL_NAME_value(a->name.fullname, i);
         for (j = 0; j < sk_GENERAL_NAME_num(b->name.fullname); j++) {
             genb = sk_GENERAL_NAME_value(b->name.fullname, j);
-            if (!GENERAL_NAME_cmp(gena, genb))
+            if (GENERAL_NAME_cmp(gena, genb) == 0)
                 return 1;
         }
     }
@@ -1427,14 +1381,16 @@ static int crldp_check_crlissuer(DIST_POINT *dp, X509_CRL *crl, int crl_score)
 {
     int i;
     const X509_NAME *nm = X509_CRL_get_issuer(crl);
+
     /* If no CRLissuer return is successful iff don't need a match */
-    if (!dp->CRLissuer)
-        return ! !(crl_score & CRL_SCORE_ISSUER_NAME);
+    if (dp->CRLissuer == NULL)
+        return (crl_score & CRL_SCORE_ISSUER_NAME) != 0;
     for (i = 0; i < sk_GENERAL_NAME_num(dp->CRLissuer); i++) {
         GENERAL_NAME *gen = sk_GENERAL_NAME_value(dp->CRLissuer, i);
+
         if (gen->type != GEN_DIRNAME)
             continue;
-        if (!X509_NAME_cmp(gen->d.directoryName, nm))
+        if (X509_NAME_cmp(gen->d.directoryName, nm) == 0)
             return 1;
     }
     return 0;
@@ -1445,29 +1401,30 @@ static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
                            unsigned int *preasons)
 {
     int i;
-    if (crl->idp_flags & IDP_ONLYATTR)
+
+    if ((crl->idp_flags & IDP_ONLYATTR) != 0)
         return 0;
-    if (x->ex_flags & EXFLAG_CA) {
-        if (crl->idp_flags & IDP_ONLYUSER)
+    if ((x->ex_flags & EXFLAG_CA) != 0) {
+        if ((crl->idp_flags & IDP_ONLYUSER) != 0)
             return 0;
     } else {
-        if (crl->idp_flags & IDP_ONLYCA)
+        if ((crl->idp_flags & IDP_ONLYCA) != 0)
             return 0;
     }
     *preasons = crl->idp_reasons;
     for (i = 0; i < sk_DIST_POINT_num(x->crldp); i++) {
         DIST_POINT *dp = sk_DIST_POINT_value(x->crldp, i);
+
         if (crldp_check_crlissuer(dp, crl, crl_score)) {
-            if (!crl->idp || idp_check_dp(dp->distpoint, crl->idp->distpoint)) {
+            if (crl->idp == NULL
+                    || idp_check_dp(dp->distpoint, crl->idp->distpoint)) {
                 *preasons &= dp->dp_reasons;
                 return 1;
             }
         }
     }
-    if ((!crl->idp || !crl->idp->distpoint)
-        && (crl_score & CRL_SCORE_ISSUER_NAME))
-        return 1;
-    return 0;
+    return (crl->idp == NULL || crl->idp->distpoint == NULL)
+            && (crl_score & CRL_SCORE_ISSUER_NAME) != 0;
 }
 
 /*
@@ -1495,7 +1452,7 @@ static int get_crl_delta(X509_STORE_CTX *ctx,
     skcrl = ctx->lookup_crls(ctx, nm);
 
     /* If no CRLs found and a near match from get_crl_sk use that */
-    if (!skcrl && crl)
+    if (skcrl == NULL && crl != NULL)
         goto done;
 
     get_crl_sk(ctx, &crl, &dcrl, &issuer, &crl_score, &reasons, skcrl);
@@ -1504,7 +1461,7 @@ static int get_crl_delta(X509_STORE_CTX *ctx,
 
  done:
     /* If we got any kind of CRL use it and return success */
-    if (crl) {
+    if (crl != NULL) {
         ctx->current_issuer = issuer;
         ctx->current_crl_score = crl_score;
         ctx->current_reasons = reasons;
@@ -1524,15 +1481,15 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl)
     int chnum = sk_X509_num(ctx->chain) - 1;
 
     /* If we have an alternative CRL issuer cert use that */
-    if (ctx->current_issuer)
+    if (ctx->current_issuer != NULL) {
         issuer = ctx->current_issuer;
     /*
      * Else find CRL issuer: if not last certificate then issuer is next
      * certificate in chain.
      */
-    else if (cnum < chnum)
+    } else if (cnum < chnum) {
         issuer = sk_X509_value(ctx->chain, cnum + 1);
-    else {
+    } else {
         issuer = sk_X509_value(ctx->chain, chnum);
         /* If not self-issued, can't check signature */
         if (!ctx->check_issued(ctx, issuer, issuer) &&
@@ -1546,39 +1503,38 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl)
     /*
      * Skip most tests for deltas because they have already been done
      */
-    if (!crl->base_crl_number) {
+    if (crl->base_crl_number == NULL) {
         /* Check for cRLSign bit if keyUsage present */
-        if ((issuer->ex_flags & EXFLAG_KUSAGE) &&
-            !(issuer->ex_kusage & KU_CRL_SIGN) &&
+        if ((issuer->ex_flags & EXFLAG_KUSAGE) != 0 &&
+            (issuer->ex_kusage & KU_CRL_SIGN) == 0 &&
             !verify_cb_crl(ctx, X509_V_ERR_KEYUSAGE_NO_CRL_SIGN))
             return 0;
 
-        if (!(ctx->current_crl_score & CRL_SCORE_SCOPE) &&
+        if ((ctx->current_crl_score & CRL_SCORE_SCOPE) == 0 &&
             !verify_cb_crl(ctx, X509_V_ERR_DIFFERENT_CRL_SCOPE))
             return 0;
 
-        if (!(ctx->current_crl_score & CRL_SCORE_SAME_PATH) &&
+        if ((ctx->current_crl_score & CRL_SCORE_SAME_PATH) == 0 &&
             check_crl_path(ctx, ctx->current_issuer) <= 0 &&
             !verify_cb_crl(ctx, X509_V_ERR_CRL_PATH_VALIDATION_ERROR))
             return 0;
 
-        if ((crl->idp_flags & IDP_INVALID) &&
+        if ((crl->idp_flags & IDP_INVALID) != 0 &&
             !verify_cb_crl(ctx, X509_V_ERR_INVALID_EXTENSION))
             return 0;
     }
 
-    if (!(ctx->current_crl_score & CRL_SCORE_TIME) &&
+    if ((ctx->current_crl_score & CRL_SCORE_TIME) == 0 &&
         !check_crl_time(ctx, crl, 1))
         return 0;
 
     /* Attempt to get issuer certificate public key */
     ikey = X509_get0_pubkey(issuer);
-
-    if (!ikey &&
+    if (ikey == NULL &&
         !verify_cb_crl(ctx, X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY))
         return 0;
 
-    if (ikey) {
+    if (ikey != NULL) {
         int rv = X509_CRL_check_suiteb(crl, ikey, ctx->param->flags);
 
         if (rv != X509_V_OK && !verify_cb_crl(ctx, rv))
@@ -1602,8 +1558,8 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x)
      * was revoked. This has since been changed since critical extensions can
      * change the meaning of CRL entries.
      */
-    if (!(ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL)
-        && (crl->flags & EXFLAG_CRITICAL) &&
+    if ((ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL) == 0
+        && (crl->flags & EXFLAG_CRITICAL) != 0 &&
         !verify_cb_crl(ctx, X509_V_ERR_UNHANDLED_CRITICAL_CRL_EXTENSION))
         return 0;
     /*
@@ -1675,7 +1631,7 @@ static int check_policy(X509_STORE_CTX *ctx)
         return 0;
     }
 
-    if (ctx->param->flags & X509_V_FLAG_NOTIFY_POLICY) {
+    if ((ctx->param->flags & X509_V_FLAG_NOTIFY_POLICY) != 0) {
         ctx->current_cert = NULL;
         /*
          * Verification errors need to be "sticky", a callback may have allowed
@@ -1702,9 +1658,9 @@ int x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth)
     time_t *ptime;
     int i;
 
-    if (ctx->param->flags & X509_V_FLAG_USE_CHECK_TIME)
+    if ((ctx->param->flags & X509_V_FLAG_USE_CHECK_TIME) != 0)
         ptime = &ctx->param->check_time;
-    else if (ctx->param->flags & X509_V_FLAG_NO_CHECK_TIME)
+    else if ((ctx->param->flags & X509_V_FLAG_NO_CHECK_TIME) != 0)
         return 1;
     else
         ptime = NULL;
@@ -1728,55 +1684,54 @@ static int internal_verify(X509_STORE_CTX *ctx)
 {
     int n = sk_X509_num(ctx->chain) - 1;
     X509 *xi = sk_X509_value(ctx->chain, n);
-    X509 *xs;
+    X509 *xs = xi;
 
-    /*
-     * With DANE-verified bare public key TA signatures, it remains only to
-     * check the timestamps of the top certificate.  We report the issuer as
-     * NULL, since all we have is a bare key.
-     */
+    ctx->error_depth = n;
     if (ctx->bare_ta_signed) {
-        xs = xi;
+        /*
+         * With DANE-verified bare public key TA signatures,
+         * on the top certificate we check only the timestamps.
+         * We report the issuer as NULL because all we have is a bare key.
+         */
         xi = NULL;
-        goto check_cert_time;
-    }
-
-    if (ctx->check_issued(ctx, xi, xi))
-        xs = xi; /* The typical case: last cert in the chain is self-issued */
-    else {
-        if (ctx->param->flags & X509_V_FLAG_PARTIAL_CHAIN) {
-            xs = xi;
-            goto check_cert_time;
-        }
-        if (n <= 0) {
-            CB_FAIL_IF(1, ctx, xi, 0, X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE);
-
-            xs = xi;
-            goto check_cert_time;
+    } else if (!ctx->check_issued(ctx, xi, xi)
+               /* exceptional case: last cert in the chain is not self-issued */
+               && ((ctx->param->flags & X509_V_FLAG_PARTIAL_CHAIN) == 0)) {
+        if (n > 0) {
+            n--;
+            ctx->error_depth = n;
+            xs = sk_X509_value(ctx->chain, n);
+        } else {
+            CB_FAIL_IF(1, ctx, xi, 0,
+                       X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE);
         }
-
-        n--;
-        ctx->error_depth = n;
-        xs = sk_X509_value(ctx->chain, n);
+        /*
+         * The below code will certainly not do a
+         * self-signature check on xi because it is not self-issued.
+         */
     }
 
     /*
-     * Do not clear ctx->error=0, it must be "sticky", only the user's callback
-     * is allowed to reset errors (at its own peril).
+     * Do not clear ctx->error = 0, it must be "sticky",
+     * only the user's callback is allowed to reset errors (at its own peril).
      */
     while (n >= 0) {
         /*-
          * For each iteration of this loop:
          * n is the subject depth
          * xs is the subject cert, for which the signature is to be checked
-         * xi is the supposed issuer cert containing the public key to use
+         * xi is NULL for DANE-verified bare public key TA signatures
+         *       else the supposed issuer cert containing the public key to use
          * Initially xs == xi if the last cert in the chain is self-issued.
-         *
-         * Skip signature check for self-signed certificates unless explicitly
+         */
+        /*
+         * Do signature check for self-signed certificates only if explicitly
          * asked for because it does not add any security and just wastes time.
          */
-        if (xs != xi || ((ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE)
-                         && (xi->ex_flags & EXFLAG_SS) != 0)) {
+        if (xi != NULL
+            && (xs != xi
+                || ((ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE)
+                    && (xi->ex_flags & EXFLAG_SS) != 0))) {
             EVP_PKEY *pkey;
             /*
              * If the issuer's public key is not available or its key usage
@@ -1810,7 +1765,7 @@ static int internal_verify(X509_STORE_CTX *ctx)
             }
         }
 
-    check_cert_time: /* in addition to RFC 5280, do also for trusted (root) cert */
+        /* in addition to RFC 5280, do also for trusted (root) cert */
         /* Calls verify callback as needed */
         if (!x509_check_cert_time(ctx, xs, n))
             return 0;
@@ -1849,6 +1804,7 @@ int X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)
 #else
     const char upper_z = 'Z';
 #endif
+
     /*-
      * Note that ASN.1 allows much more slack in the time format than RFC5280.
      * In RFC5280, the representation is fixed:
@@ -1893,7 +1849,7 @@ int X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)
     asn1_cmp_time = X509_time_adj(NULL, 0, cmp_time);
     if (asn1_cmp_time == NULL)
         goto err;
-    if (!ASN1_TIME_diff(&day, &sec, ctm, asn1_cmp_time))
+    if (ASN1_TIME_diff(&day, &sec, ctm, asn1_cmp_time) == 0)
         goto err;
 
     /*
@@ -1952,7 +1908,7 @@ ASN1_TIME *X509_time_adj_ex(ASN1_TIME *s,
     else
         time(&t);
 
-    if (s && !(s->flags & ASN1_STRING_FLAG_MSTRING)) {
+    if (s != NULL && (s->flags & ASN1_STRING_FLAG_MSTRING) == 0) {
         if (s->type == V_ASN1_UTCTIME)
             return ASN1_UTCTIME_adj(s, t, offset_day, offset_sec);
         if (s->type == V_ASN1_GENERALIZEDTIME)
@@ -2000,19 +1956,21 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
 {
     X509_CRL *crl = NULL;
     int i;
+
     STACK_OF(X509_REVOKED) *revs = NULL;
     /* CRLs can't be delta already */
-    if (base->base_crl_number || newer->base_crl_number) {
+    if (base->base_crl_number != NULL || newer->base_crl_number != NULL) {
         ERR_raise(ERR_LIB_X509, X509_R_CRL_ALREADY_DELTA);
         return NULL;
     }
     /* Base and new CRL must have a CRL number */
-    if (!base->crl_number || !newer->crl_number) {
+    if (base->crl_number == NULL || newer->crl_number == NULL) {
         ERR_raise(ERR_LIB_X509, X509_R_NO_CRL_NUMBER);
         return NULL;
     }
     /* Issuer names must match */
-    if (X509_NAME_cmp(X509_CRL_get_issuer(base), X509_CRL_get_issuer(newer))) {
+    if (X509_NAME_cmp(X509_CRL_get_issuer(base),
+                      X509_CRL_get_issuer(newer)) != 0) {
         ERR_raise(ERR_LIB_X509, X509_R_ISSUER_MISMATCH);
         return NULL;
     }
@@ -2031,8 +1989,8 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
         return NULL;
     }
     /* CRLs must verify */
-    if (skey && (X509_CRL_verify(base, skey) <= 0 ||
-                 X509_CRL_verify(newer, skey) <= 0)) {
+    if (skey != NULL && (X509_CRL_verify(base, skey) <= 0 ||
+                         X509_CRL_verify(newer, skey) <= 0)) {
         ERR_raise(ERR_LIB_X509, X509_R_CRL_VERIFY_FAILURE);
         return NULL;
     }
@@ -2058,8 +2016,8 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
      * number to correct value too.
      */
     for (i = 0; i < X509_CRL_get_ext_count(newer); i++) {
-        X509_EXTENSION *ext;
-        ext = X509_CRL_get_ext(newer, i);
+        X509_EXTENSION *ext = X509_CRL_get_ext(newer, i);
+
         if (!X509_CRL_add_ext(crl, ext, -1))
             goto memerr;
     }
@@ -2069,6 +2027,7 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
 
     for (i = 0; i < sk_X509_REVOKED_num(revs); i++) {
         X509_REVOKED *rvn, *rvtmp;
+
         rvn = sk_X509_REVOKED_value(revs, i);
         /*
          * Add only if not also in base. TODO: need something cleverer here
@@ -2076,7 +2035,7 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
          */
         if (!X509_CRL_get0_by_serial(base, &rvtmp, &rvn->serialNumber)) {
             rvtmp = X509_REVOKED_dup(rvn);
-            if (!rvtmp)
+            if (rvtmp == NULL)
                 goto memerr;
             if (!X509_CRL_add0_revoked(crl, rvtmp)) {
                 X509_REVOKED_free(rvtmp);
@@ -2086,7 +2045,7 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
     }
     /* TODO: optionally prune deleted entries */
 
-    if (skey && md && !X509_CRL_sign(crl, skey, md))
+    if (skey != NULL && md != NULL && !X509_CRL_sign(crl, skey, md))
         goto memerr;
 
     return crl;
@@ -2144,7 +2103,7 @@ STACK_OF(X509) *X509_STORE_CTX_get0_chain(const X509_STORE_CTX *ctx)
 
 STACK_OF(X509) *X509_STORE_CTX_get1_chain(const X509_STORE_CTX *ctx)
 {
-    if (!ctx->chain)
+    if (ctx->chain == NULL)
         return NULL;
     return X509_chain_up_ref(ctx->chain);
 }
@@ -2208,12 +2167,14 @@ int X509_STORE_CTX_purpose_inherit(X509_STORE_CTX *ctx, int def_purpose,
                                    int purpose, int trust)
 {
     int idx;
+
     /* If purpose not set use default */
     if (purpose == 0)
         purpose = def_purpose;
     /* If we have a purpose then check it is valid */
     if (purpose != 0) {
         X509_PURPOSE *ptmp;
+
         idx = X509_PURPOSE_get_by_id(purpose);
         if (idx == -1) {
             ERR_raise(ERR_LIB_X509, X509_R_UNKNOWN_PURPOSE_ID);
@@ -2234,10 +2195,10 @@ int X509_STORE_CTX_purpose_inherit(X509_STORE_CTX *ctx, int def_purpose,
             ptmp = X509_PURPOSE_get0(idx);
         }
         /* If trust not set then get from purpose default */
-        if (!trust)
+        if (trust == 0)
             trust = ptmp->trust;
     }
-    if (trust) {
+    if (trust != 0) {
         idx = X509_TRUST_get_by_id(trust);
         if (idx == -1) {
             ERR_raise(ERR_LIB_X509, X509_R_UNKNOWN_TRUST_ID);
@@ -2245,9 +2206,9 @@ int X509_STORE_CTX_purpose_inherit(X509_STORE_CTX *ctx, int def_purpose,
         }
     }
 
-    if (purpose && !ctx->param->purpose)
+    if (ctx->param->purpose == 0 && purpose != 0)
         ctx->param->purpose = purpose;
-    if (trust && !ctx->param->trust)
+    if (ctx->param->trust == 0 && trust != 0)
         ctx->param->trust = trust;
     return 1;
 }
@@ -2279,7 +2240,6 @@ X509_STORE_CTX *X509_STORE_CTX_new(void)
     return X509_STORE_CTX_new_ex(NULL, NULL);
 }
 
-
 void X509_STORE_CTX_free(X509_STORE_CTX *ctx)
 {
     if (ctx == NULL)
@@ -2289,7 +2249,6 @@ void X509_STORE_CTX_free(X509_STORE_CTX *ctx)
 
     /* libctx and propq survive X509_STORE_CTX_cleanup() */
     OPENSSL_free(ctx->propq);
-
     OPENSSL_free(ctx);
 }
 
@@ -2322,62 +2281,62 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
     memset(&ctx->ex_data, 0, sizeof(ctx->ex_data));
 
     /* store->cleanup is always 0 in OpenSSL, if set must be idempotent */
-    if (store)
+    if (store != NULL)
         ctx->cleanup = store->cleanup;
     else
         ctx->cleanup = 0;
 
-    if (store && store->check_issued)
+    if (store != NULL && store->check_issued != NULL)
         ctx->check_issued = store->check_issued;
     else
         ctx->check_issued = check_issued;
 
-    if (store && store->get_issuer)
+    if (store != NULL && store->get_issuer != NULL)
         ctx->get_issuer = store->get_issuer;
     else
         ctx->get_issuer = X509_STORE_CTX_get1_issuer;
 
-    if (store && store->verify_cb)
+    if (store != NULL && store->verify_cb != NULL)
         ctx->verify_cb = store->verify_cb;
     else
         ctx->verify_cb = null_callback;
 
-    if (store && store->verify)
+    if (store != NULL && store->verify != NULL)
         ctx->verify = store->verify;
     else
         ctx->verify = internal_verify;
 
-    if (store && store->check_revocation)
+    if (store != NULL && store->check_revocation != NULL)
         ctx->check_revocation = store->check_revocation;
     else
         ctx->check_revocation = check_revocation;
 
-    if (store && store->get_crl)
+    if (store != NULL && store->get_crl != NULL)
         ctx->get_crl = store->get_crl;
     else
         ctx->get_crl = NULL;
 
-    if (store && store->check_crl)
+    if (store != NULL && store->check_crl != NULL)
         ctx->check_crl = store->check_crl;
     else
         ctx->check_crl = check_crl;
 
-    if (store && store->cert_crl)
+    if (store != NULL && store->cert_crl != NULL)
         ctx->cert_crl = store->cert_crl;
     else
         ctx->cert_crl = cert_crl;
 
-    if (store && store->check_policy)
+    if (store != NULL && store->check_policy != NULL)
         ctx->check_policy = store->check_policy;
     else
         ctx->check_policy = check_policy;
 
-    if (store && store->lookup_certs)
+    if (store != NULL && store->lookup_certs != NULL)
         ctx->lookup_certs = store->lookup_certs;
     else
         ctx->lookup_certs = X509_STORE_CTX_get1_certs;
 
-    if (store && store->lookup_crls)
+    if (store != NULL && store->lookup_crls != NULL)
         ctx->lookup_crls = store->lookup_crls;
     else
         ctx->lookup_crls = X509_STORE_CTX_get1_crls;
@@ -2389,7 +2348,7 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
     }
 
     /* Inherit callbacks and flags from X509_STORE if not set use defaults. */
-    if (store)
+    if (store != NULL)
         ret = X509_VERIFY_PARAM_inherit(ctx->param, store->param);
     else
         ctx->param->inh_flags |= X509_VP_FLAG_DEFAULT | X509_VP_FLAG_ONCE;
@@ -2525,19 +2484,20 @@ X509_STORE_CTX_verify_fn X509_STORE_CTX_get_verify(const X509_STORE_CTX *ctx)
     return ctx->verify;
 }
 
-X509_STORE_CTX_get_issuer_fn X509_STORE_CTX_get_get_issuer(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_get_issuer_fn
+X509_STORE_CTX_get_get_issuer(const X509_STORE_CTX *ctx)
 {
     return ctx->get_issuer;
 }
 
 X509_STORE_CTX_check_issued_fn
-   X509_STORE_CTX_get_check_issued(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_get_check_issued(const X509_STORE_CTX *ctx)
 {
     return ctx->check_issued;
 }
 
 X509_STORE_CTX_check_revocation_fn
-    X509_STORE_CTX_get_check_revocation(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_get_check_revocation(const X509_STORE_CTX *ctx)
 {
     return ctx->check_revocation;
 }
@@ -2547,30 +2507,32 @@ X509_STORE_CTX_get_crl_fn X509_STORE_CTX_get_get_crl(const X509_STORE_CTX *ctx)
     return ctx->get_crl;
 }
 
-X509_STORE_CTX_check_crl_fn X509_STORE_CTX_get_check_crl(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_check_crl_fn
+X509_STORE_CTX_get_check_crl(const X509_STORE_CTX *ctx)
 {
     return ctx->check_crl;
 }
 
-X509_STORE_CTX_cert_crl_fn X509_STORE_CTX_get_cert_crl(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_cert_crl_fn
+X509_STORE_CTX_get_cert_crl(const X509_STORE_CTX *ctx)
 {
     return ctx->cert_crl;
 }
 
 X509_STORE_CTX_check_policy_fn
-    X509_STORE_CTX_get_check_policy(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_get_check_policy(const X509_STORE_CTX *ctx)
 {
     return ctx->check_policy;
 }
 
 X509_STORE_CTX_lookup_certs_fn
-    X509_STORE_CTX_get_lookup_certs(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_get_lookup_certs(const X509_STORE_CTX *ctx)
 {
     return ctx->lookup_certs;
 }
 
 X509_STORE_CTX_lookup_crls_fn
-    X509_STORE_CTX_get_lookup_crls(const X509_STORE_CTX *ctx)
+X509_STORE_CTX_get_lookup_crls(const X509_STORE_CTX *ctx)
 {
     return ctx->lookup_crls;
 }
@@ -2621,10 +2583,8 @@ void X509_STORE_CTX_set0_dane(X509_STORE_CTX *ctx, SSL_DANE *dane)
     ctx->dane = dane;
 }
 
-static unsigned char *dane_i2d(
-    X509 *cert,
-    uint8_t selector,
-    unsigned int *i2dlen)
+static unsigned char *dane_i2d(X509 *cert, uint8_t selector,
+                               unsigned int *i2dlen)
 {
     unsigned char *buf = NULL;
     int len;
@@ -2653,7 +2613,7 @@ static unsigned char *dane_i2d(
     return buf;
 }
 
-#define DANETLS_NONE 256        /* impossible uint8_t */
+#define DANETLS_NONE 256 /* impossible uint8_t */
 
 static int dane_match(X509_STORE_CTX *ctx, X509 *cert, int depth)
 {
@@ -2715,7 +2675,7 @@ static int dane_match(X509_STORE_CTX *ctx, X509 *cert, int depth)
      * exhausting all DANE-?? records, we've matched a PKIX-?? record, which is
      * sufficient for DANE, and what remains to do is ordinary PKIX validation.
      */
-    recnum = (dane->umask & mask) ? sk_danetls_record_num(dane->trecs) : 0;
+    recnum = (dane->umask & mask) != 0 ? sk_danetls_record_num(dane->trecs) : 0;
     for (i = 0; matched == 0 && i < recnum; ++i) {
         t = sk_danetls_record_value(dane->trecs, i);
         if ((DANETLS_USAGE_BIT(t->usage) & mask) == 0)
@@ -2759,6 +2719,7 @@ static int dane_match(X509_STORE_CTX *ctx, X509 *cert, int depth)
          */
         if (t->mtype != mtype) {
             const EVP_MD *md = dane->dctx->mdevp[mtype = t->mtype];
+
             cmpbuf = i2dbuf;
             cmplen = i2dlen;
 
@@ -2803,7 +2764,7 @@ static int check_dane_issuer(X509_STORE_CTX *ctx, int depth)
     X509 *cert;
 
     if (!DANETLS_HAS_TA(dane) || depth == 0)
-        return  X509_TRUST_UNTRUSTED;
+        return X509_TRUST_UNTRUSTED;
 
     /*
      * Record any DANE trust anchor matches, for the first depth to test, if
@@ -2815,10 +2776,10 @@ static int check_dane_issuer(X509_STORE_CTX *ctx, int depth)
         return  X509_TRUST_REJECTED;
     if (matched > 0) {
         ctx->num_untrusted = depth - 1;
-        return  X509_TRUST_TRUSTED;
+        return X509_TRUST_TRUSTED;
     }
 
-    return  X509_TRUST_UNTRUSTED;
+    return X509_TRUST_UNTRUSTED;
 }
 
 static int check_dane_pkeys(X509_STORE_CTX *ctx)
@@ -2955,9 +2916,9 @@ static int build_chain(X509_STORE_CTX *ctx)
 {
     SSL_DANE *dane = ctx->dane;
     int num = sk_X509_num(ctx->chain);
-    X509 *cert = sk_X509_value(ctx->chain, num - 1);
-    int self_signed;
-    STACK_OF(X509) *sktmp = NULL;
+    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;
@@ -2968,21 +2929,14 @@ static int build_chain(X509_STORE_CTX *ctx)
     int i;
 
     /* Our chain starts with a single untrusted element. */
-    if (!ossl_assert(num == 1 && ctx->num_untrusted == num))  {
-        ERR_raise(ERR_LIB_X509, ERR_R_INTERNAL_ERROR);
-        ctx->error = X509_V_ERR_UNSPECIFIED;
-        return 0;
-    }
-
-    self_signed = X509_self_signed(cert, 0);
-    if (self_signed < 0) {
-        ctx->error = X509_V_ERR_UNSPECIFIED;
-        return 0;
-    }
-
-#define S_DOUNTRUSTED      (1 << 0)     /* Search untrusted chain */
-#define S_DOTRUSTED        (1 << 1)     /* Search trusted store */
-#define S_DOALTERNATE      (1 << 2)     /* Retry with pruned alternate chain */
+    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 */
+#define S_DOALTERNATE (1 << 2) /* Retry with pruned alternate chain */
     /*
      * Set up search policy, untrusted if possible, trusted-first if enabled.
      * If we're doing DANE and not doing PKIX-TA/PKIX-EE, we never look in the
@@ -2992,7 +2946,7 @@ static int build_chain(X509_STORE_CTX *ctx)
      */
     search = (ctx->untrusted != NULL) ? S_DOUNTRUSTED : 0;
     if (DANETLS_HAS_PKIX(dane) || !DANETLS_HAS_DANE(dane)) {
-        if (search == 0 || ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST)
+        if (search == 0 || (ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) != 0)
             search |= S_DOTRUSTED;
         else if (!(ctx->param->flags & X509_V_FLAG_NO_ALT_CHAINS))
             may_alternate = 1;
@@ -3004,7 +2958,7 @@ static int build_chain(X509_STORE_CTX *ctx)
      * typically the content of the peer's certificate message) so can make
      * multiple passes over it, while free to remove elements as we go.
      */
-    if ((sktmp = sk_X509_dup(ctx->untrusted)) == NULL) {
+    if ((sk_untrusted = sk_X509_dup(ctx->untrusted)) == NULL) {
         ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
         ctx->error = X509_V_ERR_OUT_OF_MEM;
         return 0;
@@ -3021,13 +2975,13 @@ static int build_chain(X509_STORE_CTX *ctx)
      * this to change. ]
      */
     if (DANETLS_ENABLED(dane) && dane->certs != NULL) {
-        if (sktmp == NULL && (sktmp = sk_X509_new_null()) == NULL) {
+        if (sk_untrusted == NULL && (sk_untrusted = sk_X509_new_null()) == NULL) {
             ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
             ctx->error = X509_V_ERR_OUT_OF_MEM;
             return 0;
         }
-        if (!X509_add_certs(sktmp, dane->certs, X509_ADD_FLAG_DEFAULT)) {
-            sk_X509_free(sktmp);
+        if (!X509_add_certs(sk_untrusted, dane->certs, X509_ADD_FLAG_DEFAULT)) {
+            sk_X509_free(sk_untrusted);
             ctx->error = X509_V_ERR_OUT_OF_MEM;
             return 0;
         }
@@ -3037,8 +2991,8 @@ static int build_chain(X509_STORE_CTX *ctx)
      * Still absurdly large, but arithmetically safe, a lower hard upper bound
      * might be reasonable.
      */
-    if (ctx->param->depth > INT_MAX/2)
-        ctx->param->depth = INT_MAX/2;
+    if (ctx->param->depth > INT_MAX / 2)
+        ctx->param->depth = INT_MAX / 2;
 
     /*
      * Try to extend the chain until we reach an ultimately trusted issuer.
@@ -3048,8 +3002,7 @@ static int build_chain(X509_STORE_CTX *ctx)
     depth = ctx->param->depth + 1;
 
     while (search != 0) {
-        X509 *x;
-        X509 *xtmp = NULL;
+        X509 *issuer = NULL;
 
         /*
          * Look in the trust store if enabled for first lookup, or we've run
@@ -3085,15 +3038,14 @@ static int build_chain(X509_STORE_CTX *ctx)
                  */
                 i = alt_untrusted;
             }
-            x = sk_X509_value(ctx->chain, i-1);
+            curr = sk_X509_value(ctx->chain, i - 1);
 
-            ok = (depth < num) ? 0 : get_issuer(&xtmp, ctx, x);
+            ok = depth < num ? 0 : get_issuer(&issuer, ctx, curr);
 
             if (ok < 0) {
                 trust = X509_TRUST_REJECTED;
                 ctx->error = X509_V_ERR_STORE_LOOKUP;
-                search = 0;
-                continue;
+                break;
             }
 
             if (ok > 0) {
@@ -3114,11 +3066,10 @@ static int build_chain(X509_STORE_CTX *ctx)
                 if ((search & S_DOALTERNATE) != 0) {
                     if (!ossl_assert(num > i && i > 0 && !self_signed)) {
                         ERR_raise(ERR_LIB_X509, ERR_R_INTERNAL_ERROR);
-                        X509_free(xtmp);
+                        X509_free(issuer);
                         trust = X509_TRUST_REJECTED;
                         ctx->error = X509_V_ERR_UNSPECIFIED;
-                        search = 0;
-                        continue;
+                        break;
                     }
                     search &= ~S_DOALTERNATE;
                     for (; num > i; --num)
@@ -3141,19 +3092,15 @@ static int build_chain(X509_STORE_CTX *ctx)
                  * trusted matching issuer.  Otherwise, grow the chain.
                  */
                 if (!self_signed) {
-                    if (!sk_X509_push(ctx->chain, x = xtmp)) {
-                        X509_free(xtmp);
+                    curr = issuer;
+                    if ((self_signed = X509_self_signed(curr, 0)) < 0)
+                        goto int_err;
+                    if (!sk_X509_push(ctx->chain, curr)) {
+                        X509_free(issuer);
                         ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
                         trust = X509_TRUST_REJECTED;
                         ctx->error = X509_V_ERR_OUT_OF_MEM;
-                        search = 0;
-                        continue;
-                    }
-                    self_signed = X509_self_signed(x, 0);
-                    if (self_signed < 0) {
-                        sk_X509_free(sktmp);
-                        ctx->error = X509_V_ERR_UNSPECIFIED;
-                        return 0;
+                        break;
                     }
                 } else if (num == ctx->num_untrusted) {
                     /*
@@ -3162,14 +3109,16 @@ static int build_chain(X509_STORE_CTX *ctx)
                      * a trust anchor.  We must have an exact match to avoid
                      * possible impersonation via key substitution etc.
                      */
-                    if (X509_cmp(x, xtmp) != 0) {
+                    if (X509_cmp(curr, issuer) != 0) {
                         /* Self-signed untrusted mimic. */
-                        X509_free(xtmp);
+                        X509_free(issuer);
                         ok = 0;
-                    } else {
-                        X509_free(x);
+                    } else { /* curr "==" issuer */
+                        X509_free(curr);
                         ctx->num_untrusted = --num;
-                        (void) sk_X509_set(ctx->chain, num, x = xtmp);
+                        (void)sk_X509_set(ctx->chain, num, issuer);
+                        curr = issuer;
+                        /* no need to update self_signed */
                     }
                 }
 
@@ -3187,20 +3136,13 @@ static int build_chain(X509_STORE_CTX *ctx)
                  * certificate with ctx->num_untrusted <= num.
                  */
                 if (ok) {
-                    if (!ossl_assert(ctx->num_untrusted <= num)) {
-                        ERR_raise(ERR_LIB_X509, ERR_R_INTERNAL_ERROR);
-                        trust = X509_TRUST_REJECTED;
-                        ctx->error = X509_V_ERR_UNSPECIFIED;
-                        search = 0;
-                        continue;
-                    }
+                    if (!ossl_assert(ctx->num_untrusted <= num))
+                        goto int_err;
                     search &= ~S_DOUNTRUSTED;
-                    switch (trust = check_trust(ctx, num)) {
-                    case X509_TRUST_TRUSTED:
-                    case X509_TRUST_REJECTED:
-                        search = 0;
-                        continue;
-                    }
+                    trust = check_trust(ctx, num);
+                    if (trust == X509_TRUST_TRUSTED
+                            || trust == X509_TRUST_REJECTED)
+                        break;
                     if (!self_signed)
                         continue;
                 }
@@ -3228,26 +3170,21 @@ static int build_chain(X509_STORE_CTX *ctx)
         }
 
         /*
-         * Extend chain with peer-provided certificates
+         * Extend chain with peer-provided untrusted certificates
          */
         if ((search & S_DOUNTRUSTED) != 0) {
             num = sk_X509_num(ctx->chain);
-            if (!ossl_assert(num == ctx->num_untrusted)) {
-                ERR_raise(ERR_LIB_X509, ERR_R_INTERNAL_ERROR);
-                trust = X509_TRUST_REJECTED;
-                ctx->error = X509_V_ERR_UNSPECIFIED;
-                search = 0;
-                continue;
-            }
-            x = sk_X509_value(ctx->chain, num-1);
-
-            /*
-             * Once we run out of untrusted issuers, we stop looking for more
-             * and start looking only in the trust store if enabled.
-             */
-            xtmp = (self_signed || depth < num) ? NULL
-                                                : find_issuer(ctx, sktmp, x);
-            if (xtmp == NULL) {
+            if (!ossl_assert(num == ctx->num_untrusted))
+                goto int_err;
+            curr = sk_X509_value(ctx->chain, num - 1);
+            issuer = (self_signed || depth < num) ?
+                NULL : find_issuer(ctx, sk_untrusted, curr);
+            if (issuer == NULL) {
+                /*
+                 * Once we have reached a self-signed cert or num exceeds 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.
+                 */
                 search &= ~S_DOUNTRUSTED;
                 if (may_trusted)
                     search |= S_DOTRUSTED;
@@ -3255,44 +3192,23 @@ static int build_chain(X509_STORE_CTX *ctx)
             }
 
             /* Drop this issuer from future consideration */
-            (void) sk_X509_delete_ptr(sktmp, xtmp);
+            (void)sk_X509_delete_ptr(sk_untrusted, issuer);
 
-            if (!X509_up_ref(xtmp)) {
-                ERR_raise(ERR_LIB_X509, ERR_R_INTERNAL_ERROR);
-                trust = X509_TRUST_REJECTED;
-                ctx->error = X509_V_ERR_UNSPECIFIED;
-                search = 0;
-                continue;
-            }
-
-            if (!sk_X509_push(ctx->chain, xtmp)) {
-                X509_free(xtmp);
-                ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
-                trust = X509_TRUST_REJECTED;
-                ctx->error = X509_V_ERR_OUT_OF_MEM;
-                search = 0;
-                continue;
-            }
+            if (!X509_add_cert(ctx->chain, issuer,  X509_ADD_FLAG_UP_REF))
+                goto int_err;
 
-            x = xtmp;
             ++ctx->num_untrusted;
-            self_signed = X509_self_signed(x, 0);
-            if (self_signed < 0) {
-                sk_X509_free(sktmp);
-                ctx->error = X509_V_ERR_UNSPECIFIED;
-                return 0;
-            }
+            curr = issuer;
+            if ((self_signed = X509_self_signed(curr, 0)) < 0)
+                goto int_err;
 
             /* Check for DANE-TA trust of the topmost untrusted certificate. */
-            switch (trust = check_dane_issuer(ctx, ctx->num_untrusted - 1)) {
-            case X509_TRUST_TRUSTED:
-            case X509_TRUST_REJECTED:
-                search = 0;
-                continue;
-            }
+            trust = check_dane_issuer(ctx, ctx->num_untrusted - 1);
+            if (trust == X509_TRUST_TRUSTED || trust == X509_TRUST_REJECTED)
+                break;
         }
     }
-    sk_X509_free(sktmp);
+    sk_X509_free(sk_untrusted);
 
     /*
      * Last chance to make a trusted chain, either bare DANE-TA public-key
@@ -3316,20 +3232,26 @@ static int build_chain(X509_STORE_CTX *ctx)
     default:
         num = sk_X509_num(ctx->chain);
         CB_FAIL_IF(num > depth,
-                   ctx, NULL, num-1, X509_V_ERR_CERT_CHAIN_TOO_LONG);
+                   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);
+                   ctx, NULL, num - 1, X509_V_ERR_DANE_NO_MATCH);
         if (self_signed)
-            return verify_cb_cert(ctx, NULL, num-1,
+            return verify_cb_cert(ctx, NULL, num - 1,
                                   sk_X509_num(ctx->chain) == 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,
+        return verify_cb_cert(ctx, NULL, num - 1,
                               ctx->num_untrusted < num
                               ? X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT
                               : X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY);
     }
+
+ int_err:
+    sk_X509_free(sk_untrusted);
+    ERR_raise(ERR_LIB_X509, ERR_R_INTERNAL_ERROR);
+    ctx->error = X509_V_ERR_UNSPECIFIED;
+    return 0;
 }
 
 static const int minbits_table[] = { 80, 112, 128, 192, 256 };


More information about the openssl-commits mailing list