Commit f3e235ed authored by Viktor Dukhovni's avatar Viktor Dukhovni
Browse files

Ensure verify error is set when X509_verify_cert() fails 



Set ctx->error = X509_V_ERR_OUT_OF_MEM when verificaiton cannot
continue due to malloc failure.  Also, when X509_verify_cert()
returns <= 0 make sure that the verification status does not remain
X509_V_OK, as a last resort set it it to X509_V_ERR_UNSPECIFIED,
just in case some code path returns an error without setting an
appropriate value of ctx->error.

Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
parent 5fba3912

crypto/x509/x509_txt.c

+4 −0
Original line number Diff line number Diff line
@@ -161,6 +161,10 @@ const char *X509_verify_cert_error_string(long n) 
        return ("CA certificate key too weak");

    case X509_V_ERR_CA_MD_TOO_WEAK:

        return ("CA signature digest algorithm too weak");

    case X509_V_ERR_INVALID_CALL:

        return ("Invalid certificate verification context");

    case X509_V_ERR_STORE_LOOKUP:

        return ("Issuer certificate lookup error");



    default:

        /* Printing an error number into a static buffer is not thread-safe */

crypto/x509/x509_vfy.c

+38 −9
Original line number Diff line number Diff line
@@ -251,9 +251,11 @@ static int verify_chain(X509_STORE_CTX *ctx) 
int X509_verify_cert(X509_STORE_CTX *ctx)

{

    SSL_DANE *dane = ctx->dane;

    int ret;



    if (ctx->cert == NULL) {

        X509err(X509_F_X509_VERIFY_CERT, X509_R_NO_CERT_SET_FOR_US_TO_VERIFY);

        ctx->error = X509_V_ERR_INVALID_CALL;

        return -1;

    }
@@ -263,6 +265,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
         * cannot do another one.

         */

        X509err(X509_F_X509_VERIFY_CERT, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);

        ctx->error = X509_V_ERR_INVALID_CALL;

        return -1;

    }
@@ -273,6 +276,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
    if (((ctx->chain = sk_X509_new_null()) == NULL) ||

        (!sk_X509_push(ctx->chain, ctx->cert))) {

        X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        return -1;

    }

    X509_up_ref(ctx->cert);
@@ -283,15 +287,19 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
        !verify_cb_cert(ctx, ctx->cert, 0, X509_V_ERR_EE_KEY_TOO_SMALL))

        return 0;



    if (DANETLS_ENABLED(dane))

        ret = dane_verify(ctx);

    else

        ret = verify_chain(ctx);



    /*

     * If dane->trecs is an empty stack, we'll fail, since the user enabled

     * DANE.  If none of the TLSA records were usable, and it makes sense to

     * keep going with an unauthenticated handshake, they can handle that in

     * the verify callback, or not set SSL_VERIFY_PEER.

     * Safety-net.  If we are returning an error, we must also set ctx->error,

     * so that the chain is not considered verified should the error be ignored

     * (e.g. TLS with SSL_VERIFY_NONE).

     */

    if (DANETLS_ENABLED(dane))

        return dane_verify(ctx);

    return verify_chain(ctx);

    if (ret <= 0 && ctx->error == X509_V_OK)

        ctx->error = X509_V_ERR_UNSPECIFIED;

    return ret;

}



