From 0efa7592e3fb2ae647518a82ab19679b7c933d99 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 27 Jan 2017 20:51:33 -0500 Subject: [PATCH] dispatch_alert is not an incidental write. It is impossible to have to call dispatch_alert when writing application data. Now that we don't send warning alerts through ssl3_send_alert, all alerts are closure alerts, which means attempts to write will fail. This prunes a lot of dead code, avoiding the re-entrancy in the write path. With that gone, tracking alert_dispatch is much more straightforward. BUG=146 Change-Id: Ie5fe677daee71e463d79562f3d2cea822a92581d Reviewed-on: https://boringssl-review.googlesource.com/13500 CQ-Verified: CQ bot account: commit-bot@chromium.org Reviewed-by: Steven Valdez Reviewed-by: David Benjamin Commit-Queue: David Benjamin --- ssl/d1_pkt.c | 12 +----------- ssl/s3_pkt.c | 19 +++++-------------- 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c index c6950d58..31d9dc0a 100644 --- a/ssl/d1_pkt.c +++ b/ssl/d1_pkt.c @@ -364,15 +364,6 @@ int dtls1_write_record(SSL *ssl, int type, const uint8_t *buf, size_t len, * |ssl_write_buffer_flush|. */ assert(!ssl_write_buffer_is_pending(ssl)); - /* If we have an alert to send, lets send it */ - if (ssl->s3->alert_dispatch) { - int ret = ssl->method->dispatch_alert(ssl); - if (ret <= 0) { - return ret; - } - /* if it went, fall through and send more stuff */ - } - if (len > SSL3_RT_MAX_PLAIN_LENGTH) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return -1; @@ -397,13 +388,12 @@ int dtls1_write_record(SSL *ssl, int type, const uint8_t *buf, size_t len, } int dtls1_dispatch_alert(SSL *ssl) { - ssl->s3->alert_dispatch = 0; int ret = dtls1_write_record(ssl, SSL3_RT_ALERT, &ssl->s3->send_alert[0], 2, dtls1_use_current_epoch); if (ret <= 0) { - ssl->s3->alert_dispatch = 1; return ret; } + ssl->s3->alert_dispatch = 0; /* If the alert is fatal, flush the BIO now. */ if (ssl->s3->send_alert[0] == SSL3_AL_FATAL) { diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index d7402fcb..84f52b56 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -275,15 +275,6 @@ static int do_ssl3_write(SSL *ssl, int type, const uint8_t *buf, unsigned len) { return -1; } - /* If we have an alert to send, lets send it */ - if (ssl->s3->alert_dispatch) { - int ret = ssl->method->dispatch_alert(ssl); - if (ret <= 0) { - return ret; - } - /* if it went, fall through and send more stuff */ - } - if (len > SSL3_RT_MAX_PLAIN_LENGTH) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return -1; @@ -464,10 +455,11 @@ int ssl3_send_alert(SSL *ssl, int level, int desc) { return -1; } - if (level == SSL3_AL_FATAL) { - ssl->s3->send_shutdown = ssl_shutdown_fatal_alert; - } else if (level == SSL3_AL_WARNING && desc == SSL_AD_CLOSE_NOTIFY) { + if (level == SSL3_AL_WARNING && desc == SSL_AD_CLOSE_NOTIFY) { ssl->s3->send_shutdown = ssl_shutdown_close_notify; + } else { + assert(level == SSL3_AL_FATAL); + ssl->s3->send_shutdown = ssl_shutdown_fatal_alert; } ssl->s3->alert_dispatch = 1; @@ -484,12 +476,11 @@ int ssl3_send_alert(SSL *ssl, int level, int desc) { } int ssl3_dispatch_alert(SSL *ssl) { - ssl->s3->alert_dispatch = 0; int ret = do_ssl3_write(ssl, SSL3_RT_ALERT, &ssl->s3->send_alert[0], 2); if (ret <= 0) { - ssl->s3->alert_dispatch = 1; return ret; } + ssl->s3->alert_dispatch = 0; /* If the alert is fatal, flush the BIO now. */ if (ssl->s3->send_alert[0] == SSL3_AL_FATAL) {