- Dillo-dev - lists.mailman3.com

[PATCH v2] Unveil patch

by a1ex＠dismail.de

Hi, Here is version 2 of my unveil patch. Many thanks to Rodrigo for his kind assistance! Improvements: - Unveil is disabled by default. It can be enabled using: configure --enable-unveil - There is now a dUnveil() wrapper which simplifies the code and error handling - Locale check and custom cursor icons unveil fix To-Do: - Add prefs parsing to plugins to get 'save_dir' (may need help here) - Add a dillorc pref to enable/disable unveil (same issue as above) - Localize wget and $AUTHORITY - A few other items from my previous to-do list Regards, Alex diff -upr a/configure.ac b/configure.ac --- a/configure.ac Sat Jul 27 12:54:47 2024 +++ b/configure.ac Thu Aug 1 16:40:16 2024 @@ -36,6 +36,11 @@ AC_ARG_ENABLE([insure], [enable_insure=$enableval], [enable_insure=no]) +AC_ARG_ENABLE([unveil], + [AS_HELP_STRING([--enable-unveil], [Build with support for unveil])], + [enable_unveil=$enableval], + [enable_unveil=no]) + AC_ARG_ENABLE([ipv6], [AS_HELP_STRING([--enable-ipv6], [Build with support for IPv6])], [enable_ipv6=$enableval], @@ -619,6 +624,9 @@ if test "x$enable_insure" = "xyes" ; then CC="insure -Zoi \"compiler $CC\"" LIBS="$LIBS -lstdc++-2-libc6.1-1-2.9.0" fi +if test "x$enable_unveil" = "xyes" ; then + AC_DEFINE([ENABLE_UNVEIL], [1], [Enable unveil]) +fi if test "x$enable_threaded_dns" = "xyes" ; then CFLAGS="$CFLAGS -DD_DNS_THREADED" fi @@ -725,4 +733,5 @@ _AS_ECHO([ GIF enabled : ${enable_gif}]) _AS_ECHO([ SVG enabled : ${enable_svg}]) _AS_ECHO([]) _AS_ECHO([ HTML tests : ${html_tests_ok}]) +_AS_ECHO([ unveil enabled : ${enable_unveil}]) _AS_ECHO([]) diff -upr a/dpi/bookmarks.c b/dpi/bookmarks.c --- a/dpi/bookmarks.c Sat Jul 27 12:54:47 2024 +++ b/dpi/bookmarks.c Thu Aug 1 16:40:50 2024 @@ -1606,6 +1606,20 @@ static void termination_handler(int signum) exit(signum); } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} /* * -- MAIN ------------------------------------------------------------------- @@ -1617,6 +1631,16 @@ int main(void) { char *tok; Dsh *sh; + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + char *dil_bm = dStrconcat(dGethomedir(), "/.dillo/bm.txt", NULL); + dUnveil(dil_bm, "rwc"); + dFree(dil_bm); + unveil(NULL, NULL); + #endif + #endif + /* Arrange the cleanup function for terminations via exit() */ atexit(cleanup); diff -upr a/dpi/cookies.c b/dpi/cookies.c --- a/dpi/cookies.c Sat Jul 27 12:54:47 2024 +++ b/dpi/cookies.c Thu Aug 1 16:40:50 2024 @@ -1632,6 +1632,20 @@ static void termination_handler(int signum) exit(signum); } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} /* * -- MAIN ------------------------------------------------------------------- @@ -1643,6 +1657,16 @@ int main(void) { int sock_fd, code; char *buf; Dsh *sh; + + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "rwc"); + dFree(dil_loc); + unveil(NULL, NULL); + #endif + #endif /* Arrange the cleanup function for terminations via exit() */ atexit(cleanup); diff -upr a/dpi/datauri.c b/dpi/datauri.c --- a/dpi/datauri.c Sat Jul 27 12:54:47 2024 +++ b/dpi/datauri.c Thu Aug 1 16:40:50 2024 @@ -280,6 +280,21 @@ static unsigned char *datauri_get_data(char *url, size return data; } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} + /* * */ @@ -289,6 +304,17 @@ int main(void) unsigned char *data; int rc; size_t data_size = 0; + + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + dUnveil("/tmp", "rwc"); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "rwc"); + dFree(dil_loc); + unveil(NULL, NULL); + #endif + #endif /* Initialize the SockHandler */ sh = a_Dpip_dsh_new(STDIN_FILENO, STDOUT_FILENO, 8*1024); diff -upr a/dpi/downloads.cc b/dpi/downloads.cc --- a/dpi/downloads.cc Sat Jul 27 12:54:47 2024 +++ b/dpi/downloads.cc Thu Aug 1 16:40:50 2024 @@ -1098,12 +1098,45 @@ static void custLabelMeasure(const Fl_Label* o, int& W fl_measure(o->value, W, H, interpret_symbols); } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} - //int main(int argc, char **argv) int main() { int ww = 420, wh = 85; + + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + dUnveil("/tmp", "rwc"); + dUnveil("/etc/fonts", "r"); + dUnveil("/usr/local/bin/wget", "x"); + char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); + dUnveil(xauth_loc, "r"); + dFree(xauth_loc); + dUnveil("/usr/local/share/fonts", "r"); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "rwc"); + dFree(dil_loc); + char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); + dUnveil(dl_loc, "rwc"); + dFree(dl_loc); + unveil(NULL, NULL); + #endif + #endif Fl::lock(); diff -upr a/dpi/file.c b/dpi/file.c --- a/dpi/file.c Sat Jul 27 12:54:47 2024 +++ b/dpi/file.c Thu Aug 1 16:40:50 2024 @@ -1063,6 +1063,20 @@ static int File_check_fds(uint_t seconds) return st; } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} int main(void) { @@ -1070,6 +1084,19 @@ int main(void) socklen_t sin_sz; int sock_fd, c_st, st = 1; + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + dUnveil("/tmp", "rw"); + char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); + dUnveil(dl_loc, "rw"); + dFree(dl_loc); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "rwc"); + unveil(NULL, NULL); + #endif + #endif + /* Arrange the cleanup function for abnormal terminations */ if (signal (SIGINT, termination_handler) == SIG_IGN) signal (SIGINT, SIG_IGN); diff -upr a/dpi/ftp.c b/dpi/ftp.c --- a/dpi/ftp.c Sat Jul 27 12:54:47 2024 +++ b/dpi/ftp.c Thu Aug 1 16:40:50 2024 @@ -272,6 +272,21 @@ static int try_ftp_transfer(char *url) return (no_such_file ? -1 : (aborted ? -2 : nb)); } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} + /* * */ @@ -281,6 +296,21 @@ int main(int argc, char **argv) char *dpip_tag = NULL, *cmd = NULL, *url = NULL, *url2 = NULL; int st, rc; char *p, *d_cmd; + + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + dUnveil("/tmp", "rwc"); + dUnveil("/usr/local/bin/wget", "x"); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "rwc"); + dFree(dil_loc); + char *dl_loc = dStrconcat(dGethomedir(), "/download", NULL); + dUnveil(dl_loc, "rwc"); + dFree(dl_loc); + unveil(NULL, NULL); + #endif + #endif /* wget may need to write a temporary file... */ rc = chdir("/tmp"); diff -upr a/dpi/vsource.c b/dpi/vsource.c --- a/dpi/vsource.c Sat Jul 27 12:54:47 2024 +++ b/dpi/vsource.c Thu Aug 1 16:40:50 2024 @@ -178,6 +178,21 @@ void send_html_text(Dsh *sh, const char *url, int data a_Dpip_dsh_write_str(sh, 1, "</table></body></html>"); } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} + /* * */ @@ -187,6 +202,16 @@ int main(void) int data_size; char *dpip_tag, *cmd = NULL, *cmd2 = NULL, *url = NULL, *size_str = NULL; char *d_cmd; + + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "r"); + dFree(dil_loc); + unveil(NULL, NULL); + #endif + #endif _MSG("starting...\n"); //sleep(20); diff -upr a/dpid/main.c b/dpid/main.c --- a/dpid/main.c Sat Jul 27 12:54:47 2024 +++ b/dpid/main.c Thu Aug 1 16:41:04 2024 @@ -220,6 +220,21 @@ static int get_open_max(void) #endif } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} + /*! \todo * \li Add a dpid_idle_timeout variable to dpidrc * \bug Infinite loop if plugin crashes before it accepts a connection @@ -236,6 +251,17 @@ int main(void) services_list = NULL; //daemon(0,0); /* Use 0,1 for feedback */ /* TODO: call setsid() ?? */ + + /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + dUnveil("/usr/local/lib/dillo", "rx"); + dUnveil("/usr/local/etc/dillo", "r"); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "rwc"); + unveil(NULL, NULL); + #endif + #endif /* Allow read and write access, but only for the user. * TODO: can this cause trouble with umount? */ diff -upr a/src/dillo.cc b/src/dillo.cc --- a/src/dillo.cc Sat Jul 27 12:54:47 2024 +++ b/src/dillo.cc Thu Aug 1 16:40:06 2024 @@ -379,6 +379,21 @@ static DilloUrl *makeStartUrl(char *str, bool local) return start_url; } +/** + * Use unveil on OpenBSD + */ +static void dUnveil(const char *path, const char *perm) +{ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + if (unveil(path, perm) == -1) { + MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); + exit(1); + } + #endif + #endif +} + /* * MAIN */ @@ -462,7 +477,34 @@ int main(int argc, char **argv) fclose(fp); } dLib_show_messages(prefs.show_msg); - + + // Use unveil on OpenBSD + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__ + dUnveil("/usr/local/share/fonts", "r"); + dUnveil("/usr/local/share/icons", "r"); + dUnveil("/usr/X11R6/share/X11/locale", "r"); + dUnveil("/usr/X11R6/lib/X11/fonts", "r"); + dUnveil("/usr/local/etc/dillo", "r"); + dUnveil("/tmp", "rwc"); + dUnveil("/usr/local/bin/dpid", "x"); + dUnveil("/etc/fonts", "r"); + dUnveil("/etc/resolv.conf", "r"); + dUnveil("/etc/ssl/cert.pem", "r"); + dUnveil(prefs.save_dir, "rwc"); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + dUnveil(dil_loc, "rwc"); + dFree(dil_loc); + char *icons_loc = dStrconcat(dGethomedir(), "/.icons", NULL); + dUnveil(icons_loc, "r"); + dFree(icons_loc); + char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); + dUnveil(xauth_loc, "r"); + dFree(xauth_loc); + unveil(NULL, NULL); + #endif + #endif + // initialize internal modules a_Dpi_init(); a_Dns_init();

