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));

Reply via email to