Skip to content

Commit 7587549

Browse files
committed
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>
1 parent 8e78289 commit 7587549

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

ssl/ssl_cert.c

+13-2
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,13 @@ void ssl_cert_set_cert_cb(CERT *c, int (*cb) (SSL *ssl, void *arg), void *arg)
362362
c->cert_cb_arg = arg;
363363
}
364364

365+
/*
366+
* Verify a certificate chain
367+
* Return codes:
368+
* 1: Verify success
369+
* 0: Verify failure or error
370+
* -1: Retry required
371+
*/
365372
int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk)
366373
{
367374
X509 *x;
@@ -423,10 +430,14 @@ int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk)
423430
if (s->verify_callback)
424431
X509_STORE_CTX_set_verify_cb(ctx, s->verify_callback);
425432

426-
if (s->ctx->app_verify_callback != NULL)
433+
if (s->ctx->app_verify_callback != NULL) {
427434
i = s->ctx->app_verify_callback(ctx, s->ctx->app_verify_arg);
428-
else
435+
} else {
429436
i = X509_verify_cert(ctx);
437+
/* We treat an error in the same way as a failure to verify */
438+
if (i < 0)
439+
i = 0;
440+
}
430441

431442
s->verify_result = X509_STORE_CTX_get_error(ctx);
432443
sk_X509_pop_free(s->verified_chain, X509_free);

ssl/statem/statem_clnt.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1878,7 +1878,7 @@ WORK_STATE tls_post_process_server_certificate(SSL *s, WORK_STATE wst)
18781878
* (less clean) historic behaviour of performing validation if any flag is
18791879
* set. The *documented* interface remains the same.
18801880
*/
1881-
if (s->verify_mode != SSL_VERIFY_NONE && i <= 0) {
1881+
if (s->verify_mode != SSL_VERIFY_NONE && i == 0) {
18821882
SSLfatal(s, ssl_x509err2alert(s->verify_result),
18831883
SSL_R_CERTIFICATE_VERIFY_FAILED);
18841884
return WORK_ERROR;

0 commit comments

Comments
 (0)