Re: [LTP] [PATCH v3 09/11] Add landlock04 test

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi!

it seems like the landlock() support in kernel 6.6 is different than the 
one in 6.7. The reason why we see that error in kernel <=6.6 is related 
to how landlock is handling the rules set according to the rule we want 
to enable.

Let's suppose we want to enable the execution for a file. What we should 
be able to do, is to consider __ALL__ the rules available for landlock, 
then to enable EXEC only for a specific file. Then, if we make any other 
operation that is not EXEC, we should have a permission error. This 
translates to:

- set ruleset_attr->handled_access_fs for all available 
LANDLOCK_ACCESS_FS_* rules
- set path_beneath_attr->allowed_access to LANDLOCK_ACCESS_FS_EXEC | 
LANDLOCK_ACCESS_FS_READ (we need to read in order to execute) for a binary
- enforce the rules inside a sandbox containing the binary
- execute the binary will work
- do any other operation inside the sandbox and obtain a permissions error
- at this point, any new rule that is added, will update the list of 
landlock rules, enabling the sandbox permissions

For some reasons that I don't know (and this is evident from kselftests 
as well), if the initial rules set (ruleset_attr->handled_access_fs) is 
not identical to the rules we are going to enable 
(path_beneath_attr->allowed_access), landlock_add_rule() will fail with 
EINVAL. And this is our case for all kernels <=6.6.

I really have no idea why this happens and maybe we need to contact the 
landlock developers.

Andrea

On 7/24/24 15:47, Andrea Cervesato wrote:
> Hi Li,
>
> thanks for checking. Mmmh I don't know if it's because they added 
> LANDLOCK_RULE_NET_PORT. It sounds strange to me, since that would 
> break all the other features.
>
> Andrea
>
> On 7/24/24 14:12, Li Wang wrote:
>> Hi Petr, Andrea,
>>
>> On Wed, Jul 17, 2024 at 1:27 AM Petr Vorel <pvorel@suse.cz> wrote:
>>
>>     Hi Andrea,
>>
>>     ...
>>     > +static void enable_exec_libs(const int ruleset_fd)
>>     > +{
>>     > +     FILE *fp;
>>     > +     char line[1024];
>>     > +     char path[PATH_MAX];
>>     > +     char dependency[8][PATH_MAX];
>>     > +     int count = 0;
>>     > +     int duplicate = 0;
>>     > +
>>     > +     fp = SAFE_FOPEN("/proc/self/maps", "r");
>>     > +
>>     > +     while (fgets(line, sizeof(line), fp)) {
>>     > +             if (strstr(line, ".so") == NULL)
>>     > +                     continue;
>>     > +
>>     > +             SAFE_SSCANF(line, "%*x-%*x %*s %*x %*s %*d %s",
>>     path);
>>     > +
>>     > +             for (int i = 0; i < count; i++) {
>>     > +                     if (strcmp(path, dependency[i]) == 0) {
>>     > +                             duplicate = 1;
>>     > +                             break;
>>     > +                     }
>>     > +             }
>>     > +
>>     > +             if (duplicate) {
>>     > +                     duplicate = 0;
>>     > +                     continue;
>>     > +             }
>>     > +
>>     > +             strncpy(dependency[count], path, PATH_MAX);
>>     > +             count++;
>>     > +
>>     > +             tst_res(TINFO, "Enable read/exec permissions for
>>     %s", path);
>>     > +
>>     > +             path_beneath_attr->allowed_access =
>>     > +                     LANDLOCK_ACCESS_FS_READ_FILE |
>>     > +                     LANDLOCK_ACCESS_FS_EXECUTE;
>>     > +             path_beneath_attr->parent_fd = SAFE_OPEN(path,
>>     O_PATH | O_CLOEXEC);
>>     > +
>>     > +             SAFE_LANDLOCK_ADD_RULE(
>>     > +                     ruleset_fd,
>>     > +                     LANDLOCK_RULE_PATH_BENEATH,
>>     > +                     path_beneath_attr,
>>     > +                     0);
>>
>>     Unfortunately, on 6.6.15-amd64 kernel (random Debian machine) it
>>     fails (after
>>     fresh boot) with:
>>
>>     ...
>>     tst_supported_fs_types.c:97: TINFO: Kernel supports tmpfs
>>     tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
>>     tst_test.c:1746: TINFO: === Testing on ext2 ===
>>     tst_test.c:1111: TINFO: Formatting /dev/loop1 with ext2 opts=''
>>     extra opts=''
>>     mke2fs 1.47.0 (5-Feb-2023)
>>     tst_test.c:1123: TINFO: Mounting /dev/loop1 to
>>     /tmp/LTP_lant6WbKJ/sandbox fstyp=ext2 flags=0
>>     landlock_common.h:30: TINFO: Landlock ABI v3
>>     landlock04.c:151: TINFO: Testing LANDLOCK_ACCESS_FS_EXECUTE
>>     landlock04.c:123: TINFO: Enable read/exec permissions for
>>     /usr/lib/i386-linux-gnu/libc.so.6
>>     landlock04.c:131: TBROK: landlock_add_rule(3, 1, 0xf7f13ff4, 0):
>>     EINVAL (22)
>>
>>
>> Possibly that's because the 'LANDLOCK_RULE_PATH_BENEATH'  was
>> refactored from the v6.7 mainline kernel, so it can't add the rule 
>> correctly
>> with older kernels.
>>
>> commit 0e0fc7e8eb4a11bd9f89a9c74bc7c0e144c56203
>> Author: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> Date:   Thu Oct 26 09:47:46 2023 +0800
>>
>>     landlock: Refactor landlock_add_rule() syscall
>>
>> But this is my guess (through reading the code), I didn't do more to
>> verify that by installing such a kernel.
>>
>>
>> -- 
>> Regards,
>> Li Wang
>
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp