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?
At 2023-02-16 22:42:27, "Sergey Kandaurov" <pluk...@nginx.com> wrote:
>
>> On 16 Feb 2023, at 17:36, Jiuzhou Cui <jiuzhou...@163.com> wrote:
>>
>> Hello!
>>
>> # HG changeset patch
>> # User Jiuzhou Cui <cuijiuz...@alibaba-inc.com>
>> # Date 1676554419 -28800
>> # Thu Feb 16 21:33:39 2023 +0800
>> # Branch quic
>> # Node ID 13396c3ad10bdc8c1ac6969e965ceac510dc162f
>> # Parent b87a0dbc1150f415def5bc1e1f00d02b33519026
>> QUIC: add error code for handshake failed.
>>
>> diff -r b87a0dbc1150 -r 13396c3ad10b src/event/quic/ngx_event_quic_ssl.c
>> --- a/src/event/quic/ngx_event_quic_ssl.c Tue Oct 25 12:52:09 2022
>> +0400
>> +++ b/src/event/quic/ngx_event_quic_ssl.c Thu Feb 16 21:33:39 2023
>> +0800
>> @@ -202,7 +202,7 @@
>> SSL_get0_alpn_selected(ssl_conn, &alpn_data, &alpn_len);
>>
>> if (alpn_len == 0) {
>> - qc->error = 0x100 + SSL_AD_NO_APPLICATION_PROTOCOL;
>> + qc->error = NGX_QUIC_ERR_CRYPTO(SSL_AD_NO_APPLICATION_PROTOCOL);
>> qc->error_reason = "unsupported protocol in ALPN extension";
>>
>> ngx_log_error(NGX_LOG_INFO, c->log, 0,
>> @@ -452,6 +452,7 @@
>>
>> if (sslerr != SSL_ERROR_WANT_READ) {
>> ngx_ssl_error(NGX_LOG_ERR, c->log, 0, "SSL_do_handshake()
>> failed");
>> + qc->error = NGX_QUIC_ERR_CRYPTO(sslerr);
>> qc->error_reason = "handshake failed";
>> return NGX_ERROR;
>> }
>
>Thank you for the patch.
>
>Applying to TLS handshake, qc->error is used to keep CRYPTO_ERROR,
>a value based on the TLS alert. You are trying to set there
>something different, this is not going to work.
>
>More, qc->error is usually set in the send_alert callback as passed from
>TLS, so no need to deal with it here. Other places are QUIC protocol-
>specific additions to negotiate ALPN and to carry transport parameters,
>they are managed elsewhere.
>
>The ALPN part is ok. Looks like it was missed from the 97adb87f149b
>change, which went soon after ALPN checks added in a2c34e77cfc1.
>
># HG changeset patch
># User Sergey Kandaurov <pluk...@nginx.com>
># Date 1676558213 -14400
># Thu Feb 16 18:36:53 2023 +0400
># Branch quic
># Node ID 2fcd590d85da9c3a0205a18cb295ec316c03f18e
># Parent 12b756caaf167d2239fd3bd7a75b270ca89ca26b
>QUIC: using NGX_QUIC_ERR_CRYPTO macro in ALPN checks.
>
>Patch 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
>@@ -190,7 +190,7 @@ ngx_quic_add_handshake_data(ngx_ssl_conn
> SSL_get0_alpn_selected(ssl_conn, &alpn_data, &alpn_len);
>
> if (alpn_len == 0) {
>- qc->error = 0x100 + SSL_AD_NO_APPLICATION_PROTOCOL;
>+ qc->error = NGX_QUIC_ERR_CRYPTO(SSL_AD_NO_APPLICATION_PROTOCOL);
> qc->error_reason = "unsupported protocol in ALPN extension";
>
> ngx_log_error(NGX_LOG_INFO, c->log, 0,
>
>--
>Sergey Kandaurov
>_______________________________________________
>nginx-devel mailing list
>nginx-devel@nginx.org
>https://mailman.nginx.org/mailman/listinfo/nginx-devel
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
