[openssl-commits] [openssl] OpenSSL_1_0_1-stable update
Viktor Dukhovni
viktor at openssl.org
Thu May 26 20:39:18 UTC 2016
The branch OpenSSL_1_0_1-stable has been updated
via 3d4f83a5c4c0278ae136e70cdf0799d25f01cde3 (commit)
via aed4d5b7cb156a0fe66687f941a1aa79bf75efd1 (commit)
from 051b9604f1421fe54d10185bc5c348bd343388da (commit)
- Log -----------------------------------------------------------------
commit 3d4f83a5c4c0278ae136e70cdf0799d25f01cde3
Author: Viktor Dukhovni <openssl-users at dukhovni.org>
Date: Tue May 17 18:25:40 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. 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: Rich Salz <rsalz at openssl.org>
commit aed4d5b7cb156a0fe66687f941a1aa79bf75efd1
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: Rich Salz <rsalz at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
crypto/x509/x509_txt.c | 4 ++++
crypto/x509/x509_vfy.c | 43 +++++++++++++++++++++++++++++++++++------
crypto/x509/x509_vfy.h | 22 +++++++++++++++++++--
crypto/x509v3/v3_addr.c | 6 ++++++
doc/crypto/X509_verify_cert.pod | 13 +++++++------
ssl/s3_both.c | 3 +++
6 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index d834180..0952813 100644
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -183,6 +183,10 @@ const char *X509_verify_cert_error_string(long n)
return ("unsupported or invalid name syntax");
case X509_V_ERR_CRL_PATH_VALIDATION_ERROR:
return ("CRL path validation error");
+ 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);
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 3bad523..2e2287e 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -160,6 +160,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
STACK_OF(X509) *sktmp = NULL;
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) {
@@ -168,6 +169,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;
}
@@ -180,6 +182,8 @@ 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 end;
}
CRYPTO_add(&ctx->cert->references, 1, CRYPTO_LOCK_X509);
@@ -189,6 +193,8 @@ 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 end;
}
@@ -214,6 +220,8 @@ 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 end;
}
CRYPTO_add(&xtmp->references, 1, CRYPTO_LOCK_X509);
@@ -293,15 +301,19 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
if (ctx->check_issued(ctx, x, x))
break;
ok = ctx->get_issuer(&xtmp, ctx, x);
- if (ok < 0)
- return ok;
+ 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);
X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
- return 0;
+ ctx->error = X509_V_ERR_OUT_OF_MEM;
+ ok = -1;
+ goto end;
}
num++;
}
@@ -317,8 +329,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 end;
+ }
/* Check if we found an alternate chain */
if (ok > 0) {
/*
@@ -432,6 +446,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;
}
@@ -654,12 +672,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;
}
}
}
@@ -1469,6 +1494,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 */
@@ -1497,7 +1523,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;
}
diff --git a/crypto/x509/x509_vfy.h b/crypto/x509/x509_vfy.h
index b7d8b24..eefca75 100644
--- a/crypto/x509/x509_vfy.h
+++ b/crypto/x509/x509_vfy.h
@@ -365,13 +365,31 @@ 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
# define X509_V_ERR_CRL_PATH_VALIDATION_ERROR 54
-/* The application is not happy */
-# define X509_V_ERR_APPLICATION_VERIFICATION 50
+# if 0 /* Reserved for compatibility 1.0.2 */
+/* Suite B mode algorithm violation */
+# define X509_V_ERR_SUITE_B_INVALID_VERSION 56
+# define X509_V_ERR_SUITE_B_INVALID_ALGORITHM 57
+# define X509_V_ERR_SUITE_B_INVALID_CURVE 58
+# define X509_V_ERR_SUITE_B_INVALID_SIGNATURE_ALGORITHM 59
+# define X509_V_ERR_SUITE_B_LOS_NOT_ALLOWED 60
+# define X509_V_ERR_SUITE_B_CANNOT_SIGN_P_384_WITH_P_256 61
+
+/* Host, email and IP check errors */
+# define X509_V_ERR_HOSTNAME_MISMATCH 62
+# define X509_V_ERR_EMAIL_MISMATCH 63
+# define X509_V_ERR_IP_ADDRESS_MISMATCH 64
+# endif
+
+/* Caller error */
+# define X509_V_ERR_INVALID_CALL 65
+/* Issuer lookup error */
+# define X509_V_ERR_STORE_LOOKUP 66
/* Certificate verify flags */
diff --git a/crypto/x509v3/v3_addr.c b/crypto/x509v3/v3_addr.c
index 94cfed0..1290dec 100644
--- a/crypto/x509v3/v3_addr.c
+++ b/crypto/x509v3/v3_addr.c
@@ -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;
}
diff --git a/doc/crypto/X509_verify_cert.pod b/doc/crypto/X509_verify_cert.pod
index a22e441..4689e3a 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/ssl/s3_both.c b/ssl/s3_both.c
index 107b460..c51fc6e 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -619,7 +619,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