On Sat, Feb 18, 2017 at 4:44 PM, Daniel Ruggeri <drugg...@primary.net> wrote: > > Hi, Bill; > I've replied about the pre_connnection situation - hoping someone can > give the proposed patch a test as I don't have a handy H2 testbed.
Yup! Will review that thread - it's the -1 half (as opposed to a general -0 half for a 'pause' request while I was trying to get to reviewing the original commit.) > On the other comment, can you help me understand what redundant code is > happening per-request? When manipulating the request, there are only > four things happening differently: > 1. A check that we have data stored away from the connection filter > 2. A check that the connection data has a client IP > 3. The assignment of the data to the request_rec's structure and logging > at TRACE1 > 4. If no data was found, a check to see if it was optional and a logging > statement/return according to that result AIUI; the directives are all configured per-Server, the PROXY protocol data is fixed for the lifespan of the Connection. The PROXY protocol is significantly more binding that either x-f-f or even x-remoteip. I'm not even sure where the 'optional' scheme originated; if present when not allowed, that's a probable abuse pattern, and when not present when honored, that too indicates some malfunction and traffic shouldn't proceed IMO. I don't know that the optional list should be shipped, it's far too simple to create a completely insecure setup that won't raise eyebrows. The PROXY protocol reference spec states the connection (by origin or destination IP) follows the PROXY protocol, or it does not. Beyond that concern, I'm wondering if we shouldn't be using the *original* design of mod_remoteip, changing the conn_rec client_addr/client_ip (and null out remote_host/logname) and never alter it between requests. We can leave a conn pool note behind for the per-req processing, to retrieve the proxy IP into a req variable if desired doing the rest of the remoteip request phase, but the remaining per-req code and processing is near insignificant. Thoughts? > This should all be quite straight forward per request... In fact, it's a > much shorter logical path and less work than having to parse the > X-Forwarded-For header. So I was unspooling how we would handle stacked variables. Any PROXY protocol is the nearest hop; if multiple PROXY protocol header lines occurred, the closest would be transmitted first, etc. All local x-remoteip style values would be the next most distant hop; very similar to the haproxy protocol, it indicates some absolutely trusted edge router/balancer. Any x-f-f that occurs would reflect all the next most distant hops. Finally, any 'Forwarded' header (rfc7239) are the most distant hops. I'm basing that conclusion on the fact that all 'Forwarded'-aware intermediaries which construct a 'Forwarded' header would not carry the x-f-f, but concatenate these as closer than the nearest 'Forwarded'-aware hop. So the presence of an x-f-f header indicates the presence of a 'Forwarded'-unaware agent between this incoming connection and the closest 'Forwarded'-aware agent. I'm not suggesting these two enhancements, PROXY and RFC7239 are intertwined, we can certainly ship them in different releases, but I was having problems working out X-F-F vs Forwarded until I was working through the PROXY logic and came to the conclusion above, and am looking for others to sanity-check my logic on this.
