Hi,

On Thu, Mar 23, 2023 at 05:18:00PM +0400, Sergey Kandaurov wrote:
> On Wed, Feb 08, 2023 at 06:41:38PM +0400, Roman Arutyunyan wrote:
> > # HG changeset patch
> > # User Roman Arutyunyan <arut@nginx.com>
> > # Date 1675867049 -14400
> > # Wed Feb 08 18:37:29 2023 +0400
> > # Branch quic
> > # Node ID a3142c8833f5bf1186599e7938141f5062fac4a2
> > # Parent 3c33d39a51d334d99fcc7d2b45e8d8190c431492
> > QUIC: OpenSSL compatibility layer.
> >
> > The change allows to compile QUIC with OpenSSL which lacks BoringSSL QUIC API.
> >
> > This implementation does not support 0-RTT.
>
> [..]
>
> > diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
> > --- a/src/http/modules/ngx_http_ssl_module.c
> > +++ b/src/http/modules/ngx_http_ssl_module.c
> > @@ -9,6 +9,10 @@
> > #include <ngx_core.h>
> > #include <ngx_http.h>
> >
> > +#if (NGX_QUIC_OPENSSL_COMPAT)
> > +#include <ngx_event_quic_openssl_compat.h>
> > +#endif
> > +
> >
> > typedef ngx_int_t (*ngx_ssl_variable_handler_pt)(ngx_connection_t *c,
> > ngx_pool_t *pool, ngx_str_t *s);
> > @@ -1317,16 +1321,22 @@ ngx_http_ssl_init(ngx_conf_t *cf)
> > continue;
> > }
> >
> > + cscf = addr[a].default_server;
> > + sscf = cscf->ctx->srv_conf[ngx_http_ssl_module.ctx_index];
> > +
> > if (addr[a].opt.http3) {
> > name = "http3";
> >
> > +#if (NGX_QUIC_OPENSSL_COMPAT)
> > + if (ngx_quic_compat_init(cf, sscf->ssl.ctx) != NGX_OK) {
> > + return NGX_ERROR;
> > + }
> > +#endif
> > +
> > } else {
> > name = "ssl";
> > }
> >
> > - cscf = addr[a].default_server;
> > - sscf = cscf->ctx->srv_conf[ngx_http_ssl_module.ctx_index];
> > -
> > if (sscf->certificates) {
> >
> > if (addr[a].opt.http3 && !(sscf->protocols & NGX_SSL_TLSv1_3)) {
> > diff --git a/src/stream/ngx_stream_ssl_module.c b/src/stream/ngx_stream_ssl_module.c
> > --- a/src/stream/ngx_stream_ssl_module.c
> > +++ b/src/stream/ngx_stream_ssl_module.c
> > @@ -9,6 +9,10 @@
> > #include <ngx_core.h>
> > #include <ngx_stream.h>
> >
> > +#if (NGX_QUIC_OPENSSL_COMPAT)
> > +#include <ngx_event_quic_openssl_compat.h>
> > +#endif
> > +
> >
> > typedef ngx_int_t (*ngx_ssl_variable_handler_pt)(ngx_connection_t *c,
> > ngx_pool_t *pool, ngx_str_t *s);
> > @@ -1218,6 +1222,12 @@ ngx_stream_ssl_init(ngx_conf_t *cf)
> >
> > scf = listen[i].ctx->srv_conf[ngx_stream_ssl_module.ctx_index];
> >
> > +#if (NGX_QUIC_OPENSSL_COMPAT)
> > + if (ngx_quic_compat_init(cf, scf->ssl.ctx) != NGX_OK) {
> > + return NGX_ERROR;
> > + }
> > +#endif
> > +
> > if (scf->certificates && !(scf->protocols & NGX_SSL_TLSv1_3)) {
> > ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
> > "\"ssl_protocols\" must enable TLSv1.3 for "
>
> Following internal discussion with Roman, below is a fix for compat
> layer initialization. This is a fallout after moving initialization
> from ngx_ssl_create(). Although that change was justified, it has
> left a couple of issues:
> - SSL context can be NULL (normally caught later in function)
> - only the default server was initialized, breaks with SNI.
> The latter was caught by h3_ssl_reject_handshake.t
>
> The fix is to initialize all servers that have SSL context.
> The postconfiguration function is quite large, so this is moved
> to the separate function. While here, moved back assignments
> closer to the place of their use.
>
> # HG changeset patch
> # User Sergey Kandaurov <pluknet@nginx.com>
> # Date 1679577205 -14400
> # Thu Mar 23 17:13:25 2023 +0400
> # Branch quic
> # Node ID dcef88bdea77cf84b97580f21fbe05cc05ab631d
> # Parent f4279edda9fdbbd836a86926d86cdc727492e94c
> HTTP/3: fixed OpenSSL compatibility layer initialization.
>
> SSL context is not present if the default server has neither certificates nor
> ssl_reject_handshake enabled. Previously, this led to null pointer dereference
> before it would be caught with configuration checks.
>
> Additionally, virtual servers with distinct SSL contexts need to initialize
> compatibility layer in order to complete a QUIC handshake.
>
> diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
> --- a/src/http/modules/ngx_http_ssl_module.c
> +++ b/src/http/modules/ngx_http_ssl_module.c
> @@ -56,6 +56,10 @@ static char *ngx_http_ssl_conf_command_c
> void *data);
>
> static ngx_int_t ngx_http_ssl_init(ngx_conf_t *cf);
> +#if (NGX_QUIC_OPENSSL_COMPAT)
> +static ngx_int_t ngx_http_ssl_quic_compat_init(ngx_conf_t *cf,
> + ngx_http_conf_addr_t *addr);
> +#endif
>
>
> static ngx_conf_bitmask_t ngx_http_ssl_protocols[] = {
> @@ -1328,14 +1332,11 @@ ngx_http_ssl_init(ngx_conf_t *cf)
> continue;
> }
>
> - cscf = addr[a].default_server;
> - sscf = cscf->ctx->srv_conf[ngx_http_ssl_module.ctx_index];
> -
> if (addr[a].opt.quic) {
> name = "quic";
>
> #if (NGX_QUIC_OPENSSL_COMPAT)
> - if (ngx_quic_compat_init(cf, sscf->ssl.ctx) != NGX_OK) {
> + if (ngx_http_ssl_quic_compat_init(cf, &addr[a]) != NGX_OK) {
> return NGX_ERROR;
> }
> #endif
> @@ -1344,6 +1345,9 @@ ngx_http_ssl_init(ngx_conf_t *cf)
> name = "ssl";
> }
>
> + cscf = addr[a].default_server;
> + sscf = cscf->ctx->srv_conf[ngx_http_ssl_module.ctx_index];
> +
> if (sscf->certificates) {
>
> if (addr[a].opt.quic && !(sscf->protocols & NGX_SSL_TLSv1_3)) {
> @@ -1391,3 +1395,27 @@ ngx_http_ssl_init(ngx_conf_t *cf)
>
> return NGX_OK;
> }
> +
> +
> +static ngx_int_t
> +ngx_http_ssl_quic_compat_init(ngx_conf_t *cf, ngx_http_conf_addr_t *addr)
> +{
> + ngx_uint_t s;
> + ngx_http_ssl_srv_conf_t *sscf;
> + ngx_http_core_srv_conf_t **cscfp, *cscf;
> +
> + cscfp = addr->servers.elts;
> + for (s = 0; s < addr->servers.nelts; s++) {
> +
> + cscf = cscfp[s];
> + sscf = cscf->ctx->srv_conf[ngx_http_ssl_module.ctx_index];
> +
> + if (sscf->certificates || sscf->reject_handshake) {
> + if (ngx_quic_compat_init(cf, sscf->ssl.ctx) != NGX_OK) {
> + return NGX_ERROR;
> + }
> + }
> + }
> +
> + return NGX_OK;
> +}

Looks good
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel