All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Paul Moore <paul@paul-moore.com>, Ondrej Mosnacek <omosnace@redhat.com>
Cc: selinux@vger.kernel.org,
	"Stephen Smalley" <stephen.smalley.work@gmail.com>,
	"Christian Göttsche" <cgzones@googlemail.com>
Subject: Re: [PATCH v3] selinux: introduce an initial SID for early boot processes
Date: Thu, 23 Nov 2023 10:59:39 +1100	[thread overview]
Message-ID: <87a5r5s5jo.fsf@mail.lhotse> (raw)
In-Reply-To: <CAHC9VhTWYeMzr99HPOp43Q_gVrQg6JSPNWH5y11irqZB89X6eg@mail.gmail.com>

Paul Moore <paul@paul-moore.com> writes:
> On Tue, Nov 14, 2023 at 10:51 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>
>> Currently, SELinux doesn't allow distinguishing between kernel threads
>> and userspace processes that are started before the policy is first
>> loaded - both get the label corresponding to the kernel SID. The only
>> way a process that persists from early boot can get a meaningful label
>> is by doing a voluntary dyntransition or re-executing itself.
>>
>> Reusing the kernel label for userspace processes is problematic for
>> several reasons:
>> 1. The kernel is considered to be a privileged domain and generally
>>    needs to have a wide range of permissions allowed to work correctly,
>>    which prevents the policy writer from effectively hardening against
>>    early boot processes that might remain running unintentionally after
>>    the policy is loaded (they represent a potential extra attack surface
>>    that should be mitigated).
>> 2. Despite the kernel being treated as a privileged domain, the policy
>>    writer may want to impose certain special limitations on kernel
>>    threads that may conflict with the requirements of intentional early
>>    boot processes. For example, it is a good hardening practice to limit
>>    what executables the kernel can execute as usermode helpers and to
>>    confine the resulting usermode helper processes. However, a
>>    (legitimate) process surviving from early boot may need to execute a
>>    different set of executables.
>> 3. As currently implemented, overlayfs remembers the security context of
>>    the process that created an overlayfs mount and uses it to bound
>>    subsequent operations on files using this context. If an overlayfs
>>    mount is created before the SELinux policy is loaded, these "mounter"
>>    checks are made against the kernel context, which may clash with
>>    restrictions on the kernel domain (see 2.).
>>
>> To resolve this, introduce a new initial SID (reusing the slot of the
>> former "init" initial SID) that will be assigned to any userspace
>> process started before the policy is first loaded. This is easy to do,
>> as we can simply label any process that goes through the
>> bprm_creds_for_exec LSM hook with the new init-SID instead of
>> propagating the kernel SID from the parent.
>>
>> To provide backwards compatibility for existing policies that are
>> unaware of this new semantic of the "init" initial SID, introduce a new
>> policy capability "userspace_initial_context" and set the "init" SID to
>> the same context as the "kernel" SID unless this capability is set by
>> the policy.
>>
>> Another small backwards compatibility measure is needed in
>> security_sid_to_context_core() for before the initial SELinux policy
>> load - see the code comment for explanation.
>>
>> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>> ---
>>
>> The last version of this patch has been applied [1] and then reverted
>> [2] because of a regression breaking userspace before the policy is
>> loaded [3]. This version fixes it using Christian's suggestion [4].
>>
>> Paul's suggestion from [5] isn't really viable because both existing
>> users of security_get_initial_sid_context() need the current behavior.
>>
>> As for security_context_to_sid_core(), it doesn't seem desirable to
>> remove the ability to e.g. write "init" to /proc/self/attr/exec to
>> get the exec_sid to SECINITSID_INIT as that shouldn't affect backwards
>> compatibility. However, the caveat is that after setting something to
>> SECINITSID_INIT via "init", a query for the string form of the context
>> will return the "fake" value "kernel" instead of the expected "init".
>>
>> [1] 5b0eea835d4e ("selinux: introduce an initial SID for early boot processes")
>> [2] 817199e006e5 ("selinux: revert SECINITSID_INIT support")
>> [3] https://lore.kernel.org/selinux/87edkseqf8.fsf@mail.lhotse/
>> [4] https://lore.kernel.org/selinux/CAJ2a_DdZHFTHiRu5+ZENAwUq1Cor-jVoE9qdhb2x5uSej-MaRA@mail.gmail.com/
>> [5] https://lore.kernel.org/selinux/CAHC9VhQVQv78Xr1bDoJcuT5s441Lvc8AanMvMh8KXG-PuPTVAQ@mail.gmail.com/
>>
>>  security/selinux/hooks.c                      | 28 +++++++++++++++++++
>>  .../selinux/include/initial_sid_to_string.h   |  2 +-
>>  security/selinux/include/policycap.h          |  1 +
>>  security/selinux/include/policycap_names.h    |  1 +
>>  security/selinux/include/security.h           |  6 ++++
>>  security/selinux/ss/policydb.c                | 27 ++++++++++++++++++
>>  security/selinux/ss/services.c                | 13 ++++++++-
>>  7 files changed, 76 insertions(+), 2 deletions(-)
>
> Ondrej or Michael, can either of you confirm that the regression found
> in previous revisions of this patch have been resolved in v3?

Yeah this works for me with the same test case that previously failed.

Tested-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

  parent reply	other threads:[~2023-11-22 23:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-14 15:51 [PATCH v3] selinux: introduce an initial SID for early boot processes Ondrej Mosnacek
2023-11-21  1:35 ` Paul Moore
2023-11-21 12:09   ` Ondrej Mosnacek
2023-11-22 23:59   ` Michael Ellerman [this message]
2023-11-21 14:00 ` Stephen Smalley
2023-11-21 14:58   ` Ondrej Mosnacek
2023-11-21 23:46     ` Paul Moore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a5r5s5jo.fsf@mail.lhotse \
    --to=mpe@ellerman.id.au \
    --cc=cgzones@googlemail.com \
    --cc=omosnace@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.