tighten up dosemu security

[stsp]

I am under the heavy impression of this:
https://lwn.net/ml/oss-security/CAN_LGv3GAmdpaXVCjwp1UAH_Z6KKDnqydj68Oj4jmXRwwPE=Uw@mail.gmail.com/
and am no longer comfortable with our
minimal security model which is to only
provide the safe defaults.
Now I think something really radical needs
to be done.
My current thinking is to make the dosemu.bin
binary owned by dosemu2:dosemu2 rather
than root:root, and make it setuid+setgid.
The very simple code will need to be added
to setresuid() euid back to the user for the
time of an initialization, and then do setuid()
to switch to dosemu2 permanently, and
access DOS files only with that identity.

This way we avoid dosemu2 from ever
reading any user's files after init. The
user will need to chown all DOS files to
dosemu2:dosemu2 and add himself to
dosemu2 group in order to be able to
access the DOS files as before.

The same trick can be done with
sudo setpriv --euid=dosemu2 --egid=dosemu2
but I think requiring the user to set up
sudoers gives more troubles than to
just use setuid+setgid.

@skitt @taviso everyone, what do you
think? Does this idea sound sensible?

[skitt]

Regular users can’t give files away like that, so anything requiring a change in ownership won’t work. Groups would work, but that also requires that users belong to the target group so it’s not great.

I wonder how much effort it would take to make dosemu2 work in a flatpak, or to have it use bubblewrap to set up its own sandboxing.

I don’t think dosemu2 is a high-value target though, so probably not worth worrying about too much ;-). (I don’t mean that improving dosemu2 security isn’t useful, just that it shouldn’t result in worry or stress.)

[stsp]

Groups would work, but that also requires that users belong to the target group so it’s not great.

Is it a bad practice to have the
post-inst script adding the user
to the needed group?
But in either case, to get the group-only
approach to work, you still need the
dosemu2 process to change its uid
to "dosemu2" so that it no longer owns
the user's files. And that requires CAP_SETUID.
Additionally you need all the DOS files to
be group-writable, because in the case of
group-only approach dosemu2 will need
to use the group perms to access the DOS
files, rather than the owner's perms.
This will lead to tons of bugs when the
user forgot to make this or that file
group-writable. So yes, group-only approach
doesn't feel too great.

or to have it use bubblewrap to set up its own sandboxing.

Could you please explain in 2 words
how exactly you see that helping?
The ultimate goal is to allow dosemu2
to access the DOS files, but not the
user's files. In that task I can only
imagine 2 solutions: either you give
away the ownership of DOS files, or
you use chroot. I don't think bubblewrap
adds any "third" solution, or does it?

I don’t think dosemu2 is a high-value target though, so probably not worth worrying about too much ;-).

That's my thinking as well, and we've
yet to see some DOS virus hacking
dosbox (a much more widely-available
target than dosemu2). But still...

[stsp]

Ok so bubblewrap uses the chroot-alike
technique with the mount namespaces.
Maybe this is indeed more promising
than the UID-based separation, RTFMing... :)

[skitt]

Right, I’m thinking that a useful way of using something like bubblewrap would be to process mounts and then deny access to anything outside those; for example, once configuration files are processed, no further mounts can be added. This would be annoying for some users so it would be nice if it could be disabled...

[stsp]

Right, I’m thinking that a useful way of using something like bubblewrap would be to process mounts and then deny
access to anything outside those; for example, once configuration files are processed, no further mounts can be added.

How do you do that with bubblewrap?
So far the examples I've seen, were
passing the needed paths to bwrap,
which means you mount things even
before starting the process.

This would be annoying for some users so it would be nice if it could be disabled...

No because this is already so for a long
time. You can't mount arbitrary dir with
lredir (which is why lredir was renamed
multiple times - it simply doesn't do what
it did before). Likewise you can't execute
arbitrary binary with unix.com, and so on.
We have safe defaults, strong restrictions
and good sandboxing (unless you enable
native DPMI - its not yet sandboxed, but
who cares if its not default).

