[PATCH] Upstream: handling of certificates specified as an empty string
Maxim Dounin
mdounin at mdounin.ru
Mon Jun 6 22:03:14 UTC 2022
Hello!
On Mon, Jun 06, 2022 at 07:42:53PM +0400, Sergey Kandaurov wrote:
> > On 4 Jun 2022, at 04:09, Maxim Dounin <mdounin at mdounin.ru> wrote:
> >
> > On Wed, May 25, 2022 at 12:05:57AM +0400, Sergey Kandaurov wrote:
> >
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet at nginx.com>
> >> # 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 <pluknet at nginx.com>
> # 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/
More information about the nginx-devel
mailing list