Commit Graph

22 Commits

Author SHA1 Message Date
Thomas Cedeno
03ca0ec138 LSM: SafeSetID: Fix warnings reported by test bot
Fix multiple cast-to-union warnings related to casting kuid_t and kgid_t
types to kid_t union type. Also fix incompatible type warning that
arises from accidental omission of "__rcu" qualifier on the struct
setid_ruleset pointer in the argument list for safesetid_file_read().

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Thomas Cedeno <thomascedeno@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2020-10-13 09:17:36 -07:00
Thomas Cedeno
5294bac97e LSM: SafeSetID: Add GID security policy handling
The SafeSetID LSM has functionality for restricting setuid() calls based
on its configured security policies. This patch adds the analogous
functionality for setgid() calls. This is mostly a copy-and-paste change
with some code deduplication, plus slight modifications/name changes to
the policy-rule-related structs (now contain GID rules in addition to
the UID ones) and some type generalization since SafeSetID now needs to
deal with kgid_t and kuid_t types.

Signed-off-by: Thomas Cedeno <thomascedeno@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2020-10-13 09:17:35 -07:00
Paul E. McKenney
a60a574600 security/safesetid: Replace rcu_swap_protected() with rcu_replace_pointer()
This commit replaces the use of rcu_swap_protected() with the more
intuitively appealing rcu_replace_pointer() as a step towards removing
rcu_swap_protected().

Link: https://lore.kernel.org/lkml/CAHk-=wiAsJLw1egFEE=Z7-GGtM6wcvtyytXZA1+BHqta4gg6Hw@mail.gmail.com/
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Reported-by: kbuild test robot <lkp@intel.com>
[ paulmck: From rcu_replace() to rcu_replace_pointer() per Ingo Molnar. ]
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Micah Morton <mortonm@chromium.org>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: <linux-security-module@vger.kernel.org>
2019-10-30 08:45:57 -07:00
Micah Morton
21ab8580b3 LSM: SafeSetID: Stop releasing uninitialized ruleset
The first time a rule set is configured for SafeSetID, we shouldn't be
trying to release the previously configured ruleset, since there isn't
one. Currently, the pointer that would point to a previously configured
ruleset is uninitialized on first rule set configuration, leading to a
crash when we try to call release_ruleset with that pointer.

Acked-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-09-17 11:27:05 -07:00
Jann Horn
e10337daef LSM: SafeSetID: fix use of literal -1 in capable hook
The capable() hook returns an error number. -EPERM is actually the same as
-1, so this doesn't make a difference in behavior.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:08:03 -07:00
Jann Horn
4f72123da5 LSM: SafeSetID: verify transitive constrainedness
Someone might write a ruleset like the following, expecting that it
securely constrains UID 1 to UIDs 1, 2 and 3:

    1:2
    1:3

However, because no constraints are applied to UIDs 2 and 3, an attacker
with UID 1 can simply first switch to UID 2, then switch to any UID from
there. The secure way to write this ruleset would be:

    1:2
    1:3
    2:2
    3:3

, which uses "transition to self" as a way to inhibit the default-allow
policy without allowing anything specific.

This is somewhat unintuitive. To make sure that policy authors don't
accidentally write insecure policies because of this, let the kernel verify
that a new ruleset does not contain any entries that are constrained, but
transitively unconstrained.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:07:51 -07:00
Jann Horn
fbd9acb2dc LSM: SafeSetID: add read handler
For debugging a running system, it is very helpful to be able to see what
policy the system is using. Add a read handler that can dump out a copy of
the loaded policy.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:07:40 -07:00
Jann Horn
03638e62f5 LSM: SafeSetID: rewrite userspace API to atomic updates
The current API of the SafeSetID LSM uses one write() per rule, and applies
each written rule instantly. This has several downsides:

 - While a policy is being loaded, once a single parent-child pair has been
   loaded, the parent is restricted to that specific child, even if
   subsequent rules would allow transitions to other child UIDs. This means
   that during policy loading, set*uid() can randomly fail.
 - To replace the policy without rebooting, it is necessary to first flush
   all old rules. This creates a time window in which no constraints are
   placed on the use of CAP_SETUID.
 - If we want to perform sanity checks on the final policy, this requires
   that the policy isn't constructed in a piecemeal fashion without telling
   the kernel when it's done.

Other kernel APIs - including things like the userns code and netfilter -
avoid this problem by performing updates atomically. Luckily, SafeSetID
hasn't landed in a stable (upstream) release yet, so maybe it's not too
late to completely change the API.

The new API for SafeSetID is: If you want to change the policy, open
"safesetid/whitelist_policy" and write the entire policy,
newline-delimited, in there.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:07:29 -07:00
Jann Horn
71a98971b9 LSM: SafeSetID: fix userns handling in securityfs
Looking at current_cred() in write handlers is bad form, stop doing that.

Also, let's just require that the write is coming from the initial user
namespace. Especially SAFESETID_WHITELIST_FLUSH requires privilege over all
namespaces, and SAFESETID_WHITELIST_ADD should probably require it as well.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:07:19 -07:00
Jann Horn
78ae7df96d LSM: SafeSetID: refactor policy parsing
In preparation for changing the policy parsing logic, refactor the line
parsing logic to be less verbose and move it into a separate function.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:07:09 -07:00
Jann Horn
8068866c4a LSM: SafeSetID: refactor safesetid_security_capable()
At the moment, safesetid_security_capable() has two nested conditional
blocks, and one big comment for all the logic. Chop it up and reduce the
amount of indentation.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:06:58 -07:00
Jann Horn
1cd02a27a9 LSM: SafeSetID: refactor policy hash table
parent_kuid and child_kuid are kuids, there is no reason to make them
uint64_t. (And anyway, in the kernel, the normal name for that would be
u64, not uint64_t.)

check_setuid_policy_hashtable_key() and
check_setuid_policy_hashtable_key_value() are basically the same thing,
merge them.

Also fix the comment that claimed that (1<<8)==128.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:05:48 -07:00
Jann Horn
7ef6b3062f LSM: SafeSetID: fix check for setresuid(new1, new2, new3)
With the old code, when a process with the (real,effective,saved) UID set
(1,1,1) calls setresuid(2,3,4), safesetid_task_fix_setuid() only checks
whether the transition 1->2 is permitted; the transitions 1->3 and 1->4 are
not checked. Fix this.

This is also a good opportunity to refactor safesetid_task_fix_setuid() to
be less verbose - having one branch per set*uid() syscall is unnecessary.

Note that this slightly changes semantics: The UID transition check for
UIDs that were not in the old cred struct is now always performed against
the policy of the RUID. I think that's more consistent anyway, since the
RUID is also the one that decides whether any policy is enforced at all.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:05:37 -07:00
Jann Horn
c783d525f9 LSM: SafeSetID: fix pr_warn() to include newline
Fix the pr_warn() calls in the SafeSetID LSM to have newlines at the end.
Without this, denial messages will be buffered as incomplete lines in
log_output(), and will then only show up once something else prints into
dmesg.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
2019-07-15 08:05:26 -07:00
Linus Torvalds
5ad18b2e60 Merge branch 'siginfo-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace
Pull force_sig() argument change from Eric Biederman:
 "A source of error over the years has been that force_sig has taken a
  task parameter when it is only safe to use force_sig with the current
  task.

  The force_sig function is built for delivering synchronous signals
  such as SIGSEGV where the userspace application caused a synchronous
  fault (such as a page fault) and the kernel responded with a signal.

  Because the name force_sig does not make this clear, and because the
  force_sig takes a task parameter the function force_sig has been
  abused for sending other kinds of signals over the years. Slowly those
  have been fixed when the oopses have been tracked down.

  This set of changes fixes the remaining abusers of force_sig and
  carefully rips out the task parameter from force_sig and friends
  making this kind of error almost impossible in the future"

