Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control
On Thu, 14 May 2020, Mickaël Salaün wrote: > > fsnotify is not an LSM. > > Yes, so I'll need to add a new LSM hook for this (release) call, right? Unless an existing one will work. -- James Morris
Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control
On 14/05/2020 19:31, James Morris wrote: > On Thu, 14 May 2020, Mickaël Salaün wrote: > >>> This needs to be converted to the LSM API via superblock blob stacking. >>> >>> See Casey's old patch: >>> https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/ >> >> s_landlock_inode_refs is quite similar to s_fsnotify_inode_refs, but I >> can do it once the superblock security blob patch is upstream. Is it a >> blocker for now? What is the current status of lbs_superblock? > > Yes it is a blocker. Landlock should not be adding its own functions in > core code, it should be using the LSM API (and extending that as needed). OK, I'll use that in the next series. > >> Anyway, we also need to have a call to landlock_release_inodes() in >> generic_shutdown_super(), which does not fit the LSM framework, and I >> think it is not an issue. Landlock handling of inodes is quite similar >> to fsnotify. > > fsnotify is not an LSM. Yes, so I'll need to add a new LSM hook for this (release) call, right?
Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control
On 14/05/2020 17:58, Casey Schaufler wrote:
> On 5/14/2020 3:39 AM, Mickaël Salaün wrote:
>> On 14/05/2020 05:37, James Morris wrote:
>>> On Mon, 11 May 2020, Mickaël Salaün wrote:
>>>
>>>
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 45cc10cdf6dd..2276642f8e05 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1517,6 +1517,11 @@ struct super_block {
/* Pending fsnotify inode refs */
atomic_long_t s_fsnotify_inode_refs;
+#ifdef CONFIG_SECURITY_LANDLOCK
+ /* References to Landlock underlying objects */
+ atomic_long_t s_landlock_inode_refs;
+#endif
+
>>> This needs to be converted to the LSM API via superblock blob stacking.
>>>
>>> See Casey's old patch:
>>> https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/
>> s_landlock_inode_refs is quite similar to s_fsnotify_inode_refs, but I
>> can do it once the superblock security blob patch is upstream. Is it a
>> blocker for now? What is the current status of lbs_superblock?
>
> As no currently stackable modules conflict over the superblock
> (SELinux and Smack are the existing users) there has been no need
> to move its management into the infrastructure. The active push for
> stacking does not (yet) include everything needed for SELinux+Smack.
> It includes what is needed for SELinux+AppArmor and Smack+AppArmor.
> That does not include the superblock blob.
>
> You can include a patch in the landlock set that provides infrastructure
> management of the superblock blob. Feel free to glean it from my proposal.
OK, I'll add it to the next series.
Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control
On Thu, 14 May 2020, Mickaël Salaün wrote: > > This needs to be converted to the LSM API via superblock blob stacking. > > > > See Casey's old patch: > > https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/ > > s_landlock_inode_refs is quite similar to s_fsnotify_inode_refs, but I > can do it once the superblock security blob patch is upstream. Is it a > blocker for now? What is the current status of lbs_superblock? Yes it is a blocker. Landlock should not be adding its own functions in core code, it should be using the LSM API (and extending that as needed). > Anyway, we also need to have a call to landlock_release_inodes() in > generic_shutdown_super(), which does not fit the LSM framework, and I > think it is not an issue. Landlock handling of inodes is quite similar > to fsnotify. fsnotify is not an LSM. -- James Morris
Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control
On 5/14/2020 3:39 AM, Mickaël Salaün wrote:
> On 14/05/2020 05:37, James Morris wrote:
>> On Mon, 11 May 2020, Mickaël Salaün wrote:
>>
>>
>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>> index 45cc10cdf6dd..2276642f8e05 100644
>>> --- a/include/linux/fs.h
>>> +++ b/include/linux/fs.h
>>> @@ -1517,6 +1517,11 @@ struct super_block {
>>> /* Pending fsnotify inode refs */
>>> atomic_long_t s_fsnotify_inode_refs;
>>>
>>> +#ifdef CONFIG_SECURITY_LANDLOCK
>>> + /* References to Landlock underlying objects */
>>> + atomic_long_t s_landlock_inode_refs;
>>> +#endif
>>> +
>> This needs to be converted to the LSM API via superblock blob stacking.
>>
>> See Casey's old patch:
>> https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/
> s_landlock_inode_refs is quite similar to s_fsnotify_inode_refs, but I
> can do it once the superblock security blob patch is upstream. Is it a
> blocker for now? What is the current status of lbs_superblock?
As no currently stackable modules conflict over the superblock
(SELinux and Smack are the existing users) there has been no need
to move its management into the infrastructure. The active push for
stacking does not (yet) include everything needed for SELinux+Smack.
It includes what is needed for SELinux+AppArmor and Smack+AppArmor.
That does not include the superblock blob.
You can include a patch in the landlock set that provides infrastructure
management of the superblock blob. Feel free to glean it from my proposal.
>
> Anyway, we also need to have a call to landlock_release_inodes() in
> generic_shutdown_super(), which does not fit the LSM framework, and I
> think it is not an issue. Landlock handling of inodes is quite similar
> to fsnotify.
Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control
On 14/05/2020 05:37, James Morris wrote:
> On Mon, 11 May 2020, Mickaël Salaün wrote:
>
>
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 45cc10cdf6dd..2276642f8e05 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -1517,6 +1517,11 @@ struct super_block {
>> /* Pending fsnotify inode refs */
>> atomic_long_t s_fsnotify_inode_refs;
>>
>> +#ifdef CONFIG_SECURITY_LANDLOCK
>> +/* References to Landlock underlying objects */
>> +atomic_long_t s_landlock_inode_refs;
>> +#endif
>> +
>
> This needs to be converted to the LSM API via superblock blob stacking.
>
> See Casey's old patch:
> https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/
s_landlock_inode_refs is quite similar to s_fsnotify_inode_refs, but I
can do it once the superblock security blob patch is upstream. Is it a
blocker for now? What is the current status of lbs_superblock?
Anyway, we also need to have a call to landlock_release_inodes() in
generic_shutdown_super(), which does not fit the LSM framework, and I
think it is not an issue. Landlock handling of inodes is quite similar
to fsnotify.
Re: [PATCH v17 05/10] fs,landlock: Support filesystem access-control
On Mon, 11 May 2020, Mickaël Salaün wrote:
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 45cc10cdf6dd..2276642f8e05 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1517,6 +1517,11 @@ struct super_block {
> /* Pending fsnotify inode refs */
> atomic_long_t s_fsnotify_inode_refs;
>
> +#ifdef CONFIG_SECURITY_LANDLOCK
> + /* References to Landlock underlying objects */
> + atomic_long_t s_landlock_inode_refs;
> +#endif
> +
This needs to be converted to the LSM API via superblock blob stacking.
See Casey's old patch:
https://lore.kernel.org/linux-security-module/20190829232935.7099-2-ca...@schaufler-ca.com/
--
James Morris
[PATCH v17 05/10] fs,landlock: Support filesystem access-control
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(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_STATES for UML because it currently does not
keep the same struct inodes for the same inodes whereas these inodes are
in use.
Signed-off-by: Mickaël Salaün
Cc: Alexander Viro
Cc: James Morris
Cc: Jann Horn
Cc: Kees Cook
Cc: Serge E. Hallyn
---
Changes since v16:
* Add ARCH_EPHEMERAL_STATES and enable it for UML.
Changes since v15:
* Replace layer_levels and layer_depth with a bitfield of layers: this
enables to properly manage superset and subset of access rights,
whatever their order in the stack of layers.
Cf.
https://lore.kernel.org/lkml/e07fe473-1801-01cc-12ae-b3167f952...@digikod.net/
* Allow to open pipes and similar special files through /proc/self/fd/.
* Properly handle internal filesystems such as nsfs: always allow these
kind of roots because disconnected path cannot be evaluated.
* Remove the LANDLOCK_ACCESS_FS_LINK_TO and
LANDLOCK_ACCESS_FS_RENAME_{TO,FROM}, but use the
LANDLOCK_ACCESS_FS_REMOVE_{FILE,DIR} and LANDLOCK_ACCESS_FS_MAKE_*
instead. Indeed, it is not possible for now (and not really useful)
to express the semantic of a source and a destination.
* Check access rights to remove a directory or a file with rename(2).
* Forbid reparenting when linking or renaming. This is needed to easily
protect against possible privilege escalation by changing the place of
a file or directory in relation to an enforced access policy (from the
set of layers). This will be relaxed in the future.
* Update hooks to take into account replacement of the object's self and
beneath access bitfields with one. Simplify the code.
* Check file related access rights.
* Check d_is_negative() instead of !d_backing_inode() in
check_access_path_continue(), and continue the path walk while there
is no mapped inode e.g., with rename(2).
* Check private inode in check_access_path().
* Optimize get_file_access() when dealing with a directory.
* Add missing atomic.h .
Changes since v14:
* Simplify the object, rule and ruleset management at the expense of a
less aggressive memory freeing (contributed by Jann Horn, with
additional modifications):
- Rewrite release_inode() to use inode->sb->s_landlock_inode_refs.
- Remove useless checks in landlock_release_inodes(), clean object
pointer according to the new struct landlock_object and wait for all
iput() to complete.
- Rewrite get_inode_object() according to the new struct
landlock_object. If there is a race-condition when cleaning up an
object, we retry until the concurrent thread finished the object
cleaning.
Cf.
https://lore.kernel.org/lkml/cag48ez21ben0wl1bbmtiiu8j9jp5iewthowz4turuj+ki0y...@mail.gmail.com/
* Fix nested domains by implementing a notion of layer level and depth:
- Check for matching level ranges when walking through a file path.
- Only allow access if every layer granted the access request.
* Handles files without mount points (e.g. pipes).
* Hardens path walk by checking inode pointer values.
* Prefetches d_parent when walking to the root directory.
* Remove useless inode_alloc_security hook() (suggested by Jann Horn):
already initialized by lsm_inode_alloc().
* Remove the inode_free_security hook.
* Remove access checks that may be required for FD-only requests:
truncate, getattr, lock, chmod, chown, chgrp, ioctl. This will be
handle in a future evolution of Landlock, but right now the goal is to
lighten the code to ease review.
* Constify variables.
* Move ABI checks into syscall.c .
* Cosmetic variable renames.
Changes since v11:
* Add back, revamp and make a fully working filesystem access-control
based on paths and inodes.
* Remove the eBPF dependency.
Previous changes:
https://lore.kernel.org/lkml/20190721213116.23476-6-...@digikod.net/
---
MAINTAINERS| 1 +
arch/Kconfig | 7 +
arch/um/Kconfig| 1 +
fs/super.c | 2 +
include/linux/fs.h | 5 +
include/linux/landlock.h | 22 ++
security/landlock/Kconfig | 2 +-
security/landlock/Makefile | 2 +-
security/landlock/fs.c | 601 +
security/landlock/fs.h | 42 +++
security/landlock/setup.c | 6 +
security/landl
