Feature suggestion: Additional check for SSL misconfiguration in stream proxy.

[Maxim Dounin]

Hello!

On Thu, Apr 30, 2020 at 07:59:41AM +0300, Andrey Kulikov wrote:

> Hello,
> 
> Consider following configuration:
> 
> stream {
>     server {
>         listen 5443;
>         proxy_pass my-tls-upstream:443;
>         proxy_ssl_verify on;
>         proxy_ssl_server_name on;
>         proxy_ssl_trusted_certificate trusted_root_CAs.cer;
>     }
> }  # end stream
> 
> It is perfectly Ok for nginx, though it doesn't do what one would
> expect it to - data being send to upstream server in plain text.
> This is due to the fact that proxy_ssl if off by default.
> So all proxy_ssl_* directives being ignored.
> 
> This looks kind of error-prone, as unlike in HTTP-proxy module, we
> can't specify schema for upstream connections.
> 
> Thus, one could expect nginx to complain about misconfiguration (using
> proxy_ssl_* without specifying proxy_ssl on; first), rather than
> silently send data in cleartext.
> 
> If patch with additional checks implementation for stream-proxy module
> will be submitted, are there any chances it could be considered for
> merging into upstream?

Unlikely.

The problem is that things like "proxy_ssl_verify" and 
"proxy_ssl_server_name" are optional configuration directives, and 
they, for example, might be set globally and inherited into the 
server in question, and/or set in an include file used for all 
servers.  Or simply used in a configuration where there are 
multiple endpoints switched manually, some with "proxy_ssl on;" 
and some in plain text.

Instead, consider checking that you have "proxy_ssl on;" in the 
configuration if you want nginx to use SSL with upstream servers 
in a stream server block.

Alternatively, we may want to consider something like "ssl://" 
pseudo scheme instead of "proxy_ssl on;", similarly to how it 
works in "proxy_pass https://..." in the http module, but this 
might not be a good idea either.

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