> On 3 May 2019, at 04:59, Theo Buehler <[email protected]> wrote:
>
>> 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?
>
I believe it fell through the cracks. Would be super useful.
Mischa
>>
>> 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);
>>> }
>>>
>>
>
