Message-id: <1432721610.1784.27.camel@samsung.com>
Date:	Wed, 27 May 2015 12:13:30 +0200
From:	Lukasz Pawelczyk <l.pawelczyk@...sung.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Casey Schaufler <casey@...aufler-ca.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	"Kirill A. Shutemov" <kirill@...temov.name>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Lutomirski <luto@...capital.net>,
	David Howells <dhowells@...hat.com>,
	Fabian Frederick <fabf@...net.be>,
	Greg KH <gregkh@...uxfoundation.org>,
	James Morris <james.l.morris@...cle.com>,
	Jeff Layton <jlayton@...marydata.com>,
	Jingoo Han <jg1.han@...sung.com>,
	Joe Perches <joe@...ches.com>,
	John Johansen <john.johansen@...onical.com>,
	Jonathan Corbet <corbet@....net>,
	Kees Cook <keescook@...omium.org>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Miklos Szeredi <miklos@...redi.hu>,
	Oleg Nesterov <oleg@...hat.com>,
	Paul Moore <paul@...l-moore.com>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	Zefan Li <lizefan@...wei.com>,
	Rafal Krypa <r.krypa@...sung.com>, linux-doc@...r.kernel.org,
	linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	containers@...ts.linux-foundation.org,
	Lukasz Pawelczyk <havner@...il.com>
Subject: Re: [PATCH v2 0/7] Smack namespace

On wto, 2015-05-26 at 22:13 -0500, Eric W. Biederman wrote:
> Lukasz Pawelczyk <l.pawelczyk@...sung.com> writes:
> 
> > Hello,
> >
> > Some time ago I sent a Smack namespace documentation and a preliminary
> > LSM namespace for RFC. I've been suggested that there shouldn't be a
> > separate LSM namespace and that it should live within user namespace.
> > And this version does. This is a complete set of patches required for
> > Smack namespace.
> >
> > This was designed with a collaboration of Smack maintainer Casey
> > Schaufler.
> >
> > Smack namespace have been implemented using user namespace hooks added
> > by one of the patches. To put some context to it I paste here a
> > documentation on what Smack namespace wants to achieve.
> >
> > LSM hooks themselves are documented in the security.h file inside the
> > patch.
> >
> > The patches are based on:
> > https://github.com/cschaufler/smack-next/tree/smack-for-4.2-stacked
> >
> > The tree with them is avaiable here:
> > https://github.com/Havner/smack-namespace/tree/smack-namespace-for-4.2-stacked-v2
> >
> > Changes from v1:
> >  - "kernel/exit.c: make sure current's nsproxy != NULL while checking
> >    caps" patch has been dropped
> >  - fixed the title of the user_ns operations patch
> 
> A have not completed a review I don't understand smack well enough to
> answer some questions but I don't think I like the approach this patch
> takes to get things done.
> 
> I am flattered that you are using a mapping as I did with uid map and
> gid map.  Unfortunately I do not believe your mapping achieves what my
> mapping of uids and gids achieved.
> 
> A technical goal is to give people the tools so that a sysadmin can set
> up a system, can grant people subids and subgids, and then the user can
> proceed to do what they need to do. 

This is exactly the case with Smack namespace.


> In particular there should be
> little to no need to keep pestering the system administrator for more
> identifiers.

This all depends on the use case. When you create a new namespace for a
particular purpose you could know what labels will be required there and
map them upfront. Adding new mappings is for a use case that a container
might have new software installed that requires new labels for it. You
don't create/import new labels on a normal basis all the time.


> The flip side of that was that the mapping would ensure all of the
> existing permissions checks would work as expected, and the checks in
> the kernel could be converted without much trouble.

Again, this is exactly the case with Smack namespace. There are
additional checks in place of course, but the rules are clear about
how that works.


> Ranges of ids were choosen because they allow for a lot of possible ways
> of using uids and gids in containers, are comparitively easy to
> administer, are very fast to use, and don't need large data structures.

