> On 21 Oct 2021, at 15:15, Roman Arutyunyan <arut@nginx.com> wrote:
>
> On Tue, Oct 19, 2021 at 01:07:56PM +0300, Sergey Kandaurov wrote:
>>
>> [..]
>> Below is alternative patch, it brings closer to how OCSP validation
>> is done with SSL_read_early_data(), with its inherent design flaws.
>> Namely, the case of regular SSL session reuse is still pessimized,
>> but that shouldn't bring further slowdown with ssl_ocsp disabled,
>> which is slow by itself.
>>
>> # HG changeset patch
>> # User Sergey Kandaurov <pluknet@nginx.com>
>> # Date 1634637049 -10800
>> # Tue Oct 19 12:50:49 2021 +0300
>> # Branch quic
>> # Node ID 6f26d6656b4ef97a3a245354bd7fa9e5c8671237
>> # Parent 1798acc01970ae5a03f785b7679fe34c32adcfea
>> QUIC: speeding up processing 0-RTT.
>>
>> After fe919fd63b0b, processing QUIC streams was postponed until after handshake
>> completion, which means that 0-RTT is effectively off. With ssl_ocsp enabled,
>> it could be further delayed. This differs to how SSL_read_early_data() works.
>
> differs FROM ?
>
>> This change unlocks processing streams on successful 0-RTT packet decryption.
>>

Both forms seem to be used, but "differs to" looks less popular.
Rewrote it this way:

This differs from how OCSP validation works with SSL_read_early_data().

>> diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
>> --- a/src/event/quic/ngx_event_quic.c
>> +++ b/src/event/quic/ngx_event_quic.c
>> @@ -989,6 +989,21 @@ ngx_quic_process_payload(ngx_connection_
>> }
>> }
>>
>> + if (pkt->level == ssl_encryption_early_data && !qc->streams.initialized) {
>> + rc = ngx_ssl_ocsp_validate(c);
>> +
>> + if (rc == NGX_ERROR) {
>> + return NGX_ERROR;
>> + }
>> +
>> + if (rc == NGX_AGAIN) {
>> + c->ssl->handler = ngx_quic_init_streams;
>> +
>> + } else {
>> + ngx_quic_init_streams(c);
>> + }
>> + }
>> +
>> if (pkt->level == ssl_encryption_handshake) {
>> /*
>> * RFC 9001, 4.9.1. Discarding Initial Keys
>> 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
>> @@ -463,6 +463,11 @@ ngx_quic_crypto_input(ngx_connection_t *
>> return NGX_ERROR;
>> }
>>
>> + if (qc->streams.initialized) {
>> + /* done while processing 0-RTT */
>> + return NGX_OK;
>> + }
>> +
>> rc = ngx_ssl_ocsp_validate(c);
>>
>> if (rc == NGX_ERROR) {
>>
>
> It would be nice to always call ngx_ssl_ocsp_validate() from the same source
> file (presumably ngx_event_quic_ssl.c). But this does not seem to occur
> naturally so let's leave it as it is.
>
> Looks good.
>
> PS: Also, this can be further refactored to move ngx_ssl_ocsp_validate() inside
> ngx_quic_init_streams(). In this case we can only call ngx_quic_init_streams()
> both times.

This is feasible, if init streams closer to obtaining 0-RTT secret.
Actually, it is even better, I believe, and it's invoked just once
regardless of the number of 0-RTT packets.
Requirement for successful 0-RTT decryption doesn't buy us much.

N.B. I decided to leave in place "quic init streams" debug.
This is where streams are now actually initialized,
and it looks reasonable to see that logged only once.

# HG changeset patch
# User Sergey Kandaurov <pluknet@nginx.com>
# Date 1634832181 -10800
# Thu Oct 21 19:03:01 2021 +0300
# Branch quic
# Node ID 11119f9fda599c890a93b348310f582e3c49ebb7
# Parent 1798acc01970ae5a03f785b7679fe34c32adcfea
QUIC: refactored OCSP validation in preparation for 0-RTT support.

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
@@ -361,7 +361,6 @@ static ngx_int_t
ngx_quic_crypto_input(ngx_connection_t *c, ngx_chain_t *data)
{
int n, sslerr;
- ngx_int_t rc;
ngx_buf_t *b;
ngx_chain_t *cl;
ngx_ssl_conn_t *ssl_conn;
@@ -463,19 +462,10 @@ ngx_quic_crypto_input(ngx_connection_t *
return NGX_ERROR;
}

- rc = ngx_ssl_ocsp_validate(c);
-
- if (rc == NGX_ERROR) {
+ if (ngx_quic_init_streams(c) != NGX_OK) {
return NGX_ERROR;
}

- if (rc == NGX_AGAIN) {
- c->ssl->handler = ngx_quic_init_streams;
- return NGX_OK;
- }
-
- ngx_quic_init_streams(c);
-
return NGX_OK;
}

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
@@ -16,6 +16,7 @@
static ngx_quic_stream_t *ngx_quic_create_client_stream(ngx_connection_t *c,
uint64_t id);
static ngx_int_t ngx_quic_init_stream(ngx_quic_stream_t *qs);
+static void ngx_quic_init_streams_handler(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);
@@ -369,9 +370,37 @@ ngx_quic_init_stream(ngx_quic_stream_t *
}


-void
+ngx_int_t
ngx_quic_init_streams(ngx_connection_t *c)
{
+ ngx_int_t rc;
+ ngx_quic_connection_t *qc;
+
+ qc = ngx_quic_get_connection(c);
+
+ if (qc->streams.initialized) {
+ return NGX_OK;
+ }
+
+ rc = ngx_ssl_ocsp_validate(c);
+
+ if (rc == NGX_ERROR) {
+ return NGX_ERROR;
+ }
+
+ if (rc == NGX_AGAIN) {
+ c->ssl->handler = ngx_quic_init_streams_handler;
+ return NGX_OK;
+ }
+
+ ngx_quic_init_streams_handler(c);
+
+ return NGX_OK;
+}
+
+static void
+ngx_quic_init_streams_handler(ngx_connection_t *c)
+{
ngx_queue_t *q;
ngx_quic_stream_t *qs;
ngx_quic_connection_t *qc;
diff --git a/src/event/quic/ngx_event_quic_streams.h b/src/event/quic/ngx_event_quic_streams.h
--- a/src/event/quic/ngx_event_quic_streams.h
+++ b/src/event/quic/ngx_event_quic_streams.h
@@ -31,7 +31,7 @@ ngx_int_t ngx_quic_handle_stop_sending_f
ngx_int_t ngx_quic_handle_max_streams_frame(ngx_connection_t *c,
ngx_quic_header_t *pkt, ngx_quic_max_streams_frame_t *f);

-void ngx_quic_init_streams(ngx_connection_t *c);
+ngx_int_t ngx_quic_init_streams(ngx_connection_t *c);
void ngx_quic_rbtree_insert_stream(ngx_rbtree_node_t *temp,
ngx_rbtree_node_t *node, ngx_rbtree_node_t *sentinel);
ngx_quic_stream_t *ngx_quic_find_stream(ngx_rbtree_t *rbtree,
# HG changeset patch
# User Sergey Kandaurov <pluknet@nginx.com>
# Date 1634832186 -10800
# Thu Oct 21 19:03:06 2021 +0300
# Branch quic
# Node ID b53e361bee7dfbb027507a717e6648234a06ef13
# Parent 11119f9fda599c890a93b348310f582e3c49ebb7
QUIC: speeding up processing 0-RTT.

After fe919fd63b0b, processing QUIC streams was postponed until after handshake
completion, which means that 0-RTT is effectively off. With ssl_ocsp enabled,
it could be further delayed. This differs from how OCSP validation works with
SSL_read_early_data(). With this change, processing QUIC streams is unlocked
when obtaining 0-RTT secret.

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
@@ -71,8 +71,20 @@ ngx_quic_set_read_secret(ngx_ssl_conn_t
secret_len, rsecret);
#endif

- return ngx_quic_keys_set_encryption_secret(c->pool, 0, qc->keys, level,
- cipher, rsecret, secret_len);
+ if (ngx_quic_keys_set_encryption_secret(c->pool, 0, qc->keys, level,
+ cipher, rsecret, secret_len)
+ != 1)
+ {
+ return 0;
+ }
+
+ if (level == ssl_encryption_early_data) {
+ if (ngx_quic_init_streams(c) != NGX_OK) {
+ return 0;
+ }
+ }
+
+ return 1;
}


@@ -131,6 +143,10 @@ ngx_quic_set_encryption_secrets(ngx_ssl_
}

if (level == ssl_encryption_early_data) {
+ if (ngx_quic_init_streams(c) != NGX_OK) {
+ return 0;
+ }
+
return 1;
}


--
Sergey Kandaurov

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel