On Fri, Sep 27, 2019 at 09:28:39AM +0200, Solene Rapenne wrote:
> On Thu, Sep 26, 2019 at 05:40:38PM +0200, Otto Moerbeek wrote:
> > On Thu, Sep 26, 2019 at 05:27:08PM +0200, Solene Rapenne wrote:
> >
> > > Hi, now that we have OpenBSD::pledge I thought it would be nice to use
> > > it in devel/cvsweb
> > >
> > > I've been able to tight it to "rpath proc exec prot_exec", removing
> > > wpath and cpath was possible by commenting lines piping STDERROR to
> > > /dev/null, that doesn't mean creating dev/null is not required anymore,
> > > it's still required for cvsweb to work correctly (due to rlog I think).
> > >
> > > I updated pkg/README because this requires OpenBSD/Pledge.pm and a so
> > > file to be copied into the chroot too.
> > >
> > > I had some testing on www repository by lot of people and it worked
> > > perfectly.
> >
> > Be careful that error messages do not show up on the web pages
> > generated by not redirecting stderr...
> >
> > -Otto
>
> at least slowcgi discard stderr output, not sure for others cgi.
> if you have a better way (not writing to something) to discard the
> stderr output that would be better than making slowcgi ignoring it.
Oh, slowcgi doesn't actually discard stderr, instead it passes it
through to httpd which, in my case, logs it to /var/www/log/error.log,
but not certain we want to fill that will errors from commands cvsweb
execs.
> latest patch adding unveil
I looked at this a bit more, and with a bit of work, some feedback on
better use of pledge (and following the recommendation to unveil before
pledge), we can reduce the promises and unveil'd paths that are needed
and make them more accurate.
First, by moving `require Compress::Zlib;` and `use Cwd` above the
pledge, we no longer need to prot_exec in order to load the XS modules,
which I've been told is a terribly dangerous promise and using it should
only be done with large amounts of caution.
Then by reading the config file above, hopefully you trust the contents,
we no longer need to unveil conf/ and can instead read the list of
CVSrepositories that need to be unveiled. We can also unveil the actual
CMDs that are configured, although it's probably a good practice to
hardcode those, rather than using the default search that's in the
sample config file.
I also think we only need to unveil those CMDs "x" and not "rx", but I
may have missed something.
I did use a dup(2) of a /dev/null file handle to avoid having to open
that after pledge and unveil, although with a proper unveil, a wpath
promise is probably just as safe.
This is running on my site now, so if you want to try to find some 500
errors, I wouldn't complain.
http://cvs.afresh1.com/cgi-bin/cvsweb
Index: Makefile
===================================================================
RCS file: /cvs/ports/devel/cvsweb/Makefile,v
retrieving revision 1.62
diff -u -p -r1.62 Makefile
--- Makefile 12 Jul 2019 20:44:07 -0000 1.62
+++ Makefile 28 Sep 2019 01:47:23 -0000
@@ -3,7 +3,7 @@
COMMENT= CGI script to browse CVS repository trees
DISTNAME= cvsweb-2.0.6
-REVISION= 27
+REVISION= 28
CATEGORIES= devel www
HOMEPAGE= http://www.freebsd.org/projects/cvsweb.html
Index: patches/patch-cvsweb_cgi
===================================================================
RCS file: /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 28 Sep 2019 01:47:23 -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
@@ -37,7 +38,86 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
);
@LOGSORTKEYS = qw(cvs date rev);
-@@ -2014,20 +2009,6 @@ sub doDiff($$$$$$) {
+@@ -247,14 +242,44 @@ EOM
+
+ ##### End of configuration variables #####
+
++use Cwd qw< abs_path >;
+ use Time::Local ();
+ use IPC::Open2 qw(open2);
++use OpenBSD::Pledge;
++use OpenBSD::Unveil;
+
+ # Check if the zlib C library interface is installed, and if yes
+ # we can avoid using the extra gzip process.
+ eval { require Compress::Zlib; };
+ $has_zlib = !$@;
+
++if (-f $config) {
++ do "$config" or fatal("500 Internal Error",
++ 'Error in loading configuration file: %s<br><br>%s<br>',
++ $config, $@);
++} else {
++ fatal("500 Internal Error",
++ 'Configuration not found. Set the variable <code>$config</code> in cvsweb.cgi to your <b>cvsweb.conf</b> configuration file first.'
++ );
++}
++
++open my $DEVNULL, '>', '/dev/null' or die "Unable to open /dev/null: $!";
++
++# CVS Repositories
++for ( my $i = 1 ; $i < @CVSrepositories ; $i += 2 ) {
++ unveil( $CVSrepositories[$i][1], "r" ) || die "Unable to unveil: $!";
++}
++
++# files
++# Should these be all the %CMD?
++foreach my $file (qw< rcsdiff rlog cvs uname >) {
++ unveil( $CMD{$file}, "x" ) || die "Unable to unveil $file: $!";
++}
++
++unveil() || die "Unvable to unveil: $!";
++
++pledge( qw( rpath proc exec ) ) || die "Can't pledge: $!";
++
+ $verbose = $v;
+ $checkoutMagic = "~checkout~";
+ $pathinfo = defined($ENV{PATH_INFO}) ? $ENV{PATH_INFO} : '';
+@@ -313,16 +338,6 @@ $maycompress =
+ @stickyvars = qw(cvsroot hideattic sortby logsort f only_with_tag);
+ @unsafevars = qw(logsort only_with_tag r1 r2 rev sortby tr1 tr2);
+
+-if (-f $config) {
+- do "$config" or fatal("500 Internal Error",
+- 'Error in loading configuration file: %s<br><br>%s<br>',
+- $config, $@);
+-} else {
+- fatal("500 Internal Error",
+- 'Configuration not found. Set the variable <code>$config</code> in cvsweb.cgi to your <b>cvsweb.conf</b> configuration file first.'
+- );
+-}
+-
+ undef %input;
+ $query = $ENV{QUERY_STRING};
+
+@@ -1578,7 +1593,7 @@ sub openOutputFilter() {
+ open(STDOUT, "|-") and return;
+
+ # child of child
+- open(STDERR, '>/dev/null');
++ open(STDERR, '>&', $DEVNULL);
+ exec($output_filter) or exit -1;
+ }
+
+@@ -1833,7 +1848,6 @@ sub doCheckout($$) {
+ open(STDERR, ">&STDOUT"); # Redirect stderr to stdout
+
+ # work around a bug of cvs -p; expand symlinks
+- use Cwd 'abs_path';
+ exec($CMD{cvs}, @cvs_options,
+ '-d', abs_path($cvsroot),
+ 'co', '-p',
+@@ -2014,20 +2028,6 @@ sub doDiff($$$$$$) {
my @difftype = @{$difftype->{'opts'}};
my $human_readable = $difftype->{'colored'};
@@ -58,7 +138,24 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
if ($human_readable) {
if ($hr_ignwhite) {
push @difftype, '-w';
-@@ -2658,7 +2639,7 @@ sub printLog($;$) {
+@@ -2128,14 +2128,14 @@ sub getDirLogs($$@) {
+
+ #can't use -r<tag> as - is allowed in tagnames, but misinterpreated by rlog..
+ if (!open($fh, "-|")) { # child
+- open(STDERR, '>/dev/null'); # rlog may complain; ignore.
++ open(STDERR, '>&', $DEVNULL); # rlog may complain; ignore.
+ openOutputFilter();
+ exec($CMD{rlog}, @files) or exit -1;
+ }
+ } else {
+
+ if (!open($fh, "-|")) { # child
+- open(STDERR, '>/dev/null'); # rlog may complain; ignore.
++ open(STDERR, '>&', $DEVNULL); # rlog may complain; ignore.
+ openOutputFilter();
+ exec($CMD{rlog}, '-r', @files) or exit -1;
+ }
+@@ -2658,7 +2658,7 @@ sub printLog($;$) {
if (/^1\.1\.1\.\d+$/) {
print " <i>(vendor branch)</i>";
}
Index: pkg/README
===================================================================
RCS file: /cvs/ports/devel/cvsweb/pkg/README,v
retrieving revision 1.18
diff -u -p -r1.18 README
--- pkg/README 2 May 2019 18:58:38 -0000 1.18
+++ pkg/README 28 Sep 2019 01:47:23 -0000
@@ -22,7 +22,7 @@ cd /var/www/usr
mkdir -p bin lib libdata/perl5 libexec
cd /var/www/usr/libdata/perl5
-mkdir -p File IPC Time warnings `arch -s`-openbsd/auto/{Cwd,Fcntl} unicore
+mkdir -p File IPC Time warnings `arch -s`-openbsd/auto/{Cwd,Fcntl,OpenBSD/Pledge,OpenBSD/Unveil} `arch -s`-openbsd/OpenBSD unicore
# The "annotate" function requires this empty file:
#
@@ -72,6 +72,10 @@ cp -p /usr/libdata/perl5/`arch -s`-openb
cp -p /usr/libdata/perl5/`arch -s`-openbsd/DynaLoader.pm .
cp -p /usr/libdata/perl5/`arch -s`-openbsd/Fcntl.pm .
cp -p /usr/libdata/perl5/`arch -s`-openbsd/auto/Fcntl/Fcntl.so ./auto/Fcntl/
+cp -p /usr/libdata/perl5/`arch -s`-openbsd/OpenBSD/Pledge.pm ./OpenBSD/
+cp -p /usr/libdata/perl5/`arch -s`-openbsd/OpenBSD/Unveil.pm ./OpenBSD/
+cp -p /usr/libdata/perl5/`arch -s`-openbsd/auto/OpenBSD/Pledge/Pledge.so ./auto/OpenBSD/Pledge/
+cp -p /usr/libdata/perl5/`arch -s`-openbsd/auto/OpenBSD/Unveil/Unveil.so ./auto/OpenBSD/Unveil/
# You also need to enable slowcgi(8):
No comments:
Post a Comment