clear pf state when reconnecting a udp socket

2020-04-21 Thread Tim Kuijsten
Whenever a connected UDP socket is connected to another peer, any previous
state should be cleared in PF so that only datagrams from the newly connected
peer are deliverd to the socket.

Without this patch the following scenario can occur on machines with PF
enabled using keep state on matching rules (keep state is the default). Say
there are two peers, peer1 and peer2 and we have one connected UDP socket
"sock".

first connect(2) to the first peer: connect(sock, peer1)
Now "sock" only receives datagrams from peer1. Whenever peer2 sends a
datagram to the local end of "sock" it is not delivered to "sock"
because the foreign address in the pcb does not match. This is all
good and expected.

Assume peer1 has sent a datagram to "sock" so that state is created in pf(4).

Now we connect the same socket to the second peer: connect(sock, peer2)
Now "sock" receives datagrams from peer2 as expected but for as long
as "keep state" remains (30 to 60 seconds, see udp.first, udp.multiple
and udp.single in pf.conf(5)) datagrams from peer1 are also delivered
to "sock" even though it is connected to peer2.

The following patch fixes this behaviour and clears state on PRU_DISCONNECT.
I'm not sure if this is the best place to fix it nor whether there are more
places that should be looked at.

Found out about this while debugging a WireGuard client that was switching
back and forth between a WiFi and LTE while connected to my OpenBSD server
that uses connected UDP sockets.
Index: udp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.257
diff -u -p -c -r1.257 udp_usrreq.c
*** udp_usrreq.c6 Dec 2019 14:43:14 -   1.257
--- udp_usrreq.c21 Apr 2020 08:47:18 -
*** udp_usrreq(struct socket *so, int req, s
*** 1105,1110 
--- 1105,1115 
}
}
  
+ #if NPF > 0
+   if (so->so_state & SS_ISCONNECTED)
+   pf_inp_unlink(inp);
+ #endif
+ 
  #ifdef INET6
if (inp->inp_flags & INP_IPV6)
inp->inp_laddr6 = in6addr_any;


Re: [PATCH] correcting in-sane ntpd.conf

2019-12-09 Thread Tim Kuijsten
> Nor do you bring up the traffic to the IP addresses offered by
> pool.ntp.org.  That traffic has a pattern easily distinguished as
> "system startup".
> 
> What's the difference?  There isn't.  Yet you brought up only google.

I can understand why someone would be ok with sending some packets
to small players like pool.ntp.org and not be ok with sending packets
to extremely big and powerful companies that are in the business
of surveillance capitalism. Divide and conquer!



unveil and renameat

2019-08-01 Thread Tim Kuijsten
Today I finally got to try unveil(2) and retrofit it into one of
my applications. I really like it.

But there was one thing that tripped me up for a bit.

When trying to move a file from one directory into a subdirectory
I kept getting an ENOENT when trying to accomplish this with
renameat(2). Attached is a little sample program that shows the
behavior.

You can see the problem when trying to move the source file from
the current dir into the test dir:
$ cc unveilrenameat.c && ./a.out . unveilrenameat.c test
a.out: moving ./unveilrenameat.c to test/unveilrenameat.c
a.out: renameat: No such file or directory

Instead I would have expected it to have moved the source file into
the test directory, this is what it does when using rename(2)
instead.

Besides this nitpick, thanks for unveil!

-Tim
#include 

#include 
#include 
#include 
#include 
#include 
#include 

int
main(int argc, char **argv)
{
	const char *basedir, *file, *subdir;
	int fd1, fd2;

	if (argc != 4)
		errx(1, "usage: %s basedir file subdir", getprogname());

	basedir = argv[1];
	file = argv[2];
	subdir = argv[3];

	warnx("moving %s/%s to %s/%s", basedir, file, subdir, file);

	if (unveil(basedir, "rwc") == -1)
		err(1, "unveil");

	if (unveil(NULL, NULL) == -1)
		err(1, "unveil");

	if ((fd1 = open(basedir, O_RDONLY)) == -1)
		err(1, "open");

	if (mkdirat(fd1, subdir, 0755) == -1 && errno != EEXIST)
		err(1, "mkdirat");

	if ((fd2 = openat(fd1, subdir, O_DIRECTORY|O_RDONLY)) == -1)
		err(1, "openat");

	if (renameat(fd1, file, fd2, file) == -1)
		err(1, "renameat");

	return 0;
}