Aug. 1, 2024

Re: dpid doesn't work if configured with "--program-suffix=-new"

by Kevin Koster

Detlef Vollmann wrote: > for the time being I'd like to install the current dillo version, > next to the old existing one. > So I configured with "--program-suffix=-new". > Browsing works fine, but plugins (including bm or file) can't be > started, as the binary is called "dpid-new" but dillo tries to > start "dpid". > > It probaly wouldn't work anyway as $HOME/.dillo is hardcoded > and there's no way to give a separate configuration directory > (and sharing .dillo between different version is probably not > a good idea). What about starting Dillo with: HOME=~/dillo-new dillo Then there's a script in the source code root directory called install-dpi-local. Running (from the Dillo source code dir): mkdir ~/dillo-new HOME=~/dillo-new ./install-dpi-local should install dpid and plug-ins to the ~/dillo-new/.dillo/dpi directory where dillo will find them when started in its own home. Alternatively you could manually make symlinks (if supported by your file system) in ~/dillo-new/.dillo/dpi to the "-new" binaries at their installed locations.

July 31, 2024

dpid doesn't work if configured with "--program-suffix=-new"

by Detlef Vollmann

Hello, for the time being I'd like to install the current dillo version, next to the old existing one. So I configured with "--program-suffix=-new". Browsing works fine, but plugins (including bm or file) can't be started, as the binary is called "dpid-new" but dillo tries to start "dpid". It probaly wouldn't work anyway as $HOME/.dillo is hardcoded and there's no way to give a separate configuration directory (and sharing .dillo between different version is probably not a good idea). But it would be cleaner not to support "--program-suffix". Detlef

July 31, 2024

Unveil discussion: TO-DO list

by a1ex＠dismail.de

