First, the code is found to be irregular through checkpatch.pl.
Then I found break is really useless here.
Signed-off-by: Yanwei Gao <gaoyanwei.tx@gmail.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Yanwei Gao