This is Smack specific. The same way we could compare Smack with DAC
(even without talking about namespaces). Every new UID is just it, a
number. Every new label requires allocation and adds to the list.

I had an idea about mapping ranges of labels for a container by using
prefixes but this was dropped by Casey. Labels (with an exception of the
built-in ones) should not have any special meaning. This is Smack
design.

About performance, as it was originally with Smack. The list of labels
was a simple list. When the performance was not enough hashing was
added. I'd take the same approach here. If some use cases will require
so many mappings that this becomes a problem we will deal with it. I
don't want to complicate the patches now and I this was Casey's opinion
as well.


> With a discreet mapping of labels I have the premonition that we now
> have a large data structure that, is not as flexible in to use,
> is comparatively slow and appears to require an interaction with the
> system administrator for every label you use in a container.

Again, DAC vs Smack philosophy. The same way a new label appears in init
namespace (e.g. with a file/inode). Without a new rule you won't have an
access to it. This requires administrator intervention as well. This is
MAC after all.


> As part of that there is added a void *security pointer in the user
> namespace to apparently hang off anything anyone would like to use.
> Connected to that are hooks that have failure codes (presumably memory
> allocation failures), but the semantics are not clear.  My gut feel is
> that I would rather extend struct user_namespace to hold the smack label
> mapping table and remove all of the error codes because they would then
> be unnecessary.

How is this different then filling a void *security pointer for other
kernel objects (tasks, indodes, ipc, etc). The allocations can fail as
well. I don't see how the semantics are different here.

So you would have me a Smack generic pointer in user_namespace without
any LSM abstraction? How does that cope with LSM philosophy and recently
introduced (initial) LSM stacking?


> I am also concerned that several of the operations assume that setns
> and the like are normally privileged operations and so require the
> ability to perform other privileged operations.  Given that in the right
> circumstances setns is not privileged that seems like a semantics
> mismatch.

Sorry, I don't exactly know what you mean. setns remains unprivileged
operation here. The only limit is to refuse setns when adding a process
to a container would break Smack namespace rules. In theory this
limitation could be removed (I think), because a process with an
unmapped label would not be able to do anything in the namespace. This
is mostly for convenience.


> Or in short my gut feel says the semantics of this change are close to
> something that would be very useful, but the details make this patchset
> far less useful, usable and comprehensible than it should be.

This isn't much to go on for me unfortunately.
Anyway, thank you for your insight, and taking the time.


