gsliepen / tinc

a VPN daemon
http://tinc-vpn.org/
Other
1.87k stars 280 forks source link

Add basic sandboxing for OpenBSD #379

Closed hg closed 2 years ago

hg commented 2 years ago

Uses pledge()/unveil() to limit what tincd/tinc can do after initialization finishes.

Paths are only restricted for tincd. It's not difficult to write a similar list for tincctl, but since it's a oneshot program which isn't running for months accepting random connections from around the internet, I don't think it makes much sense to do so, taking support costs into consideration.

sandbox is a meson option (and not a boolean) since it may come in handy when adding support for other operating systems (for example, only if libseccomp is present).

Can't call myself an experienced OpenBSD user, so if anyone knowledgeable is willing to test this — be my guest. I've been running this code for a few days without any issues, but it surely has the potential to cause breakage.


This PR also makes scripts optional by adding a new configuration variable DisableScripts.

Personally, I have no use for them. Network interfaces can be configured through other means.

This is what disabling scripts allows us to do:

Here are promises with DisableScripts = yes:

openbsd$ ps -O pledge | grep [t]incd
20571 stdio,rpath,wpath,cpath,inet,unix,dns    p2  S+pU     0:00.01 ./build/src/tincd -c /tmp/foo -D

and without:

openbsd$ ps -O pledge | grep [t]incd
65412 stdio,rpath,wpath,cpath,inet,unix,dns,proc,exec    p2  S+pU     0:00.01 ./build/src/tincd -c /tmp/foo -D

and here's the full list.

I don't think we can do much better without splitting tincd into multiple restricted processes like OpenBSD does with its daemons.

hg commented 2 years ago

Now, about doing the same for Linux: it's probably going to use seccomp-bpf (maybe through libseccomp, but likely without it since rules are likely to be static, and it's not difficult to write a BPF program for a static rule list).

The question is what approach to use: either allow or deny by default.

Obviously, it would be much better to only allow syscalls we actually use, but I worry about third-party libraries. For example, tinc didn't use getrandom() until recently, but OpenSSL did, and any library could start using any newfangled syscall when some user builds tinc with a newer version we haven't tested yet.

So I lean towards blocking:

Another way would be implement both and replace --no-sandbox with a more flexible option (like --sandbox=none, or --sandbox=lax, or --sandbox=strong, with the default being lax), and let the user opt-in into stricter filtering if they wish to.

hg commented 2 years ago

I think this is about as restrictive as I can make it.

Non-standard executable paths are not available even on normal level since doing that pretty much requires rx access to everything (including home directories and other interesting places).

We could add another level (low) which permits that, but it's probably better to encourage users to put binaries into /usr/local/bin and be done with it.

hg commented 2 years ago

Thanks for the review. I think it would be best to merge #382 first. libgcrypt support was broken and I nearly missed that because it's painful to continuously retest all configurations manually.

It's caused by 'secure memory', which is initialized using mlock() on first allocation (unless you do it separately), which fails since we've dropped privileges by that point.

gsliepen commented 2 years ago

It's caused by 'secure memory', which is initialized using mlock() on first allocation (unless you do it separately), which fails since we've dropped privileges by that point.

Good catch!