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 -0000 1.55
+++ usr.sbin/httpd/config.c 11 Nov 2018 14:45:47 -0000
@@ -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 -0000 1.101
+++ usr.sbin/httpd/httpd.conf.5 11 Nov 2018 14:45:47 -0000
@@ -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 -0000 1.142
+++ usr.sbin/httpd/httpd.h 11 Nov 2018 14:45:47 -0000
@@ -400,6 +400,8 @@ SPLAY_HEAD(client_tree, client);
#define SRVFLAG_DEFAULT_TYPE 0x00800000
#define SRVFLAG_PATH_REWRITE 0x01000000
#define SRVFLAG_NO_PATH_REWRITE 0x02000000
+#define SRVFLAG_XFF 0x04000000
+#define SRVFLAG_NO_XFF 0x08000000
#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 -0000 1.107
+++ usr.sbin/httpd/parse.y 11 Nov 2018 14:45:47 -0000
@@ -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 <v.string> STRING
%token <v.number> NUMBER
@@ -979,6 +979,10 @@ logflags : STYLE logstyle
free($2);
srv_conf->flags |= SRVFLAG_ERROR_LOG;
}
+ | XFF {
+ srv_conf->flags &= ~SRVFLAG_NO_XFF;
+ srv_conf->flags |= SRVFLAG_XFF;
+ }
;
logstyle : COMMON {
@@ -1308,7 +1312,8 @@ lookup(char *s)
{ "tls", TLS },
{ "type", TYPE },
{ "types", TYPES },
- { "with", WITH }
+ { "with", WITH },
+ { "xff", XFF }
};
const struct keywords *p;
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 -0000 1.127
+++ usr.sbin/httpd/server_http.c 11 Nov 2018 14:45:47 -0000
@@ -1632,7 +1632,7 @@ server_log_http(struct client *clt, unsi
static char tstamp[64];
static char ip[INET6_ADDRSTRLEN];
time_t t;
- struct kv key, *agent, *referrer;
+ struct kv key, *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,14 @@ server_log_http(struct client *clt, unsi
agent->kv_value == NULL)
agent = NULL;
+ xff = NULL;
+ if (srv_conf->flags & SRVFLAG_XFF) {
+ key.kv_key = "X-Forwarded-For";
+ if (((xff = kv_find(&desc->http_headers, &key)) != NULL
+ && xff->kv_value == NULL))
+ xff = NULL;
+ }
+
/* Use vis to encode input values from the header */
if (clt->clt_remote_user &&
stravis(&user, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
@@ -1718,6 +1727,9 @@ server_log_http(struct client *clt, unsi
if (agent &&
stravis(&agent_v, agent->kv_value, HTTPD_LOGVIS) == -1)
goto done;
+ if (xff &&
+ stravis(&xff_v, xff->kv_value, HTTPD_LOGVIS) == -1)
+ goto done;
/* The following should be URL-encoded */
if (desc->http_path &&
@@ -1728,10 +1740,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 +1774,7 @@ done:
free(version);
free(referrer_v);
free(agent_v);
+ free(xff_v);
return (ret);
}
