Hello!
On Fri, Jul 07, 2017 at 03:38:02PM +0200, Kees Bos wrote:
> # HG changeset patch
> # User Kees Bos <cornelis....@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/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
