[PATCH] Add proxy_protocol option to mail listener

[Kees Bos]

Some inline stuff just to be sure I do understand what you mean.

On di, 2017-07-18 at 15:56 +0300, Maxim Dounin wrote:
> Hello!
> 
> On Tue, Jul 18, 2017 at 12:06:09PM +0200, Kees Bos wrote:
> 
> > 
> > # HG changeset patch
> > # User Kees Bos <cornelis.bos at gmail.com>
> > # Date 1500371531 0
> > #      Tue Jul 18 09:52:11 2017 +0000
> > # Node ID 8dd6050ca6858d9bea139067611ca5c69cfe8f18
> > # Parent  e3723f2a11b7ec1c196d59c331739bc21d9d9afd
> > 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 and as
> > address
> > in the XCLIENT call.
> > 
> > Optionally (if set), the real ips from the client that are using
> > the
> > proxy protocol can be restricted with "set_real_ip_from".
> This approach looks unsafe and counter-intuitive.
> 
> Instead, address should be changed if and only if there is 
> set_real_ip_from and it lists a particular client address, much 
> like it is done in http and stream modules.

So, "set_real_ip_from" is required as soon as "proxy_protocol" is used
in the listen directive.

Correct?


> 
> > 
> > 
> > Example config:
> > mail {
> >     server_name mail.example.com;
> >     auth_http   localhost:9000/;
> > 
> >     server {
> >         listen 143 proxy_protocol;
> >         protocol imap;
> >     }
> That is, only parsing of PROXY protocol header should happen here.

And the connection will be closed since "set_real_ip_from" is missing.

Correct?


> > 
> > 
> >     server {
> >         listen 25 proxy_protocol;
> >         protocol smtp;
> >         set_real_ip_from 127.0.0.0/8;
> >         set_real_ip_from ::/128;
> And here we can change client's address if a connection was from 
> listed addresses.
> 
> We may also consider sending the information from the header in 
> separate auth_http headers (something like Proxy-Protocol-IP, 
> Proxy-Protocol-Port?) regardless of set_real_ip_from.  But clearly 
> this should be a separate header from Client-IP to make it 
> possible for auth_http script to decide if this information should 
> be trusted or not.

Would an additional Client-Real-IP and Client-Real-Port be better?


> (There are also multiple style issues in the code.  Some are 
> outlined below, though I haven't focused on this as the code logic 
> is to be changed anyway.  Most of the comments apply to more than 
> one place.)

Tnx