Skip to content

Commit

Permalink
Merge tag 'landlock-6.0-rc4' of git://git.kernel.org/pub/scm/linux/ke…
Browse files Browse the repository at this point in the history 
…rnel/git/mic/linux

Pull landlock fix from Mickaël Salaün:
 "This fixes a mis-handling of the LANDLOCK_ACCESS_FS_REFER right when
  multiple rulesets/domains are stacked.

  The expected behaviour was that an additional ruleset can only
  restrict the set of permitted operations, but in this particular case,
  it was potentially possible to re-gain the LANDLOCK_ACCESS_FS_REFER
  right"

* tag 'landlock-6.0-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/mic/linux:
  landlock: Fix file reparenting without explicit LANDLOCK_ACCESS_FS_REFER
  • Loading branch information
@torvalds
torvalds committed Sep 2, 2022
2 parents b307e70 + 55e5592 commit 0c95f02
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 33 deletions.
48 changes: 25 additions & 23 deletions security/landlock/fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,16 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_READ_FILE)
/* clang-format on */

/*
* All access rights that are denied by default whether they are handled or not
* by a ruleset/layer. This must be ORed with all ruleset->fs_access_masks[]
* entries when we need to get the absolute handled access masks.
*/
/* clang-format off */
#define ACCESS_INITIALLY_DENIED ( \
LANDLOCK_ACCESS_FS_REFER)
/* clang-format on */

/*
* @path: Should have been checked by get_path_from_fd().
*/
Expand All @@ -167,7 +177,9 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
return -EINVAL;

/* Transforms relative access rights to absolute ones. */
access_rights |= LANDLOCK_MASK_ACCESS_FS & ~ruleset->fs_access_masks[0];
access_rights |=
LANDLOCK_MASK_ACCESS_FS &
~(ruleset->fs_access_masks[0] | ACCESS_INITIALLY_DENIED);
object = get_inode_object(d_backing_inode(path->dentry));
if (IS_ERR(object))
return PTR_ERR(object);
Expand Down Expand Up @@ -277,23 +289,12 @@ static inline bool is_nouser_or_private(const struct dentry *dentry)
static inline access_mask_t
get_handled_accesses(const struct landlock_ruleset *const domain)
{
access_mask_t access_dom = 0;
unsigned long access_bit;

for (access_bit = 0; access_bit < LANDLOCK_NUM_ACCESS_FS;
access_bit++) {
size_t layer_level;
access_mask_t access_dom = ACCESS_INITIALLY_DENIED;
size_t layer_level;

for (layer_level = 0; layer_level < domain->num_layers;
layer_level++) {
if (domain->fs_access_masks[layer_level] &
BIT_ULL(access_bit)) {
access_dom |= BIT_ULL(access_bit);
break;
}
}
}
return access_dom;
for (layer_level = 0; layer_level < domain->num_layers; layer_level++)
access_dom |= domain->fs_access_masks[layer_level];
return access_dom & LANDLOCK_MASK_ACCESS_FS;
}

static inline access_mask_t
Expand All @@ -316,8 +317,13 @@ init_layer_masks(const struct landlock_ruleset *const domain,

for_each_set_bit(access_bit, &access_req,
ARRAY_SIZE(*layer_masks)) {
if (domain->fs_access_masks[layer_level] &
BIT_ULL(access_bit)) {
/*
* Artificially handles all initially denied by default
* access rights.
*/
if (BIT_ULL(access_bit) &
(domain->fs_access_masks[layer_level] |
ACCESS_INITIALLY_DENIED)) {
(*layer_masks)[access_bit] |=
BIT_ULL(layer_level);
handled_accesses |= BIT_ULL(access_bit);
Expand Down Expand Up @@ -857,10 +863,6 @@ static int current_check_refer_path(struct dentry *const old_dentry,
NULL, NULL);
}

/* Backward compatibility: no reparenting support. */
if (!(get_handled_accesses(dom) & LANDLOCK_ACCESS_FS_REFER))
return -EXDEV;

access_request_parent1 |= LANDLOCK_ACCESS_FS_REFER;
access_request_parent2 |= LANDLOCK_ACCESS_FS_REFER;

Expand Down
155 changes: 145 additions & 10 deletions tools/testing/selftests/landlock/fs_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*
* Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
* Copyright © 2020 ANSSI
* Copyright © 2020-2021 Microsoft Corporation
* Copyright © 2020-2022 Microsoft Corporation
*/

#define _GNU_SOURCE
Expand Down Expand Up @@ -371,6 +371,13 @@ TEST_F_FORK(layout1, inval)
ASSERT_EQ(EINVAL, errno);
path_beneath.allowed_access &= ~LANDLOCK_ACCESS_FS_EXECUTE;

/* Tests with denied-by-default access right. */
path_beneath.allowed_access |= LANDLOCK_ACCESS_FS_REFER;
ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
&path_beneath, 0));
ASSERT_EQ(EINVAL, errno);
path_beneath.allowed_access &= ~LANDLOCK_ACCESS_FS_REFER;

/* Test with unknown (64-bits) value. */
path_beneath.allowed_access |= (1ULL << 60);
ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
Expand Down Expand Up @@ -1826,6 +1833,20 @@ TEST_F_FORK(layout1, link)
ASSERT_EQ(0, link(file1_s1d3, file2_s1d3));
}

