landlock: Fix same-layer rule unions
commit 8ba0005ff4 upstream.
The original behavior was to check if the full set of requested accesses
was allowed by at least a rule of every relevant layer. This didn't
take into account requests for multiple accesses and same-layer rules
allowing the union of these accesses in a complementary way. As a
result, multiple accesses requested on a file hierarchy matching rules
that, together, allowed these accesses, but without a unique rule
allowing all of them, was illegitimately denied. This case should be
rare in practice and it can only be triggered by the path_rename or
file_open hook implementations.
For instance, if, for the same layer, a rule allows execution
beneath /a/b and another rule allows read beneath /a, requesting access
to read and execute at the same time for /a/b should be allowed for this
layer.
This was an inconsistency because the union of same-layer rule accesses
was already allowed if requested once at a time anyway.
This fix changes the way allowed accesses are gathered over a path walk.
To take into account all these rule accesses, we store in a matrix all
layer granting the set of requested accesses, according to the handled
accesses. To avoid heap allocation, we use an array on the stack which
is 2*13 bytes. A following commit bringing the LANDLOCK_ACCESS_FS_REFER
access right will increase this size to reach 112 bytes (2*14*4) in case
of link or rename actions.
Add a new layout1.layer_rule_unions test to check that accesses from
different rules pertaining to the same layer are ORed in a file
hierarchy. Also test that it is not the case for rules from different
layers.
Reviewed-by: Paul Moore <paul@paul-moore.com>
Link: https://lore.kernel.org/r/20220506161102.525323-5-mic@digikod.net
Cc: stable@vger.kernel.org
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Mickaël Salaün
Greg Kroah-Hartman
Родитель
f859580c27
Коммит
f55f9e7ce6
|
|
@ -207,45 +207,67 @@ find_rule(const struct landlock_ruleset *const domain,
|
|||
return rule;
|
||||
}
|
||||
|
||||
static inline layer_mask_t unmask_layers(const struct landlock_rule *const rule,
|
||||
const access_mask_t access_request,
|
||||
layer_mask_t layer_mask)
|
||||
/*
|
||||
* @layer_masks is read and may be updated according to the access request and
|
||||
* the matching rule.
|
||||
*
|
||||
* Returns true if the request is allowed (i.e. relevant layer masks for the
|
||||
* request are empty).
|
||||
*/
|
||||
static inline bool
|
||||
unmask_layers(const struct landlock_rule *const rule,
|
||||
const access_mask_t access_request,
|
||||
layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
|
||||
{
|
||||
size_t layer_level;
|
||||
|
||||
if (!access_request || !layer_masks)
|
||||
return true;
|
||||
if (!rule)
|
||||
return layer_mask;
|
||||
return false;
|
||||
|
||||
/*
|
||||
* An access is granted if, for each policy layer, at least one rule
|
||||
* encountered on the pathwalk grants the requested accesses,
|
||||
* regardless of their position in the layer stack. We must then check
|
||||
* encountered on the pathwalk grants the requested access,
|
||||
* regardless of its position in the layer stack. We must then check
|
||||
* the remaining layers for each inode, from the first added layer to
|
||||
* the last one.
|
||||
* the last one. When there is multiple requested accesses, for each
|
||||
* policy layer, the full set of requested accesses may not be granted
|
||||
* by only one rule, but by the union (binary OR) of multiple rules.
|
||||
* E.g. /a/b <execute> + /a <read> => /a/b <execute + read>
|
||||
*/
|
||||
for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
|
||||
const struct landlock_layer *const layer =
|
||||
&rule->layers[layer_level];
|
||||
const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
|
||||
const unsigned long access_req = access_request;
|
||||
unsigned long access_bit;
|
||||
bool is_empty;
|
||||
|
||||
/* Checks that the layer grants access to the full request. */
|
||||
if ((layer->access & access_request) == access_request) {
|
||||
layer_mask &= ~layer_bit;
|
||||
|
||||
if (layer_mask == 0)
|
||||
return layer_mask;
|
||||
/*
|
||||
* Records in @layer_masks which layer grants access to each
|
||||
* requested access.
|
||||
*/
|
||||
is_empty = true;
|
||||
for_each_set_bit(access_bit, &access_req,
|
||||
ARRAY_SIZE(*layer_masks)) {
|
||||
if (layer->access & BIT_ULL(access_bit))
|
||||
(*layer_masks)[access_bit] &= ~layer_bit;
|
||||
is_empty = is_empty && !(*layer_masks)[access_bit];
|
||||
}
|
||||
if (is_empty)
|
||||
return true;
|
||||
}
|
||||
return layer_mask;
|
||||
return false;
|
||||
}
|
||||
|
||||
static int check_access_path(const struct landlock_ruleset *const domain,
|
||||
const struct path *const path,
|
||||
const access_mask_t access_request)
|
||||
{
|
||||
bool allowed = false;
|
||||
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
|
||||
bool allowed = false, has_access = false;
|
||||
struct path walker_path;
|
||||
layer_mask_t layer_mask;
|
||||
size_t i;
|
||||
|
||||
if (!access_request)
|
||||
|
|
@ -265,13 +287,20 @@ static int check_access_path(const struct landlock_ruleset *const domain,
|
|||
return -EACCES;
|
||||
|
||||
/* Saves all layers handling a subset of requested accesses. */
|
||||
layer_mask = 0;
|
||||
for (i = 0; i < domain->num_layers; i++) {
|
||||
if (domain->fs_access_masks[i] & access_request)
|
||||
layer_mask |= BIT_ULL(i);
|
||||
const unsigned long access_req = access_request;
|
||||
unsigned long access_bit;
|
||||
|
||||
for_each_set_bit(access_bit, &access_req,
|
||||
ARRAY_SIZE(layer_masks)) {
|
||||
if (domain->fs_access_masks[i] & BIT_ULL(access_bit)) {
|
||||
layer_masks[access_bit] |= BIT_ULL(i);
|
||||
has_access = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
/* An access request not handled by the domain is allowed. */
|
||||
if (layer_mask == 0)
|
||||
if (!has_access)
|
||||
return 0;
|
||||
|
||||
walker_path = *path;
|
||||
|
|
@ -283,14 +312,11 @@ static int check_access_path(const struct landlock_ruleset *const domain,
|
|||
while (true) {
|
||||
struct dentry *parent_dentry;
|
||||
|
||||
layer_mask =
|
||||
unmask_layers(find_rule(domain, walker_path.dentry),
|
||||
access_request, layer_mask);
|
||||
if (layer_mask == 0) {
|
||||
allowed = unmask_layers(find_rule(domain, walker_path.dentry),
|
||||
access_request, &layer_masks);
|
||||
if (allowed)
|
||||
/* Stops when a rule from each layer grants access. */
|
||||
allowed = true;
|
||||
break;
|
||||
}
|
||||
|
||||
jump_up:
|
||||
if (walker_path.dentry == walker_path.mnt->mnt_root) {
|
||||
|
|
|
|||
|
|
@ -22,6 +22,8 @@
|
|||
typedef u16 access_mask_t;
|
||||
/* Makes sure all filesystem access rights can be stored. */
|
||||
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
|
||||
/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
|
||||
static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
|
||||
|
||||
typedef u16 layer_mask_t;
|
||||
/* Makes sure all layers can be checked. */
|
||||
|
|
|
|||
|
|
@ -758,6 +758,113 @@ TEST_F_FORK(layout1, ruleset_overlap)
|
|||
ASSERT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
|
||||
}
|
||||
|
||||
TEST_F_FORK(layout1, layer_rule_unions)
|
||||
{
|
||||
const struct rule layer1[] = {
|
||||
{
|
||||
.path = dir_s1d2,
|
||||
.access = LANDLOCK_ACCESS_FS_READ_FILE,
|
||||
},
|
||||
/* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */
|
||||
{
|
||||
.path = dir_s1d3,
|
||||
.access = LANDLOCK_ACCESS_FS_WRITE_FILE,
|
||||
},
|
||||
{},
|
||||
};
|
||||
const struct rule layer2[] = {
|
||||
/* Doesn't change anything from layer1. */
|
||||
{
|
||||
.path = dir_s1d2,
|
||||
.access = LANDLOCK_ACCESS_FS_READ_FILE |
|
||||
LANDLOCK_ACCESS_FS_WRITE_FILE,
|
||||
},
|
||||
{},
|
||||
};
|
||||
const struct rule layer3[] = {
|
||||
/* Only allows write (but not read) to dir_s1d3. */
|
||||
{
|
||||
.path = dir_s1d2,
|
||||
.access = LANDLOCK_ACCESS_FS_WRITE_FILE,
|
||||
},
|
||||
{},
|
||||
};
|
||||
int ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1);
|
||||
|
||||
ASSERT_LE(0, ruleset_fd);
|
||||
enforce_ruleset(_metadata, ruleset_fd);
|
||||
ASSERT_EQ(0, close(ruleset_fd));
|
||||
|
||||
/* Checks s1d1 hierarchy with layer1. */
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Checks s1d2 hierarchy with layer1. */
|
||||
ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Checks s1d3 hierarchy with layer1. */
|
||||
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
|
||||
ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY));
|
||||
/* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */
|
||||
ASSERT_EQ(0, test_open(file1_s1d3, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Doesn't change anything from layer1. */
|
||||
ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer2);
|
||||
ASSERT_LE(0, ruleset_fd);
|
||||
enforce_ruleset(_metadata, ruleset_fd);
|
||||
ASSERT_EQ(0, close(ruleset_fd));
|
||||
|
||||
/* Checks s1d1 hierarchy with layer2. */
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Checks s1d2 hierarchy with layer2. */
|
||||
ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Checks s1d3 hierarchy with layer2. */
|
||||
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
|
||||
ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY));
|
||||
/* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */
|
||||
ASSERT_EQ(0, test_open(file1_s1d3, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Only allows write (but not read) to dir_s1d3. */
|
||||
ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer3);
|
||||
ASSERT_LE(0, ruleset_fd);
|
||||
enforce_ruleset(_metadata, ruleset_fd);
|
||||
ASSERT_EQ(0, close(ruleset_fd));
|
||||
|
||||
/* Checks s1d1 hierarchy with layer3. */
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Checks s1d2 hierarchy with layer3. */
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
|
||||
/* Checks s1d3 hierarchy with layer3. */
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d3, O_RDONLY));
|
||||
ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY));
|
||||
/* dir_s1d3 should now deny READ_FILE and WRITE_FILE (O_RDWR). */
|
||||
ASSERT_EQ(EACCES, test_open(file1_s1d3, O_RDWR));
|
||||
ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
|
||||
}
|
||||
|
||||
TEST_F_FORK(layout1, non_overlapping_accesses)
|
||||
{
|
||||
const struct rule layer1[] = {
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче