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();
Hi, On Thu, Aug 01, 2024 at 05:12:09PM +0200, a1ex@dismail.de wrote:
Hi,
Here is version 2 of my unveil patch. Many thanks to Rodrigo for his kind assistance!
Thank you Alex! I see this is progressing very well :-) I'll be doing some tests with your patch when I have a moment. I'll need to get an OpenBSD VM for testing probably. In the meanwhile, I just wanted to link this compatibility unveil() function for Linux, so I don't forget the link: https://github.com/rpki-client/rpki-client-portable/blob/master/compat/unvei... I think a simple approach to test this is to find out if we cannot read the ~/.ssh/id_rsa private keys from Dillo or plugins by any means. So far I think we could read them by forging a call to the file plugin and maybe by forking a new process that doesn't have the unveil() protections and then reading ~/.ssh/id_rsa from there. I saw that the OpenBSD developers have placed an "uploads" directory to place files to be available to be read from the browser, and only that directory is allowed (along with the downloads dir). It doesn't sound a bad idea to me. This way we can avoid leaving ~/ unprotected in the file plugin. What do you think?
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)
I assume you could reuse the same prefs parser from Dillo, but we would need to link the DPIs with some of the code that is now only being linked in the browser.
- 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 +}
We can probably put the dUnveil() wrapper into dlib/ so it is available in all the other parts.
/* * -- MAIN ------------------------------------------------------------------- @@ -1617,6 +1631,16 @@ int main(void) { char *tok; Dsh *sh;
+ /* Use unveil on OpenBSD */ + #ifdef ENABLE_UNVEIL + #ifdef __OpenBSD__
I think the #ifdef ENABLE_UNVEIL is enough here, as we may have other implementations handling it transparently for other platforms. Same for the other cases except inside dUnveil().
+ 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");
What happens if someone puts save_dir to $HOME?, should we restrict it maybe?
+ 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();
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();
_______________________________________________ Dillo-dev mailing list -- dillo-dev@mailman3.com To unsubscribe send an email to dillo-dev-leave@mailman3.com
Hi. Sorry I can't help dillo more. I'm always very busy with my work. El vie, 2 ago 2024 a las 1:24, Rodrigo Arias (<rodarima@gmail.com>) escribió:
To-Do: - Add prefs parsing to plugins to get 'save_dir' (may need help here)
I assume you could reuse the same prefs parser from Dillo, but we would need to link the DPIs with some of the code that is now only being linked in the browser.
You could use dParser_parse_rc_line() from dlib/dlib.[hc] ../dlib/dlib.c:int dParser_parse_rc_line(char **line, char **name, char **value) ../dlib/dlib.h:int dParser_parse_rc_line(char **line, char **name, char **value); ./srch dParser_parse_rc_line keys.cc: st = dParser_parse_rc_line(&line, &keycomb, &command); prefsparser.cc: st = dParser_parse_rc_line(&line, &name, &value); ../dpid/dpid.c: st = dParser_parse_rc_line(&line, &service, &path); Salud! Diego
Hi Rodrigo, On Fri, 2 Aug 2024 01:23:56 +0200 Rodrigo Arias <rodarima@gmail.com> wrote:
I'll be doing some tests with your patch when I have a moment. I'll need to get an OpenBSD VM for testing probably.
Sounds great! Let me know if you need any help getting OpenBSD set up.
In the meanwhile, I just wanted to link this compatibility unveil() function for Linux, so I don't forget the link:
https://github.com/rpki-client/rpki-client-portable/blob/master/compat/unvei...
Nice find. Not sure if this will be an issue: /* * This is by no means a proper implementation of unveil() but it is * good enough for rpki-client which uses only a few features. * rpki-client only uses 'r', 'rwc' and 'x' and for 'x' we need to do * some horrible hacks. */
I think a simple approach to test this is to find out if we cannot read the ~/.ssh/id_rsa private keys from Dillo or plugins by any means. So far I think we could read them by forging a call to the file plugin and maybe by forking a new process that doesn't have the unveil() protections and then reading ~/.ssh/id_rsa from there.
Right, but this attack vector seems pretty unlikely, and assumes the system is already somehow compromised (by calling a malicious program on the filesystem in an unveiled location).
I saw that the OpenBSD developers have placed an "uploads" directory to place files to be available to be read from the browser, and only that directory is allowed (along with the downloads dir). It doesn't sound a bad idea to me. This way we can avoid leaving ~/ unprotected in the file plugin. What do you think?
I believe the approach OpenBSD takes with Firefox and Chromium is to only give the browser access to '~/Downloads', which can be used for both downloads and uploads. Basically, users on OpenBSD are already used to this restriction, most likely already have a ~/Downloads directory, and don't think they would be too troubled to have this restriction in Dillo, especially if it is well documented.
To-Do: - Add prefs parsing to plugins to get 'save_dir' (may need help here)
I assume you could reuse the same prefs parser from Dillo, but we would need to link the DPIs with some of the code that is now only being linked in the browser.
I have made some efforts to do that, but was not successful yet. I included an example earlier of downloads.cc where I made some progress, but still something missing. I still wonder if it makes more sense to just use a fixed location (~/Downloads) if we are calling unveil, and override the save_dir with that. This ties back to my response above.
We can probably put the dUnveil() wrapper into dlib/ so it is available in all the other parts.
Thats a good point, I agree, and will work on it.
+ dUnveil(prefs.save_dir, "rwc");
What happens if someone puts save_dir to $HOME?, should we restrict it maybe?
To give access to ~/, and yet still protect ~/.ssh, we could do something like this (simplified example): unveil("/home/user", "rwc"); unveil("/home/user/.ssh", ""); But again, maybe it's better to override save_dir entirely if we are using unveil. Regards, Alex
Hi, On Fri, Aug 02, 2024 at 01:04:32PM +0200, a1ex@dismail.de wrote:
I think a simple approach to test this is to find out if we cannot read the ~/.ssh/id_rsa private keys from Dillo or plugins by any means. So far I think we could read them by forging a call to the file plugin and maybe by forking a new process that doesn't have the unveil() protections and then reading ~/.ssh/id_rsa from there.
Right, but this attack vector seems pretty unlikely, and assumes the system is already somehow compromised (by calling a malicious program on the filesystem in an unveiled location).
I think an RCE bug could allow an attacker to write an exploit in an HTML document (or image or any other thing parsed from the web) that would cause Dillo to execute the fork() syscall and then access those files and transfer the content to a remote host. See: https://en.wikipedia.org/wiki/Return-oriented_programming If this is the case, unless we prevent Dillo from being able to fork I don't think the unveil(2) protection is enough without pledge(2) to prevent this attack. We can test it by putting this code after the last unveil() call, as if Dillo was running an exploit: int ret = fork(); if (ret < 0) { perror("fork failed"); } else if (ret == 0) { /* Child */ FILE *f = fopen("/home/<theuser>/.ssh/id_rsa", "r"); // Adjust path if (f == NULL) { perror("game over exploit, fopen failed"); } else { int c; while ((c = fgetc(f)) != EOF) fputc(c, stdout); fclose(f); } exit(0); } If you see your SSH key in the stdout, the unveil() protection alone is not enough to mitigate this attack (check that the key exists first, I used id_rsa). This would also confirm that unveil() settings don't get inherited in forked children.
To-Do: - Add prefs parsing to plugins to get 'save_dir' (may need help here)
I assume you could reuse the same prefs parser from Dillo, but we would need to link the DPIs with some of the code that is now only being linked in the browser.
I have made some efforts to do that, but was not successful yet. I included an example earlier of downloads.cc where I made some progress, but still something missing.
I still wonder if it makes more sense to just use a fixed location (~/Downloads) if we are calling unveil, and override the save_dir with that. This ties back to my response above.
Maybe for now we can add a simple implementation of the parser just to parse the "save_dir" and the new "enable_unveil" (or similar) option, like dpid is doing for dpi_dir. Parsing the complete dillorc preferences requires splitting the preferences code into something that can be linked standalone in the dpis, which will require more effort.
What happens if someone puts save_dir to $HOME?, should we restrict it maybe?
To give access to ~/, and yet still protect ~/.ssh, we could do something like this (simplified example):
unveil("/home/user", "rwc"); unveil("/home/user/.ssh", "");
But again, maybe it's better to override save_dir entirely if we are using unveil.
I think we could just not allow $HOME to be set as save_dir (or any directory that contains $HOME, like /home) and refuse to start if this is the case. Best, Rodrigo.
Hi Rodrigo, On Sun, 4 Aug 2024 13:14:24 +0200 Rodrigo Arias <rodarima@gmail.com> wrote:
I think an RCE bug could allow an attacker to write an exploit in an HTML document (or image or any other thing parsed from the web) that would cause Dillo to execute the fork() syscall and then access those files and transfer the content to a remote host. See:
https://en.wikipedia.org/wiki/Return-oriented_programming
If this is the case, unless we prevent Dillo from being able to fork I don't think the unveil(2) protection is enough without pledge(2) to prevent this attack.
We can test it by putting this code after the last unveil() call, as if Dillo was running an exploit:
int ret = fork(); if (ret < 0) { perror("fork failed"); } else if (ret == 0) { /* Child */ FILE *f = fopen("/home/<theuser>/.ssh/id_rsa", "r"); // Adjust path if (f == NULL) { perror("game over exploit, fopen failed"); } else { int c; while ((c = fgetc(f)) != EOF) fputc(c, stdout); fclose(f); } exit(0); }
If you see your SSH key in the stdout, the unveil() protection alone is not enough to mitigate this attack (check that the key exists first, I used id_rsa). This would also confirm that unveil() settings don't get inherited in forked children.
I placed that after the last unveil call and made sure I used a valid key path. Here is the console output: game over exploit, fopen failed: No such file or directory
Maybe for now we can add a simple implementation of the parser just to parse the "save_dir" and the new "enable_unveil" (or similar) option, like dpid is doing for dpi_dir.
Ok thanks, I will try to use that as an example and see if I can get anywhere with it.
I think we could just not allow $HOME to be set as save_dir (or any directory that contains $HOME, like /home) and refuse to start if this is the case.
Not sure I understand what this achieves. So '/home/user' would be blocked, but '/home/user/foo' would be allowed? Why not just explicitly block access to ~/.ssh with unveil, and then let the user do whatever they want after that? Regards, Alex
Hi, On Sun, Aug 04, 2024 at 05:14:52PM +0200, a1ex@dismail.de wrote:
If you see your SSH key in the stdout, the unveil() protection alone is not enough to mitigate this attack (check that the key exists first, I used id_rsa). This would also confirm that unveil() settings don't get inherited in forked children.
I placed that after the last unveil call and made sure I used a valid key path. Here is the console output:
game over exploit, fopen failed: No such file or directory
From what I am seeing, fork() inherits the unveil, and exec() does not.
Hmm, I guess that makes more sense. I was thinking we could read the preferences only from Dillo, then unveil accordingly, and then all DPIs would already get the unveiled environment. But as you comment, on the exec() the unveil will reset, so we will need to re-read the dillorc file again. I think this is also a good approach in case someone runs the dpid by hand, so it adheres to what dillorc says.
Maybe for now we can add a simple implementation of the parser just to parse the "save_dir" and the new "enable_unveil" (or similar) option, like dpid is doing for dpi_dir.
Ok thanks, I will try to use that as an example and see if I can get anywhere with it.
Thanks!
I think we could just not allow $HOME to be set as save_dir (or any directory that contains $HOME, like /home) and refuse to start if this is the case.
Not sure I understand what this achieves. So '/home/user' would be blocked, but '/home/user/foo' would be allowed? Why not just explicitly block access to ~/.ssh with unveil, and then let the user do whatever they want after that?
Yeah, but there may be a lot of other unknown directories we don't want Dillo to access to (~/.config, ~/.cache, ...). So I think is a good approach to block all directories in home, except the downloads one. Another attack may involve encrypting ~/Pictures and asking for a ransom, so we should prevent any access to home that is not required. Best, Rodrigo.
Hi Rodrigo, On Mon, 5 Aug 2024 15:06:04 +0200 Rodrigo Arias <rodarima@gmail.com> wrote:
I think we could just not allow $HOME to be set as save_dir (or any directory that contains $HOME, like /home) and refuse to start if this is the case.
Sorry, I guess this is the part that confused me: "(or any directory that contains $HOME, like /home)" I agree with it, just not sure how to implement while still allowing a save_dir like '$HOME/Downloads', or '/home/user/Downloads'. Maybe it's a simple thing, but any help would be appreciated!
Not sure I understand what this achieves. So '/home/user' would be blocked, but '/home/user/foo' would be allowed? Why not just explicitly block access to ~/.ssh with unveil, and then let the user do whatever they want after that?
Yeah, but there may be a lot of other unknown directories we don't want Dillo to access to (~/.config, ~/.cache, ...). So I think is a good approach to block all directories in home, except the downloads one.
Another attack may involve encrypting ~/Pictures and asking for a ransom, so we should prevent any access to home that is not required.
I think it's unlikely that a user would explicitly choose $HOME as save_dir, but agree that it would be reasonable to take the precaution just in case. Thanks, Alex
Hi, On Mon, Aug 05, 2024 at 05:42:54PM +0200, a1ex@dismail.de wrote:
Hi Rodrigo,
On Mon, 5 Aug 2024 15:06:04 +0200 Rodrigo Arias <rodarima@gmail.com> wrote:
I think we could just not allow $HOME to be set as save_dir (or any directory that contains $HOME, like /home) and refuse to start if this is the case.
Sorry, I guess this is the part that confused me: "(or any directory that contains $HOME, like /home)"
I agree with it, just not sure how to implement while still allowing a save_dir like '$HOME/Downloads', or '/home/user/Downloads'.
Maybe it's a simple thing, but any help would be appreciated!
By "contains" I mean that $HOME cannot be inside save_dir: bad: save_dir=/ bad: save_dir=/home bad: save_dir=/home/<theuser> ok: save_dir=/home/<theuser>/foo ok: save_dir=/tmp Here is a very simple check (doesn't handle /../ and other problems): const char *home = dGethomedir(); const char *save = prefs.save_dir; int nsave = strlen(save); int nhome = strlen(home); if (nsave <= nhome) { /* Prevent save_dir="/home" and save_dir=$HOME */ if (strncmp(save, home, nsave) == 0) { MSG("save_dir cannot contain home\n"); exit(1); } } Best, Rodrigo.
Hi, On Sun, 4 Aug 2024 13:14:24 +0200 Rodrigo Arias <rodarima@gmail.com> wrote:
If this is the case, unless we prevent Dillo from being able to fork I don't think the unveil(2) protection is enough without pledge(2) to prevent this attack.
From what I am seeing, fork() inherits the unveil, and exec() does not. -Alex
participants (3)
-
a1ex＠dismail.de -
Diego -
Rodrigo Arias