Hello,
Le lun. 20 avr. 2020 à 20:37, Tim Düsterhus <t...@bastelstu.be> a écrit :
> 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.
>
Thank you for your valuable feedback. Find attached a new patch will all
your comments taken into account.
Olivier
>
> Best regards
> Tim Düsterhus
>
From e6b11f3a795ec40c8b802d9d1190f3f6bbd15f5d Mon Sep 17 00:00:00 2001
From: Olivier Doucet <oliv...@oxeva.fr>
Date: Tue, 21 Apr 2020 09:32:56 +0200
Subject: [PATCH] [DOC] Improve documentation on http-request set-src
This patch adds more explanation on how to use "http-request set-src"
and a link to "option forwardfor".
This patch can be applied to all previous version starting at 1.6
---
doc/configuration.txt | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git doc/configuration.txt doc/configuration.txt
index 5d01835d7..e695ab7f5 100644
--- doc/configuration.txt
+++ doc/configuration.txt
@@ -5114,16 +5114,23 @@ 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).
Arguments :
<expr> Is a standard HAProxy expression formed by a sample-fetch followed
by some converters.
+ See also "option forwardfor".
+
Example:
http-request set-src hdr(x-forwarded-for)
http-request set-src src,ipmask(24)
+ # After the masking this will track connections
+ # based on the IP address with the last byte zeroed out.
+ 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.
--
2.18.0.windows.1
