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 -0000 1.14
> +++ Makefile 18 Oct 2023 21:03:42 -0000
> @@ -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 <lteo@openbsd.o
> # 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 -0000 1.9
> +++ distinfo 18 Oct 2023 21:03:42 -0000
> @@ -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 -0000 1.8
> +++ patches/patch-src_iperf_api_c 18 Oct 2023 21:03:42 -0000
> @@ -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 -0000
> +++ patches/patch-src_iperf_util_c 18 Oct 2023 21:03:42 -0000
> @@ -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 -0000
> +++ patches/patch-src_main_c 18 Oct 2023 21:03:42 -0000
> @@ -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__)
> ++ /* Check for options which require filesystem access */
> ++ if (test->diskfile_name) {
> ++ if (unveil(test->diskfile_name, "crw") == -1) {
> ++ /* XXX read for client, write for server */
> ++ fprintf(stderr, "unveil diskfile: %s\n", strerror(errno));
> ++ exit(1);
> ++ }
> ++ needwr = 1;
> ++ }
> ++
> ++ if (test->pidfile) {
> ++ if (unveil(test->pidfile, "cw") == -1) {
> ++ fprintf(stderr, "unveil pidfile: %s\n", strerror(errno));
> ++ exit(1);
> ++ }
> ++ needwr = 1;
> ++ }
> ++
> ++ if (test->logfile) {
> ++ if (unveil(test->logfile, "cwr") == -1) {
> ++ fprintf(stderr, "unveil logfile: %s\n", strerror(errno));
> ++ exit(1);
> ++ }
> ++ needwr = 1;
> ++ }
> ++
> ++ /* Drop filesystem access if we can, otherwise lock unveil */
> ++ if (needwr == 0) {
> ++ if (pledge("stdio tmppath inet dns", NULL) == -1) {
> ++ fprintf(stderr, "pledge !needwr: %s\n", strerror(errno));
> ++ exit(1);
> ++ }
> ++ } else if (pledge("stdio tmppath rpath cpath wpath inet dns", NULL) ==
> -1) {
> ++ fprintf(stderr, "pledge: %s\n", strerror(errno));
> ++ exit(1);
> ++ }
> ++#endif
> +
> + if (run(test) < 0)
> + iperf_errexit(test, "error - %s", iperf_strerror(i_errno));
>
