QUIC: add error code for handshake failed.

[Jiuzhou Cui]

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" <pluknet at nginx.com> wrote:
>
>> On 16 Feb 2023, at 17:36, Jiuzhou Cui <jiuzhoucui at 163.com> wrote:
>> 
>> Hello!
>> 
>> # HG changeset patch
>> # User Jiuzhou Cui <cuijiuzhou at 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 <pluknet at 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 at nginx.org
>https://mailman.nginx.org/mailman/listinfo/nginx-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20230217/6c7cd825/attachment.htm>