Wire up alert handling for TLSv1.3.

In TLSv1.3 there are two types of alerts "closure alerts" and
"error alerts". This makes the record layer more strict and handles closure
of the read and write channels. The callback then handles the record layer to
SSL mapping/behaviour.

ok tb@

2 files changed, 78 insertions, 9 deletions

diff --git a/src/lib/libssl/tls13_lib.c b/src/lib/libssl/tls13_lib.c
index 3860ddefef..f9505fa438 100644
--- a/src/lib/libssl/tls13_lib.c
+++ b/src/lib/libssl/tls13_lib.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: tls13_lib.c,v 1.3 2019/01/21 13:45:57 jsing Exp $ */
+/*      $OpenBSD: tls13_lib.c,v 1.4 2019/02/21 17:15:00 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -61,6 +61,35 @@ tls13_cipher_hash(const SSL_CIPHER *cipher)
         return NULL;
 }
 
+static void
+tls13_alert_received_cb(uint8_t alert_level, uint8_t alert_desc, void *arg)
+{
+        struct tls13_ctx *ctx = arg;
+        SSL *s = ctx->ssl;
+
+        if (alert_desc == SSL_AD_CLOSE_NOTIFY) {
+                ctx->ssl->internal->shutdown |= SSL_RECEIVED_SHUTDOWN;
+                S3I(ctx->ssl)->warn_alert = alert_desc;
+                return;
+        }
+
+        if (alert_desc == SSL_AD_USER_CANCELLED) {
+                /*
+                 * We treat this as advisory, since a close_notify alert
+                 * SHOULD follow this alert (RFC 8446 section 6.1).
+                 */
+                return;
+        }
+
+        /* All other alerts are treated as fatal in TLSv1.3. */
+        S3I(ctx->ssl)->fatal_alert = alert_desc;
+
+        SSLerror(ctx->ssl, SSL_AD_REASON_OFFSET + alert_desc);
+        ERR_asprintf_error_data("SSL alert number %d", alert_desc);
+
+        SSL_CTX_remove_session(s->ctx, s->session);
+}
+
 struct tls13_ctx *
 tls13_ctx_new(int mode)
 {
@@ -72,7 +101,8 @@ tls13_ctx_new(int mode)
         ctx->mode = mode;
 
         if ((ctx->rl = tls13_record_layer_new(tls13_legacy_wire_read_cb,
-            tls13_legacy_wire_write_cb, NULL, NULL, ctx)) == NULL)
+            tls13_legacy_wire_write_cb, tls13_alert_received_cb, NULL,
+            ctx)) == NULL)
                 goto err;
 
         return ctx;
diff --git a/src/lib/libssl/tls13_record_layer.c b/src/lib/libssl/tls13_record_layer.c
index 8f6eb94df4..86062e387f 100644
--- a/src/lib/libssl/tls13_record_layer.c
+++ b/src/lib/libssl/tls13_record_layer.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_record_layer.c,v 1.4 2019/02/21 17:09:51 jsing Exp $ */
+/* $OpenBSD: tls13_record_layer.c,v 1.5 2019/02/21 17:15:00 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -26,6 +26,15 @@ struct tls13_record_layer {
         int change_cipher_spec_seen;
         int handshake_completed;
 
+        /*
+         * Read and/or write channels are closed due to an alert being
+         * sent or received. In the case of an error alert both channels
+         * are closed, whereas in the case of a close notify only one
+         * channel is closed.
+         */
+        int read_closed;
+        int write_closed;
+
         struct tls13_record *rrec;
         struct tls13_record *wrec;
 
@@ -180,31 +189,55 @@ static ssize_t
 tls13_record_layer_process_alert(struct tls13_record_layer *rl)
 {
         uint8_t alert_level, alert_desc;
+        ssize_t ret = TLS13_IO_FAILURE;
 
         /*
+         * RFC 8446 - sections 5.1 and 6.
+         *
          * A TLSv1.3 alert record can only contain a single alert - this means
          * that processing the alert must consume all of the record. The alert
          * will result in one of three things - continuation (user_cancelled),
          * read channel closure (close_notify) or termination (all others).
          */
         if (rl->rbuf == NULL)
-                return TLS13_IO_FAILURE;
+                goto err;
         if (rl->rbuf_content_type != SSL3_RT_ALERT)
-                return TLS13_IO_FAILURE;
+                goto err;
 
         if (!CBS_get_u8(&rl->rbuf_cbs, &alert_level))
-                return TLS13_IO_FAILURE; /* XXX - decode error alert. */
+                goto err; /* XXX - decode error alert. */
         if (!CBS_get_u8(&rl->rbuf_cbs, &alert_desc))
-                return TLS13_IO_FAILURE; /* XXX - decode error alert. */
+                goto err; /* XXX - decode error alert. */
 
         if (CBS_len(&rl->rbuf_cbs) != 0)
-                return TLS13_IO_FAILURE;
+                goto err; /* XXX - decode error alert. */
 
         tls13_record_layer_rbuf_free(rl);
 
+        /*
+         * Alert level is ignored for closure alerts (RFC 8446 section 6.1),
+         * however for error alerts (RFC 8446 section 6.2), the alert level
+         * must be specified as fatal.
+         */
+        if (alert_desc == SSL_AD_CLOSE_NOTIFY) {
+                rl->read_closed = 1;
+                ret = TLS13_IO_SUCCESS;
+        } else if (alert_desc == SSL_AD_USER_CANCELLED) {
+                /* Ignored at the record layer. */
+                ret = TLS13_IO_SUCCESS;
+        } else if (alert_level == SSL3_AL_FATAL) {
+                rl->read_closed = 1;
+                rl->write_closed = 1;
+                ret = TLS13_IO_EOF;
+        } else {
+                /* XXX - decode error alert. */
+                return TLS13_IO_FAILURE;
+        }
+
         rl->alert_cb(alert_level, alert_desc, rl->cb_arg);
 
-        return TLS13_IO_SUCCESS;
+ err:
+        return ret;
 }
 
 int
@@ -638,6 +671,9 @@ tls13_record_layer_read(struct tls13_record_layer *rl, uint8_t content_type,
 {
         ssize_t ret;
 
+        if (rl->read_closed)
+                return TLS13_IO_EOF;
+
         /* XXX - loop here with record and byte limits. */
         /* XXX - send alert... */
 
@@ -692,6 +728,9 @@ tls13_record_layer_write_record(struct tls13_record_layer *rl,
 {
         ssize_t ret;
 
+        if (rl->write_closed)
+                return TLS13_IO_EOF;
+
         /* See if there is an existing record and attempt to push it out... */
         if (rl->wrec != NULL) {
                 if ((ret = tls13_record_send(rl->wrec, rl->wire_write,