Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits

[Mickaël Salaün]

On Mon, Jul 08, 2024 at 10:53:11AM -0700, Jeff Xu wrote:
> On Mon, Jul 8, 2024 at 9:17 AM Jeff Xu <jeffxu@xxxxxxxxxx> wrote:
> >
> > Hi
> >
> > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@xxxxxxxxxxx> wrote:
> > >
> > > 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
> 
> 
> 
> 
> > > 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@xxxxxxxxxxxxxxxxxx>
> > > Cc: Christian Brauner <brauner@xxxxxxxxxx>
> > > Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> > > Cc: Paul Moore <paul@xxxxxxxxxxxxxx>
> > > Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx>
> > > Link: https://lore.kernel.org/r/20240704190137.696169-3-mic@xxxxxxxxxxx
> > > ---