Re: net/dictd: pledge dictd(8) server

On 2023/10/29 17:03:40 +0000, Klemens Nanni <kn@openbsd.org> wrote:
> On Sun, Oct 29, 2023 at 11:17:09AM +0000, Klemens Nanni wrote:
> > Regular mode uses "stdio rpath inet dns proc"; "proc" to fork a child
> > (no exec) unless '--debug nofork' or '--debug nodetach' is set.
> >
> > --inetd uses "stdio rpath".
> >
> > --pp requires "proc exec" to run, e.g. cpp(1) on the config file which
> > which gets reloaded on SIGHUP: started with --pp means "proc exec" stay.
> >
> >
> > "wpath cpath" are for log and/or pid files and dropped after setup.
> > "getpw id" are for intitial privilege drop and dropped after setup.
> >
> > Works nicely for me with a config like our example.
> > Tested with inetd.conf(5)
> > [::1]:2628 stream tcp6 nowait root /usr/local/sbin/dictd dictd -i
> >
> > Pledges could perhaps start tighter and avoid later dropping, but too
> > many nested `if (option-used)' levels make the code harder to follow, imho.
> >
> > Feedback? Objection? OK?
>
> Rebased diff after Makefile changes.
> Tests still pass with pledge.

I've been running it as daemon with this patch since you sent it and
still works fine here. The diff looks sensible to me, but I admit I
haven't checked in detail all the daemon code, just skimmed thru a few
interesting files and grepped for potential violation (and found none.)

ok op@

> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/net/dictd/Makefile,v
> diff -u -p -r1.22 Makefile
> --- Makefile 29 Oct 2023 16:55:58 -0000 1.22
> +++ Makefile 29 Oct 2023 17:03:00 -0000
> @@ -7,7 +7,7 @@ DISTNAME= dictd-$V
> PKGNAME-main= dictd-client-$V
> PKGNAME-server= dictd-server-$V
> REVISION-main= 3
> -REVISION-server=2
> +REVISION-server=3
>
> CATEGORIES= net education
>
> @@ -17,6 +17,8 @@ MAINTAINER= Klemens Nanni <kn@openbsd.or
>
> # GPL v2
> PERMIT_PACKAGE= Yes
> +
> +# uses pledge()
> WANTLIB= c maa z
>
> SITES= ${SITE_SOURCEFORGE:=dict/}
> Index: patches/patch-dictd_c
> ===================================================================
> RCS file: /cvs/ports/net/dictd/patches/patch-dictd_c,v
> diff -u -p -r1.1 patch-dictd_c
> --- patches/patch-dictd_c 27 Oct 2023 18:51:10 -0000 1.1
> +++ patches/patch-dictd_c 29 Oct 2023 17:03:00 -0000
> @@ -1,9 +1,18 @@
> -use dedicated _dictd user/group
> +- use dedicated _dictd user/group
> +- use pledge(2)
>
> Index: dictd.c
> --- dictd.c.orig
> +++ dictd.c
> -@@ -1269,9 +1272,9 @@ static void release_root_privileges( void )
> +@@ -37,6 +37,7 @@
> + #include <ctype.h>
> + #include <sys/stat.h>
> + #include <fcntl.h>
> ++#include <unistd.h> /* pledge */
> +
> + #define MAXPROCTITLE 2048 /* Maximum amount of proc title we'll use. */
> + #undef MIN
> +@@ -1269,9 +1270,9 @@ static void release_root_privileges( void )
> if (geteuid() == 0) {
> struct passwd *pwd;
>
> @@ -15,3 +24,67 @@ Index: dictd.c
> setuid(pwd->pw_uid);
> } else if ((pwd = getpwnam("nobody"))) {
> setgid(pwd->pw_gid);
> +@@ -1664,6 +1665,17 @@ int main (int argc, char **argv, char **envp)
> + default: help(); exit(0); break;
> + }
> +
> ++#ifdef __OpenBSD__
> ++ /* no need for "inet dns" when talking to inetd(8) over stdin/out */
> ++ if (inetd) {
> ++ if (pledge("stdio rpath wpath cpath getpw proc exec id", NULL) == -1)
> ++ err_fatal_errno(__func__, "pledge");
> ++ } else {
> ++ if (pledge("stdio rpath wpath cpath inet dns getpw proc exec id", NULL) == -1)
> ++ err_fatal_errno(__func__, "pledge");
> ++ }
> ++