bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
675 stars 78 forks source link

Ambient Capabilities #239

Closed stevegrubb closed 3 years ago

stevegrubb commented 4 years ago

I was scanning the system and noticed that dbus-broker has CAP_AUDIT_WRITE as an ambient capability. Grepping through the source code, I don't see any execve, popen, or system function calls. Why does it need ambient capabilities? Thx.

dvdhrm commented 4 years ago

The bus implementation dbus-broker needs CAP_AUDIT_WRITE to raise audit events. The launcher dbus-broker-launch includes CAP_AUDIT_WRITE in its ambient-set so the spawned bus implementations will inherit it.

The bus implementation itself currently does not spawn further processes, and thus could safely drop it from the ambient set.

stevegrubb commented 4 years ago

OK, that's what I was wondering about. Because ambient is for inheritance to other processes. The concern is that should the broker be exploitable, any shell the attacker pops would also have the capability. Clearing the inheritable capabilities should do the trick.

dvdhrm commented 4 years ago

OK, that's what I was wondering about. Because ambient is for inheritance to other processes. The concern is that should the broker be exploitable, any shell the attacker pops would also have the capability. Clearing the inheritable capabilities should do the trick.

Yes, correct.

stevegrubb commented 3 years ago

I spent some time running some experiments today to see what's the best solution. I think that just doing this:

prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_CLEAR_ALL, 0, 0, 0);

fixes the issue of leaky capabilities. You would do that in the thing that launches the bus implementation itself. I am writing this up for a blog so that other's can understand the evolution of ambient capabilities under various attempts to stop the spread. With the prctl solution, you do not need to link to any libraries and that makes it simplest.

dvdhrm commented 3 years ago

I spent some time running some experiments today to see what's the best solution. I think that just doing this:

prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_CLEAR_ALL, 0, 0, 0);

fixes the issue of leaky capabilities. You would do that in the thing that launches the bus implementation itself. I am writing this up for a blog so that other's can understand the evolution of ambient capabilities under various attempts to stop the spread. With the prctl solution, you do not need to link to any libraries and that makes it simplest.

We explicitly raise CAP_AUDIT_WRITE in the "thing that launches the bus implementation itself". Your recommendation is to add another prctl to clear it? That will not work.

If we cared about having ambient capabilities set, I think the bus implementation is the process to drop them from the ambient set upon startup.

dvdhrm commented 3 years ago

We now clear ambient capabilities for the broker itself (see #261), as I described in the previous comment.

Thanks for the report!