[PATCH] httpd: Write X-Forwarded-For to access.log

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] httpd: Write X-Forwarded-For to access.log

Bruno Flueckiger
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.c 4 Nov 2018 05:56:45 -0000 1.127
+++ usr.sbin/httpd/server_http.c 11 Nov 2018 08:41:18 -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,11 @@ server_log_http(struct client *clt, unsi
     agent->kv_value == NULL)
  agent = NULL;
 
+ 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 +1724,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 +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);
 }

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Sebastian Benoit-3
Bruno Flueckiger([hidden email]) 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 -0000 1.127
> +++ usr.sbin/httpd/server_http.c 11 Nov 2018 08:41:18 -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,11 @@ server_log_http(struct client *clt, unsi
>      agent->kv_value == NULL)
>   agent = NULL;
>  
> + 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 +1724,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 +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);
>  }
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Florian Obser-2
On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:

> Bruno Flueckiger([hidden email]) 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.c 4 Nov 2018 05:56:45 -0000 1.127
> > +++ usr.sbin/httpd/server_http.c 11 Nov 2018 08:41:18 -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,11 @@ server_log_http(struct client *clt, unsi
> >      agent->kv_value == NULL)
> >   agent = NULL;
> >  
> > + 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 +1724,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 +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.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Bruno Flueckiger
On 11.11.18 15:29, Florian Obser wrote:

> On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
> > Bruno Flueckiger([hidden email]) 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);
 }

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Claudio Jeker
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([hidden email]) 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 -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);
>  }
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Mischa-2


> On 11 Nov 2018, at 18:43, Claudio Jeker <[hidden email]> 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([hidden email]) 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.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);
>> }
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Bruno Flueckiger
In reply to this post by Claudio Jeker
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([hidden email]) 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 -0000 1.55
+++ usr.sbin/httpd/config.c 12 Nov 2018 10:18:41 -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 12 Nov 2018 10:18:41 -0000
@@ -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 -0000 1.142
+++ usr.sbin/httpd/httpd.h 12 Nov 2018 10:18:41 -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 12 Nov 2018 10:18:41 -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 12 Nov 2018 10:18:41 -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, *xfp;
  struct tm *tm;
  struct server_config *srv_conf;
  struct http_descriptor *desc;
