[openssl-commits] [openssl] master update
Viktor Dukhovni
viktor at openssl.org
Wed May 18 19:20:43 UTC 2016
The branch master has been updated
via f3e235ed6faa82170d857fdec3287558eb906c58 (commit)
via 5fba3912cc770bb035accb390653bda1a795f39e (commit)
from 872759053b3990dac362c1fa561fc127e1de5924 (commit)
- Log -----------------------------------------------------------------
commit f3e235ed6faa82170d857fdec3287558eb906c58
Author: Viktor Dukhovni <openssl-users at dukhovni.org>
Date: Tue May 17 13:40:57 2016 -0400
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: Richard Levitte <levitte at openssl.org>
commit 5fba3912cc770bb035accb390653bda1a795f39e
Author: Viktor Dukhovni <openssl-users at dukhovni.org>
Date: Mon May 16 21:38:03 2016 -0400
Clarify negative return from X509_verify_cert()
Reviewed-by: Richard Levitte <levitte at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
crypto/x509/x509_txt.c | 4 ++++
crypto/x509/x509_vfy.c | 47 +++++++++++++++++++++++++++++++++--------
crypto/x509v3/v3_addr.c | 6 ++++++
doc/crypto/X509_verify_cert.pod | 13 ++++++------
include/openssl/x509_vfy.h | 5 +++++
ssl/statem/statem_lib.c | 10 +++++++++
6 files changed, 70 insertions(+), 15 deletions(-)
diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index 293efcf..5341e79 100644
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -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 */
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 866aa39..a5e7789 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -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;
}
diff --git a/crypto/x509v3/v3_addr.c b/crypto/x509v3/v3_addr.c
index 53583fb..be8ca5d 100644
--- a/crypto/x509v3/v3_addr.c
+++ b/crypto/x509v3/v3_addr.c
@@ -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;
}
diff --git a/doc/crypto/X509_verify_cert.pod b/doc/crypto/X509_verify_cert.pod
index a84ef00..c7a7bb4 100644
--- a/doc/crypto/X509_verify_cert.pod
+++ b/doc/crypto/X509_verify_cert.pod
@@ -31,12 +31,13 @@ Applications rarely call this function directly but it is used by
OpenSSL internally for certificate validation, in both the S/MIME and
SSL/TLS code.
-The negative return value from X509_verify_cert() can only occur if no
-certificate is set in B<ctx> (due to a programming error); if X509_verify_cert()
-twice without reinitialising B<ctx> in between; or if a retry
-operation is requested during internal lookups (which never happens with
-standard lookup methods). It is however recommended that application check
-for <= 0 return value on error.
+A negative return value from X509_verify_cert() can occur if it is invoked
+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
+with standard lookup methods).
+Applications must check for <= 0 return value on error.
=head1 BUGS
diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index 3826c75..44f1f16 100644
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -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
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index a9e1a24..6ceb9ec 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -548,6 +548,13 @@ 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:
+ case X509_V_ERR_DANE_NO_MATCH:
+ case X509_V_ERR_EE_KEY_TOO_SMALL:
+ case X509_V_ERR_CA_KEY_TOO_SMALL:
+ case X509_V_ERR_CA_MD_TOO_WEAK:
al = SSL_AD_BAD_CERTIFICATE;
break;
case X509_V_ERR_CERT_SIGNATURE_FAILURE:
@@ -561,7 +568,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:
More information about the openssl-commits
mailing list