> On 17 Feb 2023, at 16:53, Sergey Kandaurov <pluk...@nginx.com> wrote:
>
>>
>> On 17 Feb 2023, at 09:17, Jiuzhou Cui <jiuzhou...@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 <pluk...@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 <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.
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
