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);
> }
>
