iked + isakmpd on the same machine

[Philipp]

It happened! A remote peer *requires* IKEv2 - and I've to do that on a 
machine running isakmpd with somewhat 25+ IKEv1 peers.


First hurdle: I cannot bind iked to a certain (carp) IP-address. Mad 
workaround: start isakmpd (with Listen-on) first.
Second hurdle: iked loads its SAs and eventually does this by 
creating a new empty SADB, effectivly killing all

the SAs isakmpd loaded into the kernel before?

Is there a diff sleeping out there for tackling the first hurdle?

For the second one, I've to refrain from testing this in live further 
more. First to reconstruct my Frankenstein-Lab.


Cheers for any thoughts beside mad, bro? :-)

Re: iked + isakmpd on the same machine

[Philipp]

Am 22.04.2014 17:28 schrieb Mike Belopuhov:

more like it's not supported and is not supposed to work.

not supposed as in 'not wanted'?


it's like running nginx and apache at the same time but

Quite frankly: I'm doing that in some locations ;-)


worse since there are kernel tentacles involved as well
(as you might have figured out already) that will likely

That's somehow the main problem, the two daemons are not
trying to share the pfkey2 ioctls outcome. So, I can wait til iked 
supports ikev1, too.
Using a different machine will be quite painful at the moment. 
Rock+hard place.



prevent you from doing that on the same box but different
ip addresses.

Nevertheless I'd say that a Listen-on style directive for iked
would a useful thing[tm], e.g. to default the srcid to that.

Cheers.

Re: use mallocarray in kern

[Philipp]

Sorry to break the threading, but I already expunged the original 
message..


Re: http://marc.info/?l=openbsd-techm=140529530814733w=2

The second and third hunk should use mallocarray() instead of malloc() 
in my eyes.

sizeof(Elf_Phdr) as type just doesnt make sense to me.

Hope not everyone is on the plane right now.

--double-p

-DDEBUG misses DUMP_REGS on amd64 libsa

[Philipp]

Hi,

from: sys/arch/amd64/stand/libsa/cmd_i386.c:
#ifdef DEBUG
int
Xregs(void)
{
DUMP_REGS;
return 0;
}
#endif

which is undeclared.

i386 has one in sys/arch/i386/stand/libsa/debug_md.h

--pb

Re: Request for Funding our Electricity

[Philipp]

Am 17.01.2014 22:14 schrieb Kevin Lyda:

That's a bug to be filed against an emulator. And it's easier to do
that *now* when the older hardware is around to test for bug
compatibility.


And how do you do that when the hardware has gone?


And I must admit the resistance to this is weird.


This all ends in places like NetBSD where cross-compiling is
good enough. Just to find out that a native build didnt work
any longer.

Stop dreaming in emulators.

Re: Routing issues

[Philipp]

Am 17.02.2014 09:22 schrieb Alex Mathiasen:

Thank you! This solved my problem.


Cheers.. found the hard way the other day.

There should really be some dmesg when state-tables overflow.
This silent dropping is wasting time in debugging such situations.

Sorry for talk instead of diff :-}

Re: Routing issues

[Philipp]

Am 17.02.2014 12:22 schrieb Stuart Henderson:


Writing messages that show up in dmesg is not cheap, particularly
on systems with serial console.


Well, ok. How about pflog?

Re: Routing issues

[Philipp]

Am 17.02.2014 13:11 schrieb Henning Brauer:

how do you emit such a maessage in pcap? as payload with a dummy
packet header? (N!!)


pf is taking action without telling anyone - and that's not nice.

There *are* other log() entries in pf.c already so I wonder how the 
initial

comment about 'slow via serial console' would qualify.

some blocked because of resource exhaustion reason for pflog_packet?

Just sayin...

Re: autoinstall(8) tweaks

[Philipp]

Am 15.04.2015 01:20 schrieb Ryan McBride:

On other systems where I don't know how the data will grow, I typically
configure them with something close to the auto layout, but a smaller
/home, and leave the remaining disk empty. When I get a feel for what
the data usage is in /var/daemon or /home or /usr/local, I can expand
/home or create a new partition and migrate the data.


Wouldnt that be just the point? Keep the auto-layout in big mode as it
is but do not assign all remains to /home but only x/% GB and leave the
remains of the disk unallocated.
+ default secure splitted mountpoints untouched
+ no need to fuzz with the installer
+ less burden on post install by shrinking/redoing /home to gather
space for /var/foobar or whatever
+ speeds the install-time on large disks ;-)
- new heuristics for xGB /home needed

2ct or less.

Re: autoinstall(8) tweaks

[Philipp]

Am 07.04.2015 16:55 schrieb Kirill Bychkov:

disklabel = D\na b\n\n4g\n\na a\n\n\n\n/\np\nq\n


Oh, please yes. I know that this will be PITA around (non)escaping and 
all,

but the default labelling just isnt cutting it.