Hi Rodrigo, Here are my initial thoughts on your review. Some of the stuff I will need help with, the rest I can probably manage on my own: On Sun, 28 Jul 2024 22:45:38 +0200 Rodrigo Arias <rodarima(a)gmail.com> wrote: > I recommend you add a configure switch (in configure.ac) to enable or > disable unveil(). By default you can keep it disabled and let the > user enable it manually, until we have more feedback to make it > enabled by default. Maybe by defining ENABLE_UNVEIL? You can take the > --enable-svg and ENABLE_SVG as an example. Ok, I think I can handle that. > You should also make a dillorc configure option to enable/disable the > unveil feature in runtime, so it can help debug problems. You can > make the dillorc option enabled by default, which will only take > action when unveil support has been compiled in. For dillo.cc that shouldn't be too hard. I would probably have more difficulties with with the plugins though. > The download directory is set in the dillorc configuration file, and > can be any other place. Is it viable to read the configuration first > and then unveil the appropriate directory? Got that partially working, but ran in to some challenges, see my earlier message. > You'll want to use the $(sysconfdir) autoconf variable: I can take a look, I didn't really consider portability at all since unveil is exclusive to OpenBSD, and will probably remain that way for some time. > As this won't be a simple patch, I suggest you open a PR in GitHub, > so the CI can compile your patch revisions for multiple platforms and > pass the tests. Otherwise I would have to spend the time to do it > myself. If this patch gets to a point where your concerns here are addressed, I will consider it. > The err() function is non-portable, please use Dillo MSG* macros or > perror() + exit(). This should be caught by the CI. > > Also, ensure the indentation is kept at 3 characters (not my > decision). Ok, that shouldn't be a problem. > You should find wget by locating it in the $PATH, not assuming it > would be here. Users may place their own wget binary somewhere else > and this would break the downloads. Will look at it. This brings up a completely separate question: why is wget hardcoded and not changeable as the downloader, and also has a hardcoded useragent which can't be changed by the user. To me, that's not ideal, and maybe you have some thoughts on it. > The .Xauthority file should be read from $AUTHORITY and then from > there if not set. I will try :) > Not sure if we want to constraint file:// like this. What if we are > using Dillo to read local HTML files in ~/? Would you prefer to just not do any unveil in file.c then? It's probably not a huge risk. > Plugins can also be found on the dpi_dir directory defined by the > user in the .dillo/dpidrc, so we would need to parse it first. I will look at it, but this could be more difficult for me. > I would also protect dpidrc from writing as well as dillorc. Agreed. No problem. Regards, Alex

July 30, 2024

Unveil discussion: save_dir

by a1ex＠dismail.de

Hi, I have managed to get dillo.cc to use the 'save_dir' preference to set which download path is unveiled. This was done by moving the unveil call to a point after which prefs are initialized and dillorc is parsed. Here is what the patch looks like: --- dillo.cc Sat Jun 29 16:33:08 2024 +++ dillo.cc Tue Jul 30 14:18:28 2024 @@ -23,6 +23,7 @@ #include <stdio.h> #include <unistd.h> +#include <err.h> #include <stdlib.h> #include <time.h> #include <sys/types.h> @@ -463,6 +464,58 @@ int main(int argc, char **argv) } dLib_show_messages(prefs.show_msg); + // Use unveil on OpenBSD + #ifdef __OpenBSD__ + if (unveil("/usr/local/share/fonts", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/share/icons", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/X11R6/share/X11/locale", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/X11R6/lib/X11/fonts", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/etc/dillo", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/tmp", "rwc") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/bin/dpid", "x") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/fonts", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/resolv.conf", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/ssl/cert.pem", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil(prefs.save_dir, "rwc") == -1) { + err(1, "unveil failed"); + } + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + char *icons_loc = dStrconcat(dGethomedir(), "/.icons", NULL); + if (unveil(icons_loc, "r") == -1) { + err(1, "unveil failed"); + } + dFree(icons_loc); + char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); + if (unveil(xauth_loc, "r") == -1) { + err(1, "unveil failed"); + } + dFree(xauth_loc); + #endif + // initialize internal modules a_Dpi_init(); a_Dns_init(); I have tried to do the same for downloads.cc, but haven't quite got it yet. Since prefs are not initialized in downloads.cc, I tried to copy that from dillo.cc, but am getting some linker errors when compliling. I probably missed something obvious. Any ideas? ld: error: undefined symbol: a_Prefs_init >>> referenced by downloads.cc >>> downloads_dpi-downloads.o:(main) mv -f .deps/vsource.Tpo .deps/vsource.Po ld: error: undefined symbol: Paths::init() >>> referenced by downloads.cc >>> downloads_dpi-downloads.o:(main) ld: error: undefined symbol: Paths::getPrefsFP(char const*) >>> referenced by downloads.cc >>> downloads_dpi-downloads.o:(main) ld: error: undefined symbol: PrefsParser::parse(__sFILE*) >>> referenced by downloads.cc >>> downloads_dpi-downloads.o:(main) ld: error: undefined symbol: prefs >>> referenced by downloads.cc >>> downloads_dpi-downloads.o:(main) c++: error: linker command failed with exit code 1 (use -v to see invocation) Here is what I tried: --- downloads.cc Sat Jun 29 16:33:08 2024 +++ downloads.cc Tue Jul 30 15:07:04 2024 @@ -18,6 +18,7 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <err.h> #include <errno.h> #include <fcntl.h> #include <ctype.h> @@ -42,6 +43,9 @@ #include <FL/Fl_Box.H> #include <FL/Fl_Button.H> +#include "../src/paths.hh" +#include "../src/prefs.h" +#include "../src/prefsparser.hh" #include "config.h" #include "dpiutil.h" #include "../dpip/dpip.h" @@ -1104,6 +1108,48 @@ static void custLabelMeasure(const Fl_Label* o, int& W int main() { int ww = 420, wh = 85; + FILE *fp; + + // set the default values for the preferences + a_Prefs_init(); + + // create ~/.dillo if not present + Paths::init(); + + // parse dillorc + if ((fp = Paths::getPrefsFP(PATHS_RC_PREFS))) { + PrefsParser::parse(fp); + } + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + if (unveil("/tmp", "rwc") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/fonts", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/bin/wget", "x") == -1) { + err(1, "unveil failed"); + } + char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); + if (unveil(xauth_loc, "r") == -1) { + err(1, "unveil failed"); + } + dFree(xauth_loc); + if (unveil("/usr/local/share/fonts", "r") == -1) { + err(1, "unveil failed"); + } + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + if (unveil(prefs.save_dir, "rwc") == -1) { + err(1, "unveil failed"); + } + unveil(NULL, NULL); + #endif Fl::lock(); Finally, I tried to also get ftp.c to use the 'save_dir' preference, but had less luck. I think the issue comes from the fact that this is a C program, and the others are C++. I'm wondering, is this whole unveil idea realistic at this point? Maybe I'm just too inexperienced to be doing this work. I am quite willing to keep going, but it will probably involve many more messages to the list with problems similar to the ones above. I don't want to overwhelm the list and devs with my beginner programming mistakes, it feels like I'm placing an unfair burden on you. Regards, Alex

