[PATCH 00/58] LSM: Module stacking for AppArmor

Tue Jun 4 19:58:46 UTC 2019

On 6/4/2019 10:11 AM, Stephen Smalley wrote:
> On 6/4/19 12:14 PM, Casey Schaufler wrote:
>> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
>>> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>>>> This patchset provides the changes required for
>>>> the AppArmor security module to stack safely with any other.
>>>
>>> Please explain the motivation
>>
>> I'll add some explanation for the next revision.
>> It won't be anything that I haven't posted many times
>> before, but you're right that it belongs in the log.
>>
>>> - why do we want to allow AppArmor to stack with other modules,
>>
>> First, is there a reason not to? Sure, you can confuse
>> administrators by implementing complex security policies,
>> but there are lots of ways to do that already.
>
> There are costs to doing so, e.g.
> - greater complexity in the security framework,

Taking blob management out of the modules and
into the framework makes simplifies the modules.

> - possibly greater memory and runtime overheads,

Possibly reduced memory and runtime overheads, as well.

> - potential user confusion (which security module(s) caused a given failure?)

That's not new. I've seen countless cases where users blame
SELinux or Smack when the problem is with mode bits and/or
capabilities. Not to mention that a good 50% of current Linux
users don't understand any of the security mechanisms to
begin with.

> - potential distro maintainer burden

Selection of security modules and how they are configured
has always been a burden for distro developers and maintainers.
Nothing new here.
And, they knew the job was dangerous when they took it.

> (similar to above, but performing triage when any given permission denial can have multiple causes beyond just DAC + one module, weird interactions among modules, etc)

Yama has been widely accepted by distros, and civilization
has yet to have officially been declared ended.

> It isn't free so there should be a cost/benefit analysis.

Some benchmarking is definitely in order, but most
of what's you're calling out as downside is hypothetical
or based on assumption. 

>
>>
>> AppArmor provides a different security model than SELinux,
>> TOMOYO or Smack. Smack is better at system component
>> separation, while AppArmor is better at application isolation.
>> It's a win to use each to its strength rather than trying to
>> stretch either to the edge of what it can do.
>>
>>> who would use it,
>>
>> Can't name names, but there have been multiple requests.
>>
>>> how would it be used,
>>
>> As mentioned above, Smack for system separation, AppArmor for
>> application isolation.
>
> Can you provide a concrete example of how combining the two yields a smaller, simpler configuration overall than using them individually?

Smack + AppArmor is a simpler, smaller model than the Smack policy
used in Tizen 2.

>
>>
>>> what does it provide that isn't already possible in the absence of it.
>>
>> It's not necessary that something be impossible to do any
>> other way. The question should be whether this provides for
>> a better way to achieve the goals, and this does that.
>> If I tried the come up with something that's impossible I
>> would expect the usual "you can do that with SELinux policy"
>> argument. We know we can do things. We want to have the tools
>> to do them better.
>>
>>> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?
>>
>> Some amount of merging will likely be required. But that's
>> always going to be true with parallel development tracks.
>> That's why we have git!
>>
>>> Please explain the cost of the change - what do we pay in terms of memory, runtime, or other overheads in order to support this change?
>>
>> Do you have particular benchmarks you want to see?
>> When I've supplied numbers in the past they have not
>> been remarked on.
>
> A combination of micro and macro benchmarks exercising multiple kernel subsystems would be good.  Kernel build time isn't sufficient.

Do you have preferences, or better yet, facilities
for running them? I am, alas, running on finite resources
and benchmark contributions, especially in areas where you
have specific concerns, would be most welcome.

>
>>
>>>
>>>>
>>>> A new process attribute identifies which security module
>>>> information should be reported by SO_PEERSEC and the
>>>> /proc/.../attr/current interface. This is provided by
>>>> /proc/.../attr/display. Writing the name of the security
>>>> module desired to this interface will set which LSM hooks
>>>> will be called for this information. The first security
>>>> module providing the hooks will be used by default.
>>>
>>> Doesn't this effectively undo making the hooks read-only after init, at least for the subset involved?  What are the security implications thereof?
>>
>> Any mechanism, be it a separate set of hooks, a name used to
>> do list look ups, or an sophisticated hash scheme will have that
>> impact for the processes that use it. This scheme has the best
>> performance profile of the mechanisms I experimented with and
>> avoids all sorts of special cases.
>>
>>>
>>>> The use of integer based security tokens (secids) is
>>>> generally (but not completely) replaced by a structure
>>>> lsm_export. The lsm_export structure can contain information
>>>> for each of the security modules that export information
>>>> outside the LSM layer.
>>>>
>>>> The LSM interfaces that provide "secctx" text strings
>>>> have been changed to use a structure "lsm_context"
>>>> instead of a pointer/length pair. In some cases the
>>>> interfaces used a "char *" pointer and in others a
>>>> "void *". This was necessary to ensure that the correct
>>>> release mechanism for the text is used. It also makes
>>>> many of the interfaces cleaner.
>>>>
>>>> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
>>>>
>>>> Signed-off-by: Casey Schaufler <casey at schaufler-ca.com>
>>>> ---
>>>>    drivers/android/binder.c                |  25 ++-
>>>>    fs/kernfs/dir.c                         |   6 +-
>>>>    fs/kernfs/inode.c                       |  31 ++-
>>>>    fs/kernfs/kernfs-internal.h             |   3 +-
>>>>    fs/nfs/inode.c                          |  13 +-
>>>>    fs/nfs/internal.h                       |   8 +-
>>>>    fs/nfs/nfs4proc.c                       |  17 +-
>>>>    fs/nfs/nfs4xdr.c                        |  16 +-
>>>>    fs/nfsd/nfs4proc.c                      |   8 +-
>>>>    fs/nfsd/nfs4xdr.c                       |  14 +-
>>>>    fs/nfsd/vfs.c                           |   7 +-
>>>>    fs/proc/base.c                          |   1 +
>>>>    include/linux/cred.h                    |   3 +-
>>>>    include/linux/lsm_hooks.h               |  91 +++++----
>>>>    include/linux/nfs4.h                    |   8 +-
>>>>    include/linux/security.h                | 133 +++++++++----
>>>>    include/net/af_unix.h                   |   2 +-
>>>>    include/net/netlabel.h                  |  10 +-
>>>>    include/net/scm.h                       |  14 +-
>>>>    kernel/audit.c                          |  43 ++--
>>>>    kernel/audit.h                          |   9 +-
>>>>    kernel/auditfilter.c                    |   6 +-
>>>>    kernel/auditsc.c                        |  77 ++++----
>>>>    kernel/cred.c                           |  15 +-
>>>>    net/ipv4/cipso_ipv4.c                   |  13 +-
>>>>    net/ipv4/ip_sockglue.c                  |  12 +-
>>>>    net/netfilter/nf_conntrack_netlink.c    |  29 ++-
>>>>    net/netfilter/nf_conntrack_standalone.c |  16 +-
>>>>    net/netfilter/nfnetlink_queue.c         |  38 ++--
>>>>    net/netfilter/nft_meta.c                |  13 +-
>>>>    net/netfilter/xt_SECMARK.c              |  14 +-
>>>>    net/netlabel/netlabel_kapi.c            |   5 +-
>>>>    net/netlabel/netlabel_unlabeled.c       | 101 +++++-----
>>>>    net/netlabel/netlabel_unlabeled.h       |   2 +-
>>>>    net/netlabel/netlabel_user.c            |  13 +-
>>>>    net/netlabel/netlabel_user.h            |   2 +-
>>>>    net/unix/af_unix.c                      |   6 +-
>>>>    security/apparmor/audit.c               |   4 +-
>>>>    security/apparmor/include/audit.h       |   2 +-
>>>>    security/apparmor/include/net.h         |   6 +-
>>>>    security/apparmor/include/secid.h       |   9 +-
>>>>    security/apparmor/lsm.c                 |  64 +++---
>>>>    security/apparmor/secid.c               |  42 ++--
>>>>    security/integrity/ima/ima.h            |  14 +-
>>>>    security/integrity/ima/ima_api.c        |   9 +-
>>>>    security/integrity/ima/ima_appraise.c   |   6 +-
>>>>    security/integrity/ima/ima_main.c       |  34 ++--
>>>>    security/integrity/ima/ima_policy.c     |  19 +-
>>>>    security/security.c                     | 338 +++++++++++++++++++++++++++-----
>>>>    security/selinux/hooks.c                | 259 ++++++++++++------------
>>>>    security/selinux/include/audit.h        |   5 +-
>>>>    security/selinux/include/objsec.h       |  42 +++-
>>>>    security/selinux/netlabel.c             |  25 +--
>>>>    security/selinux/ss/services.c          |  18 +-
>>>>    security/smack/smack.h                  |  18 ++
>>>>    security/smack/smack_lsm.c              | 238 +++++++++++-----------
>>>>    security/smack/smack_netfilter.c        |   8 +-
>>>>    security/smack/smackfs.c                |  12 +-
>>>>    58 files changed, 1217 insertions(+), 779 deletions(-)
>>>>
>>>
>