auditd.service: use ConditionCapability besides ConditionKernelCommandLine #136
Comments
|
Hmm, no, not "instead", but please use both checks. That way, auditd is only started if PID1 possesses the cap and "audit=0" is not on the kernel command line. That way things should work both in containers and on the host correctly. |
lahwaacz
changed the title
|
@poettering Actually, since |
|
The main issue is that the whole audit system is not compatible with containers. There has been a 7 year project to get a patch in the kernel to allow audit to work with containers. It is still not ready. So, without the kernel audit system being capable of dealing with containers, I am unsure if this fix is all that's needed or just leads you to the next problem - the kernel. |
|
@stevegrubb If that is the case, would you consider adding |
In general we advise against explicit container checks unless there are no other options, since features container managers don't have today they might learn tomorrow. And more importantly: more often than not, the lockdowns a container manager might set up for its payload often also make sense on the host itself, and should then be handled the same way.Thing is: if your PID 1 lacks the CAP_AUDIT_CONTROL cap, then invoking auditd never makes sense, regardless if we are in a container payload or on the host. Because that means that someone dropped these caps for you, and thus really doesn't want you to control audit, ever, so don't! More specifically: systemd has the global option CapabilityBoundingSet= in /etc/systemd/system.conf. It allows you to drop caps system wide, during early boot, so that the entire system lacks the caps, not a single process has them. People use this as an effective way to stop userspace from loading kernel modules after the initrd for example, but it could also be used to stop audit messages enitely from the post-initrd userspace. And I think that's a totally OK configuration choice to make. Thus, I#d really add the suggested ConditionCapability= check, regardless if we are talking about containers or not, it makes sense in either case. yes, it makes things nicer in containers, but it also makes things nicer in various host setups. Also, by adding the cap condition check, there's a nice, smooth upgrade path for container managers to magically enable audit int container payloads if one day audit is virtualized for containers: if they container manager takes the audit caps away from a container payload (like many do now, for example nspawn) then auditd would not be started inside of the container, but as soon as the kernel is updated to support virtualized audit, then the container mgr could pass the cap, and suddenly it all would just fall together nicely in the container: it would start making use of it. Hence, please do not add a Hence, my recommendation for auditd.service is to have this: |
We pass CAP_AUDIT_CONTROL since the audit libs generated ugly errors in userspace where the caps are gone. If this is changed, and the audit userspace would gracefully handle the caps being absent (the first step towards would be this check here), then we can drop the cap again from the default set of caps we pass. See nspawn's commit 88d04e31ce0837ebf937ab46c3c39a0d93ab4c7c. |
|
On 2020-09-03 19:00, Steve Grubb wrote:
The main issue is that the whole audit system is not compatible with containers. There has been a 7 year project to get a patch in the kernel to allow audit to work with containers. It is still not ready. So, without the kernel audit system being capable of dealing with containers, I am unsure if this fix is all that's needed or just leads you to the next problem - the kernel.
ghak90[1] is to track container activity in the main audit scope, which currently can and will still be the only one and it must be in the initial pid and user namespaces.
There are other issues filed[2][3] for future work to be able to stand up an audit daemon with its own config that does not influence the primary host audit daemon.
[1] linux-audit/audit-kernel#90
[2] linux-audit/audit-kernel#93
[3] linux-audit/audit-kernel#75
|
|
On 2020-09-03 14:41, Jakub Klinkovský wrote:
@poettering Actually, since `audit=0` does not affect capabilities given to the host and systemd-nspawn grants `CAP_AUDIT_CONTROL` by default, it will still fail the same way in a container. I could add `--drop-capability=CAP_AUDIT_CONTROL` but that's still not as nice as I hoped...
systemd-nspawn can give CAP_AUDIT_CONTROL all it wants to processes in pid and user namespaces, but that still won't help auditd at the moment since auditd currently MUST be run in the PID and USER init namespaces. That ability to run auditd in non-PID/USER init namespaces is the subject of linux-audit/audit-kernel#93
|
|
Cleaning up some old issues...is there any good reason to have this issue open? Auditd does a capabilities check early in its startup. If CAP_AUDIT_CONTROL is unavailable, it exits. Its behaved like this for over 5 years. And until there is kernel support, there really aren't many decisions to make about what to do with auditd and containers. |
|
Well, just exiting if the cap is missing is less than ideal. If the cap is not passed and you exit cleanly in that case in auditd this will be considered a successful service start-up by the service manager. Which is misleading though, since the service of course actually isn't running in this case. If auditd exits with an error return code in this case the service will appear as a failed service, which isn#t ideal either, since not having auditing when running inside a container is pretty much expected, and not really a problem. By using the two suggested Condition*= expressions you get the right behaviour: the service manager will not even try to start the service, it just conditions it out altogether if the requirements are not met, and thus it doesn't matter if the service ultimately fails or succeeds. So yes, the conditions matter, in order to esnure the correct information is conveyed to the user about the service: that it neither failed, nor started successfully, but simply was skipped altogether. |
|
Let's wait until there's a container implementation and then revisit this. It might be expected to run auditd in containers. Who know? |
|
Yes, but the great thing is that if you check for the condition like this, then things will magically start working once container managers support running auditd inside of containers. How so? because they can indicate audit support in containers really nicely to the payload simply by passing or taking away the audit caps to it. Adding the Condition*= check is exactly what you want to do to ensure that container managers work cleanly now, but one day will also work cleanly if audit is virtualized for containers. |
|
Closing this out as there is no container patch in the pipeline. The audit system really isn't something to put in a container today. Maybe in the future we can. |
|
Uh. The goal here was that people can put the audit subsystem in an image and then boot it either directly on a kernel (in which case audit is enabled and used) or in a container (in which case it is automatically disabled). Many distros install audit by default, and it would be good if such images would not result in all kinds of errors when used in ctonainers. |
The systemd service
auditd.serviceshould useConditionCapability=CAP_AUDIT_CONTROLinstead ofbesidesConditionKernelCommandLine=!audit=0, because the latter does not have a well defined meaning in containers. See systemd/systemd#16941 (comment) for details.The text was updated successfully, but these errors were encountered: