LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: Jann Horn <jannh@google.com>
Cc: "James Morris" <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andy Lutomirski" <luto@amacapital.net>,
	"Anton Ivanov" <anton.ivanov@cambridgegreys.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Casey Schaufler" <casey@schaufler-ca.com>,
	"Jeff Dike" <jdike@addtoit.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Kees Cook" <keescook@chromium.org>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Shuah Khan" <shuah@kernel.org>,
	"Vincent Dagonneau" <vincent.dagonneau@ssi.gouv.fr>,
	"Kernel Hardening" <kernel-hardening@lists.openwall.com>,
	"Linux API" <linux-api@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	"kernel list" <linux-kernel@vger.kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK"
	<linux-kselftest@vger.kernel.org>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	"Mickaël Salaün" <mic@linux.microsoft.com>
Subject: Re: [PATCH v26 07/12] landlock: Support filesystem access-control
Date: Thu, 14 Jan 2021 19:54:36 +0100
Message-ID: <aeb3e152-8108-89d2-0577-4b130368f14f@digikod.net> (raw)
In-Reply-To: <CAG48ez1wbAQwU-eoC9DngHyUM_5F01MJQpRnLaJFvfRUrnXBdA@mail.gmail.com>


On 14/01/2021 04:22, Jann Horn wrote:
> On Wed, Dec 9, 2020 at 8:28 PM Mickaël Salaün <mic@digikod.net> wrote:
>> Thanks to the Landlock objects and ruleset, it is possible to identify
>> inodes according to a process's domain.  To enable an unprivileged
>> process to express a file hierarchy, it first needs to open a directory
>> (or a file) and pass this file descriptor to the kernel through
>> landlock_add_rule(2).  When checking if a file access request is
>> allowed, we walk from the requested dentry to the real root, following
>> the different mount layers.  The access to each "tagged" inodes are
>> collected according to their rule layer level, and ANDed to create
>> access to the requested file hierarchy.  This makes possible to identify
>> a lot of files without tagging every inodes nor modifying the
>> filesystem, while still following the view and understanding the user
>> has from the filesystem.
>>
>> Add a new ARCH_EPHEMERAL_INODES for UML because it currently does not
>> keep the same struct inodes for the same inodes whereas these inodes are
>> in use.
>>
>> This commit adds a minimal set of supported filesystem access-control
>> which doesn't enable to restrict all file-related actions.  This is the
>> result of multiple discussions to minimize the code of Landlock to ease
>> review.  Thanks to the Landlock design, extending this access-control
>> without breaking user space will not be a problem.  Moreover, seccomp
>> filters can be used to restrict the use of syscall families which may
>> not be currently handled by Landlock.
> [...]
>> +static bool check_access_path_continue(
>> +               const struct landlock_ruleset *const domain,
>> +               const struct path *const path, const u32 access_request,
>> +               u64 *const layer_mask)
>> +{
> [...]
>> +       /*
>> +        * An access is granted if, for each policy layer, at least one rule
>> +        * encountered on the pathwalk grants the access, regardless of their
>> +        * position in the layer stack.  We must then check not-yet-seen layers
>> +        * for each inode, from the last one added to the first one.
>> +        */
>> +       for (i = 0; i < rule->num_layers; i++) {
>> +               const struct landlock_layer *const layer = &rule->layers[i];
>> +               const u64 layer_level = BIT_ULL(layer->level - 1);
>> +
>> +               if (!(layer_level & *layer_mask))
>> +                       continue;
>> +               if ((layer->access & access_request) != access_request)
>> +                       return false;
>> +               *layer_mask &= ~layer_level;
> 
> Hmm... shouldn't the last 5 lines be replaced by the following?
> 
> if ((layer->access & access_request) == access_request)
>     *layer_mask &= ~layer_level;
> 
> And then, since this function would always return true, you could
> change its return type to "void".
> 
> 
> As far as I can tell, the current version will still, if a ruleset
> looks like this:
> 
> /usr read+write
> /usr/lib/ read
> 
> reject write access to /usr/lib, right?

If these two rules are from different layers, then yes it would work as
intended. However, if these rules are from the same layer the path walk
will not stop at /usr/lib but go down to /usr, which grants write
access. This is the reason I wrote it like this and the
layout1.inherit_subset test checks that. I'm updating the documentation
to better explain how an access is checked with one or multiple layers.

Doing this way also enables to stop the path walk earlier, which is the
original purpose of this function.


> 
> 
>> +       }
>> +       return true;
>> +}

  reply index

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-09 19:28 [PATCH v26 00/12] Landlock LSM Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 01/12] landlock: Add object management Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 02/12] landlock: Add ruleset and domain management Mickaël Salaün
2021-01-14  3:21   ` Jann Horn
2020-12-09 19:28 ` [PATCH v26 03/12] landlock: Set up the security framework and manage credentials Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 04/12] landlock: Add ptrace restrictions Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 05/12] LSM: Infrastructure management of the superblock Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 06/12] fs,security: Add sb_delete hook Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 07/12] landlock: Support filesystem access-control Mickaël Salaün
2021-01-14  3:22   ` Jann Horn
2021-01-14 18:54     ` Mickaël Salaün [this message]
2021-01-14 22:43       ` Jann Horn
2021-01-15  9:10         ` Mickaël Salaün
2021-01-15 18:31           ` Jann Horn
2021-01-16 17:16             ` Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 08/12] landlock: Add syscall implementations Mickaël Salaün
2020-12-10 10:38   ` Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 09/12] arch: Wire up Landlock syscalls Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 10/12] selftests/landlock: Add user space tests Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 11/12] samples/landlock: Add a sandbox manager example Mickaël Salaün
2021-01-14  3:21   ` Jann Horn
2021-01-14 18:59     ` Mickaël Salaün
2020-12-09 19:28 ` [PATCH v26 12/12] landlock: Add user and kernel documentation Mickaël Salaün
2021-01-14  3:22 ` [PATCH v26 00/12] Landlock LSM Jann Horn
2021-01-14 19:03   ` Mickaël Salaün

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aeb3e152-8108-89d2-0577-4b130368f14f@digikod.net \
    --to=mic@digikod.net \
    --cc=anton.ivanov@cambridgegreys.com \
    --cc=arnd@arndb.de \
    --cc=casey@schaufler-ca.com \
    --cc=corbet@lwn.net \
    --cc=jannh@google.com \
    --cc=jdike@addtoit.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mic@linux.microsoft.com \
    --cc=mtk.manpages@gmail.com \
    --cc=richard@nod.at \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=vincent.dagonneau@ssi.gouv.fr \
    --cc=viro@zeniv.linux.org.uk \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git