On Tue, May 31, 2022 at 01:53:08AM +0400, Sergey Kandaurov wrote:
>
> > On 18 May 2022, at 10:57, Roman Arutyunyan <arut@nginx.com> wrote:
> >
> > # HG changeset patch
> > # User Roman Arutyunyan <arut@nginx.com>
> > # Date 1652856132 -14400
> > # Wed May 18 10:42:12 2022 +0400
> > # Branch quic
> > # Node ID a0f2d69f1fe43dfc718262235bf04d7b05f1fd68
> > # Parent 67ae4b649f2e38a44b245b7a842cf396c8250f02
> > QUIC: init_streams() callback.
> >
> > It's called after handshake completion to initialize application-level data
> > prior to creating streams.
>
> See below for 0-RTT case.
>
> >
> > HTTP/3 callback implementation switches main QUIC connection to idle and
> > reusable modes and sets keepalive timer.
> >
> > diff --git a/src/event/quic/ngx_event_quic.h b/src/event/quic/ngx_event_quic.h
> > --- a/src/event/quic/ngx_event_quic.h
> > +++ b/src/event/quic/ngx_event_quic.h
> > @@ -28,6 +28,9 @@
> > #define NGX_QUIC_STREAM_UNIDIRECTIONAL 0x02
> >
> >
> > +typedef ngx_int_t (*ngx_quic_init_streams_pt)(ngx_connection_t *c);
> > +
> > +
> > typedef enum {
> > NGX_QUIC_STREAM_SEND_READY = 0,
> > NGX_QUIC_STREAM_SEND_SEND,
> > @@ -74,6 +77,8 @@ typedef struct {
> > ngx_int_t stream_reject_code_uni;
> > ngx_int_t stream_reject_code_bidi;
> >
> > + ngx_quic_init_streams_pt init_streams;
> > +
> > u_char av_token_key[NGX_QUIC_AV_KEY_LEN];
> > u_char sr_token_key[NGX_QUIC_SR_KEY_LEN];
> > } ngx_quic_conf_t;
> > diff --git a/src/event/quic/ngx_event_quic_streams.c b/src/event/quic/ngx_event_quic_streams.c
> > --- a/src/event/quic/ngx_event_quic_streams.c
> > +++ b/src/event/quic/ngx_event_quic_streams.c
> > @@ -21,6 +21,7 @@ static ngx_quic_stream_t *ngx_quic_get_s
> > static ngx_int_t ngx_quic_reject_stream(ngx_connection_t *c, uint64_t id);
> > static void ngx_quic_init_stream_handler(ngx_event_t *ev);
> > static void ngx_quic_init_streams_handler(ngx_connection_t *c);
> > +static ngx_int_t ngx_quic_do_init_streams(ngx_connection_t *c);
> > static ngx_quic_stream_t *ngx_quic_create_stream(ngx_connection_t *c,
> > uint64_t id);
> > static void ngx_quic_empty_handler(ngx_event_t *ev);
> > @@ -571,15 +572,22 @@ ngx_quic_init_streams(ngx_connection_t *
> > return NGX_OK;
> > }
> >
> > - ngx_quic_init_streams_handler(c);
> > -
> > - return NGX_OK;
> > + return ngx_quic_do_init_streams(c);
> > }
> >
> >
> > static void
> > ngx_quic_init_streams_handler(ngx_connection_t *c)
> > {
> > + if (ngx_quic_do_init_streams(c) != NGX_OK) {
> > + ngx_quic_close_connection(c, NGX_ERROR);
> > + }
> > +}
> > +
> > +
> > +static ngx_int_t
> > +ngx_quic_do_init_streams(ngx_connection_t *c)
> > +{
> > ngx_queue_t *q;
> > ngx_quic_stream_t *qs;
> > ngx_quic_connection_t *qc;
> > @@ -588,6 +596,12 @@ ngx_quic_init_streams_handler(ngx_connec
> >
> > qc = ngx_quic_get_connection(c);
> >
> > + if (qc->conf->init_streams) {
> > + if (qc->conf->init_streams(c) != NGX_OK) {
> > + return NGX_ERROR;
> > + }
> > + }
> > +
> > for (q = ngx_queue_head(&qc->streams.uninitialized);
> > q != ngx_queue_sentinel(&qc->streams.uninitialized);
> > q = ngx_queue_next(q))
> > @@ -597,6 +611,8 @@ ngx_quic_init_streams_handler(ngx_connec
> > }
> >
> > qc->streams.initialized = 1;
> > +
> > + return NGX_OK;
> > }
> >
> >
> > diff --git a/src/http/v3/ngx_http_v3.c b/src/http/v3/ngx_http_v3.c
> > --- a/src/http/v3/ngx_http_v3.c
> > +++ b/src/http/v3/ngx_http_v3.c
> > @@ -17,21 +17,15 @@ static void ngx_http_v3_cleanup_session(
> > ngx_int_t
> > ngx_http_v3_init_session(ngx_connection_t *c)
> > {
> > - ngx_connection_t *pc;
> > ngx_pool_cleanup_t *cln;
> > ngx_http_connection_t *hc;
> > ngx_http_v3_session_t *h3c;
> >
> > - pc = c->quic->parent;
> > - hc = pc->data;
> > -
> > - if (hc->v3_session) {
> > - return NGX_OK;
> > - }
> > + hc = c->data;
> >
> > ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 init session");
> >
> > - h3c = ngx_pcalloc(pc->pool, sizeof(ngx_http_v3_session_t));
> > + h3c = ngx_pcalloc(c->pool, sizeof(ngx_http_v3_session_t));
> > if (h3c == NULL) {
> > goto failed;
> > }
> > @@ -42,16 +36,16 @@ ngx_http_v3_init_session(ngx_connection_
> > ngx_queue_init(&h3c->blocked);
> > ngx_queue_init(&h3c->pushing);
> >
> > - h3c->keepalive.log = pc->log;
> > - h3c->keepalive.data = pc;
> > + h3c->keepalive.log = c->log;
> > + h3c->keepalive.data = c;
> > h3c->keepalive.handler = ngx_http_v3_keepalive_handler;
> > h3c->keepalive.cancelable = 1;
> >
> > - h3c->table.send_insert_count.log = pc->log;
> > - h3c->table.send_insert_count.data = pc;
> > + h3c->table.send_insert_count.log = c->log;
> > + h3c->table.send_insert_count.data = c;
> > h3c->table.send_insert_count.handler = ngx_http_v3_inc_insert_count_handler;
> >
> > - cln = ngx_pool_cleanup_add(pc->pool, 0);
> > + cln = ngx_pool_cleanup_add(c->pool, 0);
> > if (cln == NULL) {
> > goto failed;
> > }
> > diff --git a/src/http/v3/ngx_http_v3.h b/src/http/v3/ngx_http_v3.h
> > --- a/src/http/v3/ngx_http_v3.h
> > +++ b/src/http/v3/ngx_http_v3.h
> > @@ -153,6 +153,7 @@ struct ngx_http_v3_session_s {
> >
> > void ngx_http_v3_init(ngx_connection_t *c);
> > void ngx_http_v3_reset_connection(ngx_connection_t *c);
> > +ngx_int_t ngx_http_v3_init_streams(ngx_connection_t *c);
> > ngx_int_t ngx_http_v3_init_session(ngx_connection_t *c);
> > ngx_int_t ngx_http_v3_check_flood(ngx_connection_t *c);
> >
> > diff --git a/src/http/v3/ngx_http_v3_module.c b/src/http/v3/ngx_http_v3_module.c
> > --- a/src/http/v3/ngx_http_v3_module.c
> > +++ b/src/http/v3/ngx_http_v3_module.c
> > @@ -249,6 +249,8 @@ ngx_http_v3_create_srv_conf(ngx_conf_t *
> > h3scf->quic.stream_reject_code_bidi = NGX_HTTP_V3_ERR_REQUEST_REJECTED;
> > h3scf->quic.active_connection_id_limit = NGX_CONF_UNSET_UINT;
> >
> > + h3scf->quic.init_streams = ngx_http_v3_init_streams;
> > +
> > return h3scf;
> > }
> >
> > diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> > --- a/src/http/v3/ngx_http_v3_request.c
> > +++ b/src/http/v3/ngx_http_v3_request.c
> > @@ -93,11 +93,6 @@ ngx_http_v3_init(ngx_connection_t *c)
> > }
> > #endif
> >
> > - if (ngx_http_v3_init_session(c) != NGX_OK) {
> > - ngx_http_close_connection(c);
> > - return;
> > - }
> > -
> > if (c->quic->id & NGX_QUIC_STREAM_UNIDIRECTIONAL) {
> > ngx_http_v3_init_uni_stream(c);
> >
> > @@ -107,6 +102,43 @@ ngx_http_v3_init(ngx_connection_t *c)
> > }
> >
> >
> > +ngx_int_t
> > +ngx_http_v3_init_streams(ngx_connection_t *c)
> > +{
> > + ngx_http_v3_session_t *h3c;
> > + ngx_http_connection_t *hc;
> > + ngx_http_v3_srv_conf_t *h3scf;
> > + ngx_http_core_loc_conf_t *clcf;
> > +
> > + if (ngx_terminate || ngx_exiting) {
> > + return NGX_ERROR;
> > + }
> > +
> > + hc = c->data;
> > +
> > + h3scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v3_module);
> > +
> > +#if (NGX_HTTP_V3_HQ)
> > + if (h3scf->hq) {
> > + return NGX_OK;
> > + }
> > +#endif
>
> Clang complains about unused variables:
>
> diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> --- a/src/http/v3/ngx_http_v3_request.c
> +++ b/src/http/v3/ngx_http_v3_request.c
> @@ -106,22 +106,26 @@ ngx_int_t
> ngx_http_v3_init_streams(ngx_connection_t *c)
> {
> ngx_http_v3_session_t *h3c;
> +#if (NGX_HTTP_V3_HQ)
> ngx_http_connection_t *hc;
> ngx_http_v3_srv_conf_t *h3scf;
> +#endif
> ngx_http_core_loc_conf_t *clcf;
>
> if (ngx_terminate || ngx_exiting) {
> return NGX_ERROR;
> }
>
> +#if (NGX_HTTP_V3_HQ)
> +
> hc = c->data;
>
> h3scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v3_module);
>
> -#if (NGX_HTTP_V3_HQ)
> if (h3scf->hq) {
> return NGX_OK;
> }
> +
> #endif

Thanks, fixed.

> if (ngx_http_v3_init_session(c) != NGX_OK) {
>
> > +
> > + if (ngx_http_v3_init_session(c) != NGX_OK) {
> > + return NGX_ERROR;
> > + }
>
> If 0-RTT data is accepted, ngx_http_v3_init_session() is called too early,
> before SSL_do_handshake() is used to apply client transport parameters.
> In particular that means inability to open a server unidirectional stream.

Discussed this. The issue manifests itself with QuicTLS because of callback
order. With BoringSSL is ok.

I suggest another patch that relocates ngx_quic_init_streams() call from
where we receive early keys to where we successfully decrypt the first
early data packet. This relocation should fix the issue for all libraries.

> > +
> > + c->idle = 1;
> > + ngx_reusable_connection(c, 1);
> > +
> > + h3c = ngx_http_v3_get_session(c);
> > + clcf = ngx_http_v3_get_module_loc_conf(c, ngx_http_core_module);
> > + ngx_add_timer(&h3c->keepalive, clcf->keepalive_timeout);
> > +
> > + return NGX_OK;
> > +}
> > +
> > +
> > #if (NGX_HTTP_V3_HQ)
> >
> > static void
> >
>
> --
> Sergey Kandaurov
>
> _______________________________________________
> nginx-devel mailing list -- nginx-devel@nginx.org
> To unsubscribe send an email to nginx-devel-leave@nginx.org
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org