diff options
| author | Matt Caswell <matt@openssl.org> | 2021-12-03 15:56:58 +0000 |
|---|---|---|
| committer | Matt Caswell <matt@openssl.org> | 2021-12-14 14:28:45 +0000 |
| commit | 758754966791c537ea95241438454aa86f91f256 (patch) | |
| tree | 7e1d324d9d840bcd4cbdb75c69444a9501e1366b /ssl | |
| parent | 8e78289c4e6f4c9f2fec073b78443736e689854d (diff) | |
Fix invalid handling of verify errors in libssl
In the event that X509_verify() returned an internal error result then
libssl would mishandle this and set rwstate to SSL_RETRY_VERIFY. This
subsequently causes SSL_get_error() to return SSL_ERROR_WANT_RETRY_VERIFY.
That return code is supposed to only ever be returned if an application
is using an app verify callback to complete replace the use of
X509_verify(). Applications may not be written to expect that return code
and could therefore crash (or misbehave in some other way) as a result.
CVE-2021-4044
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Diffstat (limited to 'ssl')
| -rw-r--r-- | ssl/ssl_cert.c | 15 | ||||
| -rw-r--r-- | ssl/statem/statem_clnt.c | 2 |
2 files changed, 14 insertions, 3 deletions
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 34351c161c..21ce168481 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -362,6 +362,13 @@ void ssl_cert_set_cert_cb(CERT *c, int (*cb) (SSL *ssl, void *arg), void *arg) c->cert_cb_arg = arg; } +/* + * Verify a certificate chain + * Return codes: + * 1: Verify success + * 0: Verify failure or error + * -1: Retry required + */ int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk) { X509 *x; @@ -423,10 +430,14 @@ int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk) if (s->verify_callback) X509_STORE_CTX_set_verify_cb(ctx, s->verify_callback); - if (s->ctx->app_verify_callback != NULL) + if (s->ctx->app_verify_callback != NULL) { i = s->ctx->app_verify_callback(ctx, s->ctx->app_verify_arg); - else + } else { i = X509_verify_cert(ctx); + /* We treat an error in the same way as a failure to verify */ + if (i < 0) + i = 0; + } s->verify_result = X509_STORE_CTX_get_error(ctx); sk_X509_pop_free(s->verified_chain, X509_free); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index d12d1e947e..c93c6b1f21 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1878,7 +1878,7 @@ WORK_STATE tls_post_process_server_certificate(SSL *s, WORK_STATE wst) * (less clean) historic behaviour of performing validation if any flag is * set. The *documented* interface remains the same. */ - if (s->verify_mode != SSL_VERIFY_NONE && i <= 0) { + if (s->verify_mode != SSL_VERIFY_NONE && i == 0) { SSLfatal(s, ssl_x509err2alert(s->verify_result), SSL_R_CERTIFICATE_VERIFY_FAILED); return WORK_ERROR; |