static int test_rename(const char *const oldpath, const char *const newpath)
{
if (rename(oldpath, newpath))
return errno;
return 0;
}

static int test_exchange(const char *const oldpath, const char *const newpath)
{
if (renameat2(AT_FDCWD, oldpath, AT_FDCWD, newpath, RENAME_EXCHANGE))
return errno;
return 0;
}

TEST_F_FORK(layout1, rename_file)
{
const struct rule rules[] = {
Expand Down Expand Up @@ -1867,10 +1888,10 @@ TEST_F_FORK(layout1, rename_file)
* to a different directory (which allows file removal).
*/
ASSERT_EQ(-1, rename(file1_s2d1, file1_s1d3));
ASSERT_EQ(EXDEV, errno);
ASSERT_EQ(EACCES, errno);
ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d1, AT_FDCWD, file1_s1d3,
RENAME_EXCHANGE));
ASSERT_EQ(EXDEV, errno);
ASSERT_EQ(EACCES, errno);
ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s2d2, AT_FDCWD, file1_s1d3,
RENAME_EXCHANGE));
ASSERT_EQ(EXDEV, errno);
Expand All @@ -1894,7 +1915,7 @@ TEST_F_FORK(layout1, rename_file)
ASSERT_EQ(EXDEV, errno);
ASSERT_EQ(0, unlink(file1_s1d3));
ASSERT_EQ(-1, rename(file1_s2d1, file1_s1d3));
ASSERT_EQ(EXDEV, errno);
ASSERT_EQ(EACCES, errno);

/* Exchanges and renames files with same parent. */
ASSERT_EQ(0, renameat2(AT_FDCWD, file2_s2d3, AT_FDCWD, file1_s2d3,
Expand Down Expand Up @@ -2014,6 +2035,115 @@ TEST_F_FORK(layout1, reparent_refer)
ASSERT_EQ(0, rename(dir_s1d3, dir_s2d3));
}

/* Checks renames beneath dir_s1d1. */
static void refer_denied_by_default(struct __test_metadata *const _metadata,
const struct rule layer1[],
const int layer1_err,
const struct rule layer2[])
{
int ruleset_fd;

ASSERT_EQ(0, unlink(file1_s1d2));

ruleset_fd = create_ruleset(_metadata, layer1[0].access, layer1);
ASSERT_LE(0, ruleset_fd);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));

/*
* If the first layer handles LANDLOCK_ACCESS_FS_REFER (according to
* layer1_err), then it allows some different-parent renames and links.
*/
ASSERT_EQ(layer1_err, test_rename(file1_s1d1, file1_s1d2));
if (layer1_err == 0)
ASSERT_EQ(layer1_err, test_rename(file1_s1d2, file1_s1d1));
ASSERT_EQ(layer1_err, test_exchange(file2_s1d1, file2_s1d2));
ASSERT_EQ(layer1_err, test_exchange(file2_s1d2, file2_s1d1));

ruleset_fd = create_ruleset(_metadata, layer2[0].access, layer2);
ASSERT_LE(0, ruleset_fd);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));

