diff options
author | David Benjamin <davidben@google.com> | 2016-06-07 19:49:36 +0300 |
---|---|---|
committer | David Benjamin <davidben@google.com> | 2016-06-09 20:29:39 +0300 |
commit | 8f1e113a73e406db710479ea4d8478890c0514ea (patch) | |
tree | 159177e46c28de6d0f6b23e1784178094a1d3c3c /crypto/x509 | |
parent | 82d0ffbac1a892e23b63d055b67b7d93e489af3e (diff) |
Ensure verify error is set when X509_verify_cert() fails.
Set ctx->error = X509_V_ERR_OUT_OF_MEM when verification 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.
(Imported from upstream's 5553a12735e11bc9aa28727afe721e7236788aab.)
Change-Id: I3231a6b2e72a3914cb9316b8e90ebaee009a1c5f
Reviewed-on: https://boringssl-review.googlesource.com/8170
Reviewed-by: David Benjamin <davidben@google.com>
Diffstat (limited to 'crypto/x509')
-rw-r--r-- | crypto/x509/x509_txt.c | 5 | ||||
-rw-r--r-- | crypto/x509/x509_vfy.c | 35 |
2 files changed, 36 insertions, 4 deletions
diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c index 86af3fec..17e6cdb9 100644 --- a/crypto/x509/x509_txt.c +++ b/crypto/x509/x509_txt.c @@ -199,6 +199,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); diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 2ed2f03e..520408fe 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -198,6 +198,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) STACK_OF(X509) *sktmp = NULL; if (ctx->cert == NULL) { OPENSSL_PUT_ERROR(X509, X509_R_NO_CERT_SET_FOR_US_TO_VERIFY); + ctx->error = X509_V_ERR_INVALID_CALL; return -1; } if (ctx->chain != NULL) { @@ -206,6 +207,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) * cannot do another one. */ OPENSSL_PUT_ERROR(X509, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); + ctx->error = X509_V_ERR_INVALID_CALL; return -1; } @@ -218,6 +220,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) ctx->chain = sk_X509_new_null(); if (ctx->chain == NULL || !sk_X509_push(ctx->chain, ctx->cert)) { OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE); + ctx->error = X509_V_ERR_OUT_OF_MEM; goto end; } X509_up_ref(ctx->cert); @@ -227,6 +230,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) if (ctx->untrusted != NULL && (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) { OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE); + ctx->error = X509_V_ERR_OUT_OF_MEM; goto end; } @@ -250,8 +254,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 end; + } /* * If successful for now free up cert so it will be picked up * again later. @@ -268,6 +274,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx) if (xtmp != NULL) { if (!sk_X509_push(ctx->chain, xtmp)) { OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE); + ctx->error = X509_V_ERR_OUT_OF_MEM; ok = 0; goto end; } @@ -349,14 +356,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 end; + } if (ok == 0) break; x = xtmp; if (!sk_X509_push(ctx->chain, x)) { X509_free(xtmp); OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE); + ctx->error = X509_V_ERR_OUT_OF_MEM; ok = 0; goto end; } @@ -493,6 +503,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; } @@ -709,12 +723,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; } } } @@ -1605,6 +1626,7 @@ static int check_policy(X509_STORE_CTX *ctx) ctx->param->policies, ctx->param->flags); if (ret == 0) { OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE); + ctx->error = X509_V_ERR_OUT_OF_MEM; return 0; } /* Invalid or inconsistent extensions */ @@ -1633,7 +1655,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; } |