[PATCH v12 23/25] NET: Add SO_PEERCONTEXT for multiple LSMs

[Stephen Smalley]

On 12/19/19 7:19 AM, Simon McVittie wrote:
> On Wed, 18 Dec 2019 at 15:50:45 -0500, Stephen Smalley wrote:
>> Ok, this seems to be a lack of support in AppArmor for saving the peer info
>> for unix/local domain sockets, so not your bug.  Doesn't implement the
>> necessary hooks.
> 
> Ubuntu kernels have working getsockopt SO_PEERSEC for AppArmor (which
> definitely works for D-Bus, or did when I tried it in the past), but it
> seems to be a downstream patch.
> 
> Is there a combination of LSMs where this works correctly and shows
> multiple LSMs' labels, either upstream or with downstream patches? D-Bus
> ought to be an early consumer of this information, but that's going to
> be difficult if there's nowhere I can test it. If there was a pair of
> in-tree or out-of-tree test LSMs that automatically labelled process
> 12345 and all sockets that it created with something like (pseudocode)
> 
> {
>    "labeltest1": "labeltest1process12345",
>    "labeltest2": "labeltest2process12345"
> }
> 
> then that would make testing straightforward.

AFAICT, you can't yet stack Smack+SELinux, and AppArmor requires 
out-of-tree patches to support SO_PEERSEC so there is no in-tree 
combination that will demonstrate the new SO_PEERCONTEXT for multiple lsms.

>  From my user-space developer perspective, I'd really like the kernel
> to have at least brief documentation of the assumptions that can be
> made about the compound format, to avoid people like me having to
> ask the linux-security-module mailing list and use the answers as our
> documentation:
> 
> - Can I assume that the LSM names (conceptually, keys in the associative
>    array) are printable ASCII? If not, can I at least assume that they are
>    printable UTF-8, or that LSMs whose names are not printable UTF-8 are
>    too weird to be worth supporting in user-space APIs?
> 
>    If this decision hasn't been made yet, I would like to request ASCII,
>    or UTF-8 if a wider character repertoire is desired - that would make
>    user-space APIs a lot easier to write.

ASCII works for me.

> 
> - What can I assume about the format of the labels (values in the
>    associative array)?
> 
>    From previous conversations on linux-security-module it seems the
>    answer is: they are intended to be printable strings; their encoding
>    is unspecified (could be ASCII or UTF-8, but equally could be Latin-1
>    or anything else); there are no reserved or disallowed characters
>    except '\0'; individual LSMs like SELinux and AppArmor might impose
>    tighter restrictions, but LSM-agnostic code can't assume those
>    restrictions are present.
> 
>    Even if the format is conceptually an unspecified encoding with every
>    nonzero byte allowed, if LSM and security policy authors establish a
>    convention that the labels are ASCII or UTF-8 without control characters
>    such as newlines, that would make user-space a lot easier to deal with.

I believe there is existing userspace code that seems to presume that 
they are NUL-terminated strings printable using %s format specifiers to 
printf-like functions in output and log messages _without_ any escaping. 
  Not just the LSM-specific libraries. systemd (for SO_PEERSEC), 
procps-ng (for /proc/pid/attr/current), perhaps others.  I think we can 
rely on that remaining true since the world would break.

> - Can I assume that the names and labels in /proc/$pid/attr/context
>    correspond exactly (same label <=> same bytes) to the ones in
>    SO_PEERCONTEXT?
> 
>    In particular, a design issue in the previous interfaces with
>    /proc/$pid/attr/current and SO_PEERSEC was that AppArmor puts
>    "unconfined\n" in /proc/$pid/attr/current, but "unconfined\0" in
>    SO_PEERSEC, and implementations of protocols like D-Bus can't know
>    that these are meant to be the same without LSM-specific knowledge
>    (namely "AppArmor profile names can't contain newlines").
> 
>    If this new API is an opportunity to declare that LSMs are expected
>    to put the same canonical form of a label in /proc/$pid/attr/context and
>    SO_PEERCONTEXT, possibly with a non-canonical version (adding '\n' or
>    '\0' or similar) exposed in the older /proc/$pid/attr/current and
>    SO_PEERSEC interfaces for backwards compatibility, then that would make
>    life a lot easier for user-space developers like me.

I'm all for this but the current implementation reuses the same 
underlying hooks as SO_PEERSEC, so it gets the same result for the 
per-lsm values.  We'd need a separate hook if we cannot alter the 
current AppArmor SO_PEERSEC format.  Technically AppArmor seemingly 
doesn't truly provide SO_PEERSEC in mainline today even though it 
implements the hook for returning the result because it never actually 
sets the peer on Unix domain or INET domain connections AFAICT, so they 
could change the format upstream without a compatibility break but it 
may break Ubuntu.  So it might require using a new hook for 
SO_PEERCONTEXT.  Which is fine with me.

> - Does the order of the names and values matter? Can I assume anything
>    about them?
> 
>    It would make the D-Bus API more straightforward if I can assume that
>    the order doesn't matter, and represent it as an unordered map
>    (associative array), like a Perl hash or Python dict.

IMHO order shouldn't matter.

> I'm asking all this from the D-Bus perspective, but anything that
> wants to store or forward LSM contexts in an LSM-agnostic way will
> have essentially the same requirements - I could imagine X11, Wayland,
> varlink etc. hitting the same problems D-Bus did.
> 
> Thanks,
>      smcv
>