* 'siginfo-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace: (27 commits)
  signal/x86: Move tsk inside of CONFIG_MEMORY_FAILURE in do_sigbus
  signal: Remove the signal number and task parameters from force_sig_info
  signal: Factor force_sig_info_to_task out of force_sig_info
  signal: Generate the siginfo in force_sig
  signal: Move the computation of force into send_signal and correct it.
  signal: Properly set TRACE_SIGNAL_LOSE_INFO in __send_signal
  signal: Remove the task parameter from force_sig_fault
  signal: Use force_sig_fault_to_task for the two calls that don't deliver to current
  signal: Explicitly call force_sig_fault on current
  signal/unicore32: Remove tsk parameter from __do_user_fault
  signal/arm: Remove tsk parameter from __do_user_fault
  signal/arm: Remove tsk parameter from ptrace_break
  signal/nds32: Remove tsk parameter from send_sigtrap
  signal/riscv: Remove tsk parameter from do_trap
  signal/sh: Remove tsk parameter from force_sig_info_fault
  signal/um: Remove task parameter from send_sigtrap
  signal/x86: Remove task parameter from send_sigtrap
  signal: Remove task parameter from force_sig_mceerr
  signal: Remove task parameter from force_sig
  signal: Remove task parameter from force_sigsegv
  ...
2019-07-08 21:48:15 -07:00
Eric W. Biederman
3cf5d076fb signal: Remove task parameter from force_sig
All of the remaining callers pass current into force_sig so
remove the task parameter to make this obvious and to make
misuse more difficult in the future.

This also makes it clear force_sig passes current into force_sig_info.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
2019-05-27 09:36:28 -05:00
Thomas Gleixner
ec8f24b7fa treewide: Add SPDX license identifier - Makefile/Kconfig
Add SPDX license identifiers to all Make/Kconfig files which:

 - Have no license information of any form

These files fall under the project license, GPL v2 only. The resulting SPDX
license identifier is:

  GPL-2.0-only

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-05-21 10:50:46 +02:00
Wei Yongjun
e7a44cfd63 LSM: fix return value check in safesetid_init_securityfs()
In case of error, the function securityfs_create_dir() returns ERR_PTR()
and never returns NULL. The NULL test in the return value check should
be replaced with IS_ERR().

Fixes: aeca4e2ca6 ("LSM: add SafeSetID module that gates setid calls")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: James Morris <james.morris@microsoft.com>
2019-02-12 10:59:22 -08:00
Micah Morton
2181e084b2 LSM: SafeSetID: remove unused include
The include for asm/syscall.h was needed in a prior version of lsm.c
that checked return values of syscall_get_nr, but since we did away with
that part of the code this include is no longer necessary. Take out this
include since it breaks builds for certain architectures. We no longer
have any arch-specific code in SafeSetID.

Signed-off-by: Micah Morton <mortonm@chromium.org>
Signed-off-by: James Morris <james.morris@microsoft.com>
2019-01-30 12:29:53 -08:00
Micah Morton
2f87324be7 LSM: SafeSetID: 'depend' on CONFIG_SECURITY
This patch changes the Kconfig file for the SafeSetID LSM to depend on
CONFIG_SECURITY as well as select CONFIG_SECURITYFS, since the policies
for the LSM are configured through writing to securityfs.

Signed-off-by: Micah Morton <mortonm@chromium.org>
Signed-off-by: James Morris <james.morris@microsoft.com>
2019-01-29 11:50:48 -08:00
Micah Morton
f67e20d20f LSM: Add 'name' field for SafeSetID in DEFINE_LSM
Without this, system boot was crashing with:

[0.174285] LSM: Security Framework initializing
[0.175277] BUG: unable to handle kernel NULL pointer dereference
...
[0.176272] Call Trace:
[0.176272]  ordered_lsm_parse+0x112/0x20b
[0.176272]  security_init+0x9b/0x3ab
[0.176272]  start_kernel+0x413/0x479
[0.176272]  secondary_startup_64+0xa4/0xb0

Signed-off-by: Micah Morton <mortonm@chromium.org>
Fixed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: James Morris <james.morris@microsoft.com>
2019-01-28 14:06:50 -08:00
Micah Morton
aeca4e2ca6 LSM: add SafeSetID module that gates setid calls
SafeSetID gates the setid family of syscalls to restrict UID/GID
transitions from a given UID/GID to only those approved by a
system-wide whitelist. These restrictions also prohibit the given
UIDs/GIDs from obtaining auxiliary privileges associated with
CAP_SET{U/G}ID, such as allowing a user to set up user namespace UID
mappings. For now, only gating the set*uid family of syscalls is
supported, with support for set*gid coming in a future patch set.

Signed-off-by: Micah Morton <mortonm@chromium.org>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: James Morris <james.morris@microsoft.com>
2019-01-25 11:22:45 -08:00