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>
diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index 86af3fe..17e6cdb 100644
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -199,6 +199,11 @@
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 2ed2f03..520408f 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -198,6 +198,7 @@
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 @@
* 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 @@
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 @@
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 @@
*/
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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;
}
diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index a9d0519..16f03d6 100644
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -292,7 +292,7 @@
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 @@
#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 @@
#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 @@
}
#endif
#endif
-
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index e16bf4b..f5d0c13 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -479,6 +479,9 @@
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 @@
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;