July 30, 2024

Re: Updated unveil patch

by a1ex＠dismail.de

Hi Rodrigo, On Sun, 28 Jul 2024 22:45:38 +0200 Rodrigo Arias <rodarima(a)gmail.com> wrote: > Hi Alex, > > On Sun, Jul 28, 2024 at 07:14:04PM +0200, a1ex(a)dismail.de wrote: > >Hi, > > > >Here is a new patch. I have done quite a bit more work on this and > >think it may be close to completion. > > Thank you for the effort! I think this is getting in good shape. > > Some preliminary comments: > ... Well, you've certainly given me a lot to think about here! I had no idea this patch would get so complicated. Thanks for the review and tips. I will try to address some of these items individually, starting from the ones I feel most capable of. This is not exactly "baby's first patch" territory anymore :) Regards, Alex

July 29, 2024

Re: Updated unveil patch

by Rodrigo Arias

Hi Soléne, Happy to see you here! On Sun, Jul 28, 2024 at 09:31:24PM +0200, Solène Rapenne wrote: >you should provide a command line argument to disable sandboxing, so in case >of a problem users can run dillo without sandboxing and see if it works better, >allowing to figure if sandboxing is the root cause of their problem. I don't think there is a need to use a command line argument, as we can add a configuration option in ~/.dillo/dillorc to disable it. >what happens if you have no ~/.dillo when you start dillo with unveil? Dillo will first try to load ~/.dillo/dillorc, if that fails it will try $prefix/etc/dillo/dillorc, and if that fails too will use the internal defaults. I propose to let the default value for the unveil in the configuration option enabled by default (which will only take effect when unveil support is compiled in). To disable it for testing purposes one can edit $prefix/etc/dillo/dillorc or copy it to ~/.dillo/dillorc and change it there (this is the default workflow for other options too). >why do you need to unveil the same directories multiple times in the >code? Dillo is composed of several plugins that work as separate programs. Including a Dillo plugin daemon (dpid) and several other builtin programs (dpis). Each of those programs is being constrained with unveil differently, as they have different requirements. Best, Rodrigo.

July 28, 2024

Re: Updated unveil patch

by Rodrigo Arias

