[openssl] master update

dev at ddvo.net dev at ddvo.net
Fri Nov 6 10:18:24 UTC 2020


The branch master has been updated
       via  3309c4b716c922172b6a7ae0cef88fad0203886d (commit)
       via  6e5e118c2a4da054db4d91cf37cc89e2e5b35e72 (commit)
       via  0e071fbce49c19f59d740dbc5ebff873cd83eefa (commit)
       via  bbc8343478376699a4aaf9416dfc520fecc5d236 (commit)
      from  7bfd934049021ebf56db3f9670961c958104445d (commit)


- Log -----------------------------------------------------------------
commit 3309c4b716c922172b6a7ae0cef88fad0203886d
Author: David von Oheimb <dev at ddvo.net>
Date:   Wed Nov 4 13:07:08 2020 +0100

    x509_vfy.c: Call verification callback individually per strict check in check_chain()
    
    Fixes #13283
    
    Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
    (Merged from https://github.com/openssl/openssl/pull/13312)

commit 6e5e118c2a4da054db4d91cf37cc89e2e5b35e72
Author: David von Oheimb <dev at ddvo.net>
Date:   Wed Nov 4 12:24:41 2020 +0100

    x509_vfy.c: Introduce CHECK_CB macro simplifying use of cert verification cb function
    
    Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
    (Merged from https://github.com/openssl/openssl/pull/13312)

commit 0e071fbce49c19f59d740dbc5ebff873cd83eefa
Author: David von Oheimb <dev at ddvo.net>
Date:   Wed Nov 4 12:23:34 2020 +0100

    CHANGES.md: Mention (strict) checks recently added to X509_verify_cert()
    
    Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
    (Merged from https://github.com/openssl/openssl/pull/13312)

commit bbc8343478376699a4aaf9416dfc520fecc5d236
Author: David von Oheimb <dev at ddvo.net>
Date:   Wed Nov 4 12:21:10 2020 +0100

    Improve doc of X509_verify_cert(), also in openssl.pod
    
    in particular regarding the checks due to X509_V_FLAG_X509_STRICT/-x509_strict
    
    Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
    (Merged from https://github.com/openssl/openssl/pull/13312)

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

Summary of changes:
 CHANGES.md                                |  24 +++
 crypto/x509/x509_vfy.c                    | 260 ++++++++++++------------------
 doc/man1/openssl.pod                      |  22 +++
 doc/man3/X509_STORE_CTX_set_verify_cb.pod |   2 +-
 doc/man3/X509_verify_cert.pod             |  14 +-
 5 files changed, 165 insertions(+), 157 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index e9e9bc13c3..1388167577 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -831,6 +831,30 @@ OpenSSL 3.0
 
    *Richard Levitte*
 
+ * Added several checks to X509_verify_cert() according to requirements in
+   RFC 5280 in case `X509_V_FLAG_X509_STRICT` is set
+   (which may be done by using the CLI option `-x509_strict`):
+   * The basicConstraints of CA certificates must be marked critical.
+   * CA certificates must explicitly include the keyUsage extension.
+   * If a pathlenConstraint is given the key usage keyCertSign must be allowed.
+   * The issuer name of any certificate must not be empty.
+   * The subject name of CA certs, certs with keyUsage crlSign,
+     and certs without subjectAlternativeName must not be empty.
+   * If a subjectAlternativeName extension is given it must not be empty.
+   * The signatureAlgorithm field and the cert signature must be consistent.
+   * Any given authorityKeyIdentifier and any given subjectKeyIdentifier
+     must not be marked critical.
+   * The authorityKeyIdentifier must be given for X.509v3 certs
+     unless they are self-signed.
+   * The subjectKeyIdentifier must be given for all X.509v3 CA certs.
+
+   *David von Oheimb*
+
+ * Certificate verification using X509_verify_cert() meanwhile rejects EC keys
+   with explicit curve parameters (specifiedCurve) as required by RFC 5480.
+
+   *Tomas Mraz*
+
  * For built-in EC curves, ensure an EC_GROUP built from the curve name is
    used even when parsing explicit parameters, when loading a encoded key
    or calling `EC_GROUP_new_from_ecpkparameters()`/
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 1b24e0156a..66e0a51694 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -170,6 +170,10 @@ static int verify_cb_cert(X509_STORE_CTX *ctx, X509 *x, int depth, int err)
     return ctx->verify_cb(0, ctx);
 }
 
+#define CHECK_CB(cond, ctx, cert, depth, err) \
+    if ((cond) && verify_cb_cert(ctx, cert, depth, err) == 0)   \
+        return 0
+
 /*-
  * Inform the verify callback of an error, CRL-specific variant.  Here, the
  * error depth and certificate are already set, we just specify the error
@@ -198,16 +202,14 @@ static int check_auth_level(X509_STORE_CTX *ctx)
          * We've already checked the security of the leaf key, so here we only
          * check the security of issuer keys.
          */
-        if (i > 0 && !check_key_level(ctx, cert) &&
-            verify_cb_cert(ctx, cert, i, X509_V_ERR_CA_KEY_TOO_SMALL) == 0)
-            return 0;
+        CHECK_CB(i > 0 && !check_key_level(ctx, cert),
+                 ctx, cert, i, X509_V_ERR_CA_KEY_TOO_SMALL);
         /*
          * We also check the signature algorithm security of all certificates
          * except those of the trust anchor at index num-1.
          */
-        if (i < num - 1 && !check_sig_level(ctx, cert) &&
-            verify_cb_cert(ctx, cert, i, X509_V_ERR_CA_MD_TOO_WEAK) == 0)
-            return 0;
+        CHECK_CB(i < num - 1 && !check_sig_level(ctx, cert),
+                 ctx, cert, i, X509_V_ERR_CA_MD_TOO_WEAK);
     }
     return 1;
 }
@@ -231,10 +233,7 @@ static int verify_chain(X509_STORE_CTX *ctx)
 
     err = X509_chain_check_suiteb(&ctx->error_depth, NULL, ctx->chain,
                                   ctx->param->flags);
-    if (err != X509_V_OK) {
-        if ((ok = verify_cb_cert(ctx, NULL, ctx->error_depth, err)) == 0)
-            return ok;
-    }
+    CHECK_CB(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);
@@ -286,9 +285,8 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
     ctx->num_untrusted = 1;
 
     /* If the peer's public key is too weak, we can stop early. */
-    if (!check_key_level(ctx, ctx->cert) &&
-        !verify_cb_cert(ctx, ctx->cert, 0, X509_V_ERR_EE_KEY_TOO_SMALL))
-        return 0;
+    CHECK_CB(!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);
@@ -475,52 +473,37 @@ static int check_chain(X509_STORE_CTX *ctx)
         int ret;
 
         x = sk_X509_value(ctx->chain, i);
-        if (!(ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL)
-            && (x->ex_flags & EXFLAG_CRITICAL)) {
-            if (!verify_cb_cert(ctx, x, i,
-                                X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION))
-                return 0;
-        }
-        if (!allow_proxy_certs && (x->ex_flags & EXFLAG_PROXY)) {
-            if (!verify_cb_cert(ctx, x, i,
-                                X509_V_ERR_PROXY_CERTIFICATES_NOT_ALLOWED))
-                return 0;
-        }
+        CHECK_CB((ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL) == 0
+                 && (x->ex_flags & EXFLAG_CRITICAL) != 0,
+                 ctx, x, i, X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION);
+        CHECK_CB(!allow_proxy_certs && (x->ex_flags & EXFLAG_PROXY),
+                 ctx, x, i, X509_V_ERR_PROXY_CERTIFICATES_NOT_ALLOWED);
         ret = X509_check_ca(x);
         switch (must_be_ca) {
         case -1:
-            if ((ctx->param->flags & X509_V_FLAG_X509_STRICT)
-                && (ret != 1) && (ret != 0)) {
-                ret = 0;
-                ctx->error = X509_V_ERR_INVALID_CA;
-            } else
-                ret = 1;
+            CHECK_CB((ctx->param->flags & X509_V_FLAG_X509_STRICT) != 0
+                         && ret != 1 && ret != 0,
+                     ctx, x, i, X509_V_ERR_INVALID_CA);
+            ret = 1;
             break;
         case 0:
-            if (ret != 0) {
-                ret = 0;
-                ctx->error = X509_V_ERR_INVALID_NON_CA;
-            } else
-                ret = 1;
+            CHECK_CB(ret != 0,  ctx, x, i, X509_V_ERR_INVALID_NON_CA);
+            ret = 1;
             break;
         default:
             /* X509_V_FLAG_X509_STRICT is implicit for intermediate CAs */
-            if ((ret == 0)
-                || ((i + 1 < num || ctx->param->flags & X509_V_FLAG_X509_STRICT)
-                    && (ret != 1))) {
-                ret = 0;
-                ctx->error = X509_V_ERR_INVALID_CA;
-            } else
-                ret = 1;
+            CHECK_CB(ret == 0
+                     || ((i + 1 < num
+                          || ctx->param->flags & X509_V_FLAG_X509_STRICT)
+                         && ret != 1), ctx, x, i, X509_V_ERR_INVALID_CA);
+            ret = 1;
             break;
         }
         if (num > 1) {
             /* Check for presence of explicit elliptic curve parameters */
             ret = check_curve(x);
-            if (ret < 0)
-                ctx->error = X509_V_ERR_UNSPECIFIED;
-            else if (ret == 0)
-                ctx->error = X509_V_ERR_EC_KEY_EXPLICIT_PARAMS;
+            CHECK_CB(ret < 0, ctx, x, i, X509_V_ERR_UNSPECIFIED);
+            CHECK_CB(ret == 0, ctx, x, i, X509_V_ERR_EC_KEY_EXPLICIT_PARAMS);
         }
         /*
          * Do the following set of checks only if strict checking is requrested
@@ -535,76 +518,73 @@ static int check_chain(X509_STORE_CTX *ctx)
                            */
             /* Check Basic Constraints according to RFC 5280 section 4.2.1.9 */
             if (x->ex_pathlen != -1) {
-                if ((x->ex_flags & EXFLAG_CA) == 0)
-                    ctx->error = X509_V_ERR_PATHLEN_INVALID_FOR_NON_CA;
-                if ((x->ex_kusage & KU_KEY_CERT_SIGN) == 0)
-                    ctx->error = X509_V_ERR_PATHLEN_WITHOUT_KU_KEY_CERT_SIGN;
+                CHECK_CB((x->ex_flags & EXFLAG_CA) == 0,
+                         ctx, x, i, X509_V_ERR_PATHLEN_INVALID_FOR_NON_CA);
+                CHECK_CB((x->ex_kusage & KU_KEY_CERT_SIGN) == 0, ctx, x, i,
+                         X509_V_ERR_PATHLEN_WITHOUT_KU_KEY_CERT_SIGN);
             }
-            if ((x->ex_flags & EXFLAG_CA) != 0
-                    && (x->ex_flags & EXFLAG_BCONS) != 0
-                    && (x->ex_flags & EXFLAG_BCONS_CRITICAL) == 0)
-                ctx->error = X509_V_ERR_CA_BCONS_NOT_CRITICAL;
+            CHECK_CB((x->ex_flags & EXFLAG_CA) != 0
+                         && (x->ex_flags & EXFLAG_BCONS) != 0
+                         && (x->ex_flags & EXFLAG_BCONS_CRITICAL) == 0,
+                     ctx, x, i, X509_V_ERR_CA_BCONS_NOT_CRITICAL);
             /* Check Key Usage according to RFC 5280 section 4.2.1.3 */
             if ((x->ex_flags & EXFLAG_CA) != 0) {
-                if ((x->ex_flags & EXFLAG_KUSAGE) == 0)
-                    ctx->error = X509_V_ERR_CA_CERT_MISSING_KEY_USAGE;
+                CHECK_CB((x->ex_flags & EXFLAG_KUSAGE) == 0,
+                         ctx, x, i, X509_V_ERR_CA_CERT_MISSING_KEY_USAGE);
             } else {
-                if ((x->ex_kusage & KU_KEY_CERT_SIGN) != 0)
-                    ctx->error = X509_V_ERR_KU_KEY_CERT_SIGN_INVALID_FOR_NON_CA;
+                CHECK_CB((x->ex_kusage & KU_KEY_CERT_SIGN) != 0, ctx, x, i,
+                         X509_V_ERR_KU_KEY_CERT_SIGN_INVALID_FOR_NON_CA);
             }
             /* Check issuer is non-empty acc. to RFC 5280 section 4.1.2.4 */
-            if (X509_NAME_entry_count(X509_get_issuer_name(x)) == 0)
-                ctx->error = X509_V_ERR_ISSUER_NAME_EMPTY;
+            CHECK_CB(X509_NAME_entry_count(X509_get_issuer_name(x)) == 0,
+                     ctx, x, i, X509_V_ERR_ISSUER_NAME_EMPTY);
             /* Check subject is non-empty acc. to RFC 5280 section 4.1.2.6 */
-            if (((x->ex_flags & EXFLAG_CA) != 0
-                 || (x->ex_kusage & KU_CRL_SIGN) != 0
-                 || x->altname == NULL
-                 ) && X509_NAME_entry_count(X509_get_subject_name(x)) == 0)
-                ctx->error = X509_V_ERR_SUBJECT_NAME_EMPTY;
-            if (X509_NAME_entry_count(X509_get_subject_name(x)) == 0
-                    && x->altname != NULL
-                    && (x->ex_flags & EXFLAG_SAN_CRITICAL) == 0)
-                ctx->error = X509_V_ERR_EMPTY_SUBJECT_SAN_NOT_CRITICAL;
+            CHECK_CB(((x->ex_flags & EXFLAG_CA) != 0
+                      || (x->ex_kusage & KU_CRL_SIGN) != 0
+                      || x->altname == NULL
+                      ) && X509_NAME_entry_count(X509_get_subject_name(x)) == 0,
+                     ctx, x, i, X509_V_ERR_SUBJECT_NAME_EMPTY);
+            CHECK_CB(X509_NAME_entry_count(X509_get_subject_name(x)) == 0
+                         && x->altname != NULL
+                         && (x->ex_flags & EXFLAG_SAN_CRITICAL) == 0,
+                     ctx, x, i, X509_V_ERR_EMPTY_SUBJECT_SAN_NOT_CRITICAL);
             /* Check SAN is non-empty according to RFC 5280 section 4.2.1.6 */
-            if (x->altname != NULL && sk_GENERAL_NAME_num(x->altname) <= 0)
-                ctx->error = X509_V_ERR_EMPTY_SUBJECT_ALT_NAME;
+            CHECK_CB(x->altname != NULL && sk_GENERAL_NAME_num(x->altname) <= 0,
+                     ctx, x, i, X509_V_ERR_EMPTY_SUBJECT_ALT_NAME);
             /* TODO add more checks on SAN entries */
             /* Check sig alg consistency acc. to RFC 5280 section 4.1.1.2 */
-            if (X509_ALGOR_cmp(&x->sig_alg, &x->cert_info.signature) != 0)
-                ctx->error = X509_V_ERR_SIGNATURE_ALGORITHM_INCONSISTENCY;
-            if (x->akid != NULL && (x->ex_flags & EXFLAG_AKID_CRITICAL) != 0)
-                ctx->error = X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL;
-            if (x->skid != NULL && (x->ex_flags & EXFLAG_SKID_CRITICAL) != 0)
-                ctx->error = X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL;
+            CHECK_CB(X509_ALGOR_cmp(&x->sig_alg, &x->cert_info.signature) != 0,
+                     ctx, x, i, X509_V_ERR_SIGNATURE_ALGORITHM_INCONSISTENCY);
+            CHECK_CB(x->akid != NULL
+                         && (x->ex_flags & EXFLAG_AKID_CRITICAL) != 0,
+                     ctx, x, i, X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL);
+            CHECK_CB(x->skid != NULL
+                         && (x->ex_flags & EXFLAG_SKID_CRITICAL) != 0,
+                     ctx, x, i, X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL);
             if (X509_get_version(x) >= 2) { /* at least X.509v3 */
                 /* Check AKID presence acc. to RFC 5280 section 4.2.1.1 */
-                if (i + 1 < num /*
-                                 * this means not last cert in chain,
-                                 * taken as "generated by conforming CAs"
-                                 */
-                        && (x->akid == NULL || x->akid->keyid == NULL))
-                    ctx->error = X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER;
+                CHECK_CB(i + 1 < num /*
+                                      * this means not last cert in chain,
+                                      * taken as "generated by conforming CAs"
+                                      */
+                         && (x->akid == NULL || x->akid->keyid == NULL), ctx,
+                         x, i, X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER);
                 /* Check SKID presence acc. to RFC 5280 section 4.2.1.2 */
-                if ((x->ex_flags & EXFLAG_CA) != 0 && x->skid == NULL)
-                    ctx->error = X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER;
+                CHECK_CB((x->ex_flags & EXFLAG_CA) != 0 && x->skid == NULL,
+                         ctx, x, i, X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER);
             } else {
-                if (sk_X509_EXTENSION_num(X509_get0_extensions(x)) > 0)
-                    ctx->error = X509_V_ERR_EXTENSIONS_REQUIRE_VERSION_3;
+                CHECK_CB(sk_X509_EXTENSION_num(X509_get0_extensions(x)) > 0,
+                         ctx, x, i, X509_V_ERR_EXTENSIONS_REQUIRE_VERSION_3);
             }
         }
