On Fri, Jun 9, 2017 at 4:17 AM, Sander Hoentjen <san...@hoentjen.eu> wrote: > On 06/08/2017 07:30 PM, Daniel Ruggeri wrote: >> Hi, all; >> With the proposal to T&R set for Monday, I wanted to draw attention to >> the PROXY protocol proposal in STATUS. Just hoping for a quick review. >> I know it appears to be a large change, but as I worked through the >> feedback, ten of the commits effectively got coded out. What we are >> left with is essentially just the donated code + safety around IPv6 + >> the ability to designate subnets that do not get PROXY processing. > > [...] I still believe it would be better to specify enabling > Proxy Protocol on a server, not vhost level. Because well, once you > enable it in one vhost it gets enabled for all vhosts using that port/ip > combination. > > Here is what I said before about it: > > Right now the patch proposes RemoteIPProxyProtocol inside a vhost config, but > wouldn't it be better (since it is connection-specific) to have something > like a ProxyProtocolListen directive? Where you say instead of: > ------ > <VirtualHost 127.0.0.1:9001> > RemoteIPProxyProtocol On > </VirtualHost> > ------ > Something like: > ------ > ProxyProtocolListen 127.0.0.1:9001 > or > ProxyProtocolEnable 127.0.0.1:9001 > ------ > > IMHO this is much cleaner than within a vhost (because that has side-effects > on other vhosts as well)
As this lives in mod_remoteip (for better or worse) let's look at what context mod_remoteip is configured in; we set up a list of those local or global *client* IP's which we trust to provide legit x-f-f (or remote-ip or otherly named) true IP address header fields. in the PROXY protocol case, we configure which *client* IP's which we *require* to submit a PROXY protocol line. Right now, we do that as a RemoteIPProxyProtocolExceptions list of those which we do *not* allow to submit a PROXY protocol line. I proposed we make the config simpler, in theory, by listing those we will trust. To your example, the *global* config line; RemoteIPProxyProtocol 127.0.0.1 [or 127.0.0.0/24] would configure all locally routed *client* requests, irrespective of which by-IP vhost, to require the PROXY protocol line. Requests from other hosts would be denied. I think that's sufficient. But if we wanted to implement your basic idea, we would still have the complication that we need to infer whether 9001 is a http, https, or h2 listener following the PROXY line processing. Your proposed syntax didn't really touch on that. It is still possible to override behavior by-vhost (ip-based, we are unprepared to read the TLS SNI or Host header at that point) but I don't see any application to do so. A given client is either an haproxy or similar, or it is not.
