QUIC: add error code for handshake failed.

Roman Arutyunyan arut at nginx.com
Wed Feb 22 13:15:46 UTC 2023


On Tue, Feb 21, 2023 at 05:54:42PM +0400, Sergey Kandaurov wrote:

[..]

> # HG changeset patch
> # User Sergey Kandaurov <pluknet at 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 at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

The patch looks good.

--
Roman Arutyunyan


More information about the nginx-devel mailing list