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.