Hi Alex, On Sun, Jul 28, 2024 at 07:14:04PM +0200, a1ex(a)dismail.de wrote: >Hi, > >Here is a new patch. I have done quite a bit more work on this and >think it may be close to completion. Thank you for the effort! I think this is getting in good shape. Some preliminary comments: Even if you compile for OpenBSD, unveil() was not introduced until OpenBSD 6.4, so there is the possibility that is not available for an user building Dillo for an old OpenBSD. I recommend you add a configure switch (in configure.ac) to enable or disable unveil(). By default you can keep it disabled and let the user enable it manually, until we have more feedback to make it enabled by default. Maybe by defining ENABLE_UNVEIL? You can take the --enable-svg and ENABLE_SVG as an example. Dillo from OpenBSD ports may enable it by default adding --enable-unveil, so users test it without having to do any extra configuration. You should also make a dillorc configure option to enable/disable the unveil feature in runtime, so it can help debug problems. You can make the dillorc option enabled by default, which will only take action when unveil support has been compiled in. >The '~/Downloads' directory has been unveiled to match the behavior of >Firefox and Chromium on OpenBSD, but Dillo's default of '/tmp' >continues to work as well. The download directory is set in the dillorc configuration file, and can be any other place. Is it viable to read the configuration first and then unveil the appropriate directory? >I have also made sure everything works fine when there is no ~/.dillo >directory, Dillo can create it, and also can access the system defaults >in '/usr/local/etc/dillo'. This is also controlled by the prefix variable, which can be set to any other directory than /usr/local. This should be passed to the code to form the complete path, an example is the DILLO_DOCDIR which is defined as: -DDILLO_DOCDIR='"$(docdir)/"' In src/Makefile.am. You'll want to use the $(sysconfdir) autoconf variable: https://www.gnu.org/prep/standards/html_node/Directory-Variables.html#Direc… >dpid is also now unveiled, as well as all of the stock plugins except >hello.dpi, I didn't see any point to that. > >Here are some other tests which I have run: > >- Regular browsing works fine >- Connect to an FTP site and download a file, also view a text file and > view an image >- Open a text and image file from /tmp and ~/Downloads >- Add/remove bookmarks >- Download a file to /tmp and ~/Downloads >- Save a page to /tmp and ~/Downloads >- View source still works >- Fonts and cursor icons are working correctly >- data: uri works correctly with text and images > >So far everything seems to be fine. I will keep testing, but would >really appreciate some help with reviewing this, there could be some >edge-cases which I missed. As this won't be a simple patch, I suggest you open a PR in GitHub, so the CI can compile your patch revisions for multiple platforms and pass the tests. Otherwise I would have to spend the time to do it myself. I'm thinking if we can automate those tests in the CI. Probably we would need to add OpenBSD as a target first, as we only have FreeBSD. >Regards, >Alex > > > >diff -upr a/dpi/bookmarks.c b/dpi/bookmarks.c >--- a/dpi/bookmarks.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/bookmarks.c Sun Jul 28 16:21:05 2024 >@@ -25,6 +25,7 @@ > #include <stddef.h> > #include <string.h> > #include <unistd.h> >+#include <err.h> > #include <errno.h> > #include <ctype.h> > #include <sys/socket.h> >@@ -1616,6 +1617,16 @@ int main(void) { > socklen_t address_size; > char *tok; > Dsh *sh; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); The err() function is non-portable, please use Dillo MSG* macros or perror() + exit(). This should be caught by the CI. Also, ensure the indentation is kept at 3 characters (not my decision). Would it make sense to add an unveil() wrapper? This would make it easier to integrate with other platforms than OpenBSD (and you also get the error checking in one place). Something like this: void dUnveil(const char *path, const char *perm) { #ifdef USE_UNVEIL #ifdef __OpenBSD__ if (unveil(path, perm) == -1) { MSG("unveil(%s, %s) failed: %s\n", path, perm, strerror(errno)); exit(1); } #endif /* Other platforms... */ #endif } Not sure if we can make all those repeated calls into something that we can reuse for all dpis and dillo main too. >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > /* Arrange the cleanup function for terminations via exit() */ > atexit(cleanup); >diff -upr a/dpi/cookies.c b/dpi/cookies.c >--- a/dpi/cookies.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/cookies.c Sun Jul 28 16:21:05 2024 >@@ -39,6 +39,7 @@ int main(void) > #include <fcntl.h> > #include <unistd.h> > #include <errno.h> >+#include <err.h> > #include <stddef.h> > #include <string.h> > #include <stdlib.h> >@@ -1643,6 +1644,16 @@ int main(void) { > int sock_fd, code; > char *buf; > Dsh *sh; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > /* Arrange the cleanup function for terminations via exit() */ > atexit(cleanup); >diff -upr a/dpi/datauri.c b/dpi/datauri.c >--- a/dpi/datauri.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/datauri.c Sun Jul 28 16:21:05 2024 >@@ -12,6 +12,7 @@ > */ > > #include <unistd.h> >+#include <err.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> >@@ -289,6 +290,19 @@ int main(void) > unsigned char *data; > int rc; > size_t data_size = 0; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > /* Initialize the SockHandler */ > sh = a_Dpip_dsh_new(STDIN_FILENO, STDOUT_FILENO, 8*1024); >diff -upr a/dpi/downloads.cc b/dpi/downloads.cc >--- a/dpi/downloads.cc Sat Jun 29 16:33:08 2024 >+++ b/dpi/downloads.cc Sun Jul 28 16:21:05 2024 >@@ -18,6 +18,7 @@ > #include <stdlib.h> > #include <string.h> > #include <unistd.h> >+#include <err.h> > #include <errno.h> > #include <fcntl.h> > #include <ctype.h> >@@ -1104,6 +1105,38 @@ static void custLabelMeasure(const Fl_Label* o, >int& W int main() > { > int ww = 420, wh = 85; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } Not sure if you may need other user-defined places for fonts. >+ if (unveil("/usr/local/bin/wget", "x") == -1) { >+ err(1, "unveil failed"); >+ } You should find wget by locating it in the $PATH, not assuming it would be here. Users may place their own wget binary somewhere else and this would break the downloads. >+ char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); >+ if (unveil(xauth_loc, "r") == -1) { >+ err(1, "unveil failed"); >+ } The .Xauthority file should be read from $AUTHORITY and then from there if not set. >+ dFree(xauth_loc); >+ if (unveil("/usr/local/share/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } Trailing whitespaces here^ >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } I'll suggest adding another rule to protect ~/.dillo/dillorc from modification. >+ dFree(dil_loc); >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); Same here, the downloads directory is given by the dillorc, you cannot assume it will be set there. >+ if (unveil(dl_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ unveil(NULL, NULL); >+ #endif > > Fl::lock(); > >diff -upr a/dpi/file.c b/dpi/file.c >--- a/dpi/file.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/file.c Sun Jul 28 16:21:05 2024 >@@ -22,6 +22,7 @@ > #include <stdlib.h> > #include <string.h> > #include <unistd.h> >+#include <err.h> > #include <sys/select.h> > #include <sys/socket.h> > #include <sys/stat.h> >@@ -1070,6 +1071,23 @@ int main(void) > socklen_t sin_sz; > int sock_fd, c_st, st = 1; > >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rw") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); >+ if (unveil(dl_loc, "rw") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } Not sure if we want to constraint file:// like this. What if we are using Dillo to read local HTML files in ~/? >+ unveil(NULL, NULL); >+ #endif >+ > /* Arrange the cleanup function for abnormal terminations */ > if (signal (SIGINT, termination_handler) == SIG_IGN) > >diff -upr a/dpi/ftp.c b/dpi/ftp.c >--- a/dpi/ftp.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/ftp.c Sun Jul 28 16:21:05 2024 >@@ -29,6 +29,7 @@ > */ > > #include <unistd.h> >+#include <err.h> > #include <sys/types.h> > #include <sys/socket.h> > #include <sys/un.h> >@@ -281,6 +282,28 @@ int main(int argc, char **argv) > char *dpip_tag = NULL, *cmd = NULL, *url = NULL, *url2 = NULL; > int st, rc; > char *p, *d_cmd; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/bin/wget", "x") == -1) { >+ err(1, "unveil failed"); >+ } Notice wget may need ~/.netrc to access FTP files. But maybe it is good to leave it out. >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); >+ if (unveil(dl_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ unveil(NULL, NULL); >+ #endif >+ > > /* wget may need to write a temporary file... */ > rc = chdir("/tmp"); >diff -upr a/dpi/vsource.c b/dpi/vsource.c >--- a/dpi/vsource.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/vsource.c Sun Jul 28 16:21:05 2024 >@@ -13,6 +13,7 @@ > */ > > #include <unistd.h> >+#include <err.h> > #include <sys/types.h> > #include <stdio.h> > #include <stdlib.h> >@@ -172,6 +173,16 @@ int main(void) > int data_size; > char *dpip_tag, *cmd = NULL, *cmd2 = NULL, *url = NULL, *size_str = >NULL; char *d_cmd; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > _MSG("starting...\n"); > //sleep(20); >diff -upr a/dpid/main.c b/dpid/main.c >--- a/dpid/main.c Sat Jun 29 16:33:08 2024 >+++ b/dpid/main.c Sun Jul 28 16:21:30 2024 >@@ -19,6 +19,7 @@ > > #include <errno.h> /* for ckd_write */ > #include <unistd.h> /* for ckd_write */ >+#include <err.h> > #include <stdlib.h> /* for exit */ > #include <assert.h> /* for assert */ > #include <sys/stat.h> /* for umask */ >@@ -236,6 +237,21 @@ int main(void) > services_list = NULL; > //daemon(0,0); /* Use 0,1 for feedback */ > /* TODO: call setsid() ?? */ >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/usr/local/lib/dillo", "rx") == -1) { >+ err(1, "unveil failed"); >+ } Plugins can also be found on the dpi_dir directory defined by the user in the .dillo/dpidrc, so we would need to parse it first. >+ if (unveil("/usr/local/etc/dillo", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { I would also protect dpidrc from writing as well as dillorc. >+ err(1, "unveil failed"); >+ } >+ unveil(NULL, NULL); I assume exec dpis don't inherit the unveil() configuration (?) >+ #endif > > /* Allow read and write access, but only for the user. > * TODO: can this cause trouble with umount? */ >diff -upr a/src/dillo.cc b/src/dillo.cc >--- a/src/dillo.cc Sat Jun 29 16:33:08 2024 >+++ b/src/dillo.cc Sun Jul 28 16:33:29 2024 >@@ -23,6 +23,7 @@ > > #include <stdio.h> > #include <unistd.h> >+#include <err.h> > #include <stdlib.h> > #include <time.h> > #include <sys/types.h> >@@ -396,6 +397,47 @@ int main(int argc, char **argv) > > srand((uint_t)(time(0) ^ getpid())); > >+ // unveil() >+ #ifdef __OpenBSD__ >+ if (unveil("/usr/local/share/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/etc/dillo", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/bin/dpid", "x") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/resolv.conf", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/ssl/cert.pem", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); >+ if (unveil(dl_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); > >+ if (unveil(xauth_loc, "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(xauth_loc); >+ unveil(NULL, NULL); >+ #endif >+ > // Some OSes exit dillo without this (not GNU/Linux). > signal(SIGPIPE, SIG_IGN); > // Establish our custom SIGCHLD handler > >diff -upr a/dpi/bookmarks.c b/dpi/bookmarks.c >--- a/dpi/bookmarks.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/bookmarks.c Sun Jul 28 16:21:05 2024 >@@ -25,6 +25,7 @@ > #include <stddef.h> > #include <string.h> > #include <unistd.h> >+#include <err.h> > #include <errno.h> > #include <ctype.h> > #include <sys/socket.h> >@@ -1616,6 +1617,16 @@ int main(void) { > socklen_t address_size; > char *tok; > Dsh *sh; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > /* Arrange the cleanup function for terminations via exit() */ > atexit(cleanup); >diff -upr a/dpi/cookies.c b/dpi/cookies.c >--- a/dpi/cookies.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/cookies.c Sun Jul 28 16:21:05 2024 >@@ -39,6 +39,7 @@ int main(void) > #include <fcntl.h> > #include <unistd.h> > #include <errno.h> >+#include <err.h> > #include <stddef.h> > #include <string.h> > #include <stdlib.h> >@@ -1643,6 +1644,16 @@ int main(void) { > int sock_fd, code; > char *buf; > Dsh *sh; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > /* Arrange the cleanup function for terminations via exit() */ > atexit(cleanup); >diff -upr a/dpi/datauri.c b/dpi/datauri.c >--- a/dpi/datauri.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/datauri.c Sun Jul 28 16:21:05 2024 >@@ -12,6 +12,7 @@ > */ > > #include <unistd.h> >+#include <err.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> >@@ -289,6 +290,19 @@ int main(void) > unsigned char *data; > int rc; > size_t data_size = 0; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > /* Initialize the SockHandler */ > sh = a_Dpip_dsh_new(STDIN_FILENO, STDOUT_FILENO, 8*1024); >diff -upr a/dpi/downloads.cc b/dpi/downloads.cc >--- a/dpi/downloads.cc Sat Jun 29 16:33:08 2024 >+++ b/dpi/downloads.cc Sun Jul 28 16:21:05 2024 >@@ -18,6 +18,7 @@ > #include <stdlib.h> > #include <string.h> > #include <unistd.h> >+#include <err.h> > #include <errno.h> > #include <fcntl.h> > #include <ctype.h> >@@ -1104,6 +1105,38 @@ static void custLabelMeasure(const Fl_Label* o, int& W > int main() > { > int ww = 420, wh = 85; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/bin/wget", "x") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); >+ if (unveil(xauth_loc, "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(xauth_loc); >+ if (unveil("/usr/local/share/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); >+ if (unveil(dl_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ unveil(NULL, NULL); >+ #endif > > Fl::lock(); > >diff -upr a/dpi/file.c b/dpi/file.c >--- a/dpi/file.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/file.c Sun Jul 28 16:21:05 2024 >@@ -22,6 +22,7 @@ > #include <stdlib.h> > #include <string.h> > #include <unistd.h> >+#include <err.h> > #include <sys/select.h> > #include <sys/socket.h> > #include <sys/stat.h> >@@ -1070,6 +1071,23 @@ int main(void) > socklen_t sin_sz; > int sock_fd, c_st, st = 1; > >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rw") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); >+ if (unveil(dl_loc, "rw") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ unveil(NULL, NULL); >+ #endif >+ > /* Arrange the cleanup function for abnormal terminations */ > if (signal (SIGINT, termination_handler) == SIG_IGN) > >diff -upr a/dpi/ftp.c b/dpi/ftp.c >--- a/dpi/ftp.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/ftp.c Sun Jul 28 16:21:05 2024 >@@ -29,6 +29,7 @@ > */ > > #include <unistd.h> >+#include <err.h> > #include <sys/types.h> > #include <sys/socket.h> > #include <sys/un.h> >@@ -281,6 +282,28 @@ int main(int argc, char **argv) > char *dpip_tag = NULL, *cmd = NULL, *url = NULL, *url2 = NULL; > int st, rc; > char *p, *d_cmd; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/bin/wget", "x") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); >+ if (unveil(dl_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ unveil(NULL, NULL); >+ #endif >+ > > /* wget may need to write a temporary file... */ > rc = chdir("/tmp"); >diff -upr a/dpi/vsource.c b/dpi/vsource.c >--- a/dpi/vsource.c Sat Jun 29 16:33:08 2024 >+++ b/dpi/vsource.c Sun Jul 28 16:21:05 2024 >@@ -13,6 +13,7 @@ > */ > > #include <unistd.h> >+#include <err.h> > #include <sys/types.h> > #include <stdio.h> > #include <stdlib.h> >@@ -172,6 +173,16 @@ int main(void) > int data_size; > char *dpip_tag, *cmd = NULL, *cmd2 = NULL, *url = NULL, *size_str = NULL; > char *d_cmd; >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ unveil(NULL, NULL); >+ #endif > > _MSG("starting...\n"); > //sleep(20); >diff -upr a/dpid/main.c b/dpid/main.c >--- a/dpid/main.c Sat Jun 29 16:33:08 2024 >+++ b/dpid/main.c Sun Jul 28 16:21:30 2024 >@@ -19,6 +19,7 @@ > > #include <errno.h> /* for ckd_write */ > #include <unistd.h> /* for ckd_write */ >+#include <err.h> > #include <stdlib.h> /* for exit */ > #include <assert.h> /* for assert */ > #include <sys/stat.h> /* for umask */ >@@ -236,6 +237,21 @@ int main(void) > services_list = NULL; > //daemon(0,0); /* Use 0,1 for feedback */ > /* TODO: call setsid() ?? */ >+ >+ /* Use unveil on OpenBSD */ >+ #ifdef __OpenBSD__ >+ if (unveil("/usr/local/lib/dillo", "rx") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/etc/dillo", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ unveil(NULL, NULL); >+ #endif > > /* Allow read and write access, but only for the user. > * TODO: can this cause trouble with umount? */ >diff -upr a/src/dillo.cc b/src/dillo.cc >--- a/src/dillo.cc Sat Jun 29 16:33:08 2024 >+++ b/src/dillo.cc Sun Jul 28 16:33:29 2024 >@@ -23,6 +23,7 @@ > > #include <stdio.h> > #include <unistd.h> >+#include <err.h> > #include <stdlib.h> > #include <time.h> > #include <sys/types.h> >@@ -396,6 +397,47 @@ int main(int argc, char **argv) > > srand((uint_t)(time(0) ^ getpid())); > >+ // unveil() >+ #ifdef __OpenBSD__ >+ if (unveil("/usr/local/share/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/etc/dillo", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/tmp", "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/usr/local/bin/dpid", "x") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/fonts", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/resolv.conf", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ if (unveil("/etc/ssl/cert.pem", "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); >+ if (unveil(dl_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dl_loc); >+ char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); >+ if (unveil(dil_loc, "rwc") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(dil_loc); >+ char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); >+ if (unveil(xauth_loc, "r") == -1) { >+ err(1, "unveil failed"); >+ } >+ dFree(xauth_loc); >+ unveil(NULL, NULL); >+ #endif >+ > // Some OSes exit dillo without this (not GNU/Linux). > signal(SIGPIPE, SIG_IGN); > // Establish our custom SIGCHLD handler >_______________________________________________ >Dillo-dev mailing list -- dillo-dev(a)mailman3.com >To unsubscribe send an email to dillo-dev-leave(a)mailman3.com