@@ -1642,6 +1642,8 @@ server_log_http(struct client *clt, unsi
  char *version = NULL;
  char *referrer_v = NULL;
  char *agent_v = NULL;
+ char *xff_v = NULL;
+ char *xfp_v = NULL;
 
  if ((srv_conf = clt->clt_srv_conf) == NULL)
  return (-1);
@@ -1667,6 +1669,9 @@ 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\""
+ *
+ * Setting the log option xff will change the combined format to:
+ * "%v %h %l %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\" %a %{remote}p"
  */
  switch (srv_conf->logformat) {
  case LOG_FORMAT_COMMON:
@@ -1708,6 +1713,19 @@ server_log_http(struct client *clt, unsi
     agent->kv_value == NULL)
  agent = NULL;
 
+ xff = xfp = 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;
+
+ key.kv_key = "X-Forwarded-Port";
+ if (((xfp = kv_find(&desc->http_headers, &key)) != NULL
+    && xfp->kv_value == NULL))
+ xfp = 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 +1736,12 @@ 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;
+ if (xfp &&
+    stravis(&xfp_v, xfp->kv_value, HTTPD_LOGVIS) == -1)
+ goto done;
 
  /* The following should be URL-encoded */
  if (desc->http_path &&
@@ -1729,7 +1753,7 @@ server_log_http(struct client *clt, unsi
 
  ret = evbuffer_add_printf(clt->clt_log,
     "%s %s - %s [%s] \"%s %s%s%s%s%s\""
-    " %03d %zu \"%s\" \"%s\"\n",
+    " %03d %zu \"%s\" \"%s\" %s %s\n",
     srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
     user, tstamp,
     server_httpmethod_byid(desc->http_method),
@@ -1740,7 +1764,9 @@ server_log_http(struct client *clt, unsi
     desc->http_version == NULL ? "" : version,
     code, len,
     referrer == NULL ? "" : referrer_v,
-    agent == NULL ? "" : agent_v);
+    agent == NULL ? "" : agent_v,
+    xff == NULL ? "-" : xff_v,
+    xfp == NULL ? "-" : xfp_v);
 
  break;
 
@@ -1762,6 +1788,8 @@ done:
  free(version);
  free(referrer_v);
  free(agent_v);
+ free(xff_v);
+ free(xfp_v);
 
  return (ret);
 }

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Bruno Flueckiger
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([hidden email]) 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 -0000 1.55
+++ usr.sbin/httpd/config.c 12 Feb 2019 13:37:55 -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.102
diff -u -p -r1.102 httpd.conf.5
--- usr.sbin/httpd/httpd.conf.5 8 Feb 2019 11:46:07 -0000 1.102
+++ usr.sbin/httpd/httpd.conf.5 12 Feb 2019 13:37:55 -0000
@@ -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
===================================================================
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 12 Feb 2019 13:37:55 -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.108
diff -u -p -r1.108 parse.y
--- usr.sbin/httpd/parse.y 8 Jan 2019 18:35:27 -0000 1.108
+++ usr.sbin/httpd/parse.y 12 Feb 2019 13:37:55 -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
@@ -976,6 +976,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 {
@@ -1305,7 +1309,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.129
diff -u -p -r1.129 server_http.c
--- usr.sbin/httpd/server_http.c 10 Feb 2019 13:41:27 -0000 1.129
+++ usr.sbin/httpd/server_http.c 12 Feb 2019 13:37:55 -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, *xfp;
  struct tm *tm;
  struct server_config *srv_conf;
  struct http_descriptor *desc;
@@ -1642,6 +1642,8 @@ server_log_http(struct client *clt, unsi
  char *version = NULL;
  char *referrer_v = NULL;
  char *agent_v = NULL;
+ char *xff_v = NULL;
+ char *xfp_v = NULL;
 
  if ((srv_conf = clt->clt_srv_conf) == NULL)
  return (-1);
@@ -1667,6 +1669,9 @@ 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\""
+ *
+ * Setting the log option xff will change the combined format to:
+ * "%v %h %l %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\" %a %{remote}p"
  */
  switch (srv_conf->logformat) {
  case LOG_FORMAT_COMMON:
@@ -1708,6 +1713,19 @@ server_log_http(struct client *clt, unsi
     agent->kv_value == NULL)
  agent = NULL;
 
+ xff = xfp = 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;
+
+ key.kv_key = "X-Forwarded-Port";
+ if (((xfp = kv_find(&desc->http_headers, &key)) != NULL
+    && xfp->kv_value == NULL))
+ xfp = NULL;
+ }
+
  /* Use vis to encode input values from the header */
  if (clt->clt_remote_user &&
     stravis(&user, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
@@ -1726,6 +1744,14 @@ server_log_http(struct client *clt, unsi
     stravis(&agent_v, agent->kv_value, HTTPD_LOGVIS) == -1)
  goto done;
 
+ if (xff &&
+    stravis(&xff_v, xff->kv_value, HTTPD_LOGVIS) == -1)
+ goto done;
+
+ if (xfp &&
+    stravis(&xfp_v, xfp->kv_value, HTTPD_LOGVIS) == -1)
+ goto done;
+
  /* The following should be URL-encoded */
  if (desc->http_path &&
     (path = url_encode(desc->http_path)) == NULL)
@@ -1736,7 +1762,7 @@ server_log_http(struct client *clt, unsi
 
  ret = evbuffer_add_printf(clt->clt_log,
     "%s %s - %s [%s] \"%s %s%s%s%s%s\""
-    " %03d %zu \"%s\" \"%s\"\n",
+    " %03d %zu \"%s\" \"%s\" %s %s\n",
     srv_conf->name, ip, user == NULL ? "-" :
     user, tstamp,
     server_httpmethod_byid(desc->http_method),
@@ -1747,7 +1773,9 @@ server_log_http(struct client *clt, unsi
     desc->http_version == NULL ? "" : version,
     code, len,
     referrer == NULL ? "" : referrer_v,
-    agent == NULL ? "" : agent_v);
+    agent == NULL ? "" : agent_v,
+    xff == NULL ? "-" : xff_v,
+    xfp == NULL ? "-" : xfp_v);
 
  break;
 
@@ -1769,6 +1797,8 @@ done:
  free(version);
  free(referrer_v);
  free(agent_v);
+ free(xff_v);
+ free(xfp_v);
 
  return (ret);
 }

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] httpd: Write X-Forwarded-For to access.log

Mischa-2

> On 12 Feb 2019, at 14:52, Bruno Flueckiger <[hidden email]> 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([hidden email]) 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 -0000 1.55
> +++ usr.sbin/httpd/config.c 12 Feb 2019 13:37:55 -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.102
> diff -u -p -r1.102 httpd.conf.5
> --- usr.sbin/httpd/httpd.conf.5 8 Feb 2019 11:46:07 -0000 1.102
> +++ usr.sbin/httpd/httpd.conf.5 12 Feb 2019 13:37:55 -0000
> @@ -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
> ===================================================================
> 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 12 Feb 2019 13:37:55 -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.108
> diff -u -p -r1.108 parse.y
> --- usr.sbin/httpd/parse.y 8 Jan 2019 18:35:27 -0000 1.108
> +++ usr.sbin/httpd/parse.y 12 Feb 2019 13:37:55 -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
> @@ -976,6 +976,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 {
> @@ -1305,7 +1309,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.129
> diff -u -p -r1.129 server_http.c
> --- usr.sbin/httpd/server_http.c 10 Feb 2019 13:41:27 -0000 1.129
> +++ usr.sbin/httpd/server_http.c 12 Feb 2019 13:37:55 -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, *xfp;
> struct tm *tm;
> struct server_config *srv_conf;
> struct http_descriptor *desc;
> @@ -1642,6 +1642,8 @@ server_log_http(struct client *clt, unsi
> char *version = NULL;
> char *referrer_v = NULL;
> char *agent_v = NULL;
> + char *xff_v = NULL;
> + char *xfp_v = NULL;
>
> if ((srv_conf = clt->clt_srv_conf) == NULL)
> return (-1);
> @@ -1667,6 +1669,9 @@ 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\""
> + *
> + * Setting the log option xff will change the combined format to:
> + * "%v %h %l %u %t \"%r\" %>s %B \"%{Referer}i\" \"%{User-agent}i\" %a %{remote}p"
> */
> switch (srv_conf->logformat) {
> case LOG_FORMAT_COMMON:
> @@ -1708,6 +1713,19 @@ server_log_http(struct client *clt, unsi
>    agent->kv_value == NULL)
> agent = NULL;
>
> + xff = xfp = 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;
> +
> + key.kv_key = "X-Forwarded-Port";
> + if (((xfp = kv_find(&desc->http_headers, &key)) != NULL
> +    && xfp->kv_value == NULL))
> + xfp = NULL;
> + }
> +
> /* Use vis to encode input values from the header */
> if (clt->clt_remote_user &&
>    stravis(&user, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
> @@ -1726,6 +1744,14 @@ server_log_http(struct client *clt, unsi
>    stravis(&agent_v, agent->kv_value, HTTPD_LOGVIS) == -1)
> goto done;
>
> + if (xff &&
> +    stravis(&xff_v, xff->kv_value, HTTPD_LOGVIS) == -1)
> + goto done;
> +
> + if (xfp &&
> +    stravis(&xfp_v, xfp->kv_value, HTTPD_LOGVIS) == -1)
> + goto done;
> +
> /* The following should be URL-encoded */
> if (desc->http_path &&
>    (path = url_encode(desc->http_path)) == NULL)
> @@ -1736,7 +1762,7 @@ server_log_http(struct client *clt, unsi
>
> ret = evbuffer_add_printf(clt->clt_log,
>    "%s %s - %s [%s] \"%s %s%s%s%s%s\""
> -    " %03d %zu \"%s\" \"%s\"\n",
> +    " %03d %zu \"%s\" \"%s\" %s %s\n",
>    srv_conf->name, ip, user == NULL ? "-" :
>    user, tstamp,
>    server_httpmethod_byid(desc->http_method),
> @@ -1747,7 +1773,9 @@ server_log_http(struct client *clt, unsi
>    desc->http_version == NULL ? "" : version,
>    code, len,
>    referrer == NULL ? "" : referrer_v,
> -    agent == NULL ? "" : agent_v);
> +    agent == NULL ? "" : agent_v,
> +    xff == NULL ? "-" : xff_v,
> +    xfp == NULL ? "-" : xfp_v);
>
> break;
>
> @@ -1769,6 +1797,8 @@ done:
> free(version);
> free(referrer_v);
> free(agent_v);
> + free(xff_v);
> + free(xfp_v);
>
> return (ret);
> }