[PATCH v2] landlock: Explain file descriptor access rights

[PATCH v2] landlock: Explain file descriptor access rights

[Mickaël Salaün]

Starting with LANDLOCK_ACCESS_FS_TRUNCATE, it is worth explaining why we
choose to restrict access checks at open time.  This new "File
descriptor access rights" section is complementary to the existing
"Inode access rights" section.  Add a new guiding principle related to
this section.

Cc: Günther Noack <gnoack3000@gmail.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20221209193813.972012-1-mic@digikod.net
---

Changes since v1:
https://lore.kernel.org/r/20221205112621.3530557-1-mic@digikod.net
* Reworded the new section based on Günther suggestions.
* Added a new guiding principle.
* Update date.
---
 Documentation/security/landlock.rst | 33 ++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
index c0029d5d02eb..95a0e4726dc5 100644
--- a/Documentation/security/landlock.rst
+++ b/Documentation/security/landlock.rst
@@ -7,7 +7,7 @@ Landlock LSM: kernel documentation
 ==================================
 
 :Author: Mickaël Salaün
-:Date: September 2022
+:Date: December 2022
 
 Landlock's goal is to create scoped access-control (i.e. sandboxing).  To
 harden a whole system, this feature should be available to any process,
@@ -41,12 +41,15 @@ Guiding principles for safe access controls
   processes.
 * Computation related to Landlock operations (e.g. enforcing a ruleset) shall
   only impact the processes requesting them.
+* Resources (e.g. file descriptors) directly obtained from the kernel by a
+  sandboxed process shall retain their scoped accesses whatever process use
+  them.  Cf. `File descriptor access rights`_.
 
 Design choices
 ==============
 
-Filesystem access rights
-------------------------
+Inode access rights
+-------------------
 
 All access rights are tied to an inode and what can be accessed through it.
 Reading the content of a directory does not imply to be allowed to read the
@@ -57,6 +60,30 @@ directory, not the unlinked inode.  This is the reason why
 ``LANDLOCK_ACCESS_FS_REMOVE_FILE`` or ``LANDLOCK_ACCESS_FS_REFER`` are not
 allowed to be tied to files but only to directories.
 
+File descriptor access rights
+-----------------------------
+
+Access rights are checked and tied to file descriptors at open time.  The
+underlying principle is that equivalent sequences of operations should lead to
+the same results, when they are executed under the same Landlock domain.
+
+Taking the ``LANDLOCK_ACCESS_FS_TRUNCATE`` right as an example, it may be
+allowed to open a file for writing without being allowed to
+:manpage:`ftruncate` the resulting file descriptor if the related file
+hierarchy doesn't grant such access right.  The following sequences of
+operations have the same semantic and should then have the same result:
+
+* ``truncate(path);``
+* ``int fd = open(path, O_WRONLY); ftruncate(fd); close(fd);``
+
+Similarly to file access modes (e.g. ``O_RDWR``), Landlock access rights
+attached to file descriptors are retained even if they are passed between
+processes (e.g. through a Unix domain socket).  Such access rights will then be
+enforced even if the receiving process is not sandboxed by Landlock.  Indeed,
+this is required to keep a consistent access control over the whole system, and
+avoid unattended bypasses through file descriptor passing (i.e. confused deputy
+attack).
+
 Tests
 =====
 

base-commit: 0b4ab8cd635e8b21e42c14b9e4810ca701babd11
-- 
2.38.1

Re: [PATCH v2] landlock: Explain file descriptor access rights

[Günther Noack]

On Fri, Dec 09, 2022 at 08:38:13PM +0100, Mickaël Salaün wrote:
> Starting with LANDLOCK_ACCESS_FS_TRUNCATE, it is worth explaining why we
> choose to restrict access checks at open time.  This new "File
> descriptor access rights" section is complementary to the existing
> "Inode access rights" section.  Add a new guiding principle related to
> this section.
> 
> Cc: Günther Noack <gnoack3000@gmail.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20221209193813.972012-1-mic@digikod.net
> ---
> 
> Changes since v1:
> https://lore.kernel.org/r/20221205112621.3530557-1-mic@digikod.net
> * Reworded the new section based on Günther suggestions.
> * Added a new guiding principle.
> * Update date.
> ---
>  Documentation/security/landlock.rst | 33 ++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
> index c0029d5d02eb..95a0e4726dc5 100644
> --- a/Documentation/security/landlock.rst
> +++ b/Documentation/security/landlock.rst
> @@ -7,7 +7,7 @@ Landlock LSM: kernel documentation
>  ==================================
>  
>  :Author: Mickaël Salaün
> -:Date: September 2022
> +:Date: December 2022
>  
>  Landlock's goal is to create scoped access-control (i.e. sandboxing).  To
>  harden a whole system, this feature should be available to any process,
> @@ -41,12 +41,15 @@ Guiding principles for safe access controls
>    processes.
>  * Computation related to Landlock operations (e.g. enforcing a ruleset) shall
>    only impact the processes requesting them.
> +* Resources (e.g. file descriptors) directly obtained from the kernel by a
> +  sandboxed process shall retain their scoped accesses whatever process use

Optional nit: Maybe add "at the time of resource acquisition" to stress that part?

