Hello! On Mon, Jun 06, 2022 at 07:42:53PM +0400, Sergey Kandaurov wrote:
> > On 4 Jun 2022, at 04:09, Maxim Dounin <[email protected]> wrote: > > > > On Wed, May 25, 2022 at 12:05:57AM +0400, Sergey Kandaurov wrote: > > > >> # HG changeset patch > >> # User Sergey Kandaurov <[email protected]> > >> # Date 1653422583 -14400 > >> # Wed May 25 00:03:03 2022 +0400 > >> # Node ID 3bb1adbb74dfcd372f7369530967cfb415900778 > >> # Parent 8a54733c9d1290e6dc2f86af18e8a976a6352e4f > >> Upstream: handling of certificates specified as an empty string. > >> > >> Now, if the directive is given an empty string, such configuration cancels > >> loading of certificates should they be inherited from the previous level. > >> This restores a previous behaviour, before variables support in > >> certificates > >> was introduced (3ab8e1e2f0f7). > >> > >> diff --git a/src/http/modules/ngx_http_grpc_module.c > >> b/src/http/modules/ngx_http_grpc_module.c > >> --- a/src/http/modules/ngx_http_grpc_module.c > >> +++ b/src/http/modules/ngx_http_grpc_module.c > >> @@ -4921,7 +4921,7 @@ ngx_http_grpc_set_ssl(ngx_conf_t *cf, ng > >> return NGX_ERROR; > >> } > >> > >> - } else { > >> + } else if (glcf->upstream.ssl_certificate->value.len) { > >> if (ngx_ssl_certificate(cf, glcf->upstream.ssl, > >> &glcf->upstream.ssl_certificate->value, > >> > >> &glcf->upstream.ssl_certificate_key->value, > >> diff --git a/src/http/modules/ngx_http_proxy_module.c > >> b/src/http/modules/ngx_http_proxy_module.c > >> --- a/src/http/modules/ngx_http_proxy_module.c > >> +++ b/src/http/modules/ngx_http_proxy_module.c > >> @@ -4970,7 +4970,7 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n > >> return NGX_ERROR; > >> } > >> > >> - } else { > >> + } else if (plcf->upstream.ssl_certificate->value.len) { > >> if (ngx_ssl_certificate(cf, plcf->upstream.ssl, > >> &plcf->upstream.ssl_certificate->value, > >> > >> &plcf->upstream.ssl_certificate_key->value, > >> diff --git a/src/http/modules/ngx_http_uwsgi_module.c > >> b/src/http/modules/ngx_http_uwsgi_module.c > >> --- a/src/http/modules/ngx_http_uwsgi_module.c > >> +++ b/src/http/modules/ngx_http_uwsgi_module.c > >> @@ -2457,7 +2457,7 @@ ngx_http_uwsgi_set_ssl(ngx_conf_t *cf, n > >> return NGX_ERROR; > >> } > >> > >> - } else { > >> + } else if (uwcf->upstream.ssl_certificate->value.len) { > >> if (ngx_ssl_certificate(cf, uwcf->upstream.ssl, > >> &uwcf->upstream.ssl_certificate->value, > >> > >> &uwcf->upstream.ssl_certificate_key->value, > > > > It probably should in the outer if instead: > > > > diff --git a/src/http/modules/ngx_http_proxy_module.c > > b/src/http/modules/ngx_http_proxy_module.c > > --- a/src/http/modules/ngx_http_proxy_module.c > > +++ b/src/http/modules/ngx_http_proxy_module.c > > @@ -4955,8 +4955,9 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n > > return NGX_ERROR; > > } > > > > - if (plcf->upstream.ssl_certificate) { > > - > > + if (plcf->upstream.ssl_certificate > > + && plcf->upstream.ssl_certificate->value.len) > > + { > > if (plcf->upstream.ssl_certificate_key == NULL) { > > ngx_log_error(NGX_LOG_EMERG, cf->log, 0, > > "no \"proxy_ssl_certificate_key\" is defined " > > > > With the corresponding change to the upstream module: > > > > diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c > > --- a/src/http/ngx_http_upstream.c > > +++ b/src/http/ngx_http_upstream.c > > @@ -1690,8 +1690,10 @@ ngx_http_upstream_ssl_init_connection(ng > > } > > } > > > > - if (u->conf->ssl_certificate && (u->conf->ssl_certificate->lengths > > - || > > u->conf->ssl_certificate_key->lengths)) > > + if (u->conf->ssl_certificate > > + && u->conf->ssl_certificate->value.len > > + && (u->conf->ssl_certificate->lengths > > + || u->conf->ssl_certificate_key->lengths)) > > { > > if (ngx_http_upstream_ssl_certificate(r, u, c) != NGX_OK) { > > ngx_http_upstream_finalize_request(r, u, > > > > > > This is more in line with the previous logic, and will avoid > > errors in configurations like the following: > > > > proxy_ssl_certificate foo.crt; > > > > location / { > > proxy_pass https://...; > > proxy_ssl_certificate ""; > > } > > > > location /foo { > > proxy_pass https://...; > > proxy_ssl_certificate_key foo.key; > > } > > > > (Which is rather strange, but not very different from the idea of > > clearing proxy_ssl_certificate.) > > > > Agreed, thanks. Below is an updated patch > with minor commit message adjustment: > > # HG changeset patch > # User Sergey Kandaurov <[email protected]> > # Date 1654529998 -14400 > # Mon Jun 06 19:39:58 2022 +0400 > # Node ID 0e9638b52608a0e611dffaacad1fc8e54f3c156c > # Parent e0cfab501dd11fdd23ad492419692269d3a01fc7 > Upstream: handling of certificates specified as an empty string. > > Now, if the directive is given an empty string, such configuration cancels > loading of certificates, in particular, if they would be otherwise inherited > from the previous level. This restores previous behaviour, before variables > support in certificates was introduced (3ab8e1e2f0f7). > > diff --git a/src/http/modules/ngx_http_grpc_module.c > b/src/http/modules/ngx_http_grpc_module.c > --- a/src/http/modules/ngx_http_grpc_module.c > +++ b/src/http/modules/ngx_http_grpc_module.c > @@ -4906,8 +4906,9 @@ ngx_http_grpc_set_ssl(ngx_conf_t *cf, ng > return NGX_ERROR; > } > > - if (glcf->upstream.ssl_certificate) { > - > + if (glcf->upstream.ssl_certificate > + && glcf->upstream.ssl_certificate->value.len) > + { > if (glcf->upstream.ssl_certificate_key == NULL) { > ngx_log_error(NGX_LOG_EMERG, cf->log, 0, > "no \"grpc_ssl_certificate_key\" is defined " > diff --git a/src/http/modules/ngx_http_proxy_module.c > b/src/http/modules/ngx_http_proxy_module.c > --- a/src/http/modules/ngx_http_proxy_module.c > +++ b/src/http/modules/ngx_http_proxy_module.c > @@ -4955,8 +4955,9 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n > return NGX_ERROR; > } > > - if (plcf->upstream.ssl_certificate) { > - > + if (plcf->upstream.ssl_certificate > + && plcf->upstream.ssl_certificate->value.len) > + { > if (plcf->upstream.ssl_certificate_key == NULL) { > ngx_log_error(NGX_LOG_EMERG, cf->log, 0, > "no \"proxy_ssl_certificate_key\" is defined " > diff --git a/src/http/modules/ngx_http_uwsgi_module.c > b/src/http/modules/ngx_http_uwsgi_module.c > --- a/src/http/modules/ngx_http_uwsgi_module.c > +++ b/src/http/modules/ngx_http_uwsgi_module.c > @@ -2487,8 +2487,9 @@ ngx_http_uwsgi_set_ssl(ngx_conf_t *cf, n > return NGX_ERROR; > } > > - if (uwcf->upstream.ssl_certificate) { > - > + if (uwcf->upstream.ssl_certificate > + && uwcf->upstream.ssl_certificate->value.len) > + { > if (uwcf->upstream.ssl_certificate_key == NULL) { > ngx_log_error(NGX_LOG_EMERG, cf->log, 0, > "no \"uwsgi_ssl_certificate_key\" is defined " > diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c > --- a/src/http/ngx_http_upstream.c > +++ b/src/http/ngx_http_upstream.c > @@ -1690,8 +1690,10 @@ ngx_http_upstream_ssl_init_connection(ng > } > } > > - if (u->conf->ssl_certificate && (u->conf->ssl_certificate->lengths > - || > u->conf->ssl_certificate_key->lengths)) > + if (u->conf->ssl_certificate > + && u->conf->ssl_certificate->value.len > + && (u->conf->ssl_certificate->lengths > + || u->conf->ssl_certificate_key->lengths)) > { > if (ngx_http_upstream_ssl_certificate(r, u, c) != NGX_OK) { > ngx_http_upstream_finalize_request(r, u, > diff --git a/src/stream/ngx_stream_proxy_module.c > b/src/stream/ngx_stream_proxy_module.c > --- a/src/stream/ngx_stream_proxy_module.c > +++ b/src/stream/ngx_stream_proxy_module.c > @@ -1069,8 +1069,10 @@ ngx_stream_proxy_ssl_init_connection(ngx > } > } > > - if (pscf->ssl_certificate && (pscf->ssl_certificate->lengths > - || pscf->ssl_certificate_key->lengths)) > + if (pscf->ssl_certificate > + && pscf->ssl_certificate->value.len > + && (pscf->ssl_certificate->lengths > + || pscf->ssl_certificate_key->lengths)) > { > if (ngx_stream_proxy_ssl_certificate(s) != NGX_OK) { > ngx_stream_proxy_finalize(s, NGX_STREAM_INTERNAL_SERVER_ERROR); > @@ -2225,8 +2227,9 @@ ngx_stream_proxy_set_ssl(ngx_conf_t *cf, > return NGX_ERROR; > } > > - if (pscf->ssl_certificate) { > - > + if (pscf->ssl_certificate > + && pscf->ssl_certificate->value.len) > + { > if (pscf->ssl_certificate_key == NULL) { > ngx_log_error(NGX_LOG_EMERG, cf->log, 0, > "no \"proxy_ssl_certificate_key\" is defined " Looks good. -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
