On Tue, Feb 21, 2023 at 05:54:42PM +0400, Sergey Kandaurov wrote:
[..]
> # HG changeset patch
> # User Sergey Kandaurov <pluk...@nginx.com>
> # Date 1676987626 -14400
> # Tue Feb 21 17:53:46 2023 +0400
> # Branch quic
> # Node ID e012c3999592fc935bfc786a232903567c512bfe
> # Parent baada8d864cbedc7557b879f89eee5c412f3ca85
> QUIC: moved "handshake failed" reason to send_alert.
>
> A QUIC handshake may fail for a variety of reasons, which breaks down into
> several cases, in order of precedence:
> - generally, the error code is reported with the send_alert callback
> - else, our own QUIC checks may set specific error code / reason phrase
> - as a last resort, handshake may fail for some reason, which falls back
> to send INTERNAL_ERROR in the CONNECTION_CLOSE frame.
> Now, in the first case, both error code and generic phrase are set in the
> send_alert callback, to preserve a specific reason phrase from overriding.
> Additionally, absent error code / reason phrase are now converted to just
> INTERNAL_ERROR, without reason phrase set.
i>
> Reported by Jiuzhou Cui.
I suggest the following commit message:
QUIC: moved "handshake failed" reason to send_alert.
A QUIC handshake may fail for a variety of reasons, which breaks down into
several cases, in order of precedence:
- a handshake error which leads to a send_alert call
- an error triggered by add_handshake_data callback
- internal errors (allocation etc)
Previously, in the first case, only error code was set in send_alert callback.
Now "handshake failed" is set there as a reason. In the second case, error
code and reason are set by add_handshake_data. In the last case, setting
error reason is now removed. Returning NGX_ERROR will lead to closing the
connection with just INTERNAL_ERROR.
Reported by Jiuzhou Cui.
> diff --git a/src/event/quic/ngx_event_quic_ssl.c
> b/src/event/quic/ngx_event_quic_ssl.c
> --- a/src/event/quic/ngx_event_quic_ssl.c
> +++ b/src/event/quic/ngx_event_quic_ssl.c
> @@ -301,6 +301,7 @@ ngx_quic_send_alert(ngx_ssl_conn_t *ssl_
> }
>
> qc->error = NGX_QUIC_ERR_CRYPTO(alert);
> + qc->error_reason = "handshake failed";
>
> return 1;
> }
> @@ -423,7 +424,6 @@ ngx_quic_crypto_input(ngx_connection_t *
>
> if (sslerr != SSL_ERROR_WANT_READ) {
> ngx_ssl_error(NGX_LOG_ERR, c->log, 0, "SSL_do_handshake()
> failed");
> - qc->error_reason = "handshake failed";
> return NGX_ERROR;
> }
> }
>
> --
> Sergey Kandaurov
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
The patch looks good.
--
Roman Arutyunyan
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
