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@xxxxxxxxxxxxxxxx> >>>> --- >>>> 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(-) >>>> >>> >
