From 7468a10b62276be4adee0fcd6aaf6244270984ab Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Tue, 13 May 2025 20:12:10 +0400 Subject: [PATCH] QUIC: adjusted handling of callback errors. Changed handshake callbacks to always return success. This allows to avoid logging SSL_do_handshake() errors with empty or cryptic "internal error" OpenSSL error messages at the inappropriate "crit" log level. Further, connections with failed callbacks are closed now right away when using OpenSSL compat layer. This change supersedes and reverts c37fdcdd1, with the conditions to check callbacks invocation kept to slightly improve code readability of control flow; they are optimized out in the resulting assembly code. --- src/event/quic/ngx_event_quic.c | 3 +++ .../quic/ngx_event_quic_openssl_compat.c | 8 ++---- src/event/quic/ngx_event_quic_ssl.c | 27 ++++++++++++------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c index 4682ecad9..a4ad85d56 100644 --- a/src/event/quic/ngx_event_quic.c +++ b/src/event/quic/ngx_event_quic.c @@ -135,6 +135,9 @@ ngx_quic_apply_transport_params(ngx_connection_t *c, ngx_quic_tp_t *ctp) if (scid.len != ctp->initial_scid.len || ngx_memcmp(scid.data, ctp->initial_scid.data, scid.len) != 0) { + qc->error = NGX_QUIC_ERR_TRANSPORT_PARAMETER_ERROR; + qc->error_reason = "invalid initial_source_connection_id"; + ngx_log_error(NGX_LOG_INFO, c->log, 0, "quic client initial_source_connection_id mismatch"); return NGX_ERROR; diff --git a/src/event/quic/ngx_event_quic_openssl_compat.c b/src/event/quic/ngx_event_quic_openssl_compat.c index a4a8ea1b6..c5762f155 100644 --- a/src/event/quic/ngx_event_quic_openssl_compat.c +++ b/src/event/quic/ngx_event_quic_openssl_compat.c @@ -437,7 +437,7 @@ ngx_quic_compat_message_callback(int write_p, int version, int content_type, ngx_quic_level_name(level), len); if (com->method->add_handshake_data(ssl, level, buf, len) != 1) { - goto failed; + return; } break; @@ -451,7 +451,7 @@ ngx_quic_compat_message_callback(int write_p, int version, int content_type, ngx_quic_level_name(level), alert, len); if (com->method->send_alert(ssl, level, alert) != 1) { - goto failed; + return; } } @@ -459,10 +459,6 @@ ngx_quic_compat_message_callback(int write_p, int version, int content_type, } return; - -failed: - - ngx_post_event(&qc->close, &ngx_posted_events); } diff --git a/src/event/quic/ngx_event_quic_ssl.c b/src/event/quic/ngx_event_quic_ssl.c index 4f7060ce4..dd7ee3702 100644 --- a/src/event/quic/ngx_event_quic_ssl.c +++ b/src/event/quic/ngx_event_quic_ssl.c @@ -72,7 +72,7 @@ ngx_quic_set_read_secret(ngx_ssl_conn_t *ssl_conn, cipher, rsecret, secret_len) != NGX_OK) { - return 0; + qc->error = NGX_QUIC_ERR_INTERNAL_ERROR; } return 1; @@ -102,7 +102,7 @@ ngx_quic_set_write_secret(ngx_ssl_conn_t *ssl_conn, cipher, wsecret, secret_len) != NGX_OK) { - return 0; + qc->error = NGX_QUIC_ERR_INTERNAL_ERROR; } return 1; @@ -136,7 +136,8 @@ ngx_quic_set_encryption_secrets(ngx_ssl_conn_t *ssl_conn, cipher, rsecret, secret_len) != NGX_OK) { - return 0; + qc->error = NGX_QUIC_ERR_INTERNAL_ERROR; + return 1; } if (level == ssl_encryption_early_data) { @@ -153,7 +154,7 @@ ngx_quic_set_encryption_secrets(ngx_ssl_conn_t *ssl_conn, cipher, wsecret, secret_len) != NGX_OK) { - return 0; + qc->error = NGX_QUIC_ERR_INTERNAL_ERROR; } return 1; @@ -199,7 +200,7 @@ ngx_quic_add_handshake_data(ngx_ssl_conn_t *ssl_conn, ngx_log_error(NGX_LOG_INFO, c->log, 0, "quic unsupported protocol in ALPN extension"); - return 0; + return 1; } SSL_get_peer_quic_transport_params(ssl_conn, &client_params, @@ -216,7 +217,7 @@ ngx_quic_add_handshake_data(ngx_ssl_conn_t *ssl_conn, ngx_log_error(NGX_LOG_INFO, c->log, 0, "missing transport parameters"); - return 0; + return 1; } p = (u_char *) client_params; @@ -231,11 +232,11 @@ ngx_quic_add_handshake_data(ngx_ssl_conn_t *ssl_conn, qc->error = NGX_QUIC_ERR_TRANSPORT_PARAMETER_ERROR; qc->error_reason = "failed to process transport parameters"; - return 0; + return 1; } if (ngx_quic_apply_transport_params(c, &ctp) != NGX_OK) { - return 0; + return 1; } qc->client_tp_done = 1; @@ -245,12 +246,14 @@ ngx_quic_add_handshake_data(ngx_ssl_conn_t *ssl_conn, out = ngx_quic_copy_buffer(c, (u_char *) data, len); if (out == NGX_CHAIN_ERROR) { - return 0; + qc->error = NGX_QUIC_ERR_INTERNAL_ERROR; + return 1; } frame = ngx_quic_alloc_frame(c); if (frame == NULL) { - return 0; + qc->error = NGX_QUIC_ERR_INTERNAL_ERROR; + return 1; } frame->data = out; @@ -412,6 +415,10 @@ ngx_quic_crypto_input(ngx_connection_t *c, ngx_chain_t *data, ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL_do_handshake: %d", n); + if (qc->error != (ngx_uint_t) -1) { + return NGX_ERROR; + } + if (n <= 0) { sslerr = SSL_get_error(ssl_conn, n);