Theo Buehler([email protected]) on 2019.05.03 04:59:16 +0200:
> 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?

Hi,

please commit it, my ok too.

If it turns out that there are still problems with any tool, we can come
back to it, but as far as i know these tools should work with this output.

/Benno

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

Reply via email to