-        if (ctx->error != X509_V_OK)
-            ret = 0;
-        if (ret == 0 && !verify_cb_cert(ctx, x, i, X509_V_OK))
-            return 0;
+
         /* check_purpose() makes the callback as needed */
         if (purpose > 0 && !check_purpose(ctx, x, purpose, i, must_be_ca))
             return 0;
         /* Check pathlen */
-        if ((i > 1) && (x->ex_pathlen != -1)
-            && (plen > (x->ex_pathlen + proxy_path_length))) {
-            if (!verify_cb_cert(ctx, x, i, X509_V_ERR_PATH_LENGTH_EXCEEDED))
-                return 0;
-        }
+        CHECK_CB(i > 1 && x->ex_pathlen != -1
+                     && plen > x->ex_pathlen + proxy_path_length,
+                 ctx, x, i, X509_V_ERR_PATH_LENGTH_EXCEEDED);
         /* Increment path length if not a self-issued intermediate CA */
         if (i > 0 && (x->ex_flags & EXFLAG_SI) == 0)
             plen++;
@@ -626,11 +606,8 @@ static int check_chain(X509_STORE_CTX *ctx)
              * increment proxy_path_length.
              */
             if (x->ex_pcpathlen != -1) {
-                if (proxy_path_length > x->ex_pcpathlen) {
-                    if (!verify_cb_cert(ctx, x, i,
-                                        X509_V_ERR_PROXY_PATH_LENGTH_EXCEEDED))
-                        return 0;
-                }
+                CHECK_CB(proxy_path_length > x->ex_pcpathlen,
+                         ctx, x, i, X509_V_ERR_PROXY_PATH_LENGTH_EXCEEDED);
                 proxy_path_length = x->ex_pcpathlen;
             }
             proxy_path_length++;
