All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/9] Landlock: IOCTL support
@ 2023-11-24 17:30 Günther Noack
  2023-11-24 17:30 ` [PATCH v6 1/9] landlock: Remove remaining "inline" modifiers in .c files Günther Noack
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Günther Noack @ 2023-11-24 17:30 UTC (permalink / raw)
  To: linux-security-module, Mickaël Salaün
  Cc: Jeff Xu, Jorge Lucangeli Obes, Allen Webb, Dmitry Torokhov,
	Paul Moore, Konstantin Meskhidze, Matt Bobrowski, linux-fsdevel,
	Günther Noack

Hello!

These patches add simple ioctl(2) support to Landlock.

Objective
~~~~~~~~~

Make ioctl(2) requests restrictable with Landlock,
in a way that is useful for real-world applications.

Proposed approach
~~~~~~~~~~~~~~~~~

Introduce the LANDLOCK_ACCESS_FS_IOCTL right, which restricts the use
of ioctl(2) on file descriptors.

We attach IOCTL access rights to opened file descriptors, as we
already do for LANDLOCK_ACCESS_FS_TRUNCATE.

If LANDLOCK_ACCESS_FS_IOCTL is handled (restricted in the ruleset),
the LANDLOCK_ACCESS_FS_IOCTL access right governs the use of all IOCTL
commands.

We make an exception for the common and known-harmless IOCTL commands
FIOCLEX, FIONCLEX, FIONBIO and FIONREAD.  These IOCTL commands are
always permitted.  Their functionality is already available through
fcntl(2).

If additionally(!), the access rights LANDLOCK_ACCESS_FS_READ_FILE,
LANDLOCK_ACCESS_FS_WRITE_FILE or LANDLOCK_ACCESS_FS_READ_DIR are
handled, these access rights also unlock some IOCTL commands which are
considered safe for use with files opened in these ways.

As soon as these access rights are handled, the affected IOCTL
commands can not be permitted through LANDLOCK_ACCESS_FS_IOCTL any
more, but only be permitted through the respective more specific
access rights.  A full list of these access rights is listed below in
this cover letter and in the documentation.

I believe that this approach works for the majority of use cases, and
offers a good trade-off between Landlock API and implementation
complexity and flexibility when the feature is used.

Current limitations
~~~~~~~~~~~~~~~~~~~

With this patch set, ioctl(2) requests can *not* be filtered based on
file type, device number (dev_t) or on the ioctl(2) request number.

On the initial RFC patch set [1], we have reached consensus to start
with this simpler coarse-grained approach, and build additional IOCTL
restriction capabilities on top in subsequent steps.

[1] https://lore.kernel.org/linux-security-module/d4f1395c-d2d4-1860-3a02-2a0c023dd761@digikod.net/

Notable implications of this approach
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* Existing inherited file descriptors stay unaffected
  when a program enables Landlock.

  This means in particular that in common scenarios,
  the terminal's IOCTLs (ioctl_tty(2)) continue to work.

* ioctl(2) continues to be available for file descriptors acquired
  through means other than open(2).  Example: Network sockets,
  memfd_create(2), file descriptors that are already open before the
  Landlock ruleset is enabled.

Examples
~~~~~~~~

Starting a sandboxed shell from $HOME with samples/landlock/sandboxer:

  LL_FS_RO=/ LL_FS_RW=. ./sandboxer /bin/bash

The LANDLOCK_ACCESS_FS_IOCTL right is part of the "read-write" rights
here, so we expect that newly opened files outside of $HOME don't work
with most IOCTL commands.

  * "stty" works: It probes terminal properties

  * "stty </dev/tty" fails: /dev/tty can be reopened, but the IOCTL is
    denied.

  * "eject" fails: ioctls to use CD-ROM drive are denied.

  * "ls /dev" works: It uses ioctl to get the terminal size for
    columnar layout

  * The text editors "vim" and "mg" work.  (GNU Emacs fails because it
    attempts to reopen /dev/tty.)

IOCTL groups
~~~~~~~~~~~~

To decide which IOCTL commands should be blanket-permitted we went
through the list of IOCTL commands mentioned in fs/ioctl.c and looked
at them individually to understand what they are about.  The following
list is for reference.

We should always allow the following IOCTL commands, which are also
available through fcntl(2) with the F_SETFD and F_SETFL commands:

 * FIOCLEX, FIONCLEX - these work on the file descriptor and
   manipulate the close-on-exec flag
 * FIONBIO, FIOASYNC - these work on the struct file and enable
   nonblocking-IO and async flags

The following command is guarded and enabled by either of
LANDLOCK_ACCESS_FS_WRITE_FILE, LANDLOCK_ACCESS_FS_READ_FILE or
LANDLOCK_ACCESS_FS_READ_DIR (G2), once one of them is handled
(otherwise by LANDLOCK_ACCESS_FS_IOCTL):

 * FIOQSIZE - get the size of the opened file

The following commands are guarded and enabled by either of
LANDLOCK_ACCESS_FS_WRITE_FILE or LANDLOCK_ACCESS_FS_READ_FILE (G2),
once one of them is handled (otherwise by LANDLOCK_ACCESS_FS_IOCTL):

These are commands that read file system internals:

 * FS_IOC_FIEMAP - get information about file extent mapping
   (c.f. https://www.kernel.org/doc/Documentation/filesystems/fiemap.txt)
 * FIBMAP - get a file's file system block number
 * FIGETBSZ - get file system blocksize

The following commands are guarded and enabled by
LANDLOCK_ACCESS_FS_READ_FILE (G3), if it is handled (otherwise by
LANDLOCK_ACCESS_FS_IOCTL):

 * FIONREAD - get the number of bytes available for reading (the
   implementation is defined per file type)
 * FIDEDUPRANGE - manipulating shared physical storage between files.

The following commands are guarded and enabled by
LANDLOCK_ACCESS_FS_WRITE_FILE (G4), if it is handled (otherwise by
LANDLOCK_ACCESS_FS_IOCTL):

 * FICLONE, FICLONERANGE - making files share physical storage between
   multiple files.  These only work on some file systems, by design.
 * FS_IOC_RESVSP, FS_IOC_RESVSP64, FS_IOC_UNRESVSP, FS_IOC_UNRESVSP64,
   FS_IOC_ZERO_RANGE: Backwards compatibility with legacy XFS
   preallocation syscalls which predate fallocate(2).

The following commands are also mentioned in fs/ioctl.c, but are not
handled specially and are managed by LANDLOCK_ACCESS_FS_IOCTL together
with all other remaining IOCTL commands:

 * FIFREEZE, FITHAW - work on superblock(!) to freeze/thaw the file
   system. Requires CAP_SYS_ADMIN.
 * Accessing file attributes:
   * FS_IOC_GETFLAGS, FS_IOC_SETFLAGS - manipulate inode flags (ioctl_iflags(2))
   * FS_IOC_FSGETXATTR, FS_IOC_FSSETXATTR - more attributes

Related Work
~~~~~~~~~~~~

OpenBSD's pledge(2) [2] restricts ioctl(2) independent of the file
descriptor which is used.  The implementers maintain multiple
allow-lists of predefined ioctl(2) operations required for different
application domains such as "audio", "bpf", "tty" and "inet".

OpenBSD does not guarantee ABI backwards compatibility to the same
extent as Linux does, so it's easier for them to update these lists in
later versions.  It might not be a feasible approach for Linux though.

[2] https://man.openbsd.org/OpenBSD-7.3/pledge.2

Changes
~~~~~~~

V6:
 * Implementation:
   * Check that only publicly visible access rights can be used when adding a
     rule (rather than the synthetic ones).  Thanks Mickaël for spotting that!
   * Move all functionality related to IOCTL groups and synthetic access rights
     into the same place at the top of fs.c
   * Move kernel doc to the .c file in one instance
   * Smaller code style issues (upcase IOCTL, vardecl at block start)
   * Remove inline modifier from functions in .c files
 * Tests:
   * use SKIP
   * Rename 'fd' to dir_fd and file_fd where appropriate
   * Remove duplicate "ioctl" mentions from test names
   * Rename "permitted" to "allowed", in ioctl and ftruncate tests
   * Do not add rules if access is 0, in test helper

V5:
 * Implementation:
   * move IOCTL group expansion logic into fs.c (implementation suggested by
     mic)
   * rename IOCTL_CMD_G* constants to LANDLOCK_ACCESS_FS_IOCTL_GROUP*
   * fs.c: create ioctl_groups constant
   * add "const" to some variables
 * Formatting and docstring fixes (including wrong kernel-doc format)
 * samples/landlock: fix ABI version and fallback attribute (mic)
 * Documentation
   * move header documentation changes into the implementation commit
   * spell out how FIFREEZE, FITHAW and attribute-manipulation ioctls from
     fs/ioctl.c are handled
   * change ABI 4 to ABI 5 in some missing places
   
V4:
 * use "synthetic" IOCTL access rights, as previously discussed
 * testing changes
   * use a large fixture-based test, for more exhaustive coverage,
     and replace some of the earlier tests with it
 * rebased on mic-next

V3:
 * always permit the IOCTL commands FIOCLEX, FIONCLEX, FIONBIO, FIOASYNC and
   FIONREAD, independent of LANDLOCK_ACCESS_FS_IOCTL
 * increment ABI version in the same commit where the feature is introduced
 * testing changes
   * use FIOQSIZE instead of TTY IOCTL commands
     (FIOQSIZE works with regular files, directories and memfds)
   * run the memfd test with both Landlock enabled and disabled
   * add a test for the always-permitted IOCTL commands

V2:
 * rebased on mic-next
 * added documentation
 * exercise ioctl(2) in the memfd test
 * test: Use layout0 for the test

---

V1: https://lore.kernel.org/linux-security-module/20230502171755.9788-1-gnoack3000@gmail.com/
V2: https://lore.kernel.org/linux-security-module/20230623144329.136541-1-gnoack@google.com/
V3: https://lore.kernel.org/linux-security-module/20230814172816.3907299-1-gnoack@google.com/
V4: https://lore.kernel.org/linux-security-module/20231103155717.78042-1-gnoack@google.com/
V5: https://lore.kernel.org/linux-security-module/20231117154920.1706371-1-gnoack@google.com/

Günther Noack (9):
  landlock: Remove remaining "inline" modifiers in .c files
  selftests/landlock: Rename "permitted" to "allowed" in ftruncate tests
  landlock: Optimize the number of calls to get_access_mask slightly
  landlock: Add IOCTL access right
  selftests/landlock: Test IOCTL support
  selftests/landlock: Test IOCTL with memfds
  selftests/landlock: Test ioctl(2) and ftruncate(2) with open(O_PATH)
  samples/landlock: Add support for LANDLOCK_ACCESS_FS_IOCTL
  landlock: Document IOCTL support

 Documentation/userspace-api/landlock.rst     |  74 ++-
 include/uapi/linux/landlock.h                |  58 +-
 samples/landlock/sandboxer.c                 |  13 +-
 security/landlock/fs.c                       | 198 ++++++-
 security/landlock/fs.h                       |   2 +
 security/landlock/limits.h                   |   5 +-
 security/landlock/ruleset.c                  |   7 +-
 security/landlock/ruleset.h                  |   2 +-
 security/landlock/syscalls.c                 |  19 +-
 tools/testing/selftests/landlock/base_test.c |   2 +-
 tools/testing/selftests/landlock/fs_test.c   | 523 ++++++++++++++++++-
 11 files changed, 829 insertions(+), 74 deletions(-)


base-commit: f12f8f84509a084399444c4422661345a15cc713
-- 
2.43.0.rc1.413.gea7ed67945-goog


^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] landlock: Add IOCTL access right
@ 2023-11-25  6:20 kernel test robot
  0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-11-25  6:20 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp

:::::: 
:::::: Manual check reason: "low confidence static check warning: security/landlock/fs.c:195:9: sparse: sparse: bad constant expression"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20231124173026.3257122-5-gnoack@google.com>
References: <20231124173026.3257122-5-gnoack@google.com>
TO: "Günther Noack" <gnoack@google.com>

Hi Günther,

kernel test robot noticed the following build warnings:

[auto build test WARNING on shuah-kselftest/next]
[also build test WARNING on shuah-kselftest/fixes linus/master v6.7-rc2]
[cannot apply to next-20231124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/G-nther-Noack/selftests-landlock-Rename-permitted-to-allowed-in-ftruncate-tests/20231125-023121
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
patch link:    https://lore.kernel.org/r/20231124173026.3257122-5-gnoack%40google.com
patch subject: [PATCH v6 4/9] landlock: Add IOCTL access right
:::::: branch date: 10 hours ago
:::::: commit date: 10 hours ago
config: i386-randconfig-062-20231125 (https://download.01.org/0day-ci/archive/20231125/202311251210.nZC1cSN9-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231125/202311251210.nZC1cSN9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202311251210.nZC1cSN9-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> security/landlock/fs.c:195:9: sparse: sparse: bad constant expression

vim +195 security/landlock/fs.c

64a4a71c8e60b9 Günther Noack 2023-11-24  181  
64a4a71c8e60b9 Günther Noack 2023-11-24  182  /**
64a4a71c8e60b9 Günther Noack 2023-11-24  183   * landlock_expand_access_fs() - Returns @access with the synthetic IOCTL group
64a4a71c8e60b9 Günther Noack 2023-11-24  184   * flags enabled if necessary.
64a4a71c8e60b9 Günther Noack 2023-11-24  185   *
64a4a71c8e60b9 Günther Noack 2023-11-24  186   * @handled: Handled FS access rights.
64a4a71c8e60b9 Günther Noack 2023-11-24  187   * @access: FS access rights to expand.
64a4a71c8e60b9 Günther Noack 2023-11-24  188   *
64a4a71c8e60b9 Günther Noack 2023-11-24  189   * Returns: @access expanded by the necessary flags for the synthetic IOCTL
64a4a71c8e60b9 Günther Noack 2023-11-24  190   * access rights.
64a4a71c8e60b9 Günther Noack 2023-11-24  191   */
64a4a71c8e60b9 Günther Noack 2023-11-24  192  static access_mask_t landlock_expand_access_fs(const access_mask_t handled,
64a4a71c8e60b9 Günther Noack 2023-11-24  193  					       const access_mask_t access)
64a4a71c8e60b9 Günther Noack 2023-11-24  194  {
64a4a71c8e60b9 Günther Noack 2023-11-24 @195  	static_assert((ioctl_groups & LANDLOCK_MASK_ACCESS_FS) == ioctl_groups);
64a4a71c8e60b9 Günther Noack 2023-11-24  196  
64a4a71c8e60b9 Günther Noack 2023-11-24  197  	return access |
64a4a71c8e60b9 Günther Noack 2023-11-24  198  	       expand_ioctl(handled, access, LANDLOCK_ACCESS_FS_WRITE_FILE,
64a4a71c8e60b9 Günther Noack 2023-11-24  199  			    LANDLOCK_ACCESS_FS_IOCTL_GROUP1 |
64a4a71c8e60b9 Günther Noack 2023-11-24  200  				    LANDLOCK_ACCESS_FS_IOCTL_GROUP2 |
64a4a71c8e60b9 Günther Noack 2023-11-24  201  				    LANDLOCK_ACCESS_FS_IOCTL_GROUP4) |
64a4a71c8e60b9 Günther Noack 2023-11-24  202  	       expand_ioctl(handled, access, LANDLOCK_ACCESS_FS_READ_FILE,
64a4a71c8e60b9 Günther Noack 2023-11-24  203  			    LANDLOCK_ACCESS_FS_IOCTL_GROUP1 |
64a4a71c8e60b9 Günther Noack 2023-11-24  204  				    LANDLOCK_ACCESS_FS_IOCTL_GROUP2 |
64a4a71c8e60b9 Günther Noack 2023-11-24  205  				    LANDLOCK_ACCESS_FS_IOCTL_GROUP3) |
64a4a71c8e60b9 Günther Noack 2023-11-24  206  	       expand_ioctl(handled, access, LANDLOCK_ACCESS_FS_READ_DIR,
64a4a71c8e60b9 Günther Noack 2023-11-24  207  			    LANDLOCK_ACCESS_FS_IOCTL_GROUP1);
64a4a71c8e60b9 Günther Noack 2023-11-24  208  }
64a4a71c8e60b9 Günther Noack 2023-11-24  209  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-12-01 14:06 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-24 17:30 [PATCH v6 0/9] Landlock: IOCTL support Günther Noack
2023-11-24 17:30 ` [PATCH v6 1/9] landlock: Remove remaining "inline" modifiers in .c files Günther Noack
2023-11-30  9:27   ` Mickaël Salaün
2023-12-01 14:05     ` Günther Noack
2023-11-24 17:30 ` [PATCH v6 2/9] selftests/landlock: Rename "permitted" to "allowed" in ftruncate tests Günther Noack
2023-11-24 17:30 ` [PATCH v6 3/9] landlock: Optimize the number of calls to get_access_mask slightly Günther Noack
2023-11-24 17:30 ` [PATCH v6 4/9] landlock: Add IOCTL access right Günther Noack
2023-11-26  7:23   ` kernel test robot
2023-11-30  9:28   ` Mickaël Salaün
2023-12-01 14:04     ` Günther Noack
2023-11-24 17:30 ` [PATCH v6 5/9] selftests/landlock: Test IOCTL support Günther Noack
2023-11-30  9:30   ` Mickaël Salaün
2023-11-24 17:30 ` [PATCH v6 6/9] selftests/landlock: Test IOCTL with memfds Günther Noack
2023-11-24 17:30 ` [PATCH v6 7/9] selftests/landlock: Test ioctl(2) and ftruncate(2) with open(O_PATH) Günther Noack
2023-11-24 17:30 ` [PATCH v6 8/9] samples/landlock: Add support for LANDLOCK_ACCESS_FS_IOCTL Günther Noack
2023-11-24 17:30 ` [PATCH v6 9/9] landlock: Document IOCTL support Günther Noack
  -- strict thread matches above, loose matches on Subject: below --
2023-11-25  6:20 [PATCH v6 4/9] landlock: Add IOCTL access right kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.