/*
@@ -562,8 +570,16 @@ static int check_name_constraints(X509_STORE_CTX *ctx) 
            if (nc) {

                int rv = NAME_CONSTRAINTS_check(x, nc);



                if (rv != X509_V_OK && !verify_cb_cert(ctx, x, i, rv))

                switch (rv) {

                case X509_V_OK:

                    break;

                case X509_V_ERR_OUT_OF_MEM:

                    return 0;

                default:

                    if (!verify_cb_cert(ctx, x, i, rv))

                        return 0;

                    break;

                }

            }

        }

    }
@@ -1457,6 +1473,7 @@ static int check_policy(X509_STORE_CTX *ctx) 
     */

    if (ctx->bare_ta_signed && !sk_X509_push(ctx->chain, NULL)) {

        X509err(X509_F_CHECK_POLICY, ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        return 0;

    }

    ret = X509_policy_check(&ctx->tree, &ctx->explicit_policy, ctx->chain,
@@ -1466,6 +1483,7 @@ static int check_policy(X509_STORE_CTX *ctx) 


    if (ret == X509_PCY_TREE_INTERNAL) {

        X509err(X509_F_CHECK_POLICY, ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        return 0;

    }

    /* Invalid or inconsistent extensions */
@@ -1496,7 +1514,12 @@ static int check_policy(X509_STORE_CTX *ctx) 


    if (ctx->param->flags & X509_V_FLAG_NOTIFY_POLICY) {

        ctx->current_cert = NULL;

        ctx->error = X509_V_OK;

        /*

         * Verification errors need to be "sticky", a callback may have allowed

         * an SSL handshake to continue despite an error, and we must then

         * remain in an error state.  Therefore, we MUST NOT clear earlier

         * verification errors by setting the error to X509_V_OK.

         */

        if (!ctx->verify_cb(2, ctx))

            return 0;

    }
@@ -2742,6 +2765,7 @@ static int build_chain(X509_STORE_CTX *ctx) 
     */

    if (ctx->untrusted && (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) {

        X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        return 0;

    }
@@ -2758,12 +2782,14 @@ static int build_chain(X509_STORE_CTX *ctx) 
    if (DANETLS_ENABLED(dane) && dane->certs != NULL) {

        if (sktmp == NULL && (sktmp = sk_X509_new_null()) == NULL) {

            X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);

            ctx->error = X509_V_ERR_OUT_OF_MEM;

            return 0;

        }

        for (i = 0; i < sk_X509_num(dane->certs); ++i) {

            if (!sk_X509_push(sktmp, sk_X509_value(dane->certs, i))) {

                sk_X509_free(sktmp);

                X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);

                ctx->error = X509_V_ERR_OUT_OF_MEM;

                return 0;

            }

        }
@@ -2827,6 +2853,7 @@ static int build_chain(X509_STORE_CTX *ctx) 


            if (ok < 0) {

                trust = X509_TRUST_REJECTED;

                ctx->error = X509_V_ERR_STORE_LOOKUP;

                search = 0;

                continue;

            }
@@ -2873,6 +2900,7 @@ static int build_chain(X509_STORE_CTX *ctx) 
                        X509_free(xtmp);

                        X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);

                        trust = X509_TRUST_REJECTED;

                        ctx->error = X509_V_ERR_OUT_OF_MEM;

                        search = 0;

                        continue;

                    }
@@ -2969,6 +2997,7 @@ static int build_chain(X509_STORE_CTX *ctx) 
            if (!sk_X509_push(ctx->chain, xtmp)) {

                X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);

                trust = X509_TRUST_REJECTED;

                ctx->error = X509_V_ERR_OUT_OF_MEM;

                search = 0;

                continue;

            }

crypto/x509v3/v3_addr.c

+6 −0
Original line number Diff line number Diff line
@@ -1166,6 +1166,11 @@ int X509v3_addr_subset(IPAddrBlocks *a, IPAddrBlocks *b) 


/*

 * Core code for RFC 3779 2.3 path validation.

 *

 * Returns 1 for success, 0 on error.

 *

 * When returning 0, ctx->error MUST be set to an appropriate value other than

 * X509_V_OK.

 */

static int addr_validate_path_internal(X509_STORE_CTX *ctx,

                                       STACK_OF(X509) *chain,
@@ -1200,6 +1205,7 @@ static int addr_validate_path_internal(X509_STORE_CTX *ctx, 
    if ((child = sk_IPAddressFamily_dup(ext)) == NULL) {

        X509V3err(X509V3_F_ADDR_VALIDATE_PATH_INTERNAL,

                  ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        ret = 0;

        goto done;

    }

doc/crypto/X509_verify_cert.pod

+1 −1
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ OpenSSL internally for certificate validation, in both the S/MIME and 
SSL/TLS code.



A negative return value from X509_verify_cert() can occur if it is invoked

incurrectly, such as with no certificate set in B<ctx>, or when it is called

incorrectly, such as with no certificate set in B<ctx>, or when it is called

twice in succession without reinitialising B<ctx> for the second call.

A negative return value can also happen due to internal resource problems or if

a retry operation is requested during internal lookups (which never happens

include/openssl/x509_vfy.h

+5 −0
Original line number Diff line number Diff line
@@ -159,6 +159,11 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth); 
# define         X509_V_ERR_CA_KEY_TOO_SMALL                     67

# define         X509_V_ERR_CA_MD_TOO_WEAK                       68



/* Caller error */

# define         X509_V_ERR_INVALID_CALL                         69

/* Issuer lookup error */

# define         X509_V_ERR_STORE_LOOKUP                         70



/* Certificate verify flags */



# if OPENSSL_API_COMPAT < 0x10100000L