Re: pf state tracking and tos/dscp
Adam Gensler openbsd at kristenandadam.net writes: all udp 1.1.1.1:4500 - 172.28.1.3:4500 MULTIPLE:MULTIPLE age 00:15:50, expires in 00:00:57, 394:196 pkts, 52356:39176 bytes, rule 37 put pass all tos higher in your ruleset and see if it matches.
Re: pf state tracking and tos/dscp
Adam Gensler openbsd at kristenandadam.net writes:
local_nets = { 172.28.1.0/24, 172.28.10.0/24, 172.28.11.0/24 }
work871 = 172.28.1.3
pass in quick inet proto udp from $work871 tos 0xB8 tag VOIP-RTP
pass in quick inet proto udp from $work871 tos 0x60 tag VOIP-SIG
pass in quick inet proto { tcp, udp } from $local_nets
Another possible thing I see, is a tunnel originating side.
Since tos rules you have are unidirectional (in terms of match),
they will create state if only first packet comes from $work871.
However, first packet coming from other side will match another
rule and create state, so all subsequent tunnel's packets will
not hit tos rules.
Re: pf state tracking and tos/dscp
The TOS class isn't (and can't be) used to match packets to the state. Once you
have created state from a packet with one TOS class, other packets with the
same src/dest ip/port match this state even if the class is different. (It has
to be this way - say you are natting - you wouldn't want a different source
port if some packets happen to use different TOS).
PF states include two queue names - one for normal traffic, one for high
prioroty traffic: empty TCP ACKs and packets with TOS 'lowdelay' (0x10). So
perhaps you would get somewhere by using 'match in from $work871 tos 0xB8 set
tos lowdelay' to re-mark the incoming packets as 'lowdelay' and using e.g. 'set
queue (vpn, vpn-fast)'.
Otherwise, if you don't need NAT, disabling state tracking for these packets
might be an option.
Adam Gensler open...@kristenandadam.net wrote:
Hi all,
I've been playing with pf for a number of months now and I've come
across a situation that I'm having trouble finding a solution for.
Specifically I'm working with the following topology:
Internet --- OpenBSD box --- Cisco router --- other devices
The Cisco router (a small 800 series router) is sitting behind an
OpenBSD box (version 5.1). This Cisco router has an IPsec tunnel
connected to another router out on the Internet. So, all the OpenBSD
box sees is a bunch of encrypted frames (udp port 4500). That's all
fine and dandy, that works well, no problems there.
However, some of the devices behind the Cisco router are setting the
tos/dscp bits on their packets. I would like to be able to prioritize
those packets in pf for altq handling. So, I've created the following
rules:
local_nets = { 172.28.1.0/24, 172.28.10.0/24, 172.28.11.0/24 }
work871 = 172.28.1.3
pass in quick inet proto udp from $work871 tos 0xB8 tag VOIP-RTP
pass in quick inet proto udp from $work871 tos 0x60 tag VOIP-SIG
pass in quick inet proto { tcp, udp } from $local_nets
The idea here being that ingress traffic from 172.28.1.3, with the
various tos values will be tagged with a specific tag. On the egress
side I match on that tag and then apply it to a queue. That isn't
working though. It seems that PF creates a state entry first for the
overall ipsec tunnel using the third rule. After that state gets
established all subsequent packets do not evaluate the rules, even if
those packets have different tos values. This leaves me to believe that
pf isn't creating a state entry tuple that contains the tos value.
I've confirmed the TOS bits are being carried through to the IP header
of the IPSEC packets. Here's a tcpdump of the incoming packets from my
LAN interface (vr0):
tcpdump: listening on vr0, link-type EN10MB (Ethernet), capture size
65535 bytes
21:37:39.464357 IP (tos 0xc0, ttl 255, id 848, offset 0, flags [none],
proto UDP (17), length 144)
172.28.1.3.4500 1.1.1.1.4500: UDP-encap:
ESP(spi=0x32284280,seq=0x12e), length 116
21:37:39.483350 IP (tos 0xc0, ttl 255, id 1707, offset 0, flags [none],
proto UDP (17), length 29)
172.28.1.3.4500 64.102.7.50.4500: isakmp-nat-keep-alive
21:37:42.365389 IP (tos 0x0, ttl 255, id 849, offset 0, flags [none],
proto UDP (17), length 152)
172.28.1.3.4500 1.1.1.1.4500: UDP-encap:
ESP(spi=0x32284280,seq=0x12f), length 124
21:37:45.465724 IP (tos 0xc0, ttl 255, id 850, offset 0, flags [none],
proto UDP (17), length 144)
172.28.1.3.4500 1.1.1.1.4500: UDP-encap:
ESP(spi=0x32284280,seq=0x130), length 116
21:37:47.370081 IP (tos 0x0, ttl 255, id 851, offset 0, flags [none],
proto UDP (17), length 152)
172.28.1.3.4500 1.1.1.1.4500: UDP-encap:
ESP(spi=0x32284280,seq=0x131), length 124
21:37:49.256302 IP (tos 0x60, ttl 255, id 852, offset 0, flags [none],
proto UDP (17), length 120)
172.28.1.3.4500 1.1.1.1.4500: UDP-encap:
ESP(spi=0x32284280,seq=0x132), length 92
From this it's clear that the last packet has tos 0x60 set. However,
if I look at this particular rule, it doesn't have any matches or state
entries:
pass in quick inet proto udp from 172.28.1.3 to any tos 0x60 keep state
tag VOIP-SIG
[ Evaluations: 34Packets: 0 Bytes: 0 States:
0 ]
[ Inserted: uid 0 pid 13666 State Creations: 0 ]
Instead, there's a state entry logged for this traffic under the third
rule:
all udp 1.1.1.1:4500 - 172.28.1.3:4500 MULTIPLE:MULTIPLE
age 00:15:50, expires in 00:00:57, 394:196 pkts, 52356:39176 bytes,
rule 37
Based on the above, it seems like the state entries don't include the
tos information making it impossible to properly classify traffic that
is encrypted with ipsec. The only way to differentiate various traffic
streams contained within the tunnel is via tos/dscp. This is fairly
common practice in the Enterprise routing space. I'd love to be able to
do the same thing here.
Am I missing something obvious? I've tested OpenBSD 5.1 and (for what
its worth) FreeBSD 9.0 and also pfsense 2.1beta1, I see the same
behavior on all of them.
Thanks in advance,
Adam
Re: [PATCH] add filter by host functionality to syslogd
On 05/22/2013 06:39 PM, Ted Unangst wrote: On Wed, May 22, 2013 at 12:06, Gregory Edigarov wrote: works for me, with only one limitation: now only for resolvable hosts, i.e one cannot have +192.168.2.1 * /some/file Looking at the diff, I think it's not resolvable hosts, but whatever hostname the sending machine decides to tell you? no, it is really a resolvable hosts. works correctly with name in /etc/hosts. My first thought is that we shouldn't rely on that, and syslogd should refuse requests entirely from servers it doesn't like. My second thought is that's what pf is for and spoofing syslog entries is already pretty easy, so this is fine, but it needs to be documented as such. -- With best regards, Gregory Edigarov
Re: [PATCH] add filter by host functionality to syslogd
no, it is really a resolvable hosts. It is unsafe.
Re: route(8) use -inet6 automatically for addresses containing :
On 2013/05/23 15:03, Sebastian Benoit wrote:
Stuart Henderson(st...@openbsd.org) on 2013.05.22 21:18:05 +0100:
On 2013/05/22 20:47, Stuart Henderson wrote:
does anyone see a downside to this?
i see none other than making the maze of options of route(8) a bit more
bizarre.
if the address family is not
explicitly specified, assume v6 if it looks like it may be an ipv6
address.
allows e.g. route get 2001:200:dff:fff1:216:3eff:feb1:44d7
without needing to specify -inet6.
oops, as pointed out by jca@, I missed aflen (or rather, saw it and
for some unknown reason thought it didn't matter, I blame my tooth ;)
Index: route.c
===
RCS file: /cvs/src/sbin/route/route.c,v
retrieving revision 1.161
diff -u -p -r1.161 route.c
--- route.c 21 Mar 2013 04:43:17 - 1.161
+++ route.c 22 May 2013 20:15:53 -
@@ -803,8 +803,13 @@ getaddr(int which, char *s, struct hoste
int afamily, bits;
if (af == 0) {
- af = AF_INET;
- aflen = sizeof(struct sockaddr_in);
+ if (strchr(s, ':') != NULL) {
+ af = AF_INET6;
+ aflen = sizeof(struct sockaddr_in6);
+ } else {
+ af = AF_INET;
+ aflen = sizeof(struct sockaddr_in);
+ }
}
afamily = af; /* local copy of af so we can change it */
ok
maybe a note in the manpage?
Possible manpage wording..
Index: route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.70
diff -u -p -r1.70 route.8
--- route.8 13 Jul 2012 10:15:53 - 1.70
+++ route.8 23 May 2013 13:49:50 -
@@ -310,6 +310,9 @@ Actual
data, in hexadecimal format
.El
.Pp
+In the absence of modifiers, an address is assumed to be IPv4,
+unless containing a : character when it is treated as IPv6.
+.Pp
The optional modifier
.Fl link
specifies that all subsequent addresses are specified as link-level addresses,
Re: [PATCH] add filter by host functionality to syslogd
On Thu, May 23, 2013 at 12:57, Gregory Edigarov wrote: On 05/22/2013 06:39 PM, Ted Unangst wrote: On Wed, May 22, 2013 at 12:06, Gregory Edigarov wrote: works for me, with only one limitation: now only for resolvable hosts, i.e one cannot have +192.168.2.1 * /some/file Looking at the diff, I think it's not resolvable hosts, but whatever hostname the sending machine decides to tell you? no, it is really a resolvable hosts. works correctly with name in /etc/hosts. I don't understand how that can be, since your patch isn't doing any DNS resolution. if (f-f_program) if (strcmp(prog, f-f_program) != 0) continue; + if (f-f_host) + if (strcmp(from,f-f_host) != 0) + continue; That's just a string compare. The remote host can send any string it wants.
Re: [PATCH] add filter by host functionality to syslogd
On 05/23/2013 07:20 PM, Ted Unangst wrote: On Thu, May 23, 2013 at 12:57, Gregory Edigarov wrote: On 05/22/2013 06:39 PM, Ted Unangst wrote: On Wed, May 22, 2013 at 12:06, Gregory Edigarov wrote: works for me, with only one limitation: now only for resolvable hosts, i.e one cannot have +192.168.2.1 * /some/file Looking at the diff, I think it's not resolvable hosts, but whatever hostname the sending machine decides to tell you? no, it is really a resolvable hosts. works correctly with name in /etc/hosts. I don't understand how that can be, since your patch isn't doing any DNS resolution. if (f-f_program) if (strcmp(prog, f-f_program) != 0) continue; + if (f-f_host) + if (strcmp(from,f-f_host) != 0) + continue; That's just a string compare. The remote host can send any string it wants. yes, it doesn't do any host resolution itself and there is no need in this, because syslogd already does this. (the resolution happens in main cycle, namely cvthname() function call). the changes just compare the hostname given in configs and the hostname reported by cvthname, then act accordinly. -- With best regards, Gregory Edigarov
Re: [PATCH] add filter by host functionality to syslogd
On Thu, May 23, 2013 at 19:55, Gregory Edigarov wrote: That's just a string compare. The remote host can send any string it wants. yes, it doesn't do any host resolution itself and there is no need in this, because syslogd already does this. (the resolution happens in main cycle, namely cvthname() function call). the changes just compare the hostname given in configs and the hostname reported by cvthname, then act accordinly. Oh, my mistake. I thought host, like program, came via the network. Thanks for explaining that. I should have looked at the whole program again.
mandoc strlcat
I was looking at mandoc and noticed it has too many strlcats (a common
affliction affecting quite a few programs.) It's faster and simpler to
use snprintf.
The code in roff.c was doing something twisty with the length argument
to strlcpy. Doing fancy length tricks kind of defeats the purpose of
having a simple to use function like strlcpy. I believe the intention
was to only copy part of the string, so I have retained that feature.
Index: html.c
===
RCS file: /cvs/src/usr.bin/mandoc/html.c,v
retrieving revision 1.30
diff -u -p -r1.30 html.c
--- html.c 28 May 2012 13:00:51 - 1.30
+++ html.c 23 May 2013 20:52:43 -
@@ -618,10 +618,7 @@ void
bufcat_style(struct html *h, const char *key, const char *val)
{
- bufcat(h, key);
- bufcat(h, :);
- bufcat(h, val);
- bufcat(h, ;);
+ bufcat_fmt(h, %s:%s;, key, val);
}
void
Index: man_html.c
===
RCS file: /cvs/src/usr.bin/mandoc/man_html.c,v
retrieving revision 1.48
diff -u -p -r1.48 man_html.c
--- man_html.c 17 Nov 2012 00:25:20 - 1.48
+++ man_html.c 23 May 2013 20:50:52 -
@@ -301,9 +301,10 @@ man_root_pre(MAN_ARGS)
struct tag *t, *tt;
char b[BUFSIZ], title[BUFSIZ];
- b[0] = 0;
if (man-vol)
- (void)strlcat(b, man-vol, BUFSIZ);
+ strlcpy(b, man-vol, sizeof(b));
+ else
+ b[0] = '\0';
assert(man-title);
assert(man-msec);
Index: mandocdb.c
===
RCS file: /cvs/src/usr.bin/mandoc/mandocdb.c,v
retrieving revision 1.42
diff -u -p -r1.42 mandocdb.c
--- mandocdb.c 24 May 2012 23:33:23 - 1.42
+++ mandocdb.c 23 May 2013 20:23:31 -
@@ -286,7 +286,6 @@ mandocdb(int argc, char *argv[])
int ch, i, flags;
DB *hash; /* temporary keyword hashtable */
BTREEINFOinfo; /* btree configuration */
- size_t sz1, sz2;
struct buf buf, /* keyword buffer */
dbuf; /* description buffer */
struct of *of; /* list of files for processing */
@@ -397,19 +396,12 @@ mandocdb(int argc, char *argv[])
perror(dir);
exit((int)MANDOCLEVEL_BADARG);
}
- if (strlcat(pbuf, /, PATH_MAX) = PATH_MAX) {
- fprintf(stderr, %s: path too long\n, pbuf);
- exit((int)MANDOCLEVEL_BADARG);
- }
-
- strlcat(mdb.dbn, pbuf, MAXPATHLEN);
- sz1 = strlcat(mdb.dbn, MANDOC_DB, MAXPATHLEN);
-
- strlcat(mdb.idxn, pbuf, MAXPATHLEN);
- sz2 = strlcat(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
- if (sz1 = MAXPATHLEN || sz2 = MAXPATHLEN) {
- fprintf(stderr, %s: path too long\n, mdb.idxn);
+ if ((size_t)snprintf(mdb.dbn, sizeof(mdb.dbn), %s/%s, pbuf,
+ MANDOC_DB) = sizeof(mdb.dbn) ||
+ (size_t)snprintf(mdb.idxn, sizeof(mdb.idxn), %s/%s, pbuf,
+ MANDOC_IDX) = sizeof(mdb.dbn)) {
+ fprintf(stderr, %s: path too long\n, mdb.dbn);
exit((int)MANDOCLEVEL_BADARG);
}
@@ -486,8 +478,8 @@ mandocdb(int argc, char *argv[])
flags = O_CREAT | O_EXCL | O_RDWR;
while (NULL == mdb.db) {
- strlcpy(mdb.dbn, MANDOC_DB, MAXPATHLEN);
- strlcat(mdb.dbn, .XX, MAXPATHLEN);
+ snprintf(mdb.dbn, sizeof(mdb.dbn), %s.XX,
+ MANDOC_DB);
if (NULL == mktemp(mdb.dbn)) {
perror(mdb.dbn);
exit((int)MANDOCLEVEL_SYSERR);
@@ -500,8 +492,8 @@ mandocdb(int argc, char *argv[])
}
}
while (NULL == mdb.idx) {
- strlcpy(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
- strlcat(mdb.idxn, .XX, MAXPATHLEN);
+ snprintf(mdb.idxn, sizeof(mdb.idxn), %s.XX,
+ MANDOC_IDX);
if (NULL == mktemp(mdb.idxn)) {
perror(mdb.idxn);
unlink(mdb.dbn);
@@ -1809,12 +1801,8 @@ ofile_dirbuild(const char *dir, const ch
continue;
}
- buf[0] = '\0';
- strlcat(buf, dir, MAXPATHLEN);
- strlcat(buf, /, MAXPATHLEN);
- sz = strlcat(buf, fn, MAXPATHLEN);
-
- if (MAXPATHLEN = sz) {
+ if ((size_t)snprintf(buf,
simplify strlcat in tcpdump
just pfctl_osfp.c. harder to replace strlcat when it's in a loop, but
some of the straight line calls can be done as snprintf followed by
one strlcat. worth it?
Index: pfctl_osfp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/pfctl_osfp.c,v
retrieving revision 1.7
diff -u -p -r1.7 pfctl_osfp.c
--- pfctl_osfp.c18 Oct 2010 15:55:28 - 1.7
+++ pfctl_osfp.c23 May 2013 21:28:31 -
@@ -1001,33 +1001,28 @@ const char *
print_ioctl(struct pf_osfp_ioctl *fp)
{
static char buf[1024];
- char tmp[32];
+ char tmp[1024];
int i, opt;
+ char *c;
- *buf = '\0';
+ c = ;
+ tmp[0] = '\0';
if (fp-fp_flags PF_OSFP_WSIZE_DC)
- strlcat(buf, *, sizeof(buf));
+ c = *;
else if (fp-fp_flags PF_OSFP_WSIZE_MSS)
- strlcat(buf, S, sizeof(buf));
+ c = S;
else if (fp-fp_flags PF_OSFP_WSIZE_MTU)
- strlcat(buf, T, sizeof(buf));
+ c = T;
else {
if (fp-fp_flags PF_OSFP_WSIZE_MOD)
- strlcat(buf, %, sizeof(buf));
+ c = %;
snprintf(tmp, sizeof(tmp), %d, fp-fp_wsize);
- strlcat(buf, tmp, sizeof(buf));
}
- strlcat(buf, :, sizeof(buf));
+ snprintf(buf, sizeof(buf), %s%s, c, tmp);
- snprintf(tmp, sizeof(tmp), %d, fp-fp_ttl);
+ snprintf(tmp, sizeof(tmp), :%d:%d:, fp-fp_ttl,
+ (fp-fp_flags PF_OSFP_DF) ? 1 : 0);
strlcat(buf, tmp, sizeof(buf));
- strlcat(buf, :, sizeof(buf));
-
- if (fp-fp_flags PF_OSFP_DF)
- strlcat(buf, 1, sizeof(buf));
- else
- strlcat(buf, 0, sizeof(buf));
- strlcat(buf, :, sizeof(buf));
if (fp-fp_flags PF_OSFP_PSIZE_DC)
strlcat(buf, *, sizeof(buf));
@@ -1083,16 +1078,10 @@ print_ioctl(struct pf_osfp_ioctl *fp)
if (i != 0)
strlcat(buf, ,, sizeof(buf));
}
- strlcat(buf, :, sizeof(buf));
-
- strlcat(buf, fp-fp_os.fp_class_nm, sizeof(buf));
- strlcat(buf, :, sizeof(buf));
- strlcat(buf, fp-fp_os.fp_version_nm, sizeof(buf));
- strlcat(buf, :, sizeof(buf));
- strlcat(buf, fp-fp_os.fp_subtype_nm, sizeof(buf));
- strlcat(buf, :, sizeof(buf));
- snprintf(tmp, sizeof(tmp), TcpOpts %d 0x%llx, fp-fp_optcnt,
+ snprintf(tmp, sizeof(tmp), :%s:%s:%s:TcpOpts %d 0x%llx,
+ fp-fp_os.fp_class_nm, fp-fp_os.fp_version_nm,
+ fp-fp_os.fp_subtype_nm, fp-fp_optcnt,
(long long int)fp-fp_tcpopts);
strlcat(buf, tmp, sizeof(buf));
Re: mandoc strlcat
On Thu, May 23, 2013 at 05:05:45PM -0400, Ted Unangst wrote: I was looking at mandoc and noticed it has too many strlcats (a common affliction affecting quite a few programs.) It's faster and simpler to use snprintf. In glibc snprintf has a memory allocation failure mode. I'm curious: is OpenBSD committed to avoiding extensions (locale features, etc) which might trigger allocation failure?
Re: mandoc strlcat
On Thu, May 23, 2013 at 05:05:45PM -0400, Ted Unangst wrote: I was looking at mandoc and noticed it has too many strlcats (a common affliction affecting quite a few programs.) It's faster and simpler to use snprintf. In glibc snprintf has a memory allocation failure mode. In OpenBSD, snprintf is designed to be thread and signal-handler safe, as long as you don't use certain dangerous features. I'm afraid I can't find documentation which defines which are dangerous or not, but remember auditing our tree to improve the situation. I'm curious: is OpenBSD committed to avoiding extensions (locale features, etc) which might trigger allocation failure? I don't know if we are commited to such a restriction. We could add such things, but then put them in the dangerous catagory, to not be used in unsafe situations... Hmm, where are our docs for that...
Re: mandoc strlcat
On Thu, May 23, 2013 at 21:38, Theo de Raadt wrote: In glibc snprintf has a memory allocation failure mode. I'm curious: is OpenBSD committed to avoiding extensions (locale features, etc) which might trigger allocation failure? Yes. I mean, what in the world is snprintf doing allocating some locale crap to implement a behavior that strlcat clearly doesn't need to allocate memory for? I don't know if we are commited to such a restriction. We could add such things, but then put them in the dangerous catagory, to not be used in unsafe situations... Hmm, where are our docs for that... It's in man signal. The only thing you can't use is floating point, because dtoa is crazy, but I think it'd even be possible to pass the buffer in from vfprintf and make that signal safe too. Just nobody cares.
Re: mandoc strlcat
It's in man signal. The only thing you can't use is floating point, because dtoa is crazy, The *5 table, yes. I tried to improve the situation there. I nearly lost my mind.