> 
> Eric
> 
> 
> > ===================================================================
> >
> > --- What is a Smack namespace ---
> >
> > Smack namespace was developed to make it possible for Smack to work
> > nicely with Linux containers where there is a full operating system
> > with its own init inside the namespace. Such a system working with
> > Smack expects to have at least partially working SMACK_MAC_ADMIN to be
> > able to change labels of processes and files. This is required to be
> > able to securely start applications under the control of Smack and
> > manage their access rights.
> >
> > It was implemented using new LSM hooks added to the user namespace
> > that were developed together with Smack namespace.
> >
> >
> > --- Design ideas ---
> >
> > "Smack namespace" is rather "Smack labels namespace" as not the whole
> > MAC is namespaced, only the labels. There is a great analogy between
> > Smack labels namespace and the user namespace part that remaps UIDs.
> >
> > The idea is to create a map of labels for a namespace so the namespace
> > is only allowed to use those labels. Smack rules are always the same
> > as in the init namespace (limited only by what labels are mapped) and
> > cannot be manipulated from the child namespace. The map is actually
> > only for labels' names. The underlying structures for labels remain
> > the same. The filesystem also stores the "unmapped" labels from the
> > init namespace.
> >
> > Let's say we have those labels in the init namespace:
> > label1
> > label2
> > label3
> >
> > and those rules:
> > label1 label2 rwx
> > label1 label3 rwx
> > label2 label3 rwx
> >
> > We create a map for a namespace:
> > label1 -> mapped1
> > label2 -> mapped2
> >
> > This means that 'label3' is completely invisible in the namespace. As if
> > it didn't exist. All the rules that include it are ignored.
> >
> > Effectively in the namespace we have only one rule:
> > mapped1 mapped2 rwx
> >
> > Which in reality is:
> > label1 label2 rwx
> >
> > All requests to access an object with a 'label3' will be denied. If it
> > ever comes to a situation where 'label3' would have to be printed
> > (e.g. reading an exec or mmap label from a file to which we have
> > access) then huh sign '?' will be printed instead.
> >
> > All the operations in the namespace on the remaining labels will have
> > to be performed using their mapped names. Things like changing own
> > process's label, changing filesystem label. Labels will also be
> > printed with their mapped names.
> >
> > You cannot import new labels in a namespace. Every operation that
> > would do so in an init namespace will return an error in the child
> > namespace. You cannot assign an unmapped or not existing label to an
> > object. You can only operate on labels that have been explicitly
> > mapped.
> >
> >
> > --- Capabilities ---
> >
> > Enabling Smack related capabilities (CAP_MAC_ADMIN and
> > CAP_MAC_OVERRIDE) is main goal of Smack namespace, so it can work
> > properly in the container. And those capabilities do work to some
> > extent. In several places where capabilities are checked compatibility
> > with Smack namespace has been introduced. Capabilities are of course
> > limited to operate only on mapped labels.
> >
> > CAP_MAC_OVERRIDE works fully, will allow you to ignore Smack access
> > rules, but only between objects that have labels mapped. So in the
> > example above having this CAP will allow e.g. label2 to write to
> > label1, but will not allow any access to label3.
> >
> > With CAP_MAC_ADMIN the following operations has been allowed inside
> > the namespace:
> > - setting and removing xattr on files, including the security.* ones
> > - setting process's own label (/proc/self/attr/current)
> > - mounting in a privileged Smack mode, which means one can specify
> >   additional mount options like: smackfsdef, smackfsfloor etc.
> >
> > Again this is also allowed only on the mapped labels. Labels on the
> > filesystem will be stored in unmapped form so they are preserved
> > through reboots.
> >
> > Such a namespace construct allows e.g. systemd (with Smack support)
> > working in a container to assign labels properly to daemons and other
> > processes.
> >
> >
> > --- Usage ---
> >
> > Smack namespace is written using LSM hooks inside user namespace. That
> > means it's connected to it.
> >
> > To create a new Smack namespace you need to unshare() user namespace
> > as usual. If that is all you do though, than there is no difference to
> > what is now. To activate the Smack namespace you need to fill the
> > labels' map. It is in a file /proc/$PID/smack_map.
> >
> > By default the map is empty and Smack namespaces are inactive (labels
> > are taken directly from a parent namespace). It also means that the
> > Smack capabilities will be inactive. After you fill the map it starts
> > to take effect in the namespace and Smack capabilities (only on mapped
> > labels) start to work.
> >
> > Due to the way Smack works only CAP_MAC_ADMIN from the parent
> > namespace (init_user_ns for now, see the "Current limitations" below)
> > is allowed to fill the map. That means that an unprivileged user is
> > still allowed to create the user namespace but it will not be able to
> > fill the labels' map (activate Smack namespace). An administrator
> > intervention is required.
> >
> > The attr_map write format is:
> > unmapped_label mapped_label
> >
> > When reading the file it shows an active map for a namespace the
> > process in question is in in the format:
> > unmapped_label -> mapped_label
> >
> > If the smack_map file is empty it means the namespace is not mapped
> > and Smack namespace is inactive (no mappings, MAC related capabilities
> > behave as they did before, meaning they are active only in
> > init_user_ns). For init_user_ns the map will always be empty.
> >
> > Writing to the map file is not disabled after the first write as it is
> > in uid_map. For Smack we have no means to map ranges of labels, hence
> > it can really be advantageous to be able to expand the map later
> > on. But you can only add to the map. You cannot remove already mapped
> > labels. You cannot change the already existing mappings. Also mappings
> > has to be 1-1. All requests to create a map where either the unmapped
> > or the mapped label already exists in the map will be denied.
> >
> > setns() with Smack namespace active has an additional check that the
> > label of a process that is calling setns() has to be already mapped in
> > the target Smack namespace for the call to succeed.
> >
> > --- Special labels ---
> >
> > Smack is using some special labels that have built-in rules. Things
> > like floor '_', dash '^', star '*', etc. Those labels are not
> > automatically mapped to the namespace. Moreover, you can choose to map
> > a different label from the init namespace to behave e.g. like floor
> > inside the namespace.
> >
> > Let's say we have no rules and those labels in the init namespace:
> > _
> > floor_to_be
> > label
> >
> > Both 'label' and 'floor_to_be' can read objects with '_'. But they
> > have no access rights to each other.
> >
> > Now let's create a map like this:
> > _             ordinary_label
> > floor_to_be   _
> > label         mapped
> >
> > Right now label 'mapped' can read label '_' which means that
> > effectively inside this namespace label 'label' has gained read access
> > to the 'floor_to_be'. The label 'ordinary_label' is exactly it, an
> > ordinary label that the built-in rules no longer apply to inside the
> > namespace.
> >
> > To sum up, special labels in the namespace behave the same as in the
> > init namespace. Not the original special labels though, but the ones
> > we map to specials. This is the only case where a namespace can have
> > access rights the init namespace does not have (like the 'label' to
> > 'floor_to_be' in the example above).
> >
> > Of course mappings like these are perfectly legal:
> > _   _
> > *   *
> > ^   ^
> >
> >
> > --- Current limitations ---
> >
> > The Smack namespace is not hierarchical yet. It is currently not
> > possible to fill a smack_map of a nested user namespace (you can still
> > create nested user namespace, it will just inherit its parent's map
> > and won't have active Smack capabilities). When hierarchy will be
> > implemented the process creating another namespace will be allowed to
> > map only labels that it has permission to itself (those that it has in
> > its own map).
> 
> > Special files inside the virtual smackfs needs to be reviewed whether
> > it's beneficial to have some of their functionality namespaced as well
> > (e.g. onlycap, syslog. ambient, etc). This would increase
> > CAP_MAC_ADMIN privileges inside the namespace.
> >
> >
> > Lukasz Pawelczyk (7):
> >   user_ns: 3 new hooks for user namespace operations
> >   smack: extend capability functions and fix 2 checks
> >   smack: abstraction layer for 2 common Smack operations
> >   smack: misc cleanups in preparation for a namespace patch
> >   smack: namespace groundwork
> >   smack: namespace implementation
> >   smack: documentation for the Smack namespace
> >
> >  Documentation/security/00-INDEX            |   2 +
> >  Documentation/security/Smack-namespace.txt | 231 +++++++++++++
> >  MAINTAINERS                                |   1 +
> >  fs/proc/base.c                             |  57 +++
> >  include/linux/lsm_hooks.h                  |  28 ++
> >  include/linux/security.h                   |  23 ++
> >  include/linux/user_namespace.h             |   9 +
> >  kernel/user.c                              |   3 +
> >  kernel/user_namespace.c                    |  18 +
> >  security/security.c                        |  28 ++
> >  security/smack/Kconfig                     |  12 +
> >  security/smack/Makefile                    |   1 +
> >  security/smack/smack.h                     | 187 +++++++++-
> >  security/smack/smack_access.c              | 191 ++++++++--
> >  security/smack/smack_lsm.c                 | 536 ++++++++++++++++++++---------
> >  security/smack/smack_ns.c                  | 471 +++++++++++++++++++++++++
> >  security/smack/smackfs.c                   | 154 +++++----
> >  17 files changed, 1702 insertions(+), 250 deletions(-)
> >  create mode 100644 Documentation/security/Smack-namespace.txt
> >  create mode 100644 security/smack/smack_ns.c

-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics



-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists