QUIC TLS: Report TLS errors properly as QUIC protocol errors

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21547)

diff --git a/include/internal/quic_error.h b/include/internal/quic_error.h
index 0d5c6b4529076a235ee55f618519edc1370ebaef..e32bdf03f8fdb8d79f4c39924018a9aeeea93f84 100644 (file)
--- a/include/internal/quic_error.h
+++ b/include/internal/quic_error.h
@@ -43,6 +43,9 @@
 #  define QUIC_ERR_CRYPTO_MISSING_EXT \
     QUIC_ERR_CRYPTO_ERR(TLS13_AD_MISSING_EXTENSION)
 
+#  define QUIC_ERR_CRYPTO_NO_APP_PROTO \
+    QUIC_ERR_CRYPTO_ERR(TLS1_AD_NO_APPLICATION_PROTOCOL)
+
 # endif
 
 #endif

diff --git a/include/internal/quic_tls.h b/include/internal/quic_tls.h
index 133e247c26cedcd876831ef8cc8146ea05534c0e..9c5fa9cd5ac3e5c2b91e0d6f90d3251773208606 100644 (file)
--- a/include/internal/quic_tls.h
+++ b/include/internal/quic_tls.h
@@ -94,4 +94,9 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls);
 int ossl_quic_tls_set_transport_params(QUIC_TLS *qtls,
                                        const unsigned char *transport_params,
                                        size_t transport_params_len);
+
+int ossl_quic_tls_get_error(QUIC_TLS *qtls,
+                            uint64_t *error_code,
+                            const char **error_msg);
+
 #endif

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 4aba4a3b267cd8de8085cbb45c0b2716b65d427f..f46b20f5c6c4115cb662d67a313ed43d32c67a6c 100644 (file)
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -1599,6 +1599,8 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
     OSSL_TIME now, deadline;
     QUIC_CHANNEL *ch = arg;
     int channel_only = (flags & QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY) != 0;
+    uint64_t error_code;
+    const char *error_msg;
 
     /*
      * When we tick the QUIC connection, we do everything we need to do
@@ -1651,9 +1653,14 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
              * generate new outgoing data.
              */
             ch->have_new_rx_secret = 0;
-            if (!channel_only)
+            if (!channel_only) {
                 ossl_quic_tls_tick(ch->qtls);
 
+                if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg))
+                    ossl_quic_channel_raise_protocol_error(ch, error_code, 0,
+                                                           error_msg);
+            }
+
             /*
              * If the handshake layer gave us a new secret, we need to do RX
              * again because packets that were not previously processable and

diff --git a/ssl/quic/quic_tls.c b/ssl/quic/quic_tls.c
index 5a0b320b5c1a5618d89a97892a24e9dd775ecec2..05baa32b41b92605c10f858df52015df48e4fc7f 100644 (file)
--- a/ssl/quic/quic_tls.c
+++ b/ssl/quic/quic_tls.c
@@ -10,6 +10,7 @@
 #include "internal/recordmethod.h"
 #include "internal/quic_tls.h"
 #include "../ssl_local.h"
+#include "internal/quic_error.h"
 
 #define QUIC_TLS_FATAL(rl, ad, err) \
     do { \
@@ -28,6 +29,18 @@ struct quic_tls_st {
     const unsigned char *local_transport_params;
     size_t local_transport_params_len;
 
+    /*
+     * QUIC error code (usually in the TLS Alert-mapped CRYPTO_ERR range). Valid
+     * only if inerror is 1.
+     */
+    uint64_t error_code;
+
+    /*
+     * Error message with static storage duration. Valid only if inerr is 1.
+     * Should be suitable for encapsulation in a CONNECTION_CLOSE frame.
+     */
+    const char *error_msg;
+
     /* Whether our SSL object for TLS has been configured for use in QUIC */
     unsigned int configured : 1;
 
@@ -628,6 +641,20 @@ void ossl_quic_tls_free(QUIC_TLS *qtls)
     OPENSSL_free(qtls);
 }
 
+static int raise_error(QUIC_TLS *qtls, uint64_t error_code,
+                       const char *error_msg)
+{
+    qtls->error_code = error_code;
+    qtls->error_msg  = error_msg;
+    qtls->inerror    = 1;
+    return 0;
+}
+
+static int raise_internal_error(QUIC_TLS *qtls)
+{
+    return raise_error(qtls, QUIC_ERR_INTERNAL_ERROR, "internal error");
+}
+
 int ossl_quic_tls_tick(QUIC_TLS *qtls)
 {
     int ret;
@@ -656,20 +683,15 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls)
 
         /* ALPN is a requirement for QUIC and must be set */
         if (qtls->args.is_server) {
-            if (sctx->ext.alpn_select_cb == NULL) {
-                qtls->inerror = 1;
-                return 0;
-            }
+            if (sctx->ext.alpn_select_cb == NULL)
+                return raise_internal_error(qtls);
         } else {
-            if (sc->ext.alpn == NULL || sc->ext.alpn_len == 0) {
-                qtls->inerror = 1;
-                return 0;
-            }
-        }
-        if (!SSL_set_min_proto_version(qtls->args.s, TLS1_3_VERSION)) {
-            qtls->inerror = 1;
-            return 0;
+            if (sc->ext.alpn == NULL || sc->ext.alpn_len == 0)
+                return raise_internal_error(qtls);
         }
+        if (!SSL_set_min_proto_version(qtls->args.s, TLS1_3_VERSION))
+            return raise_internal_error(qtls);
+
         SSL_clear_options(qtls->args.s, SSL_OP_ENABLE_MIDDLEBOX_COMPAT);
         ossl_ssl_set_custom_record_layer(sc, &quic_tls_record_method, qtls);
 
@@ -682,16 +704,12 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls)
                                             | SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
                                             add_transport_params_cb,
                                             free_transport_params_cb, qtls,
-                                            parse_transport_params_cb, qtls)) {
-            qtls->inerror = 1;
-            return 0;
-        }
+                                            parse_transport_params_cb, qtls))
+            return raise_internal_error(qtls);
 
         nullbio = BIO_new(BIO_s_null());
-        if (nullbio == NULL) {
-            qtls->inerror = 1;
-            return 0;
-        }
+        if (nullbio == NULL)
+            return raise_internal_error(qtls);
 
         /*
          * Our custom record layer doesn't use the BIO - but libssl generally
@@ -721,18 +739,17 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls)
         case SSL_ERROR_WANT_WRITE:
             return 1;
         default:
-            qtls->inerror = 1;
-            return 0;
+            return raise_internal_error(qtls);
         }
     }
 
     if (!qtls->complete) {
         /* Validate that we have ALPN */
         SSL_get0_alpn_selected(qtls->args.s, &alpn, &alpnlen);
-        if (alpn == NULL || alpnlen == 0) {
-            qtls->inerror = 1;
-            return 0;
-        }
+        if (alpn == NULL || alpnlen == 0)
+            return raise_error(qtls, QUIC_ERR_CRYPTO_NO_APP_PROTO,
+                               "no application protocol negotiated");
+
         qtls->complete = 1;
         return qtls->args.handshake_complete_cb(qtls->args.handshake_complete_cb_arg);
     }
@@ -748,3 +765,15 @@ int ossl_quic_tls_set_transport_params(QUIC_TLS *qtls,
     qtls->local_transport_params_len   = transport_params_len;
     return 1;
 }
+
+int ossl_quic_tls_get_error(QUIC_TLS *qtls,
+                            uint64_t *error_code,
+                            const char **error_msg)
+{
+    if (qtls->inerror) {
+        *error_code = qtls->error_code;
+        *error_msg = qtls->error_msg;
+    }
+
+    return qtls->inerror;
+}