[PATCH] Upstream: handling of certificates specified as an empty string

[Maxim Dounin]

Hello!

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.)

-- 
Maxim Dounin
http://mdounin.ru/