On 2021/07/02 21:56, Bjorn Ketelaars wrote:
> Enclosed is a diff for net/igmpproxy, which puts igmpproxy in an
> unprivileged chroot after startup. I'm currently discussing a more
> extensive diff with upstream.
>
> We normally do not add features to our ports, but I was wondering if
> this addition makes sense to commit as it increases security a bit.
There is past history of adding privdrop to ports in some cases
and I think it is worthwhile for a root network daemon.
> Run tested on amd64 in combination with an iptv setup.
>
> While here add daemon_flags="${SYSCONFDIR}/igmpproxy.conf" to
> igmpproxy.rc as igmpproxy will complain if no configuration file is
> given.
>
> Thoughts/tests/comments/OK?
I don't think it's likely that pledge will work out without major
refactoring, so the setuid probably is the only way to remove those
privileges.
For the filesystem access side, I'm wondering if unveil might be a bit
simpler than chroot. OTOH, that might be harder to convince upstream to
add, and if there's chance that they might accept it, it would be nice
to be able to use whatever they have directly.
> diff --git infrastructure/db/user.list infrastructure/db/user.list
> index bfb3d70510e..f2eb6a60b8d 100644
> --- infrastructure/db/user.list
> +++ infrastructure/db/user.list
> @@ -376,3 +376,4 @@ id user group port
> 865 _vger _vger net/vger
> 866 _navidrome _navidrome audio/navidrome
> 867 _notify_push www/nextcloud_notify_push
> +868 _igmpproxy _igmpproxy net/igmpproxy
> diff --git net/igmpproxy/Makefile net/igmpproxy/Makefile
> index f87a2b4fc45..6d971d68749 100644
> --- net/igmpproxy/Makefile
> +++ net/igmpproxy/Makefile
> @@ -3,7 +3,7 @@
> COMMENT = multicast router utilizing IGMP forwarding
>
> VERSION = 0.3
> -REVISION = 0
> +REVISION = 1
> DISTNAME = igmpproxy-${VERSION}
> CATEGORIES = net
> MASTER_SITES =
> https://github.com/pali/igmpproxy/releases/download/${VERSION}/
> diff --git net/igmpproxy/patches/patch-src_igmpproxy_c
> net/igmpproxy/patches/patch-src_igmpproxy_c
> index 021692a14b3..a4aee9e92f0 100644
> --- net/igmpproxy/patches/patch-src_igmpproxy_c
> +++ net/igmpproxy/patches/patch-src_igmpproxy_c
> @@ -3,7 +3,7 @@ $OpenBSD: patch-src_igmpproxy_c,v 1.1 2021/01/12 17:59:49
> sthen Exp $
> Index: src/igmpproxy.c
> --- src/igmpproxy.c.orig
> +++ src/igmpproxy.c
> -@@ -37,13 +37,10 @@
> +@@ -37,13 +37,11 @@
> * February 2005 - Johnny Egeland
> */
>
> @@ -14,12 +14,23 @@ Index: src/igmpproxy.c
> -
> #include "igmpproxy.h"
>
> ++#include <pwd.h>
> +#include <sys/sysctl.h>
> +
> static const char Usage[] =
> "Usage: igmpproxy [-h] [-n] [-d] [-v [-v]] <configfile>\n"
> "\n"
> -@@ -123,6 +120,25 @@ int main( int ArgCn, char *ArgVc[] ) {
> +@@ -68,6 +66,9 @@ static int sighandled = 0;
> + #define GOT_SIGUSR1 0x04
> + #define GOT_SIGUSR2 0x08
> +
> ++#define CHROOT_DIR "/var/empty"
> ++#define NOPRIV_USER "_igmpproxy"
> ++
> + // Holds the indeces of the upstream IF...
> + int upStreamIfIdx[MAX_UPS_VIFS];
> +
> +@@ -123,6 +124,25 @@ int main( int ArgCn, char *ArgVc[] ) {
>
> openlog("igmpproxy", LOG_PID, LOG_USER);
>
> @@ -45,25 +56,42 @@ Index: src/igmpproxy.c
> // Write debug notice with file path...
> my_log(LOG_DEBUG, 0, "Searching for config file at '%s'" ,
> configFilePath);
>
> -@@ -142,16 +158,8 @@ int main( int ArgCn, char *ArgVc[] ) {
> +@@ -140,18 +160,25 @@ int main( int ArgCn, char *ArgVc[] ) {
> + break;
> + }
>
> - if ( !NotAsDaemon ) {
> +- if ( !NotAsDaemon ) {
> ++ // Drop privileges
> ++ {
> ++ struct passwd *pw;
>
> - // Only daemon goes past this line...
> - if (fork()) exit(0);
> --
> ++ pw = getpwnam(NOPRIV_USER);
> ++ if (pw == NULL)
> ++ my_log(LOG_ERR, 0, "unknown user %s", NOPRIV_USER);
> +
> - // Detach daemon from terminal
> - if ( close( 0 ) < 0 || close( 1 ) < 0 || close( 2 ) < 0
> - || open( "/dev/null", 0 ) != 0 || dup2( 0, 1 ) < 0 || dup2(
> 0, 2 ) < 0
> - || setpgid( 0, 0 ) < 0
> - ) {
> ++ if (chroot(CHROOT_DIR) != 0 || chdir("/") != 0 ||
> ++ setgroups(1, &pw->pw_gid) != 0 ||
> ++ setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0 ||
> ++ setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) != 0)
> ++ my_log(LOG_ERR, 0, "cannot drop privileges");
> ++ }
> ++
> ++ if ( !NotAsDaemon ) {
> ++
> + if ( daemon(1, 0 ) < 0 )
> my_log( LOG_ERR, errno, "failed to detach daemon" );
> - }
> }
>
> // Go to the main loop.
> -@@ -207,6 +215,8 @@ int igmpProxyInit(void) {
> +@@ -207,6 +234,8 @@ int igmpProxyInit(void) {
> }
>
> for ( Ix = 0; (Dp = getIfByIx(Ix)); Ix++ ) {
> diff --git net/igmpproxy/pkg/PLIST net/igmpproxy/pkg/PLIST
> index 1cbe5a08909..32ebfcdd078 100644
> --- net/igmpproxy/pkg/PLIST
> +++ net/igmpproxy/pkg/PLIST
> @@ -1,4 +1,6 @@
> @comment $OpenBSD: PLIST,v 1.5 2021/01/12 17:59:50 sthen Exp $
> +@newgroup _igmpproxy:868
> +@newuser _igmpproxy:868:868:daemon:IGMP multicast routing
> daemon:/var/empty:/sbin/nologin
> @rcscript ${RCDIR}/igmpproxy
> @man man/man5/igmpproxy.conf.5
> @man man/man8/igmpproxy.8
> diff --git net/igmpproxy/pkg/igmpproxy.rc net/igmpproxy/pkg/igmpproxy.rc
> index f4366b88351..3718a74d8a2 100644
> --- net/igmpproxy/pkg/igmpproxy.rc
> +++ net/igmpproxy/pkg/igmpproxy.rc
> @@ -3,6 +3,7 @@
> # $OpenBSD: igmpproxy.rc,v 1.2 2018/01/11 19:27:05 rpe Exp $
>
> daemon="${TRUEPREFIX}/sbin/igmpproxy"
> +daemon_flags="${SYSCONFDIR}/igmpproxy.conf"
>
> . /etc/rc.d/rc.subr
>
>
