Hello!

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

> On Nov 9, 2016 19:20, "Maxim Dounin" <mdounin@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/

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel