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. 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. > 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 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(-) >> >
