Raise SSL_R_QUIC_PROTOCOL_ERROR on any QUIC protocol error

QUIC error code, frame type and reason is in error data Fixes #21337 Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Todd Short <todd.short@me.com> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #21476)
openssl · Jul 18, 2023 · 2b8126d · 2b8126d
1 parent a024ab9
commit 2b8126d
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 14 deletions.
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
@@ -1498,6 +1498,7 @@ SSL_R_PROTOCOL_IS_SHUTDOWN:207:protocol is shutdown
 SSL_R_PSK_IDENTITY_NOT_FOUND:223:psk identity not found
 SSL_R_PSK_NO_CLIENT_CB:224:psk no client cb
 SSL_R_PSK_NO_SERVER_CB:225:psk no server cb
+SSL_R_QUIC_PROTOCOL_ERROR:382:quic protocol error
 SSL_R_READ_BIO_NOT_SET:211:read bio not set
 SSL_R_READ_TIMEOUT_EXPIRED:312:read timeout expired
 SSL_R_RECORDS_NOT_RELEASED:321:records not released

diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
@@ -230,6 +230,7 @@
 # define SSL_R_PSK_IDENTITY_NOT_FOUND                     223
 # define SSL_R_PSK_NO_CLIENT_CB                           224
 # define SSL_R_PSK_NO_SERVER_CB                           225
+# define SSL_R_QUIC_PROTOCOL_ERROR                        382
 # define SSL_R_READ_BIO_NOT_SET                           211
 # define SSL_R_READ_TIMEOUT_EXPIRED                       312
 # define SSL_R_RECORDS_NOT_RELEASED                       321

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
@@ -2770,10 +2770,14 @@ void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch,
                                             const char *reason)
 {
     QUIC_TERMINATE_CAUSE tcause = {0};
+    int err_reason = error_code == QUIC_ERR_INTERNAL_ERROR
+                     ? ERR_R_INTERNAL_ERROR : SSL_R_QUIC_PROTOCOL_ERROR;
 
-    if (error_code == QUIC_ERR_INTERNAL_ERROR)
-        /* Internal errors might leave some errors on the stack. */
-        ch_save_err_state(ch);
+    ERR_raise_data(ERR_LIB_SSL, err_reason,
+                   "Error code: %llu Frame type: %llu Reason: %s",
+                   (unsigned long long) error_code,
+                   (unsigned long long) frame_type, reason);
+    ch_save_err_state(ch);
 
     tcause.error_code = error_code;
     tcause.frame_type = frame_type;

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
@@ -355,6 +355,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
     "psk identity not found"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_PSK_NO_CLIENT_CB), "psk no client cb"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_PSK_NO_SERVER_CB), "psk no server cb"},
+    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_QUIC_PROTOCOL_ERROR),
+    "quic protocol error"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_READ_BIO_NOT_SET), "read bio not set"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_READ_TIMEOUT_EXPIRED),
     "read timeout expired"},

diff --git a/test/quicfaultstest.c b/test/quicfaultstest.c
@@ -141,19 +141,9 @@ static int test_unknown_frame(void)
     if (!TEST_int_eq(SSL_get_error(cssl, ret), SSL_ERROR_SSL))
         goto err;
 
-#if 0
-    /*
-     * TODO(QUIC): We should expect an error on the queue after this - but we
-     * don't have it yet.
-     * Note, just raising the error in the obvious place causes
-     * SSL_handle_events() to succeed, but leave a spurious error on the stack.
-     * We need to either allow SSL_handle_events() to fail, or somehow delay the
-     * raising of the error until the SSL_read() call.
-     */
     if (!TEST_int_eq(ERR_GET_REASON(ERR_peek_error()),
-                     SSL_R_UNKNOWN_FRAME_TYPE_RECEIVED))
+                     SSL_R_QUIC_PROTOCOL_ERROR))
         goto err;
-#endif
 
     if (!TEST_true(qtest_check_server_frame_encoding_err(qtserv)))
         goto err;