Re: cross-reference htobe64(3) in htonl(3)
On Tue, Feb 12, 2019 at 04:46:30PM +0100, Tim Kuijsten wrote: > Found out about htobe64(3) after grepping through the source. > > -Tim morning. fixed, thanks. jmc > Index: htonl.3 > === > RCS file: /cvs/src/lib/libc/net/htonl.3,v > retrieving revision 1.4 > diff -u -p -u -r1.4 htonl.3 > --- htonl.3 10 Mar 2016 08:42:26 - 1.4 > +++ htonl.3 12 Feb 2019 15:41:13 - > @@ -85,7 +85,8 @@ and > .Xr getservent 3 . > .Sh SEE ALSO > .Xr gethostbyname 3 , > -.Xr getservent 3 > +.Xr getservent 3 , > +.Xr htobe64 3 > .Sh STANDARDS > The > .Fn htonl ,
Why both media and -mediaopt doesn't specify at the same time?
Hi, When I execute `ifconfig media XX -mediaopt YY' command, it occured error as below. $ doas ifconfig em1 media 100baseTX -mediaopt full-duplex ifconfig: may not issue both `media' and `-mediaopt' $ echo $? 1 Does anyone knows this reason? I think following patch is works fine. $ cvs diff ifconfig.c Index: ifconfig.c === RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v retrieving revision 1.379 diff -u -p -r1.379 ifconfig.c --- ifconfig.c 30 Sep 2018 18:19:24 - 1.379 +++ ifconfig.c 13 Feb 2019 01:58:18 - @@ -2797,10 +2797,6 @@ unsetmediaopt(const char *val, int d) if (actions & A_MEDIAOPTCLR) errx(1, "only one `-mediaopt' command may be issued"); - /* May not issue `media' and `-mediaopt'. */ - if (actions & A_MEDIA) - errx(1, "may not issue both `media' and `-mediaopt'"); - /* * No need to check for A_MEDIAINST, since the test for A_MEDIA * implicitly checks for A_MEDIAINST. -- ASOU Masato
make sbcreatecontrol take void * instead of caddr_t
this makes it easier to call sbcreatecontrol without requiring casts.
it makes the argument const as well, and promotes the length variable to
size_t.
ok?
Index: sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.88
diff -u -p -r1.88 socketvar.h
--- sys/socketvar.h 19 Nov 2018 13:15:37 - 1.88
+++ sys/socketvar.h 12 Feb 2019 23:52:42 -
@@ -290,7 +290,7 @@ int sbappendcontrol(struct socket *, str
void sbappendrecord(struct socket *, struct sockbuf *, struct mbuf *);
void sbcompress(struct sockbuf *sb, struct mbuf *m, struct mbuf *n);
struct mbuf *
- sbcreatecontrol(caddr_t p, int size, int type, int level);
+ sbcreatecontrol(const void *, size_t, int type, int level);
void sbdrop(struct socket *, struct sockbuf *, int);
void sbdroprecord(struct sockbuf *sb);
void sbflush(struct socket *, struct sockbuf *);
Index: kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.99
diff -u -p -r1.99 uipc_socket2.c
--- kern/uipc_socket2.c 19 Nov 2018 13:15:37 - 1.99
+++ kern/uipc_socket2.c 12 Feb 2019 23:52:42 -
@@ -1020,14 +1020,14 @@ sbdroprecord(struct sockbuf *sb)
* with the specified type for presentation on a socket buffer.
*/
struct mbuf *
-sbcreatecontrol(caddr_t p, int size, int type, int level)
+sbcreatecontrol(const void *p, size_t size, int type, int level)
{
struct cmsghdr *cp;
struct mbuf *m;
if (CMSG_SPACE(size) > MCLBYTES) {
- printf("sbcreatecontrol: message too large %d\n", size);
- return NULL;
+ printf("sbcreatecontrol: message too large %zu\n", size);
+ return (NULL);
}
if ((m = m_get(M_DONTWAIT, MT_CONTROL)) == NULL)
Re: futex(2) based pthread_rwlock*
On Wed, Jan 30, 2019 at 12:29:20PM -0200, Martin Pieuchot wrote:
> On 28/01/19(Mon) 14:57, Martin Pieuchot wrote:
> > Summer is really warm here. No need to make my machines hotter by
> > spinning a lot. So here's a futex(2) based pthread_rwlock*
> > implementation. I should look familiar to those of you that reviewed
> > the mutex & semaphore implementations. It uses amotic cas & inc/dec.
> >
> > I moved the existing implementation to rthread_rwlock_compat.c to match
> > what has been done for semaphores.
> >
> > Tests, benchmarks and reviews are more than welcome!
>
> Newer version with fewer atomics and some barrier corrections as pointed
> out by visa@.
Appologies again for the late review.
I sprinkled various comments inline. The implementation is nice, clean
and easy to read! Except, perhaps, for the COUNT macro.
So I am generally OK with this diff. One question though, is the manual
page statement about preventing writer starvation still true with your
version? I don't think so. Or perhaps I am missing the bit where a
reader checks if a writer is waiting for the lock...
It is late here. I'll have a fresh look in the morning.
This is my review until then.
>
> Index: libc/include/thread_private.h
> ===
> RCS file: /cvs/src/lib/libc/include/thread_private.h,v
> retrieving revision 1.34
> diff -u -p -r1.34 thread_private.h
> --- libc/include/thread_private.h 10 Jan 2019 18:45:33 - 1.34
> +++ libc/include/thread_private.h 30 Jan 2019 14:21:16 -
> @@ -298,6 +298,10 @@ struct pthread_cond {
> struct pthread_mutex *mutex;
> };
>
> +struct pthread_rwlock {
> + volatile unsigned int value;
> +};
> +
> #else
>
> struct pthread_mutex {
> @@ -314,6 +318,13 @@ struct pthread_cond {
> struct pthread_queue waiters;
> struct pthread_mutex *mutex;
> clockid_t clock;
> +};
> +
> +struct pthread_rwlock {
> + _atomic_lock_t lock;
> + pthread_t owner;
> + struct pthread_queue writers;
> + int readers;
> };
> #endif /* FUTEX */
>
> Index: librthread/Makefile
> ===
> RCS file: /cvs/src/lib/librthread/Makefile,v
> retrieving revision 1.54
> diff -u -p -r1.54 Makefile
> --- librthread/Makefile 12 Jan 2019 00:16:03 - 1.54
> +++ librthread/Makefile 23 Jan 2019 20:07:29 -
> @@ -27,7 +27,6 @@ SRCS= rthread.c \
> rthread_mutex_prio.c \
> rthread_mutexattr.c \
> rthread_np.c \
> - rthread_rwlock.c \
> rthread_rwlockattr.c \
> rthread_sched.c \
> rthread_stack.c \
> @@ -40,9 +39,12 @@ SRCS= rthread.c \
> ${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "mips64" || \
> ${MACHINE_ARCH} == "mips64el" || ${MACHINE_ARCH} == "powerpc" || \
> ${MACHINE_ARCH} == "sparc64"
> -SRCS+= rthread_sem.c
> +CFLAGS+= -DFUTEX
> +SRCS+= rthread_sem.c \
> + rthread_rwlock.c
> .else
> -SRCS+= rthread_sem_compat.c
> +SRCS+= rthread_sem_compat.c \
> + rthread_rwlock_compat.c
> .endif
>
> SRCDIR= ${.CURDIR}/../libpthread
> Index: librthread/rthread.h
> ===
> RCS file: /cvs/src/lib/librthread/rthread.h,v
> retrieving revision 1.63
> diff -u -p -r1.63 rthread.h
> --- librthread/rthread.h 5 Sep 2017 02:40:54 - 1.63
> +++ librthread/rthread.h 23 Jan 2019 17:02:51 -
> @@ -52,13 +52,6 @@ struct stack {
> #define PTHREAD_MAX_PRIORITY31
>
>
> -struct pthread_rwlock {
> - _atomic_lock_t lock;
> - pthread_t owner;
> - struct pthread_queue writers;
> - int readers;
> -};
> -
> struct pthread_rwlockattr {
> int pshared;
> };
> Index: librthread/rthread_rwlock.c
> ===
> RCS file: /cvs/src/lib/librthread/rthread_rwlock.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 rthread_rwlock.c
> --- librthread/rthread_rwlock.c 24 Apr 2018 16:28:42 - 1.11
> +++ librthread/rthread_rwlock.c 30 Jan 2019 14:28:01 -
> @@ -1,8 +1,7 @@
> /* $OpenBSD: rthread_rwlock.c,v 1.11 2018/04/24 16:28:42 pirofti Exp $ */
> /*
> - * Copyright (c) 2004,2005 Ted Unangst
> + * Copyright (c) 2019 Martin Pieuchot
> * Copyright (c) 2012 Philip Guenther
> - * All Rights Reserved.
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that the above
> @@ -16,11 +15,7 @@
> * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> */
> -/*
> - * rwlocks
> - */
>
> -#include
> #include
> #include
> #include
> @@ -28,6 +23,20 @@
> #include
>
> #include "rthread.h"
> +#include "synch.h"
> +
> +#define UNLOCKED 0
> +#define MAXREADER0x7ffe
> +#define
Re: bgpd handle no peers a bit better
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.02.12 12:09:30 +0100:
> For testing I was running bgpd with no neighbors defined.
> In that case the peers pointer is NULL and so bgpctl is reporting suddenly
> 'no such neighbor' which is confusing for something like 'bgpctl show rib'.
> This is kind of a regression from adding group support in bgpctl.
>
> # bgpctl show rib
> flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
>S = Stale, E = Error
> origin validation state: N = not-found, V = valid, ! = invalid
> origin: i = IGP, e = EGP, ? = Incomplete
>
> flags ovs destination gateway lpref med aspath origin
> no such neighbor
>
> With this diff the more expected behaviour is back:
> bgpctl show rib
> flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
>S = Stale, E = Error
> origin validation state: N = not-found, V = valid, ! = invalid
> origin: i = IGP, e = EGP, ? = Incomplete
>
> flags ovs destination gateway lpref med aspath origin
> AI*>N 1.0.1.0/24 0.0.0.0100 0 i
> AI*>N 1.0.2.0/24 0.0.0.0100 0 i
>
> Anyone cares enough for such an edge case?
ok benno@
> --
> :wq Claudio
>
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 control.c
> --- control.c 20 Jan 2019 23:27:48 - 1.94
> +++ control.c 12 Feb 2019 11:00:19 -
> @@ -337,7 +337,7 @@ control_dispatch_msg(struct pollfd *pfd,
> }
> }
> }
> - if (!matched) {
> + if (!matched && peers != NULL) {
> control_result(c, CTL_RES_NOSUCHPEER);
> } else if (!neighbor || !neighbor->show_timers) {
> imsg_ctl_rde(IMSG_CTL_END, imsg.hdr.pid,
> @@ -461,7 +461,7 @@ control_dispatch_msg(struct pollfd *pfd,
> for (p = peers; p != NULL; p = p->next)
> if (peer_matched(p, neighbor))
> break;
> - if (p == NULL) {
> + if (p == NULL && peers != NULL) {
> control_result(c, CTL_RES_NOSUCHPEER);
> break;
> }
>
cross-reference htobe64(3) in htonl(3)
Found out about htobe64(3) after grepping through the source. -Tim Index: htonl.3 === RCS file: /cvs/src/lib/libc/net/htonl.3,v retrieving revision 1.4 diff -u -p -u -r1.4 htonl.3 --- htonl.3 10 Mar 2016 08:42:26 - 1.4 +++ htonl.3 12 Feb 2019 15:41:13 - @@ -85,7 +85,8 @@ and .Xr getservent 3 . .Sh SEE ALSO .Xr gethostbyname 3 , -.Xr getservent 3 +.Xr getservent 3 , +.Xr htobe64 3 .Sh STANDARDS The .Fn htonl ,
bgpd, support wildcard matching on ext-community
While it has been possible to do wildcard matching on communities and
large communities for a long time, ext-communities did not offer this.
The result is more complex rulesets when cleaning ext-communities.
This diff allows the same use of '*' on ext-communities and also adds
support for local-as and neighbor-as on some forms of ext-communities.
Since the encoding of ext-communities is a bit strange not all options are
possible. Here a list of possibilities:
ext-community * * # delete any ext-community
ext-community ovs * # delete any ext-community of specified type
ext-community rt 1.2.3.4:*
ext-community rt local-as:*
ext-community rt 65001:*
ext-community rt 65001:1
ext-community rt 65001:local-as
ext-community rt local-as:1 # not this defaults to the 4-byte AS encoding
There is a bit of a gotcha since sometimes the type of the ext-community
is underspecified when using wildchars or expands. So 'ext-community rt *'
or 'ext-community soo *' will match for any of the 3 possible types
(2-byte AS, 4-byte AS and IP address). If local-as/neighbor-as is used as
expand of as-number (e.g. ext-community rt local-as:1) then bgpd will
default to the 4-byte AS type to encode the community.
ext-community encoding is a bit of a desaster and so the wildcard matching
may be a bit more limited then we probably would like.
As usual comments, test reports and OKs welcome :)
--
:wq Claudio
PS: In general people are advised to use large-communities whenever proper
expanding of neighbor-as is required since that is the only format
supporting 4-byte values in all cases.
Index: bgpd.conf.5
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.185
diff -u -p -r1.185 bgpd.conf.5
--- bgpd.conf.5 11 Feb 2019 17:45:59 - 1.185
+++ bgpd.conf.5 12 Feb 2019 15:13:56 -
@@ -1354,14 +1354,9 @@ and
.Ar local
may be set to
.Sq *
-to do wildcard matching.
-Both
-.Ar as-number
-and
-.Ar local
-may be set to
+to do wildcard matching,
.Ic neighbor-as ,
-which is expanded to the current neighbor remote AS number,
+which is expanded to the current neighbor remote AS number, or
.Ic local-as ,
which is expanded to the locally assigned AS number.
.Pp
@@ -1379,7 +1374,7 @@ which is expanded to the locally assigne
.Xc
.It Xo
.Ic ext-community
-.Ar ovs
+.Ic ovs
.Pq Ic valid | not-found | invalid
.Xc
This rule applies only to
@@ -1391,6 +1386,26 @@ Extended Communities are specified by a
.Ar subtype
and normally two values, a globally unique part (e.g. the AS number) and a
local part.
+Both
+.Ar as-number
+and
+.Ar local
+may be set to
+.Ic neighbor-as ,
+which is expanded to the current neighbor remote AS number, or
+.Ic local-as ,
+which is expanded to the locally assigned AS number.
+Wildcard matching is supported for
+.Ar local ,
+.Ar numvalue
+and
+.Ar subtype .
+If wildcard matching is used on the
+.Ar subtype
+then
+.Ar numvalue
+also needs to be set to
+.Sq * .
See also the
.Sx ATTRIBUTE SET
section for further information about the encoding.
@@ -1689,7 +1704,7 @@ to do wildcard matching.
.Xc
.It Xo
.Ic ext-community Op Ar delete
-.Ar ovs
+.Ic ovs
.Pq Ic valid | not-found | invalid
.Xc
Set or delete the
@@ -1736,6 +1751,22 @@ vrfriVRF Route Import
.Pp
Not all type and subtype value pairs are allowed by IANA and the parser
will ensure that no invalid combination is created.
+.Pp
+For
+.Cm delete ,
+.Ar subtype ,
+.Ar numvalue ,
+or
+.Ar local ,
+may be set to
+.Sq *
+to do wildcard matching.
+If wildcard matching is used on the
+.Ar subtype
+then
+.Ar numvalue
+also needs to be set to
+.Sq * .
.Pp
.It Ic localpref Ar number
Set the
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.365
diff -u -p -r1.365 bgpd.h
--- bgpd.h 11 Feb 2019 15:44:25 - 1.365
+++ bgpd.h 11 Feb 2019 17:15:03 -
@@ -772,7 +772,7 @@ struct filter_community {
struct ext {
u_int32_t data1;
u_int64_t data2;
- u_int8_ttype;
+ short type;
u_int8_tsubtype;/* if extended type */
} e;
} c;
@@ -865,8 +865,10 @@ struct filter_peers {
#define EXT_COMMUNITY_NON_TRANS_IPV4 0x41/* IPv4 specific */
#define EXT_COMMUNITY_NON_TRANS_FOUR_AS0x42/* 4 octet AS specific
*/
#define EXT_COMMUNITY_NON_TRANS_OPAQUE 0x43/* opaque ext community */
+#define EXT_COMMUNITY_UNKNOWN -1
/* BGP Origin Validation State Extended Community RFC8097 */
+#define EXT_COMMUNITY_SUBTYPE_OVS 0
#define EXT_COMMUNITY_OVS_VALID0
#define EXT_COMMUNITY_OVS_NOTFOUND 1
#define EXT_COMMUNITY_OVS_INVALID 2
@@ -876,7 +878,7 @@ struct filter_peers {
#define EXT_COMMUNITY_FLAG_VALID
httpd(8): add support for FastCGI parameters
Hi,
This patch adds support for FastCGI parameters.
Grammar change is very simple.
fastcgi param NAME VALUE
Example:
%--
server "default" {
listen on $ext_addr port 80
location "/cgi-bin/*" {
fastcgi socket "/run/slowcgi.sock"
fastcgi param TEST1 hello
fastcgi param TEST2 world
# The /cgi-bin directory is outside of the document root
root "/"
}
}
%--
Output:
%--
CGI/1.1 test script report:
GATEWAY_INTERFACE = CGI/1.1
SERVER_SOFTWARE = OpenBSD httpd
SERVER_PROTOCOL = HTTP/1.1
SERVER_NAME = default
SERVER_ADDR = 127.0.0.1
SERVER_PORT = 80
REQUEST_METHOD = GET
HTTP_ACCEPT =
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
SCRIPT_NAME = /cgi-bin/testcgi
REQUEST_URI = /cgi-bin/testcgi
PATH_INFO =
QUERY_STRING =
REMOTE_ADDR = 127.0.0.1
REMOTE_PORT = 28054
CONTENT_TYPE =
CONTENT_LENGTH =
TEST1 = hello
TEST2 = world
PATH =
%--
Initially discussed with florian@.
Thoughts? Silly bike-shedding about the grammar? OKs?
Manual page changes after grammar is OK'ed.
Paul
Index: config.c
===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.55
diff -u -p -u -p -r1.55 config.c
--- config.c20 Jun 2018 16:43:05 - 1.55
+++ config.c12 Feb 2019 14:51:23 -
@@ -231,6 +231,9 @@ config_setserver(struct httpd *env, stru
__func__, srv->srv_conf.name);
return (-1);
}
+
+ /* Configure FCGI parmeters if necessary. */
+ config_setserver_fcgiparams(env, srv);
}
}
@@ -289,6 +292,90 @@ config_settls(struct httpd *env, struct
tls.tls_chunk_offset += tls.tls_chunk_len;
data += tls.tls_chunk_len;
len -= tls.tls_chunk_len;
+ }
+
+ return (0);
+}
+
+int
+config_getserver_fcgiparams(struct httpd *env, struct imsg *imsg)
+{
+ struct server *srv;
+ struct server_config*srv_conf, *iconf;
+ struct fastcgi_param*fp;
+ uint32_t id;
+ size_t c, nc;
+ uint8_t *p = imsg->data;
+
+ memcpy(&nc, p, sizeof(nc)); /* number of params */
+ p += sizeof(nc);
+
+ memcpy(&id, p, sizeof(id)); /* server conf id */
+ srv_conf = serverconfig_byid(id);
+ p += sizeof(id);
+
+ /* Find associated server config */
+ TAILQ_FOREACH(srv, env->sc_servers, srv_entry) {
+ if (srv->srv_conf.id == id) {
+ srv_conf = &srv->srv_conf;
+ break;
+ }
+ TAILQ_FOREACH(iconf, &srv->srv_hosts, entry) {
+ if (iconf->id == id) {
+ srv_conf = iconf;
+ break;
+ }
+ }
+ }
+
+ /* Fetch FCGI parameters */
+ for (c = 0; c < nc; c++) {
+ if ((fp = calloc(1, sizeof(*fp))) == NULL)
+ return (-1);
+ memcpy(fp, p, sizeof(*fp));
+ TAILQ_INSERT_HEAD(&srv_conf->fcgiparams, fp, entry);
+
+ p += sizeof(*fp);
+ }
+
+ return (0);
+}
+
+int
+config_setserver_fcgiparams(struct httpd *env, struct server *srv)
+{
+ struct privsep *ps = env->sc_ps;
+ struct server_config*srv_conf = &srv->srv_conf;
+ struct fastcgi_param *fp;
+ struct iovec *iov;
+ size_t c = 0, nc = 0;
+
+ DPRINTF("%s: sending fcgiparam for \"%s[%u]\" to %s fd %d", __func__,
+ srv_conf->name, srv_conf->id, ps->ps_title[PROC_SERVER],
+ srv->srv_s);
+
+ if (TAILQ_EMPTY(&srv_conf->fcgiparams)) /* nothing to do */
+ return (0);
+
+ TAILQ_FOREACH(fp, &srv_conf->fcgiparams, entry) {
+ nc++;
+ }
+ if ((iov = calloc(nc + 2, sizeof(*iov))) == NULL)
+ return (-1);
+
+ iov[c].iov_base = &nc; /* number of params */
+ iov[c++].iov_len = sizeof(nc);
+ iov[c].iov_base = &srv_conf->id;/* server config id */
+ iov[c++].iov_len = sizeof(srv_conf->id);
+
+ TAILQ_FOREACH(fp, &srv_conf->fcgiparams, entry) { /* push FCGI
params */
+ iov[c].iov_base = fp;
+ iov[c++].iov_len = sizeof(*fp);
+ }
+ if (proc_composev(ps, PROC_SERVER, IMSG_CFG_FCGI, iov, c) != 0) {
+ log
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
> On 12 Feb 2019, at 14:52, Bruno Flueckiger wrote:
>
> On 12.11.18 12:40, Bruno Flueckiger wrote:
>> On 11.11.18 18:43, Claudio Jeker wrote:
>>> On Sun, Nov 11, 2018 at 06:32:53PM +0100, Bruno Flueckiger wrote:
On 11.11.18 15:29, Florian Obser wrote:
> On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
>> Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
>>> Hi
>>>
>>> When I run httpd(8) behind relayd(8) the access log of httpd contains
>>> the IP address of relayd, but not the IP address of the client. I've
>>> tried to match the logs of relayd(8) and httpd(8) using some scripting
>>> and failed.
>>>
>>> So I've written a patch for httpd(8). It stores the content of the
>>> X-Forwarded-For header in the third field of the log entry:
>>>
>>> www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
>>>
>>> Cheers,
>>> Bruno
>>
>> I'm not sure we should do this unconditionally. With no relayd or other
>> proxy infront of httpd, this is (more) data the client controls.
>
> Isn't what httpd(8) currently logs apache's common log format? If
> people are shoving that through webalizer or something that will
> break. I don't think we can do this without a config option.
> Do we need LogFormat?
>
>>
>> Could this be a problem?
>>
>> code reads ok.
>>
>> /Benno
>>
I've extended my patch with an option to the log directive. Both log xff
and log combined must be set for a server to log the content of the
X-Forwarded-For header. If only log combined is set the log entries
remain in the well-known format.
This prevents clients from getting unwanted data in the log by default.
And it makes sure the log format remains default until one decides
actively to change it.
>>>
>>> From my experience with webservices is that today logging the IP is
>>> seldomly good enough. Please also include X-Forwarded-Port and maybe
>>> X-Forwarded-Proto.
>>> In general thanks to CG-NAT logging only IP is a bit pointless.
>>>
>>> --
>>> :wq Claudio
>>>
>>
>> Thanks for the hint, Claudio.
>>
>> This version of my diff includes the two headers X-Forwarded-For and
>> X-Forwarded-Port. Instead of using the third field of the log entry I
>> add two additional fileds to it. The first one contains the value of
>> X-Forwarded-For and the second one that of X-Forwarded-Port.
>>
>> I think that appending two fields might do less harm than replacing one
>> field at the beginning of the log entry. I'm not sure that adding
>> X-Forwarded-Proto to the log really brings a benefit, so I left it away
>> in this diff.
>
> In the meantime I've run my diff on a webserver. In my experience
> webalizer has no problem with the modified log format. GoAccess on the
> other hand has troubles reading access.log, but that happens for me with
> and without the diff applied.
>
> I think that most admins would profit from the diff. The setting is
> optional so it doesn't affect everybody rightaway. And I believe that
> those who enable it are ready to reconfigure whatever log parser they
> use.
>
> Therefore I have reworked my diff so it applies to the -current tree.
Would be a very welcome addition to httpd.
Mischa
>
> Index: usr.sbin/httpd/config.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/config.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 config.c
> --- usr.sbin/httpd/config.c 20 Jun 2018 16:43:05 - 1.55
> +++ usr.sbin/httpd/config.c 12 Feb 2019 13:37:55 -
> @@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
> if ((srv_conf->flags & f) == 0)
> srv_conf->flags |= parent->flags & f;
>
> + f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
> + if ((srv_conf->flags & f) == 0)
> + srv_conf->flags |= parent->flags & f;
> +
> f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
> if ((srv_conf->flags & f) == 0) {
> srv_conf->flags |= parent->flags & f;
> Index: usr.sbin/httpd/httpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.102
> diff -u -p -r1.102 httpd.conf.5
> --- usr.sbin/httpd/httpd.conf.5 8 Feb 2019 11:46:07 - 1.102
> +++ usr.sbin/httpd/httpd.conf.5 12 Feb 2019 13:37:55 -
> @@ -464,6 +464,16 @@ If not specified, the default is
> Enable or disable logging to
> .Xr syslog 3
> instead of the log files.
> +.It Oo Ic no Oc Ic xff
> +Enable or disable logging of the request headers
> +.Ar X-Forwarded-For
> +and
> +.Ar X-Forwarded-Port
> +if
> +.Cm log combined
> +is set. These headers can be set by a reverse proxy like
> +.Xr relayd 8
> +and should contain the IP address and TCP port of the client.
> .El
> .
Re: [PATCH] httpd: Write X-Forwarded-For to access.log
On 12.11.18 12:40, Bruno Flueckiger wrote:
> On 11.11.18 18:43, Claudio Jeker wrote:
> > On Sun, Nov 11, 2018 at 06:32:53PM +0100, Bruno Flueckiger wrote:
> > > On 11.11.18 15:29, Florian Obser wrote:
> > > > On Sun, Nov 11, 2018 at 01:46:06PM +0100, Sebastian Benoit wrote:
> > > > > Bruno Flueckiger(inform...@gmx.net) on 2018.11.11 10:31:34 +0100:
> > > > > > Hi
> > > > > >
> > > > > > When I run httpd(8) behind relayd(8) the access log of httpd
> > > > > > contains
> > > > > > the IP address of relayd, but not the IP address of the client. I've
> > > > > > tried to match the logs of relayd(8) and httpd(8) using some
> > > > > > scripting
> > > > > > and failed.
> > > > > >
> > > > > > So I've written a patch for httpd(8). It stores the content of the
> > > > > > X-Forwarded-For header in the third field of the log entry:
> > > > > >
> > > > > > www.example.com 192.0.2.99 192.0.2.134 - [11/Nov/2018:09:28:48 ...
> > > > > >
> > > > > > Cheers,
> > > > > > Bruno
> > > > >
> > > > > I'm not sure we should do this unconditionally. With no relayd or
> > > > > other
> > > > > proxy infront of httpd, this is (more) data the client controls.
> > > >
> > > > Isn't what httpd(8) currently logs apache's common log format? If
> > > > people are shoving that through webalizer or something that will
> > > > break. I don't think we can do this without a config option.
> > > > Do we need LogFormat?
> > > >
> > > > >
> > > > > Could this be a problem?
> > > > >
> > > > > code reads ok.
> > > > >
> > > > > /Benno
> > > > >
> > >
> > > I've extended my patch with an option to the log directive. Both log xff
> > > and log combined must be set for a server to log the content of the
> > > X-Forwarded-For header. If only log combined is set the log entries
> > > remain in the well-known format.
> > >
> > > This prevents clients from getting unwanted data in the log by default.
> > > And it makes sure the log format remains default until one decides
> > > actively to change it.
> >
> > From my experience with webservices is that today logging the IP is
> > seldomly good enough. Please also include X-Forwarded-Port and maybe
> > X-Forwarded-Proto.
> > In general thanks to CG-NAT logging only IP is a bit pointless.
> >
> > --
> > :wq Claudio
> >
>
> Thanks for the hint, Claudio.
>
> This version of my diff includes the two headers X-Forwarded-For and
> X-Forwarded-Port. Instead of using the third field of the log entry I
> add two additional fileds to it. The first one contains the value of
> X-Forwarded-For and the second one that of X-Forwarded-Port.
>
> I think that appending two fields might do less harm than replacing one
> field at the beginning of the log entry. I'm not sure that adding
> X-Forwarded-Proto to the log really brings a benefit, so I left it away
> in this diff.
In the meantime I've run my diff on a webserver. In my experience
webalizer has no problem with the modified log format. GoAccess on the
other hand has troubles reading access.log, but that happens for me with
and without the diff applied.
I think that most admins would profit from the diff. The setting is
optional so it doesn't affect everybody rightaway. And I believe that
those who enable it are ready to reconfigure whatever log parser they
use.
Therefore I have reworked my diff so it applies to the -current tree.
Index: usr.sbin/httpd/config.c
===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.55
diff -u -p -r1.55 config.c
--- usr.sbin/httpd/config.c 20 Jun 2018 16:43:05 - 1.55
+++ usr.sbin/httpd/config.c 12 Feb 2019 13:37:55 -
@@ -427,6 +427,10 @@ config_getserver_config(struct httpd *en
if ((srv_conf->flags & f) == 0)
srv_conf->flags |= parent->flags & f;
+ f = SRVFLAG_XFF|SRVFLAG_NO_XFF;
+ if ((srv_conf->flags & f) == 0)
+ srv_conf->flags |= parent->flags & f;
+
f = SRVFLAG_AUTH|SRVFLAG_NO_AUTH;
if ((srv_conf->flags & f) == 0) {
srv_conf->flags |= parent->flags & f;
Index: usr.sbin/httpd/httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.102
diff -u -p -r1.102 httpd.conf.5
--- usr.sbin/httpd/httpd.conf.5 8 Feb 2019 11:46:07 - 1.102
+++ usr.sbin/httpd/httpd.conf.5 12 Feb 2019 13:37:55 -
@@ -464,6 +464,16 @@ If not specified, the default is
Enable or disable logging to
.Xr syslog 3
instead of the log files.
+.It Oo Ic no Oc Ic xff
+Enable or disable logging of the request headers
+.Ar X-Forwarded-For
+and
+.Ar X-Forwarded-Port
+if
+.Cm log combined
+is set. These headers can be set by a reverse proxy like
+.Xr relayd 8
+and should contain the IP address and TCP port of the client.
.El
.It Ic pass
Disable any previous
Index: usr.sbin/httpd/httpd.h
bgpd handle no peers a bit better
For testing I was running bgpd with no neighbors defined.
In that case the peers pointer is NULL and so bgpctl is reporting suddenly
'no such neighbor' which is confusing for something like 'bgpctl show rib'.
This is kind of a regression from adding group support in bgpctl.
# bgpctl show rib
flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
S = Stale, E = Error
origin validation state: N = not-found, V = valid, ! = invalid
origin: i = IGP, e = EGP, ? = Incomplete
flags ovs destination gateway lpref med aspath origin
no such neighbor
With this diff the more expected behaviour is back:
bgpctl show rib
flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
S = Stale, E = Error
origin validation state: N = not-found, V = valid, ! = invalid
origin: i = IGP, e = EGP, ? = Incomplete
flags ovs destination gateway lpref med aspath origin
AI*>N 1.0.1.0/24 0.0.0.0100 0 i
AI*>N 1.0.2.0/24 0.0.0.0100 0 i
Anyone cares enough for such an edge case?
--
:wq Claudio
Index: control.c
===
RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
retrieving revision 1.94
diff -u -p -r1.94 control.c
--- control.c 20 Jan 2019 23:27:48 - 1.94
+++ control.c 12 Feb 2019 11:00:19 -
@@ -337,7 +337,7 @@ control_dispatch_msg(struct pollfd *pfd,
}
}
}
- if (!matched) {
+ if (!matched && peers != NULL) {
control_result(c, CTL_RES_NOSUCHPEER);
} else if (!neighbor || !neighbor->show_timers) {
imsg_ctl_rde(IMSG_CTL_END, imsg.hdr.pid,
@@ -461,7 +461,7 @@ control_dispatch_msg(struct pollfd *pfd,
for (p = peers; p != NULL; p = p->next)
if (peer_matched(p, neighbor))
break;
- if (p == NULL) {
+ if (p == NULL && peers != NULL) {
control_result(c, CTL_RES_NOSUCHPEER);
break;
}
