Commit 5553a127 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.  Similarly for issuer lookup failures
and caller errors (bad parameters or invalid state).

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.

Add new and some missing error codes to X509 error -> SSL alert switch.

Reviewed-by: default avatarTim Hudson <tjh@openssl.org>
parent 96747f0f

crypto/x509/x509_txt.c

+5 −0
Original line number Diff line number Diff line
@@ -204,6 +204,11 @@ const char *X509_verify_cert_error_string(long n) 
    case X509_V_ERR_IP_ADDRESS_MISMATCH:

        return ("IP address mismatch");



    case X509_V_ERR_INVALID_CALL:

        return ("Invalid certificate verification context");

    case X509_V_ERR_STORE_LOOKUP:

        return ("Issuer certificate lookup error");



    default:

        BIO_snprintf(buf, sizeof buf, "error number %ld", n);

        return (buf);

crypto/x509/x509_vfy.c

+34 −5
Original line number Diff line number Diff line
@@ -199,6 +199,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 


    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;

    }

    if (ctx->chain != NULL) {
@@ -207,6 +208,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;

    }
@@ -219,6 +221,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;

        ok = -1;

        goto err;

    }
@@ -229,6 +232,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
    if (ctx->untrusted != NULL

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

        X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        ok = -1;

        goto err;

    }
@@ -253,8 +257,10 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
         */

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

            ok = ctx->get_issuer(&xtmp, ctx, x);

            if (ok < 0)

            if (ok < 0) {

                ctx->error = X509_V_ERR_STORE_LOOKUP;

                goto err;

            }

            /*

             * If successful for now free up cert so it will be picked up

             * again later.
@@ -271,6 +277,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
            if (xtmp != NULL) {

                if (!sk_X509_push(ctx->chain, xtmp)) {

                    X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);

                    ctx->error = X509_V_ERR_OUT_OF_MEM;

                    ok = -1;

                    goto err;

                }
@@ -352,14 +359,17 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
                break;

            ok = ctx->get_issuer(&xtmp, ctx, x);



            if (ok < 0)

            if (ok < 0) {

                ctx->error = X509_V_ERR_STORE_LOOKUP;

                goto err;

            }

            if (ok == 0)

                break;

            x = xtmp;

            if (!sk_X509_push(ctx->chain, x)) {

                X509_free(xtmp);

                X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);

                ctx->error = X509_V_ERR_OUT_OF_MEM;

                ok = -1;

                goto err;

            }
@@ -386,8 +396,10 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
            while (j-- > 1) {

                xtmp2 = sk_X509_value(ctx->chain, j - 1);

                ok = ctx->get_issuer(&xtmp, ctx, xtmp2);

                if (ok < 0)

                if (ok < 0) {

                    ctx->error = X509_V_ERR_STORE_LOOKUP;

                    goto err;

                }

                /* Check if we found an alternate chain */

                if (ok > 0) {

                    /*
@@ -515,6 +527,10 @@ int X509_verify_cert(X509_STORE_CTX *ctx) 
        sk_X509_free(sktmp);

    if (chain_ss != NULL)

        X509_free(chain_ss);



    /* Safety net, error returns must set ctx->error */

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

        ctx->error = X509_V_ERR_UNSPECIFIED;

    return ok;

}
@@ -736,12 +752,19 @@ static int check_name_constraints(X509_STORE_CTX *ctx) 
            NAME_CONSTRAINTS *nc = sk_X509_value(ctx->chain, j)->nc;

            if (nc) {

                rv = NAME_CONSTRAINTS_check(x, nc);

                if (rv != X509_V_OK) {

                switch (rv) {

                case X509_V_OK:

                    continue;

                case X509_V_ERR_OUT_OF_MEM:

                    ctx->error = rv;

                    return 0;

                default:

                    ctx->error = rv;

                    ctx->error_depth = i;

                    ctx->current_cert = x;

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

                        return 0;

                    break;

                }

            }

        }
@@ -1630,6 +1653,7 @@ static int check_policy(X509_STORE_CTX *ctx) 
                            ctx->param->policies, ctx->param->flags);

    if (ret == 0) {

        X509err(X509_F_CHECK_POLICY, ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        return 0;

    }

    /* Invalid or inconsistent extensions */
@@ -1658,7 +1682,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;

    }

crypto/x509/x509_vfy.h

+5 −2
Original line number Diff line number Diff line
@@ -368,6 +368,7 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth); 
# define         X509_V_ERR_PERMITTED_VIOLATION                  47

# define         X509_V_ERR_EXCLUDED_VIOLATION                   48

# define         X509_V_ERR_SUBTREE_MINMAX                       49

# define         X509_V_ERR_APPLICATION_VERIFICATION             50

# define         X509_V_ERR_UNSUPPORTED_CONSTRAINT_TYPE          51

# define         X509_V_ERR_UNSUPPORTED_CONSTRAINT_SYNTAX        52

# define         X509_V_ERR_UNSUPPORTED_NAME_SYNTAX              53
@@ -386,8 +387,10 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth); 
# define         X509_V_ERR_EMAIL_MISMATCH                       63

# define         X509_V_ERR_IP_ADDRESS_MISMATCH                  64



/* The application is not happy */

# define         X509_V_ERR_APPLICATION_VERIFICATION             50

/* Caller error */

# define         X509_V_ERR_INVALID_CALL                         65

/* Issuer lookup error */

# define         X509_V_ERR_STORE_LOOKUP                         66



/* Certificate verify flags */

crypto/x509v3/v3_addr.c

+6 −0
Original line number Diff line number Diff line
@@ -1211,6 +1211,11 @@ int v3_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 v3_addr_validate_path_internal(X509_STORE_CTX *ctx,

                                          STACK_OF(X509) *chain,
@@ -1245,6 +1250,7 @@ static int v3_addr_validate_path_internal(X509_STORE_CTX *ctx, 
    if ((child = sk_IPAddressFamily_dup(ext)) == NULL) {

        X509V3err(X509V3_F_V3_ADDR_VALIDATE_PATH_INTERNAL,

                  ERR_R_MALLOC_FAILURE);

        ctx->error = X509_V_ERR_OUT_OF_MEM;

        ret = 0;

        goto done;

    }

ssl/s3_both.c

+6 −0
Original line number Diff line number Diff line
@@ -535,6 +535,9 @@ int ssl_verify_alarm_type(long type) 
    case X509_V_ERR_CRL_NOT_YET_VALID:

    case X509_V_ERR_CERT_UNTRUSTED:

    case X509_V_ERR_CERT_REJECTED:

    case X509_V_ERR_HOSTNAME_MISMATCH:

    case X509_V_ERR_EMAIL_MISMATCH:

    case X509_V_ERR_IP_ADDRESS_MISMATCH:

        al = SSL_AD_BAD_CERTIFICATE;

        break;

    case X509_V_ERR_CERT_SIGNATURE_FAILURE:
@@ -548,7 +551,10 @@ int ssl_verify_alarm_type(long type) 
    case X509_V_ERR_CERT_REVOKED:

        al = SSL_AD_CERTIFICATE_REVOKED;

        break;

    case X509_V_ERR_UNSPECIFIED:

    case X509_V_ERR_OUT_OF_MEM:

    case X509_V_ERR_INVALID_CALL:

    case X509_V_ERR_STORE_LOOKUP:

        al = SSL_AD_INTERNAL_ERROR;

        break;

    case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT: