On Fri, Sep 27, 2019 at 10:02:35AM +0200, Sebastien Marie wrote:
> Just two comments.
>
> > Index: patches/patch-cvsweb_cgi
> > ===================================================================
> > RCS file: /data/cvs/ports/devel/cvsweb/patches/patch-cvsweb_cgi,v
> > retrieving revision 1.13
> > diff -u -p -r1.13 patch-cvsweb_cgi
> > --- patches/patch-cvsweb_cgi 7 Apr 2013 20:07:24 -0000 1.13
> > +++ patches/patch-cvsweb_cgi 27 Sep 2019 07:20:20 -0000
> > @@ -1,6 +1,7 @@
> > $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/04/07 20:07:24 naddy Exp $
> > ---- cvsweb.cgi.orig Thu Sep 26 22:56:05 2002
> > -+++ cvsweb.cgi Sun Apr 7 14:15:55 2013
> > +Index: cvsweb.cgi
> > +--- cvsweb.cgi.orig
> > ++++ cvsweb.cgi
> > @@ -1,4 +1,4 @@
> > -#!/usr/bin/perl -wT
> > +#!/usr/bin/perl -w
>
> any reason to remove the taint flag on perl shebang ?
no idea, it was already there
> > @@ -37,7 +38,43 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
> > );
> >
> > @LOGSORTKEYS = qw(cvs date rev);
> > -@@ -2014,20 +2009,6 @@ sub doDiff($$$$$$) {
> > +@@ -249,7 +244,26 @@ EOM
> > +
> > + use Time::Local ();
> > + use IPC::Open2 qw(open2);
> > ++use OpenBSD::Pledge;
> > ++use OpenBSD::Unveil;
> > +
> > ++pledge( qw( rpath proc exec prot_exec unveil ) ) || die "Can't pledge:
> > $!";
> > ++
> > ++# directories
> > ++unveil( "/usr/libdata/perl5/", "r" ) || die "Unable to unveil: $!";
> > ++unveil( "/cvs/", "r" ) || die "Unable to unveil: $!";
> > ++unveil( "/conf/", "r" ) || die "Unable to unveil: $!";
> > ++
> > ++# files
> > ++unveil( "/dev/null", "r" ) || die "Unable to unveil: $!";
> > ++unveil( "/usr/bin/rcsdiff", "rx" ) || die "Unable to unveil: $!";
> > ++unveil( "/usr/bin/rlog", "rx" ) || die "Unable to unveil: $!";
> > ++unveil( "/usr/bin/cvs", "rx" ) || die "Unable to unveil: $!";
> > ++unveil( "/usr/bin/uname", "rx" ) || die "Unable to unveil: $!";
> > ++
> > ++unveil() || die "Unvable to unveil: $!";
> > ++
> > ++
>
> the proper idiom is :
> - unveil() calls first
> - pledge() call without "unveil"
>
> you will have almost(*) the same result that what you have, but it is more
> obvious
> that unveil(2) is not permitted after pledge(2) call.
>
> (*) almost: with your diff, calling unveil(2) later result in an error
> (EPERM) ;
> whereas without "unveil" in pledge(2) is would result a program
> terminaison
> (pledge violation).
indeed, it makes more sense this way
>
>
> And if you want to avoid patching /dev/null usage, you could add "wpath" in
> pledge(2) and unveil("/dev/null", "rw"). The capability to write will be
> restricted to only /dev/null with pledge+unveil. so no other write will be
> allowed on filesystem.
>
good idea
