[PATCH] proxy-protocol dst variables and proxy-proxy-protocol

[Maxim Dounin]

Hello!

On Wed, Nov 09, 2016 at 08:52:14PM +0100, Bjørnar Ness wrote:

> On Nov 9, 2016 19:20, "Maxim Dounin" <mdounin at mdounin.ru> wrote:
> >
> > Hello!
> >
> > On Thu, Nov 03, 2016 at 08:37:04PM +0100, Bjørnar Ness wrote:
> >
> > > Maxim: what needs change to get this merged? Followup will be mail pp
> > > support, which I saw a patch for today from somone else.
> >
> > If this was a question to me, then the answer is:
> >
> > I'm not convinced this should be merged.  For the use case of
> > PROXY protocol bypass a more logical solution would be to avoid
> > removing the PROXY protocol header at all.  If one needs to bypass
> > it and extract the information at the same time, something similar
> > to ssl preread as recently implemented in the stream module might
> > be a better solution.
> 
> The usecase for getting access to the dst variables is that we have the:
> 
> external LB -> nginx -> foo
> 
> setup as mentioned earlier, and we want to take decisions based on the
> destination inside nginx, this may for example be a multi-brand setup, where
> the original destination address is a part of the query key to a database.
> This will allow us to just do database changes, and nginx will immediately
> be able to "see" for what destination this request was for.
> 
> The separate, but related usecase is the "proxy-proxy-protocol" usecase,
> where the upstream also needs the original ip/port src/destination data for
> logging or decisionmaking.
> 
> For this reason, I think it is practical to store the proxy protocol header as
> a whole with ptr's in the connection struct, but I am fine with changing this
> to inet_addr for example, will be more loc but a little less memory use.

The implementation of PROXY protocol in nginx basically mimics 
what is available via X-Forwarded-For in http.

Extending it with destination data may simplify some use cases.  
But it will also complicate things and will make other use cases 
less intuitive.  It is also more or less clear that destination 
can be identified using a separate destination within nginx 
itself.

As previously said, I'm not convinced this should be merged, as 
suggested changes have obvious downsides in common cases and only 
beneficial for very specific use cases.

> > If you have some ideas on how to properly support PROXY protocol
> > in mail - please comment on the relevant patch.
> 
> Reason for mentioning it here, is the mail proxy-protocol patch has the
> functionality mentioned here as a dependency. Both exim and dovecot
> understands proxy-protocol, and I want to pass it on after the auth is done,
> hence proxy-proxy-protocol.

Current question is:

What "listen ... proxy_protocol" should mean in case of mail.  In 
other modules, it just means that PROXY protocol header is parsed 
and appropriate variables are available for use.  It would be good 
to have similar meaning in mail, but there are realip module and 
no variables in mail.

In the stream module similar problem was resolved by not 
introducing "listen ... proxy_protocol" till variables support was 
added, and by adding realip module at the same time.  May be there 
are better options.

I certainly dislike what is currently suggested, that is, just 
passing an address provided via PROXY protocol to backends via 
XCLIENT.

Introducing PROXY protocol to backends instead of XCLIENT looks 
as a separate thing.

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