Hi, On Mon, Mar 04, 2019 at 02:06:02PM +0100, Bruno Flueckiger wrote: > I've completely reworked my patch for httpd(8). The last patch broke the > log format combined. And the config option was ugly. This time I've > added another log format called forwarded. It appends two fields to the > log format combined: The first field contains the value of the header > X-Forwarded-For and the second one the value of X-Forwarded-Port. If > either of the headers is empty or missing a dash (-) is written. > > The new log format is compatible with log analyzing tools like Webalizer > or GoAccess. If you run httpd(8) behind a proxy like relayd(8) the new > log format finally gives you a way to track the origin of the requests. >
Your diff looks clean and makes a lot of sense. Especially since X-Forwarded-For is a feature in relayd that I first used and documented around 2006/2007. Adding the forwarded style to httpd is a complementary feature in OpenBSD and not something for a random external web stack. OK reyk@ Anyone else, any objections? Reyk > Cheers, > Bruno > > Index: usr.sbin/httpd/httpd.conf.5 > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v > retrieving revision 1.103 > diff -u -p -r1.103 httpd.conf.5 > --- usr.sbin/httpd/httpd.conf.5 19 Feb 2019 11:37:26 -0000 1.103 > +++ usr.sbin/httpd/httpd.conf.5 27 Feb 2019 15:26:48 -0000 > @@ -450,7 +450,8 @@ The > .Ar style > can be > .Cm common , > -.Cm combined > +.Cm combined , > +.Cm forwarded > or > .Cm connection . > The styles > @@ -459,6 +460,14 @@ and > .Cm combined > write a log entry after each request similar to the standard Apache > and nginx access log formats. > +The style > +.Cm forwarded > +extends the style > +.Cm combined > +by appending two fields containing the values of the headers > +.Ar X-Forwarded-For > +and > +.Ar X-Forwarded-Port . > The style > .Cm connection > writes a summarized log entry after each connection, > Index: usr.sbin/httpd/httpd.h > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v > retrieving revision 1.143 > diff -u -p -r1.143 httpd.h > --- usr.sbin/httpd/httpd.h 19 Feb 2019 11:37:26 -0000 1.143 > +++ usr.sbin/httpd/httpd.h 27 Feb 2019 15:26:48 -0000 > @@ -437,7 +437,8 @@ SPLAY_HEAD(client_tree, client); > enum log_format { > LOG_FORMAT_COMMON, > LOG_FORMAT_COMBINED, > - LOG_FORMAT_CONNECTION > + LOG_FORMAT_CONNECTION, > + LOG_FORMAT_FORWARDED > }; > > struct log_file { > Index: usr.sbin/httpd/parse.y > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v > retrieving revision 1.110 > diff -u -p -r1.110 parse.y > --- usr.sbin/httpd/parse.y 19 Feb 2019 11:37:26 -0000 1.110 > +++ usr.sbin/httpd/parse.y 27 Feb 2019 15:26:48 -0000 > @@ -140,7 +140,7 @@ typedef struct { > %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 CA CLIENT CRL OPTIONAL PARAM > +%token CA CLIENT CRL OPTIONAL PARAM FORWARDED > %token <v.string> STRING > %token <v.number> NUMBER > %type <v.port> port > @@ -1024,6 +1024,11 @@ logstyle : COMMON { > srv_conf->flags |= SRVFLAG_LOG; > srv_conf->logformat = LOG_FORMAT_CONNECTION; > } > + | FORWARDED { > + srv_conf->flags &= ~SRVFLAG_NO_LOG; > + srv_conf->flags |= SRVFLAG_LOG; > + srv_conf->logformat = LOG_FORMAT_FORWARDED; > + } > ; > > filter : block RETURN NUMBER optstring { > @@ -1295,6 +1300,7 @@ lookup(char *s) > { "ecdhe", ECDHE }, > { "error", ERR }, > { "fastcgi", FCGI }, > + { "forwarded", FORWARDED }, > { "hsts", HSTS }, > { "include", INCLUDE }, > { "index", INDEX }, > 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 27 Feb 2019 15:26:49 -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); > @@ -1698,6 +1700,7 @@ server_log_http(struct client *clt, unsi > break; > > case LOG_FORMAT_COMBINED: > + case LOG_FORMAT_FORWARDED: > key.kv_key = "Referer"; /* sic */ > if ((referrer = kv_find(&desc->http_headers, &key)) != NULL && > referrer->kv_value == NULL) > @@ -1734,9 +1737,9 @@ server_log_http(struct client *clt, unsi > (referrer_v = url_encode(referrer->kv_value)) == NULL) > goto done; > > - ret = evbuffer_add_printf(clt->clt_log, > + if ((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\"", > srv_conf->name, ip, user == NULL ? "-" : > user, tstamp, > server_httpmethod_byid(desc->http_method), > @@ -1747,7 +1750,38 @@ 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)) == -1) > + break; > + > + if (srv_conf->logformat == LOG_FORMAT_COMBINED) > + goto finish; > + > + xff = xfp = NULL; > + > + key.kv_key = "X-Forwarded-For"; > + if ((xff = kv_find(&desc->http_headers, &key)) != NULL > + && xff->kv_value == NULL) > + xff = NULL; > + > + if (xff && > + stravis(&xff_v, xff->kv_value, HTTPD_LOGVIS) == -1) > + goto finish; > + > + key.kv_key = "X-Forwarded-Port"; > + if ((xfp = kv_find(&desc->http_headers, &key)) != NULL > + && xfp->kv_value == NULL) > + xfp = NULL; > + > + if (xfp && > + stravis(&xfp_v, xfp->kv_value, HTTPD_LOGVIS) == -1) > + goto finish; > + > + if ((ret = evbuffer_add_printf(clt->clt_log, " %s %s", > + xff == NULL ? "-" : xff_v, > + xfp == NULL ? "-" : xfp_v)) == -1) > + break; > +finish: > + ret = evbuffer_add_printf(clt->clt_log, "\n"); > > break; > > @@ -1769,6 +1803,8 @@ done: > free(version); > free(referrer_v); > free(agent_v); > + free(xff_v); > + free(xfp_v); > > return (ret); > } >