QUIC: add error code for handshake failed.

[Sergey Kandaurov]

> On 17 Feb 2023, at 16:53, Sergey Kandaurov <pluknet at nginx.com> wrote:
> 
>> 
>> On 17 Feb 2023, at 09:17, Jiuzhou Cui <jiuzhoucui at 163.com> wrote:
>> 
>> Maybe NGX_QUIC_ERR_CRYPTO(sslerr) is not suitable here.
>> 
>> I think error and error_reason should be set together.
>> 
>> My scenes is error_reason is "handshake failed", but error is 8 set by parse TRANSPORT_PARAMETER failed before. My actual problem is parse transport parameter failed, so maybe error_reason "failed to process transport parameters" is useful for me to positioning problem. What do you think about this?
>> 
> 
> Actually, both error and error_reason are usually set together.
> The problem is in overwriting error_reason that was previously set
> specifically for transport parameters parsing error, during handshake,
> with a generic "handshake failed" phrase.  Please see the patch.
> 
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1676638271 -14400
> #      Fri Feb 17 16:51:11 2023 +0400
> # Branch quic
> # Node ID 09c7414487aa82b40d4ab33738dc578363fe3273
> # Parent  2fcd590d85da9c3a0205a18cb295ec316c03f18e
> QUIC: using most specific handshake error reasons.
> 
> A QUIC handshake may fail for a variety of reasons.  Some of them arise
> from our own checks and have a knownledge to set a suitable reason phrase.
> Previously, it was overridden with a generic "handshake failed" phrase.
> The latter is used for handshake errors generated in the SSL library and
> reported with the send_alert callback.  Now a specific phrase is preserved.
> 
> 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
> @@ -423,7 +423,11 @@ 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";
> +
> +            if (!qc->error_reason) {
> +                qc->error_reason = "handshake failed";
> +            }
> +
>             return NGX_ERROR;
>         }
>     }

Alternatively, the error_reason assignment can just be lifted.

# 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.

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