So why to bother? Well, mostly because
we have a dosemu1-inherited code like
8287579. Do you have any idea why the
overflow check was inverted, and why the
surrounding code is so heavily obfuscated?
Maybe just a coincidence, maybe not.
ALso I wouldn't trust the dosemu2-written
code much as well:

$ git diff -r master |diffstat
...
629 files changed, 52147 insertions(+), 19077 deletions(-)

And this doesn't account for fdpp,
comcom64, dj64dev. The amount of
added code is huge, the amount of
developers and reviewers is tiny. The
only way to write as much code, is to
use the dirty rapid prototyping, or
nothing would be written at all.
I am sure other emulators have smaller
diffstats with more developers - they
have better code, something we can't
afford.

[andrewbird]

Perhaps it would help to setup Coverity static scanning from Github?

[stsp]

Why not?

[andrewbird]

I did set it up once for dosemu once, but it will take quite some effort to make dosemu pass the checks cleanly

[stsp]

We probably don't need to pass them
cleanly? The stats would be enough, so
that the errors are not added up.
Eventually we'll get them fixed.

[stsp]

I started to experiment with user
namespaces (which are needed together
with mount namespaces here), and I
don't understand why kernel allows to
only write 1 extent (string) to /proc/self/uid_map.
I would like to create 2 mappings: for root
and unprivileged user within the namespace,
mapping to the same unpriv user in the parent
namespace. This way I can create the needed
bind mounts in a mount namespace, and drop
the root privs. But only 1 extent is allowed, unless
you have CAP_SETUID in a parent namespace.

Does anyone (maybe @ebiederm or @amluto ?)
knows why it is so? I looked into new_idmap_permitted()
in a kernel, and I think it should be rather
straight-forward to change it to allow multiple
mappings to the same parent's uid. What was
the reason to only allow 1 mapping?

[ebiederm]

Stas Sergeev ***@***.***> writes:

I started to experiment with user namespaces (which are needed together with mount namespaces here), and I don't understand why kernel allows to only write 1 extent (string) to `/proc/self/uid_map`. I would like to create 2 mappings: for root and unprivileged user within the namespace, mapping to the same unpriv user in the parent namespace. This way I can create the needed bind mounts in a mount namespace, and drop the root privs. But only 1 extent is allowed, unless you have CAP_SETUID in a parent namespace. Does anyone (maybe @ebiederm or @amluto ?) knows why it is so? I looked into `new_idmap_permitted()` in a kernel, and I think it should be rather straight-forward to change it to allow multiple mappings to the same parent's uid. What was the reason to only allow 1 mapping?

Because the mappings are 1-1. Meaning you can't have the same uid mapped twice in either direction. It doesn't matter for setting up bind mounts. The initial user in a user namespace has all caps (within their user namespace). Which means in practice the mappings only matter for stat, and exec. So what I would do is identity map the uid I start with, setup bind mounts and then drop caps. Eric

[stsp]

Ahha, thanks!
Very interesting.

Having the multiple mappings to the
same UID can help porting the existing
programs (which is the case here), because
then the caps are dropped on euid change
from 0 to non-0. dosemu had such a code
from the very beginning.
Of course if such possibility is not planned
for the kernel, then I'll have to add a new
code in parallel with old one, that would
use libcap.

[stsp]

Is there some library to automate the
chroot env creation? I need to bind-mount
too many things to create a full-blown
chroot, so maybe there is some automation
of that task? Some libbubblewrap could
help, but it seems there is none.

[stsp]

