add pledge to devel/cvsweb

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

add pledge to devel/cvsweb

Solene Rapenne
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.


Index: Makefile
===================================================================
RCS file: /data/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 26 Sep 2019 14:24:53 -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: /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 26 Sep 2019 15:21:46 -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,27 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
  );
 
  @LOGSORTKEYS = qw(cvs date rev);
-@@ -2014,20 +2009,6 @@ sub doDiff($$$$$$) {
+@@ -249,7 +244,10 @@ EOM
+
+ use Time::Local ();
+ use IPC::Open2 qw(open2);
++use OpenBSD::Pledge;
+
++pledge( qw( rpath proc exec prot_exec ) ) || die "Can't pledge: $!";
++
+ # Check if the zlib C library interface is installed, and if yes
+ # we can avoid using the extra gzip process.
+ eval { require Compress::Zlib; };
+@@ -1578,7 +1576,7 @@ sub openOutputFilter() {
+ open(STDOUT, "|-") and return;
+
+ # child of child
+- open(STDERR, '>/dev/null');
++ #open(STDERR, '>/dev/null');
+ exec($output_filter) or exit -1;
+ }
+
+@@ -2014,20 +2012,6 @@ sub doDiff($$$$$$) {
  my @difftype       = @{$difftype->{'opts'}};
  my $human_readable = $difftype->{'colored'};
 
@@ -58,7 +79,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 +2112,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, '>/dev/null'); # 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, '>/dev/null'); # rlog may complain; ignore.
+ openOutputFilter();
+ exec($CMD{rlog}, '-r', @files) or exit -1;
+ }
+@@ -2658,7 +2642,7 @@ sub printLog($;$) {
  if (/^1\.1\.1\.\d+$/) {
  print " <i>(vendor branch)</i>";
  }
Index: pkg/README
===================================================================
RCS file: /data/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 26 Sep 2019 14:24:47 -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} `arch -s`-openbsd/OpenBSD unicore
 
 # The "annotate" function requires this empty file:
 #
@@ -72,6 +72,8 @@ 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/auto/OpenBSD/Pledge/Pledge.so ./auto/OpenBSD/Pledge/
 
 # You also need to enable slowcgi(8):
 

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Otto Moerbeek
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

>
>
> Index: Makefile
> ===================================================================
> RCS file: /data/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 26 Sep 2019 14:24:53 -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: /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 26 Sep 2019 15:21:46 -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,27 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
>   );
>  
>   @LOGSORTKEYS = qw(cvs date rev);
> -@@ -2014,20 +2009,6 @@ sub doDiff($$$$$$) {
> +@@ -249,7 +244,10 @@ EOM
> +
> + use Time::Local ();
> + use IPC::Open2 qw(open2);
> ++use OpenBSD::Pledge;
> +
> ++pledge( qw( rpath proc exec prot_exec ) ) || die "Can't pledge: $!";
> ++
> + # Check if the zlib C library interface is installed, and if yes
> + # we can avoid using the extra gzip process.
> + eval { require Compress::Zlib; };
> +@@ -1578,7 +1576,7 @@ sub openOutputFilter() {
> + open(STDOUT, "|-") and return;
> +
> + # child of child
> +- open(STDERR, '>/dev/null');
> ++ #open(STDERR, '>/dev/null');
> + exec($output_filter) or exit -1;
> + }
> +
> +@@ -2014,20 +2012,6 @@ sub doDiff($$$$$$) {
>   my @difftype       = @{$difftype->{'opts'}};
>   my $human_readable = $difftype->{'colored'};
>  
> @@ -58,7 +79,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 +2112,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, '>/dev/null'); # 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, '>/dev/null'); # rlog may complain; ignore.
> + openOutputFilter();
> + exec($CMD{rlog}, '-r', @files) or exit -1;
> + }
> +@@ -2658,7 +2642,7 @@ sub printLog($;$) {
>   if (/^1\.1\.1\.\d+$/) {
>   print " <i>(vendor branch)</i>";
>   }
> Index: pkg/README
> ===================================================================
> RCS file: /data/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 26 Sep 2019 14:24:47 -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} `arch -s`-openbsd/OpenBSD unicore
>  
>  # The "annotate" function requires this empty file:
>  #
> @@ -72,6 +72,8 @@ 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/auto/OpenBSD/Pledge/Pledge.so ./auto/OpenBSD/Pledge/
>  
>  # You also need to enable slowcgi(8):
>  
>

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Solene Rapenne
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.

latest patch adding unveil


