subject:"net\/igmpproxy \- chroot and drop privileges"

Re: net/igmpproxy - chroot and drop privileges

2021-07-06 Thread Bjorn Ketelaars

On Fri 02/07/2021 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.
> 
> 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?

New diff attached as upstream has taken the improvement...after some
changes. Instead of enforcing priv drop, it is optional. It can be set
via the configuration file.

Attached diff:
- Takes the first free id from user.list
- Updates igmpproxy to HEAD, which is the version in ports plus the priv
  drop stuff
- Explains what to do in README
- Sets 'daemon_flags' in igmpproxy.rc

Unless there is any objection I will commit later this week.
Index: infrastructure/db/user.list
===
RCS file: /cvs/ports/infrastructure/db/user.list,v
retrieving revision 1.388
diff -u -p -r1.388 user.list
--- infrastructure/db/user.list 27 May 2021 14:51:12 -  1.388
+++ infrastructure/db/user.list 6 Jul 2021 13:57:43 -
@@ -376,3 +376,4 @@ id  usergroup   port
 865 _vger  _vger   net/vger
 866 _navidrome _navidrome  audio/navidrome
 867 _notify_push   www/nextcloud_notify_push
+868 _igmpproxy _igmpproxy  net/igmpproxy
Index: net/igmpproxy/Makefile
===
RCS file: /cvs/ports/net/igmpproxy/Makefile,v
retrieving revision 1.21
diff -u -p -r1.21 Makefile
--- net/igmpproxy/Makefile  19 Jan 2021 10:50:12 -  1.21
+++ net/igmpproxy/Makefile  6 Jul 2021 13:57:43 -
@@ -2,11 +2,12 @@
 
 COMMENT =  multicast router utilizing IGMP forwarding
 
-VERSION =  0.3
-REVISION = 0
-DISTNAME = igmpproxy-${VERSION}
+GH_ACCOUNT =   pali
+GH_PROJECT =   igmpproxy
+GH_COMMIT =0e7186b300c063ef4015f1551100765ef5537d4c
+DISTNAME = igmpproxy-0.3.20210705
+
 CATEGORIES =   net
-MASTER_SITES = https://github.com/pali/igmpproxy/releases/download/${VERSION}/
 
 HOMEPAGE = https://github.com/pali/igmpproxy/
 
@@ -15,15 +16,18 @@ PERMIT_PACKAGE =Yes
 
 WANTLIB =  c
 
-USE_GMAKE =Yes
+MAKE_FLAGS =   CFLAGS="${CFLAGS} -Wall" \
+   LDFLAGS="${LDFLAGS}"
 
-CFLAGS +=  -Wall
-MAKE_FLAGS =   LDFLAGS="${LDFLAGS}"
-DEBUG_PACKAGES = ${BUILD_PACKAGES}
+CONFIGURE_STYLE =  autoreconf
 
-NO_TEST =  Yes
+DEBUG_PACKAGES =   ${BUILD_PACKAGES}
 
-CONFIGURE_STYLE =  gnu
+AUTOCONF_VERSION = 2.63
+AUTOMAKE_VERSION = 1.12
+AUTORECONF =   ./autogen.sh
+
+NO_TEST =  Yes
 
 do-install:
${INSTALL_PROGRAM} ${WRKSRC}/src/igmpproxy ${PREFIX}/sbin
Index: net/igmpproxy/distinfo
===
RCS file: /cvs/ports/net/igmpproxy/distinfo,v
retrieving revision 1.3
diff -u -p -r1.3 distinfo
--- net/igmpproxy/distinfo  12 Jan 2021 17:59:49 -  1.3
+++ net/igmpproxy/distinfo  6 Jul 2021 13:57:43 -
@@ -1,2 +1,2 @@
-SHA256 (igmpproxy-0.3.tar.gz) = 0fwkTLL7v5n3IL2j6EH+WezptpGQc3kLS4knObG4ROs=
-SIZE (igmpproxy-0.3.tar.gz) = 168403
+SHA256 (igmpproxy-0.3.20210705-0e7186b3.tar.gz) = 
DDFIU7dWYhfJngPYrN1INdRysZqXAleYLp334ii8ofs=
+SIZE (igmpproxy-0.3.20210705-0e7186b3.tar.gz) = 43316
Index: net/igmpproxy/patches/patch-igmpproxy_conf
===
RCS file: net/igmpproxy/patches/patch-igmpproxy_conf
diff -N net/igmpproxy/patches/patch-igmpproxy_conf
--- /dev/null   1 Jan 1970 00:00:00 -
+++ net/igmpproxy/patches/patch-igmpproxy_conf  6 Jul 2021 13:57:43 -
@@ -0,0 +1,15 @@
+$OpenBSD$
+
+Index: igmpproxy.conf
+--- igmpproxy.conf.orig
 igmpproxy.conf
+@@ -18,6 +18,9 @@
+ #
+ 
+ 
++chroot /var/empty
++user _igmpproxy
++
+ ##--
+ ## Enable Quickleave mode (Sends Leave instantly)
+ ##--
Index: net/igmpproxy/patches/patch-src_config_c
===
RCS file: /cvs/ports/net/igmpproxy/patches/patch-src_config_c,v
retrieving revision 1.2
diff -u -p -r1.2 patch-src_config_c
--- net/igmpproxy/patches/patch-src_config_c19 Jan 2021 10:50:12 -  
1.2
+++ net/igmpproxy/patches/patch-src_config_c6 Jul 2021 13:57:43 -
@@ -3,7 +3,7 @@ $OpenBSD: patch-src_config_c,v 1.2 2021/
 Index: src/config.c
 --- src/config.c.orig
 +++ src/config.c
-@@ -430,3 +430,21 @@ struct SubnetList *parseSubnetAddress(char *addrstr) {
+@@ -455,3 +455,21 @@ struct Subn

Re: net/igmpproxy - chroot and drop privileges

2021-07-02 Thread Stuart Henderson

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 
>  +#include 
>  +
>   static const char Usage[] =
>   "Usage: igmpproxy [-h] [-n] [-d] [-v [-v]] \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

net/igmpproxy - chroot and drop privileges

2021-07-02 Thread Bjorn Ketelaars

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.

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?


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  usergroup   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 
 +#include 
 +
  static const char Usage[] =
  "Usage: igmpproxy [-h] [-n] [-d] [-v [-v]] \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

Re: net/igmpproxy - chroot and drop privileges

Re: net/igmpproxy - chroot and drop privileges

net/igmpproxy - chroot and drop privileges

3 matches

Site Navigation

Mail list logo

Footer information