[RFC 2/4] landlock: Add LANDLOCK_ACCESS_FS_IOCTL access right

Mickaël Salaün mic at digikod.net
Mon Jun 19 14:42:07 UTC 2023


On 02/05/2023 19:17, Günther Noack wrote:
> Like the truncate right, this right is associated with a file
> descriptor at the time of open(2), and gets respected even when the
> file descriptor is used outside of the thread which it was originally
> created in.
> 
> In particular, this happens for the commonly inherited file
> descriptors stdin, stdout and stderr, if these are bound to a tty.

s/tty/TTY/

> This means that programs using tty ioctls can drop the ioctl access

s/ioctl/IOCTLs/

> right, but continue using these ioctls on the already opened input and
> output file descriptors.

I'd like a new documentation paragraph explaining the limitation of 
LANDLOCK_ACCESS_FS_IOCTL (not fine-grained; should be careful about 
fscrypt-like features for regular files; compatibility with TTY and 
other common IOCTLs), a way to get more guarantees (e.g. using nodev 
mount points when possible), and a sentence explaining that future work 
will enable a more fine-grained access control.


> 
> Signed-off-by: Günther Noack <gnoack3000 at gmail.com>
> ---
>   include/uapi/linux/landlock.h              | 19 ++++++++++++-------
>   security/landlock/fs.c                     | 20 ++++++++++++++++++--
>   security/landlock/limits.h                 |  2 +-
>   tools/testing/selftests/landlock/fs_test.c |  5 +++--
>   4 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f3223f96469..d87457a1c22 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -102,12 +102,16 @@ struct landlock_path_beneath_attr {
>    * - %LANDLOCK_ACCESS_FS_READ_FILE: Open a file with read access.
>    * - %LANDLOCK_ACCESS_FS_TRUNCATE: Truncate a file with :manpage:`truncate(2)`,
>    *   :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with
> - *   ``O_TRUNC``. Whether an opened file can be truncated with
> - *   :manpage:`ftruncate(2)` is determined during :manpage:`open(2)`, in the
> - *   same way as read and write permissions are checked during
> - *   :manpage:`open(2)` using %LANDLOCK_ACCESS_FS_READ_FILE and
> - *   %LANDLOCK_ACCESS_FS_WRITE_FILE. This access right is available since the
> - *   third version of the Landlock ABI.
> + *   ``O_TRUNC``.  This access right is available since the third version of the
> + *   Landlock ABI.
> + * - %LANDLOCK_ACCESS_FS_IOCTL: Invoke :manpage:`ioctl(2)` on the opened file.
> + *   This access right is available since the fourth version of the Landlock
> + *   ABI.
> + *
> + * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
> + * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
> + * read and write permissions are checked during :manpage:`open(2)` using
> + * %LANDLOCK_ACCESS_FS_READ_FILE and %LANDLOCK_ACCESS_FS_WRITE_FILE.
>    *
>    * A directory can receive access rights related to files or directories.  The
>    * following access right is applied to the directory itself, and the
> @@ -152,7 +156,7 @@ struct landlock_path_beneath_attr {
>    *   accessible through these syscall families: :manpage:`chdir(2)`,
>    *   :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`,
>    *   :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`,
> - *   :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`.
> + *   :manpage:`fcntl(2)`, :manpage:`access(2)`.
>    *   Future Landlock evolutions will enable to restrict them.
>    */
>   /* clang-format off */
> @@ -171,6 +175,7 @@ struct landlock_path_beneath_attr {
>   #define LANDLOCK_ACCESS_FS_MAKE_SYM			(1ULL << 12)
>   #define LANDLOCK_ACCESS_FS_REFER			(1ULL << 13)
>   #define LANDLOCK_ACCESS_FS_TRUNCATE			(1ULL << 14)
> +#define LANDLOCK_ACCESS_FS_IOCTL			(1ULL << 15)
>   /* clang-format on */
>   
>   #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index adcea0fe7e6..b13c765733c 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -147,7 +147,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
>   	LANDLOCK_ACCESS_FS_EXECUTE | \
>   	LANDLOCK_ACCESS_FS_WRITE_FILE | \
>   	LANDLOCK_ACCESS_FS_READ_FILE | \
> -	LANDLOCK_ACCESS_FS_TRUNCATE)
> +	LANDLOCK_ACCESS_FS_TRUNCATE | \
> +	LANDLOCK_ACCESS_FS_IOCTL)
>   /* clang-format on */
>   
>   /*
> @@ -1207,7 +1208,8 @@ static int hook_file_open(struct file *const file)
>   {
>   	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
>   	access_mask_t open_access_request, full_access_request, allowed_access;
> -	const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE;
> +	const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE |
> +					      LANDLOCK_ACCESS_FS_IOCTL;
>   	const struct landlock_ruleset *const dom =
>   		landlock_get_current_domain();
>   
> @@ -1280,6 +1282,19 @@ static int hook_file_truncate(struct file *const file)
>   	return -EACCES;
>   }
>   
> +static int hook_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	/*
> +	 * It is the access rights at the time of opening the file which
> +	 * determine whether ioctl can be used on the opened file later.
> +	 *
> +	 * The access right is attached to the opened file in hook_file_open().
> +	 */
> +	if (landlock_file(file)->allowed_access & LANDLOCK_ACCESS_FS_IOCTL)
> +		return 0;

Please add a new line here.

> +	return -EACCES;
> +}
> +
>   static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
>   	LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
>   
> @@ -1302,6 +1317,7 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
>   	LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
>   	LSM_HOOK_INIT(file_open, hook_file_open),
>   	LSM_HOOK_INIT(file_truncate, hook_file_truncate),
> +	LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
>   };
>   
>   __init void landlock_add_fs_hooks(void)
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 82288f0e9e5..40d8f17698b 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -18,7 +18,7 @@
>   #define LANDLOCK_MAX_NUM_LAYERS		16
>   #define LANDLOCK_MAX_NUM_RULES		U32_MAX
>   
> -#define LANDLOCK_LAST_ACCESS_FS		LANDLOCK_ACCESS_FS_TRUNCATE
> +#define LANDLOCK_LAST_ACCESS_FS		LANDLOCK_ACCESS_FS_IOCTL
>   #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
>   #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>   
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index b6c4be3faf7..fdd7d439ce4 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -446,9 +446,10 @@ TEST_F_FORK(layout1, inval)
>   	LANDLOCK_ACCESS_FS_EXECUTE | \
>   	LANDLOCK_ACCESS_FS_WRITE_FILE | \
>   	LANDLOCK_ACCESS_FS_READ_FILE | \
> -	LANDLOCK_ACCESS_FS_TRUNCATE)
> +	LANDLOCK_ACCESS_FS_TRUNCATE | \
> +	LANDLOCK_ACCESS_FS_IOCTL)
>   
> -#define ACCESS_LAST LANDLOCK_ACCESS_FS_TRUNCATE
> +#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL
>   
>   #define ACCESS_ALL ( \
>   	ACCESS_FILE | \



More information about the Linux-security-module-archive mailing list