On Fri, Mar 08, 2019 at 10:52:28AM +0100, Reyk Floeter wrote:
> 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?
That would be really nice to have. Did this slip through the cracks or
are there concerns with this diff?
>
> 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);
> > }
> >
>