> +  them.  Cf. `File descriptor access rights`_.
>  
>  Design choices
>  ==============
>  
> -Filesystem access rights
> -------------------------
> +Inode access rights
> +-------------------
>  
>  All access rights are tied to an inode and what can be accessed through it.
>  Reading the content of a directory does not imply to be allowed to read the
> @@ -57,6 +60,30 @@ directory, not the unlinked inode.  This is the reason why
>  ``LANDLOCK_ACCESS_FS_REMOVE_FILE`` or ``LANDLOCK_ACCESS_FS_REFER`` are not
>  allowed to be tied to files but only to directories.
>  
> +File descriptor access rights
> +-----------------------------
> +
> +Access rights are checked and tied to file descriptors at open time.  The
> +underlying principle is that equivalent sequences of operations should lead to
> +the same results, when they are executed under the same Landlock domain.
> +
> +Taking the ``LANDLOCK_ACCESS_FS_TRUNCATE`` right as an example, it may be
> +allowed to open a file for writing without being allowed to
> +:manpage:`ftruncate` the resulting file descriptor if the related file
> +hierarchy doesn't grant such access right.  The following sequences of
> +operations have the same semantic and should then have the same result:
> +
> +* ``truncate(path);``
> +* ``int fd = open(path, O_WRONLY); ftruncate(fd); close(fd);``
> +
> +Similarly to file access modes (e.g. ``O_RDWR``), Landlock access rights
> +attached to file descriptors are retained even if they are passed between
> +processes (e.g. through a Unix domain socket).  Such access rights will then be
> +enforced even if the receiving process is not sandboxed by Landlock.  Indeed,
> +this is required to keep a consistent access control over the whole system, and
> +avoid unattended bypasses through file descriptor passing (i.e. confused deputy
> +attack).
> +
>  Tests
>  =====
>  
> 
> base-commit: 0b4ab8cd635e8b21e42c14b9e4810ca701babd11
> -- 
> 2.38.1
> 

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

Thank you!

-- 

Re: [PATCH v2] landlock: Explain file descriptor access rights

[Mickaël Salaün]

On 12/12/2022 08:39, Günther Noack wrote:
> On Fri, Dec 09, 2022 at 08:38:13PM +0100, Mickaël Salaün wrote:
>> Starting with LANDLOCK_ACCESS_FS_TRUNCATE, it is worth explaining why we
>> choose to restrict access checks at open time.  This new "File
>> descriptor access rights" section is complementary to the existing
>> "Inode access rights" section.  Add a new guiding principle related to
>> this section.
>>
>> Cc: Günther Noack <gnoack3000@gmail.com>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Link: https://lore.kernel.org/r/20221209193813.972012-1-mic@digikod.net
>> ---
>>
>> Changes since v1:
>> https://lore.kernel.org/r/20221205112621.3530557-1-mic@digikod.net
>> * Reworded the new section based on Günther suggestions.
>> * Added a new guiding principle.
>> * Update date.
>> ---
>>   Documentation/security/landlock.rst | 33 ++++++++++++++++++++++++++---
>>   1 file changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
>> index c0029d5d02eb..95a0e4726dc5 100644
>> --- a/Documentation/security/landlock.rst
>> +++ b/Documentation/security/landlock.rst
>> @@ -7,7 +7,7 @@ Landlock LSM: kernel documentation
>>   ==================================
>>   
>>   :Author: Mickaël Salaün
>> -:Date: September 2022
>> +:Date: December 2022
>>   
>>   Landlock's goal is to create scoped access-control (i.e. sandboxing).  To
>>   harden a whole system, this feature should be available to any process,
>> @@ -41,12 +41,15 @@ Guiding principles for safe access controls
>>     processes.
>>   * Computation related to Landlock operations (e.g. enforcing a ruleset) shall
>>     only impact the processes requesting them.
>> +* Resources (e.g. file descriptors) directly obtained from the kernel by a
>> +  sandboxed process shall retain their scoped accesses whatever process use
> 
> Optional nit: Maybe add "at the time of resource acquisition" to stress that part?

I included this suggestion in -next: 
https://git.kernel.org/mic/c/4dd6da345ac2

Thanks!

> 
>> +  them.  Cf. `File descriptor access rights`_.
>>   
>>   Design choices
>>   ==============
>>   
>> -Filesystem access rights
>> -------------------------
>> +Inode access rights
>> +-------------------
>>   
>>   All access rights are tied to an inode and what can be accessed through it.
>>   Reading the content of a directory does not imply to be allowed to read the
>> @@ -57,6 +60,30 @@ directory, not the unlinked inode.  This is the reason why
>>   ``LANDLOCK_ACCESS_FS_REMOVE_FILE`` or ``LANDLOCK_ACCESS_FS_REFER`` are not
>>   allowed to be tied to files but only to directories.
>>   
>> +File descriptor access rights
>> +-----------------------------
>> +
>> +Access rights are checked and tied to file descriptors at open time.  The
>> +underlying principle is that equivalent sequences of operations should lead to
>> +the same results, when they are executed under the same Landlock domain.
>> +
>> +Taking the ``LANDLOCK_ACCESS_FS_TRUNCATE`` right as an example, it may be
>> +allowed to open a file for writing without being allowed to
>> +:manpage:`ftruncate` the resulting file descriptor if the related file
>> +hierarchy doesn't grant such access right.  The following sequences of
>> +operations have the same semantic and should then have the same result:
>> +
>> +* ``truncate(path);``
>> +* ``int fd = open(path, O_WRONLY); ftruncate(fd); close(fd);``
>> +
>> +Similarly to file access modes (e.g. ``O_RDWR``), Landlock access rights
>> +attached to file descriptors are retained even if they are passed between
>> +processes (e.g. through a Unix domain socket).  Such access rights will then be
>> +enforced even if the receiving process is not sandboxed by Landlock.  Indeed,
>> +this is required to keep a consistent access control over the whole system, and
>> +avoid unattended bypasses through file descriptor passing (i.e. confused deputy
>> +attack).
>> +
>>   Tests
>>   =====
>>   
>>
>> base-commit: 0b4ab8cd635e8b21e42c14b9e4810ca701babd11
>> -- 
>> 2.38.1
>>
> 
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
> 
> Thank you!
>