Re: Grammar and style edits to installation guide

2019-07-09 Thread Tim Kuijsten
Ian McWilliam  wrote:
> Isn't Unix a trademark of the Open Group? Hence the usage of Unix-like or 
> Un*x..

That trademark is UNIX, all caps.

According to [APUEv3]:
"The Open Group owns the UNIX trademark and uses the Single UNIX Specification
to define the interfaces an implementation must support to call itself a UNIX
system. Vendors must file conformance statements, pass test suites to verify
conformance, and license the right to use the UNIX trademark."

For example, Mac OS X 10.5 and other big commercial brands have been [certified]
and may call themselves a UNIX system (again, the all caps variant is
significant here).

[APUEv3] Advanced Programming in the UNIX Environment Third Edition p30
[certified] https://www.opengroup.org/certifications/unix



cross-reference htobe64(3) in htonl(3)

2019-02-12 Thread Tim Kuijsten

Found out about htobe64(3) after grepping through the source.

-Tim
Index: htonl.3
===
RCS file: /cvs/src/lib/libc/net/htonl.3,v
retrieving revision 1.4
diff -u -p -u -r1.4 htonl.3
--- htonl.3 10 Mar 2016 08:42:26 -  1.4
+++ htonl.3 12 Feb 2019 15:41:13 -
@@ -85,7 +85,8 @@ and
 .Xr getservent 3 .
 .Sh SEE ALSO
 .Xr gethostbyname 3 ,
-.Xr getservent 3
+.Xr getservent 3 ,
+.Xr htobe64 3
 .Sh STANDARDS
 The
 .Fn htonl ,


Re: disable the ability to change tun(4) mode from p2p to bcast and back again

2019-02-05 Thread Tim Kuijsten

On Tue, Feb 05, 2019 at 01:50:25PM +1000, David Gwynne wrote:




On 4 Feb 2019, at 22:00, Tim Kuijsten  wrote:

On Mon, Feb 04, 2019 at 12:07:22PM +1000, David Gwynne wrote:

Currently you can change a tun interface from being point to point to
being a broadcast interface. Why?


I'm using broadcast mode in my own wireguard implementation because there can 
be more than one peer on the network:
https://github.com/timkuijsten/uwg/blob/ccd39c6a9bdf36575a3bb3db06c438a2241c1134/ifn.c#L1868


But there's only one process sucking on the /dev entry, so there's just the one 
pipe. Does it make a difference to the routes you can add whether tun is only 
point to point, or is broadcast required? I don't see uwg itself adding routes, 
do you do that outside it?


I don't need to manually add routes. If I bring the interface up without 
the IFF_POINTOPOINT flag, then as soon as I assign the address and 
netmask to the interface a route for the subnet is automatically added 
[1].


About the IFF_BROADCAST flag, I thought not setting IFF_BROADCAST would 
imply IFF_POINTOPOINT but now I see I read tun(4) the wrong way and it's 
perfectly fine to run without IFF_POINTOPOINT and without IFF_BROADCAST.


[1] https://github.com/timkuijsten/uwg/blob/master/ifn.c#L294



Re: disable the ability to change tun(4) mode from p2p to bcast and back again

2019-02-04 Thread Tim Kuijsten

On Mon, Feb 04, 2019 at 12:07:22PM +1000, David Gwynne wrote:

Currently you can change a tun interface from being point to point to
being a broadcast interface. Why?


I'm using broadcast mode in my own wireguard implementation because 
there can be more than one peer on the network:

https://github.com/timkuijsten/uwg/blob/ccd39c6a9bdf36575a3bb3db06c438a2241c1134/ifn.c#L1868

-Tim



spf walk: lookup aaaa records with "a" mechanism

2018-10-14 Thread Tim Kuijsten

Hi,

When the "a" designated sender mechanism is used in an spf txt record, 
both v4 and v6 addresses are matched according to [1], so let `smtpctl 
spf walk` resolve both A and  records.


