netdev - Re: [RFC PATCH 2/2] landlock: selftests for bind and connect hooks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d09ac689-b1bf-86fa-4da5-3a0ade7fd552@digikod.net>
Date:   Tue, 8 Feb 2022 13:17:27 +0100
From:   Mickaël Salaün <mic@...ikod.net>
To:     Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
Cc:     linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
        netfilter@...r.kernel.org, yusongping@...wei.com,
        artem.kuzin@...wei.com
Subject: Re: [RFC PATCH 2/2] landlock: selftests for bind and connect hooks


On 08/02/2022 04:01, Konstantin Meskhidze wrote:
> 
> 
> 2/7/2022 3:49 PM, Mickaël Salaün пишет:

>> [...]
>>
>>>>> +    /* Create a socket 3 */
>>>>> +    sockfd_3 = socket(AF_INET, SOCK_STREAM, 0);
>>>>> +    ASSERT_LE(0, sockfd_3);
>>>>> +    /* Allow reuse of local addresses */
>>>>> +    ASSERT_EQ(0, setsockopt(sockfd_3, SOL_SOCKET, SO_REUSEADDR, 
>>>>> &one, sizeof(one)));
>>>>> +
>>>>> +    /* Set socket 3 address parameters */
>>>>> +    addr_3.sin_family = AF_INET;
>>>>> +    addr_3.sin_port = htons(SOCK_PORT_3);
>>>>> +    addr_3.sin_addr.s_addr = inet_addr(IP_ADDRESS);
>>>>> +    memset(&(addr_3.sin_zero), '\0', 8);
>>>>> +    /* Bind the socket 3 to IP address */
>>>>> +    ASSERT_EQ(0, bind(sockfd_3, (struct sockaddr *)&addr_3, 
>>>>> sizeof(addr_3)));
>>>>
>>>> Why is it allowed to bind to SOCK_PORT_3 whereas net_service_3 
>>>> forbids it?
>>>
>>>    It's allowed cause net_service_3 has empty access field.
>>>
>>>     /* Empty allowed_access (i.e. deny rules) are ignored in network
>>>      *  actions for SOCK_PORT_3 socket "object"
>>>      */
>>>      ASSERT_EQ(-1, landlock_add_rule(ruleset_fd,
>>>                                      LANDLOCK_RULE_NET_SERVICE,
>>>                                      &net_service_3, 0));
>>>      ASSERT_EQ(ENOMSG, errno);
>>>
>>>    Applying this rule returns ENOMSG errno:
>>>
>>>    /* Informs about useless rule: empty allowed_access (i.e. deny rules)
>>>     * are ignored in network actions
>>>     */
>>>          if (!net_service_attr.allowed_access) {
>>>              err = -ENOMSG;
>>>              goto out_put_ruleset;
>>>          }
>>>    This means binding socket 3 is not restricted.
>>>    For path_beneath_attr.allowed_access = 0 there is the same logic.
>>
>> I missed the ENOMSG check; the third rule has nothing to do with it. 
>> However, because the ruleset handles bind and connect actions, they 
>> must be denied by default. There is no rule allowing binding to 
>> SOCK_PORT_3. Why is it allowed?
>>
>> You can test with another SOCK_PORT_4, not covered by any rule. As for 
>> SOCK_PORT_3, it must be forbidden to bind on it.
> 
>    Apllying the third rule (net_service_3.access is empty) returns ENOMSG
>    error. That means a process hasn't been restricted by the third rule,
>    cause during search  process in network rb_tree the process won't find
>    the third rule, so binding to SOCK_PORT_3 is allowed.

Landlock is designed to deny every access rights that are handled (by a 
ruleset) by default. All rules added to a ruleset are exceptions to 
allow a subset of the handled access rights on a specific object/port.

With the current networking code, a sandboxed process can still bind or 
connect to any port except, in this test, partially for two ports. This 
approach doesn't help to isolate a process from the network.

> 
>    Maybe there is a misunderstanding here. You mean that if there is just
>    only one network rule for a particular port has been applied to a
>    process, other ports' networks actions are automatically restricted
>    until they will be added into landlock newtwork rb_tree?

Right! That is how it should be.