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>
This commit is contained in:
parent
82d0ffbac1
commit
8f1e113a73
@ -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);
|
||||
|
@ -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;
|
||||
}
|
||||
|
@ -292,7 +292,7 @@ OPENSSL_EXPORT void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth);
|
||||
X509_LOOKUP_ctrl((x),X509_L_ADD_DIR,(name),(long)(type),NULL)
|
||||
|
||||
#define X509_V_OK 0
|
||||
/* illegal error (for uninitialized values, to avoid X509_V_OK): 1 */
|
||||
#define X509_V_ERR_UNSPECIFIED 1
|
||||
|
||||
#define X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT 2
|
||||
#define X509_V_ERR_UNABLE_TO_GET_CRL 3
|
||||
@ -347,6 +347,7 @@ OPENSSL_EXPORT 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
|
||||
@ -365,8 +366,10 @@ OPENSSL_EXPORT 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 */
|
||||
|
||||
@ -614,4 +617,3 @@ OPENSSL_EXPORT const X509_POLICY_NODE *
|
||||
}
|
||||
#endif
|
||||
#endif
|
||||
|
||||
|
@ -479,6 +479,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;
|
||||
|
||||
@ -496,7 +499,10 @@ int ssl_verify_alarm_type(long type) {
|
||||
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;
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user