netdev - Re: [RFC PATCH v4 10/15] seltest/landlock: add tests for bind() 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: <0b0ddf78-12fa-ab52-ba3a-c819ed9d2ccd@digikod.net>
Date:   Fri, 8 Apr 2022 18:41:52 +0200
From:   Mickaël Salaün <mic@...ikod.net>
To:     Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
Cc:     willemdebruijn.kernel@...il.com,
        linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
        netfilter-devel@...r.kernel.org, yusongping@...wei.com,
        artem.kuzin@...wei.com, anton.sirazetdinov@...wei.com
Subject: Re: [RFC PATCH v4 10/15] seltest/landlock: add tests for bind() hooks


On 06/04/2022 16:12, Konstantin Meskhidze wrote:
> 
> 
> 4/4/2022 12:44 PM, Mickaël Salaün пишет:
>>
>> On 04/04/2022 10:28, Konstantin Meskhidze wrote:
>>>
>>>
>>> 4/1/2022 7:52 PM, Mickaël Salaün пишет:
>>
>> [...]
>>
>>>>> +static int create_socket(struct __test_metadata *const _metadata)
>>>>> +{
>>>>> +
>>>>> +        int sockfd;
>>>>> +
>>>>> +        sockfd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
>>>>> +        ASSERT_LE(0, sockfd);
>>>>> +        /* Allows to reuse of local address */
>>>>> +        ASSERT_EQ(0, setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, 
>>>>> &one, sizeof(one)));
>>>>
>>>> Why is it required?
>>>
>>>    Without SO_REUSEADDR there is an error that a socket's port is in 
>>> use.
>>
>> I'm sure there is, but why is this port reused? I think this means 
>> that there is an issue in the tests and that could hide potential 
>> issue with the tests (and then with the kernel code). Could you 
>> investigate and find the problem? This would make these tests reliable.
>    The next scenario is possible here:
>    "In order for a network connection to close, both ends have to send 
> FIN (final) packets, which indicate they will not send any additional 
> data, and both ends must ACK (acknowledge) each other's FIN packets. The 
> FIN packets are initiated by the application performing a close(), a 
> shutdown(), or an exit(). The ACKs are handled by the kernel after the 
> close() has completed. Because of this, it is possible for the process 
> to complete before the kernel has released the associated network 
> resource, and this port cannot be bound to another process until the 
> kernel has decided that it is done."
> https://hea-www.harvard.edu/~fine/Tech/addrinuse.html.
> 
> So in this case we have busy port in network selfttest and one of the 
> solution is to set SO_REUSEADDR socket option, "which explicitly allows 
> a process to bind to a port which remains in TIME_WAIT (it still only 
> allows a single process to be bound to that port). This is the both the 
> simplest and the most effective option for reducing the "address already 
> in use" error".

In know what this option does, but I'm wondering what do you need it for 
these tests: which specific line requires it and why? Isn't it a side 
effect of running partial tests? I'm worried that this hides some issues 
in the tests that may make them flaky.


>>
>> Without removing the need to find this issue, the next series should 
>> use a network namespace per test, which will confine such issue from 
>> other tests and the host.
> 
>    So there are 2 options here:
>      1. Using SO_REUSEADDR option
>      2. Using network namespace.
> 
> I prefer the first option - "the simplest and the most effective one"

If SO_REUSEADDR is really required (and justified), then it should be 
used. Either it is required or not, we should use a dedicated network 
namespace for each test anyway. This enables to not mess with the host 
and not be impacted by it neither (e.g. if some process already use such 
ports).


> 
>>
>> [...]