[PATCH v1] landlock: Fix file reparenting without explicit LANDLOCK_ACCESS_FS_REFER

[Mickaël Salaün]

On 25/08/2022 22:16, Paul Moore wrote:
> On Wed, Aug 24, 2022 at 5:04 AM Mickaël Salaün <mic at digikod.net> wrote:
>> On 23/08/2022 22:07, Günther Noack wrote:
>>> On Tue, Aug 23, 2022 at 04:41:23PM +0200, Mickaël Salaün wrote:
>>>> With the introduction of LANDLOCK_ACCESS_FS_REFER, we added the first
>>>> globally denied-by-default access right.  Indeed, this lifted an initial
>>>> Landlock limitation to rename and link files, which was initially always
>>>> denied when the source or the destination were different directories.
>>>>
>>>> This led to an inconsistent backward compatibility behavior which was
>>>> only taken into account if no domain layer were using the new
>>>> LANDLOCK_ACCESS_FS_REFER right.  However, in a scenario where layers are
>>>> using the first and the second Landlock ABI (i.e.
>>>> LANDLOCK_ACCESS_FS_REFER or not), the access control behaves like if all
>>>> domains were handling LANDLOCK_ACCESS_FS_REFER with their rules
>>>> implicitely allowing such right.
>>
>> "the access control behaves like if domains not handling
>> LANDLOCK_ACCESS_FS_REFER are in fact handling it and with their rules
>> implicitely allowing such right."
>>
>> Is this better?
> 
> I'm still looking at the actual code changes, but I had similar
> problems as Günther while I was reading the description.  While the
> new text above is different, I'm not sure it is significantly easier
> to understand.  I might suggest adding a short example to the commit
> description showing what happens now and what will change with this
> patch; similar to what Günther did in his reply.

OK, what about these new paragraphs replacing the second one?

This led to an inconsistent backward compatibility behavior which was
only taken into account if no domain layer were using the new
LANDLOCK_ACCESS_FS_REFER right. However, when restricting a thread with 
a new ruleset handling LANDLOCK_ACCESS_FS_REFER, all inherited parent 
ruleset/layers not explicitly handling LANDLOCK_ACCESS_FS_REFER would 
behave as if they were handling this access right and with all their 
rules allowing it. This means that renaming and linking files could 
became allowed by these parent layers, but all the other required 
accesses must also be granted: all layers must allow file removal or 
creation, and renaming and linking operations must not lead to privilege 
escalation according to the Landlock policy (see detailed explanation in 
commit b91c3e4ea756).

To say it another way, this bug may lifts the renaming and linking 
limitations of the initial Landlock version, and a same ruleset can 
restrict differently depending on previous or next enforced ruleset 
(i.e. inconsistent behavior). The LANDLOCK_ACCESS_FS_REFER right cannot 
give access to data not already allowed, but this doesn't follow the 
contract of the first Landlock ABI. This fix puts back the limitation 
for sandboxes that didn't opt-in for this additional right.

For instance, if a first ruleset allows LANDLOCK_ACCESS_FS_MAKE_REG on 
/dst and LANDLOCK_ACCESS_FS_REMOVE_FILE on /src, renaming /src/file to 
/dst/file is denied. However, without this fix, stacking a new ruleset 
which allows LANDLOCK_ACCESS_FS_REFER on / would now permit the 
sandboxed thread to rename /src/file to /dst/file .