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

Sergey Kandaurov pluknet at nginx.com
Mon Jun 6 15:42:53 UTC 2022


> On 4 Jun 2022, at 04:09, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> 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.)
> 

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 "

-- 
Sergey Kandaurov



More information about the nginx-devel mailing list