[PATCH RFC 00/11] LSM: Stacking for major security modules

Stephen Smalley sds at tycho.nsa.gov
Thu Apr 6 20:38:24 UTC 2017


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 <casey at schaufler-ca.com>
> > > ---
> > > 
> > >  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



More information about the Linux-security-module-archive mailing list