[PATCH] Add proxy_protocol option to mail listener

[Maxim Dounin]

Hello!

On Fri, Jul 07, 2017 at 03:38:02PM +0200, Kees Bos wrote:

> # HG changeset patch
> # User Kees Bos <cornelis.bos at gmail.com>
> # Date 1499422505 0
> #      Fri Jul 07 10:15:05 2017 +0000
> # Node ID bc79b2baf494aabb889de1e5dbe3184ff0cb9bfa
> # Parent  70e65bf8dfd7a8d39aae8ac3a209d426e6947735
> Add proxy_protocol option to mail listener
> 
> Add support for the mail handlers. This enables the use of an upstream
> loadbalancer/proxy (like haproxy) that connects with the proxy protocol.
> 
> The original ip (as exposed with the proxy protocol) will be used as
> parameter for the 'Client-IP' in the authentication call.
> 
> Example config:
> mail {
>     server_name mail.example.com;
>     auth_http   localhost:9000/;
> 
>     server {
>         listen 143 proxy_protocol;
>         protocol imap;
>     }
> }

I see at least the following problems in this patch:

1. The implementation is not complete compared to other modules 
(http, stream): it only tries to change the address passed as 
Client-IP in auth_http, but not the address used for logging 
and/or for XCLIENT.

2. It unconditionally trusts all clients who can connect to the 
port in question.  This doesn't look wise.

Just for the reference, here is a review of an earlier attempt to 
introduce proxy_protocol support in the mail proxy:

http://mailman.nginx.org/pipermail/nginx-devel/2016-November/009084.html

[...]

> @@ -55,6 +56,7 @@
>      ngx_mail_conf_ctx_t    *ctx;
>      ngx_str_t               addr_text;
>      ngx_uint_t              ssl;    /* unsigned   ssl:1; */
> +    unsigned                proxy_protocol:1;

Note that "/* unsigned   ssl:1; */" comment here means that if we 
are going to add other bitfield options, the ssl field should be 
changed to appthis should be changed to appropriate bitfield.

[...]

-- 
Maxim Dounin
http://nginx.org/