Re: Dbus and multiple LSMs (was Preferred subj= with multiple LSMs)

On Fri, 19 Jul 2019 at 13:02:24 -0700, Casey Schaufler wrote: > On 7/19/2019 11:47 AM, Simon McVittie wrote: >> I was hoping the syscall wrappers in glibc would be a viable user-space >> interface to the small amount of LSM stuff that dbus needs to use in an >> LSM-agnostic way. > I don't see how to do that without making the Fedora and Ubuntu user space > environments [not] remain functional. What I was thinking of was a second, parallel kernel <-> user-space interface (like the SO_PEERSECLABELS that I suggested) for future/updated user-space components. SO_PEERSEC would continue to return some hopefully-backwards-compatible thing, but would be deprecated, because it cannot fully represent the reality of LSM stacking while remaining backwards-compatible.

> I see display being used in scripts: > > echo apparmor > /proc/self/attr/display > apparmor-do-stuff --options --deamon > > much more than inside new or updated programs. Note that this implicitly relies on echo being a shell builtin, which is common but not guaranteed (I don't think). It would work in bash or dash, though.

If apparmor-do-stuff no longer works, and you have to wrap a shell script around it, isn't that the same amount of user-space breakage as if apparmor-do-stuff no longer works and you have to install a newer version that does work?

Either way, the sysadmin must take action to change user-space components. I think the attr/display thing only reduces the magnitude of the user-space changes required to catch up, and doesn't eliminate the fact that those changes were needed.

>> Lots of programs (including dbus-daemon) fork-and-exec arbitrary >> child processes that come from a different codebase not under our >> control and aren't necessarily LSM-stacking-aware. I don't really want >> to have to reset /proc/self/attr/display in our increasingly crowded >> after-fork-but-before-exec code path > My hope is that new and updated programs will have to tools > they need to get it right, and that those that don't won't > fall over on a well configured system. The problem I see here is that if we assume dbus-daemon is a new/updated program that has set /proc/self/attr/display = "hideous" in order to get the full stack of labels for its peer processes, then it will be causing side-effects on its separately-maintained child processes - they will no longer be able to benefit from the backwards-compatility thing where /proc/self/attr/display (effectively) defaults to the first LSM that has labels, because dbus-daemon overrode that (unless dbus-daemon takes action to reverse it between fork and exec). This partially defeats the semi-backwards-compatible handling of the existing kernel interfaces.

If dbus-daemon could read SO_PEERSECLABELS instead of SO_PEERSEC and read /proc/<pid>/attr/current_stack instead of /proc/<pid>/attr/current, leaving /proc/self/attr/display untouched, then this concern would go away.

Similarly, dbus-daemon can be linked to libselinux and/or libapparmor (on Debian it's linked to both, even in the non-stackable present, and the right one for the kernel configuration is chosen at runtime). If one of those libraries wrote to /proc/self/attr/display, then the rest of dbus-daemon's main thread and all child processes would implicitly be getting the result of that - even if dbus-daemon itself had not yet been updated for stacked LSMs (in which case it cannot be expected to reverse their action between fork and exec, because it's an older codebase that doesn't yet know that "big" LSMs can be stacked).

So I think libselinux and libapparmor should be enhanced to use new kernel interfaces that get the label they want to get (either just that label, or all the labels), instead of being enhanced to write /proc/self/attr/display to change the meaning of old kernel interfaces. Otherwise they can break other code in their process or their subprocesses.

>> instead of repurposing /proc/<pid>/attr/current >> and SO_PEERSEC to have contents that vary according to ambient process >> state in their reader? > In addition, yes. Instead of? I don't think that we can have a > backward compatibility story that flies without it. Consider only SELinux and AppArmor for a moment (I know there are other "big" LSMs like Smack, but this same reasoning applies to any pair, with appropriate search-and-replace on their names). Neither SELinux nor AppArmor: there are no labels, nothing changed. AppArmor is the only "big" LSM in the stack (Ubuntu): previously, the label was the AppArmor label; now, if attr/display is not altered, the label is the one used by the first "big" LSM in the stack, which is AppArmor. Nothing changed. SELinux is the only "big" LSM in the stack (Red Hat): same as for AppArmor being the only "big" LSM in the stack, but with s/AppArmor/SELinux/. SELinux and AppArmor stacked: this is a situation that could not exist before, so distro/sysadmin action must have been necessary to make it happen. However much ambient process state is invented, I don't see any way to make both SELinux and AppArmor user-space work without modifications: at least one of them (the one that is second in the stack) has to use new kernel interfaces, or alter attr/display to change the meaning of the old kernel interfaces, or something similar, to get the second LSM's labels. So distro/sysadmin action in user-space is also going to be necessary here whatever happens - backward compatibility has already been broken, it's only a question of how intrusive the user-space changes are. Is it really so much worse if the distro/sysadmin action taken to update user-space has to take the form of using new kernel interfaces that always do the same thing, rather than changing the meaning of old kernel interfaces?

smcv