> 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;
}
}
>
> 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
--
Sergey Kandaurov
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