Ideally I don't want to do chroot.
All I want is a namespace with user's
files "hidden" from my process.
Is there some way to do that?
Or should the full chroot be created
just for that? :(
Can someone please guide me to
the right direction?

[stsp]

I think I just need to install the
seccomp filter to disallow the
open()s of a file not belonging
to dosemu2 group (aka group-based
solution on steroids). And make
that optional...

[stsp]

There appears to be a completely new
sandboxing framework called landlock:
https://docs.kernel.org/userspace-api/landlock.html
Seems to be much better for our task
than the namespace-based approach,
as it doesn't require creating chroot env.

[stsp]

Regular users can’t give files away like that

Indeed, not like that.
Here's what seems to work:
setfacl -m u:dosemu2:rw file.
This is an unprivileged operation,
and I suppose dosemu can just
put the needed ACLs on its own,
and then setuid/setgid to dosemu2.
So I am back to the initial idea of
an UID-based separation, after all.
Hand-crafted sandbox is a dirty
business and people will find 1024+
ways of escaping it, because I've
never written a sandboxes in the
past, and getting it right from the
first time is simply not possible.
UID-based separation is OTOH
absolutely straight-forward.

In this scenario, the only priv setup
is done upon sudo make install.
You need to create dosemu2:dosemu2
user:group, chows the binary to that
and add suid/sgid bits.
On start, dosemu will add the needed
ACLs to the files and then switch UIDs.

Sounds ok?
I only wonder how difficult is that for
the packaging system to propagate.
Does debian packaging infrastructure
supports adding users and remapping
owner UIDs on a files, since the numeric
UIDs may not match?
If it is a problem for packaging, then
the sudo-based setup is possible, but
in this case someone (either user or
an installer) needs to modify /etc/sudoers.

[jschwartzenberg]

Is there a particular reason to have this for just dosemu? It would seem fitting to use Quebes OS or have another mechanism to separate all programs from each other. I think Snap also has functionality for this to hide a user's regular files. Things that need to be available for a particular Snap need to be inside it's directory below ~/snap.

[stsp]

dosemu runs untrusted binaries,
and therefore needs to establish
the sandboxing by itself too.
It doesn't care about separating
all programs from each other. It
cares about separating the untrusted
binaries from the host.

[jschwartzenberg]

Possibly the easiest way to achieve this would be to configure dosemu to use an image and not let it access any filesystem.

[stsp]

The configuration is irrelevant.
If dosemu2 is hacked, the attacker
gains all its abilities, not only those
configured for DOS progs.

[stsp]

Also note that configuration is already
secure: there is no way to mount arbitrary
dir any longer (was possible in dosemu1).
So restricting to hdimages doesn't help
a tiny bit.

[jschwartzenberg]

Can you give an example of an attack scenario that you're trying to address with this ticket? Malicious dos binaries that break out of the dosemu sandbox?

[stsp]

Exactly.

[jschwartzenberg]

How do you see this compared to the current implementations of for example QEMU and DOSBox? I would say it would be unfortunate if each application would reinvent the wheel on its own here. Maybe there are other similar tools that have something that would seem suitable to look at.

[stsp]

https://qemu-project.gitlab.io/qemu/system/security.html
Please read at least the
https://qemu-project.gitlab.io/qemu/system/security.html#principle-of-least-privilege
and
https://qemu-project.gitlab.io/qemu/system/security.html#isolation-mechanisms
sections.

dosbox seems to only have the
-securemode switch that restricts
mounts - not the security model I'd
like to have in dosemu2: we already
restrict mounts w/o any switches.
Security cannot be "optional".

[stsp]

Filled this ticket:
thkukuk/rpcsvc-proto#18
But I really don't know if it is an
UB or not, because gcc doesn't complain.

[stsp]

Filled this ticket:
https://savannah.gnu.org/bugs/index.php?65737
Now even GNU make resists to
this change-set...

[stsp]

Disabled clang's sanitizer to avoid
the problem. Not sure how good
such solution is, but for now this is
all I have.
This whole change-set feels a bit
unlucky, it seems.

[andrewbird]

So I retested with your latest suid branch there were no warnings during build or at startup, and it successfully built pcmos without error.

[stsp]

Ok, thanks, so still no way to
reproduce the instability locally. :(

[stsp]

They say rpcgen is deprecated.
So unless someone can point me
to something better, I am going
to look into searpc.

[stuaxo]

I used zeromq with msgpack to encode data for comms between different parts of a system before and it wasn't too bad - no sure if it's suitable for this.

zeromq handles making sure enough data gets where you want, then msgpack was relatively lightweight.

Having said that searpc looks pretty suitable.