/*
* Now, either the first or the second layer does not handle
* LANDLOCK_ACCESS_FS_REFER, which means that any different-parent
* renames and links are denied, thus making the layer handling
* LANDLOCK_ACCESS_FS_REFER null and void.
*/
ASSERT_EQ(EXDEV, test_rename(file1_s1d1, file1_s1d2));
ASSERT_EQ(EXDEV, test_exchange(file2_s1d1, file2_s1d2));
ASSERT_EQ(EXDEV, test_exchange(file2_s1d2, file2_s1d1));
}

const struct rule layer_dir_s1d1_refer[] = {
{
.path = dir_s1d1,
.access = LANDLOCK_ACCESS_FS_REFER,
},
{},
};

const struct rule layer_dir_s1d1_execute[] = {
{
/* Matches a parent directory. */
.path = dir_s1d1,
.access = LANDLOCK_ACCESS_FS_EXECUTE,
},
{},
};

const struct rule layer_dir_s2d1_execute[] = {
{
/* Does not match a parent directory. */
.path = dir_s2d1,
.access = LANDLOCK_ACCESS_FS_EXECUTE,
},
{},
};

/*
* Tests precedence over renames: denied by default for different parent
* directories, *with* a rule matching a parent directory, but not directly
* denying access (with MAKE_REG nor REMOVE).
*/
TEST_F_FORK(layout1, refer_denied_by_default1)
{
refer_denied_by_default(_metadata, layer_dir_s1d1_refer, 0,
layer_dir_s1d1_execute);
}

/*
* Same test but this time turning around the ABI version order: the first
* layer does not handle LANDLOCK_ACCESS_FS_REFER.
*/
TEST_F_FORK(layout1, refer_denied_by_default2)
{
refer_denied_by_default(_metadata, layer_dir_s1d1_execute, EXDEV,
layer_dir_s1d1_refer);
}

/*
* Tests precedence over renames: denied by default for different parent
* directories, *without* a rule matching a parent directory, but not directly
* denying access (with MAKE_REG nor REMOVE).
*/
TEST_F_FORK(layout1, refer_denied_by_default3)
{
refer_denied_by_default(_metadata, layer_dir_s1d1_refer, 0,
layer_dir_s2d1_execute);
}

/*
* Same test but this time turning around the ABI version order: the first
* layer does not handle LANDLOCK_ACCESS_FS_REFER.
*/
TEST_F_FORK(layout1, refer_denied_by_default4)
{
refer_denied_by_default(_metadata, layer_dir_s2d1_execute, EXDEV,
layer_dir_s1d1_refer);
}

TEST_F_FORK(layout1, reparent_link)
{
const struct rule layer1[] = {
Expand Down Expand Up @@ -2336,11 +2466,12 @@ TEST_F_FORK(layout1, reparent_exdev_layers_rename1)
ASSERT_EQ(EXDEV, errno);

/*
* However, moving the file2_s1d3 file below dir_s2d3 is allowed
* because it cannot inherit MAKE_REG nor MAKE_DIR rights (which are
* dedicated to directories).
* Moving the file2_s1d3 file below dir_s2d3 is denied because the
* second layer does not handle REFER, which is always denied by
* default.
*/
ASSERT_EQ(0, rename(file2_s1d3, file1_s2d3));
ASSERT_EQ(-1, rename(file2_s1d3, file1_s2d3));
ASSERT_EQ(EXDEV, errno);
}

TEST_F_FORK(layout1, reparent_exdev_layers_rename2)
Expand Down Expand Up @@ -2373,8 +2504,12 @@ TEST_F_FORK(layout1, reparent_exdev_layers_rename2)
ASSERT_EQ(EACCES, errno);
ASSERT_EQ(-1, rename(file1_s1d1, file1_s2d3));
ASSERT_EQ(EXDEV, errno);
/* Modify layout! */
ASSERT_EQ(0, rename(file2_s1d2, file1_s2d3));
/*
* Modifying the layout is now denied because the second layer does not
* handle REFER, which is always denied by default.
*/
ASSERT_EQ(-1, rename(file2_s1d2, file1_s2d3));
ASSERT_EQ(EXDEV, errno);

/* Without REFER source, EACCES wins over EXDEV. */
ASSERT_EQ(-1, rename(dir_s1d1, file1_s2d2));
Expand Down

0 comments on commit 0c95f02

Please sign in to comment.