Message-ID: <48103715-6e22-10ab-6b4f-06946e00a28e@schaufler-ca.com>
Date:   Tue, 28 Dec 2021 17:26:20 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.1
Subject: Re: [RFC PATCH v1 0/2] Add capabilities file to sysfs
Content-Language: en-US
To:     Francis Laniel <flaniel@linux.microsoft.com>,
        linux-kernel@vger.kernel.org
Cc:     Serge Hallyn <serge@hallyn.com>,
        linux-security-module@vger.kernel.org,
        Casey Schaufler <casey@schaufler-ca.com>
References: <20211227205500.214777-1-flaniel@linux.microsoft.com>
 <7d1d7682-dd19-1b8a-ee5a-50fe6479589b@schaufler-ca.com>
 <2639798.l7ez0hRiAL@machine>
From:   Casey Schaufler <casey@schaufler-ca.com>
In-Reply-To: <2639798.l7ez0hRiAL@machine>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 12/28/2021 5:34 AM, Francis Laniel wrote:
> Hi.
>
> Le lundi 27 décembre 2021, 23:22:41 CET Casey Schaufler a écrit :
>> On 12/27/2021 12:54 PM, Francis Laniel wrote:
>>> Hi.
>>>
>>>
>>> First, I hope you are fine and the same for your relatives.
>>>
>>> Capabilities are used to check if a thread has the right to perform a
>>> given
>>> action [1].
>>> For example, a thread with CAP_BPF set can use the bpf() syscall.
>>>
>>> Capabilities are used in the container world.
>>> In terms of code, several projects related to container maintain code
>>> where the capabilities are written alike include/uapi/linux/capability.h
>>> [2][3][4][5]. For these projects, their codebase should be updated when a
>>> new capability is added to the kernel.
>>> Some other projects rely on <sys/capability.h> [6].
>>> In this case, this header file should reflect the capabilities offered by
>>> the kernel.
>>>
>>> So, in this series, I added a new file to sysfs: /sys/kernel/capabilities.
>> This should be /sys/kernel/security/capabilities.
> I began to write code to move this under /sys/kernel/security/capabilities but
> I realized this directory is linked to CONFIG_SECURITYFS.
> This option is not required to be able to run container [1].

You're going to need to handle the case where the file is missing
regardless. It is hard to design a kernel feature based on what a
container expects when there are so many definitions of a container.

> Also, kernel/capability.c is always compiled, so I think it is better if this
> file (i.e. the one which prints capabilities to user) does not depend on any
> CONFIG_.

CONFIG_MULTUSER is going to be an issue if you really care.

> What do you think of it? Does this sound acceptable for you?

Meh. I'm not going to get worked up over it, but your rationale
is a little weak.

>
>>> The goal of this file is to be used by "container world" software to know
>>> kernel capabilities at run time instead of compile time.
>>>
>>> The underlying kernel attribute is read-only and its content is the
>>> capability number associated with the capability name:
>>> root@vm-amd64:~# cat /sys/kernel/capabilities
>>> 0       CAP_CHOWN
>>> 1       CAP_DAC_OVERRIDE
>>> ...
>>> 39      CAP_BPF
>>>
>>> The kernel already exposes the last capability number under:
>>> /proc/sys/kernel/cap_last_cap
>>> So, I think there should not be any issue exposing all the capabilities it
>>> offers.
>>> If there is any, please share it as I do not want to introduce issue with
>>> this series.
>>>
>>> Also, if you see any way to improve this series please share it as it
>>> would
>>> increase this contribution quality.
>>>
>>> Francis Laniel (2):
>>>     capability: Add cap_strings.
>>>     kernel/ksysfs.c: Add capabilities attribute.
>>>    
>>>    include/uapi/linux/capability.h |  1 +
>>>    kernel/capability.c             | 45 +++++++++++++++++++++++++++++++++
>>>    kernel/ksysfs.c                 | 18 +++++++++++++
>>>    3 files changed, 64 insertions(+)
>>>
>>> Best regards and thank you in advance for your reviews.
>>> ---
>>> [1] man capabilities
>>> [2]
>>> https://github.com/containerd/containerd/blob/1a078e6893d07fec10a4940a566
>>> 4fab21d6f7d1e/pkg/cap/cap_linux.go#L135 [3]
>>> https://github.com/moby/moby/commit/485cf38d48e7111b3d1f584d5e9eab46a902a
>>> abc#diff-2e04625b209932e74c617de96682ed72fbd1bb0d0cb9fb7c709cf47a86b6f9c1
>>> moby relies on containerd code.
>>> [4]
>>> https://github.com/syndtr/gocapability/blob/42c35b4376354fd554efc7ad35e0b
>>> 7f94e3a0ffb/capability/enum.go#L47 [5]
>>> https://github.com/opencontainers/runc/blob/00f56786bb220b55b41748231880b
>>> a0e6380519a/libcontainer/capabilities/capabilities.go#L12 runc relies on
>>> syndtr package.
>>> [6]
>>> https://github.com/containers/crun/blob/fafb556f09e6ffd4690c452ff51856b88
>>> 0c089f1/src/libcrun/linux.c#L35
>
> Best regards.
> ---
> [1] https://github.com/moby/moby/blob/
> 10aecb0e652d346130a37e5b4383eca28f594c21/contrib/check-config.sh
>
>