Index: Makefile
===================================================================
RCS file: /data/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 27 Sep 2019 07:19: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: /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
@@ -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: $!";
++
++
+ # Check if the zlib C library interface is installed, and if yes
+ # we can avoid using the extra gzip process.
+ eval { require Compress::Zlib; };
+@@ -1578,7 +1592,7 @@ sub openOutputFilter() {
+ open(STDOUT, "|-") and return;
+
+ # child of child
+- open(STDERR, '>/dev/null');
++ #open(STDERR, '>/dev/null');
+ exec($output_filter) or exit -1;
+ }
+
+@@ -2014,20 +2028,6 @@ sub doDiff($$$$$$) {
  my @difftype       = @{$difftype->{'opts'}};
  my $human_readable = $difftype->{'colored'};
 
@@ -58,7 +95,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, '>/dev/null'); # 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, '>/dev/null'); # 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: /data/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 27 Sep 2019 06:51:39 -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):
 

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Sebastien Marie-3
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 ?

> @@ -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).



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.



--
Sebastien Marie

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Solene Rapenne
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

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Sebastien Marie-3
On Fri, Sep 27, 2019 at 10:10:29AM +0200, Solene Rapenne wrote:

> On Fri, Sep 27, 2019 at 10:02:35AM +0200, Sebastien Marie wrote:
> >
> > > 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
>

ah, my bad. I missed it is a diff, and not the diff of the diff :)

--
Sebastien Marie

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Florian Obser-2
In reply to this post by Solene Rapenne
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.

That's not exactly how this works. Slowcgi transports stderr, but it
does not mix stdout and stderr.  There is a record type for stderr in
the FastCGI spec. Slowcgi transmits stderr inside of that. It's the
webserver that does something with stderr, either drop it on the floor
or log it to the error log.

In practice it does not matter, stderr does not show up on the
delivered webpage. But if someone is looking for the error output they
at least have a chance of finding it.

--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Andrew Hewus Fresh
In reply to this post by Solene Rapenne
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.

You could pre-open a /dev/null file handle and then dup(2) it instead of
opening a new one.  I haven't had time to really look at the rest of the
patch though.

https://perldoc.perl.org/functions/open.html

    You may also, in the Bourne shell tradition, specify an EXPR
    beginning with >& , in which case the rest of the string is
    interpreted as the name of a filehandle (or file descriptor, if
    numeric) to be duped (as in dup(2)) and opened.


#!/usr/bin/perl
use strict;
use warnings;

use OpenBSD::Pledge;
use OpenBSD::Unveil;

open my $DEVNULL, '>', '/dev/null' or die "Unable to open /dev/null: $!";

pledge( qw( rpath proc exec prot_exec unveil ) ) || die "Can't pledge: $!";

unveil( "/usr/libdata/perl5/", "r" ) || die "Unable to unveil: $!";
unveil( "/bin/sh", "rx" ) || die "Unable to unveil: $!";
unveil() || die "Unable to unveil: $!";

my $pid = open my $child, "-|" // die "Unable to fork: $!";
unless ($pid) {
        open STDERR, '>&', $DEVNULL or die "Unable to dup DEVNULL: $!";
        exec 'sh', '-c', 'echo stderr >&2; echo stdout';
}

print "got: $_" for readline $child;


--
andrew - http://afresh1.com

($do || !$do) && undef($try) ;  # Master of Perl, Yoda is.  Hmmmm?

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Andrew Hewus Fresh
In reply to this post by Solene Rapenne
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):
 

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Solene Rapenne
On Fri, Sep 27, 2019 at 06:53:56PM -0700, Andrew Hewus Fresh wrote:

> 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
>

Your diff works fine for me and is much better than my first attempt.
I'm ok for committing it.

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Ingo Schwarze
In reply to this post by Solene Rapenne
Hi Solene,

Solene Rapenne wrote on Thu, Sep 26, 2019 at 05:27:08PM +0200:

> Hi, now that we have OpenBSD::pledge I thought it would be nice to use
> it in devel/cvsweb

I think this is a thoroughly bad idea.

Pledge is useful for well-understood high-quality code.

But CVSweb, at this point, is very old, barely maintained, low-quality
code.  With such code, adding pledge is mostly calling for trouble in
the form of crashes that result from unexpected, but required behaviour.

Besides, the CVSweb we have in ports is an old version: 2.0.6 (September
2002) plus patches.  The latest upstream version is 3.0.6 (September
2005), but upstream is long dead.  Doing new development in the form
of patches to the port looks like a very bad idea.

Some time ago, i set up a new upstream:

  http://mandoc.bsd.lv/cvsweb/

I admit i got distracted, but i hope to return to it.

If we want to improve CVSweb (which i do think makes sense), then the
proper course of action, i think, is to

 1. Commit such improvements as can be easily done and are quite
    useful to the FreeBSD-cvsweb-2_0-branch.  Possibly, pledge can
    be part of that if we are really sure we understand what all the
    code does.  Otherwise, i think step 6 below is the proper time
    for adding pledge.

 2. Release 2.0.7 from that branch.  This can be a short-term goal,
    the only reason it didn't happen yet is that i got distracted.

 3. Update the port to 2.0.7.

 4. Update the bsd.lv and possibly cvsweb.openbsd.org servers to 2.0.7.

 5. Merge 2.0.7 into the MAIN branch (4.x revision numbers).

 6. Develop and release cvsweb-4.0.0 as a medium-term project.

 7. Update the port to 4.0.0.

 8. Update the two servers.

If you want to help with that, i'm happy to give you commit access
to the upstream repository on mandoc.bsd.lv.

If other porters think that solene's work should be committed directly
to the port, i don't veto that, but i do not consider it useful either.
I doubt that any running server will update to a new version of the
port before the release of 2.0.7.

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Solene Rapenne
On Sun, Sep 29, 2019 at 07:37:26PM +0200, Ingo Schwarze wrote:

> Hi Solene,
>
> Solene Rapenne wrote on Thu, Sep 26, 2019 at 05:27:08PM +0200:
>
> > Hi, now that we have OpenBSD::pledge I thought it would be nice to use
> > it in devel/cvsweb
>
> I think this is a thoroughly bad idea.
>
> Pledge is useful for well-understood high-quality code.
>
> But CVSweb, at this point, is very old, barely maintained, low-quality
> code.  With such code, adding pledge is mostly calling for trouble in
> the form of crashes that result from unexpected, but required behaviour.
>
> Besides, the CVSweb we have in ports is an old version: 2.0.6 (September
> 2002) plus patches.  The latest upstream version is 3.0.6 (September
> 2005), but upstream is long dead.  Doing new development in the form
> of patches to the port looks like a very bad idea.
>
> Some time ago, i set up a new upstream:
>
>   http://mandoc.bsd.lv/cvsweb/
>
> I admit i got distracted, but i hope to return to it.
>
> If we want to improve CVSweb (which i do think makes sense), then the
> proper course of action, i think, is to
>
>  1. Commit such improvements as can be easily done and are quite
>     useful to the FreeBSD-cvsweb-2_0-branch.  Possibly, pledge can
>     be part of that if we are really sure we understand what all the
>     code does.  Otherwise, i think step 6 below is the proper time
>     for adding pledge.
>
>  2. Release 2.0.7 from that branch.  This can be a short-term goal,
>     the only reason it didn't happen yet is that i got distracted.
>
>  3. Update the port to 2.0.7.
>
>  4. Update the bsd.lv and possibly cvsweb.openbsd.org servers to 2.0.7.
>
>  5. Merge 2.0.7 into the MAIN branch (4.x revision numbers).
>
>  6. Develop and release cvsweb-4.0.0 as a medium-term project.
>
>  7. Update the port to 4.0.0.
>
>  8. Update the two servers.
>
> If you want to help with that, i'm happy to give you commit access
> to the upstream repository on mandoc.bsd.lv.
>
> If other porters think that solene's work should be committed directly
> to the port, i don't veto that, but i do not consider it useful either.
> I doubt that any running server will update to a new version of the
> port before the release of 2.0.7.
>
> Yours,
>   Ingo

All of what you said make sense, I did not consider looking for a newer
version before trying to pledge cvsweb.

I don't really want to work more on cvsweb, I did this because it's a
cgi scripts run on official cvsweb.openbsd.org website and I wanted to
help there.. Altough it may be a good project for a port hackathon if
someone wants to read / debug some perl.

Reply | Threaded
Open this post in threaded view
|

Re: add pledge to devel/cvsweb

Ingo Schwarze
Hi Solene,

Solene Rapenne wrote on Tue, Oct 01, 2019 at 02:15:02PM +0200:

> I don't really want to work more on cvsweb,

Fair enough, you get to choose what you want to work on.  :-)

> I did this because it's a cgi scripts run on official
> cvsweb.openbsd.org website and I wanted to help there.

Sure, thank you, so i'll integrate your work into 2.0.7 and 4.0.x
when i find the time.

Whether to commit it to the port until then - i leave that decision to
other porters...

> Altough it may be a good project for a port hackathon if
> someone wants to read / debug some perl.

Good idea.  I might do that in Bukarest if people there consider it
a useful idea.

Yours,
  Ingo