@@ -742,9 +719,7 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
             X509_NAME_free(tmpsubject);
 
          proxy_name_done:
-            if (err != X509_V_OK
-                && !verify_cb_cert(ctx, x, i, err))
-                return 0;
+            CHECK_CB(err != X509_V_OK, ctx, x, i, err);
         }
 
         /*
@@ -774,8 +749,7 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
                 case X509_V_ERR_OUT_OF_MEM:
                     return 0;
                 default:
-                    if (!verify_cb_cert(ctx, x, i, rv))
-                        return 0;
+                    CHECK_CB(1, ctx, x, i, rv);
                     break;
                 }
             }
@@ -908,9 +882,8 @@ static int check_trust(X509_STORE_CTX *ctx, int num_untrusted)
     return X509_TRUST_UNTRUSTED;
 
  rejected:
-    if (!verify_cb_cert(ctx, x, i, X509_V_ERR_CERT_REJECTED))
-        return X509_TRUST_REJECTED;
-    return X509_TRUST_UNTRUSTED;
+    return verify_cb_cert(ctx, x, i, X509_V_ERR_CERT_REJECTED) == 0
+        ? X509_TRUST_REJECTED : X509_TRUST_UNTRUSTED;
 
  trusted:
     if (!DANETLS_ENABLED(dane))
@@ -1707,11 +1680,8 @@ static int check_policy(X509_STORE_CTX *ctx)
         for (i = 1; i < sk_X509_num(ctx->chain); i++) {
             X509 *x = sk_X509_value(ctx->chain, i);
 
-            if (!(x->ex_flags & EXFLAG_INVALID_POLICY))
-                continue;
-            if (!verify_cb_cert(ctx, x, i,
-                                X509_V_ERR_INVALID_POLICY_EXTENSION))
-                return 0;
+            CHECK_CB((x->ex_flags & EXFLAG_INVALID_POLICY) != 0,
+                     ctx, x, i, X509_V_ERR_INVALID_POLICY_EXTENSION);
         }
         return 1;
     }
@@ -1762,20 +1732,14 @@ int x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth)
     i = X509_cmp_time(X509_get0_notBefore(x), ptime);
     if (i >= 0 && depth < 0)
         return 0;
-    if (i == 0 && !verify_cb_cert(ctx, x, depth,
-                                  X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD))
-        return 0;
-    if (i > 0 && !verify_cb_cert(ctx, x, depth, X509_V_ERR_CERT_NOT_YET_VALID))
-        return 0;
+    CHECK_CB(i == 0, ctx, x, depth, X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD);
+    CHECK_CB(i > 0, ctx, x, depth, X509_V_ERR_CERT_NOT_YET_VALID);
 
     i = X509_cmp_time(X509_get0_notAfter(x), ptime);
     if (i <= 0 && depth < 0)
         return 0;
-    if (i == 0 && !verify_cb_cert(ctx, x, depth,
-                                  X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD))
-        return 0;
-    if (i < 0 && !verify_cb_cert(ctx, x, depth, X509_V_ERR_CERT_HAS_EXPIRED))
-        return 0;
+    CHECK_CB(i == 0, ctx, x, depth, X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD);
+    CHECK_CB(i < 0, ctx, x, depth, X509_V_ERR_CERT_HAS_EXPIRED);
     return 1;
 }
 
@@ -1805,9 +1769,7 @@ static int internal_verify(X509_STORE_CTX *ctx)
             goto check_cert_time;
         }
         if (n <= 0) {
-            if (!verify_cb_cert(ctx, xi, 0,
-                                X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE))
-                return 0;
+            CHECK_CB(1, ctx, xi, 0, X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE);
 
             xs = xi;
             goto check_cert_time;
@@ -1858,16 +1820,13 @@ static int internal_verify(X509_STORE_CTX *ctx)
             int ret = xs == xi && (xi->ex_flags & EXFLAG_CA) == 0
                 ? X509_V_OK : x509_signing_allowed(xi, xs);
 
-            if (ret != X509_V_OK && !verify_cb_cert(ctx, xi, issuer_depth, ret))
-                return 0;
+            CHECK_CB(ret != X509_V_OK, ctx, xi, issuer_depth, ret);
             if ((pkey = X509_get0_pubkey(xi)) == NULL) {
-                ret = X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY;
-                if (!verify_cb_cert(ctx, xi, issuer_depth, ret))
-                    return 0;
-            } else if (X509_verify(xs, pkey) <= 0) {
-                ret = X509_V_ERR_CERT_SIGNATURE_FAILURE;
-                if (!verify_cb_cert(ctx, xs, n, ret))
-                    return 0;
+                CHECK_CB(1, ctx, xi, issuer_depth,
+                         X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY);
+            } else {
+                CHECK_CB(X509_verify(xs, pkey) <= 0,
+                         ctx, xs, n, X509_V_ERR_CERT_SIGNATURE_FAILURE);
             }
         }
 
@@ -2948,9 +2907,8 @@ static int check_leaf_suiteb(X509_STORE_CTX *ctx, X509 *cert)
 {
     int err = X509_chain_check_suiteb(NULL, cert, NULL, ctx->param->flags);
 
-    if (err == X509_V_OK)
-        return 1;
-    return verify_cb_cert(ctx, cert, 0, err);
+    CHECK_CB(err != X509_V_OK, ctx, cert, 0, err);
+    return 1;
 }
 
 static int dane_verify(X509_STORE_CTX *ctx)
@@ -3394,23 +3352,19 @@ static int build_chain(X509_STORE_CTX *ctx)
     case X509_TRUST_UNTRUSTED:
     default:
         num = sk_X509_num(ctx->chain);
-        if (num > depth)
-            return verify_cb_cert(ctx, NULL, num-1,
-                                  X509_V_ERR_CERT_CHAIN_TOO_LONG);
-        if (DANETLS_ENABLED(dane) &&
-            (!DANETLS_HAS_PKIX(dane) || dane->pdpth >= 0))
-            return verify_cb_cert(ctx, NULL, num-1, X509_V_ERR_DANE_NO_MATCH);
-        if (self_signed && sk_X509_num(ctx->chain) == 1)
-            return verify_cb_cert(ctx, NULL, num-1,
-                                  X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT);
+        CHECK_CB(num > depth, ctx, NULL, num-1, X509_V_ERR_CERT_CHAIN_TOO_LONG);
+        CHECK_CB(DANETLS_ENABLED(dane)
+                     && (!DANETLS_HAS_PKIX(dane) || dane->pdpth >= 0),
+                 ctx, NULL, num-1, X509_V_ERR_DANE_NO_MATCH);
         if (self_signed)
             return verify_cb_cert(ctx, NULL, num-1,
-                                  X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN);
-        if (ctx->num_untrusted < num)
-            return verify_cb_cert(ctx, NULL, num-1,
-                                  X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT);
+                                  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,
-                              X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY);
+                              ctx->num_untrusted < num
+                              ? X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT
+                              : X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY);
     }
 }
 
diff --git a/doc/man1/openssl.pod b/doc/man1/openssl.pod
index 723ed0e2f1..2855b9eac4 100644
--- a/doc/man1/openssl.pod
+++ b/doc/man1/openssl.pod
@@ -884,6 +884,28 @@ a verification time, the check is not suppressed.
 This disables non-compliant workarounds for broken certificates.
 Thus errors are thrown on certificates not compliant with RFC 5280.
 
+When this option is set,
+among others, the following certificate well-formedness conditions are checked:
+
+=over 8
+
+=item The basicConstraints of CA certificates must be marked critical.
+=item CA certificates must explicitly include the keyUsage extension.
+=item If a pathlenConstraint is given the key usage keyCertSign must be allowed.
+=item The pathlenConstraint must not be given for non-CA certificates.
+=item The issuer name of any certificate must not be empty.
+=item The subject name of CA certs, certs with keyUsage crlSign,
+      and certs without subjectAlternativeName must not be empty.
+=item If a subjectAlternativeName extension is given it must not be empty.
+=item The signatureAlgorithm field and the cert signature must be consistent.
+=item Any given authorityKeyIdentifier and any given subjectKeyIdentifier
+      must not be marked critical.
+=item The authorityKeyIdentifier must be given for X.509v3 certs
+      unless they are self-signed.
+=item The subjectKeyIdentifier must be given for all X.509v3 CA certs.
+
+=back
+
 =item B<-ignore_critical>
 
 Normally if an unhandled critical extension is present that is not
diff --git a/doc/man3/X509_STORE_CTX_set_verify_cb.pod b/doc/man3/X509_STORE_CTX_set_verify_cb.pod
index cfde5ab5ba..fefe6a25a0 100644
--- a/doc/man3/X509_STORE_CTX_set_verify_cb.pod
+++ b/doc/man3/X509_STORE_CTX_set_verify_cb.pod
@@ -47,7 +47,7 @@ X509_STORE_CTX_set_verify_cb() sets the verification callback of B<ctx> to
 B<verify_cb> overwriting any existing callback.
 
 The verification callback can be used to customise the operation of certificate
-verification, either by overriding error conditions or logging errors for
+verification, for instance by overriding error conditions or logging errors for
 debugging purposes.
 
 However, a verification callback is B<not> essential and the default operation
diff --git a/doc/man3/X509_verify_cert.pod b/doc/man3/X509_verify_cert.pod
index 9368dc7e83..9dedcbc987 100644
--- a/doc/man3/X509_verify_cert.pod
+++ b/doc/man3/X509_verify_cert.pod
@@ -13,8 +13,15 @@ X509_verify_cert - discover and verify X509 certificate chain
 =head1 DESCRIPTION
 
 The X509_verify_cert() function attempts to discover and validate a
-certificate chain based on parameters in B<ctx>. A complete description of
-the process is contained in the L<openssl-verify(1)> manual page.
+certificate chain based on parameters in B<ctx>.
+The verification context, of type B<X509_STORE_CTX>, can be constructed
+using L<X509_STORE_CTX_new(3)> and L<X509_STORE_CTX_init(3)>.
+It usually includes a set of certificates serving as trust anchors,
+a set of non-trusted certificates that may be needed for chain construction,
+flags such as X509_V_FLAG_X509_STRICT, and various other optional components
+such as a callback function that allows customizing the verification outcome.
+A complete description of the certificate verification process is contained in
+the L<openssl-verify(1)> manual page.
 
 Applications rarely call this function directly but it is used by
 OpenSSL internally for certificate validation, in both the S/MIME and
@@ -35,7 +42,7 @@ otherwise it return zero, in exceptional circumstances it can also
 return a negative code.
 
 If the function fails additional error information can be obtained by
-examining B<ctx> using, for example X509_STORE_CTX_get_error().
+examining B<ctx> using, for example L<X509_STORE_CTX_get_error(3)>.
 
 =head1 BUGS
 
@@ -45,6 +52,7 @@ functions which use F<< <x509_vfy.h> >>.
 
 =head1 SEE ALSO
 
+L<X509_STORE_CTX_new(3)>, L<X509_STORE_CTX_init(3)>,
 L<X509_STORE_CTX_get_error(3)>
 
 =head1 COPYRIGHT


More information about the openssl-commits mailing list