On 7/19/2019 11:47 AM, Simon McVittie wrote:
Thanks for considering user-space in this, and sorry if I'm
hijacking
this thread a bit (but I think some of the things I'm raising might be
equally applicable for audit subjects).
Thank you for asking these questions. I think that if we
can address the issues around dbus we'll be in pretty good
shape in general.
On Fri, 19 Jul 2019 at 09:29:17 -0700, Casey Schaufler wrote:
> On 7/19/2019 5:15 AM, Simon McVittie wrote:
>> However, I think it would be great to have multiple-"big"-LSM-aware
>> replacements for those interfaces, which present the various LSMs as
>> multiple parallel credentials.
> Defining what would go into liblsm* is a task that has fallen to
> the chicken/egg paradox. We can't really define how the user-space
> should work without knowing how the kernel will work, and we can't
> solidify how the kernel will work until we know what user-space
> can use.
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. That's what we use in dbus at the moment (in practice
just getsockopt, but I'd also be reading /proc/self/attr/current if there
was a specification for how to normalize it to match SO_PEERSEC results)
and it's no harder than the rest of the syscall-level APIs.
I don't see how to do that without making the Fedora and Ubuntu user space
environments remain functional.
A single LSM-agnostic shared library would be the next best thing
from
my point of view.
Good, that's how it looks to me as well.
> An option that hasn't been discussed is a display option to
provide
> the Hideous format for applications that know that's what they want.
> Write "hideous" into /proc/self/attr/display, and from then on you
> get selinux='a:b:c:d',apparmor='z'. This could be used widely in
liblsm
> interfaces.
If the way to parse/split it is documented, then this would be easier
for dbus-daemon than continually resetting attr/display. It would be
especially good if you can document a way to find out which one of the
many labels would have been seen by an older user-space process that never
wrote to attr/display ("it's the first one in the list" would be fine),
so that we can put that one in our backwards-compatible API to clients.
/sys/kernel/security/lsm provides the list of all LSMs active on the system.
It would be trivial to add /sys/kernel/security/default-display-lsm which
would contain that.
Or, alternatively, we could pass it on directly to our clients and
let
*them* parse it (possibly by using liblsm), the same way AppArmor-aware
D-Bus clients have to know how to use either aa_splitcon() or their
own parsing to go from the raw SO_PEERSEC result
"/usr/bin/firefox (enforce)" to the pair ("/usr/bin/firefox",
"enforce")
that they probably actually wanted.
>> Do you mean that if process 11111 writes (for example) "apparmor" into
>> /proc/11111/attr/display, and then reads /proc/22222/attr/current
>> or queries the SO_PEERSEC of a socket opened by process 22222,
>> it will specifically see 22222's AppArmor label and not 22222's SELinux
>> label?
> Process 11111 would see the AppArmor label when reading
> /proc/22222/attr/current. The display value is controlled
> by process 11111 so that it can control what data it wants
> to see.
OK, that's what I'd hoped.
> The display is set at the task level, so should be thread safe.
OK, good. However, thinking more about this, I have other concerns:
* In library code that can be used by a thread (task) that also uses other
arbitrary libraries, or in an executable that uses libraries that might
be interested in LSMs, the only safe way to deal with attr/display would
be this sequence:
- write desired value to /proc/self/attr/display
- immediately read /proc/other/attr/current or query SO_PEERSEC
and it would not be safe to rely on writing /proc/self/attr/display
just once at startup, because some other library might have already
changed it between startup and the actual read. Paradoxically, this
maximizes the chance of breaking a reader that was relying on writing
/proc/self/attr/display once during startup.
* If an async signal handler needs to know a LSM label for whatever
reason, it will break anything in the same thread that was relying on
that sequence, because it might have interrupted them between their
write and their read:
main execution path signal handler
------------------- --------------
write "apparmor" to attr/display
(interrupted by async signal)
write "selinux" to attr/display
read attr/current or SO_PEERSEC
do other stuff with SELinux label
return
(resumes)
read attr/current or SO_PEERSEC
expect an AppArmor label
get a SELinux label
sadness ensues
Of course it's probably crazy for an async signal handler to do
this... but people do lots of odd things in async signal handlers,
and open(), read(), write(), getsockopt() are all async-signal-safe
functions, so it's at least arguably valid.
Stephen Smalley has already pointed out some of these issues.
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.
> Writing to display does not require privilege, as it affects
only
> the current process. The display is inherited on fork and reset on
> a privileged exec.
Another concern here: are you sure it shouldn't be reset on *any*
exec?
Yes, because so much of the user-space ecosystem depends on programs
that rarely get updated there has to be a way to specify it externally.
I don't like the situation, but we can't ignore it.
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 (which, according to POSIX, is not
a safe place to invoke any non-async-signal-safe function, so we can't
easily do error handling if something goes wrong there).
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.
Is there any possibility of having a parallel kernel API that,
if it exists, always returns the whole stack, maybe something
like /proc/<pid>/attr/current_stack and the SO_PEERSECLABELS that I
suggested previously,
/proc/<pid>/attr/current_stack is easy. SO_PEERSECLABELS will be
harder to sell, but would not be hard to implement if we can get
agreement on the Hideous format.
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.
(Bonus points if they are documented/defined with
a particular syntactic normalization this time, unlike the situation
with /proc/<pid>/attr/current and SO_PEERSEC where in principle you
need LSM-specific knowledge to know whether a trailing "\n" or "\0"
is safe to discard.)
I think that's necessary.
smcv