[PATCH] Add proxy_protocol option to mail listener

[Maxim Dounin]

Hello!

On Tue, Jul 18, 2017 at 03:13:21PM +0200, Kees Bos wrote:

> 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?
> 

No.

Try looking at http and/or stream modules: "listen ...  
proxy_protocol" means that nginx will accept PROXY protocol 
header, and will make its contents available via the 
$proxy_protocol_addr and $proxy_protocol_port variables.

When "set_real_ip_from ...; real_ip_header proxy_protocol;" is 
additionally used, the address obtained from the PROXY protocol 
header will be used as a client address.

> > >     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?

I don't think so.

The word "Real" is misleading.  We don't know if it's real or not, 
it is up to the script to decide if the address should be trusted 
to use PROXY protocol.

Additionally, it doesn't describe the source of the information, 
so it is a) not clear how Client-IP is different from 
Client-Real-IP, and b) if a different source will be introduced 
(for example, XCLIENT), we will have to invent another way to name 
things.

The Proxy-Protocol-IP as proposed above is an attempt to provide 
something similar to $proxy_protocol_addr and Client-IP at the 
same time.

(Given that we currently don't provide Client-Port in auth_http, 
Proxy-Protocol-Port probably is a bad idea.)

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