Current output:
$ echo netsend.nl | smtpctl spf walk
80.127.135.115
80.127.98.234

Expected output:
$ echo netsend.nl | ./smtpctl spf walk
80.127.135.115
80.127.98.234
2001:981:8a34:1:80:127:135:115
2001:984:6a6f:1:468a:5bff:fed9:87

-Tim

[1] https://tools.ietf.org/html/rfc7208#section-5.3
diff --git a/usr.sbin/smtpd/spfwalk.c b/usr.sbin/smtpd/spfwalk.c
index c4ce2e3d891..22b057963f9 100644
--- a/usr.sbin/smtpd/spfwalk.c
+++ b/usr.sbin/smtpd/spfwalk.c
@@ -192,6 +192,7 @@ dispatch_txt(struct dns_rr *rr)
}
if (strncasecmp("a:", *ap, 2) == 0) {
lookup_record(T_A, *(ap) + 2, dispatch_a);
+   lookup_record(T_, *(ap) + 2, dispatch_);
continue;
}
if (strncasecmp("exists:", *ap, 7) == 0) {


Re: [patch] acme-client listen option

2017-12-06 Thread Tim Kuijsten

On Tue, Dec 05, 2017 at 01:33:23PM -0700, Theo de Raadt wrote:

>That was also the initial design with substantial priv seperation.
>It shouldn't be designed to tap another process potentially running
>with a different uid.

Not wanting to touch processes that run with different user ids, is that
in order to fully eliminate any influence from the other process/uid on
the servproc process? servproc is quite tighly pledged with "stdio
proc". Just curious.


"proc" -- as root, right?


Idd.


acme-client was designed to updates the certs.  Only that.


Updating the certs requires communication via either http or dns.


It wasn't designed to start processes and services, kill processes,
etc.


I thought spawning httpd from base was the right direction instead of 
rolling my own simple httpd.c (much comparable to the hand-rolled http.c 
client that ships with acme-client). A route that I had originally taken 
and still think might be viable.



It looks like you are trying to bring in all the heavyweight designs
of certbot.

Why do you have a problem with the little bit of shell script running
at the correct position and privilege level?

Why does it need to be integrated?


Because serving the challenges either via a webserver or via a 
dns-server is currently a hard requirement to fulfill the core service 
of updating the certs.


I understand the reason letsencrypt came into existence is the web. So 
most environments where acme-client currently is used probably already 
have a httpd running. But I suspect the demand for acme-client on 
non-webservers will rise and it will feel more like a kludge to 
configure, start and stop a webserver in those environments.


Having said that, other than "to me it doesn't feel right", the little 
line of shell-script idd isn't much of a problem. I'll leave it for now 
and we'll see if a desire other than only me might start to exist in the 
future. ;)




Re: [patch] acme-client listen option

2017-12-05 Thread Tim Kuijsten

That was also the initial design with substantial priv seperation.
It shouldn't be designed to tap another process potentially running
with a different uid.


Not wanting to touch processes that run with different user ids, is that 
in order to fully eliminate any influence from the other process/uid on 
the servproc process? servproc is quite tighly pledged with "stdio 
proc". Just curious.




[patch] acme-client listen option

2017-12-05 Thread Tim Kuijsten
-1)
+   err(EXIT_FAILURE, "fork");
+
+   if (pids[COMP_SERV] == 0) {
+   proccomp = COMP_SERV;
+   free(alts);
+   c = servproc(serv_fds[0]);
+   exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
+   }
+   }
+
+   close(serv_fds[0]);
+
/* Jail: sandbox, file-system, user. */
 
if (pledge("stdio", NULL) == -1) {
@@ -423,10 +462,15 @@ main(int argc, char *argv[])
checkexit(pids[COMP_DNS], COMP_DNS) +
checkexit(pids[COMP_REVOKE], COMP_REVOKE);
 
+   if (pids[COMP_SERV] != -1)
+   rc += checkexit(pids[COMP_SERV], COMP_SERV);
+   else
+   rc++;
+
free(alts);
return rc != COMP__MAX ? EXIT_FAILURE : (c == 2 ? EXIT_SUCCESS : 2);
 usage:
fprintf(stderr,
-   "usage: acme-client [-ADFnrv] [-f configfile] domain\n");
+   "usage: acme-client [-ADFlnrv] [-f configfile] domain\n");
return EXIT_FAILURE;
 }