July 28, 2024

Re: Updated unveil patch

by Solène Rapenne

Le 28/07/2024 à 19:14, a1ex(a)dismail.de a écrit : > Hi, > > Here is a new patch. I have done quite a bit more work on this and > think it may be close to completion. > > The '~/Downloads' directory has been unveiled to match the behavior of > Firefox and Chromium on OpenBSD, but Dillo's default of '/tmp' > continues to work as well. > > I have also made sure everything works fine when there is no ~/.dillo > directory, Dillo can create it, and also can access the system defaults > in '/usr/local/etc/dillo'. > > dpid is also now unveiled, as well as all of the stock plugins except > hello.dpi, I didn't see any point to that. > > Here are some other tests which I have run: > > - Regular browsing works fine > - Connect to an FTP site and download a file, also view a text file and > view an image > - Open a text and image file from /tmp and ~/Downloads > - Add/remove bookmarks > - Download a file to /tmp and ~/Downloads > - Save a page to /tmp and ~/Downloads > - View source still works > - Fonts and cursor icons are working correctly > - data: uri works correctly with text and images > > So far everything seems to be fine. I will keep testing, but would > really appreciate some help with reviewing this, there could be some > edge-cases which I missed. > > Regards, > Alex > > you should provide a command line argument to disable sandboxing, so in case of a problem users can run dillo without sandboxing and see if it works better, allowing to figure if sandboxing is the root cause of their problem. what happens if you have no ~/.dillo when you start dillo with unveil? why do you need to unveil the same directories multiple times in the code?

July 28, 2024

Updated unveil patch

by a1ex＠dismail.de

Hi, Here is a new patch. I have done quite a bit more work on this and think it may be close to completion. The '~/Downloads' directory has been unveiled to match the behavior of Firefox and Chromium on OpenBSD, but Dillo's default of '/tmp' continues to work as well. I have also made sure everything works fine when there is no ~/.dillo directory, Dillo can create it, and also can access the system defaults in '/usr/local/etc/dillo'. dpid is also now unveiled, as well as all of the stock plugins except hello.dpi, I didn't see any point to that. Here are some other tests which I have run: - Regular browsing works fine - Connect to an FTP site and download a file, also view a text file and view an image - Open a text and image file from /tmp and ~/Downloads - Add/remove bookmarks - Download a file to /tmp and ~/Downloads - Save a page to /tmp and ~/Downloads - View source still works - Fonts and cursor icons are working correctly - data: uri works correctly with text and images So far everything seems to be fine. I will keep testing, but would really appreciate some help with reviewing this, there could be some edge-cases which I missed. Regards, Alex diff -upr a/dpi/bookmarks.c b/dpi/bookmarks.c --- a/dpi/bookmarks.c Sat Jun 29 16:33:08 2024 +++ b/dpi/bookmarks.c Sun Jul 28 16:21:05 2024 @@ -25,6 +25,7 @@ #include <stddef.h> #include <string.h> #include <unistd.h> +#include <err.h> #include <errno.h> #include <ctype.h> #include <sys/socket.h> @@ -1616,6 +1617,16 @@ int main(void) { socklen_t address_size; char *tok; Dsh *sh; + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + unveil(NULL, NULL); + #endif /* Arrange the cleanup function for terminations via exit() */ atexit(cleanup); diff -upr a/dpi/cookies.c b/dpi/cookies.c --- a/dpi/cookies.c Sat Jun 29 16:33:08 2024 +++ b/dpi/cookies.c Sun Jul 28 16:21:05 2024 @@ -39,6 +39,7 @@ int main(void) #include <fcntl.h> #include <unistd.h> #include <errno.h> +#include <err.h> #include <stddef.h> #include <string.h> #include <stdlib.h> @@ -1643,6 +1644,16 @@ int main(void) { int sock_fd, code; char *buf; Dsh *sh; + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + unveil(NULL, NULL); + #endif /* Arrange the cleanup function for terminations via exit() */ atexit(cleanup); diff -upr a/dpi/datauri.c b/dpi/datauri.c --- a/dpi/datauri.c Sat Jun 29 16:33:08 2024 +++ b/dpi/datauri.c Sun Jul 28 16:21:05 2024 @@ -12,6 +12,7 @@ */ #include <unistd.h> +#include <err.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -289,6 +290,19 @@ int main(void) unsigned char *data; int rc; size_t data_size = 0; + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + if (unveil("/tmp", "rwc") == -1) { + err(1, "unveil failed"); + } + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + unveil(NULL, NULL); + #endif /* Initialize the SockHandler */ sh = a_Dpip_dsh_new(STDIN_FILENO, STDOUT_FILENO, 8*1024); diff -upr a/dpi/downloads.cc b/dpi/downloads.cc --- a/dpi/downloads.cc Sat Jun 29 16:33:08 2024 +++ b/dpi/downloads.cc Sun Jul 28 16:21:05 2024 @@ -18,6 +18,7 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <err.h> #include <errno.h> #include <fcntl.h> #include <ctype.h> @@ -1104,6 +1105,38 @@ static void custLabelMeasure(const Fl_Label* o, int& W int main() { int ww = 420, wh = 85; + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + if (unveil("/tmp", "rwc") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/fonts", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/bin/wget", "x") == -1) { + err(1, "unveil failed"); + } + char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); + if (unveil(xauth_loc, "r") == -1) { + err(1, "unveil failed"); + } + dFree(xauth_loc); + if (unveil("/usr/local/share/fonts", "r") == -1) { + err(1, "unveil failed"); + } + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); + if (unveil(dl_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dl_loc); + unveil(NULL, NULL); + #endif Fl::lock(); diff -upr a/dpi/file.c b/dpi/file.c --- a/dpi/file.c Sat Jun 29 16:33:08 2024 +++ b/dpi/file.c Sun Jul 28 16:21:05 2024 @@ -22,6 +22,7 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <err.h> #include <sys/select.h> #include <sys/socket.h> #include <sys/stat.h> @@ -1070,6 +1071,23 @@ int main(void) socklen_t sin_sz; int sock_fd, c_st, st = 1; + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + if (unveil("/tmp", "rw") == -1) { + err(1, "unveil failed"); + } + char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); + if (unveil(dl_loc, "rw") == -1) { + err(1, "unveil failed"); + } + dFree(dl_loc); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + unveil(NULL, NULL); + #endif + /* Arrange the cleanup function for abnormal terminations */ if (signal (SIGINT, termination_handler) == SIG_IGN) diff -upr a/dpi/ftp.c b/dpi/ftp.c --- a/dpi/ftp.c Sat Jun 29 16:33:08 2024 +++ b/dpi/ftp.c Sun Jul 28 16:21:05 2024 @@ -29,6 +29,7 @@ */ #include <unistd.h> +#include <err.h> #include <sys/types.h> #include <sys/socket.h> #include <sys/un.h> @@ -281,6 +282,28 @@ int main(int argc, char **argv) char *dpip_tag = NULL, *cmd = NULL, *url = NULL, *url2 = NULL; int st, rc; char *p, *d_cmd; + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + if (unveil("/tmp", "rwc") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/bin/wget", "x") == -1) { + err(1, "unveil failed"); + } + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); + if (unveil(dl_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dl_loc); + unveil(NULL, NULL); + #endif + /* wget may need to write a temporary file... */ rc = chdir("/tmp"); diff -upr a/dpi/vsource.c b/dpi/vsource.c --- a/dpi/vsource.c Sat Jun 29 16:33:08 2024 +++ b/dpi/vsource.c Sun Jul 28 16:21:05 2024 @@ -13,6 +13,7 @@ */ #include <unistd.h> +#include <err.h> #include <sys/types.h> #include <stdio.h> #include <stdlib.h> @@ -172,6 +173,16 @@ int main(void) int data_size; char *dpip_tag, *cmd = NULL, *cmd2 = NULL, *url = NULL, *size_str = NULL; char *d_cmd; + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "r") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + unveil(NULL, NULL); + #endif _MSG("starting...\n"); //sleep(20); diff -upr a/dpid/main.c b/dpid/main.c --- a/dpid/main.c Sat Jun 29 16:33:08 2024 +++ b/dpid/main.c Sun Jul 28 16:21:30 2024 @@ -19,6 +19,7 @@ #include <errno.h> /* for ckd_write */ #include <unistd.h> /* for ckd_write */ +#include <err.h> #include <stdlib.h> /* for exit */ #include <assert.h> /* for assert */ #include <sys/stat.h> /* for umask */ @@ -236,6 +237,21 @@ int main(void) services_list = NULL; //daemon(0,0); /* Use 0,1 for feedback */ /* TODO: call setsid() ?? */ + + /* Use unveil on OpenBSD */ + #ifdef __OpenBSD__ + if (unveil("/usr/local/lib/dillo", "rx") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/etc/dillo", "r") == -1) { + err(1, "unveil failed"); + } + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + unveil(NULL, NULL); + #endif /* Allow read and write access, but only for the user. * TODO: can this cause trouble with umount? */ diff -upr a/src/dillo.cc b/src/dillo.cc --- a/src/dillo.cc Sat Jun 29 16:33:08 2024 +++ b/src/dillo.cc Sun Jul 28 16:33:29 2024 @@ -23,6 +23,7 @@ #include <stdio.h> #include <unistd.h> +#include <err.h> #include <stdlib.h> #include <time.h> #include <sys/types.h> @@ -396,6 +397,47 @@ int main(int argc, char **argv) srand((uint_t)(time(0) ^ getpid())); + // unveil() + #ifdef __OpenBSD__ + if (unveil("/usr/local/share/fonts", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/etc/dillo", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/tmp", "rwc") == -1) { + err(1, "unveil failed"); + } + if (unveil("/usr/local/bin/dpid", "x") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/fonts", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/resolv.conf", "r") == -1) { + err(1, "unveil failed"); + } + if (unveil("/etc/ssl/cert.pem", "r") == -1) { + err(1, "unveil failed"); + } + char *dl_loc = dStrconcat(dGethomedir(), "/Downloads", NULL); + if (unveil(dl_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dl_loc); + char *dil_loc = dStrconcat(dGethomedir(), "/.dillo", NULL); + if (unveil(dil_loc, "rwc") == -1) { + err(1, "unveil failed"); + } + dFree(dil_loc); + char *xauth_loc = dStrconcat(dGethomedir(), "/.Xauthority", NULL); + if (unveil(xauth_loc, "r") == -1) { + err(1, "unveil failed"); + } + dFree(xauth_loc); + unveil(NULL, NULL); + #endif + // Some OSes exit dillo without this (not GNU/Linux). signal(SIGPIPE, SIG_IGN); // Establish our custom SIGCHLD handler

July 28, 2024