On Mon, Jul 08, 2024 at 10:53:11AM -0700, Jeff Xu wrote:

On Mon, Jul 8, 2024 at 9:17 AM Jeff Xu [off-list ref] wrote:

quoted

Hi

On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün [off-list ref] wrote:

quoted

These new SECBIT_SHOULD_EXEC_CHECK, SECBIT_SHOULD_EXEC_RESTRICT, and
their *_LOCKED counterparts are designed to be set by processes setting
up an execution environment, such as a user session, a container, or a
security sandbox.  Like seccomp filters or Landlock domains, the
securebits are inherited across proceses.

When SECBIT_SHOULD_EXEC_CHECK is set, programs interpreting code should
check executable resources with execveat(2) + AT_CHECK (see previous
patch).

When SECBIT_SHOULD_EXEC_RESTRICT is set, a process should only allow
execution of approved resources, if any (see SECBIT_SHOULD_EXEC_CHECK).

Do we need both bits ?
When CHECK is set and RESTRICT is not, the "check fail" executable
will still get executed, so CHECK is for logging ?
Does RESTRICT imply CHECK is set, e.g. What if CHECK=0 and RESTRICT = 1 ?

The intention might be "permissive mode"?  if so, consider reuse
existing selinux's concept, and still with 2 bits:
SECBIT_SHOULD_EXEC_RESTRICT
SECBIT_SHOULD_EXEC_RESTRICT_PERMISSIVE

SECBIT_SHOULD_EXEC_CHECK is for user space to check with execveat+AT_CHECK.

SECBIT_SHOULD_EXEC_RESTRICT is for user space to restrict execution by
default, and potentially allow some exceptions from the list of
checked-and-allowed files, if SECBIT_SHOULD_EXEC_CHECK is set.

Without SECBIT_SHOULD_EXEC_CHECK, SECBIT_SHOULD_EXEC_RESTRICT is to deny
any kind of execution/interpretation.

With only SECBIT_SHOULD_EXEC_CHECK, user space should just check and log
any denied access, but ignore them.  So yes, it is similar to the
SELinux's permissive mode.

This is explained in the next patch as comments.

The *_LOCKED variants are useful and part of the securebits concept.

-Jeff

quoted

quoted

For a secure environment, we might also want
SECBIT_SHOULD_EXEC_CHECK_LOCKED and SECBIT_SHOULD_EXEC_RESTRICT_LOCKED
to be set.  For a test environment (e.g. testing on a fleet to identify
potential issues), only the SECBIT_SHOULD_EXEC_CHECK* bits can be set to
still be able to identify potential issues (e.g. with interpreters logs
or LSMs audit entries).

It should be noted that unlike other security bits, the
SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT bits are
dedicated to user space willing to restrict itself.  Because of that,
they only make sense in the context of a trusted environment (e.g.
sandbox, container, user session, full system) where the process
changing its behavior (according to these bits) and all its parent
processes are trusted.  Otherwise, any parent process could just execute
its own malicious code (interpreting a script or not), or even enforce a
seccomp filter to mask these bits.

Such a secure environment can be achieved with an appropriate access
control policy (e.g. mount's noexec option, file access rights, LSM
configuration) and an enlighten ld.so checking that libraries are
allowed for execution e.g., to protect against illegitimate use of
LD_PRELOAD.

Scripts may need some changes to deal with untrusted data (e.g. stdin,
environment variables), but that is outside the scope of the kernel.

The only restriction enforced by the kernel is the right to ptrace
another process.  Processes are denied to ptrace less restricted ones,
unless the tracer has CAP_SYS_PTRACE.  This is mainly a safeguard to
avoid trivial privilege escalations e.g., by a debugging process being
abused with a confused deputy attack.

Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <redacted>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20240704190137.696169-3-mic@digikod.net (local)
---