Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
Thanks, fixed. On 2023/11/20 22:06, Alexander Bluhm wrote: > This breaks iperf3 in my setup. > > root@ot50:.../~# iperf3 -sD > Abort trap (core dumped) > > iperf3[72726]: pledge "proc", syscall 2 > > Program terminated with signal SIGABRT, Aborted. > #0 _thread_sys_fork () at /tmp/-:2 > 2 /tmp/-: No such file or directory. > (gdb) bt > #0 _thread_sys_fork () at /tmp/-:2 > #1 0x6b01f3ff9bf18acf in ?? () > #2 0x04b673981f86 in daemon (nochdir=0, noclose=0) > at /usr/src/lib/libc/gen/daemon.c:41 > #3 0x04b3c8078629 in ?? () > #4 0x04b3c8078423 in ?? () > #5 0x04b3c8078131 in ?? () > #6 0x in ?? () > > Pledge should be done after initialization, but before running phase. > Call it after daemon(3). > > bluhm > > On Sat, Oct 21, 2023 at 07:28:06PM +0100, Stuart Henderson wrote: > > ...also as was as syscalls, socket options could do with checking over too. > > > > If everything is in order then there's not much point adding a configure > > flag really, just check for pledge > > > > -- > > Sent from a phone, apologies for poor formatting. > > > > On 21 October 2023 19:01:33 Stuart Henderson wrote: > > > > > It hasn't been properly reviewed to check if there are any syscalls which > > > aren't covered by the pledge. I found the diskfile one which you missed, > > > but haven't checked over nm output to look for more. > > > > > > -- > > > Sent from a phone, apologies for poor formatting. > > > > > > On 21 October 2023 18:57:55 Mikhail wrote: > > > > > >> On Sat, Oct 21, 2023 at 06:38:57PM +0100, Stuart Henderson wrote: > > >>> Err, sending that upstream is a bit premature. > > >> > > >> Reasons? It works fine in my testing, also it's enabled only with > > >> --enable-openbsd-sandbox, so if something arise it's very easy for the > > >> users to check without this code. And during review the devs can point > > >> to improvements. > > >> > > >> I can close the PR, it's not a problem. >
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
This breaks iperf3 in my setup. root@ot50:.../~# iperf3 -sD Abort trap (core dumped) iperf3[72726]: pledge "proc", syscall 2 Program terminated with signal SIGABRT, Aborted. #0 _thread_sys_fork () at /tmp/-:2 2 /tmp/-: No such file or directory. (gdb) bt #0 _thread_sys_fork () at /tmp/-:2 #1 0x6b01f3ff9bf18acf in ?? () #2 0x04b673981f86 in daemon (nochdir=0, noclose=0) at /usr/src/lib/libc/gen/daemon.c:41 #3 0x04b3c8078629 in ?? () #4 0x04b3c8078423 in ?? () #5 0x04b3c8078131 in ?? () #6 0x in ?? () Pledge should be done after initialization, but before running phase. Call it after daemon(3). bluhm On Sat, Oct 21, 2023 at 07:28:06PM +0100, Stuart Henderson wrote: > ...also as was as syscalls, socket options could do with checking over too. > > If everything is in order then there's not much point adding a configure > flag really, just check for pledge > > -- > Sent from a phone, apologies for poor formatting. > > On 21 October 2023 19:01:33 Stuart Henderson wrote: > > > It hasn't been properly reviewed to check if there are any syscalls which > > aren't covered by the pledge. I found the diskfile one which you missed, > > but haven't checked over nm output to look for more. > > > > -- > > Sent from a phone, apologies for poor formatting. > > > > On 21 October 2023 18:57:55 Mikhail wrote: > > > >> On Sat, Oct 21, 2023 at 06:38:57PM +0100, Stuart Henderson wrote: > >>> Err, sending that upstream is a bit premature. > >> > >> Reasons? It works fine in my testing, also it's enabled only with > >> --enable-openbsd-sandbox, so if something arise it's very easy for the > >> users to check without this code. And during review the devs can point > >> to improvements. > >> > >> I can close the PR, it's not a problem.
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
...also as was as syscalls, socket options could do with checking over too. If everything is in order then there's not much point adding a configure flag really, just check for pledge -- Sent from a phone, apologies for poor formatting. On 21 October 2023 19:01:33 Stuart Henderson wrote: It hasn't been properly reviewed to check if there are any syscalls which aren't covered by the pledge. I found the diskfile one which you missed, but haven't checked over nm output to look for more. -- Sent from a phone, apologies for poor formatting. On 21 October 2023 18:57:55 Mikhail wrote: On Sat, Oct 21, 2023 at 06:38:57PM +0100, Stuart Henderson wrote: Err, sending that upstream is a bit premature. Reasons? It works fine in my testing, also it's enabled only with --enable-openbsd-sandbox, so if something arise it's very easy for the users to check without this code. And during review the devs can point to improvements. I can close the PR, it's not a problem.
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
It hasn't been properly reviewed to check if there are any syscalls which aren't covered by the pledge. I found the diskfile one which you missed, but haven't checked over nm output to look for more. -- Sent from a phone, apologies for poor formatting. On 21 October 2023 18:57:55 Mikhail wrote: On Sat, Oct 21, 2023 at 06:38:57PM +0100, Stuart Henderson wrote: Err, sending that upstream is a bit premature. Reasons? It works fine in my testing, also it's enabled only with --enable-openbsd-sandbox, so if something arise it's very easy for the users to check without this code. And during review the devs can point to improvements. I can close the PR, it's not a problem.
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
On Sat, Oct 21, 2023 at 08:57:33PM +0300, Mikhail wrote: > On Sat, Oct 21, 2023 at 06:38:57PM +0100, Stuart Henderson wrote: > > Err, sending that upstream is a bit premature. > > Reasons? It works fine in my testing, also it's enabled only with > --enable-openbsd-sandbox, so if something arise it's very easy for the > users to check without this code. And during review the devs can point > to improvements. > > I can close the PR, it's not a problem. Ok, as always good thought comes last - openbsd users will use the port version anyway...
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
On Sat, Oct 21, 2023 at 06:38:57PM +0100, Stuart Henderson wrote: > Err, sending that upstream is a bit premature. Reasons? It works fine in my testing, also it's enabled only with --enable-openbsd-sandbox, so if something arise it's very easy for the users to check without this code. And during review the devs can point to improvements. I can close the PR, it's not a problem.
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
Err, sending that upstream is a bit premature. -- Sent from a phone, apologies for poor formatting. On 21 October 2023 15:53:34 Mikhail wrote: On Wed, Oct 18, 2023 at 10:28:47PM +0100, Stuart Henderson wrote: On 2023/10/18 22:19, Mikhail wrote: > [cc'ing maintainer] > > Inlined patch updates iperf3 to 3.15 (3 bug fixes, details here - > https://github.com/esnet/iperf/releases/tag/3.15). > > I run iperf on public server with unfirewalled ports, so I'd like it to > be pledged/unveiled, -I and --logfile options are working fine. > > Probably we could drop privs more granularly, but for I'd like to keep > things simple. > Diff below for a few things I noticed. There may be others. Here is a try to push this upstream: https://github.com/esnet/iperf/pull/1586 Worth waiting a little bit for their comments.
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
On Wed, Oct 18, 2023 at 10:28:47PM +0100, Stuart Henderson wrote: > On 2023/10/18 22:19, Mikhail wrote: > > [cc'ing maintainer] > > > > Inlined patch updates iperf3 to 3.15 (3 bug fixes, details here - > > https://github.com/esnet/iperf/releases/tag/3.15). > > > > I run iperf on public server with unfirewalled ports, so I'd like it to > > be pledged/unveiled, -I and --logfile options are working fine. > > > > Probably we could drop privs more granularly, but for I'd like to keep > > things simple. > > > > Diff below for a few things I noticed. There may be others. Here is a try to push this upstream: https://github.com/esnet/iperf/pull/1586 Worth waiting a little bit for their comments.
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
lteo, are you still interested in maintaining iperf3?
On 2023/10/18 22:28, Stuart Henderson wrote:
> On 2023/10/18 22:19, Mikhail wrote:
> > [cc'ing maintainer]
> >
> > Inlined patch updates iperf3 to 3.15 (3 bug fixes, details here -
> > https://github.com/esnet/iperf/releases/tag/3.15).
> >
> > I run iperf on public server with unfirewalled ports, so I'd like it to
> > be pledged/unveiled, -I and --logfile options are working fine.
> >
> > Probably we could drop privs more granularly, but for I'd like to keep
> > things simple.
> >
>
> Diff below for a few things I noticed. There may be others.
>
> Since you only unveil /dev/urandom (plus the extra paths allowed
> by tmppath and for dns access) you can add a small patch to use
> arc4random and drop rpath and /dev/urandom access in most cases.
>
> You don't handle --file. This needs cpath wpath for server, or
> rpath for client. I just added to the existing needwr mechanism
> rather than separating the two. This could possibly be more
> granular but there are probably complications with --bidir and
> --reverse.
>
> Rather than locking unveil for the needwr case, I called pledge
> again with unveil removed, I think this makes it a little more
> clear what the final pledges are for the two cases. (We could
> also drop the early pledge before parsing options, I don't think
> it adds much in this case, and pledges are often done like that,
> though I've left it for now).
>
> Couple of other minor tweaks (error message fixes, don't mix
> definitions and code,
>
> I've tested various options including --tos without problems,
> but not exhaustively.
>
>
> Index: Makefile
> ===
> RCS file: /cvs/ports/net/iperf3/Makefile,v
> retrieving revision 1.14
> diff -u -p -r1.14 Makefile
> --- Makefile 27 Sep 2023 14:18:10 - 1.14
> +++ Makefile 18 Oct 2023 21:03:42 -
> @@ -1,6 +1,6 @@
> COMMENT= tool to measure maximum achievable bandwidth on IP networks
>
> -V= 3.14
> +V= 3.15
> PKGNAME= iperf3-${V}
> DISTNAME=iperf-${V}
>
> @@ -15,6 +15,7 @@ MAINTAINER= Lawrence Teo # BSD 3-clause
> PERMIT_PACKAGE= Yes
>
> +# uses pledge unveil
> WANTLIB += c m
>
> SITES= https://downloads.es.net/pub/iperf/
> Index: distinfo
> ===
> RCS file: /cvs/ports/net/iperf3/distinfo,v
> retrieving revision 1.9
> diff -u -p -r1.9 distinfo
> --- distinfo 3 Aug 2023 14:32:28 - 1.9
> +++ distinfo 18 Oct 2023 21:03:42 -
> @@ -1,2 +1,2 @@
> -SHA256 (iperf-3.14.tar.gz) = cj/MQwoCe8aVJij6KjrHdYSh0L0ygnXlc/ybIGwVUAQ=
> -SIZE (iperf-3.14.tar.gz) = 647944
> +SHA256 (iperf-3.15.tar.gz) = vbd8EfcrzpAhSIMVlXf6JEEgE+YrIIPPX1Q5HXmx2P8=
> +SIZE (iperf-3.15.tar.gz) = 649330
> Index: patches/patch-src_iperf_api_c
> ===
> RCS file: /cvs/ports/net/iperf3/patches/patch-src_iperf_api_c,v
> retrieving revision 1.8
> diff -u -p -r1.8 patch-src_iperf_api_c
> --- patches/patch-src_iperf_api_c 3 Aug 2023 14:32:28 - 1.8
> +++ patches/patch-src_iperf_api_c 18 Oct 2023 21:03:42 -
> @@ -3,7 +3,7 @@ Default to IPv4.
> Index: src/iperf_api.c
> --- src/iperf_api.c.orig
> +++ src/iperf_api.c
> -@@ -2860,7 +2860,7 @@ iperf_defaults(struct iperf_test *testp)
> +@@ -2884,7 +2884,7 @@ iperf_defaults(struct iperf_test *testp)
> testp->stats_interval = testp->reporter_interval = 1;
> testp->num_streams = 1;
>
> Index: patches/patch-src_iperf_util_c
> ===
> RCS file: patches/patch-src_iperf_util_c
> diff -N patches/patch-src_iperf_util_c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-src_iperf_util_c18 Oct 2023 21:03:42 -
> @@ -0,0 +1,21 @@
> +Index: src/iperf_util.c
> +--- src/iperf_util.c.orig
> src/iperf_util.c
> +@@ -57,6 +57,9 @@
> + */
> + int readentropy(void *out, size_t outsize)
> + {
> ++#if defined(__OpenBSD__)
> ++arc4random_buf(out, outsize);
> ++#else
> + static FILE *frandom;
> + static const char rndfile[] = "/dev/urandom";
> +
> +@@ -75,6 +78,7 @@ int readentropy(void *out, size_t outsize)
> + rndfile,
> + feof(frandom) ? "EOF" : strerror(errno));
> + }
> ++#endif
> + return 0;
> + }
> +
> Index: patches/patch-src_main_c
> ===
> RCS file: patches/patch-src_main_c
> diff -N patches/patch-src_main_c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-src_main_c 18 Oct 2023 21:03:42 -
> @@ -0,0 +1,67 @@
> +Add pledge and unveil
> +
> +Index: src/main.c
> +--- src/main.c.orig
> src/main.c
> +@@ -59,6 +59,15 @@ main(int argc, char **argv)
> + {
> + struct iperf_test *test;
> +
> ++#if defined(__OpenBSD__)
> ++int needwr = 0;
> ++
> ++if
Re: [update, add pledge/unveil] net/iperf3 3.14 -> 3.15
On 2023/10/18 22:19, Mikhail wrote:
> [cc'ing maintainer]
>
> Inlined patch updates iperf3 to 3.15 (3 bug fixes, details here -
> https://github.com/esnet/iperf/releases/tag/3.15).
>
> I run iperf on public server with unfirewalled ports, so I'd like it to
> be pledged/unveiled, -I and --logfile options are working fine.
>
> Probably we could drop privs more granularly, but for I'd like to keep
> things simple.
>
Diff below for a few things I noticed. There may be others.
Since you only unveil /dev/urandom (plus the extra paths allowed
by tmppath and for dns access) you can add a small patch to use
arc4random and drop rpath and /dev/urandom access in most cases.
You don't handle --file. This needs cpath wpath for server, or
rpath for client. I just added to the existing needwr mechanism
rather than separating the two. This could possibly be more
granular but there are probably complications with --bidir and
--reverse.
Rather than locking unveil for the needwr case, I called pledge
again with unveil removed, I think this makes it a little more
clear what the final pledges are for the two cases. (We could
also drop the early pledge before parsing options, I don't think
it adds much in this case, and pledges are often done like that,
though I've left it for now).
Couple of other minor tweaks (error message fixes, don't mix
definitions and code,
I've tested various options including --tos without problems,
but not exhaustively.
Index: Makefile
===
RCS file: /cvs/ports/net/iperf3/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- Makefile27 Sep 2023 14:18:10 - 1.14
+++ Makefile18 Oct 2023 21:03:42 -
@@ -1,6 +1,6 @@
COMMENT= tool to measure maximum achievable bandwidth on IP networks
-V= 3.14
+V= 3.15
PKGNAME= iperf3-${V}
DISTNAME= iperf-${V}
@@ -15,6 +15,7 @@ MAINTAINER= Lawrence Teo https://downloads.es.net/pub/iperf/
Index: distinfo
===
RCS file: /cvs/ports/net/iperf3/distinfo,v
retrieving revision 1.9
diff -u -p -r1.9 distinfo
--- distinfo3 Aug 2023 14:32:28 - 1.9
+++ distinfo18 Oct 2023 21:03:42 -
@@ -1,2 +1,2 @@
-SHA256 (iperf-3.14.tar.gz) = cj/MQwoCe8aVJij6KjrHdYSh0L0ygnXlc/ybIGwVUAQ=
-SIZE (iperf-3.14.tar.gz) = 647944
+SHA256 (iperf-3.15.tar.gz) = vbd8EfcrzpAhSIMVlXf6JEEgE+YrIIPPX1Q5HXmx2P8=
+SIZE (iperf-3.15.tar.gz) = 649330
Index: patches/patch-src_iperf_api_c
===
RCS file: /cvs/ports/net/iperf3/patches/patch-src_iperf_api_c,v
retrieving revision 1.8
diff -u -p -r1.8 patch-src_iperf_api_c
--- patches/patch-src_iperf_api_c 3 Aug 2023 14:32:28 - 1.8
+++ patches/patch-src_iperf_api_c 18 Oct 2023 21:03:42 -
@@ -3,7 +3,7 @@ Default to IPv4.
Index: src/iperf_api.c
--- src/iperf_api.c.orig
+++ src/iperf_api.c
-@@ -2860,7 +2860,7 @@ iperf_defaults(struct iperf_test *testp)
+@@ -2884,7 +2884,7 @@ iperf_defaults(struct iperf_test *testp)
testp->stats_interval = testp->reporter_interval = 1;
testp->num_streams = 1;
Index: patches/patch-src_iperf_util_c
===
RCS file: patches/patch-src_iperf_util_c
diff -N patches/patch-src_iperf_util_c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ patches/patch-src_iperf_util_c 18 Oct 2023 21:03:42 -
@@ -0,0 +1,21 @@
+Index: src/iperf_util.c
+--- src/iperf_util.c.orig
src/iperf_util.c
+@@ -57,6 +57,9 @@
+ */
+ int readentropy(void *out, size_t outsize)
+ {
++#if defined(__OpenBSD__)
++arc4random_buf(out, outsize);
++#else
+ static FILE *frandom;
+ static const char rndfile[] = "/dev/urandom";
+
+@@ -75,6 +78,7 @@ int readentropy(void *out, size_t outsize)
+ rndfile,
+ feof(frandom) ? "EOF" : strerror(errno));
+ }
++#endif
+ return 0;
+ }
+
Index: patches/patch-src_main_c
===
RCS file: patches/patch-src_main_c
diff -N patches/patch-src_main_c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ patches/patch-src_main_c18 Oct 2023 21:03:42 -
@@ -0,0 +1,67 @@
+Add pledge and unveil
+
+Index: src/main.c
+--- src/main.c.orig
src/main.c
+@@ -59,6 +59,15 @@ main(int argc, char **argv)
+ {
+ struct iperf_test *test;
+
++#if defined(__OpenBSD__)
++int needwr = 0;
++
++if (pledge("stdio tmppath rpath cpath wpath inet dns unveil", NULL) ==
-1) {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++}
++#endif
++
+ // XXX: Setting the process affinity requires root on most systems.
+ // Is this a feature we really need?
+ #ifdef TEST_PROC_AFFINITY
+@@ -104,6 +113,45 @@ main(int argc, char **argv)
+ usage();
+ exit(1);
+ }
++
++#if defined(__OpenBSD__)
++
[update, add pledge/unveil] net/iperf3 3.14 -> 3.15
[cc'ing maintainer]
Inlined patch updates iperf3 to 3.15 (3 bug fixes, details here -
https://github.com/esnet/iperf/releases/tag/3.15).
I run iperf on public server with unfirewalled ports, so I'd like it to
be pledged/unveiled, -I and --logfile options are working fine.
Probably we could drop privs more granularly, but for I'd like to keep
things simple.
diff refs/heads/master refs/heads/iperf3
commit - de754ab24f5686d70c44225d7fe12704063ff1de
commit + 5bdda7a75b52879eba6e0f671c5f3c95701254f7
blob - 1c29e29d94370a9345b58f34cdd29525f0fb9e53
blob + 1693ea4aab9e9051e306c4069736b501aa811193
--- net/iperf3/Makefile
+++ net/iperf3/Makefile
@@ -1,6 +1,6 @@
COMMENT= tool to measure maximum achievable bandwidth on IP networks
-V= 3.14
+V= 3.15
PKGNAME= iperf3-${V}
DISTNAME= iperf-${V}
@@ -15,6 +15,7 @@ MAINTAINER= Lawrence Teo
# BSD 3-clause
PERMIT_PACKAGE=Yes
+# uses pledge unveil
WANTLIB += c m
SITES= https://downloads.es.net/pub/iperf/
blob - aee4720c9e7a7c01d458ad75fa6ffacc4f3c5bcc
blob + 5e78d43fb08210550fb72de43ea73eb845202b01
--- net/iperf3/distinfo
+++ net/iperf3/distinfo
@@ -1,2 +1,2 @@
-SHA256 (iperf-3.14.tar.gz) = cj/MQwoCe8aVJij6KjrHdYSh0L0ygnXlc/ybIGwVUAQ=
-SIZE (iperf-3.14.tar.gz) = 647944
+SHA256 (iperf-3.15.tar.gz) = vbd8EfcrzpAhSIMVlXf6JEEgE+YrIIPPX1Q5HXmx2P8=
+SIZE (iperf-3.15.tar.gz) = 649330
blob - b14f7a1e19110d400b65c7d78c413e1d1136b36d
blob + ed586e35ac74ce8929882aac0e6c4c619fbebca9
--- net/iperf3/patches/patch-src_iperf_api_c
+++ net/iperf3/patches/patch-src_iperf_api_c
@@ -3,7 +3,7 @@ Default to IPv4.
Index: src/iperf_api.c
--- src/iperf_api.c.orig
+++ src/iperf_api.c
-@@ -2860,7 +2860,7 @@ iperf_defaults(struct iperf_test *testp)
+@@ -2884,7 +2884,7 @@ iperf_defaults(struct iperf_test *testp)
testp->stats_interval = testp->reporter_interval = 1;
testp->num_streams = 1;
blob - /dev/null
blob + 0596ef57b03120c490a2cac6fafbd8e529f03807 (mode 644)
--- /dev/null
+++ net/iperf3/patches/patch-src_main_c
@@ -0,0 +1,65 @@
+Add pledge and unveil
+
+Index: src/main.c
+--- src/main.c.orig
src/main.c
+@@ -59,6 +59,18 @@ main(int argc, char **argv)
+ {
+ struct iperf_test *test;
+
++#if defined(__OpenBSD__)
++if (pledge("stdio tmppath rpath cpath wpath inet unveil", NULL) == -1) {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++}
++
++if (unveil("/dev/urandom", "r") == -1) {
++ fprintf(stderr, "unveil urandom: %s\n", strerror(errno));
++ exit(1);
++}
++#endif
++
+ // XXX: Setting the process affinity requires root on most systems.
+ // Is this a feature we really need?
+ #ifdef TEST_PROC_AFFINITY
+@@ -104,6 +116,40 @@ main(int argc, char **argv)
+ usage();
+ exit(1);
+ }
++
++#if defined(__OpenBSD__)
++int needwr = 0;
++
++/* Check for the features which require wpath and cpath */
++if (test->pidfile) {
++ if (unveil(test->pidfile, "cw") == -1) {
++ fprintf(stderr, "uneveil pidfile: %s\n", strerror(errno));
++ exit(1);
++ } else
++ needwr = 1;
++}
++
++if (test->logfile) {
++ if (unveil(test->logfile, "cwr") == -1) {
++ fprintf(stderr, "uneveil logfile: %s\n", strerror(errno));
++ exit(1);
++ } else
++ needwr = 1;
++}
++
++/* Drop wpath and cpath if we can */
++if (needwr == 0) {
++ if (pledge("stdio tmppath rpath inet unveil", NULL) == -1) {
++ fprintf(stderr, "pledge needwr: %s\n", strerror(errno));
++ exit(1);
++ }
++}
++
++if (unveil(NULL, NULL) == -1) {
++ fprintf(stderr, "unveil NULL: %s\n", strerror(errno));
++ exit(1);
++}
++#endif
+
+ if (run(test) < 0)
+ iperf_errexit(test, "error - %s", iperf_strerror(i_errno));
