On Tue, Jun 11, 2019 at 08:59:55AM +0200, Solene Rapenne wrote:
> On Fri, Jun 07, 2019 at 11:41:14PM +0200, Solene Rapenne wrote:
> > On Fri, Jun 07, 2019 at 07:25:45PM +0100, Stuart Henderson wrote:
> > > On 2019/06/07 19:05, Solene Rapenne wrote:
> > > > Hi,
> > > >
> > > > This is a first draft to add pledge and unveil to net/irssi.
> > > >
> > > > About the Makefile, I added PORTHOME=${WRKDIST} so "make test" run with
> > > > 100% of success.
> > > >
> > > > The current implementation of pledge/unveil is under a #ifdef
> > > > HAVE_PLEDGE so I defined it there.
> > >
> > > Nothing is calling pledge_init (I noticed because there's no "p" flag
> > > showing in ps, and to be honest also because it didn't crash like I was
> > > expecting it to with my configuration ;-)
> >
> > one patch was missing :(
> >
> > full patch below, I reapplied it on a fresh net/irssi folder and I can
> > see the "p" state in ps
> >
> > The security features can be disabled (a bit of work is required to
> > disable the pledge in net-nonblock.c (you should already have it enabled
> > when you were connected)) so people requiring plugins not working can
> > still use irssi in the current state. /bin/ , /usr/bin/ and
> > /usr/local/bin could be added to unveil I think. My main goal was to
> > prevent irssi to write scripts and exec them, with the current unveil
> > from this patch, irssi can't write under ~/.irssi/scripts but can exec
> > them.
> >
>
> I reworked the patch a bit. I removed HAVE_PLEDGE and made a function to
> get if user want to disable that feature or not, so I can use it in
> other files, like in net-nonblock.c, when you use -u then all
> pledge/unveil calls are skipped correctly.
>
> When not disabled, it will show a message in irssi log (easy to view it
> with irssi -! to disable autoconnect).
>
> irssi doesn't exec plugins but load them, so the x flag is useless for
> the scripts directory. unveil is still useful there because it sets the
> filesystem scope of plugins, you cant' do `cat /something` from a perl
> script nor you can do open(IN,'</home/solene/.id_rsa').
>
> I'm not sure keeping the unveil call for ~/irclogs is a good idea, it
> won't work at first run because unveil assume a non existent path is a
> file, so you need to run irssi twice the first time you activate logs.
> People having logs in ~/irclogs can still redefine logs inside ~/.irssi/
> and make a symlink to it in ~/irclogs
Hi Solene,
As I mentioned on irc, I don't think this is good for a number of
reasons. The logs directory can be changed in ~/.irssi/config, via
command line, _AND_ at runtime (/SET autolog_path <path). There are
other paths and settings that may be changed at runtime as well, or
simply through /reload [config] that are not handled by your diff.
In addition to what Stuart said, irssi is using libperl here for perl
scripts, which means the irssi unveil will be applied to them. There
are many uses for perl scripts, and many involve reading arbitrary files
and there's no way to know that upfront.
It might be worth inverting this to add a flag to enable pledge/unveil,
but doing it by default would not be ok, imho.
BTW, for anyone considering adding pledges to ports, please consider
studying examples in base and following the canonicial promises order.
Seeing,
> ++ if (pledge("dns inet stdio",NULL) == -1)
Instead of
> ++ if (pledge("stdio dns inet", NULL) == -1)
Always makes me suspect that this wasn't done..
-Bryan.
> I had lot of feedback about this change not being plugin friendly, but I
> had no real feedback either. I would say that if it kills your plugins,
> just use flag -u or maybe I should make this feature non default and
> activate it with -u?
>
> I don't use much plugins in irssi and none interacts with the FS at all.
> If some requires dbus for notification, I think it's safe to unveil the
> dbus socket path as chromium does.
>
>
> Index: Makefile
> ===================================================================
> RCS file: /data/cvs/ports/net/irssi/Makefile,v
> retrieving revision 1.79
> diff -u -p -r1.79 Makefile
> --- Makefile 18 Feb 2019 18:35:57 -0000 1.79
> +++ Makefile 11 Jun 2019 06:38:27 -0000
> @@ -5,6 +5,7 @@ COMMENT = modular IRC client with many f
> V = 1.2.0
> DISTNAME = irssi-$V
> PKGSPEC = irssi-=$V
> +REVISION = 0
>
> CATEGORIES = net
>
> @@ -15,6 +16,7 @@ MAINTAINER = Klemens Nanni <kn@openbsd.o
> # GPLv2+
> PERMIT_PACKAGE_CDROM = Yes
>
> +# use pledge()
> WANTLIB += c crypto curses gcrypt glib-2.0 gmodule-2.0 gpg-error \
> iconv intl m otr pcre perl pthread ssl
>
> @@ -44,6 +46,9 @@ CONFIGURE_ARGS += --with-socks
> LIB_DEPENDS += security/dante
> WANTLIB += socks
> .endif
> +
> +# required for 100% tests to pass
> +PORTHOME= ${WRKDIST}
>
> MAKE_FLAGS = scriptdir="${SYSCONFDIR}/irssi/scripts" \
> themedir="${SYSCONFDIR}/irssi/themes"
> Index: patches/patch-src_core_net-nonblock_c
> ===================================================================
> RCS file: patches/patch-src_core_net-nonblock_c
> diff -N patches/patch-src_core_net-nonblock_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_core_net-nonblock_c 11 Jun 2019 06:37:38 -0000
> @@ -0,0 +1,17 @@
> +$OpenBSD$
> +
> +Index: src/core/net-nonblock.c
> +--- src/core/net-nonblock.c.orig
> ++++ src/core/net-nonblock.c
> +@@ -60,6 +60,11 @@ int net_gethostbyname_nonblock(const char *addr, GIOCh
> + "Using blocking resolving");
> + }
> +
> ++ if(pledge_enabled()) {
> ++ if (pledge("dns inet stdio",NULL) == -1)
> ++ { printf("Error pledge non-block\n"); exit(1); }
> ++ }
> ++
> + /* child */
> + srand(time(NULL));
> +
> Index: patches/patch-src_core_net-nonblock_c.orig
> ===================================================================
> RCS file: patches/patch-src_core_net-nonblock_c.orig
> diff -N patches/patch-src_core_net-nonblock_c.orig
> Index: patches/patch-src_fe-common_core_fe-common-core_c
> ===================================================================
> RCS file: patches/patch-src_fe-common_core_fe-common-core_c
> diff -N patches/patch-src_fe-common_core_fe-common-core_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_fe-common_core_fe-common-core_c 11 Jun 2019 06:46:51
> -0000
> @@ -0,0 +1,96 @@
> +$OpenBSD$
> +
> +Index: src/fe-common/core/fe-common-core.c
> +--- src/fe-common/core/fe-common-core.c.orig
> ++++ src/fe-common/core/fe-common-core.c
> +@@ -51,6 +51,8 @@
> +
> + #include <signal.h>
> +
> ++#include <pwd.h>
> ++
> + static char *autocon_server;
> + static char *autocon_password;
> + static int autocon_port;
> +@@ -58,6 +60,8 @@ static int no_autoconnect;
> + static char *cmdline_nick;
> + static char *cmdline_hostname;
> +
> ++static int no_unveil;
> ++
> + void fe_core_log_init(void);
> + void fe_core_log_deinit(void);
> +
> +@@ -99,6 +103,56 @@ void window_commands_deinit(void);
> +
> + static void sig_setup_changed(void);
> +
> ++int pledge_enabled()
> ++{
> ++ return no_unveil ? 0 : 1;
> ++}
> ++
> ++void pledge_init()
> ++{
> ++ if(pledge_enabled()) {
> ++ struct passwd *pw;
> ++ int user_id = getuid();
> ++ char path[200];
> ++
> ++ pw = getpwuid(user_id);
> ++ if (pw == NULL)
> ++ { printf("can't get pw of current user\n"); exit(1); }
> ++
> ++ if( unveil("/etc/ssl","r") == -1 )
> ++ { printf("error unveil /etc/ssl/\n"); exit(1); }
> ++
> ++ if( unveil("/etc/resolv.conf","r") == -1 )
> ++ { printf("error unveil /etc/resolv.conf\n"); exit(1); }
> ++
> ++ if( unveil("/dev/null","rw") == -1 )
> ++ { printf("error unveil dev/null\n"); exit(1); }
> ++
> ++ if( unveil("/usr/local/libdata/perl5/","r") == -1 )
> ++ { printf("error unveil /usr/local/libdata/perl5/\n"); exit(1); }
> ++
> ++ if( unveil("/usr/libdata/perl5/","r") == -1 )
> ++ { printf("error unveil /usr/libdata/perl5/\n"); exit(1); }
> ++
> ++ snprintf(path,sizeof(path), "%s/irclogs",pw->pw_dir);
> ++ if( unveil(path,"rwc") == -1 )
> ++ { printf("error unveil %s\n",path); exit(1); }
> ++
> ++ snprintf(path,sizeof(path), "%s/.irssi/",pw->pw_dir);
> ++ if( unveil(path,"rwc") == -1 )
> ++ { printf("error unveil %s\n",path); exit(1); }
> ++
> ++ snprintf(path,sizeof(path), "%s/.irssi/scripts",pw->pw_dir);
> ++ if( unveil(path,"r") == -1 )
> ++ { printf("error unveil %s\n",path); exit(1); }
> ++
> ++ if (pledge("dns inet tty flock stdio cpath wpath rpath prot_exec
> proc unveil getpw",NULL) == -1)
> ++ { printf("error pledge\n"); exit(1); }
> ++
> ++ }
> ++
> ++}
> ++
> + static void sig_connected(SERVER_REC *server)
> + {
> + MODULE_DATA_SET(server, g_new0(MODULE_SERVER_REC, 1));
> +@@ -133,6 +187,7 @@ void fe_common_core_register_options(void)
> + { "noconnect", '!', 0, G_OPTION_ARG_NONE, &no_autoconnect,
> "Disable autoconnecting", NULL },
> + { "nick", 'n', 0, G_OPTION_ARG_STRING, &cmdline_nick, "Specify
> nick to use", NULL },
> + { "hostname", 'h', 0, G_OPTION_ARG_STRING, &cmdline_hostname,
> "Specify host name to use", NULL },
> ++ { "disable-unveil", 'u', 0, G_OPTION_ARG_NONE, &no_unveil, "Disable
> unveil and pledge security features", NULL },
> + { NULL }
> + };
> +
> +@@ -140,6 +195,7 @@ void fe_common_core_register_options(void)
> + autocon_password = NULL;
> + autocon_port = 0;
> + no_autoconnect = FALSE;
> ++ no_unveil = FALSE;
> + cmdline_nick = NULL;
> + cmdline_hostname = NULL;
> + args_register(options);
> Index: patches/patch-src_fe-common_core_fe-common-core_c.orig
> ===================================================================
> RCS file: patches/patch-src_fe-common_core_fe-common-core_c.orig
> diff -N patches/patch-src_fe-common_core_fe-common-core_c.orig
> Index: patches/patch-src_fe-text_irssi_c
> ===================================================================
> RCS file: patches/patch-src_fe-text_irssi_c
> diff -N patches/patch-src_fe-text_irssi_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_fe-text_irssi_c 11 Jun 2019 06:37:46 -0000
> @@ -0,0 +1,28 @@
> +$OpenBSD$
> +
> +Index: src/fe-text/irssi.c
> +--- src/fe-text/irssi.c.orig
> ++++ src/fe-text/irssi.c
> +@@ -210,6 +210,10 @@ static void textui_finish_init(void)
> + printformat(NULL, NULL, MSGLEVEL_CRAP|MSGLEVEL_NO_ACT,
> TXT_WELCOME_FIRSTTIME);
> + }
> +
> ++ if(pledge_enabled()) {
> ++ printformat(NULL, NULL, MSGLEVEL_CRAP|MSGLEVEL_NO_ACT,
> TXT_UNVEIL_ENABLED);
> ++ }
> ++
> + /* see irc-servers-setup.c:init_userinfo */
> + if (user_settings_changed)
> + printformat(NULL, NULL, MSGLEVEL_CLIENTNOTICE,
> TXT_WELCOME_INIT_SETTINGS);
> +@@ -333,7 +337,11 @@ int main(int argc, char **argv)
> + }
> +
> + g_log_set_always_fatal(loglev);
> ++
> ++ pledge_init();
> ++
> + textui_finish_init();
> ++
> + main_loop = g_main_new(TRUE);
> +
> + /* Does the same as g_main_run(main_loop), except we
> Index: patches/patch-src_fe-text_irssi_c.orig
> ===================================================================
> RCS file: patches/patch-src_fe-text_irssi_c.orig
> diff -N patches/patch-src_fe-text_irssi_c.orig
> Index: patches/patch-src_fe-text_module-formats_c
> ===================================================================
> RCS file: patches/patch-src_fe-text_module-formats_c
> diff -N patches/patch-src_fe-text_module-formats_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_fe-text_module-formats_c 11 Jun 2019 06:30:46
> -0000
> @@ -0,0 +1,18 @@
> +$OpenBSD$
> +
> +Index: src/fe-text/module-formats.c
> +--- src/fe-text/module-formats.c.orig
> ++++ src/fe-text/module-formats.c
> +@@ -99,7 +99,11 @@ FORMAT_REC gui_text_formats[] =
> + "Use the /HELP command to get detailed information about%:"
> + "the available commands.%:"
> + "- - - - - - - - - - - - - - - - - - - - - - - - - - - -", 0 },
> +- { "welcome_init_settings", "The following settings were initialized", 0
> },
> ++ { "welcome_init_settings","The following settings were initialized", 0
> },
> ++ { "unveil_enabled",
> ++ "Unveil security features is enabled, your logs and scripts must be
> under ~/.irssi/%:"
> ++ "Irssi and its plugins are denied access to others paths, this may
> break some plugins.%:"
> ++ "This can be disabled with the flag -u at runtime.", 0 },
> +
> + { NULL, NULL, 0 }
> + };
> Index: patches/patch-src_fe-text_module-formats_h
> ===================================================================
> RCS file: patches/patch-src_fe-text_module-formats_h
> diff -N patches/patch-src_fe-text_module-formats_h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_fe-text_module-formats_h 11 Jun 2019 06:31:02
> -0000
> @@ -0,0 +1,13 @@
> +$OpenBSD$
> +
> +Index: src/fe-text/module-formats.h
> +--- src/fe-text/module-formats.h.orig
> ++++ src/fe-text/module-formats.h
> +@@ -59,6 +59,7 @@ enum {
> + TXT_IRSSI_BANNER,
> + TXT_WELCOME_FIRSTTIME,
> + TXT_WELCOME_INIT_SETTINGS,
> ++ TXT_UNVEIL_ENABLED,
> +
> + TXT_COUNT
> + };
>
>
