On Tue, Feb 11, 2020 at 11:45:44AM +0100, Michael wrote: > Hello Matthias, > > On Mon, Feb 10, 2020 at 02:43:05PM +0100, Matthias Schmidt wrote: > > Hi Michael, > > > > On 10.02.2020 14:31, Michael wrote: > > > On Fri, Feb 07, 2020 at 03:27:33PM +0100, Michael wrote: > > > > Hello ports@, > > > > > > > > this patch adds pledge() to net/ngircd. Tested on amd64 with ngircd > > > > running with TLS. Unfortunately the promises can't be further reduced > > > > since this would break /rehash (i.e. reloading the config) later. But > > > > this is better than nothing. > > > > > > > > [...] > > > > > > solene@ pointed out that if the option "PidFile" is being used > > > unlink()ing the file later fails. However I personally don't like adding > > > another promise just for that. I can't see any sensible use case for > > > ngircds PID file; the Option itself is not set by default. > > > > > > So my idea would be to either skip or remove the PidFile code, or just > > > ignore the issue. The abort happens after shutting everything else down > > > and starting ngircd again works even if the old PID file is still in > > > place. Both variants mean changing or breaking functionality but that > > > would be bearable given the low impact IMHO. Using unveil() might also > > > be an option. > > > > > > Any thoughts on this? > > > > Active ngircd user here. I personally use the PID file with my monitoring > > system > > for process supervision (monit in my case). Although I could use process > > name > > matching, getting the PID from the PIDFile seems more natural. > > > > Cheers > > > > Matthias > > > > PS: I would appreciate a pledged ngircd very much. > > > > Yes, pledge() for ngircd is long overdue :) Below is an updated patch > that handles having PidFile configured. I am unsure if details like > having PidFile set enables the "cpath" promise should be documented or > not. > > Index: Makefile > =================================================================== > RCS file: /cvs/ports/net/ngircd/Makefile,v > retrieving revision 1.18 > diff -u -p -u -p -r1.18 Makefile > --- Makefile 12 Jul 2019 20:48:34 -0000 1.18 > +++ Makefile 11 Feb 2020 10:24:57 -0000 > @@ -4,6 +4,8 @@ COMMENT = lightweight irc server > > DISTNAME = ngircd-25 > > +REVISION = 0 > + > CATEGORIES = net > > HOMEPAGE = https://ngircd.barton.de/ > Index: patches/patch-src_ngircd_ngircd_c > =================================================================== > RCS file: /cvs/ports/net/ngircd/patches/patch-src_ngircd_ngircd_c,v > retrieving revision 1.4 > diff -u -p -u -p -r1.4 patch-src_ngircd_ngircd_c > --- patches/patch-src_ngircd_ngircd_c 3 Dec 2014 10:32:18 -0000 1.4 > +++ patches/patch-src_ngircd_ngircd_c 11 Feb 2020 10:24:57 -0000 > @@ -1,7 +1,25 @@ > $OpenBSD: patch-src_ngircd_ngircd_c,v 1.4 2014/12/03 10:32:18 jasper Exp $ > ---- src/ngircd/ngircd.c.orig Mon Jul 14 13:26:07 2014 > -+++ src/ngircd/ngircd.c Tue Dec 2 20:05:31 2014 > -@@ -563,7 +563,7 @@ Setup_FDStreams(int fd) > +Index: src/ngircd/ngircd.c > +--- src/ngircd/ngircd.c.orig > ++++ src/ngircd/ngircd.c > +@@ -259,6 +259,16 @@ main(int argc, const char *argv[]) > + exit(1); > + } > + > ++ /* XXX using a PID file needs cpath to unlink() later */ > ++ if(Conf_PidFile[0]) { > ++ if ( pledge("stdio inet dns rpath proc getpw cpath", > NULL) == -1) > ++ err(1, "pledge"); > ++ } > ++ else { > ++ if ( pledge("stdio inet dns rpath proc getpw", NULL) == > -1) > ++ err(1, "pledge"); > ++ } > ++ > + /* Initialize modules, part II: these functions are eventually > + * called with already dropped privileges ... */ > + Channel_Init(); > +@@ -563,7 +573,7 @@ Setup_FDStreams(int fd) > #if !defined(SINGLE_USER_OS) > > /** > @@ -10,7 +28,7 @@ $OpenBSD: patch-src_ngircd_ngircd_c,v 1. > * > * @param uid User ID > * @param gid Group ID > -@@ -587,7 +587,7 @@ NGIRCd_getNobodyID(uid_t *uid, gid_t *gid ) > +@@ -587,7 +597,7 @@ NGIRCd_getNobodyID(uid_t *uid, gid_t *gid ) > } > #endif > > @@ -19,7 +37,7 @@ $OpenBSD: patch-src_ngircd_ngircd_c,v 1. > if (!pwd) > return false; > > -@@ -703,11 +703,11 @@ NGIRCd_Init(bool NGIRCd_NoDaemon) > +@@ -703,11 +713,11 @@ NGIRCd_Init(bool NGIRCd_NoDaemon) > if (Conf_UID == 0) { > pwd = getpwuid(0); > Log(LOG_INFO, > Index: patches/patch-src_ngircd_proc_c > =================================================================== > RCS file: patches/patch-src_ngircd_proc_c > diff -N patches/patch-src_ngircd_proc_c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-src_ngircd_proc_c 11 Feb 2020 10:24:57 -0000 > @@ -0,0 +1,15 @@ > +$OpenBSD$ > + > +Index: src/ngircd/proc.c > +--- src/ngircd/proc.c.orig > ++++ src/ngircd/proc.c > +@@ -76,6 +76,9 @@ Proc_Fork(PROC_STAT *proc, int *pipefds, void (*cbfunc > + return -1; > + case 0: > + /* New child process: */ > ++ /* XXX no PAM, fork only for DNS */ > ++ if (pledge("stdio dns", NULL) == -1) > ++ err(1, "pledge"); > + #ifdef HAVE_ARC4RANDOM_STIR > + arc4random_stir(); > + #endif >
I'm ok with the last patch, it's harmless (no special configuration required) and will improve security. I CC maintainer, it's up to tsg now
