Re: [PATCH] httpd: Write X-Forwarded-For to access.log
> On 12 Feb 2019, at 14:52, Bruno Flueckiger wrote:
>
> On 12.11.18 12:40, Bruno Flueckiger wrote:
>> On 11.11.18 18:43, Claudio Jeker wrote:
>>> On Sun, Nov 11, 2018 at 06:32:53PM +0100, Bruno Flueckiger wrote:
On 11.11.18 15:29, Florian Obser wrote:
> On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
>> Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
>>> Hi
>>>
>>> When I run httpd(8) behind relayd(8) the access log of httpd contains
>>> the IP address of relayd, but not the IP address of the client. I've
>>> tried to match the logs of relayd(8) and httpd(8) using some scripting
>>> and failed.
>>>
>>> So I've written a patch for httpd(8). It stores the content of the
>>> X-Forwarded-For header in the third field of the log entry:
>>>
>>> www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
>>>
>>> Cheers,
>>> Bruno
>>
>> I'm not sure we should do this unconditionally. With no relayd or other
>> proxy infront of httpd, this is (more) data the client controls.
>
> Isn't what httpd(8) currently logs apache's common log format? If
> people are shoving that through webalizer or something that will
> break. I don't think we can do this without a config option.
> Do we need LogFormat?
>
>>
>> Could this be a problem?
>>
>> code reads ok.
>>
>> /Benno
>>
I've extended my patch with an option to the log directive. Both log xff
and log combined must be set for a server to log the content of the
X-Forwarded-For header. If only log combined is set the log entries
remain in the well-known format.
This prevents clients from getting unwanted data in the log by default.
And it makes sure the log format remains default until one decides
actively to change it.
>>>
>>> From my experience with webservices is that today logging the IP is
>>> seldomly good enough. Please also include X-Forwarded-Port and maybe
>>> X-Forwarded-Proto.
>>> In general thanks to CG-NAT logging only IP is a bit pointless.
>>>
>>> --
>>> :wq Claudio
>>>
>>
>> Thanks for the hint, Claudio.
>>
>> This version of my diff includes the two headers X-Forwarded-For and
>> X-Forwarded-Port. Instead of using the third field of the log entry I
>> add two additional fileds to it. The first one contains the value of
>> X-Forwarded-For and the second one that of X-Forwarded-Port.
>>
>> I think that appending two fields might do less harm than replacing one
>> field at the beginning of the log entry. I'm not sure that adding
>> X-Forwarded-Proto to the log really brings a benefit, so I left it away
>> in this diff.
>
> In the meantime I've run my diff on a webserver. In my experience
> webalizer has no problem with the modified log format. GoAccess on the
> other hand has troubles reading access.log, but that happens for me with
> and without the diff applied.
>
> I think that most admins would profit from the diff. The setting is
> optional so it doesn't affect everybody rightaway. And I believe that
> those who enable it are ready to reconfigure whatever log parser they
> use.
>
> Therefore I have reworked my diff so it applies to the -current tree.
Would be a very welcome addition to httpd.
Mischa
>
> Index: usr.sbin/httpd/config.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/config.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 config.c
> --- usr.sbin/httpd/config.c 20 Jun 2018 16:43:05 - 1.55
> +++ usr.sbin/httpd/config.c 12 Feb 2019 13:37:55 -
> @@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
> if ((srv_conf->flags & f) == 0)
> srv_conf->flags |= parent->flags & f;
>
> + f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
> + if ((srv_conf->flags & f) == 0)
> + srv_conf->flags |= parent->flags & f;
> +
> f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
> if ((srv_conf->flags & f) == 0) {
> srv_conf->flags |= parent->flags & f;
> Index: usr.sbin/httpd/httpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.102
> diff -u -p -r1.102 httpd.conf.5
> --- usr.sbin/httpd/httpd.conf.5 8 Feb 2019 11:46:07 - 1.102
> +++ usr.sbin/httpd/httpd.conf.5 12 Feb 2019 13:37:55 -
> @@ -464,6 +464,16 @@ If not specified, the default is
> Enable or disable logging to
> .Xr syslog 3
> instead of the log files.
> +.It Oo Ic no Oc Ic xff
> +Enable or disable logging of the request headers
> +.Ar X-Forwarded-For
> +and
> +.Ar X-Forwarded-Port
> +if
> +.Cm log combined
> +is set. These headers can be set by a reverse proxy like
> +.Xr relayd 8
> +and should contain the IP address and TCP port of the client.
> .El
>
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
On 12.11.18 12:40, Bruno Flueckiger wrote:
> On 11.11.18 18:43, Claudio Jeker wrote:
> > On Sun, Nov 11, 2018 at 06:32:53PM +0100, Bruno Flueckiger wrote:
> > > On 11.11.18 15:29, Florian Obser wrote:
> > > > On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
> > > > > Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> > > > > > Hi
> > > > > >
> > > > > > When I run httpd(8) behind relayd(8) the access log of httpd
> > > > > > contains
> > > > > > the IP address of relayd, but not the IP address of the client. I've
> > > > > > tried to match the logs of relayd(8) and httpd(8) using some
> > > > > > scripting
> > > > > > and failed.
> > > > > >
> > > > > > So I've written a patch for httpd(8). It stores the content of the
> > > > > > X-Forwarded-For header in the third field of the log entry:
> > > > > >
> > > > > > www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
> > > > > >
> > > > > > Cheers,
> > > > > > Bruno
> > > > >
> > > > > I'm not sure we should do this unconditionally. With no relayd or
> > > > > other
> > > > > proxy infront of httpd, this is (more) data the client controls.
> > > >
> > > > Isn't what httpd(8) currently logs apache's common log format? If
> > > > people are shoving that through webalizer or something that will
> > > > break. I don't think we can do this without a config option.
> > > > Do we need LogFormat?
> > > >
> > > > >
> > > > > Could this be a problem?
> > > > >
> > > > > code reads ok.
> > > > >
> > > > > /Benno
> > > > >
> > >
> > > I've extended my patch with an option to the log directive. Both log xff
> > > and log combined must be set for a server to log the content of the
> > > X-Forwarded-For header. If only log combined is set the log entries
> > > remain in the well-known format.
> > >
> > > This prevents clients from getting unwanted data in the log by default.
> > > And it makes sure the log format remains default until one decides
> > > actively to change it.
> >
> > From my experience with webservices is that today logging the IP is
> > seldomly good enough. Please also include X-Forwarded-Port and maybe
> > X-Forwarded-Proto.
> > In general thanks to CG-NAT logging only IP is a bit pointless.
> >
> > --
> > :wq Claudio
> >
>
> Thanks for the hint, Claudio.
>
> This version of my diff includes the two headers X-Forwarded-For and
> X-Forwarded-Port. Instead of using the third field of the log entry I
> add two additional fileds to it. The first one contains the value of
> X-Forwarded-For and the second one that of X-Forwarded-Port.
>
> I think that appending two fields might do less harm than replacing one
> field at the beginning of the log entry. I'm not sure that adding
> X-Forwarded-Proto to the log really brings a benefit, so I left it away
> in this diff.
In the meantime I've run my diff on a webserver. In my experience
webalizer has no problem with the modified log format. GoAccess on the
other hand has troubles reading access.log, but that happens for me with
and without the diff applied.
I think that most admins would profit from the diff. The setting is
optional so it doesn't affect everybody rightaway. And I believe that
those who enable it are ready to reconfigure whatever log parser they
use.
Therefore I have reworked my diff so it applies to the -current tree.
Index: usr.sbin/httpd/config.c
===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.55
diff -u -p -r1.55 config.c
--- usr.sbin/httpd/config.c 20 Jun 2018 16:43:05 - 1.55
+++ usr.sbin/httpd/config.c 12 Feb 2019 13:37:55 -
@@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
if ((srv_conf->flags & f) == 0)
srv_conf->flags |= parent->flags & f;
+ f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
+ if ((srv_conf->flags & f) == 0)
+ srv_conf->flags |= parent->flags & f;
+
f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
if ((srv_conf->flags & f) == 0) {
srv_conf->flags |= parent->flags & f;
Index: usr.sbin/httpd/httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.102
diff -u -p -r1.102 httpd.conf.5
--- usr.sbin/httpd/httpd.conf.5 8 Feb 2019 11:46:07 - 1.102
+++ usr.sbin/httpd/httpd.conf.5 12 Feb 2019 13:37:55 -
@@ -464,6 +464,16 @@ If not specified, the default is
Enable or disable logging to
.Xr syslog 3
instead of the log files.
+.It Oo Ic no Oc Ic xff
+Enable or disable logging of the request headers
+.Ar X-Forwarded-For
+and
+.Ar X-Forwarded-Port
+if
+.Cm log combined
+is set. These headers can be set by a reverse proxy like
+.Xr relayd 8
+and should contain the IP address and TCP port of the client.
.El
.It Ic pass
Disable any previous
Index: usr.sbin/httpd/httpd.h
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
On 11.11.18 18:43, Claudio Jeker wrote:
> On Sun, Nov 11, 2018 at 06:32:53PM +0100, Bruno Flueckiger wrote:
> > On 11.11.18 15:29, Florian Obser wrote:
> > > On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
> > > > Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> > > > > Hi
> > > > >
> > > > > When I run httpd(8) behind relayd(8) the access log of httpd contains
> > > > > the IP address of relayd, but not the IP address of the client. I've
> > > > > tried to match the logs of relayd(8) and httpd(8) using some scripting
> > > > > and failed.
> > > > >
> > > > > So I've written a patch for httpd(8). It stores the content of the
> > > > > X-Forwarded-For header in the third field of the log entry:
> > > > >
> > > > > www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
> > > > >
> > > > > Cheers,
> > > > > Bruno
> > > >
> > > > I'm not sure we should do this unconditionally. With no relayd or other
> > > > proxy infront of httpd, this is (more) data the client controls.
> > >
> > > Isn't what httpd(8) currently logs apache's common log format? If
> > > people are shoving that through webalizer or something that will
> > > break. I don't think we can do this without a config option.
> > > Do we need LogFormat?
> > >
> > > >
> > > > Could this be a problem?
> > > >
> > > > code reads ok.
> > > >
> > > > /Benno
> > > >
> >
> > I've extended my patch with an option to the log directive. Both log xff
> > and log combined must be set for a server to log the content of the
> > X-Forwarded-For header. If only log combined is set the log entries
> > remain in the well-known format.
> >
> > This prevents clients from getting unwanted data in the log by default.
> > And it makes sure the log format remains default until one decides
> > actively to change it.
>
> From my experience with webservices is that today logging the IP is
> seldomly good enough. Please also include X-Forwarded-Port and maybe
> X-Forwarded-Proto.
> In general thanks to CG-NAT logging only IP is a bit pointless.
>
> --
> :wq Claudio
>
Thanks for the hint, Claudio.
This version of my diff includes the two headers X-Forwarded-For and
X-Forwarded-Port. Instead of using the third field of the log entry I
add two additional fileds to it. The first one contains the value of
X-Forwarded-For and the second one that of X-Forwarded-Port.
I think that appending two fields might do less harm than replacing one
field at the beginning of the log entry. I'm not sure that adding
X-Forwarded-Proto to the log really brings a benefit, so I left it away
in this diff.
Index: usr.sbin/httpd/config.c
===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.55
diff -u -p -r1.55 config.c
--- usr.sbin/httpd/config.c 20 Jun 2018 16:43:05 - 1.55
+++ usr.sbin/httpd/config.c 12 Nov 2018 10:18:41 -
@@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
if ((srv_conf->flags & f) == 0)
srv_conf->flags |= parent->flags & f;
+ f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
+ if ((srv_conf->flags & f) == 0)
+ srv_conf->flags |= parent->flags & f;
+
f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
if ((srv_conf->flags & f) == 0) {
srv_conf->flags |= parent->flags & f;
Index: usr.sbin/httpd/httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.101
diff -u -p -r1.101 httpd.conf.5
--- usr.sbin/httpd/httpd.conf.5 20 Jun 2018 16:43:05 - 1.101
+++ usr.sbin/httpd/httpd.conf.5 12 Nov 2018 10:18:41 -
@@ -455,6 +455,16 @@ If not specified, the default is
Enable or disable logging to
.Xr syslog 3
instead of the log files.
+.It Oo Ic no Oc Ic xff
+Enable or disable logging of the request headers
+.Ar X-Forwarded-For
+and
+.Ar X-Forwarded-Port
+if
+.Cm log combined
+is set. These headers can be set by a reverse proxy like
+.Xr relayd 8
+and should contain the IP address and TCP port of the client.
.El
.It Ic pass
Disable any previous
Index: usr.sbin/httpd/httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.142
diff -u -p -r1.142 httpd.h
--- usr.sbin/httpd/httpd.h 11 Oct 2018 09:52:22 - 1.142
+++ usr.sbin/httpd/httpd.h 12 Nov 2018 10:18:41 -
@@ -400,6 +400,8 @@ SPLAY_HEAD(client_tree, client);
#define SRVFLAG_DEFAULT_TYPE 0x0080
#define SRVFLAG_PATH_REWRITE 0x0100
#define SRVFLAG_NO_PATH_REWRITE0x0200
+#define SRVFLAG_XFF0x0400
+#define SRVFLAG_NO_XFF 0x0800
#define SRVFLAG_BITS \
"\10\01INDEX\02NO_INDEX\03AUTO_INDEX\04NO_AUTO_INDEX" \
Index:
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
> On 11 Nov 2018, at 18:43, Claudio Jeker wrote:
>
>> On Sun, Nov 11, 2018 at 06:32:53PM +0100, Bruno Flueckiger wrote:
>>> On 11.11.18 15:29, Florian Obser wrote:
On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> Hi
>
> When I run httpd(8) behind relayd(8) the access log of httpd contains
> the IP address of relayd, but not the IP address of the client. I've
> tried to match the logs of relayd(8) and httpd(8) using some scripting
> and failed.
>
> So I've written a patch for httpd(8). It stores the content of the
> X-Forwarded-For header in the third field of the log entry:
>
> www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
>
> Cheers,
> Bruno
I'm not sure we should do this unconditionally. With no relayd or other
proxy infront of httpd, this is (more) data the client controls.
>>>
>>> Isn't what httpd(8) currently logs apache's common log format? If
>>> people are shoving that through webalizer or something that will
>>> break. I don't think we can do this without a config option.
>>> Do we need LogFormat?
>>>
Could this be a problem?
code reads ok.
/Benno
>>
>> I've extended my patch with an option to the log directive. Both log xff
>> and log combined must be set for a server to log the content of the
>> X-Forwarded-For header. If only log combined is set the log entries
>> remain in the well-known format.
>>
>> This prevents clients from getting unwanted data in the log by default.
>> And it makes sure the log format remains default until one decides
>> actively to change it.
>
> From my experience with webservices is that today logging the IP is
> seldomly good enough. Please also include X-Forwarded-Port and maybe
> X-Forwarded-Proto.
> In general thanks to CG-NAT logging only IP is a bit pointless.
Or with relayd in front of it. :)
Welcome addition to httpd.
Mischa
>
> --
> :wq Claudio
>
>> Index: usr.sbin/httpd/config.c
>> ===
>> RCS file: /cvs/src/usr.sbin/httpd/config.c,v
>> retrieving revision 1.55
>> diff -u -p -r1.55 config.c
>> --- usr.sbin/httpd/config.c20 Jun 2018 16:43:05 -1.55
>> +++ usr.sbin/httpd/config.c11 Nov 2018 14:45:47 -
>> @@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
>>if ((srv_conf->flags & f) == 0)
>>srv_conf->flags |= parent->flags & f;
>>
>> +f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
>> +if ((srv_conf->flags & f) == 0)
>> +srv_conf->flags |= parent->flags & f;
>> +
>>f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
>>if ((srv_conf->flags & f) == 0) {
>>srv_conf->flags |= parent->flags & f;
>> Index: usr.sbin/httpd/httpd.conf.5
>> ===
>> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
>> retrieving revision 1.101
>> diff -u -p -r1.101 httpd.conf.5
>> --- usr.sbin/httpd/httpd.conf.520 Jun 2018 16:43:05 -1.101
>> +++ usr.sbin/httpd/httpd.conf.511 Nov 2018 14:45:47 -
>> @@ -455,6 +455,14 @@ If not specified, the default is
>> Enable or disable logging to
>> .Xr syslog 3
>> instead of the log files.
>> +.It Oo Ic no Oc Ic xff
>> +Enable or disable logging of the request header
>> +.Ar X-Forwarded-For
>> +if
>> +.Cm log combined
>> +is set. This header can be set by a reverse proxy like
>> +.Xr relayd 8
>> +and should contain the IP address of the client.
>> .El
>> .It Ic pass
>> Disable any previous
>> Index: usr.sbin/httpd/httpd.h
>> ===
>> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
>> retrieving revision 1.142
>> diff -u -p -r1.142 httpd.h
>> --- usr.sbin/httpd/httpd.h11 Oct 2018 09:52:22 -1.142
>> +++ usr.sbin/httpd/httpd.h11 Nov 2018 14:45:47 -
>> @@ -400,6 +400,8 @@ SPLAY_HEAD(client_tree, client);
>> #define SRVFLAG_DEFAULT_TYPE0x0080
>> #define SRVFLAG_PATH_REWRITE0x0100
>> #define SRVFLAG_NO_PATH_REWRITE0x0200
>> +#define SRVFLAG_XFF0x0400
>> +#define SRVFLAG_NO_XFF0x0800
>>
>> #define SRVFLAG_BITS\
>>"\10\01INDEX\02NO_INDEX\03AUTO_INDEX\04NO_AUTO_INDEX"\
>> Index: usr.sbin/httpd/parse.y
>> ===
>> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
>> retrieving revision 1.107
>> diff -u -p -r1.107 parse.y
>> --- usr.sbin/httpd/parse.y1 Nov 2018 00:18:44 -1.107
>> +++ usr.sbin/httpd/parse.y11 Nov 2018 14:45:47 -
>> @@ -139,7 +139,7 @@ typedef struct {
>> %tokenLISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON PORT
>> PREFORK
>> %tokenPROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG TCP
>> TICKET
>> %tokenTIMEOUT
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
On Sun, Nov 11, 2018 at 06:32:53PM +0100, Bruno Flueckiger wrote:
> On 11.11.18 15:29, Florian Obser wrote:
> > On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
> > > Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> > > > Hi
> > > >
> > > > When I run httpd(8) behind relayd(8) the access log of httpd contains
> > > > the IP address of relayd, but not the IP address of the client. I've
> > > > tried to match the logs of relayd(8) and httpd(8) using some scripting
> > > > and failed.
> > > >
> > > > So I've written a patch for httpd(8). It stores the content of the
> > > > X-Forwarded-For header in the third field of the log entry:
> > > >
> > > > www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
> > > >
> > > > Cheers,
> > > > Bruno
> > >
> > > I'm not sure we should do this unconditionally. With no relayd or other
> > > proxy infront of httpd, this is (more) data the client controls.
> >
> > Isn't what httpd(8) currently logs apache's common log format? If
> > people are shoving that through webalizer or something that will
> > break. I don't think we can do this without a config option.
> > Do we need LogFormat?
> >
> > >
> > > Could this be a problem?
> > >
> > > code reads ok.
> > >
> > > /Benno
> > >
>
> I've extended my patch with an option to the log directive. Both log xff
> and log combined must be set for a server to log the content of the
> X-Forwarded-For header. If only log combined is set the log entries
> remain in the well-known format.
>
> This prevents clients from getting unwanted data in the log by default.
> And it makes sure the log format remains default until one decides
> actively to change it.
>From my experience with webservices is that today logging the IP is
seldomly good enough. Please also include X-Forwarded-Port and maybe
X-Forwarded-Proto.
In general thanks to CG-NAT logging only IP is a bit pointless.
--
:wq Claudio
> Index: usr.sbin/httpd/config.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/config.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 config.c
> --- usr.sbin/httpd/config.c 20 Jun 2018 16:43:05 - 1.55
> +++ usr.sbin/httpd/config.c 11 Nov 2018 14:45:47 -
> @@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
> if ((srv_conf->flags & f) == 0)
> srv_conf->flags |= parent->flags & f;
>
> + f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
> + if ((srv_conf->flags & f) == 0)
> + srv_conf->flags |= parent->flags & f;
> +
> f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
> if ((srv_conf->flags & f) == 0) {
> srv_conf->flags |= parent->flags & f;
> Index: usr.sbin/httpd/httpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.101
> diff -u -p -r1.101 httpd.conf.5
> --- usr.sbin/httpd/httpd.conf.5 20 Jun 2018 16:43:05 - 1.101
> +++ usr.sbin/httpd/httpd.conf.5 11 Nov 2018 14:45:47 -
> @@ -455,6 +455,14 @@ If not specified, the default is
> Enable or disable logging to
> .Xr syslog 3
> instead of the log files.
> +.It Oo Ic no Oc Ic xff
> +Enable or disable logging of the request header
> +.Ar X-Forwarded-For
> +if
> +.Cm log combined
> +is set. This header can be set by a reverse proxy like
> +.Xr relayd 8
> +and should contain the IP address of the client.
> .El
> .It Ic pass
> Disable any previous
> Index: usr.sbin/httpd/httpd.h
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.142
> diff -u -p -r1.142 httpd.h
> --- usr.sbin/httpd/httpd.h11 Oct 2018 09:52:22 - 1.142
> +++ usr.sbin/httpd/httpd.h11 Nov 2018 14:45:47 -
> @@ -400,6 +400,8 @@ SPLAY_HEAD(client_tree, client);
> #define SRVFLAG_DEFAULT_TYPE 0x0080
> #define SRVFLAG_PATH_REWRITE 0x0100
> #define SRVFLAG_NO_PATH_REWRITE 0x0200
> +#define SRVFLAG_XFF 0x0400
> +#define SRVFLAG_NO_XFF 0x0800
>
> #define SRVFLAG_BITS \
> "\10\01INDEX\02NO_INDEX\03AUTO_INDEX\04NO_AUTO_INDEX" \
> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.107
> diff -u -p -r1.107 parse.y
> --- usr.sbin/httpd/parse.y1 Nov 2018 00:18:44 - 1.107
> +++ usr.sbin/httpd/parse.y11 Nov 2018 14:45:47 -
> @@ -139,7 +139,7 @@ typedef struct {
> %token LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON
> PORT PREFORK
> %token PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG
> TCP TICKET
> %token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD
>
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
On 11.11.18 15:29, Florian Obser wrote:
> On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
> > Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> > > Hi
> > >
> > > When I run httpd(8) behind relayd(8) the access log of httpd contains
> > > the IP address of relayd, but not the IP address of the client. I've
> > > tried to match the logs of relayd(8) and httpd(8) using some scripting
> > > and failed.
> > >
> > > So I've written a patch for httpd(8). It stores the content of the
> > > X-Forwarded-For header in the third field of the log entry:
> > >
> > > www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
> > >
> > > Cheers,
> > > Bruno
> >
> > I'm not sure we should do this unconditionally. With no relayd or other
> > proxy infront of httpd, this is (more) data the client controls.
>
> Isn't what httpd(8) currently logs apache's common log format? If
> people are shoving that through webalizer or something that will
> break. I don't think we can do this without a config option.
> Do we need LogFormat?
>
> >
> > Could this be a problem?
> >
> > code reads ok.
> >
> > /Benno
> >
I've extended my patch with an option to the log directive. Both log xff
and log combined must be set for a server to log the content of the
X-Forwarded-For header. If only log combined is set the log entries
remain in the well-known format.
This prevents clients from getting unwanted data in the log by default.
And it makes sure the log format remains default until one decides
actively to change it.
Index: usr.sbin/httpd/config.c
===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.55
diff -u -p -r1.55 config.c
--- usr.sbin/httpd/config.c 20 Jun 2018 16:43:05 - 1.55
+++ usr.sbin/httpd/config.c 11 Nov 2018 14:45:47 -
@@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
if ((srv_conf->flags & f) == 0)
srv_conf->flags |= parent->flags & f;
+ f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
+ if ((srv_conf->flags & f) == 0)
+ srv_conf->flags |= parent->flags & f;
+
f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
if ((srv_conf->flags & f) == 0) {
srv_conf->flags |= parent->flags & f;
Index: usr.sbin/httpd/httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.101
diff -u -p -r1.101 httpd.conf.5
--- usr.sbin/httpd/httpd.conf.5 20 Jun 2018 16:43:05 - 1.101
+++ usr.sbin/httpd/httpd.conf.5 11 Nov 2018 14:45:47 -
@@ -455,6 +455,14 @@ If not specified, the default is
Enable or disable logging to
.Xr syslog 3
instead of the log files.
+.It Oo Ic no Oc Ic xff
+Enable or disable logging of the request header
+.Ar X-Forwarded-For
+if
+.Cm log combined
+is set. This header can be set by a reverse proxy like
+.Xr relayd 8
+and should contain the IP address of the client.
.El
.It Ic pass
Disable any previous
Index: usr.sbin/httpd/httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.142
diff -u -p -r1.142 httpd.h
--- usr.sbin/httpd/httpd.h 11 Oct 2018 09:52:22 - 1.142
+++ usr.sbin/httpd/httpd.h 11 Nov 2018 14:45:47 -
@@ -400,6 +400,8 @@ SPLAY_HEAD(client_tree, client);
#define SRVFLAG_DEFAULT_TYPE 0x0080
#define SRVFLAG_PATH_REWRITE 0x0100
#define SRVFLAG_NO_PATH_REWRITE0x0200
+#define SRVFLAG_XFF0x0400
+#define SRVFLAG_NO_XFF 0x0800
#define SRVFLAG_BITS \
"\10\01INDEX\02NO_INDEX\03AUTO_INDEX\04NO_AUTO_INDEX" \
Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.107
diff -u -p -r1.107 parse.y
--- usr.sbin/httpd/parse.y 1 Nov 2018 00:18:44 - 1.107
+++ usr.sbin/httpd/parse.y 11 Nov 2018 14:45:47 -
@@ -139,7 +139,7 @@ typedef struct {
%token LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON PORT PREFORK
%token PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG TCP TICKET
%token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD REQUEST
-%token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
+%token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE XFF
%token CA CLIENT CRL OPTIONAL
%token STRING
%token NUMBER
@@ -979,6 +979,10 @@ logflags : STYLE logstyle
free($2);
srv_conf->flags |= SRVFLAG_ERROR_LOG;
}
+ | XFF {
+ srv_conf->flags &= ~SRVFLAG_NO_XFF;
+
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
> Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> > Hi
> >
> > When I run httpd(8) behind relayd(8) the access log of httpd contains
> > the IP address of relayd, but not the IP address of the client. I've
> > tried to match the logs of relayd(8) and httpd(8) using some scripting
> > and failed.
> >
> > So I've written a patch for httpd(8). It stores the content of the
> > X-Forwarded-For header in the third field of the log entry:
> >
> > www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
> >
> > Cheers,
> > Bruno
>
> I'm not sure we should do this unconditionally. With no relayd or other
> proxy infront of httpd, this is (more) data the client controls.
Isn't what httpd(8) currently logs apache's common log format? If
people are shoving that through webalizer or something that will
break. I don't think we can do this without a config option.
Do we need LogFormat?
>
> Could this be a problem?
>
> code reads ok.
>
> /Benno
>
>
> > Index: usr.sbin/httpd/server_http.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> > retrieving revision 1.127
> > diff -u -p -r1.127 server_http.c
> > --- usr.sbin/httpd/server_http.c4 Nov 2018 05:56:45 - 1.127
> > +++ usr.sbin/httpd/server_http.c11 Nov 2018 08:41:18 -
> > @@ -1632,7 +1632,7 @@ server_log_http(struct client *clt, unsi
> > static char tstamp[64];
> > static char ip[INET6_ADDRSTRLEN];
> > time_t t;
> > - struct kvkey, *agent, *referrer;
> > + struct kvkey, *agent, *referrer, *xff;
> > struct tm *tm;
> > struct server_config*srv_conf;
> > struct http_descriptor *desc;
> > @@ -1642,6 +1642,7 @@ server_log_http(struct client *clt, unsi
> > char*version = NULL;
> > char*referrer_v = NULL;
> > char*agent_v = NULL;
> > + char*xff_v = NULL;
> >
> > if ((srv_conf = clt->clt_srv_conf) == NULL)
> > return (-1);
> > @@ -1666,7 +1667,7 @@ server_log_http(struct client *clt, unsi
> > *
> > * httpd's format is similar to these Apache LogFormats:
> > * "%v %h %l %u %t \"%r\" %>s %B"
> > -* "%v %h %l %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\""
> > +* "%v %h %a %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\""
> > */
> > switch (srv_conf->logformat) {
> > case LOG_FORMAT_COMMON:
> > @@ -1708,6 +1709,11 @@ server_log_http(struct client *clt, unsi
> > agent->kv_value == NULL)
> > agent = NULL;
> >
> > + key.kv_key = "X-Forwarded-For";
> > + if ((xff = kv_find(>http_headers, )) != NULL &&
> > + xff->kv_value == NULL)
> > + xff = NULL;
> > +
> > /* Use vis to encode input values from the header */
> > if (clt->clt_remote_user &&
> > stravis(, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
> > @@ -1718,6 +1724,9 @@ server_log_http(struct client *clt, unsi
> > if (agent &&
> > stravis(_v, agent->kv_value, HTTPD_LOGVIS) == -1)
> > goto done;
> > + if (xff &&
> > + stravis(_v, xff->kv_value, HTTPD_LOGVIS) == -1)
> > + goto done;
> >
> > /* The following should be URL-encoded */
> > if (desc->http_path &&
> > @@ -1728,10 +1737,10 @@ server_log_http(struct client *clt, unsi
> > goto done;
> >
> > ret = evbuffer_add_printf(clt->clt_log,
> > - "%s %s - %s [%s] \"%s %s%s%s%s%s\""
> > + "%s %s %s %s [%s] \"%s %s%s%s%s%s\""
> > " %03d %zu \"%s\" \"%s\"\n",
> > - srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
> > - user, tstamp,
> > + srv_conf->name, ip, xff == NULL ? "-" : xff_v,
> > + clt->clt_remote_user == NULL ? "-" : user, tstamp,
> > server_httpmethod_byid(desc->http_method),
> > desc->http_path == NULL ? "" : path,
> > desc->http_query == NULL ? "" : "?",
> > @@ -1762,6 +1771,7 @@ done:
> > free(version);
> > free(referrer_v);
> > free(agent_v);
> > + free(xff_v);
> >
> > return (ret);
> > }
> >
>
--
I'm not entirely sure you are real.
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> Hi
>
> When I run httpd(8) behind relayd(8) the access log of httpd contains
> the IP address of relayd, but not the IP address of the client. I've
> tried to match the logs of relayd(8) and httpd(8) using some scripting
> and failed.
>
> So I've written a patch for httpd(8). It stores the content of the
> X-Forwarded-For header in the third field of the log entry:
>
> www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
>
> Cheers,
> Bruno
I'm not sure we should do this unconditionally. With no relayd or other
proxy infront of httpd, this is (more) data the client controls.
Could this be a problem?
code reads ok.
/Benno
> Index: usr.sbin/httpd/server_http.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.127
> diff -u -p -r1.127 server_http.c
> --- usr.sbin/httpd/server_http.c 4 Nov 2018 05:56:45 - 1.127
> +++ usr.sbin/httpd/server_http.c 11 Nov 2018 08:41:18 -
> @@ -1632,7 +1632,7 @@ server_log_http(struct client *clt, unsi
> static char tstamp[64];
> static char ip[INET6_ADDRSTRLEN];
> time_t t;
> - struct kvkey, *agent, *referrer;
> + struct kvkey, *agent, *referrer, *xff;
> struct tm *tm;
> struct server_config*srv_conf;
> struct http_descriptor *desc;
> @@ -1642,6 +1642,7 @@ server_log_http(struct client *clt, unsi
> char*version = NULL;
> char*referrer_v = NULL;
> char*agent_v = NULL;
> + char*xff_v = NULL;
>
> if ((srv_conf = clt->clt_srv_conf) == NULL)
> return (-1);
> @@ -1666,7 +1667,7 @@ server_log_http(struct client *clt, unsi
>*
>* httpd's format is similar to these Apache LogFormats:
>* "%v %h %l %u %t \"%r\" %>s %B"
> - * "%v %h %l %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\""
> + * "%v %h %a %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\""
>*/
> switch (srv_conf->logformat) {
> case LOG_FORMAT_COMMON:
> @@ -1708,6 +1709,11 @@ server_log_http(struct client *clt, unsi
> agent->kv_value == NULL)
> agent = NULL;
>
> + key.kv_key = "X-Forwarded-For";
> + if ((xff = kv_find(>http_headers, )) != NULL &&
> + xff->kv_value == NULL)
> + xff = NULL;
> +
> /* Use vis to encode input values from the header */
> if (clt->clt_remote_user &&
> stravis(, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
> @@ -1718,6 +1724,9 @@ server_log_http(struct client *clt, unsi
> if (agent &&
> stravis(_v, agent->kv_value, HTTPD_LOGVIS) == -1)
> goto done;
> + if (xff &&
> + stravis(_v, xff->kv_value, HTTPD_LOGVIS) == -1)
> + goto done;
>
> /* The following should be URL-encoded */
> if (desc->http_path &&
> @@ -1728,10 +1737,10 @@ server_log_http(struct client *clt, unsi
> goto done;
>
> ret = evbuffer_add_printf(clt->clt_log,
> - "%s %s - %s [%s] \"%s %s%s%s%s%s\""
> + "%s %s %s %s [%s] \"%s %s%s%s%s%s\""
> " %03d %zu \"%s\" \"%s\"\n",
> - srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
> - user, tstamp,
> + srv_conf->name, ip, xff == NULL ? "-" : xff_v,
> + clt->clt_remote_user == NULL ? "-" : user, tstamp,
> server_httpmethod_byid(desc->http_method),
> desc->http_path == NULL ? "" : path,
> desc->http_query == NULL ? "" : "?",
> @@ -1762,6 +1771,7 @@ done:
> free(version);
> free(referrer_v);
> free(agent_v);
> + free(xff_v);
>
> return (ret);
> }
>
[PATCH] httpd: Write X-Forwarded-For to access.log
Hi
When I run httpd(8) behind relayd(8) the access log of httpd contains
the IP address of relayd, but not the IP address of the client. I've
tried to match the logs of relayd(8) and httpd(8) using some scripting
and failed.
So I've written a patch for httpd(8). It stores the content of the
X-Forwarded-For header in the third field of the log entry:
www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
Cheers,
Bruno
Index: usr.sbin/httpd/server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.127
diff -u -p -r1.127 server_http.c
--- usr.sbin/httpd/server_http.c4 Nov 2018 05:56:45 - 1.127
+++ usr.sbin/httpd/server_http.c11 Nov 2018 08:41:18 -
@@ -1632,7 +1632,7 @@ server_log_http(struct client *clt, unsi
static char tstamp[64];
static char ip[INET6_ADDRSTRLEN];
time_t t;
- struct kvkey, *agent, *referrer;
+ struct kvkey, *agent, *referrer, *xff;
struct tm *tm;
struct server_config*srv_conf;
struct http_descriptor *desc;
@@ -1642,6 +1642,7 @@ server_log_http(struct client *clt, unsi
char*version = NULL;
char*referrer_v = NULL;
char*agent_v = NULL;
+ char*xff_v = NULL;
if ((srv_conf = clt->clt_srv_conf) == NULL)
return (-1);
@@ -1666,7 +1667,7 @@ server_log_http(struct client *clt, unsi
*
* httpd's format is similar to these Apache LogFormats:
* "%v %h %l %u %t \"%r\" %>s %B"
-* "%v %h %l %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\""
+* "%v %h %a %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\""
*/
switch (srv_conf->logformat) {
case LOG_FORMAT_COMMON:
@@ -1708,6 +1709,11 @@ server_log_http(struct client *clt, unsi
agent->kv_value == NULL)
agent = NULL;
+ key.kv_key = "X-Forwarded-For";
+ if ((xff = kv_find(>http_headers, )) != NULL &&
+ xff->kv_value == NULL)
+ xff = NULL;
+
/* Use vis to encode input values from the header */
if (clt->clt_remote_user &&
stravis(, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
@@ -1718,6 +1724,9 @@ server_log_http(struct client *clt, unsi
if (agent &&
stravis(_v, agent->kv_value, HTTPD_LOGVIS) == -1)
goto done;
+ if (xff &&
+ stravis(_v, xff->kv_value, HTTPD_LOGVIS) == -1)
+ goto done;
/* The following should be URL-encoded */
if (desc->http_path &&
@@ -1728,10 +1737,10 @@ server_log_http(struct client *clt, unsi
goto done;
ret = evbuffer_add_printf(clt->clt_log,
- "%s %s - %s [%s] \"%s %s%s%s%s%s\""
+ "%s %s %s %s [%s] \"%s %s%s%s%s%s\""
" %03d %zu \"%s\" \"%s\"\n",
- srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
- user, tstamp,
+ srv_conf->name, ip, xff == NULL ? "-" : xff_v,
+ clt->clt_remote_user == NULL ? "-" : user, tstamp,
server_httpmethod_byid(desc->http_method),
desc->http_path == NULL ? "" : path,
desc->http_query == NULL ? "" : "?",
@@ -1762,6 +1771,7 @@ done:
free(version);
free(referrer_v);
free(agent_v);
+ free(xff_v);
return (ret);
}
