performance is affected after merge OCSP changeset

[Roman Arutyunyan]

On Tue, Oct 19, 2021 at 01:07:56PM +0300, Sergey Kandaurov wrote:
> 
> > On 12 Oct 2021, at 14:31, Sergey Kandaurov <pluknet at nginx.com> wrote:
> > 
> > 
> >> On 12 Oct 2021, at 10:41, sun edward <sunzhiyong3210 at gmail.com> wrote:
> >> 
> >> Hi, 
> >>    There is a changeset fe919fd63b0b  "client certificate validation with OCSP" , after merge this changeset, the performance seems not as good as before, the avg response time increased about 50~60ms.  is there a way to optimize this problem?
> >> 
> > 
> > Are you referring to processing 0-RTT HTTP/3 requests?
> > 
> > Anyway, please try this change and report back.
> > 
> > # HG changeset patch
> > # User Sergey Kandaurov <pluknet at nginx.com>
> > # Date 1634038108 -10800
> > #      Tue Oct 12 14:28:28 2021 +0300
> > # Branch quic
> > # Node ID af4bd86814fdd0a2da3f7b8a965c41923ebeedd5
> > # Parent  9d47948842a3fd1c658a9676e638ef66207ffdcd
> > QUIC: speeding up processing 0-RTT.
> > 
> > After fe919fd63b0b, processing 0-RTT was postponed until after handshake
> > completion (typically seen as 2-RTT), including both ssl_ocsp on and off.
> > This change allows to start OCSP checks with reused SSL handshakes,
> > which eliminates 1 additional RTT allowing to process 0-RTT as expected.
> > 
> > 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
> > @@ -410,6 +410,10 @@ ngx_quic_crypto_input(ngx_connection_t *
> >             return NGX_ERROR;
> >         }
> > 
> > +        if (SSL_session_reused(c->ssl->connection)) {
> > +            goto ocsp;
> > +        }
> > +
> >         return NGX_OK;
> >     }
> > 
> > @@ -463,6 +467,7 @@ ngx_quic_crypto_input(ngx_connection_t *
> >         return NGX_ERROR;
> >     }
> > 
> > +ocsp:
> >     rc = ngx_ssl_ocsp_validate(c);
> > 
> >     if (rc == NGX_ERROR) {
> > 
> 
> 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 at 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.
> 
> 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) {
> 
> 
> -- 
> Sergey Kandaurov
> 
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

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.

-- 
Roman Arutyunyan