Index: netproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/netproc.c,v
retrieving revision 1.14
diff -u -p -r1.14 netproc.c
--- netproc.c   27 Nov 2017 01:58:52 -  1.14
+++ netproc.c   5 Dec 2017 11:16:44 -
@@ -565,7 +565,7 @@ dofullchain(struct conn *c, const char *
  * account key information.
  */
 int
-netproc(int kfd, int afd, int Cfd, int cfd, int dfd, int rfd,
+netproc(int kfd, int afd, int Cfd, int cfd, int dfd, int rfd, int sfd,
 int newacct, int revocate, struct authority_c *authority,
 const char *const *alts,size_t altsz)
 {
@@ -737,13 +737,19 @@ netproc(int kfd, int afd, int Cfd, int c
}
 
/*
-* Write our acknowledgement that the challenges are over.
-* The challenge process will remove all of the files.
+* Write our acknowledgement that the challenges are over to both the
+* challenge process and the server process, if started.
+* The challenge process will remove all of the files and the server
+* process will stop the http server.
 */
 
if (writeop(Cfd, COMM_CHNG_OP, CHNG_STOP) <= 0)
goto out;
 
+   if (sfd != -1)
+   if (writeop(sfd, COMM_SERV_OP, SERV_STOP) <= 0)
+   goto out;
+
/* Wait to receive the certificate itself. */
 
if ((cert = readstr(kfd, COMM_CERT)) == NULL)
@@ -782,6 +788,8 @@ out:
close(Cfd);
close(dfd);
close(rfd);
+   if (sfd != -1)
+   close(sfd);
free(cert);
free(url);
free(thumb);
Index: parse.h
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.h,v
retrieving revision 1.9
diff -u -p -r1.9 parse.h
--- parse.h 27 Nov 2017 16:53:04 -  1.9
+++ parse.h 5 Dec 2017 11:16:44 -
@@ -61,6 +61,7 @@ struct keyfile {
 #define ACME_OPT_NEWACCT   0x0002
 #define ACME_OPT_NEWDKEY   0x0004
 #define ACME_OPT_CHECK 0x0008
+#define ACME_OPT_LISTEN0x0016
 
 struct acme_conf {
int  opts;
Index: servproc.c
===
RCS file: servproc.c
diff -N servproc.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ servproc.c  5 Dec 2017 11:16:44 -
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2017 Tim Kuijsten <i...@netsend.nl>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "extern.h"
+
+#define HTTPDCONF "/etc/httpd.acme.conf"
+#define HTTPDBIN "/usr/sbin/httpd"
+
+int
+servproc(int netsock)
+{
+   int rc = 0, s, fd = -1;
+   pid_t   pid;
+   charcfg[] = "server \"acmeclient\" {\n"
+   "   listen on egress port 80\n"
+   "   location \"/.well-known/acme-challenge/*\" {\n"
+   "   root \"/acme\"\n"
+   "   root strip 2\n"
+   "   }\n"
+   "}\

acme-client listen option

2017-10-01 Thread Tim Kuijsten
c = servproc(serv_fds[0]);
+   exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
+   }
+   }
+
+   close(serv_fds[0]);
+
/* Jail: sandbox, file-system, user. */
 
if (pledge("stdio", NULL) == -1) {
@@ -425,10 +464,15 @@ main(int argc, char *argv[])
checkexit(pids[COMP_DNS], COMP_DNS) +
checkexit(pids[COMP_REVOKE], COMP_REVOKE);
 
+   if (pids[COMP_SERV] != -1)
+   rc += checkexit(pids[COMP_SERV], COMP_SERV);
+   else
+   rc++;
+
free(alts);
return rc != COMP__MAX ? EXIT_FAILURE : (c == 2 ? EXIT_SUCCESS : 2);
 usage:
fprintf(stderr,
-   "usage: acme-client [-ADFnrv] [-f configfile] domain\n");
+   "usage: acme-client [-ADFlnrv] [-f configfile] domain\n");
return EXIT_FAILURE;
 }
Index: netproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/netproc.c,v
retrieving revision 1.13
diff -u -p -r1.13 netproc.c
--- netproc.c   24 Jan 2017 13:32:55 -  1.13
+++ netproc.c   1 Oct 2017 18:23:28 -
@@ -565,7 +565,7 @@ dofullchain(struct conn *c, const char *
  * account key information.
  */
 int
-netproc(int kfd, int afd, int Cfd, int cfd, int dfd, int rfd,
+netproc(int kfd, int afd, int Cfd, int cfd, int dfd, int rfd, int sfd,
 int newacct, int revocate, struct authority_c *authority,
 const char *const *alts,size_t altsz, const char *agreement)
 {
@@ -737,13 +737,19 @@ netproc(int kfd, int afd, int Cfd, int c
}
 
/*
-* Write our acknowledgement that the challenges are over.
-* The challenge process will remove all of the files.
+* Write our acknowledgement that the challenges are over to both the
+* challenge process and the server process, if started.
+* The challenge process will remove all of the files and the server 
process
+* will stop the http server.
 */
 
if (writeop(Cfd, COMM_CHNG_OP, CHNG_STOP) <= 0)
goto out;
 
+   if (sfd != -1)
+   if (writeop(sfd, COMM_SERV_OP, SERV_STOP) <= 0)
+   goto out;
+
/* Wait to receive the certificate itself. */
 
if ((cert = readstr(kfd, COMM_CERT)) == NULL)
@@ -782,6 +788,8 @@ out:
close(Cfd);
close(dfd);
close(rfd);
+   if (sfd != -1)
+   close(sfd);
free(cert);
free(url);
free(thumb);
Index: parse.h
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.h,v
retrieving revision 1.7
diff -u -p -r1.7 parse.h
--- parse.h 21 Jan 2017 12:59:06 -  1.7
+++ parse.h 1 Oct 2017 18:23:28 -
@@ -62,6 +62,7 @@ struct keyfile {
 #define ACME_OPT_NEWACCT   0x0002
 #define ACME_OPT_NEWDKEY   0x0004
 #define ACME_OPT_CHECK 0x0008
+#define ACME_OPT_LISTEN0x0016
 
 struct acme_conf {
int  opts;
Index: servproc.c
===
RCS file: servproc.c
diff -N servproc.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ servproc.c  1 Oct 2017 18:23:28 -
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2017 Tim Kuijsten <i...@netsend.nl>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "extern.h"
+
+#define HTTPDCONF "/etc/httpd.acme.conf"
+#define HTTPDBIN "/usr/sbin/httpd"
+
+int
+servproc(int netsock)
+{
+   int rc = 0, s, fd = -1;
+   pid_t   pid;
+   charcfg[] = "server \"acmeclient\" {\n"
+   "   listen on egress port 80\n"
+   "   location \"/.well-known/acme-challenge/*\" {\n"
+   "   root \"/acme\"\n"
+   "   root strip 2\n"
+   "   }\n"
+   "}\n";
+
+   if (pledge("stdio wpath cpath proc exec", NULL) == -1)
+   err(EXIT_FAILURE, "%s: pledge", __func__);
+
+   /*
+* 

plug mem leak in ldapd/btree.c

2016-11-30 Thread Tim Kuijsten

Plug a memory leak in btree_close.

From 
https://github.com/OrangeTide/btree/commit/e186331494b213286934bcc03a1d8c4650836e3b


Index: btree.c
===
RCS file: /cvs/src/usr.sbin/ldapd/btree.c,v
retrieving revision 1.36
diff -u -p -r1.36 btree.c
--- btree.c 20 Mar 2016 00:01:22 -  1.36
+++ btree.c 30 Nov 2016 21:05:14 -
@@ -1176,6 +1176,8 @@ btree_close(struct btree *bt)
DPRINTF("ref is zero, closing btree %p", bt);
close(bt->fd);
mpage_flush(bt);
+   free(bt->lru_queue);
+   free(bt->path);
free(bt->page_cache);
free(bt);
} else