From mboxrd@z Thu Jan 1 00:00:00 1970 From: sds@tycho.nsa.gov (Stephen Smalley) Date: Thu, 06 Apr 2017 16:38:24 -0400 Subject: [PATCH RFC 00/11] LSM: Stacking for major security modules In-Reply-To: References: <509e0281-9f8a-83c2-f9d6-5532903cda46@schaufler-ca.com> <1491503171.4532.10.camel@tycho.nsa.gov> Message-ID: <1491511104.4532.17.camel@tycho.nsa.gov> To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Thu, 2017-04-06 at 13:10 -0700, Casey Schaufler wrote: > On 4/6/2017 11:26 AM, Stephen Smalley wrote: > > On Wed, 2017-04-05 at 14:39 -0700, Casey Schaufler wrote: > > > Subject: [PATCH RFC 00/11] LSM: Stacking for major security > > > modules > > > > > > I am again sending this as an RFC. If you stop at patch 04 you > > > can > > > use any combination of modules so long as you use only one of > > > SELinux and Smack. Patches 05-10 take you most of the way to > > > complete > > > stacking, but cannot be said to completely address all the > > > issues. > > > Patch 11 provides for management of the yet unused task blob. > > > > > > This patch set implements stacking for "major" security modules > > > that use cred and file blobs. Management of security blobs is > > > moved from the security modules and into the LSM infrastructure. > > > This has been proposed in the past by Serge Hallyn and David > > > Howells. > > > This implementation owes much to their work. > > > > > > The bulk of the change is in abstracting use of blobs within the > > > security modules. This allows the modules to share a single blob > > > and hides the details from the code. There is? > > > > > > Modules are required to declare the amount of space they require > > > for each blob they use. Because modules deal with blobs during > > > their > > > initialization the blob sizes must be declared prior to module > > > initialization. The module initialization becomes a two step > > > process. > > > > > > Security module stacking is optional. If stacking is not > > > configured, > > > the CONFIG_DEFAULT_SECURITY value is used, just as before. If > > > stacking > > > is configured using CONFIG_SECURITY_STACKING the modules desired > > > for > > > the stack are selected individually. AppArmor would be selected > > > by > > > specifying CONFIG_SECURITY_APPARMOR_STACKED. The > > > CONFIG_DEFAULT_SECURITY > > > is ignored. The security= boot option is still respected and has > > > the > > > same behavior as before, allowing a single module to be used > > > instead > > > of > > > the specified stack. > > > > > > To accommodate multiple active modules a security "context" is > > > defined to use a regular format: > > > > > > lsmname='lsmvalue'[,lsmname='lsmvalue']... > > > > > > This is not supported by any existing user space run time code. > > > > > > I have tested these patches in various configurations of Ubuntu > > > and > > > Fedora. I have had much better success with SELinux in permissive > > > mode > > > than enforcing, but that appears to be a result of user space > > > code > > > issues. Smack and SELinux together have limited success, again > > > because > > > of the context format. > > > > I think in order for this to be viable, it must not break existing > > userspace if CONFIG_SECURITY_STACKING=y if only one module is > > enabled. > > If there's only one module enabled you don't need > CONFIG_SECURITY_STACKING=y. Nonetheless, I see the point. > It's lots cleaner if this can be a compile time > difference rather than something detected at run time, > but I'll factor that in to the next version. > > > ?Ideally, it wouldn't even break existing userspace with multiple > > modules, so long as they do not conflict in their usage of > > userspace > > APIs (e.g. if only one implements getpeersec_stream, why mutate its > > result and break userspace?). > > Remember that > "system_u:object_r:netlabel_peer_t:s0" is a legitimate Smack label. That's only a problem if they both return a result from getpeersec_stream (true for Smack+SELinux, false for Smack+TOMOYO). So determining whether there is more than one enabled module that implements getpeersec_stream is sufficient to decide whether you need to disambiguate the label for SO_PEERSEC. > I don't think doing "System" vs > "selinux='system_u:object_r:netlabel_peer_t:s0',smack='System'" > on a per-API granularity is a good idea. The author of user space > code oughtn't be compelled to figure out which APIs will identity > the components and when they don't which security module will > provide the unadorned data. You're already leaving /proc/self/attr/current alone for compatibility reasons, so not clear this is fundamentally different (except maybe you don't think you can introduce a new socket option to get the "full" context as easily as you can add a new proc node). If you don't want to do it piecemeal, then you could just specify which module gets ownership of all of the userspace APIs via a config option or something. > At some point it would be a good idea to have a liblsm that > would make varying and/or multiple security modules easier to > deal with. I know systemd and dbus could use such a library to > good effect. Yes, but in the meantime, if you want to be able to test CONFIG_SECURITY_STACKING=y with modules in enforcing mode on distributions that enable a major security module, it seems like you need to provide some way of handling this compatibly. > > > At present, it breaks the selinux-testsuite even with > > CONFIG_SECURITY_STACKING=n, all in the inet_socket tests; looks > > like > > network labeling is broken.??FWIW, output was: > > Thanks for running the tests. I'll be looking into this soonest. > > > > > inet_socket/test ............ getsockopt: SO_PEERSEC: Protocol not > > available > > read: Connection reset by peer > > inet_socket/test ............ 1/33? > > #???Failed test at inet_socket/test line 27. > > inet_socket/client:??expected > > unconfined_u:unconfined_r:test_inet_client_t:s0-s0:c0.c1023, got? > > I'd be interested to see what it got. It was an empty string, because the server didn't get a label. > > > inet_socket/test ............ 3/33? > > #???Failed test at inet_socket/test line 45. > > inet_socket/test ............ 26/33? > > #???Failed test at inet_socket/test line 219. > > > > #???Failed test at inet_socket/test line 227. > > inet_socket/test ............ 30/33? > > #???Failed test at inet_socket/test line 245. > > > > #???Failed test at inet_socket/test line 253. > > # Looks like you failed 6 tests of 33. > > inet_socket/test ............ Dubious, test returned 6 (wstat 1536, > > 0x600) > > Failed 6/33 subtests > > > > These all pass on security-next and on v4.11-rc4, so it is > > definitely > > something in your patches.? > > > > > Patch 01 Adds a smack subdirectory in /proc/.../attr (proposed > > > separately) > > > Patch 02 Move management of the cred blob to the LSM > > > infrastructure. > > > Patch 03 Move management of the file blob to the LSM > > > infrastructure. > > > Patch 04 Change how the security modules get selected. > > > Patch 05 Infrastructure blob management for IPC, keys, sockets. > > > Patch 06 Fixes Smack's sk_free hook. > > > Patch 07 Support mount options for multiple security modules. > > > Patch 08 Change secids from a u32 to a structure. > > > Patch 09 Netlabel consistency enforcment in sendmsg. > > > Patch 10 Fixes a compile issue in one Smack configuration. > > > Patch 11 Infrastructure blob management for the new task blob. > > > > > > These patches can be found in git at: > > > > > > https://github.com/cschaufler/smack-next.git#stacking-4.11-rc4 > > > > > > Signed-off-by: Casey Schaufler > > > --- > > > > > > ?Documentation/security/LSM.txt??????????|???33 +- > > > ?drivers/usb/core/devio.c????????????????|???13 +- > > > ?fs/btrfs/super.c????????????????????????|???10 +- > > > ?fs/proc/base.c??????????????????????????|???96 ++- > > > ?fs/proc/internal.h??????????????????????|????1 + > > > ?fs/xattr.c??????????????????????????????|????6 +- > > > ?include/linux/audit.h???????????????????|???10 +- > > > ?include/linux/cred.h????????????????????|????3 +- > > > ?include/linux/lsm_hooks.h???????????????|???76 ++- > > > ?include/linux/sched/signal.h????????????|????2 +- > > > ?include/linux/security.h????????????????|??227 +++++-- > > > ?include/net/flow.h??????????????????????|????5 +- > > > ?include/net/netlabel.h??????????????????|???16 +- > > > ?include/net/scm.h???????????????????????|????4 +- > > > ?kernel/audit.c??????????????????????????|???25 +- > > > ?kernel/audit.h??????????????????????????|????9 +- > > > ?kernel/auditfilter.c????????????????????|????4 +- > > > ?kernel/auditsc.c????????????????????????|???42 +- > > > ?kernel/cred.c???????????????????????????|???19 +- > > > ?kernel/signal.c?????????????????????????|????6 +- > > > ?net/ipv4/cipso_ipv4.c???????????????????|????5 +- > > > ?net/ipv4/ip_sockglue.c??????????????????|????6 +- > > > ?net/netfilter/nf_conntrack_netlink.c????|???12 +- > > > ?net/netfilter/nf_conntrack_standalone.c |????6 +- > > > ?net/netfilter/nfnetlink_queue.c?????????|????9 +- > > > ?net/netfilter/xt_AUDIT.c????????????????|????9 +- > > > ?net/netfilter/xt_SECMARK.c??????????????|????6 +- > > > ?net/netlabel/netlabel_kapi.c????????????|???56 +- > > > ?net/netlabel/netlabel_unlabeled.c???????|???30 +- > > > ?net/netlabel/netlabel_unlabeled.h???????|????2 +- > > > ?net/netlabel/netlabel_user.c????????????|????4 +- > > > ?net/unix/af_unix.c??????????????????????|????6 +- > > > ?net/xfrm/xfrm_policy.c??????????????????|????6 +- > > > ?net/xfrm/xfrm_state.c???????????????????|????3 +- > > > ?security/Kconfig????????????????????????|???86 +++ > > > ?security/apparmor/context.c?????????????|????2 - > > > ?security/apparmor/include/context.h?????|???25 +- > > > ?security/apparmor/lsm.c?????????????????|??111 ++-- > > > ?security/integrity/ima/ima_policy.c?????|????7 +- > > > ?security/security.c?????????????????????| 1046 > > > +++++++++++++++++++++++++++++-- > > > ?security/selinux/hooks.c????????????????|??720 +++++++++------ > > > ------ > > > ?security/selinux/include/audit.h????????|????2 +- > > > ?security/selinux/include/objsec.h???????|???87 ++- > > > ?security/selinux/include/xfrm.h?????????|????9 +- > > > ?security/selinux/netlabel.c?????????????|???17 +- > > > ?security/selinux/selinuxfs.c????????????|????5 +- > > > ?security/selinux/ss/services.c??????????|???13 +- > > > ?security/selinux/xfrm.c?????????????????|???29 +- > > > ?security/smack/smack.h??????????????????|???95 ++- > > > ?security/smack/smack_access.c???????????|????2 +- > > > ?security/smack/smack_lsm.c??????????????|??751 +++++++++------ > > > ---- > > > --- > > > ?security/smack/smack_netfilter.c????????|???28 +- > > > ?security/smack/smackfs.c????????????????|???28 +- > > > ?security/tomoyo/common.h????????????????|???25 +- > > > ?security/tomoyo/domain.c????????????????|????4 +- > > > ?security/tomoyo/securityfs_if.c?????????|???13 +- > > > ?security/tomoyo/tomoyo.c????????????????|???55 +- > > > ?57 files changed, 2647 insertions(+), 1280 deletions(-) > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux- > > security-module" in > > the body of a message to majordomo at vger.kernel.org > > More majordomo info at??http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html