+		_mode=$(sed -E '/^ *filename 
(.*\/)?auto_(install|upgrade);$/!d;s//\2/;q' $_lf)

+   _path=$(sed -E '/^ *filename (.*\/)[^/]+;$/!d;s//\1/;q' $_lf)


hostname based auto_{install,upgrade}.conf would be just too handy 
(tweaking into that via

madness symlinking already).

Sorry for hijacking that answer, deleted the previous mail just to 
early.


--pb

Re: autoinstall(8): using multiple set sources?

[Philipp]

Am 08.08.2015 01:26 schrieb Alexander Hall:

Try adding
   Set name(s) = done
Here, like you would manually do (albeit likely implicit by just
pressing enter).


Bit counterintuitive at first, but works! Thanks a bunch.

autoinstall(8): using multiple set sources?

[Philipp]

While heavy playing with autoinstall(8), I came across that I cannot 
make it happen to
install the usual sets from CD/ISO and additional ones like site58.tgz 
from a webserver.


install.conf snips:
root disk = wd0
Use (W)hole disk = W
Location of sets = cd
Set name(s) = all
Location of sets = http
HTTP Server = 192.168.2.101
Server directory = /
Set = all
site58.tgz = yes

The problem is the way the answers are popped; if I ctrl-c the installer 
after the first
set selection, both 'Set' and 'Set name(s)' are already removed from 
/ai.conf.


The site58.tgz will be shown as available, but wont be selected.

Actually one will see that 'all' is being used twice on the first 
selection - and that comes

from: select_sets()
for resp in $resp; do

Cannot provide a diff to deal with it.. only idea I had so far is 
including the install-method

into the 'ask' for sets.

smtpd workarounds for KAME sin6_scope_id

[Philipp]

Hello

As said in the other mail I'm currently working on building OpenSMTPD
on other platforms. A problem I found is the workaround for
sin6_scope_id. The problem with the workaround is that FreeBSD don't
expose IN6_IS_ADDR_MC_INTFACELOCAL(). After a bit digging in the code
I found this workaround isn't needed for FreeBSD. I don't know what
about other OS with the KAME stack. Currently I see two ways to fix
this:

1.
Implement sin6_scope_id correct in OpenBSD and remove the workaround.

2.
Add an extra check in the portable build process if this workaround is
needed.

Whats your opinion for this problem?

Philipp

[diff] usr.sbin/smtpd add missing includes

[Philipp]

Hello

I'm currently working on getting OpenSMTPD-portable build. During this
I found some missing includes.

diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y
index 7de52a1c568..b1307c4daa6 100644
--- a/usr.sbin/smtpd/parse.y
+++ b/usr.sbin/smtpd/parse.y
@@ -28,6 +28,8 @@
 #include 
 #include 
 
+#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/usr.sbin/smtpd/smtpctl.c b/usr.sbin/smtpd/smtpctl.c
index 3d5926efdad..9e88f150ae5 100644
--- a/usr.sbin/smtpd/smtpctl.c
+++ b/usr.sbin/smtpd/smtpctl.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 

Re: [diff] usr.sbin/smtpd add missing includes

[Philipp]

[2021-10-18 11:09] Jonathan Gray 
> On Sun, Oct 17, 2021 at 04:23:50PM +0200, Philipp wrote:
> > Hello
> > 
> > I'm currently working on getting OpenSMTPD-portable build. During this
> > I found some missing includes.
>
> It would help if you could describe the platform you are building on and
> show the compile errors.

Oh sorry, I currently work on Debian and FreeBSD.

Error on Debian 11.1 with clang-11:
==
clang-11 -DHAVE_CONFIG_H -I. -I../..  -I../../usr.sbin/smtpd 
-I../../openbsd-compat -I../../openbsd-compat/err_h 
-I../../openbsd-compat/libasr -I. -I/usr/include  
-DSMTPD_CONFDIR=\"/usr/local/etc\" -DPATH_CHROOT=\"/var/empty\" 
-DPATH_SMTPCTL=\"/usr/local/sbin/smtpctl\" 
-DPATH_MAILLOCAL=\"/usr/local/libexec/opensmtpd/mail.local\" 
-DPATH_MAKEMAP=\"/usr/local/sbin/makemap\" 
-DPATH_LIBEXEC=\"/usr/local/libexec/opensmtpd\" -DHAVE_CONFIG_H -DNO_IO 
-DCONFIG_MINIMUM -DPATH_GZCAT=\"/bin/zcat\" 
-DPATH_ENCRYPT=\"/usr/local/libexec/opensmtpd/encrypt\" -g -O2  -fPIC -DPIC 
-Qunused-arguments -Wunknown-warning-option -Wall -Wpointer-arith 
-Wuninitialized -Wsign-compare -Wformat-security -Wsizeof-pointer-memaccess 
-Wno-pointer-sign -Wno-unused-result -fno-strict-aliasing -fno-builtin-memset 
-D_BSD_SOURCE -D_DEFAULT_SOURCE  -D_GNU_SOURCE -c -o 
../../usr.sbin/smtpd/smtpctl-smtpctl.o `test -f 
'../../usr.sbin/smtpd/smtpctl.c' || echo './'`../../usr.sbin/smtpd/smtpctl.c
../../usr.sbin/smtpd/smtpctl.c:519:7: warning: implicit declaration of function 
'setgroups' is invalid in C99 [-Wimplicit-function-declaration]
if ((setgroups(1, >pw_gid) ||
 ^
../../usr.sbin/smtpd/smtpctl.c:1067:12: warning: implicit declaration of 
function 'getgrnam' is invalid in C99 [-Wimplicit-function-declaration]
if ((gr = getgrnam(SMTPD_QUEUE_GROUP)) == NULL)
  ^
../../usr.sbin/smtpd/smtpctl.c:1067:10: warning: incompatible integer to 
pointer conversion assigning to 'struct group *' from 'int' [-Wint-conversion]
if ((gr = getgrnam(SMTPD_QUEUE_GROUP)) == NULL)
^ ~~~
../../usr.sbin/smtpd/smtpctl.c:1069:13: error: incomplete definition of type 
'struct group'
else if (gr->gr_gid != getegid())
 ~~^
../../usr.sbin/smtpd/smtpctl.c:1058:9: note: forward declaration of 'struct 
group'
struct group*gr;
   ^
3 warnings and 1 error generated.
==

The include grp.h fixes the build, with some other portable specific
changes:
https://github.com/OpenSMTPD/OpenSMTPD/pull/1153
https://github.com/OpenSMTPD/OpenSMTPD/pull/1154

First error on FreeBSD 13.0-RELEASE-p4 with clang12:
==
clang12 -DHAVE_CONFIG_H -I. -I../..  -I../../usr.sbin/smtpd 
-I../../openbsd-compat  -I../../openbsd-compat/err_h 
-I../../openbsd-compat/paths_h   -I../../openbsd-compat/libtls -I. 
-DSMTPD_CONFDIR=\"/usr/local/etc\"   
-DPATH_CHROOT=\"/var/empty\"
-DPATH_SMTPCTL=\"/usr/local/sbin/smtpctl\"  
-DPATH_MAILLOCAL=\"/usr/local/libexec/opensmtpd/mail.local\"
-DPATH_MAKEMAP=\"/usr/local/sbin/makemap\"  
-DPATH_LIBEXEC=\"/usr/local/libexec/opensmtpd\" -DHAVE_CONFIG_H 
-I/usr/local/include -DIO_TLS -DCA_FILE=\"/etc/ssl/cert.pem\" 
-I/usr/local/include  -fPIC -DPIC -Qunused-arguments -Wunknown-warning-option 
-Wall -Wpointer-arith -Wuninitialized -Wsign-compare -Wformat-security 
-Wsizeof-pointer-memaccess -Wno-pointer-sign -Wno-unused-result 
-fno-strict-aliasing -fno-builtin-memset -c -o 
../../usr.sbin/smtpd/smtpd-parse.o `test -f '../../usr.sbin/smtpd/parse.c' || 
echo './'`../../usr.sbin/smtpd/parse.c
../../usr.sbin/smtpd/parse.y:2639:2: warning: implicitly declaring library 
function 'free' with type 'void (void *)' [-Wimplicit-function-declaration]
free(msg);
^
../../usr.sbin/smtpd/parse.y:2639:2: note: include the header  or 
explicitly provide a declaration for 'free'
../../usr.sbin/smtpd/parse.y:2646:10: warning: implicitly declaring library 
function 'strcmp' with type 'int (const char *, const char *)' 
[-Wimplicit-function-declaration]
return (strcmp(k, ((const struct keywords *)e)->k_name));
^
../../usr.sbin/smtpd/parse.y:2646:10: note: include the header  or 
explicitly provide a declaration for 'strcmp'
../../usr.sbin/smtpd/parse.y:2758:6: warning: implicit declaration of function 
'bsearch' is invalid in C99 [-Wimplicit-function-declaration]
p = bsearch(s, keywords, sizeof(keywords)/sizeof(keywords[0]),
^
../../usr.sbin/smtpd/parse.y:2758:4: warning: incompatible integer to pointer 
conversion assigning to 'const struct keywords *' from 'int'

smtpd: implement nullmx RFC 7505

[Philipp]

Hi

Setting Null MX is a way for domainowners to indicate that the domain
does not accept mail. Currently a Null MX causes a tempfail and the
mail will be queued and tried to resubmitted till a timeout. With the
attached patch a Null MX causes a permfail. This way the sender will
directly get a bounce with the message "Domain does not accept mail".

Because some domains set the MX record to "localhost." to get a similar
efect the secound patch ignores "localhost." MX entries and handles a MX
containing only "localhost." records like a Null MX.

Philipp
From 2970019967e967d98ec30f86549f38788bff6081 Mon Sep 17 00:00:00 2001
From: Philipp 
Date: Sun, 2 Jul 2023 01:27:35 +0200
Subject: [PATCH 1/2] implement rfc 7505 (Null MX)

Null MX is to indicate that a domain does not accept mail.
---
 usr.sbin/smtpd/dns.c   | 28 +++-
 usr.sbin/smtpd/mta.c   |  4 
 usr.sbin/smtpd/smtpd.h |  2 ++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/usr.sbin/smtpd/dns.c b/usr.sbin/smtpd/dns.c
index 4cf5d23d1d1..d510fa2b5aa 100644
--- a/usr.sbin/smtpd/dns.c
+++ b/usr.sbin/smtpd/dns.c
@@ -44,6 +44,7 @@ struct dns_session {
 	size_t			 mxfound;
 	int			 error;
 	int			 refcount;
+	int			 nullmx;
 };
 
 static void dns_lookup_host(struct dns_session *, const char *, int);
@@ -195,7 +196,7 @@ dns_dispatch_host(struct asr_result *ar, void *arg)
 
 	s = lookup->session;
 
-	for (ai = ar->ar_addrinfo; ai; ai = ai->ai_next) {
+	for (ai = ar->ar_addrinfo; s->nullmx == 0 && ai; ai = ai->ai_next) {
 		s->mxfound++;
 		m_create(s->p, IMSG_MTA_DNS_HOST, 0, 0, -1);
 		m_add_id(s->p, s->reqid);
@@ -215,10 +216,12 @@ dns_dispatch_host(struct asr_result *ar, void *arg)
 	if (--s->refcount)
 		return;
 
-	m_create(s->p, IMSG_MTA_DNS_HOST_END, 0, 0, -1);
-	m_add_id(s->p, s->reqid);
-	m_add_int(s->p, s->mxfound ? DNS_OK : DNS_ENOTFOUND);
-	m_close(s->p);
+	if (s->nullmx == 0) {
+		m_create(s->p, IMSG_MTA_DNS_HOST_END, 0, 0, -1);
+		m_add_id(s->p, s->reqid);
+		m_add_int(s->p, s->mxfound ? DNS_OK : DNS_ENOTFOUND);
+		m_close(s->p);
+	}
 	free(s);
 }
 
@@ -259,6 +262,21 @@ dns_dispatch_mx(struct asr_result *ar, void *arg)
 		unpack_rr(, );
 		if (rr.rr_type != T_MX)
 			continue;
+
+		/* Null MX */
+		if (rr.rr.mx.preference == 0 && rr.rr.mx.exchange[0] == 0) {
+			m_create(s->p,  IMSG_MTA_DNS_HOST_END, 0, 0, -1);
+			m_add_id(s->p, s->reqid);
+			m_add_int(s->p, DNS_NULLMX);
+			m_close(s->p);
+			if (found == 0)
+free(s);
+			else
+s->nullmx = 1;
+			found++;
+			break;
+		}
+
 		print_dname(rr.rr.mx.exchange, buf, sizeof(buf));
 		buf[strlen(buf) - 1] = '\0';
 		dns_lookup_host(s, buf, rr.rr.mx.preference);
diff --git a/usr.sbin/smtpd/mta.c b/usr.sbin/smtpd/mta.c
index c0d0fc02931..25e158b68a8 100644
--- a/usr.sbin/smtpd/mta.c
+++ b/usr.sbin/smtpd/mta.c
@@ -1088,6 +1088,10 @@ mta_on_mx(void *tag, void *arg, void *data)
 		else
 			relay->failstr = "No MX found for domain";
 		break;
+	case DNS_NULLMX:
+		relay->fail = IMSG_MTA_DELIVERY_PERMFAIL;
+		relay->failstr = "Domain does not accept mail";
+		break;
 	default:
 		fatalx("bad DNS lookup error code");
 		break;
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 6781286928d..9f4732d1c27 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1026,6 +1026,8 @@ enum dns_error {
 	DNS_EINVAL,
 	DNS_ENONAME,
 	DNS_ENOTFOUND,
+	/* rfc 7505 */
+	DNS_NULLMX,
 };
 
 enum lka_resp_status {
-- 
2.39.2

From ace283bbedc1e7594c850e0ae6f3b6d9d456ba77 Mon Sep 17 00:00:00 2001
From: Philipp 
Date: Sun, 2 Jul 2023 01:50:20 +0200
Subject: [PATCH 2/2] filter localhost MX

A localhost MX only cause a loop. Also handle only a localhost
like a Null MX.
---
 usr.sbin/smtpd/dns.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/usr.sbin/smtpd/dns.c b/usr.sbin/smtpd/dns.c
index d510fa2b5aa..0df221f3755 100644
--- a/usr.sbin/smtpd/dns.c
+++ b/usr.sbin/smtpd/dns.c
@@ -235,6 +235,7 @@ dns_dispatch_mx(struct asr_result *ar, void *arg)
 	struct dns_rr		 rr;
 	char			 buf[512];
 	size_t			 found;
+	int			 localhost;
 
 	if (ar->ar_h_errno && ar->ar_h_errno != NO_DATA &&
 	ar->ar_h_errno != NOTIMP) {
@@ -258,6 +259,7 @@ dns_dispatch_mx(struct asr_result *ar, void *arg)
 	unpack_query(, );
 
 	found = 0;
+	localhost = 0;
 	for (; h.ancount; h.ancount--) {
 		unpack_rr(, );
 		if (rr.rr_type != T_MX)
@@ -279,11 +281,27 @@ dns_dispatch_mx(struct asr_result *ar, void *arg)
 
 		print_dname(rr.rr.mx.exchange, buf, sizeof(buf));
 		buf[strlen(buf) - 1] = '\0';
+		if (strcasecmp("localhost", buf) == 0) {
+			log_warnx("ignore localhost MX-entry for domain <%s>",
+			s->name);
+			localhost++;
+			continue;
+		}
 		dns_lookup_host(s, buf, rr.rr.mx.preference);
 		found++;
 	}
 	f

Re: smtpd: implement nullmx RFC 7505

[Philipp]

[2023-10-17 17:32] Omar Polo 
> sorry for the terrifc delay.
>
> On 2023/10/01 14:59:15 +0200, Philipp  wrote:
> > Hi
> > 
> > Setting Null MX is a way for domainowners to indicate that the domain
> > does not accept mail. Currently a Null MX causes a tempfail and the
> > mail will be queued and tried to resubmitted till a timeout. With the
> > attached patch a Null MX causes a permfail. This way the sender will
> > directly get a bounce with the message "Domain does not accept mail".
>
> I'm not sure how much widespread the usage is, but it doesn't seem a
> bad feature.  It's simple to implement and it provides some (very
> small) value.

There is a blogpost[0] which says about 1% of the domains use Null MX and
about 0.7% set the MX to "localhost." (probably for the same propose).

> In general I'm OK with the idea, but would need some OKs from other
> developers too.
>
> There is one part of the RFC7505 that I'd like to quote and discuss
> with you however.  The last paragraph of the section 3 says:
>
> : A domain that advertises a null MX MUST NOT advertise any other MX
> : RR.
>
> Your implementation handles the case where there are more than one MX
> by setting the `nullmx' field in the shared struct when we encounter
> one, and then don't process the other MXs when that field is set.
> This is fine.
>
> However, I think we can simplify here by not honouring the Null MX
> when there are other MX records.  We're not violating the RFC.  See
> diff below to see what I mean.  The only difference is in dns.c, the
> rest is the same with yours.

My reasaning for that was: When you set Null MX you explicitly opt out
from reciving mail even when you violate the spec. But if you want
it diffrent I can change it.

But I don't think your proposed patch is a good solution, because the
result depend on the order of the RR in the repsonse. The problem is
when the first entry in the response is a Null MX your patch truncate
the other results and a bounce is generated. But when the Null MX is
later in the response the other entries are used to deliver the mail.

> So far I've only compile-tested the code.  I don't have a spare domain
> to test and don't know of anyone using a Null MX.

To test Null MX you can use example.com. To test the localhost patch a
friend of me has set up mxtest.yannikenss.de.

> > Because some domains set the MX record to "localhost." to get a similar
> > efect the secound patch ignores "localhost." MX entries and handles a MX
> > containing only "localhost." records like a Null MX.
>
> This could be simplified too.  On top of diff below, it would need
> just something among the lines of:

When localhost and Null MX should be handled the same this could be
simplified in one check, yes. But that "localhost." and Null MX are
handled different was on purpose. Because I would say setting a Null MX
is a explicit opt out from reciving mail and setting MX to localhost
is implicit. As said before, I have no problem which changing this.

> I'm still not convinced about this part however.  It feels wrong to me
> to hardcode such a check, it seems too much paranoic.  The follow-up
> would be to check for localhost in dns_dispatch_host too?  ;)

The idea[1] about this part was because setting "localhost." as MX is used
in a similiar way as Null MX[0]. So handle this in a simmilar way as
Null MX make sense. This way the sender will get a better bounce message.

Philipp

[0] https://www.netmeister.org/blog/mx-diversity.html

[1] I still belive OpenSMTPD should only connect to public routable
addresses for relay. So don't connect to something like 127.0.0.1/8,
::1/128, RFC1918 addresses, ULA, ...
But this is independent of this patch.

Re: smtpd: implement nullmx RFC 7505

[Philipp]

[2023-10-18 11:42] Omar Polo 
> On 2023/10/18 08:40:14 +0100, Stuart Henderson  wrote:
> > On 2023/10/17 22:27, Philipp wrote:
> > > [2023-10-17 17:32] Omar Polo 
> > > > [...]
>
> > > But I don't think your proposed patch is a good solution, because the
> > > result depend on the order of the RR in the repsonse. The problem is
> > > when the first entry in the response is a Null MX your patch truncate
> > > the other results and a bounce is generated. But when the Null MX is
> > > later in the response the other entries are used to deliver the mail.
>
> that's true; I fell for a shorter diff.  I've attached below a version
> that doesn't depend on the order.

Your patch looks mostly good. I only have a small nitpick style comment:

> +
> + if (rr.rr.mx.preference == 0 && !strcmp(buf, "")) {

strcmp doesn't return a bool like value. Something like:

if (rr.rr.mx.preference == 0 && strcmp(buf, "") == 0)

is imho better to read.

[patch] usr.sbin/smtpd filter localhost relays

[Philipp]

Hi

On github someone reported an issue[0] regarding localhost MX entries.
Currently smtpd will just use the localhost relay. This leads to a
loop. Here a patch filtering localhost and localhost addresses for MX
requests.

As next step you could implement Null-MX (rfc 7505).

Philipp

[0] https://github.com/OpenSMTPD/OpenSMTPD/issues/1190

diff --git a/usr.sbin/smtpd/dns.c b/usr.sbin/smtpd/dns.c
index f7c6b3df..7389efec 100644
--- a/usr.sbin/smtpd/dns.c
+++ b/usr.sbin/smtpd/dns.c
@@ -53,6 +53,7 @@ struct dns_lookup {
struct dns_session  *session;
char*host;
int  preference;
+   int  filter_localhost;
 };
 
 struct dns_session {
@@ -65,7 +66,7 @@ struct dns_session {
int  refcount;
 };
 
-static void dns_lookup_host(struct dns_session *, const char *, int);
+static void dns_lookup_host(struct dns_session *, const char *, int, int);
 static void dns_dispatch_host(struct asr_result *, void *);
 static void dns_dispatch_mx(struct asr_result *, void *);
 static void dns_dispatch_mx_preference(struct asr_result *, void *);
@@ -139,7 +140,7 @@ dns_imsg(struct mproc *p, struct imsg *imsg)
case IMSG_MTA_DNS_HOST:
m_get_string(, );
m_end();
-   dns_lookup_host(s, host, -1);
+   dns_lookup_host(s, host, -1, 0);
return;
 
case IMSG_MTA_DNS_MX:
@@ -205,6 +206,28 @@ dns_imsg(struct mproc *p, struct imsg *imsg)
}
 }
 
+static int
+is_localhost(struct sockaddr *sa)
+{
+   struct sockaddr_in6 *ipv6;
+   struct sockaddr_in *ipv4;
+   uint32_t addr;
+
+   switch (sa->sa_family) {
+   case AF_INET6:
+   // May check also for v6 mapped v4 addresses
+   ipv6 = (struct sockaddr_in6 *)sa;
+   return IN6_IS_ADDR_LOOPBACK(>sin6_addr);
+   case AF_INET:
+   ipv4 = (struct sockaddr_in *)sa;
+   addr = ntohl(ipv4->sin_addr.s_addr);
+   return ((addr >> 24) & 0xff) == 127;
+   default:
+   log_warnx("warn: unknown family in sockaddr");
+   }
+   return 0;
+}
+
 static void
 dns_dispatch_host(struct asr_result *ar, void *arg)
 {
@@ -215,6 +238,10 @@ dns_dispatch_host(struct asr_result *ar, void *arg)
s = lookup->session;
 
for (ai = ar->ar_addrinfo; ai; ai = ai->ai_next) {
+   if (lookup->filter_localhost && is_localhost(ai->ai_addr)) {
+   log_warnx("warn: ignore localhost address for host %s", 
lookup->host);
+   continue;
+   }
s->mxfound++;
m_create(s->p, IMSG_MTA_DNS_HOST, 0, 0, -1);
m_add_id(s->p, s->reqid);
@@ -280,14 +307,18 @@ dns_dispatch_mx(struct asr_result *ar, void *arg)
continue;
print_dname(rr.rr.mx.exchange, buf, sizeof(buf));
buf[strlen(buf) - 1] = '\0';
-   dns_lookup_host(s, buf, rr.rr.mx.preference);
+   if (strcasecmp("localhost", buf)==0) {
+   log_warnx("ignore localhost MX-entry for domain <%s>", 
lookup->host);
+   continue;
+   }
+   dns_lookup_host(s, buf, rr.rr.mx.preference, 1);
found++;
}
free(ar->ar_data);
 
/* fallback to host if no MX is found. */
if (found == 0)
-   dns_lookup_host(s, s->name, 0);
+   dns_lookup_host(s, s->name, 0, 1);
 }
 
 static void
@@ -340,7 +371,7 @@ dns_dispatch_mx_preference(struct asr_result *ar, void *arg)
 }
 
 static void
-dns_lookup_host(struct dns_session *s, const char *host, int preference)
+dns_lookup_host(struct dns_session *s, const char *host, int preference, int 
filter_localhost)
 {
struct dns_lookup   *lookup;
struct addrinfo  hints;
@@ -350,6 +381,7 @@ dns_lookup_host(struct dns_session *s, const char *host, 
int preference)
 
lookup = xcalloc(1, sizeof *lookup);
lookup->preference = preference;
+   lookup->filter_localhost = filter_localhost;
lookup->host = xstrdup(host);
lookup->session = s;
s->refcount++;

Re: write(2) man page

[Philipp Schafft]

reflum,

On Sun, 2013-02-24 at 09:12 +0530, Sachidananda wrote:
  Also, for the record, it's open that seeks to the end.  You can open
 a
  file with O_APPEND and seek back to the beginning, and write will
 not
  seek to the end again.
 My observation is, if I open(2) the file with O_APPEND it seeks to
 the 
 end. And I lseek back to the beginning and write(2) to it, write does 
 seek back to the end and write the data at the end. 

From POSIX:
 If the O_APPEND flag of the file status flags is set, the file offset
 shall be set to the end of the file prior to each write and no
 intervening file modification operation shall occur between changing
 the file offset and the write operation.
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html)

As of my understanding this is to allow race condition free appending to
files such as log files so no data is lost. because of the fact that
write can return a 'short write' data may be interleaved somehow but no
data is lost.

(Example of such a case: two processes/threads using the same logfile to
log errors. something happens and both write down an error message at
the same time. O_APPEND ensures no data is lost here.)

Hope that helped! :)

-- 
Philipp.
 (Rah of PH2)


signature.asc
Description: This is a digitally signed message part

Re: Make alpha 2038-safe

[Philipp Schafft]

flum,

On Wed, 2016-02-17 at 12:05 -0500, Michael McConville wrote:
> Now that we have 64-bit time. Otherwise, all the people running Alpha in
> 2038 will yell at us.


> Index: sys/arch/alpha/alpha/clock.c
> ===
> RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 clock.c
> --- sys/arch/alpha/alpha/clock.c  23 Mar 2012 15:51:25 -  1.21
> +++ sys/arch/alpha/alpha/clock.c  17 Feb 2016 16:59:22 -
> @@ -212,9 +212,8 @@ inittodr(time_t base)
> [...]
> + if (year < MINYEAR || ct.mon < 1 || ct.mon > 12 || ct.day < 1 ||
>   ct.day > 31 || ct.hour > 23 || ct.min > 59 || ct.sec > 59) {
> [...]

I'm not sure if this applies here as well. In some places sec == 60 is
used for leap seconds.

Thank you for your work.

-- 
Philipp.
 (Rah of PH2)


signature.asc
Description: This is a digitally signed message part

RELAYD_ANCHOR as a relayd.conf option

[Philipp Buehler]

Moin,

while trying to push rdomain setups a bit further, I noticed that relayd 
is using a fixed anchor.
For the pre-rdomain days this was sufficient, but nowadays that might 
look a bit different.


Some dance with 'match pftag', carefully crafted (read:unique) 
rdr-subanchor-names

can make the thing work to some extend.

Reading through pfe_filter.c, I dont see a problem with making this a 
relayd.conf option (just

as authpf has already).
Maybe some not-so-obvious pitfalls with multiple relayd (one per 
rdomain)?


--
pb

Re: multiple routing tables

[Philipp Buehler]

Am 15.05.2016 12:10 schrieb Stefan Sperling:

They key point seems to be that you're trying to route between 
different

rdomains. I believe you must use pf to route traffic coming from this
IP (which is in rdomain 0) to vether1 (which is in rdomain 2)


or look into pair(4), also.

--
pb

undocumented -P/-I in relayd, vmd, httpd, ...

[Philipp Buehler]

Hi there,

while crawling through relayd source, I noticed that there is I:P: in 
getopt.
P is obviously setting the proc-title, but I am unsure what to "get" 
from an

instance-number via -I.

This found way into httpd, snmpd, switchd and vmd also; mainly while 
g2k16.


If someone dares to explain it, I can mop up a diff for the manpages.

ciao
--
pb

relayd(8): more rdomain integration diff

[Philipp Buehler]

Hi folks,

after trying forth and back to overcome some limitations in relayd along 
multiple
"instances" and rdomain/rtable I decided to scrub some rust of my C/yacc 
and

produced the following diffs against -current to relayd and relayctl.

Feats:
- relayd/relayctl: -s sockname; obviously and blatantly taken from 
ospfd/ospfctl
- relayd: -a anchorname; overcome the problem that one stopping relayd 
cleans
 out everything below 'relayd/*'  or not having any seperation 
(includes) in first

 place
- relayd.conf: add 'rtable' as a tableopt; reasoning for "only" there:
 - route -T N exec will already put one into 'on rdomain N'
 - 'listen' already has an 'interface' option to bound to a specific 
rdomain (unless

 I am mistaken here on the kernel/addressing side
 - generally unsure if this has a real use case anyway

Tests done:
- running two relayd on different anchors, rdomains (via route exec) and 
so on,
 notable to me was the point of purging the main/final anchor in 
flush_rulesets.


Caveats:
- didnt write C/yacc in some years, so esp. string-handling could need 
some eyeballs


Patch-white-space-drama; if the patches have mangled WS, there are also 
here:

https://github.com/double-p/smtf/tree/master/patches

Everybody coming, see you in Tokyo!

PS: iked lacks -s, ikectl doesnt... :)
--
pbIndex: relayctl.8
===
RCS file: /cvs/src/usr.sbin/relayctl/relayctl.8,v
retrieving revision 1.32
diff -u -p -r1.32 relayctl.8
--- relayctl.8	28 Nov 2015 01:22:44 -	1.32
+++ relayctl.8	28 Feb 2017 20:09:34 -
@@ -23,6 +23,7 @@
 .Nd control the relay daemon
 .Sh SYNOPSIS
 .Nm
+.Op Fl s Ar socket
 .Ar command
 .Op Ar argument ...
 .Sh DESCRIPTION
@@ -32,6 +33,17 @@ program controls the
 .Xr relayd 8
 daemon.
 .Pp
+The following options are available:
+.Bl -tag -width Ds
+.It Fl s Ar socket
+Use
+.Ar socket
+instead of the default
+.Pa /var/run/relayd.sock
+to communicate with
+.Xr relayd 8 .
+.El
+.Pp
 The following commands are available:
 .Bl -tag -width Ds
 .It Cm host disable Op Ar name | id
@@ -105,6 +117,7 @@ again.
 .Sh FILES
 .Bl -tag -width "/var/run/relayd.sockXX" -compact
 .It Pa /var/run/relayd.sock
+Default
 .Ux Ns -domain
 socket used for communication with
 .Xr relayd 8 .
Index: relayctl.c
===
RCS file: /cvs/src/usr.sbin/relayctl/relayctl.c,v
retrieving revision 1.57
diff -u -p -r1.57 relayctl.c
--- relayctl.c	3 Sep 2016 14:44:21 -	1.57
+++ relayctl.c	28 Feb 2017 20:09:34 -
@@ -88,7 +88,8 @@ usage(void)
 {
 	extern char *__progname;
 
-	fprintf(stderr, "usage: %s command [argument ...]\n", __progname);
+	fprintf(stderr, "usage: %s [-s socket] command [argument ...]\n",
+ 	__progname);
 	exit(1);
 }
 
@@ -101,9 +102,25 @@ main(int argc, char *argv[])
 	int			 ctl_sock;
 	int			 done = 0;
 	int			 n, verbose = 0;
+	int			 ch;
+	char			*sockname;
+
+	sockname = RELAYD_SOCKET;
+	while ((ch = getopt(argc, argv, "s:")) != -1) {
+		switch (ch) {
+		case 's':
+			sockname = optarg;
+			break;
+		default:
+			usage();
+			/* NOTREACHED */
+		}
+	}
+	argc -= optind;
+	argv += optind;
 
 	/* parse options */
-	if ((res = parse(argc - 1, argv + 1)) == NULL)
+	if ((res = parse(argc, argv)) == NULL)
 		exit(1);
 
 	/* connect to relayd control socket */
@@ -112,7 +129,7 @@ main(int argc, char *argv[])
 
 	bzero(, sizeof(sun));
 	sun.sun_family = AF_UNIX;
-	(void)strlcpy(sun.sun_path, RELAYD_SOCKET, sizeof(sun.sun_path));
+	(void)strlcpy(sun.sun_path, sockname, sizeof(sun.sun_path));
  reconnect:
 	if (connect(ctl_sock, (struct sockaddr *), sizeof(sun)) == -1) {
 		/* Keep retrying if running in monitor mode */
@@ -121,7 +138,7 @@ main(int argc, char *argv[])
 			usleep(100);
 			goto reconnect;
 		}
-		err(1, "connect: %s", RELAYD_SOCKET);
+		err(1, "connect: %s", sockname);
 	}
 
 	if (pledge("stdio", NULL) == -1)
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.214
diff -u -p -r1.214 parse.y
--- parse.y	5 Jan 2017 13:53:09 -	1.214
+++ parse.y	1 Mar 2017 15:50:24 -
@@ -805,6 +805,13 @@ tableopts	: CHECK tablecheck
 break;
 			}
 		}
+		| RTABLE NUMBER {
+			if ($2 < 0 || $2 > RT_TABLEID_MAX) {
+yyerror("invalid rtable id %d", $2);
+YYERROR;
+			}
+			table->conf.rtable = $2;
+		}
 		;
 
 /* should be in sync with sbin/pfctl/parse.y's hashkey */
Index: pfe_filter.c
===
RCS file: /cvs/src/usr.sbin/relayd/pfe_filter.c,v
retrieving revision 1.61
diff -u -p -r1.61 pfe_filter.c
--- pfe_filter.c	24 Jan 2017 10:49:14 -	1.61
+++ pfe_filter.c	1 Mar 2017 15:50:24 -
@@ -60,7 +60,7 @@ init_tables(struct relayd *env)
 	i = 0;
 
 	TAILQ_FOREACH(rdr, env->sc_rdrs, entry) {
-		if (strlcpy(tables[i].pfrt_anchor, RELAYD_ANCHOR "/",
+		if (strlcpy(tables[i].pfrt_anchor, env->sc_baseanchor,
 		

Re: usermod.8 patch

[Philipp Buehler]

Am 31.03.2017 15:39 schrieb Jeremie Courreges-Anglas:

I think the current wording is fine; no need for an option to set
_default_ values.


options are good - as long as they're optional --art

--
pb

Re: sysupgrade: select sets to install

[Philipp Buehler]

Am 10.07.2019 20:18 schrieb Theo de Raadt:
Ofcourse there are also custom sets, like site${VERSION}-*.tgz . Which 
is

something to keep in mind.


Yeah, we could delete support for that entirely

Those of you so used to pushing buttons and requiring special features
used by a limited subset of the community really don't know what hole
you are starting to dig


Please do not for the install-installer. I think those who are really 
using site*.tgz
know that automation is obliteration and don't need site (or selected 
sets

back then) "awareness" in the update tools.

While I think sys{upgrade,patch,merge} are useful, who installs such
machines (e.g. laptops) in a "site.tgz" way anyway?

--
pb

Re: man.cgi(8): turn off HTML5 autocomplete for the query input field

[Philipp Buehler]

Am 10.01.2020 15:58 schrieb Tim Baumgard:

I found out that Apple requires nonstandard [1] attributes to fully


The other day nonstandard "gave" us javascript around the globe...

Cheers for putting this one in, was really kinda PITA and I didn't
know about this attribute.

--
pb

Re: Request for Funding our Electricity

[Peter J. Philipp]

On 01/14/14 21:56, Theo de Raadt wrote:

Hi,

 Anyone want to suggest we hold a bake sale?

I just donated a little bit.  Looking for roughly 10 dozen like minded
people.  I'm not suggesting a bake sale but one thing I noticed with the
freebsdfoundation.org's website, that I think works out good, is that
they have a donation meter on how much was put in the hat.  I think
something like this would benefit OpenBSD too.  Just there would need to
be someone able to make such a meter on a website.  Also showing how
much came from private donations vs. corporate donations would be
interesting to see.

Cheers,

-peter

Re: 5.5 and dual-boot

[jean-philipp luiggi]

Le 07/03/2014 12:02 PM, Bob Beck a écrit :

actually more painful than having to boot windows is to always have
something handy to boot the snap from in order to dd the bootblock off
in case you forget to do it before rebooting, or you're fucked.


Hi Bob,

Yeah and hopefully, with a recent post on undeadly, I managed to have an 
USB install media. More easy than going all around the company

hunting for an external USB drive. :)

Regards.

Jean-Philippe


--
Ce message a été vérifié par MailScanner
pour des virus ou des polluriels et rien de
suspect n'a été trouvé.
For all your IT requirements visit: http://www.transtec.co.uk

Re: 5.5 and dual-boot

[jean-philipp luiggi]

Le 07/03/2014 12:13 PM, Theo de Raadt a écrit :

actually more painful than having to boot windows is to always have
something handy to boot the snap from in order to dd the bootblock off
in case you forget to do it before rebooting, or you're fucked.

The new installboot was enabled around a month ago.  The issue is only
being talked about now.

Apparently... this dual-boot issue is only seen now, indicating that
these people don't upgrade very often, or participate in the test
cycles leading up to release.


Hi Theo,

In fact I sent an email on bugs@ on 04/02/2014 (subject was installboot 
and ERR M).
First, it was suspected a problem because of using an old bsd.rd 
instead of the one provided with the snapshot.
This is after some tests in the next weeks that I found the culprit 
(openbsd.pbr).  Moreover as boot(8) continued to be changed, I wanted to 
be sure before posting.


Regards.

Jean-Philippe.

--
Ce message a été vérifié par MailScanner
pour des virus ou des polluriels et rien de
suspect n'a été trouvé.
For all your IT requirements visit: http://www.transtec.co.uk

IP option IP_RECVTTL question

[Peter J. Philipp]

Hi,

I wrote a DNS server and I'm collecting TTL information from the remote 
nameservers that query my daemon.  Everything works well, when I view the
logs I see:

Feb  3 10:43:48 uranus wildcarddnsd[5705]: request on descriptor 14 interface 
em0 from XXX.XXX.XXX.XX (ttl=113, region=255) for centroid.eu. type=A(1) 
class=1, answering centroid.eu.

Where the TTL is logged as 113 in this example.

But occasionally I get this on OpenBSD/i386 and /amd64...

Feb  3 10:45:01 uranus wildcarddnsd[5705]: request on descriptor 14 interface 
em0 from XXX.XX.XX.XX (ttl=27263547, region=255) for goldflipper.net. 
type=A(1) class=1, answering goldflipper.net.

Where the TTL is a bad value.  I think I'm using the code the right way after
the manpage ip(4):

 If the IP_RECVTTL option is enabled on a SOCK_DGRAM or SOCK_RAW socket,
 the recvmsg(2) call will return the TTL of the received datagram.  The
 msg_control field in the msghdr structure points to a buffer that
 contains a cmsghdr structure followed by the TTL value.  The cmsghdr
 fields have the following values:

   cmsg_len = CMSG_LEN(sizeof(struct in_addr))
   cmsg_level = IPPROTO_IP
   cmsg_type = IP_RECVTTL

And if I'm not mistaken the size of struct in_addr is 4.

Since sourceforge is down I can't refer you to the webcvs but they still 
offer the files for download if you want to take a peak at the source code
that'd be great!  http://sourceforge.net/projects/wildcarddns/files/

Here are some snippets from my source code perhaps you can spot what I'm
doing wrong?

...
if (setsockopt(udp[i], IPPROTO_IP, IP_RECVTTL, on, sizeof(on))  0) {

...
#ifdef __FreeBSD__
u_char *ttlptr;
#elif __OpenBSD__
struct in_addr *ttlptr;
#else
int *ttlptr;
#endif

   u_int32_t received_ttl;


...
#if __FreeBSD__

ttlptr = (u_char *) CMSG_DATA(cmsg);
received_ttl = (u_int)*ttlptr;
#elif __OpenBSD__

ttlptr = (struct in_addr *) CMSG_DATA(cmsg);
memcpy(received_ttl, ttlptr, sizeof(u_int32_t));
#else

ttlptr = (int *) CMSG_DATA(cmsg);
received_ttl = (u_int)*ttlptr;
#endif

...
syslog(LOG_INFO, request on descriptor %u interface \%s\ from %s (ttl=%u, 
region=%d) for \%s\ type=%s class=%u, answering \%s\, so, ident[i], 
address, received_ttl, aregion, question-converted_name, 
get_dns_type(ntohs(question-hdr-qtype)), ntohs(question-hdr-qclass), 
replystring);

...

Unfortunately this #define hell is a mess but it's the only way to get this
portable among BSD's and Linux.  I'm hoping someone can tell where I'm going
wrong with this and why it's right sometimes and wrong the occasional time.

Regards,

-peter

Re: IP option IP_RECVTTL question

[Peter J. Philipp]

On Thu, Feb 03, 2011 at 01:51:47PM +0100, Otto Moerbeek wrote:
 cmsg_len = CMSG_LEN(sizeof(struct in_addr))
 cmsg_level = IPPROTO_IP
 cmsg_type = IP_RECVTTL
  
  And if I'm not mistaken the size of struct in_addr is 4.
 
 This looks like a documentation error. Actually, the data length is
 CMSG_LEN(sizeof(u_char)), as can be seen in
 src/netinet/ip_input.c:1744 and the implementation of
 sbcreatecontrol() in src/kern/uipc_socket2.c. Try the FreeBSD code, it
 looks that has a goof chanche of working. 
 
 If this is indeed the case, I'd like to know so I can fix the man page.
 
   -Otto

Hi,

At first glance it seems to be working with the FreeBSD code.  On both i386
and amd64.  Thanks!

So it must be a documentation bug.  Thanks for your help!

-peter

last patch, idea

[Peter J. Philipp]

Hi,

while going through my wtmp with last(1) I noticed there could be a better
way than always gunzip'ing wtmp files and then using last -f.  I've made
a patch for your consideration that does the following:

a) it checks if the file is a gzipped file by looking at the wtmp's file magic
b) it writes the gzipped file to a /tmp location uncompressed so that the
   normal way of operation can be done on the tmp file.

I didn't want to start the discussion empty handed whether this is a good
patch or not, so I made the patch but it needs cleanup and probably a manpage
change.  Let me know if this could go in, before I do any more work.

-peter


--- last.c.orig Sat Apr  9 22:33:55 2011
+++ last.c  Sat Apr  9 23:49:39 2011
@@ -45,10 +45,12 @@
 #include tzfile.h
 #include unistd.h
 #include utmp.h
+#include zlib.h
 
 #defineNO  0   /* false/no */
 #defineYES 1   /* true/yes */
 #define ATOI2(ar)  ((ar)[0] - '0') * 10 + ((ar)[1] - '0'); (ar) += 2;
+#define GZIP   \037\213  /* gzipped file */
 
 static struct utmp buf[1024];  /* utmp read buffer */
 
@@ -74,6 +76,7 @@
 static time_t  snaptime = 0;   /* report only at this time */
 static int calculate = 0;
 static int seconds = 0;
+static char*tmpdir = NULL;
 
 voidaddarg(int, char *);
 struct ttytab  *addtty(char *);
@@ -85,6 +88,10 @@
 voidwtmp(void);
 voidcheckargs(void);
 voidusage(void);
+char   *create_tmp(char *);
+void   cleanup_tmp(char *);
+intisgzipped(void);
+void   gzcopy(void);
 
 #define NAME_WIDTH 9
 #define HOST_WIDTH 24
@@ -161,8 +168,18 @@
}
}
 
+   if (isgzipped()) {
+   tmpdir = create_tmp(file);
+   gzcopy();
+   file = tmpdir;
+   }
+
+   
checkargs();
wtmp();
+
+   if (tmpdir != NULL)
+   cleanup_tmp(tmpdir);
exit(0);
 }
 
@@ -624,6 +641,10 @@
snprintf(str, sizeof str, \ninterrupted %10.10s %8.8s \n,
ct, ct + 11);
write(STDOUT_FILENO, str, strlen(str));
+
+   if (tmpdir != NULL)
+   cleanup_tmp(tmpdir);
+
if (signo == SIGINT)
_exit(1);
 }
@@ -637,4 +658,103 @@
usage: %s [-csT] [-d date] [-f file] [-h host]
 [-n number] [-t tty] [user ...]\n, __progname);
exit(1);
+}
+
+/*
+ * create a temporary directory where a temporary file can be put in
+ */
+
+char *
+create_tmp(char *file)
+{
+   static char tmpfile[MAXPATHLEN];
+   char d0[MAXPATHLEN];
+   
+   char *p;
+   char *basename;
+
+   snprintf(d0, sizeof(d0), /tmp/last.);
+   mkdtemp(d0);
+
+   basename = strrchr(file, '/');
+   snprintf(tmpfile, sizeof(tmpfile), %s/%s, d0, basename);
+   p = strrchr(tmpfile, '.');
+   *p = '\0';
+
+   return ((char *)tmpfile);
+}  
+
+/*
+ * clean temporary file and directory in /tmp
+ */
+
+void
+cleanup_tmp(char *tmpfile)
+{
+   char *sep;
+
+   unlink(tmpfile);
+   sep = strrchr(tmpfile, '/');
+   *sep = '\0';
+   
+   rmdir(tmpfile);
+}
+
+/*
+ * determine if a wtmp file is gzipped or not, taken from /etc/magic 
+ */
+
+int
+isgzipped(void)
+{
+   char buf[2];
+   char *cmp = GZIP;
+   int fd;
+
+   if ((fd = open(file, O_RDONLY, 0))  0) {
+   perror(open);
+   exit(1);
+   }
+
+   if (read(fd, buf, sizeof(buf)) != sizeof(buf)) {
+   perror(read);
+   exit(1);
+   }   
+
+   close(fd);
+
+   if (memcmp(cmp, buf, sizeof(buf)) == 0) {
+   return (1);
+   }
+
+   return (0);
+}
+
+/*
+ * copy gzipped file to file tmpdir
+ */
+void
+gzcopy(void)
+{
+   int fd, len;
+
+   char buf[512];
+   gzFile *gzt;
+   
+   fd = open(tmpdir, O_WRONLY | O_CREAT, 0600);
+   if (fd  0) {
+   perror(open);
+   exit(1);
+   }
+
+   gzt = gzopen(file, r);
+   while ((len = gzread(gzt, buf, sizeof(buf)))  0) {
+   if (write(fd, buf, len)  0) {
+   perror(write);
+   exit(1);
+   }
+   }
+
+   gzclose(gzt);
+   close(fd);
 }

Re: last patch, idea

[Peter J. Philipp]

On Sun, Apr 10, 2011 at 10:08:24AM -0400, Ian Darwin wrote:
  Having tried to do things like gzcat /var/log/wtmp.0.gz | last -f /dev/stdin
  before, I'd certainly find it useful and this is less intrusive than 
  modifying
  last(8) so it could work with standard input.
 
 Unless you run an extermely large shop like Beck does, or have extremely
 tiny disks, why not just remove all the Z flags from newsyslog.conf?
 This has the side effect of not having to gunzip /var/log/daemon*  friends.

This is a good idea.  I guess I don't need my patch then.  I won't clean it
up then either.

 I guess we inherited this log gzipping from 4BSD, but in those days
 a 300MB disk cost a month's salary. Plus another week's salary or so
 for the trucking charges.

funny. :-)

-peter

Anyone interested in writing a driver for this?

[Peter J. Philipp]

Hi,

I have a USB device called a USB FM transmitter from Keene Electronics.
It looks like this when I plug it in.

uaudio0 at uhub1 port 1 configuration 1 interface 0 HOLTEK B-LINK USB Audio 
rev 1.10/1.00 addr 3
uaudio0: audio rev 1.00, 2 mixer controls
audio1 at uaudio0
uhidev0 at uhub1 port 1 configuration 1 interface 2 HOLTEK B-LINK USB Audio 
rev 1.10/1.00 addr 3
uhidev0: no report descriptor

Basically there is two USB hids to this device, one is a standard audio device
that acts like a sound card and transmits on FM.  The second part controls it,
sets its frequency and other things such as US/Europe FM stepping, TX gain
etc.  The driver for the audio exists as you can see.  The second part is
partially done by kenchy who did an incredible amount of reverse engineering
of this device and wrote a libUSB program to set frequency and turns it on.

https://github.com/kenchy/keene-usb-audio

But the device has a few more knobs that I would love to see in a driver,
such as TX gain and US/Europe settings, basically everything that the Windows
XP driver does.

Is anyone who is knowledged in writing a USB driver interested in writing 
a driver for this?  I'd send you this device free of charge as it's 
collecting dust over here.  There was a USB extension cable in the package
which I have misplaced other than that there is a mini CDROM and the 
device itself.

Kenchy has no license on his libusb program so its info can easily be used
to make a BSD licensed driver out of this.

Let me know in private email if you're interested.

-peter

Re: Anyone interested in writing a driver for this?

[Peter J. Philipp]

On Thu, Jul 07, 2011 at 07:45:48PM +0200, Peter J. Philipp wrote:
 Hi,
 
 I have a USB device called a USB FM transmitter from Keene Electronics.
 It looks like this when I plug it in.

I've found someone to send it to.  If they have no luck they said they'd
take it along to the next hackathon they're attending in order to give it
to someone who may also be interested.

Thanks!

-peter

smtpd.conf.5 match reality

[Peter J. Philipp]

Thanks to kdump I was able to figure this one out before reading the source.

-peter

? smtpd.conf.5.patch
Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.45
diff -u -r1.45 smtpd.conf.5
--- smtpd.conf.53 Oct 2011 17:57:43 -   1.45
+++ smtpd.conf.522 Nov 2011 17:57:56 -
@@ -417,7 +417,7 @@
 The configuration file would look like this:
 .Bd -literal -offset indent
 listen on lo0
-listen on bnx0 tls certificate mail.example.com.crt enable auth
+listen on bnx0 tls certificate mail.example.com enable auth
 map aliases { source db /etc/mail/aliases.db }
 accept for local deliver to mda /path/to/mda -f -
 accept from all for domain example.org deliver to mda /path/to/mda -f -

path correction

[Peter J. Philipp]

This probably saw some debate in the past, which I did not see.  On my IRC
channel it is concensus that the path given out is dangerous.

-peter

Index: dot.profile
===
RCS file: /cvs/src/etc/skel/dot.profile,v
retrieving revision 1.4
diff -u -r1.4 dot.profile
--- dot.profile 16 Feb 2005 06:56:57 -  1.4
+++ dot.profile 13 Apr 2012 15:05:11 -
@@ -2,5 +2,5 @@
 #
 # sh/ksh initialization
 
-PATH=$HOME/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/games:.
+PATH=/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/games:$HOME/bin:.
 export PATH HOME TERM

Re: path correction

[Peter J. Philipp]

On Fri, Apr 13, 2012 at 05:08:32PM +0200, Peter J. Philipp wrote:
 This probably saw some debate in the past, which I did not see.  On my IRC
 channel it is concensus that the path given out is dangerous.

I'd like to retract this patch.  I lied.  Yes I told a lie.  Danger talking or
discussing about this patch will make friends foes.  Just leave it as it is, 
it's been so for 11 years anyhow.  

-peter

ip6(4) manpage update

[Peter J. Philipp]

Hi,

I just got through a thread in misc@,

http://marc.info/?l=openbsd-miscm=133934252713974w=2

and it seems like the sample code in ip6(4) is wrong.  I've made adjustments
but it doesn't look as nice anymore, perhaps someone can look over it?  These
changes will really help someone first time trying the sample code, I think.
Credit should be given to Simon Perreault, I just did what he suggested.

-peter

patch below (against 5.1):


Index: ip6.4
===
RCS file: /cvs/src/share/man/man4/ip6.4,v
retrieving revision 1.25
diff -u -r1.25 ip6.4
--- ip6.4   8 Sep 2011 16:43:56 -   1.25
+++ ip6.4   11 Jun 2012 19:26:44 -
@@ -251,6 +251,7 @@
 };
 .Ed
 .It Dv IPV6_HOPLIMIT Fa int *
+.It Dv IPV6_RECVHOPLIMIT Fa int *
 Get or set whether the hop limit header field from subsequent packets
 will be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -590,7 +591,7 @@
  * returned along with the payload.
  */
 optval = 1;
-if (setsockopt(s, IPPROTO_IPV6, IPV6_HOPLIMIT, optval,
+if (setsockopt(s, IPPROTO_IPV6, IPV6_RECVHOPLIMIT, optval,
 sizeof(optval)) == -1)
err(1, setsockopt);
 
@@ -685,6 +686,15 @@
 .%A B. Fenner
 .%A A. Rudoff
 .%T UNIX Network Programming, third edition
+.Re
+.Rs
+.%A W. Stevens
+.%A M. Thomas
+.%A E. Nordmark
+.%A T. Jinmei
+.%T Advanced Sockets Application Program Interface (API) for IPv6
+.%R RFC 3542
+.%D May 2003
 .Re
 .Sh STANDARDS
 Most of the socket options are defined in RFC 2292 or RFC 2553.

Re: ip6(4) manpage update

[Peter J. Philipp]

On Sat, Jun 16, 2012 at 07:17:16PM -0700, Philip Guenther wrote:
  You can expect the same issue with IPV6_PKTINFO, IPV6_HOPOPTS, IPV6_DSTOPTS,
  and IPV6_RTHDR. The RECV part was added to them in RFC3542.
 
 Yep.  In addition, the text should be clarified to indicate that
 turning on IPV6_RECV* will result in the process getting cmsg data of
 type IPV6_*.  E.g., IPV6_RECVHOPLIMIT turns on receiving of
 IPV6_HOPLIMIT cmsg data.
 
 Peter, do you want to take a stab at that part too?
 
 
 (There's also a typo in the current page: s/HOPTLIMIT/HOPLIMIT/)
 
 
 Philip Guenther

Sure I'll take a stab at it but its very difficult I found (I was also 
distracted by freeing a bird from the attic).  Here goes:

-peter

Index: ip6.4
===
RCS file: /cvs/src/share/man/man4/ip6.4,v
retrieving revision 1.25
diff -u -r1.25 ip6.4
--- ip6.4   8 Sep 2011 16:43:56 -   1.25
+++ ip6.4   17 Jun 2012 10:45:19 -
@@ -237,7 +237,7 @@
 .It Dv IPV6_PORTRANGE_LOW
 Use a low, reserved range (600\-1023).
 .El
-.It Dv IPV6_PKTINFO Fa int *
+.It Dv IPV6_RECVPKTINFO Fa int *
 Get or set whether additional information about subsequent packets will
 be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -250,14 +250,19 @@
unsigned intipi6_ifindex; /* send/recv if index */
 };
 .Ed
-.It Dv IPV6_HOPLIMIT Fa int *
+
+Turning this option on will result in this process getting cmsg data of
+type IPV6_PKTINFO.
+.It Dv IPV6_RECVHOPLIMIT Fa int *
 Get or set whether the hop limit header field from subsequent packets
 will be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
-calls.
+calls. 
 The value is stored as an
 .Vt int
 in the ancillary data returned.
+Turning this option on will result in this process getting cmsg data of
+type IPV6_HOPLIMIT.
 .\ .It Dv IPV6_NEXTHOP Fa int *
 .\ Get or set whether the address of the next hop for subsequent
 .\ packets will be provided as ancillary data along with the payload in
@@ -269,7 +274,7 @@
 .\ structure in the ancillary data returned.
 .\ .Pp
 .\ This option requires superuser privileges.
-.It Dv IPV6_HOPOPTS Fa int *
+.It Dv IPV6_RECVHOPOPTS Fa int *
 Get or set whether the hop-by-hop options from subsequent packets will be
 provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -288,8 +293,10 @@
 .Fn inet6_option_space
 routine and family of routines may be used to manipulate this data.
 .Pp
-This option requires superuser privileges.
-.It Dv IPV6_DSTOPTS Fa int *
+This option requires superuser privileges.  
+Turning this option on will result in this process getting cmsg data of
+type IPV6_HOPOPTS.
+.It Dv IPV6_RECVDSTOPTS Fa int *
 Get or set whether the destination options from subsequent packets will
 be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -309,6 +316,8 @@
 routine and family of routines may be used to manipulate this data.
 .Pp
 This option requires superuser privileges.
+Turning this option on will result in this process getting cmsg data of
+type IPV6_DSTOPTS.
 .It Dv IPV6_TCLASS Fa int *
 Get or set the value of the traffic class field used for outgoing datagrams
 on this socket.
@@ -321,7 +330,7 @@
 calls.
 The header field is stored as a single value of type
 .Vt int .
-.It Dv IPV6_RTHDR Fa int *
+.It Dv IPV6_RECVRTHDR Fa int *
 Get or set whether the routing header from subsequent packets will be
 provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -343,6 +352,8 @@
 routine and family of routines may be used to manipulate this data.
 .Pp
 This option requires superuser privileges.
+Turning this option on will result in this process getting cmsg data of
+type IPV6_RTHDR.
 .It Dv IPV6_PKTOPTIONS Fa struct cmsghdr *
 Get or set all header options and extension headers at one time on the
 last packet sent or received on the socket.
@@ -413,11 +424,11 @@
 .El
 .Pp
 The
-.Dv IPV6_PKTINFO ,
-.\ .Dv IPV6_NEXTHOP ,
-.Dv IPV6_HOPLIMIT ,
-.Dv IPV6_HOPOPTS ,
-.Dv IPV6_DSTOPTS ,
+.Dv IPV6_RECVPKTINFO ,
+.\ .Dv IPV6_RECVNEXTHOP ,
+.Dv IPV6_RECVHOPLIMIT ,
+.Dv IPV6_RECVHOPOPTS ,
+.Dv IPV6_RECVDSTOPTS ,
 and
 .Dv IPV6_RTHDR
 options will return ancillary data along with payload contents in subsequent
@@ -429,7 +440,7 @@
 and
 .Va cmsg_type
 set to respective option name value (e.g.,
-.Dv IPV6_HOPTLIMIT ) .
+.Dv IPV6_HOPLIMIT ) .
 These options may also be used directly as ancillary
 .Va cmsg_type
 values in
@@ -455,7 +466,7 @@
 can be set by the
 .Dv IPV6_MULTICAST_IF
 socket option, through the
-.Dv IPV6_PKTINFO
+.Dv IPV6_RECVPKTINFO
 option, and through the
 .Va sin6_scope_id
 field of the socket address passed to the
@@ -590,7 +601,7 @@
  * returned along with the payload.
  */
 optval = 1;
-if (setsockopt(s, IPPROTO_IPV6, IPV6_HOPLIMIT, optval,
+if (setsockopt(s, IPPROTO_IPV6, IPV6_RECVHOPLIMIT, optval,
 sizeof(optval)) == -1)
err(1, 

Re: ip6(4) manpage update

[Peter J. Philipp]

On Sun, Jun 17, 2012 at 12:49:08PM +0200, Peter J. Philipp wrote:
 On Sat, Jun 16, 2012 at 07:17:16PM -0700, Philip Guenther wrote:
   You can expect the same issue with IPV6_PKTINFO, IPV6_HOPOPTS, 
   IPV6_DSTOPTS,
   and IPV6_RTHDR. The RECV part was added to them in RFC3542.
  
  Yep.  In addition, the text should be clarified to indicate that
  turning on IPV6_RECV* will result in the process getting cmsg data of
  type IPV6_*.  E.g., IPV6_RECVHOPLIMIT turns on receiving of
  IPV6_HOPLIMIT cmsg data.
  
  Peter, do you want to take a stab at that part too?
  
  
  (There's also a typo in the current page: s/HOPTLIMIT/HOPLIMIT/)
  
  
  Philip Guenther
 
 Sure I'll take a stab at it but its very difficult I found (I was also 
 distracted by freeing a bird from the attic).  Here goes:

attempt two.. process was the wrong word, I substituted it with socket.

-peter

Index: ip6.4
===
RCS file: /cvs/src/share/man/man4/ip6.4,v
retrieving revision 1.25
diff -u -r1.25 ip6.4
--- ip6.4   8 Sep 2011 16:43:56 -   1.25
+++ ip6.4   17 Jun 2012 10:45:19 -
@@ -237,7 +237,7 @@
 .It Dv IPV6_PORTRANGE_LOW
 Use a low, reserved range (600\-1023).
 .El
-.It Dv IPV6_PKTINFO Fa int *
+.It Dv IPV6_RECVPKTINFO Fa int *
 Get or set whether additional information about subsequent packets will
 be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -250,14 +250,19 @@
unsigned intipi6_ifindex; /* send/recv if index */
 };
 .Ed
-.It Dv IPV6_HOPLIMIT Fa int *
+
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_PKTINFO.
+.It Dv IPV6_RECVHOPLIMIT Fa int *
 Get or set whether the hop limit header field from subsequent packets
 will be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
-calls.
+calls. 
 The value is stored as an
 .Vt int
 in the ancillary data returned.
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_HOPLIMIT.
 .\ .It Dv IPV6_NEXTHOP Fa int *
 .\ Get or set whether the address of the next hop for subsequent
 .\ packets will be provided as ancillary data along with the payload in
@@ -269,7 +274,7 @@
 .\ structure in the ancillary data returned.
 .\ .Pp
 .\ This option requires superuser privileges.
-.It Dv IPV6_HOPOPTS Fa int *
+.It Dv IPV6_RECVHOPOPTS Fa int *
 Get or set whether the hop-by-hop options from subsequent packets will be
 provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -288,8 +293,10 @@
 .Fn inet6_option_space
 routine and family of routines may be used to manipulate this data.
 .Pp
-This option requires superuser privileges.
-.It Dv IPV6_DSTOPTS Fa int *
+This option requires superuser privileges.  
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_HOPOPTS.
+.It Dv IPV6_RECVDSTOPTS Fa int *
 Get or set whether the destination options from subsequent packets will
 be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -309,6 +316,8 @@
 routine and family of routines may be used to manipulate this data.
 .Pp
 This option requires superuser privileges.
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_DSTOPTS.
 .It Dv IPV6_TCLASS Fa int *
 Get or set the value of the traffic class field used for outgoing datagrams
 on this socket.
@@ -321,7 +330,7 @@
 calls.
 The header field is stored as a single value of type
 .Vt int .
-.It Dv IPV6_RTHDR Fa int *
+.It Dv IPV6_RECVRTHDR Fa int *
 Get or set whether the routing header from subsequent packets will be
 provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -343,6 +352,8 @@
 routine and family of routines may be used to manipulate this data.
 .Pp
 This option requires superuser privileges.
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_RTHDR.
 .It Dv IPV6_PKTOPTIONS Fa struct cmsghdr *
 Get or set all header options and extension headers at one time on the
 last packet sent or received on the socket.
@@ -413,11 +424,11 @@
 .El
 .Pp
 The
-.Dv IPV6_PKTINFO ,
-.\ .Dv IPV6_NEXTHOP ,
-.Dv IPV6_HOPLIMIT ,
-.Dv IPV6_HOPOPTS ,
-.Dv IPV6_DSTOPTS ,
+.Dv IPV6_RECVPKTINFO ,
+.\ .Dv IPV6_RECVNEXTHOP ,
+.Dv IPV6_RECVHOPLIMIT ,
+.Dv IPV6_RECVHOPOPTS ,
+.Dv IPV6_RECVDSTOPTS ,
 and
 .Dv IPV6_RTHDR
 options will return ancillary data along with payload contents in subsequent
@@ -429,7 +440,7 @@
 and
 .Va cmsg_type
 set to respective option name value (e.g.,
-.Dv IPV6_HOPTLIMIT ) .
+.Dv IPV6_HOPLIMIT ) .
 These options may also be used directly as ancillary
 .Va cmsg_type
 values in
@@ -455,7 +466,7 @@
 can be set by the
 .Dv IPV6_MULTICAST_IF
 socket option, through the
-.Dv IPV6_PKTINFO
+.Dv IPV6_RECVPKTINFO
 option, and through the
 .Va sin6_scope_id
 field of the socket address passed to the
@@ -590,7 +601,7 @@
  * returned along with the payload.
  */
 optval = 1

Re: ip6(4) manpage update

[Peter J. Philipp]

On Mon, Jun 18, 2012 at 08:06:06AM +0100, Jason McIntyre wrote:
 the blank line above should be a .Pp.
 
 also this diff adds trailing whitespace at eol in a few places. please
 remove it.
 
 except for that, i'm fine with this diff, if some developer wants to
 take it.
 
 jmc

Awesome!  Well here is my attempt three then.


Index: ip6.4
===
RCS file: /cvs/src/share/man/man4/ip6.4,v
retrieving revision 1.25
diff -u -r1.25 ip6.4
--- ip6.4   8 Sep 2011 16:43:56 -   1.25
+++ ip6.4   18 Jun 2012 08:06:35 -
@@ -237,7 +237,7 @@
 .It Dv IPV6_PORTRANGE_LOW
 Use a low, reserved range (600\-1023).
 .El
-.It Dv IPV6_PKTINFO Fa int *
+.It Dv IPV6_RECVPKTINFO Fa int *
 Get or set whether additional information about subsequent packets will
 be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -250,7 +250,10 @@
unsigned intipi6_ifindex; /* send/recv if index */
 };
 .Ed
-.It Dv IPV6_HOPLIMIT Fa int *
+.Pp
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_PKTINFO.
+.It Dv IPV6_RECVHOPLIMIT Fa int *
 Get or set whether the hop limit header field from subsequent packets
 will be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -258,6 +261,8 @@
 The value is stored as an
 .Vt int
 in the ancillary data returned.
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_HOPLIMIT.
 .\ .It Dv IPV6_NEXTHOP Fa int *
 .\ Get or set whether the address of the next hop for subsequent
 .\ packets will be provided as ancillary data along with the payload in
@@ -269,7 +274,7 @@
 .\ structure in the ancillary data returned.
 .\ .Pp
 .\ This option requires superuser privileges.
-.It Dv IPV6_HOPOPTS Fa int *
+.It Dv IPV6_RECVHOPOPTS Fa int *
 Get or set whether the hop-by-hop options from subsequent packets will be
 provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -289,7 +294,9 @@
 routine and family of routines may be used to manipulate this data.
 .Pp
 This option requires superuser privileges.
-.It Dv IPV6_DSTOPTS Fa int *
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_HOPOPTS.
+.It Dv IPV6_RECVDSTOPTS Fa int *
 Get or set whether the destination options from subsequent packets will
 be provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -309,6 +316,8 @@
 routine and family of routines may be used to manipulate this data.
 .Pp
 This option requires superuser privileges.
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_DSTOPTS.
 .It Dv IPV6_TCLASS Fa int *
 Get or set the value of the traffic class field used for outgoing datagrams
 on this socket.
@@ -321,7 +330,7 @@
 calls.
 The header field is stored as a single value of type
 .Vt int .
-.It Dv IPV6_RTHDR Fa int *
+.It Dv IPV6_RECVRTHDR Fa int *
 Get or set whether the routing header from subsequent packets will be
 provided as ancillary data along with the payload in subsequent
 .Xr recvmsg 2
@@ -343,6 +352,8 @@
 routine and family of routines may be used to manipulate this data.
 .Pp
 This option requires superuser privileges.
+Turning this option on will result in this socket getting cmsg data of
+type IPV6_RTHDR.
 .It Dv IPV6_PKTOPTIONS Fa struct cmsghdr *
 Get or set all header options and extension headers at one time on the
 last packet sent or received on the socket.
@@ -413,11 +424,11 @@
 .El
 .Pp
 The
-.Dv IPV6_PKTINFO ,
-.\ .Dv IPV6_NEXTHOP ,
-.Dv IPV6_HOPLIMIT ,
-.Dv IPV6_HOPOPTS ,
-.Dv IPV6_DSTOPTS ,
+.Dv IPV6_RECVPKTINFO ,
+.\ .Dv IPV6_RECVNEXTHOP ,
+.Dv IPV6_RECVHOPLIMIT ,
+.Dv IPV6_RECVHOPOPTS ,
+.Dv IPV6_RECVDSTOPTS ,
 and
 .Dv IPV6_RTHDR
 options will return ancillary data along with payload contents in subsequent
@@ -429,7 +440,7 @@
 and
 .Va cmsg_type
 set to respective option name value (e.g.,
-.Dv IPV6_HOPTLIMIT ) .
+.Dv IPV6_HOPLIMIT ) .
 These options may also be used directly as ancillary
 .Va cmsg_type
 values in
@@ -455,7 +466,7 @@
 can be set by the
 .Dv IPV6_MULTICAST_IF
 socket option, through the
-.Dv IPV6_PKTINFO
+.Dv IPV6_RECVPKTINFO
 option, and through the
 .Va sin6_scope_id
 field of the socket address passed to the
@@ -590,7 +601,7 @@
  * returned along with the payload.
  */
 optval = 1;
-if (setsockopt(s, IPPROTO_IPV6, IPV6_HOPLIMIT, optval,
+if (setsockopt(s, IPPROTO_IPV6, IPV6_RECVHOPLIMIT, optval,
 sizeof(optval)) == -1)
err(1, setsockopt);
 
@@ -685,6 +696,15 @@
 .%A B. Fenner
 .%A A. Rudoff
 .%T UNIX Network Programming, third edition
+.Re
+.Rs
+.%A W. Stevens
+.%A M. Thomas
+.%A E. Nordmark
+.%A T. Jinmei
+.%T Advanced Sockets Application Program Interface (API) for IPv6
+.%R RFC 3542
+.%D May 2003
 .Re
 .Sh STANDARDS
 Most of the socket options are defined in RFC 2292 or RFC 2553.

tftpd patch

[Peter J. Philipp]

Hi,

I have the weird scenario when I try to tftp a file from a remote tftpd that's
also openbsd that my pf doesn't keep a state open.  This is something I need
to fix, however I found this in the logs on the remote tftpd and it's 
misleading:

Jun 28 14:03:21 hostname tftpd[2506]: recv: Connection refused

It first boggled my mind what it's trying to recv and then it came to me...
the write error message is delayed because of the ICMP port unreachable 
travel time at which point the descriptor is already blocking in read I guess.
So I have changed it to this:

Jun 28 14:03:21 hostname tftpd[2506]: sendfile: Connection refused

which to me is a lot more explanatory on what it fails on.  sendfile is
the function not the syscall.  I'd rather see send in there than recv.

Here is the patch:


Index: tftpd.c
===
RCS file: /cvs/src/libexec/tftpd/tftpd.c,v
retrieving revision 1.63
diff -u -r1.63 tftpd.c
--- tftpd.c 27 Oct 2009 23:59:32 -  1.63
+++ tftpd.c 28 Jun 2012 18:00:29 -
@@ -669,7 +669,10 @@
error = 1;
if (errno == EINTR)
continue;
-   syslog(LOG_ERR, recv: %m);
+   if (errno == ECONNREFUSED) 
+   syslog(LOG_ERR, sendfile: %m);
+   else
+   syslog(LOG_ERR, recv: %m);
goto abort;
}
ap-th_opcode = ntohs((u_short)ap-th_opcode);



If you think kittens will die because of this patch then don't commit it
but otherwise I'm just trying to make sense of this better.

Cheers,

-peter

USB Wireless Micro Adapter IWL 4000 support

[Peter J. Philipp]

First off I'd like to say that today luck was with me.  Big time.  I went to
a local store (saturn.de) to buy a wireless usb adapter and picked one out 
that I thought was supported.  I did not take my netbook with me so I didn't
know if it would work or not.  So when I got home it was detected as ugen0...
damn.  So then I compared the USB Vendor in a list and made an educated guess
that this is a urtwn(4) device and hacked the support into the kernel and
rebooted.  Then I prayed and it did configure and attach!  success then I
did a ifconfig urtwn scan and it worked, success and then I read the manpage
on how to configure wireless as I've never done it on OpenBSD since 2002.

Anyhow the driver specification needs a better name I called it PJPUK for
my initials and unknown.  But I can give you information about the box what
I bought:   This is a ISY (simply connected) USB Wireless Micro Adapter,
IWL 4000 and claimed to be a (N300 standard).  Thank you Damien Bergamini
for this driver!

Here is some relevant information:

ifconfig urtwn0

urtwn0: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 1500
lladdr ec:1a:59:0d:fa:1c
priority: 4
groups: wlan egress
media: IEEE802.11 autoselect (OFDM54 mode 11g)
status: active
ieee80211: nwid AREA27 chan 11 bssid 24:65:11:68:a1:43 196dB wpakey 
CENSORED wpaprotos wpa1,wpa2 wpaakms psk wpaciphers tkip,ccmp wpagroupcipher 
tkip
inet6 fe80::ee1a:59ff:fe0d:fa1c%urtwn0 prefixlen 64 scopeid 0x4
inet 192.168.178.36 netmask 0xff00 broadcast 192.168.178.255

usbdevs -v

Controller /dev/usb0:
addr 1: high speed, self powered, config 1, EHCI root hub(0x), ATI(0x1002), 
rev 1.00
 port 1 addr 2: high speed, power 500 mA, config 1, Belkin Wireless 
Adapter(0x21f2), Realtek(0x050d), rev 2.00, iSerialNumber 00e04c01
 port 2 powered
 port 3 powered
 port 4 powered
 port 5 powered
Controller /dev/usb1:
...


The diff against a -current snapshot from last week:

Index: if_urtwn.c
===
RCS file: /cvs/src/sys/dev/usb/if_urtwn.c,v
retrieving revision 1.23
diff -u -r1.23 if_urtwn.c
--- if_urtwn.c  17 Sep 2012 15:14:14 -  1.23
+++ if_urtwn.c  20 Nov 2012 15:11:56 -
@@ -83,6 +83,7 @@
{ USB_VENDOR_AZUREWAVE, USB_PRODUCT_AZUREWAVE_RTL8188CE_2 },
{ USB_VENDOR_AZUREWAVE, USB_PRODUCT_AZUREWAVE_RTL8188CU },
{ USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_F7D2102 },
+   { USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_PJPUK },
{ USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_RTL8188CU },
{ USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_RTL8192CU },
{ USB_VENDOR_CHICONY,   USB_PRODUCT_CHICONY_RTL8188CUS_1 },
Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.591
diff -u -r1.591 usbdevs
--- usbdevs 14 Nov 2012 16:53:49 -  1.591
+++ usbdevs 20 Nov 2012 15:11:59 -
@@ -1118,6 +1118,7 @@
 product BELKIN F5U120  0x1203  F5U120-PC Hub
 product BELKIN RTL8192CU   0x2102  RTL8192CU
 product BELKIN F7D2102 0x2103  F7D2102
+product BELKIN PJPUK   0x21f2  PJPUK
 product BELKIN ZD1211B 0x4050  ZD1211B
 product BELKIN F5D5055 0x5055  F5D5055
 product BELKIN F5D7050 0x7050  F5D7050 54g USB Network Adapter


and last but not least a dmesg:

OpenBSD 5.2-current (SATURN) #1: Tue Nov 20 15:57:38 CET 2012
p...@saturn.centroid.eu:/usr/src/sys/arch/amd64/compile/SATURN
RTC BIOS diagnostic error 80clock_battery
real mem = 4003721216 (3818MB)
avail mem = 3874660352 (3695MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xe3e70 (51 entries)
bios0: vendor Acer version V1.08 date 12/06/2011
bios0: Acer AO722
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP HPET APIC MCFG BOOT SLIC SSDT SSDT
acpi0: wakeup devices SPB2(S4) GEC_(S4) USB0(S3) USB4(S3) P2P_(S5)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpihpet0 at acpi0: 14318180 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD C-60 APU with Radeon(tm) HD Graphics, 998.31 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,MWAIT,SSSE3,CX16,POPCNT,NXE,MMXX,FFXSR,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,IBS,SKINIT,ITSC
cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 
16-way L2 cache
cpu0: 8 4MB entries fully associative
cpu0: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
cpu0: apic clock running at 199MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD C-60 APU with Radeon(tm) HD Graphics, 997.51 MHz
cpu1: 

Re: USB Wireless Micro Adapter IWL 4000 support

[Peter J. Philipp]

On Tue, Nov 20, 2012 at 04:33:27PM +0100, Peter J. Philipp wrote:
 urtwn0 at uhub0 port 1 Realtek Belkin Wireless Adapter rev 2.00/2.00 addr 2
 urtwn0: MAC/BB RTL8192CU, RF 6052 2T2R, address ec:1a:59:0d:fa:1c

Hrmm, sometimes it does not detect right.  I had to cold boot my netbook
last for it to detect, the dmesg has also changed:

urtwn0 at uhub0 port 1 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: MAC/BB RTL8192CU, RF 6052 2T2R, address ec:1a:59:0d:fa:1c

For the fact that it works is amazing to me.  However not perfectly, and 
that skillset is beyond me, but I thought I'd let you know.

-peter

urtwn(4) patch

[Peter J. Philipp]

Hi,

I previously sent out a patch for this device support here:

   Linkname: 'USB Wireless Micro Adapter IWL 4000 support' - MARC
URL: http://marc.info/?l=openbsd-techm=135342591418924w=2

Now I've looked at the usbdevs file a little closer and finally replaced
my PJPUK device with something a little more proper, here is patch:


Index: if_urtwn.c
===
RCS file: /cvs/src/sys/dev/usb/if_urtwn.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 if_urtwn.c
--- if_urtwn.c  15 Apr 2013 09:23:01 -  1.25
+++ if_urtwn.c  11 May 2013 21:36:59 -
@@ -83,6 +83,7 @@ static const struct usb_devno urtwn_devs
{ USB_VENDOR_AZUREWAVE, USB_PRODUCT_AZUREWAVE_RTL8188CE_2 },
{ USB_VENDOR_AZUREWAVE, USB_PRODUCT_AZUREWAVE_RTL8188CU },
{ USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_F7D2102 },
+   { USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_RTL8192CU_4 },
{ USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_RTL8188CU },
{ USB_VENDOR_BELKIN,USB_PRODUCT_BELKIN_RTL8192CU },
{ USB_VENDOR_CHICONY,   USB_PRODUCT_CHICONY_RTL8188CUS_1 },
Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.598
diff -u -p -u -r1.598 usbdevs
--- usbdevs 7 Mar 2013 23:39:14 -   1.598
+++ usbdevs 11 May 2013 21:39:19 -
@@ -1120,6 +1120,7 @@ product BELKIN RTL8188CU  0x1102  RTL8188C
 product BELKIN F5U120  0x1203  F5U120-PC Hub
 product BELKIN RTL8192CU   0x2102  RTL8192CU
 product BELKIN F7D2102 0x2103  F7D2102
+product BELKIN RTL8192CU_4 0x21f2  RTL8192CU
 product BELKIN ZD1211B 0x4050  ZD1211B
 product BELKIN F5D5055 0x5055  F5D5055
 product BELKIN F5D7050 0x7050  F5D7050 54g USB Network Adapter


The particular device doesn't always attach right, as you can see from this
grep from my kernel dmesg:

urtwn0 at uhub0 port 2 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: could not set configuration no
urtwn0 detached
urtwn0 at uhub0 port 1 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: MAC/BB RTL8192CU, RF 6052 2T2R, address ec:1a:59:0d:fa:1c
urtwn0 at uhub0 port 1 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: could not set configuration no
urtwn0 detached
urtwn0 at uhub0 port 1 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: could not set configuration no
urtwn0 detached
urtwn0 at uhub0 port 2 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: could not set configuration no
urtwn0 detached
urtwn0 at uhub0 port 1 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: MAC/BB RTL8192CU, RF 6052 2T2R, address ec:1a:59:0d:fa:1c
urtwn0 at uhub0 port 2 Belkin Components PJPUK rev 2.00/2.00 addr 2
urtwn0: MAC/BB RTL8192CU, RF 6052 2T2R, address ec:1a:59:0d:fa:1c
urtwn0 at uhub0 port 2 Belkin Components RTL8192CU rev 2.00/2.00 addr 2
urtwn0: MAC/BB RTL8192CU, RF 6052 2T2R, address ec:1a:59:0d:fa:1c
urtwn0 at uhub0 port 2 Belkin Components RTL8192CU rev 2.00/2.00 addr 2
urtwn0: could not set configuration no
urtwn0 detached
urtwn0 at uhub0 port 1 Realtek Belkin Wireless Adapter rev 2.00/2.00 addr 2
urtwn0: MAC/BB RTL8192CU, RF 6052 2T2R, address ec:1a:59:0d:fa:1c

it sometimes (1 in 3 attempts) bails with could not set configuration no
I looked at it a little closer and think it's probably in the usb device 
driver code that it determines this.  

-peter

Re: smtpd w/ async DNS

[Peter J. Philipp]

On Sat, Oct 30, 2010 at 04:55:36PM +0200, Gilles Chehade wrote:
 Hi tech@,
 
 A new tarball with all reported issues fixed is available at:
 
 http://www.poolp.org/~gilles/smtpd-asyncdns.tar.gz
 
 smtpd now catches changes in /etc/resolv.conf and should work fine with 
 inet6 records.
 I have done some stress testing here and it's been stable.
 
 asr has also been improved by eric@ who cleaned up code, fixed some bugs 
 and added tcp support, honors the family keyword in /etc/resolv.conf.
 
 Please test fast and report issues so that we can move to the next 
 features ;-)
 
 Gilles

Hi,

Is this a typo?  on line 40 in asr.c

#define DEFAULT_CONFlookup bind file\nameserver 127.0.0.1\n

should it be

lookup bind file\nnameserver 127.0.0.1\n?

regards,

-peter

Re: smtpd w/ async DNS

[Peter J. Philipp]

On Sat, Oct 30, 2010 at 05:28:42PM +0200, Gilles Chehade wrote:
 It was a typo indeed, tarball has been updated and also contains a fix for
 a crash experienced by todd@ when using relay via
 
 Gilles

I had a look at the pack.c file where the DNS compression is being handled.
It looks good to me.  But I have one concern that needs to be confirmed.
In function dname_expand() on lines:

54  ptr = 256 * (n  ~0xc0) + data[offset + 1];
55  if (ptr = offset)
56  return (-1);

The pointer is checked against offset meaning that a compression loop can't 
occur.  This is good.  However what happens if you have a DNS reply packet
with a name with two labels in it, one being a normal label of a name and the 
second being a compression pointer that points back to the first label, 
kinda like so..

[8]centroid[C0 back to 8]

I'm worried it might go into an infinite loop or crash even.

Perhaps it should check that it cannot go back to a label inside a dns name that
is being parsed.

Otherwise rockin' code!  I don't understand it all but the little I do it looks
really high quality!

-peter

/bsd: nd6_ns_input: duplicate IP6 address 2001:0a60:f074:0004::0001

[Peter J. Philipp]

Hi,

I got a new firewall and had to do some plumbing, and _reused_ an IPv6 address
block that was already on an interface (tun0).  Everything worked still but
I got these messages on the firewall (uranus):

Jan  7 16:55:47 uranus /bsd: nd6_ns_input: duplicate IP6 address 
2001:0a60:f074:0004::0001

I googled this message and it seems some other people also have this message
in their kernel.

So I started to chase this message in the kernel and it turns out the old
firewall (cordelia) was sending IPv6 Neighbour Solicitation packets with a 
source address of 2001:a60:f074:4::1.  Since it's IP6 address was 
2001:a60:f074:4::2 I don't know how it got the ::1 until I looked at an 
unused /etc/hostname.tun0 file and it was incorrectly set at 
2001:a60:f074:4::1/64 too.  So I was chasing why it would still send the
solicitation with both source address and destination address being
2001:a60:f074:4::1 and I got lost in the code, but I produced this patch
that may be useful?


Index: nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.55
diff -u -r1.55 nd6_nbr.c
--- nd6_nbr.c   8 Feb 2010 11:56:09 -   1.55
+++ nd6_nbr.c   8 Jan 2011 10:18:25 -
@@ -474,6 +475,14 @@
 */
bzero(src_sa.sin6_addr, sizeof(src_sa.sin6_addr));
}
+
+   if (IN6_ARE_ADDR_EQUAL(src_sa.sin6_addr, dst_sa.sin6_addr)) {
+   log(LOG_INFO, nd6_ns_output: source is same
+   as destination: dst=%s\n,
+   ip6_sprintf(dst_sa.sin6_addr));
+   goto bad;
+   }
+
ip6-ip6_src = src_sa.sin6_addr;
nd_ns = (struct nd_neighbor_solicit *)(ip6 + 1);
nd_ns-nd_ns_type = ND_NEIGHBOR_SOLICIT;





With this patch the packet is stopped on the misconfigured machine and doesn't
cause errors on another machine due to its misconfiguration, while hopefully
still being a nagging pain in the dmesg.

-peter

Re: cksum(1) patch

[Peter J. Philipp]

On Wed, May 13, 2009 at 10:40:13AM +0200, Otto Moerbeek wrote:
 Come to think of it, why don't you just putchar(tolower(hf-name[i]))
 in a loop? Saves you the calloc and error handling.
 
 Also, don't forget to fix usage().
 
   -Otto

Yeah, thanks.  Well I got good and critical feedback and Otto's prodding was 
good enough to make me rewrite this puny patch.  Gone are errno, calloc() and 
in is the putchar().  I stayed away from adding sthen's idea, perhaps he can do 
the patch for that.  Patch follows:


? cksum.1-orig
? cksum.patch
? md5.c-orig
Index: cksum.1
===
RCS file: /cvs/src/bin/md5/cksum.1,v
retrieving revision 1.19
diff -u -r1.19 cksum.1
--- cksum.1 8 Feb 2009 17:15:09 -   1.19
+++ cksum.1 13 May 2009 10:03:46 -
@@ -42,7 +42,7 @@
 .Sh SYNOPSIS
 .Nm cksum
 .Bk -words
-.Op Fl bpqrtx
+.Op Fl blpqrtx
 .Op Fl a Ar algorithms
 .Op Fl c Op Ar checklist ...
 .Op Fl o Ar 1 | 2
@@ -162,6 +162,8 @@
 option may not be used in conjunction with more than a single
 .Fl a
 option.
+.It Fl l
+outputs the algorithms available and exits.
 .It Fl o Ar 1 | 2
 Use historic algorithms instead of the (superior) default one
 (see below).
Index: md5.c
===
RCS file: /cvs/src/bin/md5/md5.c,v
retrieving revision 1.50
diff -u -r1.50 md5.c
--- md5.c   6 Sep 2008 12:01:34 -   1.50
+++ md5.c   13 May 2009 10:03:46 -
@@ -210,14 +210,14 @@
struct hash_list hl;
size_t len;
char *cp, *input_string;
-   int fl, error, base64;
+   int fl, error, base64, i;
int bflag, cflag, pflag, rflag, tflag, xflag;
 
static const char *optstr[5] = {
bcpqrs:tx,
bcpqrs:tx,
bcpqrs:tx,
-   a:bco:pqrs:tx,
+   a:bco:lpqrs:tx,
a:bco:pqrs:tx
};
 
@@ -315,6 +315,15 @@
if (hftmp == TAILQ_END(hl))
hash_insert(hl, hf, 0);
break;
+   case 'l':
+   for (hf = functions; hf-name != NULL; hf++) {
+   len = strlen(hf-name);
+   for (i = 0; i  len; i++) {
+   putchar(tolower(hf-name[i]));
+   }   
+   putchar('\n');
+   }
+   exit(0);
case 'p':
pflag = 1;
break;

Re: cksum(1) patch

[Peter J. Philipp]

On Wed, May 13, 2009 at 12:20:44PM +0200, Otto Moerbeek wrote:
 You forgot to fix usage(). Also, I think it makes sense to allow -l
 for sum(1) too, so that both commands that take -a also take -l. 
 
   -Otto

Eeek.  Ok this will do then:

Regards,
-p


? cksum.1-orig
? cksum.patch
? md5.c-orig
Index: cksum.1
===
RCS file: /cvs/src/bin/md5/cksum.1,v
retrieving revision 1.19
diff -u -r1.19 cksum.1
--- cksum.1 8 Feb 2009 17:15:09 -   1.19
+++ cksum.1 13 May 2009 11:27:53 -
@@ -42,7 +42,7 @@
 .Sh SYNOPSIS
 .Nm cksum
 .Bk -words
-.Op Fl bpqrtx
+.Op Fl blpqrtx
 .Op Fl a Ar algorithms
 .Op Fl c Op Ar checklist ...
 .Op Fl o Ar 1 | 2
@@ -50,7 +50,7 @@
 .Op Ar file ...
 .Ek
 .Nm sum
-.Op Fl bpqrtx
+.Op Fl blpqrtx
 .Op Fl a Ar algorithms
 .Op Fl c Op Ar checklist ...
 .Op Fl o Ar 1 | 2
@@ -162,6 +162,8 @@
 option may not be used in conjunction with more than a single
 .Fl a
 option.
+.It Fl l
+outputs the algorithms available and exits.
 .It Fl o Ar 1 | 2
 Use historic algorithms instead of the (superior) default one
 (see below).
Index: md5.c
===
RCS file: /cvs/src/bin/md5/md5.c,v
retrieving revision 1.50
diff -u -r1.50 md5.c
--- md5.c   6 Sep 2008 12:01:34 -   1.50
+++ md5.c   13 May 2009 11:27:53 -
@@ -210,15 +210,15 @@
struct hash_list hl;
size_t len;
char *cp, *input_string;
-   int fl, error, base64;
+   int fl, error, base64, i;
int bflag, cflag, pflag, rflag, tflag, xflag;
 
static const char *optstr[5] = {
bcpqrs:tx,
bcpqrs:tx,
bcpqrs:tx,
-   a:bco:pqrs:tx,
-   a:bco:pqrs:tx
+   a:bco:lpqrs:tx,
+   a:bco:lpqrs:tx
};
 
TAILQ_INIT(hl);
@@ -315,6 +315,15 @@
if (hftmp == TAILQ_END(hl))
hash_insert(hl, hf, 0);
break;
+   case 'l':
+   for (hf = functions; hf-name != NULL; hf++) {
+   len = strlen(hf-name);
+   for (i = 0; i  len; i++) {
+   putchar(tolower(hf-name[i]));
+   }   
+   putchar('\n');
+   }
+   exit(0);
case 'p':
pflag = 1;
break;
@@ -794,7 +803,7 @@
break;
case MODE_CKSUM:
case MODE_SUM:
-   fprintf(stderr, usage: %s [-bpqrtx] [-a algorithms] 
+   fprintf(stderr, usage: %s [-blpqrtx] [-a algorithms] 
[-c [checklist ...]] [-o 1 | 2]\n
   %*s [-s string] [file ...]\n,
__progname, (int)strlen(__progname), );

Re: cksum(1) patch

[Peter J. Philipp]

  And I seem to remember the diff was inspired by Solaris.
 
 $ uname -a
 SunOS foo 5.10 Generic_127128-11 i86pc i386 i86pc
 $ cksum -l
 cksum: illegal option -- l
 Usage: cksum [file ...]

It was inspired by digest(1) not cksum.

sycorax$ uname -a
SunOS sycorax 5.10 Generic_137138-09 i86pc i386 i86pc
sycorax$ digest -l
sha1
md5
sha256
sha384
sha512
sycorax$ 

Regards,

-peter

an XOR improvement of 1%

[Peter J. Philipp]

Hi,

I have made a patch against 5.7 that improves the speed of xor for amd64
by 1% (timed on a seperate userland program).   I tested the userland
program against an i386 and a amd64 host, didn't have access to any other
architectures.  

If a hardcore developer thinks this is worth it ... feel free to include
something similar to my patch.  The modes this affects is the CTR and the 
XTS AES modes, the latter being tested by me on my amd64 host with a encrypted
sparse file:

sd1 at scsibus3 targ 1 lun 0: OPENBSD, SR CRYPTO, 005 SCSI2 0/direct fixed
sd1: 1023MB, 512 bytes/sector, 2096561 sectors

It worked so the function must be working.  I have attached my patch for 
review inline.  It goes against /sys/crypto/xform.c

-peter

--- xform.c.origMon Jun  8 09:29:27 2015
+++ xform.c Mon Jun  8 09:34:14 2015
@@ -106,6 +106,8 @@
 u_int32_t deflate_decompress(u_int8_t *, u_int32_t, u_int8_t **);
 u_int32_t lzs_dummy(u_int8_t *, u_int32_t, u_int8_t **);
 
+void xorfunc(u_int8_t *, u_int8_t *, int);
+
 #define AESCTR_NONCESIZE   4
 #define AESCTR_IVSIZE  8
 #define AESCTR_BLOCKSIZE   16
@@ -499,8 +501,11 @@
if (++ctx-ac_block[i])   /* continue on overflow */
break;
rijndaelEncrypt(ctx-ac_ek, ctx-ac_nr, ctx-ac_block, keystream);
+#if 0
for (i = 0; i  AESCTR_BLOCKSIZE; i++)
data[i] ^= keystream[i];
+#endif
+   xorfunc(data, keystream, AESCTR_BLOCKSIZE);
explicit_bzero(keystream, sizeof(keystream));
 }
 
@@ -557,8 +562,11 @@
else
rijndael_decrypt(ctx-key1, block, data);
 
+#if 0
for (i = 0; i  AES_XTS_BLOCKSIZE; i++)
data[i] ^= ctx-tweak[i];
+#endif
+   xorfunc(data, ctx-tweak, AES_XTS_BLOCKSIZE);
 
/* Exponentiate tweak */
carry_in = 0;
@@ -676,4 +684,27 @@
 {
*out = NULL;
return (0);
+}
+
+void
+xorfunc(u_int8_t *output, u_int8_t *input, int len)
+{
+int i;
+#if __amd64__
+u_int8_t *i0, *i1, *i2, *i3;
+u_int8_t *o0, *o1, *o2, *o3;
+
+for (i = 0; i  len; i += 4) {
+i0 = (u_int8_t *)input[0 + i]; i1=(u_int8_t *)input[1 + i];
+i2 = (u_int8_t *)input[2 + i]; i3=(u_int8_t *)input[3 + i];
+o0 = (u_int8_t *)output[0 + i]; o1=(u_int8_t *)output[1 + i];
+o2 = (u_int8_t *)output[2 + i]; o3=(u_int8_t *)output[3 + i];
+
+*o0 ^= *i0; *o1 ^= *i1; *o2 ^= *i2; *o3 ^= *i3;
+}
+#else
+for (i = 0; i  len; i++) {
+output[i] ^= input[i];
+}
+#endif
 }

pledge idea

[Peter J. Philipp]

Hi deraadt,

I know you know I don't code well, but in order to show you what's on my 
mind I had to write code, I took the bsearch() from the ieee80211 code, so
perhaps there is a better way (like always) perhaps to unify the function 
between these two areas.

The reason I did this is to save on cpu cycles when you look at X amount
of processes all pledging...one time'd process won't show much difference.

below's diff:

-peter


? pledge.diff
Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.90
diff -u -p -u -r1.90 kern_pledge.c
--- kern/kern_pledge.c  28 Oct 2015 17:38:52 -  1.90
+++ kern/kern_pledge.c  29 Oct 2015 16:23:04 -
@@ -58,6 +58,32 @@
 
 int canonpath(const char *input, char *buf, size_t bufsize);
 int substrcmp(const char *p1, size_t s1, const char *p2, size_t s2);
+int pledge_cmp(const void *pi, const void *ph);
+
+/* taken from net80211/ieee80211_regdomain.c */
+const void *pledge_bsearch(const void *, const void *, size_t, size_t,
+int (*)(const void *, const void *));
+
+const void *
+pledge_bsearch(const void *key, const void *base0, size_t nmemb, size_t size,
+int (*compar)(const void *, const void *))
+{
+const char *base = base0;
+int lim, cmp;
+const void *p;
+
+for (lim = nmemb; lim != 0; lim >>= 1) {
+p = base + (lim >> 1) * size;
+cmp = (*compar)(key, p);
+if (cmp == 0)
+return ((const void *)p);
+if (cmp > 0) {  /* key > p: move right */
+base = (const char *)p + size;
+lim--;
+} /* else move left */
+}
+return (NULL);
+}
 
 const u_int pledge_syscalls[SYS_MAXSYSCALL] = {
[SYS_exit] = 0x,
@@ -256,40 +282,42 @@ const u_int pledge_syscalls[SYS_MAXSYSCA
[SYS_swapctl] = PLEDGE_VMINFO,  /* XXX should limit to "get" operations 
*/
 };
 
-static const struct {
+/* MUST be sorted! */
+static const struct PR {
char *name;
int flags;
 } pledgereq[] = {
-   { "stdio",  PLEDGE_STDIO },
-   { "rpath",  PLEDGE_RPATH },
-   { "wpath",  PLEDGE_WPATH },
-   { "tmppath",PLEDGE_TMPPATH },
-   { "inet",   PLEDGE_INET },
-   { "unix",   PLEDGE_UNIX },
+   { "abort",  0 },/* XXX reserve for later */
+   { "cpath",  PLEDGE_CPATH },
{ "dns",PLEDGE_DNS },
+   { "exec",   PLEDGE_EXEC },
+   { "fattr",  PLEDGE_FATTR },
+   { "flock",  PLEDGE_FLOCK },
{ "getpw",  PLEDGE_GETPW },
-   { "sendfd", PLEDGE_SENDFD },
-   { "recvfd", PLEDGE_RECVFD },
-   { "ioctl",  PLEDGE_IOCTL },
{ "id", PLEDGE_ID },
-   { "route",  PLEDGE_ROUTE },
+   { "inet",   PLEDGE_INET },
+   { "ioctl",  PLEDGE_IOCTL },
{ "mcast",  PLEDGE_MCAST },
-   { "tty",PLEDGE_TTY },
{ "proc",   PLEDGE_PROC },
-   { "exec",   PLEDGE_EXEC },
-   { "cpath",  PLEDGE_CPATH },
-   { "fattr",  PLEDGE_FATTR },
{ "prot_exec",  PLEDGE_PROTEXEC },
-   { "flock",  PLEDGE_FLOCK },
{ "ps", PLEDGE_PS },
-   { "vminfo", PLEDGE_VMINFO },
+   { "recvfd", PLEDGE_RECVFD },
+   { "route",  PLEDGE_ROUTE },
+   { "rpath",  PLEDGE_RPATH },
+   { "sendfd", PLEDGE_SENDFD },
{ "settime",PLEDGE_SETTIME },
-   { "abort",  0 },/* XXX reserve for later */
+   { "stdio",  PLEDGE_STDIO },
+   { "tmppath",PLEDGE_TMPPATH },
+   { "tty",PLEDGE_TTY },
+   { "unix",   PLEDGE_UNIX },
+   { "vminfo", PLEDGE_VMINFO },
+   { "wpath",  PLEDGE_WPATH },
 };
 
 int
 sys_pledge(struct proc *p, void *v, register_t *retval)
 {
+   struct PR *pr = NULL;
struct sys_pledge_args /* {
syscallarg(const char *)request;
syscallarg(const char **)paths;
@@ -300,7 +328,7 @@ sys_pledge(struct proc *p, void *v, regi
if (SCARG(uap, request)) {
size_t rbuflen;
char *rbuf, *rp, *pn;
-   int f, i;
+   int f;
 
rbuf = malloc(MAXPATHLEN, M_TEMP, M_WAITOK);
error = copyinstr(SCARG(uap, request), rbuf, MAXPATHLEN,
@@ -321,16 +349,15 @@ sys_pledge(struct proc *p, void *v, regi
*pn++ = '\0';
}
 
-   for (f = i = 0; i < 

Re: pledge idea

[Peter J. Philipp]

On 10/29/15 18:51, Reyk Floeter wrote:
> On Thu, Oct 29, 2015 at 04:32:25PM +, Peter J. Philipp wrote:
>> Hi deraadt,
>>
>> I know you know I don't code well, but in order to show you what's on my 
>> mind I had to write code, I took the bsearch() from the ieee80211 code, so
>> perhaps there is a better way (like always) perhaps to unify the function 
>> between these two areas.
>>
>> The reason I did this is to save on cpu cycles when you look at X amount
>> of processes all pledging...one time'd process won't show much difference.
>>
> I'm not deraadt, but -
>
> smart but have you considered that this is not worth the effort?
> pledge() is only called once or twice during a process' lifetime -
> start, pledge, run - the linear search on such a short list is still
> relatively fast, and the entries are even sorted in the order of
> relevance.  By convention "stdio" always comes first.  So what is the
> impact without your diff of pledge in src/bin/sleep/sleep.c:
>
> if (pledge("stdio", NULL) == -1)
> err(1, "pledge");

Hi Reyk,

deraadt already told me there was a patch for this already.  Yes it
would be more cycles for stdio I see that.

Thanks for your effort in making me see this.

-peter

> $ time obj/sleep 0.01 
> 0m00.01s real 0m00.00s user 0m00.01s system
>
> Are you trying to solve an issue?
>
> Reyk
>
>> below's diff:
>>
>> -peter
>>
>>
>> ? pledge.diff
>> Index: kern/kern_pledge.c
>> ===
>> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
>> retrieving revision 1.90
>> diff -u -p -u -r1.90 kern_pledge.c
>> --- kern/kern_pledge.c   28 Oct 2015 17:38:52 -  1.90
>> +++ kern/kern_pledge.c   29 Oct 2015 16:23:04 -
>> @@ -58,6 +58,32 @@
>>  
>>  int canonpath(const char *input, char *buf, size_t bufsize);
>>  int substrcmp(const char *p1, size_t s1, const char *p2, size_t s2);
>> +int pledge_cmp(const void *pi, const void *ph);
>> +
>> +/* taken from net80211/ieee80211_regdomain.c */
>> +const void *pledge_bsearch(const void *, const void *, size_t, size_t,
>> +int (*)(const void *, const void *));
>> +
>> +const void *
>> +pledge_bsearch(const void *key, const void *base0, size_t nmemb, size_t 
>> size,
>> +int (*compar)(const void *, const void *))
>> +{
>> +const char *base = base0;
>> +int lim, cmp;
>> +const void *p;
>> +
>> +for (lim = nmemb; lim != 0; lim >>= 1) {
>> +p = base + (lim >> 1) * size;
>> +cmp = (*compar)(key, p);
>> +if (cmp == 0)
>> +return ((const void *)p);
>> +if (cmp > 0) {  /* key > p: move right */
>> +base = (const char *)p + size;
>> +lim--;
>> +} /* else move left */
>> +}
>> +return (NULL);
>> +}
>>  
>>  const u_int pledge_syscalls[SYS_MAXSYSCALL] = {
>>  [SYS_exit] = 0x,
>> @@ -256,40 +282,42 @@ const u_int pledge_syscalls[SYS_MAXSYSCA
>>  [SYS_swapctl] = PLEDGE_VMINFO,  /* XXX should limit to "get" operations 
>> */
>>  };
>>  
>> -static const struct {
>> +/* MUST be sorted! */
>> +static const struct PR {
>>  char *name;
>>  int flags;
>>  } pledgereq[] = {
>> -{ "stdio",  PLEDGE_STDIO },
>> -{ "rpath",  PLEDGE_RPATH },
>> -{ "wpath",  PLEDGE_WPATH },
>> -{ "tmppath",PLEDGE_TMPPATH },
>> -{ "inet",   PLEDGE_INET },
>> -{ "unix",   PLEDGE_UNIX },
>> +{ "abort",  0 },/* XXX reserve for later */
>> +{ "cpath",  PLEDGE_CPATH },
>>  { "dns",PLEDGE_DNS },
>> +{ "exec",   PLEDGE_EXEC },
>> +{ "fattr",  PLEDGE_FATTR },
>> +{ "flock",  PLEDGE_FLOCK },
>>  { "getpw",  PLEDGE_GETPW },
>> -{ "sendfd", PLEDGE_SENDFD },
>> -{ "recvfd", PLEDGE_RECVFD },
>> -{ "ioctl",  PLEDGE_IOCTL },
>>  { "id", PLEDGE_ID },
>> -{ "route",  PLEDGE_ROUTE },
&

Re: pledge idea

[Peter J. Philipp]

On Thu, Oct 29, 2015 at 06:39:58PM +0100, Peter J. Philipp wrote:
> Hi Reyk,
> 
> deraadt already told me there was a patch for this already.  Yes it
> would be more cycles for stdio I see that.
> 
> Thanks for your effort in making me see this.
> 
> -peter
> 
> > $ time obj/sleep 0.01 
> > 0m00.01s real 0m00.00s user 0m00.01s system
> >
> > Are you trying to solve an issue?

Hi,

I just want to revisit this because I couldn't believe it.  I turned on
system accounting and rebooted my test box.  Here is what I found the following
programs were run this many times:

23 sh
10 ntpd
 9 smtpd
 7 domainname
 6 id
 6 getty
 6 getcap
 6 basename

I'm gonna stop here.  Some of these programs were not pledged yet in my sources
(-current from last week).  

So I did the tedious work of adding up the cycles of pledge/strcmp on sh binary
and then gave the bsearch idea a guessed average of 6 rounds per lookup.  What 
I came up with was that pledge/strcmp has 120 cycles where bsearch had 60 
cycles.  Multiplied by 23 times would give pledge/strcmp 2760 cycles and 
bsearch 1380 cycles.  /bin/sh is probably always going to be the most used in
any system, well it is at startup.

Another comparison was for getty:
pledge/strcmp 420 cycles
bsearch = 216 cycles

getcap had stdio and rpath which is 3 cycles, here it wins against bsearch
which has 12 cycles.

I do understand that most little programs such as basename have only stdio
in pledge and thus will save cycles but when you average it all out by 
occurences there might be a case for using bsearch?

I know it was just halloween but I'm still creeped out by this.

-peter

httpd patch

[Peter J. Philipp]

Hello,

I had nothing better to do tonight after work so I read a little in httpd.
I have come up with a patch for i386 and any architecture where off_t != 
size_t.

So on i386 there is this:

uranus$ ./sizetest
off_t = 8
size_t = 4

and I have these files in a directory:

uranus$ ls -lhi
total 12672
364207 -rw-r--r--  3 root  daemon   4.9G Jan 15 21:06 output.txt
364207 -rw-r--r--  3 root  daemon   4.9G Jan 15 21:06 output2.txt
364207 -rw-r--r--  3 root  daemon   4.9G Jan 15 21:06 output3.txt

A download (cancelled, but it doesn't matter) of the httpd without my patch
looks like so:

default 192.168.1.127 - - [15/Jan/2016:21:11:55 +0100] "GET /public/output2.txt 
HTTP/1.1" 200 948961280 "http://192.168.1.1/public/; "Mozilla/5.0 (X11; OpenBSD 
amd64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.157 
Safari/537.36"

A download (cancelled again) of the httpd with my patch looks like so:

default 192.168.1.127 - - [15/Jan/2016:21:18:07 +0100] "GET /public/output3.txt 
HTTP/1.1" 200 5243928576 "http://192.168.1.1/public/; "Mozilla/5.0 (X11; 
OpenBSD amd64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.157 
Safari/537.36"

It looks accurate in this case.  Checking again with ls:

uranus$ ls -li output3.txt  
364207 -rw-r--r--  3 root  daemon  5243928576 Jan 15 21:06 output3.txt

Absolutely.

patch follows:

Cheers,
-peter

? httpd.patch
Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.96
diff -u -p -u -r1.96 httpd.h
--- httpd.h 3 Aug 2015 11:45:17 -   1.96
+++ httpd.h 15 Jan 2016 20:19:12 -
@@ -602,7 +602,7 @@ const char *
 server_http_host(struct sockaddr_storage *, char *, size_t);
 char   *server_http_parsehost(char *, char *, size_t, int *);
 ssize_t server_http_time(time_t, char *, size_t);
-int server_log_http(struct client *, u_int, size_t);
+int server_log_http(struct client *, u_int, off_t);
 
 /* server_file.c */
 int server_file(struct httpd *, struct client *);
Index: server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.96
diff -u -p -u -r1.96 server_http.c
--- server_http.c   31 Jul 2015 00:10:51 -  1.96
+++ server_http.c   15 Jan 2016 20:19:12 -
@@ -1450,7 +1450,7 @@ server_httperror_cmp(const void *a, cons
 }
 
 int
-server_log_http(struct client *clt, u_int code, size_t len)
+server_log_http(struct client *clt, u_int code, off_t len)
 {
static char  tstamp[64];
static char  ip[INET6_ADDRSTRLEN];
@@ -1511,7 +1511,7 @@ server_log_http(struct client *clt, u_in
goto done;
 
ret = evbuffer_add_printf(clt->clt_log,
-   "%s %s - %s [%s] \"%s %s%s%s%s%s\" %03d %zu\n",
+   "%s %s - %s [%s] \"%s %s%s%s%s%s\" %03d %qu\n",
srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
user, tstamp,
server_httpmethod_byid(desc->http_method),
@@ -1559,7 +1559,7 @@ server_log_http(struct client *clt, u_in
 
ret = evbuffer_add_printf(clt->clt_log,
"%s %s - %s [%s] \"%s %s%s%s%s%s\""
-   " %03d %zu \"%s\" \"%s\"\n",
+   " %03d %qu \"%s\" \"%s\"\n",
srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
user, tstamp,
server_httpmethod_byid(desc->http_method),

Re: httpd patch

[Peter J. Philipp]

On Fri, Jan 15, 2016 at 08:36:05PM +, Peter J. Philipp wrote:
> Hello,
> 
> I had nothing better to do tonight after work so I read a little in httpd.
> I have come up with a patch for i386 and any architecture where off_t != 
> size_t.
> 
> So on i386 there is this:
> 
> uranus$ ./sizetest
> off_t = 8
> size_t = 4
> 
> and I have these files in a directory:
> 
> uranus$ ls -lhi
> total 12672
> 364207 -rw-r--r--  3 root  daemon   4.9G Jan 15 21:06 output.txt
> 364207 -rw-r--r--  3 root  daemon   4.9G Jan 15 21:06 output2.txt
> 364207 -rw-r--r--  3 root  daemon   4.9G Jan 15 21:06 output3.txt
> 
> A download (cancelled, but it doesn't matter) of the httpd without my patch
> looks like so:
> 
> default 192.168.1.127 - - [15/Jan/2016:21:11:55 +0100] "GET 
> /public/output2.txt HTTP/1.1" 200 948961280 "http://192.168.1.1/public/; 
> "Mozilla/5.0 (X11; OpenBSD amd64) AppleWebKit/537.36 (KHTML, like Gecko) 
> Chrome/44.0.2403.157 Safari/537.36"
> 
> A download (cancelled again) of the httpd with my patch looks like so:
> 
> default 192.168.1.127 - - [15/Jan/2016:21:18:07 +0100] "GET 
> /public/output3.txt HTTP/1.1" 200 5243928576 "http://192.168.1.1/public/; 
> "Mozilla/5.0 (X11; OpenBSD amd64) AppleWebKit/537.36 (KHTML, like Gecko) 
> Chrome/44.0.2403.157 Safari/537.36"
> 
> It looks accurate in this case.  Checking again with ls:
> 
> uranus$ ls -li output3.txt  
> 364207 -rw-r--r--  3 root  daemon  5243928576 Jan 15 21:06 output3.txt
> 
> Absolutely.
> 
> patch follows:
> 
> Cheers,
> -peter
> 
> ? httpd.patch
> Index: httpd.h
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.96
> diff -u -p -u -r1.96 httpd.h
> --- httpd.h   3 Aug 2015 11:45:17 -   1.96
> +++ httpd.h   15 Jan 2016 20:19:12 -
> @@ -602,7 +602,7 @@ const char *
>server_http_host(struct sockaddr_storage *, char *, size_t);
>  char *server_http_parsehost(char *, char *, size_t, int *);
>  ssize_t   server_http_time(time_t, char *, size_t);
> -int   server_log_http(struct client *, u_int, size_t);
> +int   server_log_http(struct client *, u_int, off_t);
>  
>  /* server_file.c */
>  int   server_file(struct httpd *, struct client *);
> Index: server_http.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.96
> diff -u -p -u -r1.96 server_http.c
> --- server_http.c 31 Jul 2015 00:10:51 -  1.96
> +++ server_http.c 15 Jan 2016 20:19:12 -
> @@ -1450,7 +1450,7 @@ server_httperror_cmp(const void *a, cons
>  }
>  
>  int
> -server_log_http(struct client *clt, u_int code, size_t len)
> +server_log_http(struct client *clt, u_int code, off_t len)
>  {
>   static char  tstamp[64];
>   static char  ip[INET6_ADDRSTRLEN];
> @@ -1511,7 +1511,7 @@ server_log_http(struct client *clt, u_in
>   goto done;
>  
>   ret = evbuffer_add_printf(clt->clt_log,
> - "%s %s - %s [%s] \"%s %s%s%s%s%s\" %03d %zu\n",
> + "%s %s - %s [%s] \"%s %s%s%s%s%s\" %03d %qu\n",
>   srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
>   user, tstamp,
>   server_httpmethod_byid(desc->http_method),
> @@ -1559,7 +1559,7 @@ server_log_http(struct client *clt, u_in
>  
>   ret = evbuffer_add_printf(clt->clt_log,
>   "%s %s - %s [%s] \"%s %s%s%s%s%s\""
> - " %03d %zu \"%s\" \"%s\"\n",
> + " %03d %qu \"%s\" \"%s\"\n",
>   srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
>   user, tstamp,
>   server_httpmethod_byid(desc->http_method),


Hello again,

I couldn't sleep because for some reason my head was spinning around this 
code.  In sleep I reviewed what I remembered of this code and noticed two
things.

1.  My patch was against 5.8 not -current, so it needed special hand fixing.

2.  The "Range" code required the same attention as my original attention.

I'm gonna need someone to look over my changes closely that I'm gonna put
forward next, because -current httpd doesn't compile on my system due to
some changes in SSL, and I don't want to fully go -current just yet on this
box (my only i386 box).  The following has a change in content_length from
size_t to off_t in function server_partial_file_

Re: httpd patch

[Peter J. Philipp]

On Sat, Jan 16, 2016 at 04:35:16AM +, Peter J. Philipp wrote:
> Hello again,
> 
> I couldn't sleep because for some reason my head was spinning around this 
> code.  In sleep I reviewed what I remembered of this code and noticed two
> things.
> 
> 1.  My patch was against 5.8 not -current, so it needed special hand fixing.
> 
> 2.  The "Range" code required the same attention as my original attention.
> 
> I'm gonna need someone to look over my changes closely that I'm gonna put
> forward next, because -current httpd doesn't compile on my system due to
> some changes in SSL, and I don't want to fully go -current just yet on this
> box (my only i386 box).  The following has a change in content_length from
> size_t to off_t in function server_partial_file_request() because it does 
> this:
> 
> content_length = range->end - range->start + 1;
> 
> and range->end and range->start are both off_t.
> 
> Here then the patches against -current (need review and testing):

Here is my third attempt.  Someone correctly told me that off_t is signed.
This makes it difficult to make it clean as it was before my attempts.  But
take a look at this code and see what I mean.  The below is untested.

I set content_length to 0 because it's better than having a negative value
fly around the code execution path.

-peter


? httpd.patch
Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.102
diff -u -p -u -r1.102 httpd.h
--- httpd.h 2 Dec 2015 15:13:00 -   1.102
+++ httpd.h 16 Jan 2016 04:58:16 -
@@ -597,7 +597,7 @@ const char *
 server_http_host(struct sockaddr_storage *, char *, size_t);
 char   *server_http_parsehost(char *, char *, size_t, int *);
 ssize_t server_http_time(time_t, char *, size_t);
-int server_log_http(struct client *, unsigned int, size_t);
+int server_log_http(struct client *, unsigned int, off_t);
 
 /* server_file.c */
 int server_file(struct httpd *, struct client *);
Index: server_file.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.60
diff -u -p -u -r1.60 server_file.c
--- server_file.c   3 Aug 2015 11:45:17 -   1.60
+++ server_file.c   16 Jan 2016 04:58:16 -
@@ -303,7 +303,7 @@ server_partial_file_request(struct httpd
struct media_type   *media, multipart_media;
struct range*range;
struct evbuffer *evb = NULL;
-   size_t   content_length;
+   off_tcontent_length;
int  code = 500, fd = -1, i, nranges, ret;
uint32_t boundary;
char content_range[64];
@@ -386,6 +386,9 @@ server_partial_file_request(struct httpd
"byteranges; boundary=%ud", boundary);
media = _media;
}
+
+   if (content_length < 0)
+   content_length = 0;
 
ret = server_response_http(clt, 206, media, content_length,
MINIMUM(time(NULL), st->st_mtim.tv_sec));
Index: server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.103
diff -u -p -u -r1.103 server_http.c
--- server_http.c   7 Dec 2015 20:30:17 -   1.103
+++ server_http.c   16 Jan 2016 04:58:17 -
@@ -1443,7 +1443,7 @@ server_httperror_cmp(const void *a, cons
 }
 
 int
-server_log_http(struct client *clt, unsigned int code, size_t len)
+server_log_http(struct client *clt, unsigned int code, off_t len)
 {
static char  tstamp[64];
static char  ip[INET6_ADDRSTRLEN];
@@ -1504,7 +1504,7 @@ server_log_http(struct client *clt, unsi
goto done;
 
ret = evbuffer_add_printf(clt->clt_log,
-   "%s %s - %s [%s] \"%s %s%s%s%s%s\" %03d %zu\n",
+   "%s %s - %s [%s] \"%s %s%s%s%s%s\" %03d %qd\n",
srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
user, tstamp,
server_httpmethod_byid(desc->http_method),
@@ -1552,7 +1552,7 @@ server_log_http(struct client *clt, unsi
 
ret = evbuffer_add_printf(clt->clt_log,
"%s %s - %s [%s] \"%s %s%s%s%s%s\""
-   " %03d %zu \"%s\" \"%s\"\n",
+   " %03d %qd \"%s\" \"%s\"\n",
srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
user, tstamp,
server_httpmethod_byid(desc->http_method),

Re: I have a program I wish to submit for the base

[Peter J. Philipp]

Luke, don't feel bad.  Very little code that is "offered" gets taken by
the OpenBSD project.  OpenBSD really only takes when they see benefit
for the project.  An example for that is openssh.  What you really want
to do is focus on your own projects and make them available somewhere so
that when OpenBSD gets wind of it they'll take it.

Cheers,

-peter

On 01/29/16 09:19, Nicholas Marriott wrote:
> Firstly, I don't think we need this in base and I think there is little
> to no chance of it being taken, even if the code is improved.
>
> Secondly:
>
> - The code is still miles off style(9) and isn't really a consistent
>   style within itself either.
>
> - Forking uname(1)? What? No offence, but that is hilarious :-). Why
>   fork uname(1) for uname(3) but not date(1) for gettimeofday(2)?
>
> - Why would you fork sed either?
>
> I think C is the wrong tool for this. Why not write a shell, perl, or
> python script?
>
> Then if people start to use it you could make a port.
>
>
>
> On Fri, Jan 29, 2016 at 01:34:30AM -0600, Luke Small wrote:
>> I think I fixed all your suggestions. I don't strictly adhere to kernel
>> normal in the use of comments and I parse command-line arguments without
>> using getopt(3), but the method is robust.
>>
>> -Luke
>>
>> >
>>  o I definitely don't think camel case will be accepted
>>
>>  o I'm pretty sure strtonum(3) is strongly preferred over strtod(3) et al.
>>

TSIG authentication in libasr

[Peter J. Philipp]

Hi,

I have a patch for TSIG authentication in libasr.  It is enabled by the "tsig"
keyword in /etc/resolv.conf.  My /etc/resolv.conf looks like this:

search centroid.eu
#nameserver 192.168.34.1
nameserver 200.46.208.61
tsig secret-key.:DONTTRY
lookup file bind

The HMAC over the TSIG is SHA256-HMAC by default but this can be expanded if
willed.

I have tested this by relinking ping and running chrome on websites that I
never visited before, it works.  If you need help setting this up on a BIND
nameserver so that it can recurse a TSIG authenticated lookup, let me know
I can share my example.

The code follows.  If there is an interest for this, I'll clean it up and 
write a manpage, I worked on getting this working as a start.


? asr.patch
Index: asr.c
===
RCS file: /cvs/src/lib/libc/asr/asr.c,v
retrieving revision 1.50
diff -u -p -u -r1.50 asr.c
--- asr.c   16 Dec 2015 16:32:30 -  1.50
+++ asr.c   27 Feb 2016 10:26:33 -
@@ -527,6 +527,7 @@ pass0(char **tok, int n, struct asr_ctx 
 {
int  i, j, d;
const char  *e;
+   char*p;
struct sockaddr_storage ss;
 
if (!strcmp(tok[0], "nameserver")) {
@@ -548,6 +549,32 @@ pass0(char **tok, int n, struct asr_ctx 
return;
ac->ac_domain = strdup(tok[1]);
 
+   } else if (!strcmp(tok[0], "tsig")) {
+   if (n != 2)
+   return;
+   if (ac->ac_use_tsig)
+   return;
+
+   p = strchr(tok[1], ':');
+   if (p == NULL)
+   return;
+
+   *p = '\0';
+   if (_asr_dname_from_fqdn(tok[1], ac->ac_tsig_key, 
sizeof(ac->ac_tsig_key)) == -1)
+   return;
+
+   /* RFC 4635 */
+   if (_asr_dname_from_fqdn("hmac-sha256.", ac->ac_dn_algorithm, 
sizeof(ac->ac_dn_algorithm)) == -1)
+   return;
+
+   p++;
+   ac->ac_use_tsig = 1;
+   
+   ac->ac_dn_algorithm_len = strlen(ac->ac_dn_algorithm);
+   ac->ac_algorithm_size = 32;
+
+   ac->ac_tsig_password = strdup(p);
+   
} else if (!strcmp(tok[0], "lookup")) {
/* ensure that each lookup is only given once */
for (i = 1; i < n; i++)
Index: asr_private.h
===
RCS file: /cvs/src/lib/libc/asr/asr_private.h,v
retrieving revision 1.38
diff -u -p -u -r1.38 asr_private.h
--- asr_private.h   16 Dec 2015 16:32:30 -  1.38
+++ asr_private.h   27 Feb 2016 10:26:33 -
@@ -135,7 +135,13 @@ struct asr_ctx {
int  ac_nstimeout;
int  ac_nsretries;
struct sockaddr *ac_ns[ASR_MAXNS];
-
+   /* pjp below */
+   int  ac_use_tsig;
+   char*ac_tsig_password;
+   char ac_tsig_key[MAXDNAME];
+   char ac_dn_algorithm[MAXDNAME];
+   int  ac_dn_algorithm_len;
+   int  ac_algorithm_size;
 };
 
 struct asr {
@@ -301,6 +307,7 @@ __BEGIN_HIDDEN_DECLS
 void _asr_pack_init(struct asr_pack *, char *, size_t);
 int _asr_pack_header(struct asr_pack *, const struct asr_dns_header *);
 int _asr_pack_query(struct asr_pack *, uint16_t, uint16_t, const char *);
+int _asr_tsig_query(struct asr_pack *, const struct asr_dns_header *, struct 
asr_ctx *, const char *, uint16_t);
 void _asr_unpack_init(struct asr_unpack *, const char *, size_t);
 int _asr_unpack_header(struct asr_unpack *, struct asr_dns_header *);
 int _asr_unpack_query(struct asr_unpack *, struct asr_dns_query *);
@@ -308,6 +315,7 @@ int _asr_unpack_rr(struct asr_unpack *, 
 int _asr_sockaddr_from_str(struct sockaddr *, int, const char *);
 ssize_t _asr_dname_from_fqdn(const char *, char *, size_t);
 ssize_t _asr_addr_as_fqdn(const char *, int, char *, size_t);
+void _asr_hmac_sha256(u_char *text, int text_len, u_char *key, int key_len, 
char *digest);
 
 /* asr.c */
 static void *_asr_resolver(void);
Index: asr_utils.c
===
RCS file: /cvs/src/lib/libc/asr/asr_utils.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 asr_utils.c
--- asr_utils.c 9 Sep 2015 15:49:34 -   1.13
+++ asr_utils.c 27 Feb 2016 10:26:34 -
@@ -32,6 +32,8 @@
 #include 
 #include 
 
+#include 
+
 #include "asr_private.h"
 
 static int dname_check_label(const char *, size_t);
@@ -373,6 +375,14 @@ pack_data(struct asr_pack *p, const void
 }
 
 static int
+pack_u32(struct asr_pack *p, uint32_t v)
+{
+   v = htonl(v);
+
+   return (pack_data(p, , 4));
+}
+
+static int
 pack_u16(struct asr_pack *p, uint16_t v)
 {
v = htons(v);
@@ -415,6 +425,89 @@ _asr_pack_query(struct asr_pack *p, uint
 }
 
 int
+_asr_tsig_query(struct asr_pack *p, const struct 

Re: asr: support for RES_USE_DNSSEC

[Peter J. Philipp]

Hi,

I'm not the best in reading patches, so I'm going to query you.  Does
your patch check for the "AD" flag from the resolver?  As basically a
DNSSEC able recursive nameserver should set this meaning it has
authenticated the data.  I wrote a patch for DNSSEC (possibly erroneous
by comparing it to you) and posted it to #opensmtpd in hopes that eric
would see it.  Much of that functionality is superfluous now but it does
have an "AD_MASK" check.

Here is my patch from last year, which I gave up on, feel free to cherry
pick anything needed out of it.  You'll see some similarities but they
are different enough to show two different peoples work.

http://centroid.eu/private/dnssec.patch.txt

Yours is a lot more complete of course.

Cheers,

-peter

On 02/25/17 19:24, Jeremie Courreges-Anglas wrote:
> Jeremie Courreges-Anglas  writes:
>
>> This flag is useful for software that wants to rely on the resolver to
>> perform DNSSEC validation.  Among the use cases there are DANE and SSHFP
>> records, and the obvious interfaces that I think are useful are
>> res_mkquery and getrrsetbyname.  The latter still doesn't support
>> DNSSEC, another diff will follow.
> Diff on top of the previous one.  I'd like to make getrrsetbyname(3)
> "use" RES_USE_DNSSEC again.  This would improve support for ssh -o
> VerifyHostKeyDNS (SSHFP records).
>
> The "easy" way would be something like:
>
> Index: getrrsetbyname_async.c
> ===
> RCS file: /d/cvs/src/lib/libc/asr/getrrsetbyname_async.c,v
> retrieving revision 1.11
> diff -u -p -p -u -r1.11 getrrsetbyname_async.c
> --- getrrsetbyname_async.c23 Feb 2017 17:04:02 -  1.11
> +++ getrrsetbyname_async.c25 Feb 2017 17:25:25 -
> @@ -42,6 +42,7 @@ getrrsetbyname_async(const char *hostnam
>   struct asr_query *as;
>  
>   ac = _asr_use_resolver(asr);
> + ac->ac_options |= RES_USE_DNSSEC;
>   if ((as = _asr_async_new(ac, ASR_GETRRSETBYNAME)) == NULL)
>   goto abort; /* errno set */
>   as->as_run = getrrsetbyname_async_run;
>
> IIUC this means that we modify the thread-local resolver options used by
> subsequent queries.  Cleaning up by resetting the flag before returning
> doesn't work in all cases because you could have overlap between two
> active getrrsetbyname_async and eg getaddrinfo_async contexts.
>
> The diff below instead adds an as_flags member to struct asr_query, and
> merges the flags of the various union members.  struct rrset and struct
> ni keep their flags member, as they are a different kind of flags.
> A subset of as_flags is passed down to the child ASR_SEND subq, the only
> flag that is inherited right now is ASYNC_DNSSEC, which allows
> getrrsetbyname_async to communicate its intent.
>
> That's a bit of churn for a small improvement, maybe there is a simpler
> diff?
>
> Comments welcome.
>
>
> diff -pruN asr.1/asr.c asr/asr.c
> --- asr.1/asr.c   Sat Feb 25 17:57:40 2017
> +++ asr/asr.c Sat Feb 25 17:58:10 2017
> @@ -244,7 +244,7 @@ _asr_async_free(struct asr_query *as)
>   case ASR_SEND:
>   if (as->as_fd != -1)
>   close(as->as_fd);
> - if (as->as.dns.obuf && !(as->as.dns.flags & ASYNC_EXTOBUF))
> + if (as->as.dns.obuf && !(as->as_flags & ASYNC_EXTOBUF))
>   free(as->as.dns.obuf);
>   if (as->as.dns.ibuf)
>   free(as->as.dns.ibuf);
> diff -pruN asr.1/asr_private.h asr/asr_private.h
> --- asr.1/asr_private.h   Sat Feb 25 17:57:40 2017
> +++ asr/asr_private.h Sat Feb 25 18:12:23 2017
> @@ -156,15 +156,19 @@ struct asr {
>  #define  ASYNC_NODATA0x0100
>  #define  ASYNC_AGAIN 0x0200
>  
> +#define  ASYNC_DNSSEC0x1000
>  #define  ASYNC_EXTOBUF   0x2000
>  
>  #define  ASYNC_NO_INET   0x0001
>  #define  ASYNC_NO_INET6  0x0002
>  
> +#define  ASYNC_ASR_SEND_MASK (ASYNC_DNSSEC)
> +
>  struct asr_query {
>   int (*as_run)(struct asr_query *, struct asr_result *);
>   struct asr_ctx  *as_ctx;
>   int  as_type;
> + int  as_flags;
>   int  as_state;
>  
>   /* cond */
> @@ -183,7 +187,6 @@ struct asr_query {
>  
>   union {
>   struct {
> - int  flags;
>   uint16_t reqid;
>   int  class;
>   int  type;
> @@ -206,7 +209,6 @@ struct asr_query {
>   } dns;
>  
>   struct {
> - int  flags;
>   int  class;
>   int  type;
>   char*name;
> @@ -249,7 +251,6 @@ struct asr_query {
>   char*fqdn;
>   struct addrinfo 

Re: asr: support for RES_USE_DNSSEC

[Peter J. Philipp]

On Mon, Feb 27, 2017 at 12:35:33AM +0100, Jeremie Courreges-Anglas wrote:
> Setting the AD flag for a query is possible, however those semantics are
> newer than the EDNS0 extension.  As far as I know, rfc6840 introduced
> AD=1 for queries in 2013, whereas rfc3225 specifies the DO flag since
> 2001.
> 
>   https://tools.ietf.org/html/rfc3225
>   https://tools.ietf.org/html/rfc6840#section-5.7
> 
> Also EDNS0 can give you more than 512 bytes on UDP (if the resolver
> supports it).  So I thought I'd rather implement RES_USE_DNSSEC on top
> of EDNS0.
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Jeremie & tech@,

Thanks for considering my patch.  OpenBSD tremendously improves with this 
work of yours, I'm all for it!  However to make use of this DNSSEC mode, 
the channel to the recursive DNS server has to be absolutely secure (for DO 
or AD in a response).  

My looming question that noone wants to ask because it's a bit (a lot)
of work for the programmer(s) is: can we work toward the goal of a validating
dnssec resolver?  (I know it's a lot of work, we'd need a group and an 
architect perhaps, ultimately the RFC's are the guideline)  Luckily I'm
between projects outside of my main job and I think I can contribute a little.

Is any (DNS) security programmers interested in this?  I can come to Paris
for EuroBSDCon to get together for this matter, but I'd want to get started
earlier if we form a small group for that matter.

Regards,
-peter

Re: asr: support for RES_USE_DNSSEC

[Peter J. Philipp]

On Mon, Feb 27, 2017 at 10:19:52AM +0100, Jeremie Courreges-Anglas wrote:
> > Thanks for considering my patch.  OpenBSD tremendously improves with this 
> > work of yours, I'm all for it!  However to make use of this DNSSEC mode, 
> > the channel to the recursive DNS server has to be absolutely secure (for DO 
> > or AD in a response).  
> 
> Yes, the assumption is that the resolver listed in /etc/resolv.conf is
> trusted, including the network in between.
> 
> The easiest method is to run unbound on 127.0.0.1 with
> "auto-trust-anchor-file".

I had a patch somewhere for TSIG as well somewhere, give me some time to
find it.  TSIG can secure the channel as well, but my implementation wasn't
all that pretty.

> > My looming question that noone wants to ask because it's a bit (a lot)
> > of work for the programmer(s) is: can we work toward the goal of a 
> > validating
> > dnssec resolver?
> 
> Please clarify: do you mean "stub resolver" here, ie the code that runs
> in libc?

Yes, that is what I mean.  That way the recursive dns server doesn't have to
be on the same box and answers are validated locally.

Regards,
-peter

Re: asr: support for RES_USE_DNSSEC

[Peter J. Philipp]

On Mon, Feb 27, 2017 at 10:26:48AM +0100, Peter J. Philipp wrote:
> I had a patch somewhere for TSIG as well somewhere, give me some time to
> find it.  TSIG can secure the channel as well, but my implementation wasn't
> all that pretty.

Here is the patch, it would need fixing up, and it only would work with BIND
nameservers currently :-(.

http://marc.info/?l=openbsd-tech=145656997013119=2

Regards,

-peter

Re: asr: support for RES_USE_DNSSEC

[Peter J. Philipp]

On Mon, Feb 27, 2017 at 11:14:13AM +0100, Jeremie Courreges-Anglas wrote:
> "Peter J. Philipp" <p...@centroid.eu> writes:
> 
> > On Mon, Feb 27, 2017 at 10:26:48AM +0100, Peter J. Philipp wrote:
> >> I had a patch somewhere for TSIG as well somewhere, give me some time to
> >> find it.  TSIG can secure the channel as well, but my implementation wasn't
> >> all that pretty.
> >
> > Here is the patch, it would need fixing up, and it only would work with BIND
> > nameservers currently :-(.
> >
> > http://marc.info/?l=openbsd-tech=145656997013119=2
> 
> Interesting.  I can't speak for others but I don't think that TSIG is
> a good solution.  Shared secrets don't buy us much, especially if we
> have to store them in /etc/resolv.conf.
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

You're right.  If I may point you to RFC 4033 section 7 (not a long read) it
talks about securing this channel for stub resolvers.  It actually talks about
SIG(0) or TSIG (like my example, which isn't the best use).  And it talks about
IPSEC to secure the channel (another good way) but who can say that they have
an IPSEC tunnel to their nameserver especially when it's the ISP's?  Really
all that we're left with (if we want security) is a local recursive nameserver.

I think making the stub resolver security aware and validating would be a good
thing but it's not a 1 person effort unless you can invest 2-3 years into it
and work for a living on the side.  I'm open to help (still) and I know I'm
talking to the right people here, yet a group should form and a mandate be set
in place for this to work best.

Cheers,
-peter

pf.conf.5 patch

[Peter J. Philipp]

Hi,

Please consider this patch for the pf.conf.5 manpage, it took me hours to
figure out what went wrong with my network after parts stopped working due
to this example.  Changing it to what I have now makes it work right.

Symptoms without this fix caused IPv6 neighbours to stop pinging/being 
available and even the NAT64 did not work anymore.

Thank you,

-peter


Index: pf.conf.5
===
RCS file: /cvs/src/share/man/man5/pf.conf.5,v
retrieving revision 1.552
diff -u -p -u -r1.552 pf.conf.5
--- pf.conf.5   14 May 2016 08:21:40 -  1.552
+++ pf.conf.5   24 Sep 2016 09:55:23 -
@@ -863,8 +863,8 @@ translated to 2001:db8::c633:6464.
 .Pp
 In the reverse case the following rules are identical:
 .Bd -literal -offset indent
-pass in inet6 af-to inet from 198.51.100.1 to 0.0.0.0/0
-pass in inet6 af-to inet from 198.51.100.1
+pass in inet6 from any to 64:ff9b::/96 af-to inet from 198.51.100.1 to 
0.0.0.0/0
+pass in inet6 from any to 64:ff9b::/96 af-to inet from 198.51.100.1
 .Ed
 .Pp
 The destination IPv4 address is assumed to be embedded inside the

pointer corruption in exec_script.c

[Peter J. Philipp]

Hi,

In my tinkering with the ELFSEC mechanism, I have noticed something
possibly troubling.  In /sys/kern/exec_script.c shellname is a pointer
to cp which is a pointer to hdrstr which is a pointer to epp->ep_hdr...
When calling the intended set shellname variable, later, I get part of the ELF
header of the program that the script executes.  This would be bogus IMO.
So I think what's needed here is a malloc, an strlcpy and a free later.

Here is a patch for review,

Regards,
-peter


Index: exec_script.c
===
RCS file: /cvs/src/sys/kern/exec_script.c,v
retrieving revision 1.40
diff -u -p -u -r1.40 exec_script.c
--- exec_script.c   11 Feb 2017 19:51:06 -  1.40
+++ exec_script.c   9 May 2017 19:44:46 -
@@ -184,7 +184,8 @@ check_shell:
 
/* set up the parameters for the recursive check_exec() call */
epp->ep_ndp->ni_dirfd = AT_FDCWD;
-   epp->ep_ndp->ni_dirp = shellname;
+   epp->ep_ndp->ni_dirp = malloc(shellnamelen + 1, M_EXEC, M_WAITOK);
+   strlcpy((char *)epp->ep_ndp->ni_dirp, shellname, shellnamelen + 1);
epp->ep_ndp->ni_segflg = UIO_SYSSPACE;
epp->ep_flags |= EXEC_INDIR;
 
@@ -271,6 +272,9 @@ fail:
}
free(shellargp, M_EXEC, 4 * sizeof(char *));
}
+
+   /* free epp->ep_ndp->ni_dirp */
+   free((char *)epp->ep_ndp->ni_dirp, M_EXEC, shellnamelen + 1);
 
/*
 * free any vmspace-creation commands,

Re: pointer corruption in exec_script.c

[Peter J. Philipp]

On Tue, May 09, 2017 at 10:05:28PM +0200, Peter J. Philipp wrote:
> Hi,
> 
> In my tinkering with the ELFSEC mechanism, I have noticed something
> possibly troubling.  In /sys/kern/exec_script.c shellname is a pointer
> to cp which is a pointer to hdrstr which is a pointer to epp->ep_hdr...
> When calling the intended set shellname variable, later, I get part of the ELF
> header of the program that the script executes.  This would be bogus IMO.
> So I think what's needed here is a malloc, an strlcpy and a free later.
> 
> Here is a patch for review,

Sorry the patch is bad because I fail to check what I want to free for NULL.
You probably caught it.  Better let someone with lots of experience handle
this.

-peter




> Regards,
> -peter
> 
> 
> Index: exec_script.c
> ===
> RCS file: /cvs/src/sys/kern/exec_script.c,v
> retrieving revision 1.40
> diff -u -p -u -r1.40 exec_script.c
> --- exec_script.c 11 Feb 2017 19:51:06 -  1.40
> +++ exec_script.c 9 May 2017 19:44:46 -
> @@ -184,7 +184,8 @@ check_shell:
>  
>   /* set up the parameters for the recursive check_exec() call */
>   epp->ep_ndp->ni_dirfd = AT_FDCWD;
> - epp->ep_ndp->ni_dirp = shellname;
> + epp->ep_ndp->ni_dirp = malloc(shellnamelen + 1, M_EXEC, M_WAITOK);
> + strlcpy((char *)epp->ep_ndp->ni_dirp, shellname, shellnamelen + 1);
>   epp->ep_ndp->ni_segflg = UIO_SYSSPACE;
>   epp->ep_flags |= EXEC_INDIR;
>  
> @@ -271,6 +272,9 @@ fail:
>   }
>   free(shellargp, M_EXEC, 4 * sizeof(char *));
>   }
> +
> + /* free epp->ep_ndp->ni_dirp */
> + free((char *)epp->ep_ndp->ni_dirp, M_EXEC, shellnamelen + 1);
>  
>   /*
>* free any vmspace-creation commands,

Re: pointer corruption in exec_script.c

[Peter J. Philipp]

Yeah, thanks... I don't know what I was drinking yesterday it was only
ice-tea, sorry for that noise.  In retrospect I unearthed another hole
in my own non-committed implementation and it will need to be rewritten
to work ever.

While I profited on that knowledge, you guys did not or only indirectly,
again sorry.

-peter


On 05/09/17 23:14, Ted Unangst wrote:
> Peter J. Philipp wrote:
>> In my tinkering with the ELFSEC mechanism, I have noticed something
>> possibly troubling.  In /sys/kern/exec_script.c shellname is a pointer
>> to cp which is a pointer to hdrstr which is a pointer to epp->ep_hdr...
>> When calling the intended set shellname variable, later, I get part of the 
>> ELF
>> header of the program that the script executes.  This would be bogus IMO.
> i don't think the pointer is supposed to last that long. the check_exec call
> does all sorts of things to epp.

My ELFSEC implementation (signed binaries for amd64)

[Peter J. Philipp]

2);
+   if (error != 0) {
+   printf("elfsec debug: elf_read_from %d\n", 
error);
+   VOP_CLOSE(esvp, FREAD, p->p_ucred, p);
+   free(ph2, M_TEMP, phsize2);
+   goto bad;
+   }
+
+   HMAC_SHA256_Update(, (u_int8_t *)ph2, phsize2);
+
+   memset(, 0, sizeof(es.digest));
+   HMAC_SHA256_Final(es.digest, );
+   free(ph2, M_TEMP, phsize2);
+
+   VOP_CLOSE(esvp, FREAD, p->p_ucred, p);
+
+   /* now compare the elfsechdr with the checksum */
+   eshdr = (ELFSEChdr *)pp;
+   if (memcmp(eshdr->hmac, es.digest, 
SHA256_DIGEST_LENGTH) != 0) {
+   printf("ELFSEC violation!  Not executing binary 
(%s) for uid %u\n", epp->ep_ndp->ni_dirp, p->p_ucred->cr_uid);
+   goto bad;
+   }
+   break;
+#endif /* __amd64__ */
+
default:
/*
 * Not fatal, we don't need to understand everything
@@ -710,6 +823,11 @@ exec_elf_makecmds(struct proc *p, struct
 */
break;
}
+   }
+
+   if (elfsecactive == 1 && visited_elfsec == 0 && suser(p, 0) != 0) {
+   printf("ELFSEC violation!  Not executing binary (%s) for uid 
%u\n", epp->ep_ndp->ni_dirp, p->p_ucred->cr_uid);
+   goto bad;
}
 
phdr += exe_base;
Index: sys/kern/exec_elfsec.c
===
RCS file: sys/kern/exec_elfsec.c
diff -N sys/kern/exec_elfsec.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ sys/kern/exec_elfsec.c  5 May 2017 09:34:47 -
@@ -0,0 +1,49 @@
+/* exec_elfsec.c - $Id$ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+
+
+int elfsecactive = 0;
+char elfseckey[32];
+
+int
+sys_elfsec(struct proc *p, void *v, register_t *retval)
+{
+   struct sys_elfsec_args /* {
+   syscallarg(char *) buf;
+   syscallarg(size_t) nbyte;
+   } */ *uap = v;
+
+   char *key;
+   size_t size;
+   int error;
+
+   if (securelevel > 0)
+   return (EPERM);
+
+   if ((error = suser(p, 0)) != 0)
+   return (error);
+
+key = SCARG(uap, buf);
+size = SCARG(uap, nbyte);
+
+   if (size != sizeof(elfseckey)) 
+   return (EINVAL);
+
+   if ((error = copyin(key, , sizeof(elfseckey))) != 0)
+   return (error);
+
+   elfsecactive = 1;
+   printf("elfsec active\n");
+
+   return (0);
+}
Index: sys/kern/init_sysent.c
===
RCS file: /var/cvsroot/src/src/sys/kern/init_sysent.c,v
retrieving revision 1.1.1.1
diff -u -p -u -r1.1.1.1 init_sysent.c
--- sys/kern/init_sysent.c  4 May 2017 19:58:28 -   1.1.1.1
+++ sys/kern/init_sysent.c  5 May 2017 09:34:41 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: init_sysent.c,v 1.186 2016/09/26 16:43:58 jca Exp $   */
+/* $OpenBSD$   */
 
 /*
  * System call switch table.
@@ -751,5 +751,7 @@ struct sysent sysent[] = {
sys___set_tcb },/* 329 = __set_tcb */
{ 0, 0, SY_NOLOCK | 0,
sys___get_tcb },/* 330 = __get_tcb */
+   { 2, s(struct sys_elfsec_args), 0,
+   sys_elfsec },   /* 331 = elfsec */
 };
 
Index: sys/kern/syscalls.c
===
RCS file: /var/cvsroot/src/src/sys/kern/syscalls.c,v
retrieving revision 1.1.1.1
diff -u -p -u -r1.1.1.1 syscalls.c
--- sys/kern/syscalls.c 4 May 2017 19:58:28 -   1.1.1.1
+++ sys/kern/syscalls.c 5 May 2017 09:34:34 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: syscalls.c,v 1.185 2016/09/26 16:43:58 jca Exp $  */
+/* $OpenBSD$   */
 
 /*
  * System call names.
@@ -393,4 +393,5 @@ char *syscallnames[] = {
"#328 (obsolete __tfork51)",/* 328 = obsolete __tfork51 */
"__set_tcb",/* 329 = __set_tcb */
"__get_tcb",/* 330 = __get_tcb */
+   "elfsec",   /* 331 = elfsec */
 };
Index: sys/kern/syscalls.master
===
RCS file: /var/cvsroot/src/src/sys/kern/syscalls.master,v
retrieving revision 1.1.1.1
diff -u -p -u -r1.1.1.1 syscalls.master
--- sys/kern/syscalls.master4 May 2017 19:58:28 -   1.1.1.1
+++ sys/kern/syscalls.master5 May 2017 09:34:28 -
@@ -563,3 +563,4 @@
 328OBSOL   __tfork51

Re: My ELFSEC implementation (signed binaries for amd64)

[Peter J. Philipp]

On Fri, May 05, 2017 at 05:25:57PM +0100, Kevin Chadwick wrote:
> > There was concern about my use of MD5 HMAC's so I 
> > took them out.  The ELF header of 32 bit systems is too small to fit
> > SHA256 checksums, so I'm leaving it out.
> 
> Have you considered CMAC which can be truncated if need be and also
> could take advantage of AES acceleration.
> 
> Alternatively, signify perhaps.

I never considered that.  In discussion with a friend, I did consider
truncating a SHA256 HMAC, but that didn't feel right.

If CMAC's can be truncated then this entire implementation can be rewritten
to not truncate for 64 bit machines and truncate for 32 bit machines.

The code to this should be straight forward and I'll work on that next I
guess.  I have a 32 bit firewall here that I'd love to ELFSEC.

I know too little about signify in-kernel, I know I love it as a userland
program.

Regards,
-peter

Re: My ELFSEC implementation (signed binaries for amd64)

[Peter J. Philipp]

On Fri, May 05, 2017 at 10:48:30PM +, Christian Weisgerber wrote:
> On 2017-05-05, "Peter J. Philipp" <p...@centroid.eu> wrote:
> 
> > This is my second official contribution to what I call ELFSEC, it places a 
> > signature in binaries, in the ELF header to be exact.
> -snip-
> 
> How does this defend against binary code introduced as a shared
> library by way of LD_LIBRARY_PATH or LD_PRELOAD?
> 
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de

Hi,

It doesn't check shared libraries, afaik.  If it did that then my test 
environment wouldn't work.  So this is a gaping hole.  I'll need some time 
to see where in the kernel shared libraries have their ELF header checked.  
Maybe the fix is trivial...

Regards,
-peter

save_errno for SHA256File()

[Peter J. Philipp]

Hi,

I have a program that constantly stalls on reading /etc/spwd.db with 
SHA256File() (from sha2.h).  Here is the program flow:

>
sha256file: Operation not permitted
on file: /etc/spwd.db
2f6574632f737077642e6462
^C
beta$ stat /etc/spwd.db
1024 78977 -rw-r- 1 root _shadow 327856 57344 "Oct 23 14:58:27 2017" "Oct 
17 13:54:38 2017" "Oct 17 13:54:38 2017" 16384 112 0 /etc/spwd.db
beta$ id
uid=0(root) gid=0(wheel) groups=0(wheel), 2(kmem), 3(sys), 4(tty), 5(operator), 
20(staff), 31(guest)
<

I don't know what's up but my research led me to create a patch for this 
function, it basically completes there what was started before because
close() can rewrite errno afaik.

If anyone has a hint as to why my SHA256File() returns with NULL and sets errno
to 0 that would really interest me.  My program does no setuid or seteuid at
all!  If you'd like to see the program, I can provide that but I wanted
to put preference to the patch here.

Patch (against 6.2) below signature.

-peter

Index: helper.c
===
RCS file: /cvs/src/lib/libc/hash/helper.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 helper.c
--- helper.c21 Sep 2016 04:38:57 -  1.16
+++ helper.c23 Oct 2017 13:06:46 -
@@ -71,13 +71,17 @@ HASHFileChunk(const char *filename, char
return (NULL);
if (len == 0) {
if (fstat(fd, ) == -1) {
+   save_errno = errno;
close(fd);
+   errno = save_errno;
return (NULL);
}
len = sb.st_size;
}
if (off > 0 && lseek(fd, off, SEEK_SET) < 0) {
+   save_errno = errno;
close(fd);
+   errno = save_errno;
return (NULL);
}
 

nice side-effect, but rebound doesn't play

[Peter J. Philipp]

Hi,

Yesterday I was messing with my network and particularily my workstation with
the goal of having an internal nameserver serve "internal.centroid.eu" zones
for my computers at home, and also do "168.192.in-addr.arpa" reverse.  I had
no luck diverting this from BIND, and then something unexpected happened to me.

I accidentally put the internal nameserver in the first nameserver entry of 
/etc/resolv.conf and it looked up the internal zones and replied "REFUSED"
on anything not configured on the nameserver.  The resolver in OpenBSD then
immediately switched to the second nameserver (not even a 1 ms latency) and
looked up the external address there.  Needless to say this does what I tried
to do with BIND.

Then came rebound into the picture and it doesn't do this, why?  Because
rebound only uses 1 nameserver entry in the config.  I did a quick hack around
rebound (please don't use it probably has memory problems) and determined that
it was able to do this with some code modification and if done in a style that
Ted uses it could fit in the code without sticking out too much.  So take my
hack as a proof-of-concept and it's just for show.

My ultimate question then is "why does the resolver do it this way?" was this
on purpose?  (probably a question for Eric).  I don't mind this and don't want
to instigate a "oh this is wrong" and then have someone change the behaviour...
Because it works for me without having to reconfigure the recursing nameserver.
And, I have a query log locally if I turn up logging on my authoritative name-
server, which can be good for analysis with scripts.

So the rebound patch is after my signature, it works but probably is not in
a style that openbsd wants in their code or that Ted would want in his code.
Take it as a proof of concept.  But... can we have something like that in 
rebound?  Otherwise rebound won't be used here.

-peter


Index: rebound.c
===
RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
retrieving revision 1.98
diff -u -p -u -r1.98 rebound.c
--- rebound.c   1 May 2018 15:14:43 -   1.98
+++ rebound.c   14 Jul 2018 05:03:07 -
@@ -52,6 +52,10 @@ union sockun {
struct sockaddr_in6 i6;
 };
 
+struct cfg {
+   union sockun r[6];
+};
+
 static struct timespec now;
 static int debug;
 static int daemonized;
@@ -340,8 +344,9 @@ servfail(int ud, uint16_t id, struct soc
 }
 
 static struct request *
-newrequest(int ud, struct sockaddr *remoteaddr)
+newrequest(int ud, struct cfg *cfg)
 {
+   struct sockaddr *remoteaddr = >r->a;
union sockun from;
socklen_t fromlen;
struct request *req;
@@ -369,6 +374,15 @@ newrequest(int ud, struct sockaddr *remo
 
conntotal += 1;
if (hit) {
+   if (hit->resp->flags & (0x8000 | 0x5)) { /* got a refused */
+   struct cfg *ncfg = cfg;
+   ncfg++;
+   if (ncfg->r->a.sa_family != AF_INET ||
+   ncfg->r->a.sa_family != AF_INET6)
+   return NULL;
+
+   return (newrequest(ud, ncfg));
+   }
hit->resp->id = dnsreq->id;
memcpy(hit->resp->qname, dnsreq->qname, namelen);
sendto(ud, hit->resp, hit->resplen, 0, , fromlen);
@@ -560,8 +574,9 @@ fail:
 }
 
 static struct request *
-newtcprequest(int ld, struct sockaddr *remoteaddr)
+newtcprequest(int ld, struct cfg *cfg)
 {
+   struct sockaddr *remoteaddr = >r->a;
struct request *req;
int client;
 
@@ -719,16 +734,17 @@ preloadPTR(const char *ip, const char *n
 }
 
 static int
-readconfig(int conffd, union sockun *remoteaddr)
+readconfig(int conffd, struct cfg *cfg)
 {
const char ns[] = "nameserver";
const char rc[] = "record";
char buf[1024];
char *p;
-   struct sockaddr_in *sin = >i;
-   struct sockaddr_in6 *sin6 = >i6;
+   struct sockaddr_in *sin = [0].r->i;
+   struct sockaddr_in6 *sin6 = [0].r->i6;
FILE *conf;
int rv = -1;
+   int num_ns = 0;
 
conf = fdopen(conffd, "r");
 
@@ -744,7 +760,10 @@ readconfig(int conffd, union sockun *rem
if (strcmp(p, "127.0.0.1") == 0)
continue;
 
-   memset(remoteaddr, 0, sizeof(*remoteaddr));
+   if (num_ns >= 5)
+   continue;
+
+   memset([num_ns].r, 0, sizeof(union sockun));
if (inet_pton(AF_INET, p, >sin_addr) == 1) {
sin->sin_len = sizeof(*sin);
sin->sin_family = AF_INET;
@@ -756,6 +775,9 @@ readconfig(int conffd, union sockun *rem
sin6->sin6_port = htons(53);
rv = AF_INET6;
}
+   num_ns++;
+   

Re: nice side-effect, but rebound doesn't play

[Peter J. Philipp]

oops I just realised I never activated rebound by putting a nameserver entry
for 127.0.0.1 at the top of the nameservers list.  This makes my patch broken
then since I tested it and it could only find the lookup once and then refused
any more queries after that.  Sorry about that!  However in theory my request
is right and it would be cool to have multiple nameserver entries that it
reads from the resolv.conf and then tries.  (Oh noI'm a feature creep!)

Apologies,
-peter

On Sat, Jul 14, 2018 at 07:24:09AM +0200, Peter J. Philipp wrote:
> Hi,
> 
> Yesterday I was messing with my network and particularily my workstation with
> the goal of having an internal nameserver serve "internal.centroid.eu" zones
> for my computers at home, and also do "168.192.in-addr.arpa" reverse.  I had
> no luck diverting this from BIND, and then something unexpected happened to 
> me.
> 
> I accidentally put the internal nameserver in the first nameserver entry of 
> /etc/resolv.conf and it looked up the internal zones and replied "REFUSED"
> on anything not configured on the nameserver.  The resolver in OpenBSD then
> immediately switched to the second nameserver (not even a 1 ms latency) and
> looked up the external address there.  Needless to say this does what I tried
> to do with BIND.
> 
> Then came rebound into the picture and it doesn't do this, why?  Because
> rebound only uses 1 nameserver entry in the config.  I did a quick hack around
> rebound (please don't use it probably has memory problems) and determined that
> it was able to do this with some code modification and if done in a style that
> Ted uses it could fit in the code without sticking out too much.  So take my
> hack as a proof-of-concept and it's just for show.
> 
> My ultimate question then is "why does the resolver do it this way?" was this
> on purpose?  (probably a question for Eric).  I don't mind this and don't want
> to instigate a "oh this is wrong" and then have someone change the 
> behaviour...
> Because it works for me without having to reconfigure the recursing 
> nameserver.
> And, I have a query log locally if I turn up logging on my authoritative name-
> server, which can be good for analysis with scripts.
> 
> So the rebound patch is after my signature, it works but probably is not in
> a style that openbsd wants in their code or that Ted would want in his code.
> Take it as a proof of concept.  But... can we have something like that in 
> rebound?  Otherwise rebound won't be used here.
> 
> -peter
> 
> 
> Index: rebound.c
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
> retrieving revision 1.98
> diff -u -p -u -r1.98 rebound.c
> --- rebound.c 1 May 2018 15:14:43 -   1.98
> +++ rebound.c 14 Jul 2018 05:03:07 -
> @@ -52,6 +52,10 @@ union sockun {
>   struct sockaddr_in6 i6;
>  };
>  
> +struct cfg {
> + union sockun r[6];
> +};
> +
>  static struct timespec now;
>  static int debug;
>  static int daemonized;
> @@ -340,8 +344,9 @@ servfail(int ud, uint16_t id, struct soc
>  }
>  
>  static struct request *
> -newrequest(int ud, struct sockaddr *remoteaddr)
> +newrequest(int ud, struct cfg *cfg)
>  {
> + struct sockaddr *remoteaddr = >r->a;
>   union sockun from;
>   socklen_t fromlen;
>   struct request *req;
> @@ -369,6 +374,15 @@ newrequest(int ud, struct sockaddr *remo
>  
>   conntotal += 1;
>   if (hit) {
> + if (hit->resp->flags & (0x8000 | 0x5)) { /* got a refused */
> + struct cfg *ncfg = cfg;
> + ncfg++;
> + if (ncfg->r->a.sa_family != AF_INET ||
> + ncfg->r->a.sa_family != AF_INET6)
> + return NULL;
> +
> + return (newrequest(ud, ncfg));
> + }
>   hit->resp->id = dnsreq->id;
>   memcpy(hit->resp->qname, dnsreq->qname, namelen);
>   sendto(ud, hit->resp, hit->resplen, 0, , fromlen);
> @@ -560,8 +574,9 @@ fail:
>  }
>  
>  static struct request *
> -newtcprequest(int ld, struct sockaddr *remoteaddr)
> +newtcprequest(int ld, struct cfg *cfg)
>  {
> + struct sockaddr *remoteaddr = >r->a;
>   struct request *req;
>   int client;
>  
> @@ -719,16 +734,17 @@ preloadPTR(const char *ip, const char *n
>  }
>  
>  static int
> -readconfig(int conffd, union sockun *remoteaddr)
> +readconfig(int conffd, struct cfg *cfg)
>  {
>   const char ns[] = "nameserver";
>   const char rc[] = 

define rebound magic numbers

[Peter J. Philipp]

Hi,

While reading through rebound, I noticed the author uses a lot of magic numbers
in DNS flags field.  I present OpenBSD a set of #defines that I wrote in 2002
on an OpenBSD/macppc iBook in Montreal.  If I didn't write all of it then, I
followed up with it in 2005 when my own DNS server came into fruition.  The
defines can also be gotten from here and are under a BSD license:

http://centroid.eu/cgi-bin/cvsweb/~checkout~/delphinusdns/delphinusdnsd/ddd-dns.h?rev=1.6=text/plain

patch which defines magic numbers in rebound follows after my sig.

I won't cry if you don't like it.

Regards,
-peter


Index: rebound.c
===
RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
retrieving revision 1.98
diff -u -p -u -r1.98 rebound.c
--- rebound.c   1 May 2018 15:14:43 -   1.98
+++ rebound.c   13 Jul 2018 13:33:28 -
@@ -43,6 +43,29 @@
 
 #define MINIMUM(a,b) (((a)<(b))?(a):(b))
 
+/* 
+ * flags RFC 1035, page 26
+ */
+
+#define DNS_REPLY   0x8000  /* if set response if not set query */
+#define DNS_NOTIFY  0x2000  /* a NOTIFY query RFC 1996 */
+#define DNS_SREQ0x1000  /* if set a server status request (STATUS) */
+#define DNS_INV 0x800   /* if set an inverse query */
+#define DNS_AUTH0x400   /* Authoritative Answer (AA) in replies */
+#define DNS_TRUNC   0x200   /* Truncated (TC) */
+#define DNS_RECURSE 0x100   /* if set Recursion Desired (RD) */
+#define DNS_RECAVAIL0x80/* if set Recursion Available (RA) */
+#define DNS_BADTIME 0x12/* RCODE (18) BADTIME RFC 2845 p. 3 */
+#define DNS_BADKEY  0x11/* RCODE (17) BADKEY RFC 2845 p. 3 */
+#define DNS_BADSIG  0x10/* RCODE (16) BADSIG RFC 2845 p. 3 */
+#define DNS_BADVERS 0x10/* RCODE (16) BADVERS RFC 2671 p. 6 */
+#define DNS_REFUSED 0x5 /* RCODE - Refused */
+#define DNS_NOTIMPL 0x4 /* RCODE - Not Implemented */
+#define DNS_NAMEERR 0x3 /* RCODE - Name Error, NXDOMAIN */
+#define DNS_SERVFAIL0x2 /* RCODE - Server Failure */
+#define DNS_FORMATERR   0x1 /* RCODE - Format Error */
+#define DNS_NOERR   0x0 /* RCODE - No error */
+
 uint16_t randomid(void);
 
 union sockun {
@@ -335,7 +358,7 @@ servfail(int ud, uint16_t id, struct soc
 
memset(, 0, sizeof(pkt));
pkt.id = id;
-   pkt.flags = htons(1 << 15 | 0x2);
+   pkt.flags = htons(DNS_REPLY | DNS_SERVFAIL);
sendto(ud, , sizeof(pkt), 0, fromaddr, fromlen);
 }
 
@@ -645,7 +668,7 @@ preloadcache(const char *name, uint16_t 
req = malloc(reqlen);
 
req->id = 0;
-   req->flags = htons(0x100);
+   req->flags = htons(DNS_RECURSE);
req->qdcount = htons(1);
req->ancount = 0;
req->nscount = 0;
@@ -662,7 +685,7 @@ preloadcache(const char *name, uint16_t 
resplen = reqlen + 2 + 2 + 2 + 4 + 2 + rdatalen;
resp = malloc(resplen);
memcpy(resp, req, reqlen);
-   resp->flags = htons(0x100 | 0x8000);/* response */
+   resp->flags = htons(DNS_RECURSE | DNS_REPLY);   /* response */
resp->ancount = htons(1);
p = (char *)resp + reqlen;
len = htons(sizeof(*req));

httpd/logger.c patch

[Peter J. Philipp]

Hi,

While auditing something in and around /usr/src/usr.sbin/httpd/logger.c 
(didn't find what I was looking for), I noticed that logger_log() was 
returning with an int but the return value was not processed at all.  Here 
is a small patch that makes the return value void.  I tested this patch
with compilation and running it.

Regards,
-peter


Index: logger.c
===
RCS file: /cvs/src/usr.sbin/httpd/logger.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 logger.c
--- logger.c7 Feb 2018 03:28:05 -   1.21
+++ logger.c11 Mar 2018 21:38:38 -
@@ -41,7 +41,7 @@ intlogger_open_fd(struct imsg *);
 int logger_open(struct server *, struct server_config *, void *);
 voidlogger_init(struct privsep *, struct privsep_proc *p, void *);
 int logger_start(void);
-int logger_log(struct imsg *);
+voidlogger_log(struct imsg *);
 
 static uint32_t last_log_id = 0;
 
@@ -236,7 +236,7 @@ logger_start(void)
return (0);
 }
 
-int
+void
 logger_log(struct imsg *imsg)
 {
char*logline;
@@ -257,7 +257,7 @@ logger_log(struct imsg *imsg)
 
if (log == NULL || log->log_fd == -1) {
log_warnx("log file %s not opened", log ? log->log_name : "");
-   return (0);
+   return;
}
 
/* XXX get_string() would sanitize the string, but add a malloc */
@@ -268,10 +268,10 @@ logger_log(struct imsg *imsg)
 
if (dprintf(log->log_fd, "%s\n", logline) == -1) {
if (logger_start() == -1)
-   return (-1);
+   return;
}
 
-   return (0);
+   return;
 }
 
 int

this fixes gif(4) on 6.3

[Peter J. Philipp]

Hello,

Yesterday I wrote to misc@ with this:

https://marc.info/?l=openbsd-misc=152302592426018=2

I apologize with the inline paste, thunderbird is just not good enough for
this stuff.

Anyhow I have produced this patch after upgrading the 6.2 box to 6.3.  It all
works now:

Here is my config:

gif1: flags=8051 mtu 1280
index 12 priority 0 llprio 3
groups: gif
tunnel: inet6 fd43:5602:29bd:16:0:dead:beef:1 -> 
fd43:5602:29bd:16:0:dead:beef:16 ttl 64 nodf rdomain 2
inet 172.16.16.10 --> 172.16.16.16 netmask 0xff00
inet6 fe80::290:bff:fe19:5604%gif1 ->  prefixlen 64 scopeid 0xc

uranus$ ping6 fe80::baae:edff:fe73:a76c%gif1  
PING fe80::baae:edff:fe73:a76c%gif1 (fe80::baae:edff:fe73:a76c%gif1): 56 data 
bytes
64 bytes from fe80::baae:edff:fe73:a76c%gif1: icmp_seq=0 hlim=64 time=8.767 ms
64 bytes from fe80::baae:edff:fe73:a76c%gif1: icmp_seq=1 hlim=64 time=9.854 ms
^C
--- fe80::baae:edff:fe73:a76c%gif1 ping statistics ---
2 packets transmitted, 2 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 8.767/9.311/9.854/0.543 ms
uranus$ ping 172.16.16.16
PING 172.16.16.16 (172.16.16.16): 56 data bytes
64 bytes from 172.16.16.16: icmp_seq=0 ttl=255 time=10.329 ms
64 bytes from 172.16.16.16: icmp_seq=1 ttl=255 time=12.994 ms
^C
--- 172.16.16.16 ping statistics ---
2 packets transmitted, 2 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 10.329/11.662/12.994/1.332 ms

The patch follows:


Index: if_gif.c
===
RCS file: /cvs/src/sys/net/if_gif.c,v
retrieving revision 1.113
diff -u -p -u -r1.113 if_gif.c
--- if_gif.c15 Mar 2018 21:01:18 -  1.113
+++ if_gif.c7 Apr 2018 07:59:54 -
@@ -338,7 +338,7 @@ gif_send(struct gif_softc *sc, struct mb
ip6->ip6_flow = htonl(flow);
ip6->ip6_vfc |= IPV6_VERSION;
ip6->ip6_plen = htons(len);
-   ip6->ip6_nxt = IPPROTO_GRE;
+   ip6->ip6_nxt = proto;
ip6->ip6_hlim = ttl;
ip6->ip6_src = sc->sc_tunnel.t_src6;
ip6->ip6_dst = sc->sc_tunnel.t_dst6;


Best regards,
-peter

fstat -r flag to display rdomains on sockets

[Peter J. Philipp]

Hi,

I've been running iked for a while now and have been able to guess which iked
belongs to which rdomain by the cpu counter but as I'm using the other iked
more the cpu counter is about the same and it's confusing when I have to 
restart iked with route exec.

I introduce the -r flag to fstat in order to display rdomains on sockets in
order to find the right iked.  The fix is simple but because some peoples
scripts may depend on the old output I made it an extra flag.

Here is how it looks like with and without the -r flag:

uranus$ fstat -rp 84169
USER CMD  PID   FD MOUNTINUM  MODE R/WSZ|DV
_ikediked   84169 text /  105245  -r-xr-xr-x r   321760
_ikediked   84169   wd /var   129920  drwxr-xr-x r  512
_ikediked   84169 root /var   129920  drwxr-xr-x r  512
_ikediked   841690 /  105143  crw-rw-rw-rw null
_ikediked   841691 /  105143  crw-rw-rw-rw null
_ikediked   841692 /  105143  crw-rw-rw-rw null
_ikediked   841693* 30 raw 2 0xd2f8f844  
_ikediked   841694* rdomain 2 internet dgram udp *:500   
_ikediked   841695* rdomain 2 internet dgram udp *:4500
_ikediked   841696* rdomain 2 internet6 dgram udp *:500
_ikediked   841697* rdomain 2 internet6 dgram udp *:4500
_ikediked   841698* unix stream 0xd3eb4180 <-> 0xd3eb4100
_ikediked   84169   10* unix stream 0xd3eb4280 <-> 0xd3eb4200
_ikediked   84169   11 kqueue 0xd32a8ad4 0 state: W

uranus$ fstat -p 47863
USER CMD  PID   FD MOUNTINUM  MODE R/WSZ|DV
_ikediked   47863 text /  105245  -r-xr-xr-x r   321760
_ikediked   47863   wd /var   129920  drwxr-xr-x r  512
_ikediked   47863 root /var   129920  drwxr-xr-x r  512
_ikediked   478630 /  105143  crw-rw-rw-rw null
_ikediked   478631 /  105143  crw-rw-rw-rw null
_ikediked   478632 /  105143  crw-rw-rw-rw null
_ikediked   478633* 30 raw 2 0xd3258e88
_ikediked   478634* internet dgram udp *:500   
_ikediked   478635* internet dgram udp *:4500  
_ikediked   478636* internet6 dgram udp *:500  
_ikediked   478637* internet6 dgram udp *:4500 
_ikediked   478638* unix stream 0xd3e67c00 <-> 0xd3e67b80  
_ikediked   47863   10* unix stream 0xd3e67d00 <-> 0xd3e67c80  
_ikediked   47863   11 kqueue 0xd32a8e7c 0 state: W

patch below signature.

Regards,
-peter

Index: fstat.1
===
RCS file: /cvs/src/usr.bin/fstat/fstat.1,v
retrieving revision 1.56
diff -u -p -u -r1.56 fstat.1
--- fstat.1 16 Mar 2018 16:58:26 -  1.56
+++ fstat.1 7 Apr 2018 10:27:52 -
@@ -37,7 +37,7 @@
 .Nd display status of open files
 .Sh SYNOPSIS
 .Nm fstat
-.Op Fl fnosv
+.Op Fl fnorsv
 .Op Fl M Ar core
 .Op Fl N Ar system
 .Op Fl p Ar pid
@@ -86,6 +86,8 @@ Useful for checking progress as a proces
 This information is only visible to the user or superuser.
 .It Fl p Ar pid
 Report all files open by the specified process.
+.It Fl r
+Report which rdomain a socket belongs to.
 .It Fl s
 Report per file io statistics in two additional columns
 .Sq XFERS
Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.92
diff -u -p -u -r1.92 fstat.c
--- fstat.c 6 Apr 2018 14:05:06 -   1.92
+++ fstat.c 7 Apr 2018 10:27:52 -
@@ -98,6 +98,7 @@ int   oflg;   /* display file offset */
 intsflg;   /* display file xfer/bytes counters */
 intvflg;   /* display errors in locating kernel data objects etc... */
 intcflg;   /* fuser only */
+intrflg;   /* display rdomains */
 
 intfuser;  /* 1 if we are fuser, 0 if we are fstat */
 intsigno;  /* signal to send (fuser only) */
@@ -148,7 +149,7 @@ main(int argc, char *argv[])
arg = -1;
what = KERN_FILE_BYPID;
nlistf = memf = NULL;
-   oflg = 0;
+   rflg = oflg = 0;
 
/* are we fstat(1) or fuser(1)? */
if (strcmp(__progname, "fuser") == 0) {
@@ -156,7 +157,7 @@ main(int argc, char *argv[])
optstr = "cfks:uM:N:";
} else {
fuser = 0;
-   optstr = "fnop:su:vN:M:";
+   optstr = "fnop:rsu:vN:M:";
}
 
/*
@@ -203,6 +204,9 @@ main(int argc, char *argv[])
}
what = 

Re: return packets may not be desired to be scrubbed

[Peter J. Philipp]

On Thu, Mar 29, 2018 at 10:01:02PM +0200, Peter J. Philipp wrote:
...
> The end result is here.  I add 2 arguments to pf_scrub() for rule/state
> direction that is desired and direction that the packet is taking.  Then
> in random-id the logic does not scrub when we had an "outbound scrub" and
> the packets direction is inbound.
> 
> Happy Easter!  May your pf get a little faster!
> 
> -peter

I'd like to retract this patch.  Sorry, it doesn't do what I expected, I
didn't test it well enough, and did just now and it doesn't do what I had
imagined.  I'll re-visit this some time later.  Happy Easter, again!

Regards,
-peter

return packets may not be desired to be scrubbed

[Peter J. Philipp]

Hi,

While writing my own patches to the OpenBSD kernel and the pf subsystem, I
noticed that random-id packets scrub twice.  I noticed this by copying 
random-id's code and modifying it a little.  From that grew a little patch
for scrub and random-id and I'd like OpenBSD to consider it.  I sent a
mail to misc@ before asking "why scrub twice?" and didn't find an answer.
I did the work for myself and put it up for you in a small HTML file on
my website.  It is here:

http://centroid.eu/private/steg-patch.html

The end result is here.  I add 2 arguments to pf_scrub() for rule/state
direction that is desired and direction that the packet is taking.  Then
in random-id the logic does not scrub when we had an "outbound scrub" and
the packets direction is inbound.

Happy Easter!  May your pf get a little faster!

-peter


Index: sys/net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1063
diff -u -p -u -r1.1063 pf.c
--- sys/net/pf.c6 Mar 2018 17:35:53 -   1.1063
+++ sys/net/pf.c29 Mar 2018 19:44:28 -
@@ -7018,7 +7018,7 @@ done:
}
 
pf_scrub(pd.m, s->state_flags, pd.af, s->min_ttl,
-   s->set_tos);
+   s->set_tos, s->direction, dir);
pf_tag_packet(pd.m, s->tag, s->rtableid[pd.didx]);
if (pqid || (pd.tos & IPTOS_LOWDELAY)) {
qid = s->pqid;
@@ -7031,7 +7031,7 @@ done:
}
} else {
pf_scrub(pd.m, r->scrub_flags, pd.af, r->min_ttl,
-   r->set_tos);
+   r->set_tos, r->direction, dir);
if (pqid || (pd.tos & IPTOS_LOWDELAY)) {
qid = r->pqid;
if (r->scrub_flags & PFSTATE_SETPRIO)
Index: sys/net/pf_norm.c
===
RCS file: /cvs/src/sys/net/pf_norm.c,v
retrieving revision 1.209
diff -u -p -u -r1.209 pf_norm.c
--- sys/net/pf_norm.c   6 Feb 2018 09:16:11 -   1.209
+++ sys/net/pf_norm.c   29 Mar 2018 19:44:28 -
@@ -1540,7 +1540,7 @@ pf_normalize_mss(struct pf_pdesc *pd, u_
 
 void
 pf_scrub(struct mbuf *m, u_int16_t flags, sa_family_t af, u_int8_t min_ttl,
-u_int8_t tos)
+u_int8_t tos, u_int8_t ruledir, u_int8_t dir)
 {
struct ip   *h = mtod(m, struct ip *);
 #ifdef INET6
@@ -1574,6 +1574,7 @@ pf_scrub(struct mbuf *m, u_int16_t flags
 
/* random-id, but not for fragments */
if (flags & PFSTATE_RANDOMID && af == AF_INET &&
-   !(h->ip_off & ~htons(IP_DF)))
+   !(h->ip_off & ~htons(IP_DF)) &&
+   (ruledir == PF_INOUT || ruledir == PF_FWD || ruledir == dir))
h->ip_id = htons(ip_randomid());
 }
Index: sys/net/pfvar.h
===
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.476
diff -u -p -u -r1.476 pfvar.h
--- sys/net/pfvar.h 9 Feb 2018 09:35:03 -   1.476
+++ sys/net/pfvar.h 29 Mar 2018 19:44:28 -
@@ -1764,7 +1764,7 @@ int   pf_normalize_tcp_stateful(struct pf_
struct pf_state *, struct pf_state_peer *, struct pf_state_peer *,
int *);
 intpf_normalize_mss(struct pf_pdesc *, u_int16_t);
-void   pf_scrub(struct mbuf *, u_int16_t, sa_family_t, u_int8_t, u_int8_t);
+void   pf_scrub(struct mbuf *, u_int16_t, sa_family_t, u_int8_t, u_int8_t, 
u_int8_t, u_int8_t);
 int32_tpf_state_expires(const struct pf_state *);
 void   pf_purge_expired_fragments(void);
 intpf_routable(struct pf_addr *addr, sa_family_t af, struct pfi_kif *,

if_pppoe.c patch

[Peter J. Philipp]

I have "covered" up PPPoE Session ID's from users because it is a value that
is only gotten on the Data Link layer and historically non-root users did not
have access to that.  It really is a value that doesn't concern them.  I have
wrapped the display with a suser() conditional.  The magic value 0x is
not used/reserved according to RFC 2516.

as root:

beta# ifconfig pppoe0
pppoe0: flags=8810 mtu 1492
index 12 priority 0 llprio 3
dev:  state: initial
sid: 0x0 PADI retries: 0 PADR retries: 0
groups: pppoe

as non-root:

beta$ ifconfig pppoe0 
pppoe0: flags=8810 mtu 1492
index 12 priority 0 llprio 3
dev:  state: initial
sid: 0x PADI retries: 0 PADR retries: 0
groups: pppoe


patch follows:

Index: if_pppoe.c
===
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.67
diff -u -p -u -r1.67 if_pppoe.c
--- if_pppoe.c  19 Feb 2018 08:59:52 -  1.67
+++ if_pppoe.c  18 Jan 2019 09:50:58 -
@@ -874,7 +876,12 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
struct pppoeconnectionstate *state =
(struct pppoeconnectionstate *)data;
state->state = sc->sc_state;
-   state->session_id = sc->sc_session;
+
+   if (! suser(p))
+   state->session_id = sc->sc_session;
+   else
+   state->session_id = 0x; /* reserved sid */
+
state->padi_retry_no = sc->sc_padi_retried;
state->padr_retry_no = sc->sc_padr_retried;
state->session_time.tv_sec = sc->sc_session_time.tv_sec;

Re: if_pppoe.c patch

[Peter J. Philipp]

On Sun, Jan 20, 2019 at 12:56:22PM +, Stuart Henderson wrote:
> On 2019/01/18 10:59, Peter J. Philipp wrote:
> > I have "covered" up PPPoE Session ID's from users because it is a value that
> > is only gotten on the Data Link layer and historically non-root users did 
> > not
> > have access to that.  It really is a value that doesn't concern them.  I 
> > have
> > wrapped the display with a suser() conditional.  The magic value 0x is
> > not used/reserved according to RFC 2516.
> 
> No real comment on whether we should do this or not (it feels a bit like
> restricting arp to root..) but if it is done, it would seem better to use

Not like restricting arp to root..., it's more like TCP hiding its ISN.

> a value which cannot be sent on the wire, rather than one which is just
> reserved. (And actually hide it from ifconfig rather than displaying a
> bogus value).

I'll try to get around to it tomorrow if I can.  Otherwise just drop the
request. :-)

Best Regards,
-peter

> > as root:
> > 
> > beta# ifconfig pppoe0
> > pppoe0: flags=8810 mtu 1492
> > index 12 priority 0 llprio 3
> > dev:  state: initial
> > sid: 0x0 PADI retries: 0 PADR retries: 0
> > groups: pppoe
> > 
> > as non-root:
> > 
> > beta$ ifconfig pppoe0 
> > pppoe0: flags=8810 mtu 1492
> > index 12 priority 0 llprio 3
> > dev:  state: initial
> > sid: 0x PADI retries: 0 PADR retries: 0
> > groups: pppoe
> > 
> > 
> > patch follows:
> > 
> > Index: if_pppoe.c
> > ===
> > RCS file: /cvs/src/sys/net/if_pppoe.c,v
> > retrieving revision 1.67
> > diff -u -p -u -r1.67 if_pppoe.c
> > --- if_pppoe.c  19 Feb 2018 08:59:52 -  1.67
> > +++ if_pppoe.c  18 Jan 2019 09:50:58 -
> > @@ -874,7 +876,12 @@ pppoe_ioctl(struct ifnet *ifp, unsigned 
> > struct pppoeconnectionstate *state =
> > (struct pppoeconnectionstate *)data;
> > state->state = sc->sc_state;
> > -   state->session_id = sc->sc_session;
> > +
> > +   if (! suser(p))
> > +   state->session_id = sc->sc_session;
> > +   else
> > +   state->session_id = 0x; /* reserved sid */
> > +
> > state->padi_retry_no = sc->sc_padi_retried;
> > state->padr_retry_no = sc->sc_padr_retried;
> > state->session_time.tv_sec = sc->sc_session_time.tv_sec;
> > 

handling of magic number in LCP echo replies

[Peter J. Philipp]

Hi,

I'd like to get some help determining if this is a problem per se.  In 
/sys/net/if_spppsubr.c lines 1323-1327 the nmagic is assembled and checked
against sp->lcp.magic, and if it doesn't match then it does something weird.  
It resets the sp->pp_alivecnt to 0.  This to me does nothing much other than 
resetting a counter (which only tears down a connection if it has achieved
MAXALIVECNT.

At first I thought it should tear down the link but RFC 1661 says that this
is the pessimistic approach around page 46.

The RFC is a little vague, because it leaves this as unspecified on page
47:

---
  Procedures for recovery from either case are unspecified, and may
  vary from implementation to implementation.  A somewhat
  pessimistic procedure is to assume a LCP Down event.  A further
  Open event will begin the process of re-establishing the link,
  which can't complete until the looped-back condition is
  terminated, and Magic-Numbers are successfully negotiated.  A more
  optimistic procedure (in the case of a looped-back link) is to
  begin transmitting LCP Echo-Request packets until an appropriate
  Echo-Reply is received, indicating a termination of the looped-
  back condition.
---

I'm shrugging my shoulders as I see an unspecified area and it's not
really handled.  Is no code better than some unspecified code?  Is any code
better than leaving it alone?  In my scenario I only looked at the code,
I have not tested anything, to see if its a detriment.

I have checked FreeBSD source and it also does this in head.

Regards,
-peter

ntpd is too noisy about 'DNS lookup tempfail' on IPv6 only hosts

[Peter J. Philipp]

Hi,

I have an IPv6 only host arrowhead.ip6.centroid.eu, that has very noisy:
 
Oct 29 09:12:48 arrowhead ntpd[18744]: DNS lookup tempfail
Oct 29 09:21:45 arrowhead last message repeated 2 times

in fact:

arrowhead# grep 'DNS lookup tempfail' /var/log/daemon | wc -l
1354

This is because the pool.ntp.org servers as configured don't give back 
answers.  I'm trying to streamline this a little and only ask for  queries
if there is no v4 connectivity.  With change of the 'stdio dns' pledge to
'stdio inet dns' this is possible, when using another constraint from google.
There is no network traffic, just a route lookup if IPv4 is possible at all.

Here is my patch, under my sig.

-peter

Index: config.c
===
RCS file: /cvs/src/usr.sbin/ntpd/config.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 config.c
--- config.c7 Jul 2019 07:14:57 -   1.32
+++ config.c6 Nov 2019 07:36:07 -
@@ -30,8 +30,9 @@
 
 #include "ntpd.h"
 
-struct ntp_addr*host_ip(const char *);
-int host_dns1(const char *, struct ntp_addr **, int);
+struct ntp_addr*host_ip(const char *);
+inthost_dns1(const char *, struct ntp_addr **, int);
+static int test_v4_gw(void);
 
 static u_int32_tmaxid = 0;
 static u_int32_tconstraint_maxid = 0;
@@ -59,7 +60,7 @@ host_ip(const char *s)
struct ntp_addr *h = NULL;
 
memset(, 0, sizeof(hints));
-   hints.ai_family = AF_UNSPEC;
+   hints.ai_family = (test_v4_gw() == 0) ? AF_UNSPEC : AF_INET6;
hints.ai_socktype = SOCK_DGRAM; /*dummy*/
hints.ai_flags = AI_NUMERICHOST;
if (getaddrinfo(s, "0", , ) == 0) {
@@ -94,7 +95,7 @@ host_dns1(const char *s, struct ntp_addr
struct ntp_addr *h, *hh = NULL;
 
memset(, 0, sizeof(hints));
-   hints.ai_family = AF_UNSPEC;
+   hints.ai_family = (test_v4_gw() == 0) ? AF_UNSPEC : AF_INET6;
hints.ai_socktype = SOCK_DGRAM; /* DUMMY */
hints.ai_flags = AI_ADDRCONFIG;
error = getaddrinfo(s, NULL, , );
@@ -181,3 +182,28 @@ new_constraint(void)
return (p);
 }
 
+static int
+test_v4_gw(void)
+{
+   struct sockaddr_in sin;
+   socklen_t st = sizeof(struct sockaddr_in);
+   int so;
+
+   so = socket(AF_INET, SOCK_DGRAM, 0);
+   if (so < 0) {
+   return 0;
+   }
+
+   memset(, 0, sizeof(sin));
+   sin.sin_family = AF_INET;
+   sin.sin_addr.s_addr = inet_addr(CONN_CONSTRAINT);
+   sin.sin_port = htons(53);
+   
+   if (connect(so, (struct sockaddr *), st) < 0) {
+   close(so);
+   return 0;
+   }
+
+   close(so);
+   return 1;
+}
Index: ntp_dns.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntp_dns.c,v
retrieving revision 1.24
diff -u -p -u -r1.24 ntp_dns.c
--- ntp_dns.c   27 Jun 2019 15:18:42 -  1.24
+++ ntp_dns.c   6 Nov 2019 07:36:07 -
@@ -98,7 +98,7 @@ ntp_dns(struct ntpd_conf *nconf, struct 
fatal(NULL);
imsg_init(ibuf_dns, PARENT_SOCK_FILENO);
 
-   if (pledge("stdio dns", NULL) == -1)
+   if (pledge("stdio inet dns", NULL) == -1)
err(1, "pledge");
 
probe_root();
@@ -170,7 +170,7 @@ dns_dispatch_imsg(struct ntpd_conf *ncon
strlen(name) != len)
fatalx("invalid %s received", str);
if ((cnt = host_dns(name, nconf->status.synced,
-   )) == -1)
+   )) <= 0)
break;
buf = imsg_create(ibuf_dns, imsg.hdr.type,
imsg.hdr.peerid, 0,
Index: ntpd.h
===
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.h,v
retrieving revision 1.146
diff -u -p -u -r1.146 ntpd.h
--- ntpd.h  16 Jul 2019 14:15:40 -  1.146
+++ ntpd.h  6 Nov 2019 07:36:07 -
@@ -40,6 +40,7 @@
 #defineCONFFILE"/etc/ntpd.conf"
 #define DRIFTFILE  "/var/db/ntpd.drift"
 #defineCTLSOCKET   "/var/run/ntpd.sock"
+#define CONN_CONSTRAINT"8.8.8.8"   /* to test connectivity */
 
 #defineINTERVAL_QUERY_NORMAL   30  /* sync to peers every 
n secs */
 #defineINTERVAL_QUERY_PATHETIC 60

Re: ntpd is too noisy about 'DNS lookup tempfail' on IPv6 only hosts

[Peter J. Philipp]

On Wed, Nov 06, 2019 at 11:30:32AM +0100, Florian Obser wrote:
> > @@ -94,7 +95,7 @@ host_dns1(const char *s, struct ntp_addr
> > struct ntp_addr *h, *hh = NULL;
> >  
> > memset(, 0, sizeof(hints));
> > -   hints.ai_family = AF_UNSPEC;
> > +   hints.ai_family = (test_v4_gw() == 0) ? AF_UNSPEC : AF_INET6;
> > hints.ai_socktype = SOCK_DGRAM; /* DUMMY */
> > hints.ai_flags = AI_ADDRCONFIG;
> 
> you just implemented a variation of AI_ADDRCONFIG

Oh you're right!  Good you're looking over me Florian! So here is the
right patch then that I want OpenBSD to consider, with it I don't see 
the timeouts messages (are they needed?):

Best Regards,

-peter


Index: ntp_dns.c
===
RCS file: /cvs/src/usr.sbin/ntpd/ntp_dns.c,v
retrieving revision 1.24
diff -u -p -u -r1.24 ntp_dns.c
--- ntp_dns.c   27 Jun 2019 15:18:42 -  1.24
+++ ntp_dns.c   6 Nov 2019 10:39:36 -
@@ -170,7 +170,7 @@ dns_dispatch_imsg(struct ntpd_conf *ncon
strlen(name) != len)
fatalx("invalid %s received", str);
if ((cnt = host_dns(name, nconf->status.synced,
-   )) == -1)
+   )) <= 0)
break;
buf = imsg_create(ibuf_dns, imsg.hdr.type,
imsg.hdr.peerid, 0,

Re: ppppoe octeon kernel panic .6.6

[Peter J. Philipp]

Hi Miod,

Thanks for helping.  With this patch unfortunatly I still get a trap 2 on
my small unifi security gateway which I pulled out again to test your patch.

--->
cnmac0: 192.168.177.35 lease accepted from 192.168.177.1 (24:a4:3c:06:9f:16)
pppoe0: received unexpected PADO
pppoe0: host unique tag found, but it belongs to a connection in state 3

Trap cause = 2 Frame 0x98000ffdb860
Trap PC 0x811ac34c RA 0x813a09bc fault 0x0
smallcpy+0x8 (1,980001e1e476,1,2)  ra 0x813a09bc sp 0x98000ffdb9
b8, sz 0
sppp_auth_send+0x10c (1,980001e1e476,1,2)  ra 0x8139c844 sp 0x98
000ffdb9b8, sz 144
sppp_lcp_tlu+0x274 (1,980001e1e476,1,2)  ra 0x81396514 sp 0x9800
0ffdba48, sz 128
sppp_cp_input+0x141c (1,980001e1e476,1,2)  ra 0x81394a08 sp 0x98
000ffdbac8, sz 112
sppp_input+0x1d0 (1,980001e1e476,1,2)  ra 0x8148d2e4 sp 
0x98000ffdbb38, sz 80
pppoeintr+0xf9c (1,980001e1e476,1,2)  ra 0x814a44d8 sp 0x98000ff
dbb88, sz 400
User-level: pid 68736
stopped on non ddb fault
Stopped at  smallcpy+0x8:   lbu v1,0(a0)
ddb{0}> 
<---

Sorry that it does this.  My patch still stands I'm amazed!

Best Regards,

-peter



On Wed, Oct 23, 2019 at 05:15:41PM -, Miod Vallat wrote:
> 
> > Try changing all the final 0 in sppp_auth_send() to 0UL and this ought
> > to work. This function needs __attribute__((__sentinel__)) as well to
> > prevent such mistakes from occurring again.
> 
> The sentinel attribute wants a pointer, not a zero size_t,
> unfortunately.
> 
> Please try this diff.
> 
> Index: if_spppsubr.c
> ===
> RCS file: /OpenBSD/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.179
> diff -u -p -r1.179 if_spppsubr.c
> --- if_spppsubr.c 24 Jun 2019 21:36:53 -  1.179
> +++ if_spppsubr.c 23 Oct 2019 17:12:53 -
> @@ -3340,7 +3340,7 @@ sppp_chap_input(struct sppp *sp, struct 
>  sizeof digest, digest,
>  strlen(sp->myauth.name),
>  sp->myauth.name,
> -0);
> +0UL);
>   break;
>  
>   case CHAP_SUCCESS:
> @@ -3460,7 +3460,7 @@ sppp_chap_input(struct sppp *sp, struct 
>   /* action scn, tld */
>   sppp_auth_send(, sp, CHAP_FAILURE, h->ident,
>  sizeof(FAILMSG) - 1, (u_char *)FAILMSG,
> -0);
> +0UL);
>   chap.tld(sp);
>   break;
>   }
> @@ -3469,7 +3469,7 @@ sppp_chap_input(struct sppp *sp, struct 
>   sp->state[IDX_CHAP] == STATE_OPENED)
>   sppp_auth_send(, sp, CHAP_SUCCESS, h->ident,
>  sizeof(SUCCMSG) - 1, (u_char *)SUCCMSG,
> -0);
> +0UL);
>   if (sp->state[IDX_CHAP] == STATE_REQ_SENT) {
>   sppp_cp_change_state(, sp, STATE_OPENED);
>   chap.tlu(sp);
> @@ -3647,7 +3647,7 @@ sppp_chap_scr(struct sppp *sp)
>  (size_t)AUTHCHALEN, sp->chap_challenge,
>  strlen(sp->myauth.name),
>  sp->myauth.name,
> -0);
> +0UL);
>  }
>  /*
>   *--*
> @@ -3726,7 +3726,7 @@ sppp_pap_input(struct sppp *sp, struct m
>   sppp_auth_send(, sp, PAP_NAK, h->ident,
>  sizeof mlen, (const char *),
>  sizeof(FAILMSG) - 1, (u_char *)FAILMSG,
> -0);
> +0UL);
>   pap.tld(sp);
>   break;
>   }
> @@ -3737,7 +3737,7 @@ sppp_pap_input(struct sppp *sp, struct m
>   sppp_auth_send(, sp, PAP_ACK, h->ident,
>  sizeof mlen, (const char *),
>  sizeof(SUCCMSG) - 1, (u_char *)SUCCMSG,
> -0);
> +0UL);
>   }
>   if (sp->state[IDX_PAP] == STATE_REQ_SENT) {
>   sppp_cp_change_state(, sp, STATE_OPENED);
> @@ -3952,7 +3952,7 @@ sppp_pap_scr(struct sppp *sp)
>  (size_t)idlen, sp->myauth.name,
>  sizeof pwdlen, (const char *),
>  (size_t)pwdlen, sp->myauth.secret,
> -0);
> +0UL);
>  }
>  /*
>   * Random miscellaneous functions.

Re: ppppoe octeon kernel panic .6.6

[Peter J. Philipp]

Hi Janne,

I think the way this patch is, it's ghetto, I don't even know if OpenBSD
wants to take it on, hence I sent it as a hint.  What really would be cool is
to find out why exactly the trap 2 happens, because the pppoe code works on
a lot of other archs.

My effort is selfish because I want my octeon router doing gateway and not
an old i386 router :-), I was in a race with myself to get this going again.

But if OpenBSD for whatever reason feels the code is almost acceptable, then
the magic number 0x5, 1 can be replaced with a couple of defines perhaps:

#define SPPP_AUTH_SEND_FIRST_ROW0x1
#define SPPP_AUTH_SEND_SECOND_ROW   0x2
#define SPPP_AUTH_SEND_THIRD_ROW0x4

and then the 0x5 would become 
(SPPP_AUTH_SEND_FIRST_ROW|SPPP_AUTH_SEND_THIRD_ROW) and the one 1 would become 
SPPP_AUTH_SEND_FIRST_ROW.

We can also rename the seq to row, to make it ever more obvious that the
varargs are in pairs and one pair is a row.  

Looking at this I agree it's ugly.  Perhaps it needs to be refactored again or
downright fixed (ie. like the other archs, or are they somehow broken too but
behave differently?).

Best Regards,
-peter


On Wed, Oct 23, 2019 at 11:47:03AM +0200, Janne Johansson wrote:
> Den ons 23 okt. 2019 kl 09:15 skrev Peter J. Philipp :
> 
> > Hi Holger & Tech,
> >
> > I have made my octeon router work again and I have a patch.
> >
> >
> Truncated it a lot, leaving the things I reacted on:
> 
> 
> > -   sppp_auth_send(, sp, CHAP_RESPONSE, h->ident,
> > -  sizeof dsize, (const char *),
> > +   sppp_auth_send(, sp, CHAP_RESPONSE, h->ident, 0x1,
> > +  1, dsize,
> >
> 
> 
> > -   sppp_auth_send(, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP],
> > -  sizeof clen, (const char *),
> > +   sppp_auth_send(, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP],
> > 0x1,
> > +  1, clen,
> >
> 
> 
> > -  sizeof mlen, (const char *),
> > +  0x1,
> > +  1, mlen,
> >
> 
> 
> > -  sizeof idlen, (const char *),
> > -  (size_t)idlen, sp->myauth.name,
> > -  sizeof pwdlen, (const char *),
> > -  (size_t)pwdlen, sp->myauth.secret,
> > +  0x5,
> > +  1, s_id,
> > +  s_id, sp->myauth.name,
> > +  1, s_pwd,
> > +  s_pwd, sp->myauth.secret,
> >
> 
> For all the good this patch might do, it still replaces a lot of things in
> the source with names like XYZlen with 0x1 or 0x5,
> so it will be much harder later on to figure out why some part is sending 5
> over to the function it calls. 8-/
> Magic constants makes peoples eyes hurt.
> 
> -- 
> May the most significant bit of your life be positive.

Re: ppppoe octeon kernel panic .6.6

[Peter J. Philipp]

On Wed, Oct 23, 2019 at 08:21:50AM +0200, Holger Glaess wrote:
> hi
> 
> 
> here the traceback , i hope ;)

Hi Holger & Tech,

I have made my octeon router work again and I have a patch.  But I'm not an
openbsd developer, nor is this patch official in any way.  It was a lot of
debugging and refactoring I had to do in /sys/net/if_spppsubr.c because
the varargs were really screwy.  size_t is not a standard builtin vararg
I believe and there was some sideffects with that.  I also applied a header
include for strlen() in this patch.

This patch should be CC'ed to tech@ and they can disect it and use it for
hints.  I have not tested this patch on any arch other than octeon.  In the
end it was not time wasted I spent 2 mornings and 2 nights on this.

You should be OK extracing sys.tar.gz in your octeon and build a kernel like
the normal way.  I know the octeons are usually low on diskspace.

Best Regards,
-peter


--- if_spppsubr.c.orig  Tue Oct 22 18:49:47 2019
+++ if_spppsubr.c   Wed Oct 23 08:03:35 2019
@@ -64,6 +64,7 @@
 #endif
 
 #include 
+#include 
 
 # define UNTIMEOUT(fun, arg, handle)   \
timeout_del(&(handle))
@@ -233,7 +234,7 @@
 int newstate);
 void sppp_auth_send(const struct cp *cp,
   struct sppp *sp, unsigned int type, u_int id,
-  ...);
+   u_int bitmask, ...);
 
 void sppp_up_event(const struct cp *cp, struct sppp *sp);
 void sppp_down_event(const struct cp *cp, struct sppp *sp);
@@ -3277,7 +3278,8 @@
STDDCL;
struct lcp_header *h;
int len, x;
-   u_char *value, *name, digest[AUTHCHALEN], dsize;
+   u_char *value, *name, digest[AUTHCHALEN];
+   int dsize;
int value_len, name_len;
MD5_CTX ctx;
 
@@ -3335,8 +3337,8 @@
MD5Final(digest, );
dsize = sizeof digest;
 
-   sppp_auth_send(, sp, CHAP_RESPONSE, h->ident,
-  sizeof dsize, (const char *),
+   sppp_auth_send(, sp, CHAP_RESPONSE, h->ident, 0x1,
+  1, dsize,
   sizeof digest, digest,
   strlen(sp->myauth.name),
   sp->myauth.name,
@@ -3458,7 +3460,7 @@
if (value_len != sizeof digest ||
timingsafe_bcmp(digest, value, value_len) != 0) {
/* action scn, tld */
-   sppp_auth_send(, sp, CHAP_FAILURE, h->ident,
+   sppp_auth_send(, sp, CHAP_FAILURE, h->ident, 0, 
   sizeof(FAILMSG) - 1, (u_char *)FAILMSG,
   0);
chap.tld(sp);
@@ -3467,7 +3469,7 @@
/* action sca, perhaps tlu */
if (sp->state[IDX_CHAP] == STATE_REQ_SENT ||
sp->state[IDX_CHAP] == STATE_OPENED)
-   sppp_auth_send(, sp, CHAP_SUCCESS, h->ident,
+   sppp_auth_send(, sp, CHAP_SUCCESS, h->ident, 0,
   sizeof(SUCCMSG) - 1, (u_char *)SUCCMSG,
   0);
if (sp->state[IDX_CHAP] == STATE_REQ_SENT) {
@@ -3634,7 +3636,7 @@
 void
 sppp_chap_scr(struct sppp *sp)
 {
-   u_char clen;
+   int clen;
 
/* Compute random challenge. */
arc4random_buf(sp->chap_challenge, sizeof(sp->chap_challenge));
@@ -3642,8 +3644,8 @@
 
sp->confid[IDX_CHAP] = ++sp->pp_seq;
 
-   sppp_auth_send(, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP],
-  sizeof clen, (const char *),
+   sppp_auth_send(, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP], 0x1,
+  1, clen,
   (size_t)AUTHCHALEN, sp->chap_challenge,
   strlen(sp->myauth.name),
   sp->myauth.name,
@@ -3671,7 +3673,8 @@
STDDCL;
struct lcp_header *h;
int len, x;
-   u_char *name, *passwd, mlen;
+   u_char *name, *passwd;
+   int mlen;
int name_len, passwd_len;
 
len = m->m_pkthdr.len;
@@ -3724,7 +3727,8 @@
/* action scn, tld */
mlen = sizeof(FAILMSG) - 1;
sppp_auth_send(, sp, PAP_NAK, h->ident,
-  sizeof mlen, (const char *),
+  0x1,
+  1, mlen,
   sizeof(FAILMSG) - 1, (u_char *)FAILMSG,
   0);
pap.tld(sp);
@@ -3735,7 +3739,8 @@
sp->state[IDX_PAP] == STATE_OPENED) {
mlen = sizeof(SUCCMSG) - 1;
sppp_auth_send(, sp, PAP_ACK, h->ident,
-  sizeof mlen, (const char *),
+  

Re: ppppoe octeon kernel panic .6.6

[Peter J. Philipp]

On Wed, Oct 23, 2019 at 11:18:11AM +0200, Martin Pieuchot wrote:
> On 23/10/19(Wed) 08:43, Peter J. Philipp wrote:
> > Hi Holger & Tech,
> 
> Hello Peter,
> 
> > I have made my octeon router work again and I have a patch.  But I'm not an
> > openbsd developer, nor is this patch official in any way. 
> 
> Could you explain in words what is the issue?  Why does your diff
> prevent it?
> 
> Thanks!

Hi,

The system has a trap 2, which I looked up as:

#define T_TLB_LD_MISS   2   /* TLB miss on load or ifetch */

what happens before this patch, I think, is that there is a varargs size_t 
(which is size 8 in mips64), that gets promoted (I think) in varargs to int 
(which would likely be size 4).  Then what happens is the char * that is 
va_arg'ed after that is somehow corrupted on length 1, bcopy would trap #2 
on this.

If it is a varargs corruption in the builtin mips64 clang code then I don't
want to have to fix that, as I don't even know where to look.  Instead I 
laid out every buffer very carefully and using int's and char *'s so that 
trap 2 is not called.  I made a bitmask in the called function that usually 
gets the trap that it will treat length of one as an int and the rest as a 
char *.

I admit it was a lot of trial and error I must have gone through over 20+ 
compile/reboot cycles to get it right.  I tested this on a unifi security
gateway and an ER-8.  Both agreed to everything and at that spot is not trap 2.

I may have avoided the condition, but the condition may still be there on
other code.  I didn't look.  To easen debug of this I'll attach some config 
files for npppd that I used in my lab that you can see for yourself on any 
octeon pppoe.

I don't know if these were standard example npppd's or if they were left over
from an earlier project.

Thanks!

-peter

beta$ more npppd.conf
###npppd.conf 

tunnel PPPOE protocol pppoe { 
 listen on interface ix1
 pppoe-desc-in-pktdump yes 
 pppoe-desc-out-pktdump yes 
 pppoe-session-in-pktdump yes 
 pppoe-session-out-pktdump yes 
 authentication-method pap
} 

ipcp IPCP { 
 pool-address 10.0.0.2-10.0.0.254 
 dns-servers 192.168.177.40
} 

interface tun1 address 10.0.0.1 ipcp IPCP 
authentication LOCAL type local { 
  users-file "/etc/npppd/npppd-users" 
} 
bind tunnel from PPPOE authenticated by LOCAL to tun1
##

beta# more /etc/npppd/npppd-users
# $OpenBSD: npppd-users,v 1.1 2012/09/20 12:51:43 yasuoka Exp $
# sample npppd-users file.  see npppd-users(5)

testcaller:\
:password=Verizon:\
:framed-ip-address=10.0.0.103

##




> > --- if_spppsubr.c.orig  Tue Oct 22 18:49:47 2019
> > +++ if_spppsubr.c   Wed Oct 23 08:03:35 2019
> > @@ -64,6 +64,7 @@
> >  #endif
> >  
> >  #include 
> > +#include 
> >  
> >  # define UNTIMEOUT(fun, arg, handle)   \
> > timeout_del(&(handle))
> > @@ -233,7 +234,7 @@
> >  int newstate);
> >  void sppp_auth_send(const struct cp *cp,
> >struct sppp *sp, unsigned int type, u_int id,
> > -  ...);
> > +   u_int bitmask, ...);
> >  
> >  void sppp_up_event(const struct cp *cp, struct sppp *sp);
> >  void sppp_down_event(const struct cp *cp, struct sppp *sp);
> > @@ -3277,7 +3278,8 @@
> > STDDCL;
> > struct lcp_header *h;
> > int len, x;
> > -   u_char *value, *name, digest[AUTHCHALEN], dsize;
> > +   u_char *value, *name, digest[AUTHCHALEN];
> > +   int dsize;
> > int value_len, name_len;
> > MD5_CTX ctx;
> >  
> > @@ -3335,8 +3337,8 @@
> > MD5Final(digest, );
> > dsize = sizeof digest;
> >  
> > -   sppp_auth_send(, sp, CHAP_RESPONSE, h->ident,
> > -  sizeof dsize, (const char *),
> > +   sppp_auth_send(, sp, CHAP_RESPONSE, h->ident, 0x1,
> > +  1, dsize,
> >sizeof digest, digest,
> >strlen(sp->myauth.name),
> >sp->myauth.name,
> > @@ -3458,7 +3460,7 @@
> > if (value_len != sizeof digest ||
> > timingsafe_bcmp(digest, value, value_len) != 0) {
> > /* action scn, tld */
> > -   sppp_auth_send(, sp, CHAP_FAILURE, h->ident,
> > +   sppp_auth_send(, sp, CHAP_FAILURE, h->ident, 0, 
> >sizeof(FAILMSG) - 1, (u_char *)FAILMSG,
> >   

patch for dump for high percentages

[Peter J. Philipp]

Hi,

I have a patch for dump(8) if it is generally considered bad if percentage
done is over 100.0%.  I checked the archives on marc.info for this and didn't
see any discussion whether this was a topic before.

Here is the odd DUMP message I got on a host:

DUMP: 102.41% done, finished in 0:00

And here is the patch:

Index: optr.c
===
RCS file: /cvs/src/sbin/dump/optr.c,v
retrieving revision 1.40
diff -u -p -u -r1.40 optr.c
--- optr.c  22 Jan 2019 16:16:26 -  1.40
+++ optr.c  29 Feb 2020 16:19:25 -
@@ -202,6 +202,7 @@ void
 timeest(void)
 {
time_t  tnow, deltat;
+   float percent;
 
(void) time();
if (tnow >= tschedule) {
@@ -211,8 +212,9 @@ timeest(void)
deltat = tstart_writing - tnow +
(1.0 * (tnow - tstart_writing))
/ blockswritten * tapesize;
+   percent = (blockswritten * 100.0) / tapesize;
msg("%3.2f%% done, finished in %lld:%02lld\n",
-   (blockswritten * 100.0) / tapesize,
+   (percent > 100.0) ? 100.0 : percent,
(long long)deltat / 3600,
((long long)deltat % 3600) / 60);
}


I tested this and it seems to report like before, only when it hits 100.0 it
will not go higher, that particular codepath I didn't hit though.

beta# dump -0uaf - /var | gzip -c > /dev/null
  DUMP: Date of this level 0 dump: Sat Feb 29 17:20:26 2020
  DUMP: Date of last level 0 dump: the epoch
  DUMP: Dumping /dev/rsd2e (/var) to standard output
  DUMP: mapping (Pass I) [regular files]
  DUMP: mapping (Pass II) [directories]
  DUMP: estimated 23891439 tape blocks.
  DUMP: Volume 1 started at: Sat Feb 29 17:20:37 2020
  DUMP: dumping (Pass III) [directories]
  DUMP: dumping (Pass IV) [regular files]
  DUMP: 25.98% done, finished in 0:14

Let me know what you think,

Regards,
-peter

add DIOCRADDADDRS ioctl to kern_pledge pf

[Peter J. Philipp]

Hi,

I'm in the process of building a program that adds IP addresses to a table, 
from the network,  It is HMAC'ed.

I was stopped by a pledge, it seems it was not configured.  Here is the
ktrace snippet:

 40051 table-server CALL  open(0xbb705fb11f6,0x2)
 40051 table-server NAMI  "/dev/pf"
 40051 table-server RET   open 4
 40051 table-server CALL  kbind(0x7f7a2b08,24,0x2de4af929c6b5090)
 40051 table-server RET   kbind 0
 40051 table-server CALL  ioctl(4,DIOCRADDTABLES,0x7f7a32a8)
 40051 table-server RET   ioctl 0
 40051 table-server CALL  kbind(0x7f7a2b08,24,0x2de4af929c6b5090)
 40051 table-server RET   kbind 0
 40051 table-server CALL  ioctl(4,DIOCRADDADDRS,0x7f7a32a8)
 40051 table-server PLDG  ioctl, "tty", errno 1 Operation not permitted
 40051 table-server PSIG  SIGABRT SIG_DFL
 40051 table-server NAMI  "table-server.core"

Here is a patch to consider, it compiles but I haven't tested it yet because
I'm unsure if there is a reason why this DIOCR* was left out.

I'm guessing, if the patch is OK,  I'll have to leave the pledge out for 6.6 
which is what this is intended for.  Sad, but OK, at least there is unveil.


Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.256
diff -u -p -u -r1.256 kern_pledge.c
--- kern_pledge.c   8 Dec 2019 23:08:59 -   1.256
+++ kern_pledge.c   14 Jan 2020 17:51:19 -
@@ -1205,6 +1205,7 @@ pledge_ioctl(struct proc *p, long com, s
case DIOCADDRULE:
case DIOCGETSTATUS:
case DIOCNATLOOK:
+   case DIOCRADDADDRS:
case DIOCRADDTABLES:
case DIOCRCLRADDRS:
case DIOCRCLRTABLES:




Cheers,
-peter

Re: add DIOCRADDADDRS ioctl to kern_pledge pf

[Peter J. Philipp]

On Tue, Jan 14, 2020 at 11:05:38AM -0700, Theo de Raadt wrote:
> Some of the pledges (such as "pf") exist to support a cluster of
> programs -- not just 1 program -- and improve their security by limiting
> what they can do.  So that when the program gets subverted due something
> on it's input, the damage it can do is limited.


> Additionally, the list of operations permitted is kept very small, so that
> the switch() case in the kernel don't turn into a bloated fiasco.

OK.

> Your proposal supports 1 program.  Which isn't even in base.  There is
> no way I'm going accept this change.

OK, I see.  Yes it was selfish of me.

> Beyond that I need to once again point out a major missed step:
> 
> Your proposal is to permit DIOCRADDADDRS in all the current programs
> using "pf".  But you did not assessment to determine if there is a
> downside to giving them such access.  You simply wanted to barrel along
> forward, to support your one little program.
> 
> Ignoring the impact of your changes on the rest of the ecosystem is
> totally nuts.

Thanks for pointing that out.  I did take a look at one program in base
though and noticed it wasn't pledged.  But yes I barrelled along.  Sorry.

Regards,
-peter 

Re: powerpc: mplock & WITNESS

[Peter J. Philipp]

On Thu, Apr 09, 2020 at 10:58:29PM -0400, George Koehler wrote:



> In the trace, #0 and #1 are wrong, but the rest of the trace looks
> good enough for WITNESS.  I added an artificial lock order reversal to
> ums(4) for WITNESS to catch.  I got this trace,
> 
> #0  0xe4d764   
> #1  witness_checkorder+0x308
> #2  mtx_enter+0x50  
> #3  ums_attach+0x68 
> #4  config_attach+0x270
> ...
> 
> "#0  0xe4d764" is stack garbage: a function saves its lr in its
> caller's frame, but stacktrace_save_at() first reads the lr slot in
> its own frame.

I wonder if this is the stack to stacktrace_save()?  Would sprinkling
__inline's at the function declaration help that any?


> "#1  witness_checkorder+0x308" is a dead value.  In the disassembly
> (objdump -dlr db_trace.o), clang optimized stacktrace_save_at() to
> skip saving its lr on the stack, because it is a leaf function (that
> never calls other functions).  The lr from the stack isn't the return
> address for stacktrace_save_at(), but might be a leftover return
> address from some other function (that needed to save lr).

I'm gonna boot my G5 in a sec (which I'm considering renaming tractor or 
jetengine) so that I can see that too.  I also haven't had a witness event
yet hoping on that in a second.

> #2 and after seem to be correct; "#3  ums_attach+0x68" points exactly
> to where I am grabbing the second lock.  This is enough for WITNESS,
> so we might want to use your diff now, and fix #0 and #1 later.
> 
> Also know that a compiler may optimize stacktrace_save_at() to have
> no stack frame.  Our clang 8.0.1 never does this (because it always
> sets r31 = stack pointer r1, so it always needs a stack frame to save
> the caller's r31), but gcc and newer clang might.  I don't know how
> __builtin_frame_address(0) works if the stack frame is gone.

Oh then __inline perhaps wouldn't work?


> --George
> 

Great debug George!  Thanks!

Best Regards,
-peter

arm64 mainbus.c patch

[Peter J. Philipp]

Hi,

While code-reading the riscv64 port (which leans on some arm64 code), I have
found a small gotcha in /sys/arch/arm64/dev/mainbus.c.  The patch is self
explanatory and leans on the fix from simplebus.c line 210.


Index: mainbus.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/mainbus.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 mainbus.c
--- mainbus.c   23 Oct 2019 09:27:43 -  1.15
+++ mainbus.c   9 Apr 2020 07:02:13 -
@@ -220,7 +220,7 @@ mainbus_attach_node(struct device *self,
 
len = OF_getproplen(node, "reg");
line = (sc->sc_acells + sc->sc_scells) * sizeof(uint32_t);
-   if (len > 0 && (len % line) == 0) {
+   if (len > 0 && line > 0 && (len % line) == 0) {
reg = malloc(len, M_TEMP, M_WAITOK);
OF_getpropintarray(node, "reg", reg, len);
 


Basically it avoids a divide by zero if acells or scells happen to be 0.
Probably not possible but this check is also in simplebus.c so thought I'd
get it reported.

Best Regards,
-peter

Re: powerpc: mplock & WITNESS

[Peter J. Philipp]

It's April 9th for me, so no chance for April 1st things.  Both patches didn't
boot (they loaded on ofwboot though) for me.  I assume you wanted me to enable
WITNESS option which I did.  The kernel did not print anything so it must have
done something before openfirmware...

I'm going to check out my sources anew perhaps there was a something amiss,
compiling takes a while on a G5.

Best Regards,
-peter

On Thu, Apr 09, 2020 at 11:09:02AM +0200, Martin Pieuchot wrote:
> On 09/04/20(Thu) 10:53, Mark Kettenis wrote:
> > > Date: Thu, 9 Apr 2020 10:01:09 +0200
> > > From: Martin Pieuchot 
> > > [...]
> > > + lastsp = sp;
> > > + sp = *(vaddr_t *)sp;
> > > +
> > > + if ((sp == 0) || (sp <= lastsp))
> > > + break;
> > 
> > I think checking the alignment of sp here like you do for lr would be
> > a good idea.  Otherwise an unaligned access might happen.  Since
> > powerpc uses separate interrupt stacks, the (sp <= lastsp) check might
> > truncate the stack trace.
> 
> Something like in the diff below?
> 
> Index: arch/powerpc/conf/files.powerpc
> ===
> RCS file: /cvs/src/sys/arch/powerpc/conf/files.powerpc,v
> retrieving revision 1.55
> diff -u -p -r1.55 files.powerpc
> --- arch/powerpc/conf/files.powerpc   25 Jan 2018 15:06:29 -  1.55
> +++ arch/powerpc/conf/files.powerpc   9 Apr 2020 07:38:37 -
> @@ -13,7 +13,6 @@ filearch/powerpc/powerpc/process_machde
>  file arch/powerpc/powerpc/sys_machdep.c
>  file arch/powerpc/powerpc/trap.c
>  file arch/powerpc/powerpc/vm_machdep.c
> -file arch/powerpc/powerpc/lock_machdep.c multiprocessor
>  file arch/powerpc/powerpc/intr.c
>  file arch/powerpc/powerpc/softintr.c
>  
> Index: arch/powerpc/ddb/db_trace.c
> ===
> RCS file: /cvs/src/sys/arch/powerpc/ddb/db_trace.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 db_trace.c
> --- arch/powerpc/ddb/db_trace.c   7 Nov 2019 16:08:08 -   1.14
> +++ arch/powerpc/ddb/db_trace.c   9 Apr 2020 09:07:17 -
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -221,4 +222,39 @@ db_stack_trace_print(db_expr_t addr, int
>   --count;
>   }
>   (*pr)("end trace frame: 0x%lx, count: %d\n", sp, count);
> +}
> +
> +void
> +stacktrace_save_at(struct stacktrace *st, unsigned int skip)
> +{
> + vaddr_t  lr, sp, lastsp;
> +
> + sp = (vaddr_t)__builtin_frame_address(0);
> + KASSERT(INKERNEL(sp) || ININTSTK(sp));
> +
> + st->st_count = 0;
> + while (st->st_count < STACKTRACE_MAX) {
> + lr = *(vaddr_t *)(sp + 4) - 4;
> + if (lr & 3)
> + break;
> +
> + if (skip == 0)
> + st->st_pc[st->st_count++] = lr;
> + else
> + skip--;
> +
> + lastsp = sp;
> + sp = *(vaddr_t *)sp;
> +
> + if ((sp == 0) || (sp & 3) || (sp <= lastsp))
> + break;
> + if (!INKERNEL(sp) && !ININTSTK(sp))
> + break;
> + }
> +}
> +
> +void
> +stacktrace_save(struct stacktrace *st)
> +{
> + return stacktrace_save_at(st, 0);
>  }
> Index: arch/powerpc/include/mplock.h
> ===
> RCS file: /cvs/src/sys/arch/powerpc/include/mplock.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 mplock.h
> --- arch/powerpc/include/mplock.h 4 Dec 2017 09:51:03 -   1.3
> +++ arch/powerpc/include/mplock.h 9 Apr 2020 07:37:25 -
> @@ -1,52 +1,10 @@
>  /*   $OpenBSD: mplock.h,v 1.3 2017/12/04 09:51:03 mpi Exp $  */
>  
> -/*
> - * Copyright (c) 2004 Niklas Hallqvist.  All rights reserved.
> - *
> - * Redistribution and use in source and binary forms, with or without
> - * modification, are permitted provided that the following conditions
> - * are met:
> - * 1. Redistributions of source code must retain the above copyright
> - *notice, this list of conditions and the following disclaimer.
> - * 2. Redistributions in binary form must reproduce the above copyright
> - *notice, this list of conditions and the following disclaimer in the
> - *documentation and/or other materials provided with the distribution.
> - *
> - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> - * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> - * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> - * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> - * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> 

Re: powerpc: mplock & WITNESS

[Peter J. Philipp]

On Thu, Apr 09, 2020 at 01:08:12PM +0200, Martin Pieuchot wrote:
> On 09/04/20(Thu) 12:20, Peter J. Philipp wrote:
> > It's April 9th for me, so no chance for April 1st things.  Both patches 
> > didn't
> > boot (they loaded on ofwboot though) for me.  I assume you wanted me to 
> > enable
> > WITNESS option which I did.  The kernel did not print anything so it must 
> > have
> > done something before openfirmware...
> 
> Do they boot without WITNESS?  They might be other issues to address.

Just an update to list.

GENERIC.MP without WITNESS - did not boot
GENERIC without WITNESS - did boot
GENERIC with WITNESS - panic'ed on boot.

It took some time determining if GENERIC.MP was compiled right.  After newfs
of /usr/{obj,src}, cvs update and patching the info above was gathered.

I have provided Martin a screenshot jpg of the panic on boot.  If you want
it let me know.

Best Regards,
-peter