On 2/20/2017 10:58 AM, William A Rowe Jr wrote: > On Sat, Feb 18, 2017 at 4:44 PM, Daniel Ruggeri <[email protected]> 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.)
No worries at all. Reviews are important. I am curious what you mean about -1 half vs -0 half, though. Is that -1.5 vs -.5? :-) > >> 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. Sorry to mix threads. I just replied a moment ago with a bit of reasoning behind the Optional use case. While it's possible that a server admin could mistakenly enable something they don't intend to or open things up more than they should, that's applicable any time someone enables some sort of authnz. I'm happy to reinforce this point in the docs for the Optional case but I still think enough utility is there to include it. > > 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. I'm not sure if multiple PROXY lines are permitted. Looking at section 4.1, I think the intent is that PROXY-aware servers would continue propagating the original client IP address in any PROXY headers it emits. For example, in the diagram in section 4.1, PX2 should emit a PROXY header to the backend server that has the client IP it received from the PROXY header in PX1. Ref: http://www.haproxy.org/download/1.8/doc/proxy-protocol.txt > 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. Yep, I follow the thought process and agree. This assumes that the intermediary isn't being clever or dumb by... * Sending the traffic as it received it (so not technically complying with any of the methods of propagating client and intermediary info) * Sending an appropriate 7239 header, but blindly passing X-Forwarded-For * Rewriting both headers to contain the same data in their expected formats FWIW, I feel the struggle of unwrapping all of this, too. At $dayjob, because of the potential silliness of various intermediaries, we chose to create a custom header that is always written (dropped if it comes to us) when our edge devices receive a connection. > > 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. Actually, you bring up a really good point that I had not explicitly considered. If a backend server is presented with a PROXY header AND an HTTP header (either a X-Forwarded-For or Forwarded), which header should it use? In theory there can be many intermediate hops that can add to the data. This is important for us on two fronts: * For mod_remoteip, we'd have to decide which to use. The current method is to prefer PROXY. * If we add PROXY support to mod_proxy, we have to decide which to propagate -- Daniel Ruggeri
