Hi Alex, On Sat, Jul 20, 2024 at 09:03:48PM +0200, a1ex@dismail.de wrote:
Hi Rodrigo,
On Sat, 20 Jul 2024 16:34:47 +0200 Rodrigo Arias <rodarima@gmail.com> wrote:
+ if (unveil("/home", "rwc") == -1) {
We may want to constraint this a bit further, so a malicious actor cannot read anything from /home/.config. Maybe only /home/.dillo and the downloads directory would be suitable?
Absolutely, that was my initial intention, but just wanted to keep the example patch as simple as possible. There are a number of things in $HOME which we probably don't want the browser having access to.
Ups, I meant $HOME, not /home :-) There is a dGethomedir() function in dlib/dlib.h to help with this, which also works with other systems.
+ if (pledge("stdio rpath wpath cpath inet unix dns tty proc prot_exec",
Does this work with plugins, when the dpid daemon is not running?, as I believe it has to fork and exec the dpid program.
I started with a mindset of "whats the bare minimum of permissions we can get away with". But its clear that we would need "exec" as well for full functionality.
I can see clear benefits on restricting the file system access, but do you have in mind which type of attack the pledge() configuration would you help mitigate? I think allowing exec and inet is too permissive and would allow an attacker to easily spawn a remote shell as soon as a RCE bug is found. Maybe we can remove exec by spawning first the dpid daemon and then restricting the exec syscall with pledge. Currently it is only started when a request to a dpi plugin via a_Dpi_ccc().
At some point I may try to submit an improved patch to the OpenBSD ports maintainers. Unfortunately that won't do much for users of Linux and other systems.
I think is a good idea more OpenBSD people review this part, as I'm not very familiar with pledge(2). However, I'm afraid having this patch only in OpenBSD may cause a negative effect, as we may start receiving bug reports that are only happening on OpenBSD with that patch and cannot be reproduced on Linux or other systems without it. Maybe you can make a hardened Dillo port, so people can still test the non-hardened port to determine if the problem comes from these changes. This could be also controlled by a dillorc option (Dillo shouldn't be able to write dillorc), so it can be disabled for testing. The latter could be applied to upstream Dillo if we allow compiling the pledge part conditionally and would benefit from the CI tests on BSD too. Best, Rodrigo.