Olivier, Am 20.04.20 um 20:03 schrieb Olivier D: > I'm using gmail so I add to attach patches and was not able to send them > directly. If format is wrong, tell me :) >
Format looks good to me. Your commit message however does not (fully) follow the instructions within the CONTRIBUTING file (https://github.com/haproxy/haproxy/blob/dfad6a41ad9f012671b703788dd679cf24eb8c5a/CONTRIBUTING#L562-L567): > As a rule of thumb, your patch MUST NEVER be made only of a subject line, > it *must* contain a description. Even one or two lines, or indicating > whether a backport is desired or not. It turns out that single-line commits > are so rare in the Git world that they require special manual (hence > painful) handling when they are backported, and at least for this reason > it's important to keep this in mind. Regarding the patch itself: > diff --git doc/configuration.txt doc/configuration.txt > index 5d01835d7..ddfabcd92 100644 > --- doc/configuration.txt > +++ doc/configuration.txt > @@ -6735,7 +6735,8 @@ option forwardfor [ except <network> ] [ header <name> > ] [ if-none ] > header for a known source address or network by adding the "except" keyword > followed by the network address. In this case, any source IP matching the > network will not cause an addition of this header. Most common uses are > with > - private networks or 127.0.0.1. > + private networks or 127.0.0.1. Another way to do it is to tell HAProxy to > + trust a custom header with "http-request set-src". This change looks incorrect to me. "option forwardfor" is for sending, not "receiving" IP addresses. > Alternatively, the keyword "if-none" states that the header will only be > added if it is not present. This should only be used in perfectly trusted > @@ -6760,6 +6761,14 @@ option forwardfor [ except <network> ] [ header <name> > ] [ if-none ] > mode http > option forwardfor header X-Client > > + Example : > + # Trust a specific header and use it as origin IP. > + # If not found, source IP will be used. > + frontend www > + mode http > + http-request set-src CF-Connecting-IP I believe this should read `http-request set-src %[req.hdr(CF-Connecting-IP)]`. However: 1. I don't like having company specific headers in there. Especially since Cloudflare supports the standard XFF. 2. I don't consider that a useful addition. > + option forwardfor > + > See also : "option httpclose", "option http-server-close", > "option http-keep-alive" > Patch 2: > diff --git doc/configuration.txt doc/configuration.txt > index ddfabcd92..49324fa53 100644 > --- doc/configuration.txt > +++ doc/configuration.txt > @@ -5114,7 +5114,8 @@ http-request set-src <expr> [ { if | unless } > <condition> ] > This is used to set the source IP address to the value of specified > expression. Useful when a proxy in front of HAProxy rewrites source IP, but > provides the correct IP in a HTTP header; or you want to mask source IP for > - privacy. > + privacy. All subsequent calls to src field will return this value > + (see example). This change looks good to me. > Arguments : > <expr> Is a standard HAProxy expression formed by a sample-fetch > followed > @@ -5124,6 +5125,11 @@ http-request set-src <expr> [ { if | unless } > <condition> ] > http-request set-src hdr(x-forwarded-for) > http-request set-src src,ipmask(24) > > + Example: Only a single "Example:" heading is used throughout the documentation. As the first line can be shared with the previous example you could write something like: # After the masking this will track connections based on the IP address with the last octet zeroed out. > + # This will track connection based on header IP > + http-request set-src hdr(x-forwarded-for) > + http-request track-sc0 src > + > When possible, set-src preserves the original source port as long as the > address family allows it, otherwise the source port is set to 0. Best regards Tim Düsterhus
