Resolve a TODO in ssl3_dispatch_alert

Properly handle the case where there is pending write data and we want
to send an alert.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19550)

diff --git a/ssl/d1_msg.c b/ssl/d1_msg.c
index 279435ca03e171380b813ed9cbdadae2b2355f07..eb84ed64701d55936659696a0cf8486c140f5f07 100644 (file)
--- a/ssl/d1_msg.c
+++ b/ssl/d1_msg.c
@@ -48,7 +48,7 @@ int dtls1_dispatch_alert(SSL *ssl)
     if (s == NULL)
         return 0;
 
-    s->s3.alert_dispatch = 0;
+    s->s3.alert_dispatch = SSL_ALERT_DISPATCH_NONE;
 
     memset(buf, 0, sizeof(buf));
     *ptr++ = s->s3.send_alert[0];

diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index 88f596e23905715e3985dd458801685b42f51db3..2520288dd4c2aaa07ca2911154611f4be1eba648 100644 (file)
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -621,7 +621,7 @@ int do_dtls1_write(SSL_CONNECTION *sc, int type, const unsigned char *buf,
     int ret;
 
     /* If we have an alert to send, lets send it */
-    if (sc->s3.alert_dispatch) {
+    if (sc->s3.alert_dispatch > 0) {
         i = s->method->ssl_dispatch_alert(s);
         if (i <= 0)
             return i;

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index dc0b8b3d9efe4e11424ee45697607bf057aae787..b4435bf0201b3458ad23760ad3a1ac9d90ff52af 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -315,7 +315,7 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
     }
 
     /* If we have an alert to send, lets send it */
-    if (s->s3.alert_dispatch) {
+    if (s->s3.alert_dispatch > 0) {
         i = ssl->method->ssl_dispatch_alert(ssl);
         if (i <= 0) {
             /* SSLfatal() already called if appropriate */

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 0cf79548722df75fd548cd149672b7ee1079d571..e7078efa6c5aaaa74a6185b72244dbcf3c0b4c19 100644 (file)
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -4443,11 +4443,11 @@ int ssl3_shutdown(SSL *s)
         ssl3_send_alert(sc, SSL3_AL_WARNING, SSL_AD_CLOSE_NOTIFY);
         /*
          * our shutdown alert has been sent now, and if it still needs to be
-         * written, s->s3.alert_dispatch will be true
+         * written, s->s3.alert_dispatch will be > 0
          */
-        if (sc->s3.alert_dispatch)
+        if (sc->s3.alert_dispatch > 0)
             return -1;        /* return WANT_WRITE */
-    } else if (sc->s3.alert_dispatch) {
+    } else if (sc->s3.alert_dispatch > 0) {
         /* resend it if not sent */
         ret = s->method->ssl_dispatch_alert(s);
         if (ret == -1) {
@@ -4469,8 +4469,8 @@ int ssl3_shutdown(SSL *s)
         }
     }
 
-    if ((sc->shutdown == (SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN)) &&
-        !sc->s3.alert_dispatch)
+    if ((sc->shutdown == (SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN))
+            && sc->s3.alert_dispatch == SSL_ALERT_DISPATCH_NONE)
         return 1;
     else
         return 0;

diff --git a/ssl/s3_msg.c b/ssl/s3_msg.c
index c656cd767085a7a8d3026d3f8053e2c238221a11..3fcea15e279e4c36c1331c871f8a027f4eef1f49 100644 (file)
--- a/ssl/s3_msg.c
+++ b/ssl/s3_msg.c
@@ -61,7 +61,7 @@ int ssl3_send_alert(SSL_CONNECTION *s, int level, int desc)
     if ((level == SSL3_AL_FATAL) && (s->session != NULL))
         SSL_CTX_remove_session(s->session_ctx, s->session);
 
-    s->s3.alert_dispatch = 1;
+    s->s3.alert_dispatch = SSL_ALERT_DISPATCH_PENDING;
     s->s3.send_alert[0] = level;
     s->s3.send_alert[1] = desc;
     if (!RECORD_LAYER_write_pending(&s->rlayer)) {
@@ -85,10 +85,9 @@ int ssl3_dispatch_alert(SSL *s)
     if (sc == NULL)
         return -1;
 
-    sc->s3.alert_dispatch = 0;
-
     if (sc->rlayer.wrlmethod == NULL) {
         /* No write record layer so we can't sent and alert. We just ignore it */
+        sc->s3.alert_dispatch = SSL_ALERT_DISPATCH_NONE;
         return 1;
     }
 
@@ -104,20 +103,43 @@ int ssl3_dispatch_alert(SSL *s)
     templ.buf = &sc->s3.send_alert[0];
     templ.buflen = 2;
 
-    /* TODO(RECLAYER): What happens if there is already a write pending? */
-    if (RECORD_LAYER_write_pending(&sc->rlayer))
-        return -1;
+    if (RECORD_LAYER_write_pending(&sc->rlayer)) {
+        if (sc->s3.alert_dispatch != SSL_ALERT_DISPATCH_RETRY) {
+            /*
+             * We have a write pending but it wasn't from a previous call to
+             * this function! Can we ever get here? Maybe via API misuse??
+             * Give up.
+             */
+            sc->s3.alert_dispatch = SSL_ALERT_DISPATCH_NONE;
+            return -1;
+        }
+        /* Retry what we've already got pending */
+        i = HANDLE_RLAYER_WRITE_RETURN(sc,
+                sc->rlayer.wrlmethod->retry_write_records(sc->rlayer.wrl));
+        if (i <= 0) {
+            /* Could be NBIO. Keep alert_dispatch as SSL_ALERT_DISPATCH_RETRY */
+            return -1;
+        }
+        sc->rlayer.wpend_tot = 0;
+        sc->s3.alert_dispatch = SSL_ALERT_DISPATCH_NONE;
+        return 1;
+    }
 
     i = HANDLE_RLAYER_WRITE_RETURN(sc,
             sc->rlayer.wrlmethod->write_records(sc->rlayer.wrl, &templ, 1));
+
     if (i <= 0) {
-        sc->s3.alert_dispatch = 1;
+        sc->s3.alert_dispatch = SSL_ALERT_DISPATCH_RETRY;
+        sc->rlayer.wpend_tot = templ.buflen;
+        sc->rlayer.wpend_type = templ.type;
+        sc->rlayer.wpend_buf = templ.buf;
     } else {
         /*
          * Alert sent to BIO - now flush. If the message does not get sent due
          * to non-blocking IO, we will not worry too much.
          */
         (void)BIO_flush(sc->wbio);
+        sc->s3.alert_dispatch = SSL_ALERT_DISPATCH_NONE;
 
         if (sc->msg_callback)
             sc->msg_callback(1, sc->version, SSL3_RT_ALERT, sc->s3.send_alert,

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index e15bf29ffb77b12837d47a8639c06fcee582648d..f8b8378ee9b3af6c7ea757db8bc12149274e5f17 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2418,7 +2418,7 @@ ossl_ssize_t SSL_sendfile(SSL *s, int fd, off_t offset, size_t size, int flags)
     }
 
     /* If we have an alert to send, lets send it */
-    if (sc->s3.alert_dispatch) {
+    if (sc->s3.alert_dispatch > 0) {
         ret = (ossl_ssize_t)s->method->ssl_dispatch_alert(s);
         if (ret <= 0) {
             /* SSLfatal() already called if appropriate */

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 50e3b653725c66303278b0c5db763d494ecc24c4..da01e904a30c32669a485235be86ecc994968395 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -400,6 +400,15 @@
 # define SSL_READ_ETM(s) (s->s3.flags & TLS1_FLAGS_ENCRYPT_THEN_MAC_READ)
 # define SSL_WRITE_ETM(s) (s->s3.flags & TLS1_FLAGS_ENCRYPT_THEN_MAC_WRITE)
 
+/* alert_dispatch values */
+
+/* No alert pending */
+# define SSL_ALERT_DISPATCH_NONE    0
+/* Alert pending */
+# define SSL_ALERT_DISPATCH_PENDING 1
+/* Pending alert write needs to be retried */
+# define SSL_ALERT_DISPATCH_RETRY   2
+
 /* Mostly for SSLv3 */
 # define SSL_PKEY_RSA            0
 # define SSL_PKEY_RSA_PSS_SIGN   1