On 06/09/2017 03:29 PM, William A Rowe Jr wrote: > 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 not process the globally routed request for a PROXY > line. Now I'm not sure if I understand you. I agree with you that a whitelist is better than a blacklist, so adding
`RemoteIPProxyProtocol` as a list of *clients* that are allowed to use the *server/port* that is specified in `ProxyProtocolEnable` would make sense to me. > > 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. No because it doesn't need to. Because proxy protocol is prepended all that needs to be done is strip it, and store the information for later use. So you just configure that as normal. > > 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. > Well yes, I guess my main point is that the behaviour is set for a server_ip:port combination, so it doesn't work for name-based virtual hosts. That led me to the ProxyProtocolEnable idea.
